Skip to content

[3.4] Fix cubic root computation in calibrateHandEye#18165

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
catree:fix_hand_eye_calibration_Andreff_NaN_3.4
Aug 25, 2020
Merged

[3.4] Fix cubic root computation in calibrateHandEye#18165
opencv-pushbot merged 1 commit intoopencv:3.4from
catree:fix_hand_eye_calibration_Andreff_NaN_3.4

Conversation

@catree
Copy link
Copy Markdown
Contributor

@catree catree commented Aug 21, 2020

Merge with extra: opencv/opencv_extra#800

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.
  • N/A The feature is well documented and sample code can be built with the project CMake

Specific fix for 3.4 branch since I don't want to have ugly code to test for C++11 features in master just because 3.4 branch is tied to obsolete < C++11 version.

@catree catree force-pushed the fix_hand_eye_calibration_Andreff_NaN_3.4 branch from f704c14 to 2dcb5be Compare August 22, 2020 20:44
Copy link
Copy Markdown
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 for contribution!

//Eq 15
double det = determinant(R);
R = pow(sign_double(det) / abs(det), 1.0/3.0) * R;
R = cubic_root(sign_double(det) / abs(det)) * R;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is wrong with cv:cubeRoot()?

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.

What is wrong with cv:cubeRoot()`?

Didn't know of this function.

For master, I still think it is better and cleaner to use C+11 std::cbrt since cubeRoot is only single-precision accuracy.
Feel free to close the master PR if it is easier to merge 3.4 in master.


The code does this:

image

@catree catree force-pushed the fix_hand_eye_calibration_Andreff_NaN_3.4 branch 2 times, most recently from 37f2bcc to 062ec7e Compare August 24, 2020 03:44
@catree catree force-pushed the fix_hand_eye_calibration_Andreff_NaN_3.4 branch from 062ec7e to 379b83e Compare August 25, 2020 01:38
Copy link
Copy Markdown
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.

Well done! Thank you for contribution 👍

@opencv-pushbot opencv-pushbot merged commit 1bea537 into opencv:3.4 Aug 25, 2020
@alalek alalek mentioned this pull request Aug 26, 2020
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.

3 participants