I propose to not touch the constructor, but add setLegacyFlag() method. It's easier to remove legacy method in future than break compatibility for all code that uses Charuco.

I agree that we should not have an overload ctor, but rather a legacy flag (set to true by default) as the last parameter in the existing ctor (with an explanation in the ctor documentation), and go without a setLegacy method.

The reasons are below:

• Migrating from pre-4.6.0 to 4.6.0 breaks the code silently (when using a physical target manufactured with pre 4.6.0) and it is difficult for a novice developer to figure out why.
• Migrating from pre-4.5.5 to 4.7.0 breaks the code explicitly because the ctor is different (Size instead of two ints), as in @stefan523's example code in ChArUco pattern has broken backward compatibility #23152. When the developer fixes the compilation error, they trigger the silent failures of the item above. If we introduce a legacy flag set to true by default, this is avoided and backward compatibility is preserved even if the developer is not aware of the legacy flag.
• Migrating from 4.6.0 to 4.7.0 requires the developer to explicitly update the ctor both in the Size parameters and in the legacy flag set to false (that is, assuming they are analyzing a target fabricated using 4.6.0 code). This is the only case that actually leads to backward compatibility issues per my suggestion, when the developer is not aware of the role of the legacy flag.

I think that it is more important to maintain backward compatibility with the dozens of released versions that have been employed to produce physical charuco targets that are still is use in production lines. Deciding otherwise will inevitably lead to systems version upgrades silently failing without an evident explanation.

I propose to not touch the constructor, but add setLegacyFlag() method. It's easier to remove legacy method in future than break compatibility for all code that uses Charuco.

I'm not sure if I understand the ask correctly.
I did keep the existing ctor as-is and added an overload with legacy flag in order to pass the ABI compliance check:
https://pullrequest.opencv.org/buildbot/builders/precommit_linux64/builds/102489/steps/Compare%20ABI%20dumps/logs/report-html
Adding a setLegacyFlag() method would add a new sympol as well. And this would break code, trigger an ABI compliance failure if removed in the future, would it not? I don't see the difference in that respect.

The existing ctor already creates a board. Adding a setLegacyFlag() method that is called after the constructor has run would mean to clear the board and recreate it differently. Is that really the ask? I don't see the advantage, only things that could go bad in the workflow. Or do I get it wrong?

Either way I'm fine with anything that enables usage of existing targets and data. @asmorkalov, please advise and I'll try to adapt.

I believe we should minimize or do not bring code with legacy/buggy behavior from opencv_contrib into the main repository (as we don't really want to support/maintain that).

If setLegacyFlag() doesn't work, then I propose to create CharucoBoardLegacy or cv::aruco::legacy::CharucoBoard in opencv_contrib/modules/aruco for this purpose.
There is inheritance with Board base class and Pimpl design, but some trick is required to expose Board::Impl to opencv_contrib (we don't need that in public OpenCV headers).

Deciding otherwise will inevitably lead to systems version upgrades silently failing without an evident explanation.

one way or another, mentioning this change in the release notes would have been nice..

probably it makes sense to dub the BBF order (i.e. 4.6, 4.7 behavior) "legacy" and hence set the flag to false by default.

I doubt that with that many WBF patterns in the wild and WBF effectively being the default from 4.8 on BBF will be anything but legacy.

Resolved: constructor is not being touched. Different board layouts are supported by setBBF(bool) method.

Sorry, there is useful information for discussion here. Please don't mark as resolved for now.

@AleksandrPanov, what are the required steps to merge this pull request?

I'm interest as well to understand when this pul request will be close?

@AleksandrPanov Did you really verify that?

Default pattern must be like this: https://github.com/opencv/opencv/blob/4.7.0/doc/pattern.png

We start discussion of this patch with setLegacyPattern() flag. No aggrement to use BBF or other unclear abbreviations (first, even, odd in one description is a true mess).
Description should mention changes after 4.5.5/4.6.0 releases.

Default behavior must be preserved (4.7.0).

@alalek, @AleksandrPanov, all raised concerns should've been resolved now. Please approve.

@alalek, I liked @stefan523's idea to give a logical name to the board design parameter. I think that @stefan523 need choose a different name (instead of BBF) and improve the description in the documentation.

Pattern design was changed to implement for calibration compatibility with charuco chessboards (interactive calibration tool + kalibr). For a better understanding, I suggest using this board design parameter as a new parameter, and not just a flag for compatibility with old solutions. I could suggest the name of the parameter as chessboardPatternCompatibility.

I was wrong, the default behavior should be like in 4.7.0.

Documentation/comments should be more clear now.
I had the parameter change to legacyParamter per directions in the other comment, but I am happy to change it again to anything else like chessboardPatternCompatibility if this would be the accepted name?

Resolved: constructor is not being touched. Different board layouts are supported by setBBF(bool) method.