Skip to content

Added '@ref' to 3.4 Camera Calibration and 3D Reconstruction documentation#19089

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
IanMaquignaz:fix_34_calib3d_parameterReferences
Dec 15, 2020
Merged

Added '@ref' to 3.4 Camera Calibration and 3D Reconstruction documentation#19089
opencv-pushbot merged 1 commit intoopencv:3.4from
IanMaquignaz:fix_34_calib3d_parameterReferences

Conversation

@IanMaquignaz
Copy link
Copy Markdown
Contributor

@IanMaquignaz IanMaquignaz commented Dec 12, 2020

Added '@ref' to enumerations to enable doxygen checks in Camera Calibration and 3D Reconstruction documentation.

Replaces #18872

Notes:

  • Did not check enumerations with '#'.
  • Could not add @ref to enumerations in @code blocks.

Observations (not addressed)

  • stereoCalibrate(...) and fisheye::stereoCalibrate(...) has random '?' in description? See here and here
    CALIB_FIX_INTRINSIC Fix cameraMatrix? and distCoeffs? so that only R, T, E, and F matrices are estimated.
  • fisheye::stereoCalibrate() has no enumeration option CALIB_ZERO_DISPARITY. See here

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

@IanMaquignaz
Copy link
Copy Markdown
Contributor Author

Build has one warning:
warning: unable to resolve reference to `fisheye::CALIB_ZERO_DISPARITY' for \ref command

There is no option for fisheye::CALIB_ZERO_DISPARITY in fisheye. See here for line

CALIB_FIX_INTRINSIC = 1 << 8,
CALIB_FIX_PRINCIPAL_POINT = 1 << 9
CALIB_FIX_PRINCIPAL_POINT = 1 << 9,
CALIB_ZERO_DISPARITY = 0x00400
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.

Fixes issue where fisheye::CALIB_ZERO_DISPARITY does not exit

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.

= 0x00400

= cv::CALIB_ZERO_DISPARITY

to ensure that the same value is used anywhere (implementation still uses cv::CALIB_ZERO_DISPARITY internally).

Copy link
Copy Markdown
Contributor Author

@IanMaquignaz IanMaquignaz Dec 14, 2020

Choose a reason for hiding this comment

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

That shows up weird in the docs.

Just testing this before pushing:
TEST_F(fisheyeTest, stereoRectify)
{
// For consistency purposes
CV_Assert(cv::CALIB_ZERO_DISPARITY == cv::fisheye::CALIB_ZERO_DISPARITY)

Is that ok? or is it better to have explicit docs

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.

I believe check is ok here. But please use CV_StaticAssert() for that (in C++11 it is compilation error)

@IanMaquignaz
Copy link
Copy Markdown
Contributor Author

I removed the force_builders_only=docs,linux as I think it no longer applies.

@asmorkalov asmorkalov self-requested a review December 15, 2020 07:25
Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

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

Looks good to me. Please squash commits and we can merge the patch then.

…fisheye::CALIB_ZERO_DISPARITY == cv::CALIB_ZERO_DISPARITY == 0x400 == 1 << 10.

Fisheye test has been updated to use new enum cv::fisheye::CALIB_ZERO_DISPARITY and included CV_StaticAssert(...) to ensure cv::CALIB_ZERO_DISPARITY == cv::fisheye::CALIB_ZERO_DISPARITY.
@IanMaquignaz IanMaquignaz force-pushed the fix_34_calib3d_parameterReferences branch from a897378 to 085a131 Compare December 15, 2020 17:37
@IanMaquignaz
Copy link
Copy Markdown
Contributor Author

done @asmorkalov

@asmorkalov asmorkalov self-requested a review December 15, 2020 18:21
Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov 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!

@opencv-pushbot opencv-pushbot merged commit 4107dc7 into opencv:3.4 Dec 15, 2020
@IanMaquignaz IanMaquignaz deleted the fix_34_calib3d_parameterReferences branch December 15, 2020 21:22
@param cameraMatrix Input/output 3x3 floating-point camera intrinsic matrix
\f$\cameramatrix{A}\f$ . If CV\_CALIB\_USE\_INTRINSIC\_GUESS
and/or CALIB_FIX_ASPECT_RATIO are specified, some or all of fx, fy, cx, cy must be
\f$\cameramatrix{A}\f$ . If @ref CV_CALIB_USE_INTRINSIC_GUESS
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.

fixup: #19154

@IanMaquignaz
Copy link
Copy Markdown
Contributor Author

Updated missed references in master #19230

@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

Labels

category: calib3d category: documentation Documentation fix or update port to 5.x is needed Label for maintainers. Authors of PR can ignore this

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants