Skip to content

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

Closed
IanMaquignaz wants to merge 9 commits intoopencv:3.4from
IanMaquignaz:fixDocumentation_Calib3D_ParameterReferences
Closed

Added '@ref' to Camera Calibration and 3D Reconstruction documentation#18872
IanMaquignaz wants to merge 9 commits intoopencv:3.4from
IanMaquignaz:fixDocumentation_Calib3D_ParameterReferences

Conversation

@IanMaquignaz
Copy link
Copy Markdown
Contributor

@IanMaquignaz IanMaquignaz commented Nov 20, 2020

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

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
force_builders_only=docs,linux

@IanMaquignaz
Copy link
Copy Markdown
Contributor Author

IanMaquignaz commented Nov 20, 2020

Not sure why the build failed... I only modified the Docs which built with the following warnings:

Couple of the warnings stem from half of this enum disapearing:
enum { CALIB_CB_ADAPTIVE_THRESH = 1, CALIB_CB_NORMALIZE_IMAGE = 2, CALIB_CB_FILTER_QUADS = 4, CALIB_CB_FAST_CHECK = 8, CALIB_CB_EXHAUSTIVE = 16, CALIB_CB_ACCURACY = 32, CALIB_CB_LARGER = 64, CALIB_CB_MARKER = 128 };
I assume running git rebase -i --onto upstream/3.4 upstream/master produced this result?

@alalek
Copy link
Copy Markdown
Member

alalek commented Nov 20, 2020

what the failed

Save current work and perform step-by-step small updates. There are too many changes on Diff page.

@asmorkalov asmorkalov added the category: documentation Documentation fix or update label Nov 20, 2020
However, if not all of the point pairs ( \f$srcPoints_i\f$, \f$dstPoints_i\f$ ) fit the rigid perspective
transformation (that is, there are some outliers), this initial estimate will be poor. In this case,
you can use one of the three robust methods. The methods RANSAC, LMeDS and RHO try many different
you can use one of the three robust methods. The methods RANSAC, LMEDS and RHO try many different
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.

In my opinion, when referring to the method we should keep LMeDS/LMedS.

See Least Median of Squares from a tutorial by Zhengyou Zhang on Parameter Estimation Techniques: A Tutorial with Application to Conic Fitting, published in October 1995.

malliaridis and others added 8 commits November 20, 2020 18:42
- Add python explanation for erosion and dilation
- Add java explanation for erosion and dilation
- Restructure and reword specific sections
…enumerations with '#'. Could not add '@ref' to enumerations in '@code' blocks.
@IanMaquignaz
Copy link
Copy Markdown
Contributor Author

OK, so... code has been rebuilt locally a few times now and code builds fine with OpenCV master. The build issues seen here stem from the rebase (git rebase -i --onto upstream/3.4 upstream/master), where enums and parameters have changed to reflect what's shown below in 3.4.

In Master:
//! type of the robust estimation algorithm enum { LMEDS = 4, //!< least-median of squares algorithm RANSAC = 8, //!< RANSAC algorithm RHO = 16, //!< RHO algorithm USAC_DEFAULT = 32, //!< USAC algorithm, default settings USAC_PARALLEL = 33, //!< USAC, parallel version USAC_FM_8PTS = 34, //!< USAC, fundamental matrix 8 points USAC_FAST = 35, //!< USAC, fast settings USAC_ACCURATE = 36, //!< USAC, accurate settings USAC_PROSAC = 37, //!< USAC, sorted points, runs PROSAC USAC_MAGSAC = 38 //!< USAC, runs MAGSAC++ };

In 3.4:
//! type of the robust estimation algorithm enum { LMEDS = 4, //!< least-median of squares algorithm RANSAC = 8, //!< RANSAC algorithm RHO = 16 //!< RHO algorithm };

The question therefore is should I be rebasing to merge with 3.4? and if yes, how do I approach this? I've identified one USAC function declaration which appears without reference (see below), but thus far my attempts at a build without this header has only produced further errors downstream.

CV_EXPORTS_W Mat findEssentialMat( InputArray points1, InputArray points2, InputArray cameraMatrix1, InputArray cameraMatrix2, InputArray dist_coeff1, InputArray dist_coeff2, OutputArray mask, const UsacParams &params);

@alalek
Copy link
Copy Markdown
Member

alalek commented Nov 21, 2020

Need to create 2 PRs:

  • keep this PR for 3.4: common changes and specific for 3.4 version
  • separate PR for master: specific changes for master version (not necessary to duplicate "common" changes from 3.4 - they will be automatically merged during regular "Merge 3.4" process)

@IanMaquignaz IanMaquignaz force-pushed the fixDocumentation_Calib3D_ParameterReferences branch from f46557a to 2069535 Compare November 21, 2020 06:45
@IanMaquignaz
Copy link
Copy Markdown
Contributor Author

Need to create 2 PRs

If there's no way to merge my current working alterations in master into 3.4 without painstakingly re-editing calid3d.hpp's 3467 lines then I'm not sure I have the time to complete this task.

Alternatively, I've pushed my latest rebase revision which compiles locally for 3.4. Though functional, it reintroduces documentation for findChessboardCornersSB() which is currently omitted from 3.4. Is this latest commit something which would be acceptable? It can be tweaked, but if everything can be done in 3.4 then that would be ideal. No need to identify un-applied changes to redo in master.

@IanMaquignaz
Copy link
Copy Markdown
Contributor Author

IanMaquignaz commented Nov 23, 2020

@alalek how do you get the opencv-pushbot to rebuilt everything not just the docs? Docs are fine, just have warnings

@alalek
Copy link
Copy Markdown
Member

alalek commented Nov 23, 2020

findChessboardCornersSB()

should be removed from this patch (because it goes to 3.4 branch).
Look on "Files changed" tab on PR page and check for unrelated/unwanted changes.


rebuilt everything

I reduced testing scope (PR is under active development, so full scope is not really necessary).
Let fix docs and linux builders first.

@asmorkalov
Copy link
Copy Markdown
Contributor

@IanMaquignaz Do you have any progress with the patch?

@IanMaquignaz
Copy link
Copy Markdown
Contributor Author

@asmorkalov I have not had a chance to work on this since my last push. That being said, to accomplish the changes suggested by alalek, I need to create a fresh branch and restart.

I may have some time next week.

@IanMaquignaz
Copy link
Copy Markdown
Contributor Author

See new PR #19089

@asmorkalov
Copy link
Copy Markdown
Contributor

The patch is replaced with more focused PRs for 3.4 and master .

@asmorkalov asmorkalov closed this Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: documentation Documentation fix or update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants