Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix aruco _filterTooCloseCandidates #2900

Merged
merged 1 commit into from
Apr 1, 2021
Merged

Conversation

spirraw
Copy link

@spirraw spirraw commented Mar 24, 2021

_filterTooCloseCandidates was occasionally not choosing the biggest
contour of a candidate group.

Firstly, if the biggest contour of a candidate group happened to be
the first candidate of that group, it would never be chosen due to the
comparison loop skipping it.

Secondly, contour sizes were being compared by their circumference
(via counting the number of points in a contour) rather than area.
In some cases it is possible for a smaller candidate to have more
points than a bigger one. This was fixed by comparing the contour
area as defined by the contour's corners.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake
force_builders=linux,docs

_filterTooCloseCandidates was occasionally not choosing the biggest
contour of a candidate group.

Firstly, if the biggest contour of a candidate group happened to be
the first candidate of that group, it would never be chosen due to the
comparison loop skipping it.

Secondly, contour sizes were being compared by their circumference
(via counting the number of points in a contour) rather than area.
In some cases it is possible for a smaller candidate to have more
points than a bigger one. This was fixed by comparing the contour
area as defined by the contour's corners.
@alalek
Copy link
Member

alalek commented Mar 24, 2021

@spirraw Thank you for contribution!
Could you please attach an image (lossless .png format is preferable) with this problem? So we can validate the fix.

/cc @szk1509 Could you please take a look?

@spirraw
Copy link
Author

spirraw commented Mar 24, 2021

Here is an image with which the issue should be reproducible.
raw
Here is a visualization of the contours that are passed to _filterTooCloseCandidates:
candidates
They are grouped correctly, however, because the inner candidate has more points than the outer one, it is incorrectly chosen as the biggest contour in the group.

@Chris-Bee
Copy link

This might be related to the incorrect settings of the threshold windows described here #2811 . Using the settings described there might solver your issue.

@spirraw
Copy link
Author

spirraw commented Apr 1, 2021

Prior to making this patch, I was also able to work around the problem by adjusting the adaptive thresholding parameters. Doing so eliminates the inner contours entirely, and as a result the bugs in _filterTooCloseCandidates are mitigated. While it is possible to to work around specific cases by adjusting the adaptive thresholding parameters, doing so may adversely affect performance in other cases. This patch is about fixing two underlying issues in _filterTooCloseCandidates, the function's entire purpose is to filter out these "inner contours".

@szk1509
Copy link
Contributor

szk1509 commented Apr 1, 2021

hi @spirraw :)
it all looks pretty reasonable, I haven't had the time to do the testing I normally do, but in broad strokes a big 👍

Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 👍

@opencv-pushbot opencv-pushbot merged commit a124db8 into opencv:master Apr 1, 2021
@alalek alalek mentioned this pull request Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants