opencv_contrib
opencv_contrib copied to clipboard
CharucoBoard_create regression / incompatible patterns (only even row count) with 4.6.0
System information (version)
OpenCV = 4.6.0 / 4.5.5
Detailed description
cv2.aruco.CharucoBoard_create creates a different pattern in 4.6.0 than in 4.5.5 for even row counts. This leads to backwards incompatibility with previous versions, previously manufactured targets, etc.
OpenCV 4.5.5:

OpenCV 4.6.0:

Maybe this regression was introduced when resolving issue https://github.com/opencv/opencv_contrib/issues/2623 ?
Steps to reproduce
import cv2
aruco_dict = cv2.aruco.getPredefinedDictionary(cv2.aruco.DICT_5X5_1000)
charuco_board = cv2.aruco.CharucoBoard_create(
squaresX = 5,
squaresY = 4,
squareLength = 4,
markerLength = 3,
dictionary = aruco_dict
)
img_board = charuco_board.draw((250,200), marginSize=0, borderBits=1)
cv2.imwrite('charuco_OpenCV-' + cv2.__version__ + '.png', img_board)
Behavior is aligned with chessboard: https://github.com/opencv/opencv/blob/4.x/doc/pattern.png
Behavior is aligned with chessboard: https://github.com/opencv/opencv/blob/4.x/doc/pattern.png
That pattern in that link is 10x7 (although it claims to be 9x6 in the caption...or does it mean 9x6 corners? nvm.) and this looks the same in both versions.

Anyhow the change breaks backwards compatibiltiy (SW&HW).
It would help a lot to e.g. have a switch to enable the legacy pattern of pre4.6.0.
About the chessboard pattern, this is indeed a 9x6 pattern since only corners from intersection are detected / used.

