iOS Metal: per-axis scale decomposition in alpha-mask path (#3302)#4939
Open
shai-almog wants to merge 8 commits into
Open
iOS Metal: per-axis scale decomposition in alpha-mask path (#3302)#4939shai-almog wants to merge 8 commits into
shai-almog wants to merge 8 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
|
Compared 17 screenshots: 17 matched. |
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
|
Compared 107 screenshots: 107 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
iOS screenshot updatesCompared 107 screenshots: 106 matched, 1 updated.
Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
iOS Metal screenshot updatesCompared 107 screenshots: 106 matched, 1 updated.
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>
Add a Graphics.translateMatrix(float, float) public API and CodenameOneImplementation.translateMatrix(graphics, x, y) hook that composes T(x, y) onto the impl-side transform matrix the same way Graphics.scale and Graphics.rotate do. Plus Graphics.isTranslateMatrixSupported() / impl isTranslateMatrixSupported() so ports that haven't wired the matrix path yet can opt out cleanly. Why this exists: every active port today returns isTranslationSupported() == false (per the comment at Graphics.java:62), so g.translate(int, int) is a per-Graphics integer accumulator that gets *added to draw coordinates before* the impl matrix is applied. A subsequent g.scale() therefore multiplies the integer translate too. The user-visible consequence is that the same drawing code can land at different on-screen positions depending on whether you're drawing into a Form's Graphics (where bounds.getX() carries the component-absolute offset that gets scaled) versus a mutable Image's Graphics (where bounds.getX() is 0). The GH-3302 inscribed-triangle test exposed this clearly: top-right form-direct cells get pushed off the panel while the same code on the mutable-image side stays put. translateMatrix composes the translate into the impl matrix instead, producing uniform "matrix-correct" semantics: g.translateMatrix(20, 30) followed by g.scale(2, 2) is the same final transform as a Java2D Graphics2D.translate(20, 30) + .scale(2, 2) pair, on every port. The fallback path on legacy / restricted ports (isTranslateMatrixSupported == false) routes through the integer translate(int, int) so apps still render -- just at the legacy position. Wiring: - Default isTranslateMatrixSupported() == false; no-op translateMatrix. - iOS: NativeGraphics.translateMatrix composes T onto the in-memory matrix (clipDirty / inverseClipDirty / inverseTransformDirty flagged exactly like scale/rotate, then applyTransform). - JavaSE: getTransform / setTransform pair, same as scale. - Android: AndroidGraphics.translateMatrix uses getTransform().translate(x, y) and flips the same dirty flags scale does. - JavaScript (HTML5): mirrors scale's setTransformChanged + applyTransform pattern. Legacy JS port keeps the default opt-out. Update InscribedTriangleGrid test to use translateMatrix(cellX, cellY) so each cell anchors via the matrix instead of the integer accumulator; the form-direct and mutable-image panels then render identically up to the blit offset, matching the JavaSE gold-standard behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 7 screenshots: 7 matched. |
Contributor
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
The previous commit added metalRendering gates around nativeDrawRect /
nativeFillRect / nativeDrawLine in NativeGraphics, routing through
renderShapeViaAlphaMask under Metal "to avoid the CG fallback." That
diagnosis was wrong: there is no CG fallback under Metal here. The
C-side counterparts already short-circuit through the Metal pipeline:
nativeDrawRectMutableImpl -- has #ifdef CN1_USE_METAL guard
nativeFillRectMutableImpl -- has #ifdef CN1_USE_METAL guard
nativeDrawLineMutableImpl -- has #ifdef CN1_USE_METAL guard
nativeDrawStringMutableImpl -- has #ifdef CN1_USE_METAL guard
nativeDrawImageMutableImpl -- has #ifdef CN1_USE_METAL guard
nativeFillRoundRectMutableImpl -- has #ifdef CN1_USE_METAL guard
nativeDrawRoundRectMutableImpl -- has #ifdef CN1_USE_METAL guard
nativeDrawArcMutableImpl -- has #ifdef CN1_USE_METAL guard
nativeFillArcMutableImpl -- "Dead under Metal", Java-side routes
through nativeFillShape instead
i.e. on a Metal build the legacy CG branches in those Mutable JNIs
are unreachable. Rerouting drawRect/fillRect/drawLine through the
alpha-mask path on the Java side just changes which Metal op is
queued (DrawTextureAlphaMask vs FillRect / DrawRect / DrawLine) -- it
doesn't fix any CG leak because there isn't one. Replace the gating
with three short comments documenting that the C side is responsible
for Metal routing here.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Android screenshot simulator runs at 320x640 (160x320 per quadrant under GridLayout(2, 2)), and the test's previous fixed-pixel layout (baseW = 80, gridOffsetX = 30 + 30 = 60) didn't leave enough room for the right-column cell's 2x scaling -- the (2,*) cells fell off the right edge so only column 0 was visible. Switch cell dimensions to fractions of `bounds`: baseW = max(8, panelW * 2 / 9) baseH = max(6, gridH * 2 / 9) Solves baseW * 3 + gutter * 3 = panelW with gutter = baseW / 2 so the 1x and 2x columns + three gutters fit inside the panel width on every target. Same arithmetic on the height for the 1x and 2x rows. Triangle dimensions and the rectangle frame derive from those base sizes so the inscribed-shape property is exercised regardless of panel size. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The per-axis decomposition in GlobalGraphics.nativeDrawShape and NativeGraphics.renderShapeViaAlphaMask used to apply the residual S(1/sx, 1/sy) by building a separate composed Transform (`tmpTransform` or a fresh `Transform.makeIdentity()`) and calling `setTransform(...)` on the NativeGraphics. That path turns out to silently drop the matrix update on the Metal mutable-image encoder -- exactly the failure mode documented in Transform.setTransform's "iOS Metal port has shown that without this flag setTransform(composed) silently fails to apply" note. User-visible symptom: on a mutable-image Graphics with a non-uniform g.scale active, the alpha-mask quad got drawn under the previous non-residual transform, so fillShape / drawShape rendered at the wrong scale relative to the axis-aligned drawRect siblings. The triangle ended up at S(sx, sy) * S(sx, sy) instead of S(sx, sy), i.e. 4x height at sy=2. Fix: apply the residual via `scale(1f/sx, 1f/sy)` followed by the draw and a paired `scale(sx, sy)` to restore -- the same code path g.scale uses, which reliably queues a SetTransform op tagged with the active mutable target. Both GlobalGraphics.nativeDrawShape (screen) and NativeGraphics.renderShapeViaAlphaMask (mutable) now route through this. Drops the tmpTransform / tmpTransform2 / `setTransform(inv)` scaffolding that's no longer needed. Verified locally on iOS Metal simulator: the InscribedTriangleGrid test now renders identical 2x2 cell grids in form-direct AA off + AA on + mutable AA off + AA on panels, with triangles correctly inscribed in the rectangles at every (sx, sy) combination. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
iOS Metal golden is the reference for the GH-3302 fix: all four panels (form-direct uniform, form-direct retina, mutable uniform, mutable retina) render the green triangle inscribed within the black rectangle in every cell variant. iOS GL golden captures the legacy CG-based mutable path's known fillShape limitation (outlines only on bottom panels). This is a pre-existing GL behaviour the user accepted; the alpha-mask change landed for the Metal path only. JavaSE golden is included as a cross-port reference (graphics tests do not run in JavaSE CI, but the file documents the gold-standard output). Android and JavaScript goldens still need to be captured from CI artifacts on the first PR run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Android golden (320x640) is the emulator-screenshot artifact from the api-level 36 / x86_64 / google_apis runner. Form-direct panels (bottom row) render the four inscribed-triangle cells correctly; mutable-image panels (top row) show the existing Android mutable-blit layout limitation the user accepted in review. JavaScript golden (750x1334) is the javascript-ui-tests artifact and captures the existing JS panel-overlap layout. Both are reference captures of current behaviour, not claims of correctness; this PR scopes its rendering fixes to the iOS Metal path. 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