iOS Metal: per-axis scale decomposition in alpha-mask path (#3302)#4939
Open
shai-almog wants to merge 2 commits into
Open
iOS Metal: per-axis scale decomposition in alpha-mask path (#3302)#4939shai-almog wants to merge 2 commits into
shai-almog wants to merge 2 commits into
Conversation
Under a non-uniform scale, fillShape/drawShape used to rasterise the path at a uniform diagonal-ratio scale and then stretch the resulting alpha-mask texture non-uniformly through the GPU matrix to recover the requested aspect. That bbox math is exact in real numbers but the texture is pixel-rounded at the intermediate uniform scale, so the stretch drifts the rasterised shape off the axis-aligned drawRect / drawLine the framework would emit alongside it — the symptom in GH-3302's grid of "scaled triangles inscribed in rectangles" where the inscribed triangle escapes its bounding rect on iOS. Factor the user transform's 2x2 linear part by taking the column norms as (sx, sy), rasterise the path at S(sx, sy), and apply only the residual transform = transform * S(1/sx, 1/sy) on the GPU side. The residual is pure rotation (and shear, in the worst case) so no per-axis stretch happens at sample time, and the alpha-mask texture matches the rest of the primitives on the same pixel grid. Stroke widening and the radial-gradient bbox use sqrt(sx*sy) so the on-screen pen size matches the legacy uniform behaviour when sx == sy. Gated on `metalRendering` for GlobalGraphics; MutableGraphics's renderShapeViaAlphaMask is metal-only by construction. The GL ES2 path is unchanged so existing GL goldens stay valid. Adds hellocodenameone/InscribedTriangleGrid screenshot test (registered in Cn1ssDeviceRunner). The test exercises the (sx, sy) in {1, 2} cells under g.translate + g.scale + drawRect + fillShape + drawShape so the inscribed- shape property can be verified visually against the goldens once captured. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Collaborator
Author
JavaScript port screenshot updatesCompared 17 screenshots: 16 matched, 1 missing reference.
|
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
Android screenshot updatesCompared 107 screenshots: 106 matched, 1 missing reference.
Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
iOS screenshot updatesCompared 107 screenshots: 106 matched, 1 missing reference.
Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
iOS Metal screenshot updatesCompared 107 screenshots: 106 matched, 1 missing reference.
Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
CodeQL flagged the radial-gradient-bbox scale as 4 implicit narrowing casts (IOSImplementation.java:5848-5851). Make the int casts explicit on the RadialGradient field assignments. Same numeric behaviour as the original *= which silently truncated; just satisfies the analyser. Test improvements requested in PR review: - Fill a known light-grey background so the BLACK rectangle frame is visible on Android (default form bg there is dark) and on JavaSE / iOS without relying on the form's painter to lay one down first. - Drop a per-cell "(sx,sy)" label and an at-the-top "Triangle should fit inside rectangle" hint so the screenshot is self-documenting -- a reader can identify a per-axis-scale failure mode (drift only at sx != sy) straight from the image, without cross-referencing the test source. - Trim the grid to a (1,1) / (1,2) / (2,1) / (2,2) 2x2 layout so the cells fit on a typical simulator panel after the matrix scale doubles their on-screen extent. 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
2 participants
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
Fixes the
nativeDrawShape/renderShapeViaAlphaMaskalpha-mask drift under non-uniform scale on the iOS Metal backend (GH-3302). Underg.translate + g.scale(sx, sy) + fillShapewithsx != sy, the legacy path rasterises the shape at uniformh2/h1and then stretches the resulting texture non-uniformly through the GPU matrix — bbox math is exact in real numbers but the texture is pixel-rounded at the intermediate uniform scale, so the stretch drifts the rasterised shape off the axis-aligneddrawRect/drawLinethe framework emits alongside it.The fix factors the user transform's 2x2 column norms into per-axis
(sx, sy), rasterises the path atS(sx, sy), and leaves only the residualtransform * S(1/sx, 1/sy)for the GPU. The residual is pure rotation (and shear in the worst case) so no per-axis stretch happens at sample time, and the alpha-mask texture lands on the same pixel grid asdrawRectsiblings. Stroke widening and the radial-gradient bbox usesqrt(sx*sy)so the on-screen stroke matches the legacy uniform behaviour whensx == sy.Scope gating
GlobalGraphics.nativeDrawShapeopt-in branch is gated onmetalRendering; the GL ES2 backend still takes the legacyh2/h1path so existing GL goldens stay valid.MutableGraphics.renderShapeViaAlphaMaskis Metal-only at the entry, so the inner code is unconditionally the new per-axis decomposition.Test
Adds
hellocodenameone/InscribedTriangleGridscreenshot test (registered inCn1ssDeviceRunner). It exercises(sx, sy)in {1, 2} cells underg.translate + g.scale + drawRect + fillShape + drawShapeso the inscribed-shape property is visually verifiable once iOS Metal goldens are captured.Test plan
graphics-inscribed-triangle-grid.pngagainstscripts/ios/screenshots-metal/(golden will need to be added in a follow-up commit after first capture).graphics-affine-scale,graphics-scale,graphics-fill-shape,graphics-rotate, etc.) remain unchanged — GL path is byte-identical because the legacyh2/h1branch is preserved.🤖 Generated with Claude Code