Move contrib charuco to main objdetect by AleksandrPanov · Pull Request #22986 · opencv/opencv

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

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

matlabbe mentioned this pull request Aug 26, 2024

ROS2 humble on ARM, OpenCV charuco.hpp not found error introlab/rtabmap_ros#1200

Closed

Uh oh!

Conversation

AleksandrPanov commented Dec 19, 2022 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

asmorkalov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AleksandrPanov commented Dec 19, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asmorkalov commented Dec 20, 2022

Uh oh!

asmorkalov commented Dec 20, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asmorkalov commented Dec 22, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AleksandrPanov Dec 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AleksandrPanov Dec 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AleksandrPanov Dec 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alalek commented Dec 27, 2022

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

asmorkalov commented Dec 28, 2022

Uh oh!

AleksandrPanov commented Dec 19, 2022 •

edited by alalek

Loading

AleksandrPanov Dec 22, 2022 •

edited

Loading

AleksandrPanov Dec 23, 2022 •

edited

Loading

AleksandrPanov Dec 23, 2022 •

edited

Loading