Skip to content

Bugfix for solvePnPRansac with SOLVEPNP_ITERATIVE#22914

Merged
asmorkalov merged 1 commit intoopencv:3.4from
tozanski:tomoz/ransac-bugfix
Dec 22, 2022
Merged

Bugfix for solvePnPRansac with SOLVEPNP_ITERATIVE#22914
asmorkalov merged 1 commit intoopencv:3.4from
tozanski:tomoz/ransac-bugfix

Conversation

@tozanski
Copy link
Copy Markdown
Contributor

@tozanski tozanski commented Dec 3, 2022

The current implementation overwrites the result rotation and translation in every iteration. If SOLVEPNP_ITERATIVE was run as a refinement it will start from the incorrect initial transformation thus degrading the final outcome.

There is not bug report I am aware of nor any test or documentation that references the existence of this bug

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 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

The current implementation overwrites the result rotation and translation in every iteration.
If SOLVEPNP_ITERATIVE was run as a refinement it will start from the incorrect initial
transformation thus  degrading the final outcome.
@asmorkalov asmorkalov requested a review from vpisarev December 5, 2022 05:21
@asmorkalov asmorkalov added this to the 4.7.0 milestone Dec 5, 2022
@asmorkalov asmorkalov added the pr: needs test New functionality requires minimal tests set label Dec 5, 2022
@asmorkalov
Copy link
Copy Markdown
Contributor

@tozanski Could you add simple test or at least example, where the patch significantly improves result?

@asmorkalov asmorkalov requested a review from savuor December 12, 2022 12:47
@vpisarev
Copy link
Copy Markdown
Contributor

thank you! the whole solvePnPRansac looks broken to me, but your patch makes it a little less buggy :)

@asmorkalov asmorkalov removed the pr: needs test New functionality requires minimal tests set label Dec 22, 2022
@asmorkalov
Copy link
Copy Markdown
Contributor

I have not found simple way to test the fix. Merged as is.

@asmorkalov asmorkalov merged commit 44dfe62 into opencv:3.4 Dec 22, 2022
@alalek alalek mentioned this pull request Dec 24, 2022
@alalek alalek mentioned this pull request Jan 8, 2023
try
{
result = solvePnP(opoints_inliers, ipoints_inliers, cameraMatrix,
if (flags == SOLVEPNP_ITERATIVE && !useExtrinsicGuess)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need this condition ? Shouldn't that be for all methods ? Thx

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the issue only occurs forSOLVEPNP_ITERATIVE because AFAIK the other methods do not use extrinsic guess which was overwritten in each RANSAC iteration

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.

5 participants