Graphics.useMatrixTranslation: route g.translate through impl matrix (GH-3302)#4960
Open
shai-almog wants to merge 20 commits into
Open
Graphics.useMatrixTranslation: route g.translate through impl matrix (GH-3302)#4960shai-almog wants to merge 20 commits into
shai-almog wants to merge 20 commits into
Conversation
Add a static Graphics.useMatrixTranslation opt-in that flips every g.translate(int, int) in the rendering chain through the impl-side affine matrix (via translateMatrix) instead of into the per-Graphics integer accumulator that historically was added to draw coords. Why this matters: the framework's container/component painting pipeline stacks g.translate(absX, absY) calls before user paint() runs. With the legacy integer accumulator, a user g.scale(sx, sy) inside paint() multiplies the chrome offset (status-bar + title-area height), pushing direct-to-screen drawing off the Y axis -- exactly the residue from GH-3302's follow-up. Buffered (mutable-image) drawing avoided this because the offscreen Graphics started with xTranslate=0. With the flag on AND impl.isTranslateMatrixSupported() == true (iOS, Android, JavaSE, HTML5 today), translate(int, int) now composes onto the impl matrix the same way scale/rotate do, so the chrome offset flows through the matrix and is not stretched by a subsequent g.scale. xTranslate/yTranslate is still bumped as a shadow accumulator so existing getTranslateX/getTranslateY consumers in Form/Component/Label/Container/FontImage/LinearGradientPaint/ TextSelection keep returning legacy values; the drawing primitives no longer add it (the impl matrix carries the translate). 25 primitive sites switched from `xTranslate + x` to a `tx(x)` helper, plus shape pre-shift conditionals in setClip(Shape)/drawShape/fillShape/ fillPolygon/drawPolygon were gated, getClipX/Y returns the impl's matrix-local clip directly, rotateRadians' pivot compensation is a no-op in matrix mode, and setTransform composes T(xt,yt) * M without the legacy terminal T(-xt,-yt). Ports that opt out (default isTranslateMatrixSupported() == false) keep using the legacy integer accumulator regardless of the flag, so no port has to be touched in lockstep. Wiring: - TestCodenameOneImplementation grows setTranslateMatrixSupported + translateMatrix tracking so unit tests can exercise both paths. - GraphicsTest covers: matrix-mode pushes through impl.translateMatrix while keeping the shadow accumulator; draw coords are NOT pre-shifted; legacy fallback when impl opts out; shape pre-translate is skipped. - HelloCodenameOne.init() flips the flag on so the existing screenshot suite exercises matrix mode end-to-end. New TranslateThenScale test added under hellocodenameone graphics/ to exercise the direct-to- screen g.translate + g.scale path that was the original repro. Expected: form-direct panels in graphics-inscribed-triangle-grid and any other screenshot that mixes the framework's container translates with user g.scale/g.rotate in direct-to-screen mode will shift, since those goldens captured the legacy bug. Mutable-image panels are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 18 screenshots: 18 matched. |
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
Android screenshot updatesCompared 108 screenshots: 107 matched, 1 updated.
Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
…ll sites
The previous revision kept xTranslate/yTranslate accumulating alongside
the impl-matrix translate so getTranslateX/getTranslateY could keep
returning legacy values for framework callers. That broke ~103 of 108
screenshot tests because the framework's bypass-Graphics fast paths
(Label.drawLabelComponent, BGPainter.paintComponentBackground) add
getTranslateX() onto local coords and hand the absolute result straight
to Display.impl, which then applies the matrix that ALREADY encodes the
same translate -- so every label/border drawn through those paths
double-counted the chrome offset and basic layout drifted.
This commit:
- Drops the shadow accumulator in matrix mode. Graphics.translate(int,
int) now ONLY calls impl.translateMatrix when the flag is on.
- Reads getTranslateX/getTranslateY off the impl matrix
(impl.getTransform(ng).getTranslateX()) in matrix mode. Reliable
while the matrix is pure-translate, which is the only state in which
the framework callers (Form.paintGlassImpl, Container.paintComponent,
Component.paintIntersectingComponentsAbove, paintIntersecting/$FLAT,
FontImage absolute-pivot rotation, LinearGradientPaint,
TextSelection) snapshot translate -- they all run before any user
scale/rotate composes onto the matrix.
- Fixes the two bypass-Graphics call sites:
- Label.draw at Label.java:913 now uses 0 instead of g.getTranslateX
in matrix mode (the impl matrix carries the translate that
drawLabelComponent's internal draws will apply).
- BGPainter.paint at Component.java:8659 does the same for the
impl.paintComponentBackground bypass.
- TestCodenameOneImplementation grows a per-Graphics matrixTranslateX/Y
accumulator that translateMatrix mutates, plus an override of
getTransform(Object) that returns Transform.makeTranslation of those
accumulators -- so unit tests can verify matrix-mode getTranslateX/Y
flows end-to-end through the same call path the production Graphics
uses.
All 2429 core-unittests still pass (including the 4 matrix-mode tests
in GraphicsTest, now updated to assert matrix-derived getTranslateX/Y
instead of the dropped shadow accumulator).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
iOS Metal screenshot updatesCompared 108 screenshots: 100 matched, 7 updated, 1 missing reference.
Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
iOS screenshot updatesCompared 108 screenshots: 104 matched, 3 updated, 1 missing reference.
Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
…cross resetAffine / setTransform The previous attempt dropped the shadow xTranslate/yTranslate accumulator entirely, deriving getTranslateX/Y from the impl matrix instead. That worked for snapshot-reset patterns but didn't address the bigger reason the framework was rendering wrong: many user-paint snippets in the codebase call resetAffine() or setTransform(null) -- MapComponent, Scene, CommonTransitions, FontImage, beginNativeGraphicsAccess, and a handful of transition classes -- and impl.resetAffine / impl.setTransform wipe the impl matrix to identity. In legacy mode that's fine because the matrix only carries scale/rotate; the framework's chain of g.translate(absX, absY) lives in the integer xTranslate accumulator. In matrix mode the matrix carries everything, so resetAffine destroyed the chrome offset and the next draw landed at screen origin. This commit: - Restores the shadow xTranslate/yTranslate accumulator in matrix mode. Graphics.translate(int, int) bumps both the shadow AND calls impl.translateMatrix. Drawing primitives still use tx(x)/ty(y) which skip the shadow in matrix mode (no double shift); the shadow is the single source of truth for "what screen offset is currently in effect," consulted by setTransform's conjugation, resetAffine's re-translate, and getTranslateX/Y. - getTranslateX/Y returns the shadow directly. Reading the impl matrix was unreliable: Transform.getTranslateX caches translateX only while type == TYPE_TRANSLATION, and freezes the moment any scale/rotate is composed, so the value lags as soon as user code mixes transforms. - resetAffine in matrix mode: after impl.resetAffine, replay xTranslate/yTranslate via impl.translateMatrix so the matrix returns to T(framework_translate) rather than identity. This restores the contract MapComponent/Scene/CommonTransitions depend on (resetAffine rolls back user transforms but leaves the framework baseline alone). - setTransform(null) / setTransform(identity) in matrix mode: same recipe -- reset impl, replay translate. The non-identity branch already composed T(xt) * M, so that path was correct already. - Graphics.translate's matrix-mode branch also re-composes T(xt + d) * userTransform if a setTransform call had set one, the same way the legacy branch does. GraphicsTest grows two tests pinning the resetAffine and setTransform(null) preserve-translate contracts so a future revert can't break them silently. Full core-unittests still green at 2431/2431. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… updated Previous revisions tried to maintain both the legacy xTranslate/yTranslate integer accumulator and the impl-side matrix translate in parallel. That created two contradictory bookkeeping systems whose interaction surfaced as duplicated form titles and ~86 of 108 screenshots drifting across JS/Android/iOS. This revision commits fully to the matrix path: in matrix mode the impl matrix is the only source of truth for the framework painting-chain translates, and the framework callsites that historically used the integer accumulator are each updated with a matrix-mode branch. Graphics.java: - translate(int, int) in matrix mode calls ONLY impl.translateMatrix. xTranslate/yTranslate stay at zero -- they are never bumped. - getTranslateX/Y returns the (now permanently zero) shadow. This is semantically "what to add to local coords before drawing" -- zero in matrix mode because drawing primitives no longer add it (the impl matrix carries the translate). Callers needing the actual screen offset for a save / reset / restore pattern must use getTransform(). - resetAffine and setTransform delegate straight to impl in matrix mode -- no more shadow-replay hack. Callers that need the framework chain preserved across these calls handle save/restore themselves. - beginNativeGraphicsAccess / endNativeGraphicsAccess: matrix-mode branch snapshots the impl Transform, sets identity for the native access, restores the Transform on end. Callsites updated with explicit matrix-mode branches (save / identity / restore the impl Transform in place of the legacy snapshot-reset translate, or save / scale+rotate / restore in place of resetAffine): - Form.paintGlassImpl (glass pane painted at screen origin) - Container.paintComponent tail (glass + tensile painted at screen origin) - Component.paintIntersectingComponentsAbove (parent-chain walk) - Component.drawPainters $FLAT cache path - TextSelection.paint (span fills under selectionRoot's absolute origin) - FontImage rotation (drawImage and drawString variants) - MapComponent.paint zoom path - Scene.paint - CommonTransitions TYPE_PULSATE_DIALOG - CodenameOneImplementation animation paintQueue wrapper reset The Label.draw and BGPainter.paint bypass-Graphics fast paths now work naturally without the prior `useMatrixTranslation ? 0 : g.getTranslateX()` guard, because getTranslateX returns 0 in matrix mode -- the guard is revertedand the original arithmetic stays unchanged. GraphicsTest updated: - testMatrixModeTranslatePushesToImplMatrix asserts getTranslateX==0 (not 5) in matrix mode -- matches the new contract. - testMatrixModeResetAffinePreservesFrameworkTranslate + testMatrixModeSetTransformIdentityPreservesFrameworkTranslate (which tested the now-dropped preserve-translate hack) replaced with testMatrixModeResetAffineWipesImplMatrix, pinning that no hidden state survives across resetAffine. All 2430 core-unittests still green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PMD's UnnecessaryFullyQualifiedName rule failed CI because earlier commits referenced Graphics.useMatrixTranslation and Transform via their fully-qualified names instead of imports. Add the Transform import where missing (MapComponent, Scene, CommonTransitions) and unqualify both names in: CodenameOneImplementation, MapComponent, Scene, CommonTransitions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Root cause of Android rendering coming back blank ("text missing from
many"): Container.paintComponent / Form.paintGlassImpl /
Component.paintIntersectingComponentsAbove / Component.drawPainters
($FLAT branch) / TextSelection.paint /
Graphics.beginNativeGraphicsAccess all call g.setTransform(null) in
their matrix-mode branches. Graphics.setTransform was forwarding the
null straight to impl.setTransform, and AndroidAsyncView.setTransform
NPEs unconditionally on its argument (getTransform().setTransform(t)
reads t.type / t.scaleX / etc with no null guard), tanking the entire
paint frame.
Route setTransform(null)/setTransform(identity) through impl.resetAffine
in matrix mode -- every port already implements that as
"matrix -> identity" without dereferencing a Transform.
Also add a spotbugs-exclude entry for Graphics.useMatrixTranslation
covering MS_SHOULD_BE_FINAL and UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD:
the field must stay mutable (app init code flips it once at startup)
and public (sole opt-in knob), but SpotBugs flags it because the only
writes happen in *other* modules (e.g. HelloCodenameOne.kt in the
hellocodenameone-common module) that the core module's analysis
doesn't see.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…leaks
In matrix mode setTransform was setting userTransform = transform.copy()
for non-identity transforms. That field is part of the legacy
T(xt) * M * T(-xt) conjugation machinery; matrix mode does not conjugate,
so the field served no purpose -- and it leaked.
Concrete leak: Container.paint matrix-mode tail does
Transform saved = g.getTransform();
g.setTransform(null); // routes to impl.resetAffine
paintGlass(g); paintTensile(g);
g.setTransform(saved); // here userTransform = saved
... outer caller does g.translate(-getX(), -getY()) ...
The trailing translate composes T(-getX, -getY) onto the IMPL matrix but
does not touch userTransform, so userTransform stays at the pre-translate
value forever. Any subsequent g.getTransform() in matrix mode hit the
"if (userTransform != null) return userTransform.copy()" early-return
and returned the stale snapshot instead of the actual impl matrix --
which is exactly what framework save/restore patterns rely on.
Fix: in matrix mode setTransform, always leave userTransform null. The
impl matrix is the only source of truth, and getTransform reads from
it directly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tate Root cause of the Android-specific "missing text" / blank-Theme-test breakage in matrix mode: AsyncGraphics overrides scale, rotate, setTransform, and resetAffine so each one (1) updates the AsyncGraphics local transform field used for clip / bounds checks and (2) pushes an AsyncOp into pendingRenderingOperations so the underlying canvas gets the same transform mutation at playback time. But it did NOT override translateMatrix -- so calls inherited AndroidGraphics.translateMatrix, which only updates the local Transform and does not queue an op. The result: with Graphics.useMatrixTranslation = true, every framework g.translate(absX, absY) (status bar height, title-area height, every container in the chain) routes through impl.translateMatrix; the async-view's local transform sees them, but the queued draw ops play back against an underlying canvas matrix that never received them. Form chrome and Theme components paint at screen origin with no offset; nested containers render outside the visible clip. Override translateMatrix in AsyncGraphics to mirror the scale path: update getTransform(), flip the same dirty flags, and queue an AsyncOp whose execute() calls underlying.translateMatrix(x, y). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…chor Previous matrix-mode setTransform replaced the impl matrix wholesale, wiping the framework painting-chain translates. The TransformRotation test (Transform.makeIdentity().translate(cx, cy).rotate(45deg) .translate(-cx, -cy) then g.setTransform(t) then g.fillRect(cx-25, cy-25, 50, 50)) computes (cx, cy) as PARENT-LOCAL coords (bounds.getX() + 100). In legacy mode setTransform conjugated as T(xt) * M * T(-xt) where xt was the framework painting-chain translate, so the rotation ended up centered at screen (xt + cx, yt + cy). In the old matrix-mode path the impl matrix became just t, drawing the rotated rect at screen (cx, cy) -- inside the form-origin coordinate system, not the panel. Result: the four panel grids drew their rotated diamonds at the wrong screen position; the top two went off-screen and the bottom two appeared in the same spot. Same root cause behind graphics-affine-scale, graphics-scale, graphics-tile-image, graphics-transform-translation, FloatingActionButton (BorderLayer painter calls setTransform). Fix: maintain an INTERNAL matrixFrameworkX/Y shadow accumulator that mirrors the framework painting-chain translates -- bumped by g.translate(int, int) in matrix mode. It is NOT exposed via getTranslateX/Y (so bypass-Graphics paths like Label.drawLabelComponent and BGPainter.paintComponentBackground keep passing local coords and don't double-shift). It IS used by setTransform / resetAffine to rebuild the impl matrix as T(matrixFrameworkX) * userTransform (no terminal T(-matrixFrameworkX) because vertex coords aren't pre-shifted in matrix mode). - translate(int, int) in matrix mode: bumps matrixFrameworkX/Y AND calls impl.translateMatrix. If userTransform is set, recomposes the impl matrix so subsequent draws use T(new framework) * userTransform. - setTransform(M) in matrix mode: impl matrix = T(matrixFrameworkX) * M. Non-identity M sets userTransform; null/identity restores impl matrix to T(matrixFrameworkX). - resetAffine in matrix mode: impl matrix = T(matrixFrameworkX) (same recipe as setTransform(null)). Note: matrixFrameworkX/Y bumping happens on every g.translate inside the framework painting chain. It also bumps on user g.translate calls inside paint(). That matches legacy's xTranslate behavior, which also included user-issued g.translate in the conjugation anchor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…oords drawPeerComponent's punch-hole clearRect bypasses the tx()/ty() shift and passes peer.getAbsoluteX/Y straight to impl.clearRect. iOS NativeGraphics.clearRect calls applyTransform before clearing, so in matrix mode the impl matrix that already encodes the framework painting-chain translates would apply on top -- double-translating the cleared rect to (framework + absX). The native peer would appear as a solid background-color rect over the canvas because the punch hole landed off-screen. In matrix mode subtract matrixFrameworkX/Y so impl sees local coords; the matrix T(framework) then puts the cleared rect back at the peer's true screen-absolute position. Legacy unchanged (matrix is identity, so passing absolute coords directly is correct). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…et callsites to translate-based pattern
Previous matrix-mode Container.paint / Form.paintGlassImpl /
Component.paintIntersectingComponentsAbove / drawPainters / TextSelection
used Transform.getTransform + setTransform(null) + setTransform(saved)
to snapshot/reset/restore the impl matrix during glass and overlay
painting. That ran into a self-defeating problem with my recently-added
setTransform conjugation: setTransform(saved) where `saved` came from
g.getTransform() ALREADY contained T(matrixFrameworkX) baked in, and
the matrix-mode setTransform re-conjugated by T(matrixFrameworkX) --
double-translating the restore and tanking Android paint to 1/108 in
the previous CI run.
Fix: expose matrixFrameworkTranslateX/Y -- a public framework-internal
getter that returns xTranslate in legacy mode and the matrixFrameworkX
shadow in matrix mode. Revert every snapshot-reset callsite to the
legacy translate-based recipe:
int tx = g.matrixFrameworkTranslateX();
int ty = g.matrixFrameworkTranslateY();
g.translate(-tx, -ty); // matrix -> identity
paintAtScreenAbsolute(g); // glass / tensile / parent walk / FLAT
g.translate(tx, ty); // restore
g.translate in matrix mode composes onto the impl matrix AND bumps
matrixFrameworkX, so this pattern brings the impl matrix back to
identity (matrixFrameworkX -> 0) for the inner paint, then restores
both. No setTransform involved -> no double conjugation.
Callsites updated:
- Form.paintGlassImpl
- Container.paintComponent tail
- Component.paintIntersectingComponentsAbove
- Component.drawPainters $FLAT branch
- TextSelection.paint
- FontImage rotation (both drawString and drawImage variants)
- Graphics.beginNativeGraphicsAccess
- CodenameOneImplementation paint-queue wrapper reset
- MapComponent.paint zoom path
- Scene.paint
- CommonTransitions TYPE_PULSATE_DIALOG
- Graphics.drawPeerComponent (peer punch-hole clearRect; matrix mode
still has to subtract matrixFrameworkX since clearRectImpl bypasses
tx() shifting -- the impl matrix would otherwise translate the
punch-hole to the wrong screen position)
Matrix-mode setTransform / resetAffine keep their conjugation: user
code like TransformRotation.drawContent that builds Transform.makeIdentity
.translate(cx, cy).rotate(...).translate(-cx, -cy) and calls
g.setTransform(t) continues to render the rotation centered on the
local pivot at screen (framework + cx, framework + cy).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…shot-reset callsites to translate-based pattern" This reverts commit 9a5ae15.
Three commits between 7af8707 (AsyncGraphics translateMatrix fix that brought Android 21->93 matched) and HEAD introduced regressions that dropped iOS/Android/iOS-Metal to ~1-2 / 108 matched: 501fc54 conjugate user setTransform around framework anchor c237464 subtract framework anchor from peer clearRect coords 9a5ae15 framework anchor accessor + revert snapshot-reset callsites bc0da53 revert "framework anchor accessor + ..." The bc0da53 revert undid 9a5ae15 but the conjugation in 501fc54 (matrixFrameworkX shadow + setTransform composition) was still active and is itself the root cause of the iOS/iOS-Metal/Android regressions. Rather than chase more commits that interact, restore Graphics.java to the 7af8707 revision -- the last known-good state that scored Android 93/108. Other files were unchanged across those intermediate commits, so a single-file revert is sufficient. From here we can address the specific remaining issues at 93/108 (Picker / FAB / graphics-affine-scale / graphics-rotate / graphics-scale / graphics-transform-rotation / graphics-transform- translation / graphics-tile-image / BrowserComponent) more carefully without taking on the broad setTransform-conjugation cost that broke the working tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ation
In matrix mode the impl matrix carries the framework painting-chain
translates that legacy keeps in xTranslate/yTranslate, so the legacy
snapshot-reset pattern -- "translate(-getTranslateX, ...) ; paint ;
translate(+getTranslateX, ...)" -- no-ops because getTranslateX is
zero. Earlier attempts replaced those callsites with getTransform/
setTransform pairs, but that double-translates whenever a user
setTransform call was issued in between (the saved snapshot already
encodes the framework anchor, and setTransform re-applies it).
This patch keeps the legacy translate-based recipe in both modes by
mirroring the framework chain into an internal matrixFrameworkX/Y
shadow:
- Graphics.translate(int,int) in matrix mode bumps the shadow,
composes T(x,y) onto the impl matrix, and re-applies userTransform
conjugation when one is active.
- Graphics.setTransform(M) conjugates as T(matrixFrameworkX) * M so
user transforms apply in screen-absolute coords around the
framework anchor, matching legacy semantics.
- Graphics.resetAffine() preserves the framework anchor in matrix
mode by re-applying T(matrixFrameworkX, matrixFrameworkY) after
the impl resets to identity.
- Graphics.getTransform() returns identity (not impl.getTransform)
in matrix mode so snapshot/restore patterns don't feed the
framework anchor back into the conjugation step.
- Graphics.drawPeerComponent subtracts matrixFrameworkX from peer
absolute coords so the punch-hole clearRect lands at the right
screen position (the impl matrix already encodes the shift).
The new public accessor `matrixFrameworkTranslateX/Y` returns the
right framework anchor for the current mode (xTranslate in legacy,
matrixFrameworkX in matrix mode). Snapshot-reset callsites that
previously branched on `Graphics.useMatrixTranslation` now use this
accessor uniformly:
- Form.paintGlassImpl
- Container.paintComponent tail
- Component.paintIntersectingComponentsAbove
- Component.drawPainters $FLAT branch
- TextSelection span fills
- FontImage rotation (both call sites)
- Graphics.beginNativeGraphicsAccess
- CodenameOneImplementation paint-queue wrapper reset
Bypass-Graphics fast paths (Label.drawLabelComponent,
BGPainter.paintComponentBackground) keep reading getTranslateX,
which intentionally stays at zero in matrix mode -- those paths
hand coords to impl.drawLabelComponent directly and would double-
shift if the matrix anchor leaked through getTranslateX.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AndroidPeer.paint computed the WebView/PeerComponent screen-absolute position as `getX() + g.getTranslateX()`. In matrix mode getTranslateX is intentionally zero (the framework painting-chain translates live in the impl matrix, not the integer accumulator), so the peer was being laid out at the component's local-coords origin instead of its screen-absolute origin -- visible as BrowserComponent rendering with the top of its HTML content clipped above the visible area. Switch the three callsites to `g.matrixFrameworkTranslateX/Y` which returns getTranslateX in legacy mode and the matrixFrameworkX shadow in matrix mode, so the peer lands at the right screen pixel in both. Promote two Android goldens where matrix mode renders correctly and the legacy golden encoded a bug: - graphics-inscribed-triangle-grid: the top-left panel (form-direct, non-AA) in legacy mode showed only the largest cell because the per-Graphics integer translate accumulator multiplied through the subsequent g.scale -- which is exactly the GH-3302 bug this whole branch fixes. The test was written using g.translateMatrix so it *should* produce identical pixels across the four render paths, and matrix mode now does. - graphics-translate-then-scale: new test, no prior reference. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
drainPendingDisplayFrame does: context.save(); context.beginPath(); context.rect(cropX, cropY, cropW, cropH); context.clip(); The rect+clip is evaluated under whatever transform the canvas state had at save() time. In matrix mode the Graphics layer now stashes T(framework_anchor) (and any leftover user transform) in the canvas state, and ClipShape ops can also leave a setTransform behind that the outer save/restore preserves across drains. The crop ends up as a translated/rotated polygon -- ops in this drain paint under the leaked transform AND through the wrong clip, and Toolbar/title pixels never land inside it (the chart-line, chart-pie and chart-rotated-pie JS regressions, plus graphics-large-stroke-dirty- clip). Force identity AFTER save() and BEFORE the crop rect+clip. The per- op SetTransform queue inside this drain still sets the per-paint transform as before, and the outer restore() pops back to whatever pre-drain state was active. This is the same fix that landed on origin/moving-initializr-to-new- js-port as 339ecfd / e273a74 for the legacy graphics-clip-under- rotation rotation-leak repro; matrix mode trips the exact same code path much more reliably because every paint now writes a non-trivial transform to the canvas state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
BufferedGraphics shadows HTML5Graphics's `transform` field with its own and overrides rotate/scale/setTransform/etc. accordingly. It never overrode translateMatrix, so calls to translateMatrix on the form's main graphics inherited the parent's implementation, which mutated the parent's `transform` field instead of the BufferedGraphics- side one. The parent's field is not read by the queued SetTransform ops, so every translateMatrix call silently no-op'd. This is harmless when only a couple of test sites call translateMatrix directly (InscribedTriangleGrid was the only repro on master). Under the new matrix-mode Graphics layer it becomes severe: every framework painting-chain g.translate is now routed through translateMatrix, so Toolbar titles, BorderLayout-CENTER-only forms (chart-pie, chart- rotated-pie etc., graphics-large-stroke-dirty-clip), and any component whose screen position depends on accumulated parent translates render at (0,0) on the form's canvas and get clipped / overdrawn. Override translateMatrix on BufferedGraphics so the right `transform` field receives the composition, mirroring how scale/rotate already work. This is the cherry-pick of origin/moving-initializr-to-new-js-port's 4d17766; that branch needed it for the same master-API merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Update the 17 stale JavaScript-port screenshot goldens that the
matrix-mode Graphics layer now renders differently. Matrix mode now
brings JS in line with the iOS rendering pipeline -- specifically,
the StatusBar component above the Form's Toolbar now paints
(matching the iOS golden) instead of collapsing to zero height as
the legacy JS path did. Side-by-side comparison with the existing
iOS goldens confirms the new JS pixels are the correct rendering
of each test, not a regression.
Tests promoted:
- chart-bar, chart-bar-stacked, chart-bubble, chart-combined-xy,
chart-cubic-line, chart-doughnut, chart-line, chart-pie,
chart-radar, chart-range-bar, chart-rotated-pie, chart-scatter,
chart-time, chart-transform
- graphics-clip-under-rotation (matrix mode no longer leaks the
rotation transform to the form title bar)
- graphics-inscribed-triangle-grid (master JS golden was the
GH-3302 bug -- all four panels now render the 2x2 inner grid
correctly via the BufferedGraphics.translateMatrix override)
- graphics-large-stroke-dirty-clip (title bar paints again now
that drainPendingDisplayFrame resets to identity before the crop
clip is set)
Added (no prior reference):
- graphics-translate-then-scale
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
…dens
CN1MetalBeginMutableImageDraw / End save/restore the screen encoder,
projection, framebuffer dimensions and stencil reference around a
mutable-target side-trip on the drain queue. Up to now they did NOT
save/restore currentTransform -- the file-static simd_float4x4 that
every SetTransform ExecutableOp's execute() mutates via
CN1MetalSetTransform. Once matrix-mode g.translate routes every
framework painting-chain translate through SetTransform, the mutable
side-trip's last SetTransform op leaks into every screen draw queued
after the side-trip ends:
- FloatingActionButton's lazy icon FontImage build runs while the
layered pane is being painted; the FAB's own drawLabelComponent
after that came up against the icon-build's transform and landed
off-screen entirely (FAB invisible).
- graphics-tile-image's first panel (form-direct non-AA) ran AFTER
the four cells had each cycled through mutable-target draws, so
its bottom half painted at a wrong y offset and showed as empty.
- Switch's Track/Thumb mutable-image build (createPlatformTrack/
Thumb -> Image.createImage -> drawing into the mutable) trips
the same path for every Switch on every paint -- visibly worse
in matrix mode because the mutable's SetTransform now carries
the framework anchor, not just a per-component scale/rotate.
Save currentTransform in Begin alongside the other side-trip state
and restore it in End so screen-target ops dequeued after End read
the form's framework_anchor back, not the mutable's coord system.
Goldens promoted (iOS Metal matched after the fix, no remaining diff):
- FloatingActionButtonTheme_light / _dark: FAB now renders in the
bottom-right corner with its blue + icon.
- graphics-tile-image: all four panels paint the blue tiled pattern.
- graphics-inscribed-triangle-grid: matrix-mode actual matches the
Android/JS promotion -- legacy iOS Metal golden encoded the same
GH-3302 form-direct non-AA clipping bug.
- graphics-translate-then-scale: new test, no prior reference.
Note: SwitchTheme_light / _dark and kotlin still show their Switch's
track at the wrong y -- gausianBlurImage on iOS Metal calls
flushBuffer mid-paint to read back the mutable's MTLTexture, which
drains the queue and resets currentTransform via CN1MetalSetFramebuffer.
The Java-side NativeGraphics.transformApplied flag is stale across
that flush (still true even though the Metal side was wiped), so the
next form draw queues without a preceding SetTransform op. That is a
separate bug and needs its own fix; not blocking the FAB / tile-image
/ goldens promotion above.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The native gausianBlurImage path used to drain the entire upcoming
ExecutableOp queue via flushBuffer just to make sure the mutable's
shadow-ring fillArc ops had hit the GPU before
CN1MetalReadMutableImageAsUIImage sampled the texture. The global
flush was massively over-broad:
- flushBuffer triggers a full drawFrame, which calls
CN1MetalBeginFrame and resets the global currentTransform to
identity. Form-side NativeGraphics.transformApplied is still
`true` across that round-trip (Java has no idea the native
side has been wiped), so the next form draw queues with no
preceding SetTransform op. At the next normal drawFrame the
drain executes that form draw against currentTransform=identity
-- visible as Switch's track/thumb rendering at screen
(local_x, local_y) instead of the component's screen position
on every Switch in matrix mode (SwitchTheme / kotlin).
- flushBuffer also drains form ops that the form is still in the
middle of accumulating, ending the mid-paint frame with all
of them already executed. The next drawFrame then has only
the form ops queued AFTER the blur, missing the SetTransform
sequence that originally led up to them.
Add a targeted flushOpsForMutableImage:(GLUIImage*)image entry
point that walks the upcoming queue, extracts only the ops whose
target == the specific image being blurred, opens a fresh mutable
encoder for it, executes the extracted ops there, and commits.
Everything else stays in upcomingTarget, in order, for the next
normal drawFrame to drain. CN1MetalFlushMutableImageSync(image)
called inside CN1MetalReadMutableImageAsUIImage then has a
committed command buffer to waitUntilCompleted on, so the readback
sees the painted pixels just like before.
Replace the global flushBuffer call in IOSNative.m's gausianBlurImage
with the targeted drain. Matrix-mode iOS Metal screenshot tests that
contain a Switch (SwitchTheme_light/_dark, kotlin) now render every
switch at its correct y; the FAB / tile-image / graphics-inscribed-
triangle-grid fixes from 59d3b14 are preserved. Goldens promoted.
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
Adds a static
Graphics.useMatrixTranslationopt-in that flips everyg.translate(int, int)in the rendering chain through the impl-side affine matrix (viatranslateMatrix) instead of the legacy per-Graphics integer accumulator that was added to draw coords. Closes the residual Y-axis drift from GH-3302's direct-to-screen mode.Why
The framework's container/component painting pipeline stacks
g.translate(absX, absY)calls before userpaint()runs. With the legacy integer accumulator a userg.scale(sx, sy)insidepaint()multiplied the chrome offset (status-bar + title-area height), pushing direct-to-screen drawing off the Y axis. Buffered (mutable-image) drawing avoided this because the offscreen Graphics started withxTranslate=0.With the flag on AND
impl.isTranslateMatrixSupported() == true(iOS, Android, JavaSE, HTML5),translate(int, int)composes onto the impl matrix the same wayscale/rotatedo.xTranslate/yTranslateis still bumped as a shadow accumulator so existinggetTranslateX/getTranslateYconsumers inForm/Component/Label/Container/FontImage/LinearGradientPaint/TextSelectionkeep returning legacy values; the drawing primitives no longer add it (the matrix carries it).Ports that opt out (default
isTranslateMatrixSupported() == false) keep using the legacy path regardless of the flag — no port needs lockstep changes.Changes
Graphics.java: flag +matrixMode()/tx/tyhelpers; 25 primitive sites switched fromxTranslate + xtotx(x); shape pre-shift gated insetClip(Shape)/drawShape/fillShape/fillPolygon/drawPolygon;getClipX/Yskips- xTranslatein matrix mode;rotateRadianspivot compensation is a no-op;setTransformcomposesT(xt,yt) * Mwithout the legacy terminalT(-xt,-yt).TestCodenameOneImplementation:setTranslateMatrixSupported+translateMatrixtracking so unit tests exercise both paths.GraphicsTest: 4 new tests covering matrix-mode push-through, no-pre-shift, opt-out fallback, shape pre-translate skip. Full unittests suite (2429 tests) green.HelloCodenameOne.init(): flipsGraphics.useMatrixTranslation = trueso the screenshot suite exercises matrix mode end-to-end.TranslateThenScalescreenshot test underhellocodenameone graphics/for the direct-to-screeng.translate + g.scalerepro from the issue.Test plan
hellocodenameone-commoncompiles with the flag flip (JDK 17)codenameone-core-unittestssuite — 2429 / 2429 passgraphics-inscribed-triangle-gridform-direct panels and any test mixing framework container translates with userg.scale/g.rotatein direct-to-screen mode. Mutable-image panels unaffected. Goldens to be promoted after visual review.🤖 Generated with Claude Code