Move contrib charuco to main objdetect #22986

AleksandrPanov · 2022-12-19T07:59:03Z

move Charuco API from contrib to main repo:

add CharucoDetector:

CharucoDetector::detectBoard(InputArray image, InputOutputArrayOfArrays markerCorners, InputOutputArray markerIds, 
                             OutputArray charucoCorners, OutputArray charucoIds) const // detect charucoCorners and/or markerCorners
CharucoDetector::detectDiamonds(InputArray image, InputOutputArrayOfArrays _markerCorners,
                                InputOutputArrayOfArrays _markerIds, OutputArrayOfArrays _diamondCorners,
                                OutputArray _diamondIds) const

add matchImagePoints() for CharucoBoard
remove contrib aruco dependencies from interactive-calibration tool
move almost all aruco tests to objdetect

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

**WIP**
build_image:Docs=docs-js:18.04
Xbuild_contrib:Docs=OFF

build_image:Custom=javascript
buildworker:Custom=linux-1

asmorkalov

I propose:

Do not use inheritance for Charuco. There is no so much in common, but Charuco class gets "detectMarkers", "refineMarkers", etc
It makes sense to add constructor with cv::Ptr<ArucoDetector>
m.b. extract Charuco interface to dedicated header file.

modules/objdetect/include/opencv2/objdetect/aruco_detector.hpp

AleksandrPanov · 2022-12-19T12:08:22Z

I propose:

Do not use inheritance for Charuco. There is no so much in common, but Charuco class gets "detectMarkers", "refineMarkers", etc

It makes sense to add constructor with cv::Ptr<ArucoDetector>

m.b. extract Charuco interface to dedicated header file.

I use inheritance for CharucoDetector to skip setters/getters realisations (getDictionary/setDictionary/getDetectorParameters/setDetectorParameters/...)

apps/interactive-calibration/frameProcessor.cpp

modules/objdetect/src/aruco/aruco_detector.cpp

asmorkalov · 2022-12-20T08:29:23Z

alexander@paradox:/mnt/projects/Projects/OpenCV/opencv-build$ ./bin/opencv_interactive-calibration -t=charuco
Hot keys:
esc - exit application
s - save current data to .xml file
r - delete last frame
u - enable/disable applying undistortion
d - delete all frames
v - switch visualization
[ERROR:0@0.002] global persistence.cpp:505 open Can't open file: 'defaultConfig.xml' in read mode
Warning: Unable to open defaultConfig.xml Application started with default advanced parameters
terminate called after throwing an instance of 'cv::Exception'
  what():  OpenCV(4.6.0-dev) /mnt/projects/Projects/OpenCV/opencv-master/apps/interactive-calibration/main.cpp:107: error: (-213:The function/feature is not implemented) in function 'main'
> Aruco module is disabled in current build configuration. Consider usage of another calibration pattern
> 
Аварийный останов (стек памяти сброшен на диск)

There is old charuco check in main()

asmorkalov · 2022-12-20T08:43:08Z

Also it makes sense to add Charuco dictionary name as command line parameter and remove it from extended config. It's more obvious and handy solution.

modules/objdetect/include/opencv2/objdetect/aruco_board.hpp

modules/objdetect/include/opencv2/objdetect/charuco_detector.hpp

apps/interactive-calibration/frameProcessor.cpp

modules/objdetect/include/opencv2/objdetect/charuco_detector.hpp

asmorkalov · 2022-12-22T11:04:33Z

apps/interactive-calibration/parametersController.cpp

@@ -134,7 +128,7 @@ bool calib::parametersController::loadFromParser(cv::CommandLineParser &parser)
    }
    else if(templateType.find("charuco", 0) == 0) {
        mCapParams.board = chAruco;
-        mCapParams.boardSize = cv::Size(6, 8);
+        mCapParams.boardSize = cv::Size(5, 7);


no need to touch it.

After fix the vars h/w is setting num corners for CharucoBard (like in chessboard)
After fix CharucoBoard create looks like this:
cv::aruco::CharucoBoard::create(mBoardSize.width + 1, mBoardSize.height + 1, capParams.charucoSquareLength,
capParams.charucoMarkerSize, mArucoDictionary);

mCapParams.boardSize = cv::Size(5, 7); was fixed to support compatibility with old default parameters

modules/objdetect/include/opencv2/objdetect/charuco_detector.hpp

asmorkalov · 2022-12-22T11:50:07Z

@alalek The PR is almost ready for merge. Could you take a look?

alalek · 2022-12-22T12:19:14Z

modules/objdetect/include/opencv2/objdetect/charuco_detector.hpp

+namespace aruco {
+
+//! @addtogroup aruco
+//! @{


Documentation is still broken since #22368 .

There is no aruco group: http://pullrequest.opencv.org/buildbot/export/pr/22986/docs/d5/d54/group__objdetect.html

Now this looks like this:

Is it ok?
https://pullrequest.opencv.org/buildbot/export/pr/22986/docs/d5/d54/group__objdetect.html

alalek · 2022-12-22T12:22:10Z

modules/objdetect/include/opencv2/objdetect/aruco_board.hpp

+     * @param imgPoints Vector of vectors of the projections of board marker corner points.
+     */
+    CV_WRAP void matchImagePoints(InputArrayOfArrays detectedCorners, InputArray detectedIds,
+                                  OutputArray objPoints, OutputArray imgPoints) const override;
 protected:
    struct CharucoImpl;


class CV_EXPORTS_W CharucoBoard : public Board {

How many PImpl fields in this class with such inheritance?

Must be one (for the discussion above).

Do you suggest make PImpl fields as private and prevent class inheritors using this field? In the ImageCollection (imgcodecs), Model (dnn), IntelligentScissorsMB classes PImpl fields set as protected and can be used by the class's inheritors.

private

Where is it suggested?

They must reuse the single "impl" field instead of adding a new one.

See Model (dnn) inheritance pattern.

fields GridImpl and CharucoImpl were removed from GridBoard, CharucoBoard

So, change in virtual void matchImagePoints() is not needed anymore.

BTW, too many unnecessary Ptr<> in implementation, especially without NULL checks.
Reference should be used instead.

change most Ptr<> to reference

@alalek, do you want me to remove the virtuality for the function matchImagePoints() and generateImage()? Do you want me to use virtuality into Pimpl implementation?

I want this kind of code to work correctly:

cv::Ptr<cv::aruco::Board> board = cv::aruco::CharucoBoard::create(3, 3); std::vector<cv::Point2f> a1; std::vector<int> a2; std::vector<cv::Point2f> a3; std::vector<cv::Point2f> a4; board->matchImagePoints(a1, a2, a3, a4);

without virtual in matchImagePoints() I need to create func virtual void pimplMatchImagePoints() and call this from main Boards classes

Should I do it?

This done - remove public virtual API for Board classes

asmorkalov · 2022-12-22T15:04:57Z

modules/objdetect/include/opencv2/objdetect/aruco_detector.hpp

+/** @defgroup objdetect_aruco
+ * ArUco Marker Detection


/build/precommit_docs/4.x/opencv/modules/objdetect/include/opencv2/objdetect/aruco_detector.hpp:13: warning: missing title after \defgroup objdetect_aruco

See how it looks in final docs: https://pullrequest.opencv.org/buildbot/export/pr/22986/docs/d5/d54/group__objdetect.html

Now this looks like this:

Is it ok?
https://pullrequest.opencv.org/buildbot/export/pr/22986/docs/d5/d54/group__objdetect.html

No dots in headers

"ArUco" will better

modules/objdetect/include/opencv2/objdetect.hpp

modules/objdetect/include/opencv2/objdetect/aruco_board.hpp

alalek · 2022-12-27T11:32:37Z

@asmorkalov Is there final approval?

modules/objdetect/include/opencv2/objdetect/aruco_board.hpp

alalek · 2022-12-27T17:12:36Z

modules/objdetect/include/opencv2/objdetect/aruco_board.hpp

+protected:
+    Board(Ptr<BoardImpl> impl);
+    template<typename T> T& as() { CV_Assert(boardImpl); return static_cast<T&>(*boardImpl); }
+    template<typename T> const T& as() const { CV_Assert(boardImpl); return static_cast<const T&>(*boardImpl); }


This could be thrown away from public headers:

See TextRecognitionModel_Impl::from(impl)

modules/objdetect/include/opencv2/objdetect/aruco_board.hpp

asmorkalov · 2022-12-28T08:23:54Z

@alalek Ready for review

…uco_to_main_objdetect merge with opencv/opencv_contrib#3394 move Charuco API from contrib to main repo: - add CharucoDetector: ``` CharucoDetector::detectBoard(InputArray image, InputOutputArrayOfArrays markerCorners, InputOutputArray markerIds, OutputArray charucoCorners, OutputArray charucoIds) const // detect charucoCorners and/or markerCorners CharucoDetector::detectDiamonds(InputArray image, InputOutputArrayOfArrays _markerCorners, InputOutputArrayOfArrays _markerIds, OutputArrayOfArrays _diamondCorners, OutputArray _diamondIds) const ``` - add `matchImagePoints()` for `CharucoBoard` - remove contrib aruco dependencies from interactive-calibration tool - move almost all aruco tests to objdetect ### Pull Request Readiness Checklist 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

asmorkalov reviewed Dec 19, 2022

View reviewed changes

modules/objdetect/include/opencv2/objdetect/aruco_detector.hpp Outdated Show resolved Hide resolved

modules/objdetect/include/opencv2/objdetect/aruco_detector.hpp Outdated Show resolved Hide resolved

AleksandrPanov force-pushed the move_contrib_charuco_to_main_objdetect branch from 492d8c7 to 2e12af2 Compare December 19, 2022 11:49

AleksandrPanov mentioned this pull request Dec 19, 2022

Move contrib charuco to main objdetect opencv/opencv_contrib#3394

Merged

6 tasks

AleksandrPanov changed the title ~~add new CharucoDetector API from contrib~~ Move contrib charuco to main objdetect Dec 19, 2022

AleksandrPanov force-pushed the move_contrib_charuco_to_main_objdetect branch from f767839 to 8304d06 Compare December 19, 2022 21:50

AleksandrPanov added category: objdetect test labels Dec 19, 2022

AleksandrPanov force-pushed the move_contrib_charuco_to_main_objdetect branch from f226e34 to 9de448c Compare December 20, 2022 07:52

asmorkalov reviewed Dec 20, 2022

View reviewed changes

AleksandrPanov force-pushed the move_contrib_charuco_to_main_objdetect branch from 9de448c to 2eeae14 Compare December 20, 2022 09:12

AleksandrPanov added 3 commits December 20, 2022 14:47

move charuco, add new CharucoDetector API

99d3bc7

fix interactive-calibration tool

319d71f

add matchImagePoints() for Charuco

5be4c66

AleksandrPanov force-pushed the move_contrib_charuco_to_main_objdetect branch from eb0d580 to 5be4c66 Compare December 20, 2022 11:47

fix interactive-calibration tool

0b8e274

AleksandrPanov force-pushed the move_contrib_charuco_to_main_objdetect branch from dd0f91e to 0b8e274 Compare December 20, 2022 14:52

AleksandrPanov added 2 commits December 20, 2022 23:59

create charuco_detector.cpp/hpp

20f2f7e

fixes

56d17ff

AleksandrPanov force-pushed the move_contrib_charuco_to_main_objdetect branch from 1919f7d to 56d17ff Compare December 20, 2022 22:07

AleksandrPanov added 2 commits December 21, 2022 02:41

add detectDiamonds

0e56eee

move aruco tests to objdetect

f4041a2

AleksandrPanov force-pushed the move_contrib_charuco_to_main_objdetect branch from c59db63 to f4041a2 Compare December 21, 2022 00:36

AleksandrPanov marked this pull request as ready for review December 21, 2022 00:49

AleksandrPanov added 2 commits December 21, 2022 10:16

fix solvePnP

dcd11e4

move aruco perf tests to objdetect

100cc55

AleksandrPanov force-pushed the move_contrib_charuco_to_main_objdetect branch from d1a7317 to 100cc55 Compare December 21, 2022 07:40

asmorkalov reviewed Dec 21, 2022

View reviewed changes

asmorkalov reviewed Dec 22, 2022

View reviewed changes

asmorkalov requested a review from alalek December 22, 2022 11:48

fix docs

50e0fb4

alalek reviewed Dec 22, 2022

View reviewed changes

asmorkalov reviewed Dec 22, 2022

View reviewed changes

try fix doc

ffe588f

AleksandrPanov force-pushed the move_contrib_charuco_to_main_objdetect branch from 63b8512 to ffe588f Compare December 22, 2022 16:11

remove public GridImpl, CharucoImpl

1f214f2

AleksandrPanov force-pushed the move_contrib_charuco_to_main_objdetect branch from fdeb045 to 1f214f2 Compare December 22, 2022 18:58

AleksandrPanov added 2 commits December 23, 2022 11:11

fix docs

cb3aa2f

change Ptr<> to reference

0ed1de9

AleksandrPanov force-pushed the move_contrib_charuco_to_main_objdetect branch from 6b6d1b9 to 0ed1de9 Compare December 23, 2022 09:23

return np.float32

24bc756

asmorkalov approved these changes Dec 23, 2022

View reviewed changes

modules/objdetect/include/opencv2/objdetect.hpp Outdated Show resolved Hide resolved

modules/objdetect/include/opencv2/objdetect/aruco_board.hpp Outdated Show resolved Hide resolved

AleksandrPanov added 2 commits December 23, 2022 14:51

fix docs

c82d524

remove public virtual API for Board classes

88e10ee

alalek assigned asmorkalov Dec 26, 2022

Got rid of ::create for aruco boards.

db2e081

alalek reviewed Dec 27, 2022

View reviewed changes

asmorkalov added 3 commits December 27, 2022 22:15

Bindings fix for Aruco boards.

68b6ec2

Code review fixes.

8cd2949

Code review fixes.

3f0a507

Cleanup

2fce006

asmorkalov merged commit 1210348 into opencv:4.x Dec 28, 2022

alalek mentioned this pull request Jan 8, 2023

(5.x) Merge 4.x #23113

Merged

hacker1024 mentioned this pull request Jan 8, 2024

OpenCV 4.7.0 fictionlab/ros_aruco_opencv#27

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move contrib charuco to main objdetect #22986

Move contrib charuco to main objdetect #22986

AleksandrPanov commented Dec 19, 2022 •

edited by alalek

asmorkalov left a comment

AleksandrPanov commented Dec 19, 2022

asmorkalov commented Dec 20, 2022

asmorkalov commented Dec 20, 2022

asmorkalov Dec 22, 2022

AleksandrPanov Dec 22, 2022

asmorkalov commented Dec 22, 2022

alalek Dec 22, 2022

AleksandrPanov Dec 22, 2022

alalek Dec 22, 2022

AleksandrPanov Dec 22, 2022 •

edited

alalek Dec 22, 2022

AleksandrPanov Dec 22, 2022

alalek Dec 23, 2022

AleksandrPanov Dec 23, 2022

AleksandrPanov Dec 23, 2022

AleksandrPanov Dec 23, 2022 •

edited

AleksandrPanov Dec 23, 2022 •

edited

asmorkalov Dec 22, 2022

asmorkalov Dec 22, 2022

AleksandrPanov Dec 22, 2022

asmorkalov Dec 23, 2022

asmorkalov Dec 23, 2022

AleksandrPanov Dec 23, 2022

alalek commented Dec 27, 2022

alalek Dec 27, 2022

asmorkalov commented Dec 28, 2022

Move contrib charuco to main objdetect #22986

Move contrib charuco to main objdetect #22986

Conversation

AleksandrPanov commented Dec 19, 2022 • edited by alalek

Pull Request Readiness Checklist

asmorkalov left a comment

Choose a reason for hiding this comment

AleksandrPanov commented Dec 19, 2022

asmorkalov commented Dec 20, 2022

asmorkalov commented Dec 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asmorkalov commented Dec 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AleksandrPanov Dec 22, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AleksandrPanov Dec 23, 2022 • edited

Choose a reason for hiding this comment

AleksandrPanov Dec 23, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alalek commented Dec 27, 2022

Choose a reason for hiding this comment

asmorkalov commented Dec 28, 2022

AleksandrPanov commented Dec 19, 2022 •

edited by alalek

AleksandrPanov Dec 22, 2022 •

edited

AleksandrPanov Dec 23, 2022 •

edited

AleksandrPanov Dec 23, 2022 •

edited