Repository navigation
fix: Image.Read returns C-contiguous pixels; drawing into a strided image raises - #162
Merged
Merged
Conversation
…mage raises Every draw_* method failed on an image from Image.Read with "Layout of the output array img is incompatible with cv::Mat". convert() reordered the color planes with fancy indexing, image[:, :, [2, 1, 0]]; NumPy does not promise a C-contiguous result for that and returns a planar layout (strides (W, 1, H*W)). OpenCV's in-place drawing functions refuse such an array. The BGRA->BGR slice, reduce= and roi= also returned strided views. - convert(): reorder planes with np.take (a contiguous copy, which is what the old comment about Torch wanted), and guarantee a C-contiguous result at the single exit point (no copy if it already is). Images from Image.Read, iread and iread_iter are therefore always drawable; their docstrings now say so. - Image(array) still keeps a reference to the array it is given (no copy, no forced layout), so an image made from a transposed or reversed view is not contiguous. Rather than silently copying, every method that draws into an image in place checks first and raises a ValueError, before anything is drawn, that says to use Image(array, copy=True), which always gives a C-contiguous copy. The check is _require_c_contiguous() in base/graphics.py, called by draw_box, draw_labelbox, draw_text, draw_point, draw_line and draw_circle (so Image.draw_* and BaseFeature2D.draw are covered), and explicitly by BaseFeature2D.draw2 and the two Fiducial/Board draw() methods that call cv2.drawFrameAxes. - Docs: a .. warning:: on each of those methods and a .. note:: in Image(). Also fixes a malformed ".. note::Wildcard" directive in iread_iter. Adds tests/test_image_contiguous.py (29 tests; most fail without the fix). Reproduced end to end on OpenCV 5.0.0 with NumPy 2.5.4. OpenCV 4.14.0 (NumPy 2.2.6) rejects strided arrays in cv2.line/circle/putText in the same way when called directly, so this is not specific to OpenCV 5; older 4.x not tested. Fixes #158. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 15 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
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
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.
Summary
Every
Image.draw_*method failed on an image fromImage.ReadwithLayout of the output array img is incompatible with cv::Mat.convert()reordered the color planes with fancy indexing,image[:, :, [2, 1, 0]], and NumPy does not promise a C-contiguous result for that (it returns a planar layout, strides(W, 1, H*W)). OpenCV's in-place drawing functions (line,circle,putText,drawFrameAxes, ...) refuse such an array. The BGRA→BGR slice,reduce=androi=also returned strided views.What changes:
convert()now reorders planes withnp.takeand returnsnp.ascontiguousarray(image)at its single exit (no copy if already contiguous). Images fromImage.Read,ireadandiread_iterare always drawable, and their docstrings say so.Image(array)is unchanged: it still keeps a reference to the array it is given. An image built from a transposed or reversed view is therefore not contiguous. Rather than silently copying (which would make aliasing depend on the array's layout), every method that draws into an image in place now raises aValueError, before anything is drawn, telling the user to useImage(array, copy=True), which always gives a C-contiguous copy._require_c_contiguous()inbase/graphics.py, called bydraw_box,draw_labelbox,draw_text,draw_point,draw_lineanddraw_circle(so allImage.draw_*andBaseFeature2D.draware covered), and explicitly byBaseFeature2D.draw2and the twodraw()methods inImageFiducials.pythat callcv2.drawFrameAxes... warning::on each writing method and a.. note::inImage()explaining the rule. Also fixes a malformed.. note::Wildcarddirective iniread_iter.Behavior change to be aware of: drawing into a non-contiguous image used to fail inside OpenCV with an obscure error; it now raises
ValueErrorwith an explanation.Related issue
Fixes #158
Verification
tests/test_image_contiguous.py, 29 tests: contiguity on everyconvertpath, drawing onImage.Readimages, the error and pixels left untouched,copy=Truefixing it,Image(a)still referencinga, featuredraw/draw2. Run against unfixedmain, 16 of the 20 tests from the first version fail with theputText/linelayout errors.pytest tests: 1012 passed, 96 skipped.cv2.line/circle/putTextin the same way when called directly, so this is not specific to OpenCV 5. I did not run MVTB end to end on OpenCV 4, and did not test older 4.x.Fiducial.drawand the boarddraw()(a fiducial is heavy to build in a test; the check is the same one-line call).Checklist
pytest)🤖 Generated with Claude Code