Skip to content

Improve robustness for fitEllipseAMS#26810

Merged
asmorkalov merged 2 commits intoopencv:4.xfrom
MaximSmolskiy:improve-robustness-for-fitEllipseAMS
Jan 22, 2025
Merged

Improve robustness for fitEllipseAMS#26810
asmorkalov merged 2 commits intoopencv:4.xfrom
MaximSmolskiy:improve-robustness-for-fitEllipseAMS

Conversation

@MaximSmolskiy
Copy link
Copy Markdown
Contributor

Pull Request Readiness Checklist

Related to #26694

Added functionality to add noise to points in degenerate cases and try again for fitEllipseAMS. fitEllipseNoDirect and fitEllipseDirect already have this

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 another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the 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

@MaximSmolskiy MaximSmolskiy force-pushed the improve-robustness-for-fitEllipseAMS branch from 4f9d602 to c1cb5c8 Compare January 20, 2025 18:52
@asmorkalov
Copy link
Copy Markdown
Contributor

@MaximSmolskiy Thanks a lot for the contribution! Do you have couple of cases that are significantly improved by the patch or some quality metric/benchmark? The idea to get really random noise looks good, but the patch brings more changes and their effect is not obvious.

@MaximSmolskiy
Copy link
Copy Markdown
Contributor Author

MaximSmolskiy commented Jan 21, 2025

@MaximSmolskiy Thanks a lot for the contribution! Do you have couple of cases that are significantly improved by the patch or some quality metric/benchmark? The idea to get really random noise looks good, but the patch brings more changes and their effect is not obvious.

@asmorkalov Patch brings only changes about adding random noise to points if without that points configuration is quite degenerate. Current diff is scary, but if you enable Hide whitespace in Diff view - there won't be many changes.

I borrowed idea from fitEllipseDirect.
We move current calculations for construction of matrix M and checking its singularity by determinant into a loop of 2 iterations:

  1. original points
  2. after adding random noise to points

If matrix M is non-singular then we immediately break loop and continue original AMS algorithm calculations, if not then we continue to call fitEllipseNoDirect.

Degenerate case Imgproc_FitEllipseAMS_HorizontalLine was improved by patch - before we call fitEllipseNoDirect, but now after adding random noise to points original AMS algorithm copes on its own

@asmorkalov asmorkalov self-assigned this Jan 22, 2025
@asmorkalov asmorkalov added this to the 4.12.0 milestone Jan 22, 2025
@asmorkalov asmorkalov merged commit 8ab0ad6 into opencv:4.x Jan 22, 2025
@MaximSmolskiy MaximSmolskiy deleted the improve-robustness-for-fitEllipseAMS branch January 22, 2025 09:53
@asmorkalov asmorkalov mentioned this pull request Feb 19, 2025
NanQin555 pushed a commit to NanQin555/opencv that referenced this pull request Feb 24, 2025
…-for-fitEllipseAMS

Improve robustness for fitEllipseAMS opencv#26810

### Pull Request Readiness Checklist

Related to opencv#26694 

Added functionality to add noise to points in degenerate cases and try again for `fitEllipseAMS`. `fitEllipseNoDirect` and `fitEllipseDirect` already have this

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

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [x] There is a reference to the original bug report and related work
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
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.

2 participants