Fix #4188: RGBImage scaled drawImage renders blank#4986
Merged
Conversation
RGBImage.drawImage(Graphics, Object, int, int, int, int) fell through to Image's base implementation, which dispatches via the null native peer and the iOS Metal / OpenGL pipelines (and equivalent on Android / JavaSE) silently rendered nothing. The bug was platform-independent because the broken dispatch happens before the renderer is reached. Override the scaled-draw path on RGBImage: save the graphics transform, compose a matrix-correct translate + scale via translateMatrix / scale, emit drawRGB at native size so the platform pipeline performs the scaling, then restore the transform. Using translateMatrix (not the integer translate accumulator) keeps the composition uniform with the scale step. Falls back to CPU scaleArray only on ports without matrix- correct translate (legacy JavaScript). Add a screenshot test path to the unit test harness: - TestCodenameOneImplementation now persists an affine transform on TestGraphics and rasterises drawRGB (inverse-mapped nearest neighbour) through it, with overrides for setTransform / getTransform / setTransformAffine / translateMatrix / scale / resetAffine. - RGBImageTest adds four @formtest cases for integer upscale, offset draws, downscale and transform restoration, asserting full-canvas pixel arrays. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Collaborator
Author
|
Compared 110 screenshots: 110 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 108 screenshots: 108 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 110 screenshots: 110 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
…lback The translateMatrix + scale approach worked when drawing to a mutable image (xTranslate/yTranslate are 0) but rendered off-target on the screen. On ports where impl.isTranslationSupported() is false (iOS), g.translate(...) accumulates xTranslate/yTranslate on the Graphics object and those are baked into draw coordinates BEFORE the impl matrix is applied. A naked translateMatrix + scale composition therefore multiplied the accumulator by the scale factor, shifting the image by (sx-1)*xTranslate, (sy-1)*yTranslate. Use Graphics.setTransform instead: it conjugates the matrix with T(xTranslate, yTranslate) * M * T(-xTranslate, -yTranslate), cancelling the accumulator so the image lands at (xT + x, yT + y) regardless of the scale factor. Also drop the isTransformSupported / isTranslateMatrixSupported gates and the CPU scaleArray fallback. All shipped ports (iOS Metal/OpenGL, Android, JavaSE) return true for both checks, so the gates always passed in practice and the fallback was dead code on production platforms while costing an extra int[] allocation on the legacy JS port (whose matrix support is functionally adequate anyway). Add a regression test that pre-translates the graphics before the scaled draw and asserts the image lands at (xTranslate + x, yTranslate + y) -- this is the case the previous tests missed because they drew into a fresh canvas where xTranslate/yTranslate were zero. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The DrawImage test in scripts/hellocodenameone exercises the drawImage(RGBImage, x, y, w, h) overload twice -- previously broken under #4188 -- so those regions in the prior reference were left as the canvas background instead of the scaled gradient sphere. With the fix, the scaled RGBImage now renders, so the stored reference no longer matches and the Android instrumentation tests fail at "graphics-draw-image-rect: Screenshot differs (320x640 px, bit depth 8)". Promote the artifact produced by the failing CI run (which is the correct rendering -- the previously-black regions now show the gradient sphere) as the new Android golden. iOS / iOS Metal / JavaScript goldens almost certainly also need a refresh, but their screenshot-comparison workflows did not run on this PR (the iOS workflow only builds the ScreenshotTest.m sources without comparing) so those updates are deferred to whichever PR re-triggers those pipelines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The iOS UI workflow flagged the same diff Android did -- "Test 'graphics-draw-image-rect': Screenshot differs (1179x2556 px, bit depth 8)" in both build-ios and build-ios-metal -- but scripts-ios.yml does not set CN1SS_FAIL_ON_MISMATCH=1, so the job reports the mismatch without failing. That hid the iOS goldens from my first sweep. Visually the same as Android: the previously-blank regions under the two drawImage(rgbImage, x, y, w, h) calls now show the scaled gradient sphere. Promote the artifacts produced by build-ios and build-ios-metal as the new goldens. scripts/javascript/screenshots/graphics-draw-image-rect.png also needs refreshing for the same reason, but the JS workflow does not run on this PR (its path filter excludes CodenameOne/src/**), so deferring that until the JS pipeline produces a new artifact. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
RGBImage.drawImage(Graphics, Object, int, int, int, int)previously fell through toImage's base implementation, which dispatches via the null native peer; the iOS Metal / OpenGL paths (and Android Skia / JavaSE Graphics2D) all silently rendered nothing because the bitmap pointer they received was null. The bug is platform-independent — the broken dispatch happens above the renderer.RGBImage: save the current graphics transform, compose a matrix-correct translate + scale viag.translateMatrix/g.scale, emitdrawRGBat the image's native size so the platform pipeline performs the scaling in hardware, then restore the transform in afinallyblock. UsingtranslateMatrix(not the integertranslateaccumulator) keeps the composition uniform with the scale step. The CPUscaleArrayfallback is kept only for ports without matrix-correct translate (legacy JavaScript).TestCodenameOneImplementationnow persists an affine transform onTestGraphicsanddrawRGBrasterises (inverse-mapped nearest-neighbour) through it. New overrides coversetTransform/getTransform/setTransformAffine/translateMatrix/scale/resetAffine. Existing tests were stubs that never exercised these paths, so behaviour is preserved.Test plan
mvn testinmaven/core-unittests— 2505/2505 pass.@FormTestcases inRGBImageTest:testScaledDrawImageIntegerScale— 2×2 source → 4×4 canvas, asserts every pixel.testScaledDrawImageAtOffset— 2×2 source drawn at (1, 1) into a 6×6 canvas, asserts the inner 4×4 plus the unaltered border ring.testScaledDrawImageDownscale— 4×4 source → 2×2 canvas, asserts the representative pixel per quadrant.testScaledDrawImagePreservesPriorTransform— asserts the graphics transform is identical before and after the scaled draw.Fixes #4188.
🤖 Generated with Claude Code