fix: prevent crashes when previewing dense ChArUco boards#979
Merged
Conversation
A board with many squares could crash the preview in two separate ways. First, OpenCV refuses to draw the board at certain output sizes, and which sizes fail is hard to predict, so picking one fixed size is not safe. The board now renders at the requested size and, if OpenCV rejects it, bumps the size up by a pixel and tries again until it works. Second, a board can need more markers than its dictionary holds, which also crashes the render. The dictionary now resizes itself to the smallest one that still covers the board, applied whenever the board is saved or loaded. If a board needs more markers than any dictionary in the family can hold, the GUI warns and reverts to the last valid board instead of crashing.
The preview test converts a rendered board to a QPixmap, which is screen-backed. On headless CI runners without a window server the native platform plugin returns a null pixmap, so the test failed on macOS. The offscreen plugin has a raster backend that works headlessly on every platform; setting it before any QApplication is created fixes the false negative while keeping the test exercising the real preview path.
The offscreen-platform approach didn't hold: with loadfile distribution a worker creates a QApplication for an earlier GUI test before this module is imported, so setting the platform here came too late and macOS still produced a null pixmap. The #978 crash was an exception in the preview path, so a clean return is the real regression guard. QPixmap is screen-backed and comes back null on headless runners, which is not the bug under test, so this no longer asserts on its contents. That the render produces real pixels is already covered at the domain level by the hand-edited-board test that calls board_img directly.
The test drove the preview's QPixmap conversion, but a QPixmap is backed by the windowing system and comes back empty on headless CI runners, so it failed on macOS for an environment reason rather than a real defect. The only honest assertion left would have been "the call did not raise," which is weak enough to read as coverage theater. The #978 crash itself lived in board_img and the dictionary fit, and both are still covered by the domain tests. What this test uniquely touched was the display-only conversion glue, which is not worth a windowing dependency in CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #978.
A dense ChArUco board (many squares, or a small dictionary pool) crashed the board preview. There were two separate crashes that looked alike.
What was wrong
generateImagerefuses to draw the board at certain output sizes, and which sizes fail is hard to predict, so there is no single safe size to pick. This is the crash the issue reporter hit.The fix
board_imgrenders at the requested size and, if OpenCV rejects it, nudges the size up a pixel and retries until it works, with a bounded retry count.INTER_AREAfor a cleaner thumbnail.DictionaryCapacityError. In the GUI this never persists or displays a broken board: the spinboxes revert to the last valid board with a warning dialog.Testing
Ten tests in
tests/test_charuco.pycover the render retry across known failure sizes, the pool auto-fit (grow, shrink, no-op, passthrough, over-capacity), persistence round-trips, and the project-open repro through the real repository-to-preview path.basedpyrightandruffare clean on the changed files.