Hmm, some more precision.
This was the code to generate the pattern (see https://github.com/opencv/opencv/pull/11046):
python gen_pattern.py -r 10 -c 7 -a A4 -T checkerboard -s 25
But when you are doing camera calibration (see https://github.com/opencv/opencv/blob/4.x/samples/cpp/calibration.cpp), you have to generate a 9x6 grid / corner points vector. That's why it is written 9x6.
The below patch seems to work for me, adding a "legacy" switch when creating the board.
diff --git a/modules/aruco/include/opencv2/aruco/charuco.hpp b/modules/aruco/include/opencv2/aruco/charuco.hpp
index 2417b2fd..69b511a1 100644
--- a/modules/aruco/include/opencv2/aruco/charuco.hpp
+++ b/modules/aruco/include/opencv2/aruco/charuco.hpp
@@ -92,13 +92,14 @@ class CV_EXPORTS_W CharucoBoard : public Board {
* @param markerLength marker side length (same unit than squareLength)
* @param dictionary dictionary of markers indicating the type of markers.
* The first markers in the dictionary are used to fill the white chessboard squares.
+ * @param legacy legacy chessboard pattern (pre 4.6.0)
* @return the output CharucoBoard object
*
* This functions creates a CharucoBoard object given the number of squares in each direction
* and the size of the markers and chessboard squares.
*/
CV_WRAP static Ptr<CharucoBoard> create(int squaresX, int squaresY, float squareLength,
- float markerLength, const Ptr<Dictionary> &dictionary);
+ float markerLength, const Ptr<Dictionary> &dictionary, bool legacy = false);
/**
*
@@ -126,6 +127,9 @@ class CV_EXPORTS_W CharucoBoard : public Board {
// marker side length (normally in meters)
float _markerLength;
+
+ // legacy chessboard pattern (pre 4.6.0)
+ bool _legacy;
};
diff --git a/modules/aruco/src/charuco.cpp b/modules/aruco/src/charuco.cpp
index b179bc63..2676647e 100644
--- a/modules/aruco/src/charuco.cpp
+++ b/modules/aruco/src/charuco.cpp
@@ -101,7 +101,11 @@ void CharucoBoard::draw(Size outSize, OutputArray _img, int marginSize, int bord
for(int y = 0; y < _squaresY; y++) {
for(int x = 0; x < _squaresX; x++) {
- if(y % 2 != x % 2) continue; // white corner, dont do anything
+ if(_legacy) {
+ if((y + ((_squaresY + 1) % 2)) % 2 != x % 2) continue; // white corner, dont do anything
+ } else {
+ if(y % 2 != x % 2) continue; // white corner, dont do anything
+ }
double startX, startY;
startX = squareSizePixels * double(x);
@@ -120,7 +124,7 @@ void CharucoBoard::draw(Size outSize, OutputArray _img, int marginSize, int bord
/**
*/
Ptr<CharucoBoard> CharucoBoard::create(int squaresX, int squaresY, float squareLength,
- float markerLength, const Ptr<Dictionary> &dictionary) {
+ float markerLength, const Ptr<Dictionary> &dictionary, bool legacy) {
CV_Assert(squaresX > 1 && squaresY > 1 && markerLength > 0 && squareLength > markerLength);
Ptr<CharucoBoard> res = makePtr<CharucoBoard>();
@@ -129,6 +133,7 @@ Ptr<CharucoBoard> CharucoBoard::create(int squaresX, int squaresY, float squareL
res->_squaresY = squaresY;
res->_squareLength = squareLength;
res->_markerLength = markerLength;
+ res->_legacy = legacy;
res->dictionary = dictionary;
float diffSquareMarkerLength = (squareLength - markerLength) / 2;
@@ -137,7 +142,11 @@ Ptr<CharucoBoard> CharucoBoard::create(int squaresX, int squaresY, float squareL
for(int y = 0; y < squaresY; y++) {
for(int x = 0; x < squaresX; x++) {
- if(y % 2 == x % 2) continue; // black corner, no marker here
+ if(legacy) {
+ if((y + ((squaresY + 1) % 2)) % 2 == x % 2) continue; // black corner, no marker here
+ } else {
+ if(y % 2 == x % 2) continue; // black corner, no marker here
+ }
vector<Point3f> corners(4);
corners[0] = Point3f(x * squareLength + diffSquareMarkerLength,
@stefan523 there is behavior change only with even row count. There were several bugs: https://github.com/opencv/opencv_contrib/issues/2623, https://github.com/opencv/opencv_contrib/issues/2604
This path fixed this and now CharucoBoard behavior is aligned with chessboard: https://github.com/opencv/opencv/blob/4.x/doc/pattern.png
You can use the suggested patch, but it is recommended to use new patterns (with even row count) as the calibration might not work correctly on old patterns (with even row count). The legacy version not added to avoid using even templates, with increased re-projection error.
The legacy version not added to avoid using even templates, with increased re-projection error.
Why do you say that? What is this based on?
it is recommended to use new patterns (with even row count) as the calibration might not work correctly on old patterns (with even row count)
Production code around the world relies on physical boards with even row counts, produced using pre-4.6.0 code, that cannot and will not be physically replaced to the new pattern. This renders opencv 4.6.0 unusable in many calibration/tracking systems, including ones that I developed and maintain. In #3301 the guy bought a board from calib.io, should he throw it out? Should calib.io discard their stock?
@AleksandrPanov I don't think you realize that this was a huge breaking change. A legacy flag set to true by default is the least that can be done to avoid breaking existing code when upgrading (speaking from experience here). @alalek what do you think?
Now that 4.6.0 is out in the wild it should be worth to consider actively breaking API backward compatibility and not setting a default option, forcing developers to choose (legacy or new). Otherwise unsuspecting individuals might stumble over this issue for quite a while. Maybe someone is producing new targets with 4.6.0 now, which would break if a future version has the legacy flag set to true by default?
Either way it would be really appreciated to have the legacy functionality accessible without applying a patch manually and compiling opencv+contrib from source.
PS: The patch I suggested above does not change the order of IDs for "legacy", so maybe it would not re-open issues https://github.com/opencv/opencv_contrib/issues/2623 and https://github.com/opencv/opencv_contrib/issues/2604?
I understand the reasoning behind the change, but I believe that backward compatibility (huge) trumps consistency here. I strongly support the opinion that a legacy switch (on by default) is needed. For developers printing patterns ad-hoc the change might not seem dramatic, but ChAruCo patterns are used in mission critical production software pipelines by many customers worldwide. No-one expects an OpenCV version update to break compatibility with older patterns. Considerable investments have been made into stock. Keeping separate pre- and post- 4.6.0 ChAruCo boards is wasteful in terms of time and resources.
Jakob // Calib.io
I went ahead and created pull request https://github.com/opencv/opencv_contrib/pull/3305 with this patch. Legacy is set to "true" by default. Some small adjustments were necessary to get the unit tests working properly with both board types. All is pass now, and there are no fails regarding bad reprojection errors or similar. Changing or removing the default value of legacy would be simple enough if that would be the final verdict.
I was calibrating with 4.6.0 and 8x11 board charuco board and always got huge reprojection error.
This bug is not easy to find.
I downgraded the opencv to 4.5.*, hope we can have a fix soon
I updated pull request https://github.com/opencv/opencv_contrib/pull/3305 to resolve merge conflicts due to ongoing main branch development. What are the next steps to move forward? @AleksandrPanov, please comment.
Andrew Martchenko showed a workaround in this comment: https://github.com/opencv/opencv_contrib/issues/3367#issuecomment-1292874997
opencv 4.7.0 is now released, with aruco moved to the main repo (not contrib). This problem was not addressed, and the legacy flag was not introduced. @stefan523 I think you should reopen the issue under the main repo and port your PR
@gmedan
opencv 4.7.0 is now released, with aruco moved to the main repo (not contrib). This problem was not addressed, and the legacy flag was not introduced. @stefan523 I think you should reopen the issue under the main repo and port your PR
Done:
- https://github.com/opencv/opencv/pull/23152
- https://github.com/opencv/opencv/pull/23153
Kudos @stefan523!
Fixed in main branch by https://github.com/opencv/opencv/pull/23153 for next OpenCV release 4.8.0.