Skip to content

Conversation

@AleksandrPanov
Copy link
Contributor

@AleksandrPanov AleksandrPanov commented Jan 15, 2023

The problem was in python bindings in void matchImagePoints(InputArrayOfArrays detectedCorners, InputArray detectedIds, OutputArray objPoints, OutputArray imgPoints).

detectedCorners may include aruco marker corners as vector<vector<Point2f>> or vector<Mat>. Any Mat in vector<Mat> must have 4 rows for 4 corners for one marker (one marker has 4 corners).

detectedCorners may include Charuco corners as vector<Point2f> or Mat. Python bindings add extra dimension to detectedCorners and therefore vector<Mat> case is additionally processed for charuco corners.

Until the PR merge, you can using custom matchImagePoints():

def matchImagePoints(charuco_board, charuco_corners, charuco_ids): objPoints = [] imgPoints = [] for i in range(0, len(charuco_ids)): index = charuco_ids[i] objPoints.append(charuco_board.getChessboardCorners()[index]) # objPoints[-1][0][1] = charuco_board.getRightBottomCorner()[1] - objPoints[-1][0][1] # set old axis direction imgPoints.append(charuco_corners[i]) return np.array(objPoints), np.array(imgPoints) 

Use sample from #23139 to see problem.

args:
-w=5 -h=7 -sl=0.04 -ml=0.02 -d=10 -v=choriginal.jpg

With fix:
image

detectedCorners includes charuco corners in this case. Python bindings add extra dimension to detectedCorners and therefore without fix detectedCorners processed as aruco marker corners and matchImagePoints returns bad values:

image

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
@AleksandrPanov AleksandrPanov mentioned this pull request Jan 15, 2023
6 tasks
@AleksandrPanov AleksandrPanov marked this pull request as ready for review January 15, 2023 23:59
@AleksandrPanov AleksandrPanov changed the title fix matchImagePoints fix aruco matchImagePoints Jan 16, 2023
@AleksandrPanov AleksandrPanov force-pushed the aruco_fix_matchImagePoints branch 5 times, most recently from 42d97fa to b9dbe13 Compare January 16, 2023 12:14
@AleksandrPanov AleksandrPanov changed the title fix aruco matchImagePoints fix charuco matchImagePoints Jan 16, 2023
@AleksandrPanov AleksandrPanov force-pushed the aruco_fix_matchImagePoints branch 4 times, most recently from 69f69e8 to a0ff116 Compare January 26, 2023 22:05
@AleksandrPanov AleksandrPanov force-pushed the aruco_fix_matchImagePoints branch 2 times, most recently from d6b393b to 9bf43c3 Compare February 12, 2023 23:54
@AleksandrPanov AleksandrPanov force-pushed the aruco_fix_matchImagePoints branch 2 times, most recently from ce709d5 to ed8a99d Compare February 13, 2023 17:43
@AleksandrPanov AleksandrPanov force-pushed the aruco_fix_matchImagePoints branch from ed8a99d to 697b2ec Compare February 14, 2023 08:33
Copy link
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.

👍 Looks good!

Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

From what other part of OpenCV code did you bring that design pattern?
Looks like we have malformed API design - InputArrayOfArrays is misused here.

/cc @vpisarev


Accessing elements without type checks must be avoided.

@AleksandrPanov AleksandrPanov force-pushed the aruco_fix_matchImagePoints branch 4 times, most recently from be2be26 to 605fbf9 Compare February 15, 2023 14:40
@AleksandrPanov
Copy link
Contributor Author

I fixed most of the issue (also updated docs, added cpp test). The issue of InputArrayOfArrays API design is discussed above in the conversation.

@asmorkalov asmorkalov self-assigned this Apr 6, 2023
@asmorkalov asmorkalov added this to the 4.8.0 milestone Apr 6, 2023
@AleksandrPanov AleksandrPanov force-pushed the aruco_fix_matchImagePoints branch 3 times, most recently from bc0cb3b to 890522a Compare April 20, 2023 21:43
@asmorkalov asmorkalov force-pushed the aruco_fix_matchImagePoints branch from 890522a to 4ba06c3 Compare April 27, 2023 09:06
@asmorkalov asmorkalov merged commit af1c63c into opencv:4.x Apr 27, 2023
@asmorkalov asmorkalov mentioned this pull request May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment