Skip to content

fix extendDictionary#24228

Merged
asmorkalov merged 1 commit intoopencv:4.xfrom
AleksandrPanov:fix_extendDictionary
Sep 8, 2023
Merged

fix extendDictionary#24228
asmorkalov merged 1 commit intoopencv:4.xfrom
AleksandrPanov:fix_extendDictionary

Conversation

@AleksandrPanov
Copy link
Copy Markdown
Contributor

@AleksandrPanov AleksandrPanov commented Sep 5, 2023

Fixes #24222

Now, if the size of base Dictionary is higher than markers, only the first markers in base Dictionary are taken and no new marker is added.

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 asmorkalov added the pr: needs test New functionality requires minimal tests set label Sep 6, 2023
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Sep 6, 2023 

/home/ci/opencv/modules/objdetect/src/aruco/aruco_dictionary.cpp: In function 'cv::aruco::Dictionary cv::aruco::extendDictionary(int, int, const cv::aruco::Dictionary&, int)':
/home/ci/opencv/modules/objdetect/src/aruco/aruco_dictionary.cpp:374:123: error: cannot call constructor 'cv::Range::Range' directly [-fpermissive]
  374 |         out.bytesList = Mat(baseDictionary.bytesList.clone(), Range::Range(0, min(nMarkers, baseDictionary.bytesList.rows)));
      |                                                                                                                           ^
/home/ci/opencv/modules/objdetect/src/aruco/aruco_dictionary.cpp:374:123: note: for a function-style cast, remove the redundant '::Range'

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Sep 6, 2023

Also please add a test for it.

@AleksandrPanov AleksandrPanov force-pushed the fix_extendDictionary branch 3 times, most recently from 53d8638 to 13b4aff Compare September 6, 2023 18:39
asmorkalov
asmorkalov requested changes Sep 7, 2023
View reviewed changes
modules/objdetect/test/test_boarddetection.cpp Outdated

TEST(CV_ArucoDictionary, extendDictionary) {
aruco::Dictionary base_dictionary = aruco::getPredefinedDictionary(aruco::DICT_4X4_250);
ASSERT_EQ(base_dictionary.bytesList.rows, 250);
Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov Sep 7, 2023

Choose a reason for hiding this comment

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

The assertion does not make sense. It's constant in OpenCV with known size.

Copy link
Copy Markdown
Contributor Author

@AleksandrPanov AleksandrPanov Sep 7, 2023

Choose a reason for hiding this comment

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

removed

@asmorkalov asmorkalov added this to the 4.9.0 milestone Sep 7, 2023
@asmorkalov asmorkalov self-assigned this Sep 7, 2023
@AleksandrPanov AleksandrPanov reopened this Sep 7, 2023
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Sep 7, 2023 

FAILED: modules/objdetect/CMakeFiles/opencv_test_objdetect.dir/test/test_boarddetection.cpp.o 
/usr/bin/ccache /usr/bin/c++  -D_USE_MATH_DEFINES -D__OPENCV_BUILD=1 -D__OPENCV_TESTS=1 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/ci/opencv/3rdparty/quirc/include -I/home/ci/opencv/modules/ts/include -I/home/ci/opencv/modules/objdetect/include -I/home/ci/opencv/modules/imgcodecs/include -I/home/ci/opencv/modules/videoio/include -I/home/ci/opencv/modules/core/include -I/home/ci/opencv/modules/flann/include -I/home/ci/opencv/modules/imgproc/include -I/home/ci/opencv/modules/dnn/include -I/home/ci/opencv/modules/features2d/include -I/home/ci/opencv/modules/calib3d/include -I/home/ci/opencv/modules/highgui/include -isystem . -isystem /usr/include/eigen3 -fsigned-char -W -Wall -Wreturn-type -Wnon-virtual-dtor -Waddress -Wsequence-point -Wformat -Wformat-security -Wmissing-declarations -Wundef -Winit-self -Wpointer-arith -Wshadow -Wsign-promo -Wuninitialized -Wsuggest-override -Wno-delete-non-virtual-dtor -Wno-comment -Wimplicit-fallthrough=3 -Wno-strict-overflow -fdiagnostics-show-option -pthread -fomit-frame-pointer -ffunction-sections -fdata-sections    -fvisibility=hidden -fvisibility-inlines-hidden -O3 -DNDEBUG  -DNDEBUG -fPIE   -std=c++11 -MD -MT modules/objdetect/CMakeFiles/opencv_test_objdetect.dir/test/test_boarddetection.cpp.o -MF modules/objdetect/CMakeFiles/opencv_test_objdetect.dir/test/test_boarddetection.cpp.o.d -o modules/objdetect/CMakeFiles/opencv_test_objdetect.dir/test/test_boarddetection.cpp.o -c /home/ci/opencv/modules/objdetect/test/test_boarddetection.cpp
In file included from /home/ci/opencv/modules/ts/include/opencv2/ts.hpp:128,
                 from /home/ci/opencv/modules/objdetect/test/test_precomp.hpp:7,
                 from /home/ci/opencv/modules/objdetect/test/test_boarddetection.cpp:6:
/home/ci/opencv/modules/ts/include/opencv2/ts/ts_gtest.h: In instantiation of 'testing::AssertionResult testing::internal::CmpHelperEQ(const char*, const char*, const T1&, const T2&) [with T1 = int; T2 = cv::Mat]':
/home/ci/opencv/modules/ts/include/opencv2/ts/ts_gtest.h:21354:23:   required from 'static testing::AssertionResult testing::internal::EqHelper<lhs_is_null_literal>::Compare(const char*, const char*, const T1&, const T2&) [with T1 = int; T2 = cv::Mat; bool lhs_is_null_literal = false]'
/home/ci/opencv/modules/objdetect/test/test_boarddetection.cpp:326:5:   required from here
/home/ci/opencv/modules/ts/include/opencv2/ts/ts_gtest.h:21326:3: error: could not convert 'cv::operator==(double, const cv::Mat&)((* & rhs))' from 'cv::MatExpr' to 'bool'
21326 |   if (lhs == rhs) {
      |   ^~
      |   |
      |   cv::MatExpr

asmorkalov
asmorkalov approved these changes Sep 7, 2023
View reviewed changes
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.

👍

@AleksandrPanov AleksandrPanov added test and removed pr: needs test New functionality requires minimal tests set labels Sep 7, 2023
@asmorkalov asmorkalov merged commit 757d296 into opencv:4.x Sep 8, 2023
@asmorkalov asmorkalov mentioned this pull request Sep 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asmorkalov asmorkalov asmorkalov approved these changes

Assignees

@asmorkalov asmorkalov

Labels

bug category: objdetect test

Projects

None yet

Milestone

4.9.0

Development

Successfully merging this pull request may close these issues.

Incorrect dictionary size returned from cv::aruco::extendDictionary

2 participants

@AleksandrPanov @asmorkalov