Graphics.useMatrixTranslation: route g.translate through impl matrix (GH-3302)#4960
Graphics.useMatrixTranslation: route g.translate through impl matrix (GH-3302)#4960shai-almog wants to merge 13 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>
JavaScript port screenshot updatesCompared 18 screenshots: 0 matched, 17 updated, 1 missing reference.
|
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Android screenshot updatesCompared 108 screenshots: 93 matched, 14 updated, 1 missing reference.
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>
iOS Metal screenshot updatesCompared 108 screenshots: 88 matched, 19 updated, 1 missing reference.
Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
iOS screenshot updatesCompared 108 screenshots: 92 matched, 15 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>
1 participant
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