fix type cast in drawDetectedMarkers, drawDetectedCornersCharuco, drawDetectedDiamonds by AleksandrPanov · Pull Request #24247 · opencv/opencv

AleksandrPanov · 2023-09-08T11:06:37Z

drawDetectedMarkers(), drawDetectedCornersCharuco(), drawDetectedDiamonds() uses only Point2f (CV_32FC2) for detected corners. But this corners are casted to int later in the code.
drawDetectedMarkers(), drawDetectedDiamonds() has a requirement on CV_32FC2 for corners. This requirement is lost in drawDetectedCornersCharuco().

Strict input data requirements have been removed. Added only the requirement to have 2 channels. The input corners are then casted to CV_32SC2.

added tests

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

asmorkalov · 2023-09-08T11:12:09Z

modules/objdetect/src/aruco/charuco_detector.cpp

              (_image.getMat().channels() == 1 || _image.getMat().channels() == 3));
    CV_Assert((_charucoCorners.getMat().total() == _charucoIds.getMat().total()) ||
              _charucoIds.getMat().total() == 0);
+    CV_Assert(_charucoCorners.getMat().type() == CV_32FC2);


I propose alternative:

CV_Assert(_charucoCorners.channels() == 2); ... _charucoCorners.getMat().convertTo(..., ...);

No need to limit type here. Just use proper conversions.

then I will remove the limits from all functions (drawDetectedMarkers, drawDetectedCornersCharuco, drawDetectedDiamonds)

asmorkalov · 2023-09-08T11:12:54Z

modules/objdetect/src/aruco/charuco_detector.cpp

              _charucoIds.getMat().total() == 0);
+    CV_Assert(_charucoCorners.getMat().type() == CV_32FC2);

    size_t nCorners = _charucoCorners.getMat().total();


InputArray has total() method. getMat() is redundant.

removed getMat()

asmorkalov · 2023-09-11T06:06:56Z

modules/objdetect/src/aruco/aruco_detector.cpp

        Mat currentMarker = _corners.getMat(i);
-        CV_Assert(currentMarker.total() == 4 && currentMarker.type() == CV_32FC2);
+        CV_Assert(currentMarker.total() == 4 && currentMarker.channels() == 2);
+        currentMarker.convertTo(currentMarker, CV_32SC2);


It makes sense, to check if the input is already 32SC2. convertTo does not check it.

added check:

if (currentMarker.type() != CV_32SC2) currentMarker.convertTo(currentMarker, CV_32SC2);

asmorkalov · 2023-09-11T06:08:03Z

modules/objdetect/src/aruco/charuco_detector.cpp

        Mat currentMarker = _corners.getMat(i);
-        CV_Assert(currentMarker.total() == 4 && currentMarker.type() == CV_32FC2);
+        CV_Assert(currentMarker.total() == 4 && currentMarker.channels() == 2);
+        currentMarker.convertTo(currentMarker, CV_32SC2);


The same for type check.

added check:

if (currentMarker.type() != CV_32SC2) currentMarker.convertTo(currentMarker, CV_32SC2);

asmorkalov · 2023-09-11T06:09:21Z

modules/objdetect/test/test_charucodetection.cpp

+    vector<vector<Point2d>> detected_double = {{Point2d(0., 0.), Point2d(10., 0.), Point2d(10., 10.), Point2d(0., 10.)}};
+    vector<vector<Point2d>> detected_int = {{Point(0, 0), Point(10, 0), Point2d(10, 10), Point2d(0, 10)}};


The same test type Point2d and Point2d. Should it be 2i in the second case?

fixed:
vector<vector<Point2i>> detected_int = {{Point(0, 0), Point(10, 0), Point(10, 10), Point(0, 10)}};

asmorkalov · 2023-09-11T17:03:01Z

modules/objdetect/test/test_charucodetection.cpp

+    for (size_t i = 0ull; i < 4ull; i++) {
+        m = moments(contours[i]);
+        detectedCenter = Point(cvRound(m.m10/m.m00), cvRound(m.m01/m.m00));
+        ASSERT_TRUE(find(detected_int[0].begin(), detected_int[0].end(), detectedCenter) != detected_int[0].end());


drawDetectedCornersCharuco is called for _float, but checked for _int. Also _double version is initialized, but not used.

detected_int and detected_float are equal, using int will help to avoid rounding errors
the "_double" version will be added

…onds added tests

asmorkalov

👍

AleksandrPanov added bug test category: objdetect labels Sep 8, 2023

asmorkalov reviewed Sep 8, 2023

View reviewed changes

asmorkalov added this to the 4.9.0 milestone Sep 8, 2023

AleksandrPanov force-pushed the fix_drawDetectedCornersCharuco_type_error branch from d65f3d3 to 093878a Compare September 8, 2023 14:12

AleksandrPanov changed the title ~~fix drawDetectedCornersCharuco type error~~ fix type cast in drawDetectedMarkers, drawDetectedCornersCharuco, drawDetectedDiamonds Sep 8, 2023

AleksandrPanov marked this pull request as draft September 8, 2023 15:53

AleksandrPanov force-pushed the fix_drawDetectedCornersCharuco_type_error branch 3 times, most recently from 4dde75d to b7b8183 Compare September 8, 2023 17:47

AleksandrPanov marked this pull request as ready for review September 8, 2023 17:58

AleksandrPanov force-pushed the fix_drawDetectedCornersCharuco_type_error branch 4 times, most recently from 4bfb003 to 587dc3f Compare September 9, 2023 09:38

asmorkalov reviewed Sep 11, 2023

View reviewed changes

AleksandrPanov force-pushed the fix_drawDetectedCornersCharuco_type_error branch from 587dc3f to fdec321 Compare September 11, 2023 07:56

AleksandrPanov marked this pull request as draft September 11, 2023 13:01

AleksandrPanov force-pushed the fix_drawDetectedCornersCharuco_type_error branch from fdec321 to 8f24080 Compare September 11, 2023 16:43

asmorkalov reviewed Sep 11, 2023

View reviewed changes

AleksandrPanov force-pushed the fix_drawDetectedCornersCharuco_type_error branch from 8f24080 to 100b163 Compare September 11, 2023 21:52

AleksandrPanov marked this pull request as ready for review September 11, 2023 21:53

fix drawDetectedCornersCharuco, drawDetectedMarkers, drawDetectedDiam…

ae1d1b6

…onds added tests

AleksandrPanov force-pushed the fix_drawDetectedCornersCharuco_type_error branch from 100b163 to ae1d1b6 Compare September 12, 2023 09:18

asmorkalov approved these changes Sep 12, 2023

View reviewed changes

asmorkalov merged commit 9761492 into opencv:4.x Sep 12, 2023

asmorkalov self-assigned this Sep 12, 2023

asmorkalov mentioned this pull request Sep 28, 2023

(5.x) Merge 4.x #24338

Merged

		vector<vector<Point2d>> detected_double = {{Point2d(0., 0.), Point2d(10., 0.), Point2d(10., 10.), Point2d(0., 10.)}};
		vector<vector<Point2d>> detected_int = {{Point(0, 0), Point(10, 0), Point2d(10, 10), Point2d(0, 10)}};

Uh oh!

Conversation

AleksandrPanov commented Sep 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

asmorkalov Sep 8, 2023 • 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!

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!

asmorkalov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AleksandrPanov commented Sep 8, 2023 •

edited

Loading

asmorkalov Sep 8, 2023 •

edited

Loading