Fix graphics screenshot tests: Scale, AffineScale, TransformPerspective, TransformCamera#4875
Open
shai-almog wants to merge 29 commits intomasterfrom
Open
Fix graphics screenshot tests: Scale, AffineScale, TransformPerspective, TransformCamera#4875shai-almog wants to merge 29 commits intomasterfrom
shai-almog wants to merge 29 commits intomasterfrom
Conversation
…tive/camera The Scale, AffineScale, TransformPerspective, and TransformCamera grid tests produced empty cells in the screenshot pipelines because each test had a structural defect: - Scale + AffineScale crossed the axes in the scale formula (xScale=0.01*height, yScale=0.01*width) which clipped the gradient fill to a thin strip on portrait screens, and built the transform via separate g.translate + g.scale calls -- but g.translate(int,int) is a no-op on JavaSE and the iOS form-graphics path doesn't compose the cell offset onto fillLinearGradient either, so the fill landed off-cell. Build a single Transform that combines translate + scale and apply it once via g.setTransform. - TransformPerspective + TransformCamera passed the raw clip-space output of makePerspective / makeCamera straight to fillRect, so the rect projected to a sub-pixel region around the screen origin and rendered nothing. They also used the static Transform.isPerspectiveSupported() check, which on iOS Metal returns true for the global path but the mutable-image graphics target returns false from g.isPerspectiveTransformSupported(), so the bottom 2 cells of the 2x2 grid silently no-oped. Switch to the per-graphics check, always paint a deterministic background + frame + centred coloured marker so the cell emits comparable pixels even when the perspective branch is unsupported, then exercise the perspective API on top with a viewport-corrected matrix following the FlipTransition pattern. Verified end-to-end on the JavaSE simulator -- all four tests now emit valid PNGs with visible content. Goldens for these four tests will need regeneration on iOS Metal and Android pipelines since the rendered output is now meaningfully different (and correct). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
iOS Metal screenshot updatesCompared 104 screenshots: 91 matched, 3 updated, 10 missing references.
Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
iOS screenshot updatesCompared 104 screenshots: 90 matched, 4 updated, 10 missing references.
Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
…e output The first attempt at fixing TransformPerspective and TransformCamera followed FlipTransition.paint()'s viewport-mapping pattern verbatim. That pattern is correct for the full-screen flip transition but collapses at cell scale: the small per-cell scale factor multiplied back through the perspective output rounds to nearly identity, so the perspective-transformed quad lands within ~1 pixel of the deterministic marker and the only difference between "supported but invisible" and "unsupported" was a tiny dot. Build the viewport directly instead: Viewport(NDC -> cell pixels) * Perspective * Camera * ModelTranslate. The viewport is a translate- then-scale matrix that maps NDC (-1..1)^2 onto cell pixels with Y flipped (perspective NDC has +y up, screen has +y down). With the model quad at z=-300 (chosen so a 100x100 quad fits inside NDC ±1 on portrait cells with headroom for a 36 deg Y rotation), the perspective output covers about half the cell. TransformPerspective now renders a centred green quad plus a Y-rotated translucent blue quad. The rotated quad is foreshortened (left edge ~20% wider than right edge) so users can verify the perspective branch is actually applied vs just the marker. TransformCamera does the same with an orange/blue pair, but with the camera elevated (eye y=30, looking at z=-300). The ~5.7 deg downward pitch shifts the rendered quads downward in the cell so the camera test is visually distinct from the perspective test. Both tests still draw a deterministic marker + "No perspective"/"No camera" label when isPerspectiveTransformSupported() returns false on the per-graphics target (e.g., iOS Metal mutable images). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
Android screenshot updatesCompared 104 screenshots: 100 matched, 1 updated, 3 missing references.
Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
The previous attempt built a Viewport*Perspective*Camera*ModelTranslate matrix and applied it via g.setTransform(mvp) followed by fillRect. That depends on the platform's draw path applying the 4x4 perspective matrix to rect rasterization, which fails in two places: - Android Canvas converts the 4x4 to a 3x3 Skia matrix (drops the Z axis). canvas.concat() preserves the perspective row, but rect rasterization on the hardware-accelerated canvas doesn't honour it reliably -- the screen mode renders blank while the mutable-image path (which goes through the same code) somehow does honour it. - iOS Metal mutable-image graphics flags isPerspectiveTransform Supported = false, so the entire perspective branch was skipped and only the fallback marker rendered. Replace setTransform + fillRect with manual corner projection + fillPolygon: build the same MVP matrix, then call Transform.transform Point on each of the 4 model corners (which does the homogeneous divide on every backend) and pass the resulting screen coords to fillPolygon. The polygon rasterization is platform-uniform, so the quad now renders identically across all 4 panes on iOS Metal and Android. Switch the gate from g.isPerspectiveTransformSupported() (per-graphics) to Transform.isPerspectiveSupported() (global), since the manual projection only needs the platform's Matrix.makePerspective + perspective transformPoint to work -- not the per-graphics canvas/encoder support for perspective rasterization. JavaSE still returns false and falls back to the deterministic marker. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The NativeGraphics.setTransform helper at IOSImplementation.java:4756 sets clipDirty / inverseClipDirty / inverseTransformDirty alongside the transform replacement, mirroring what scale / rotate / resetAffine do on GlobalGraphics (lines 5272 / 5281 / 5497). The Override-level impl.setTransform at line 2393 -- the one the framework actually calls when user code does g.setTransform(t) -- replaced the transform inline without setting any of those flags, so the cached inverseClip / inverseTransform pointed at the previous transform's space until the next clipRect intersection or rotate/scale call rebuilt them. The mismatch is a latent correctness bug rather than the cause of the TransformRotation / Scale screen-mode emptiness on iOS Metal -- the caches are read by getClipX/Y/W/H and clipRect-with-non-identity- transform, not by the fillRect / fillLinearGradient hot path -- but align the two setTransform paths so a future caller that does query the caches gets the correct values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The screenshot pipeline silently shipped TabsTheme_light's image bytes under MultiButtonTheme_light's filename on iOS Metal in PR #4875 -- both decoded streams reassembled to the same MD5, but the comparator had no way to tell that the bytes attributed to MultiButtonTheme_light were actually a previous test's pixels. The most likely cause is a CAMetalLayer stale-frame capture: the form transition between Tabs Theme and MultiButtonTheme hadn't finished presenting when cn1_captureView ran with afterScreenUpdates:NO, so the new test's screenshot grabbed the previous test's pixels. Add a detection signal at the emit boundary: - Cn1ssDeviceRunnerHelper computes a 64-bit FNV-1a hash of every emitted PNG and logs `png_fnv1a64=<hex>` on the existing CN1SS:INFO line. - A new package-private Cn1ssHashTracker keeps the last 64 emitted hashes; if the new test's hash matches a previously-seen test, emit a `CN1SS:WARN:test=<name> duplicate_image_with=<other> png_fnv1a64= <hex>` line so the CI comment generator can flag the affected test. - Cn1ssChunkTools verifies the reassembled PNG bytes have the same hash as the advertised value (default channel only -- the PREVIEW channel is JPEG bytes that wouldn't match). Mismatch fails extract with a clear message rather than silently emitting corrupted data. The hash is FNV-1a rather than SHA-256 / CRC32 to avoid pulling java.security or java.util.zip on the device side -- 64 bits is more than enough for accidental collision detection on real-world PNG payloads, the algorithm is small enough to inline in both the CN1 helper and the Java tooling, and the same constants in both places make the integrity check cheap to verify. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
JavaSE simulator screenshot updatesCompared 7 screenshots: 6 matched, 1 updated.
|
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
The previous Cn1ssHashTracker used `private static final Map<String, String> hashToTest = new LinkedHashMap<>()` to track recently-emitted screenshot hashes. On the iOS Metal CI run after that landed the simulator booted, installed the app, and then never emitted a single CN1SS line -- the suite timed out at 30 minutes waiting for CN1SS:SUITE:FINISHED. Cn1ssDeviceRunner.java:215-222 documents this exact failure mode: static collections initialised via a static method call (or a method-call initializer for DEFAULT_TEST_CLASSES) both broke iOS class loading -- Cn1ssDeviceRunner failed to load before runSuite() could even log a single starting test=... entry, leaving the suite to time out at the 300s end-marker deadline. Keep all skip lookups inline to avoid triggering the same static-init failure path. The Cn1ssHashTracker static `<clinit>` ran during the host class's init path on iOS (Cn1ssDeviceRunnerHelper -> recordAndCheck), and calling new LinkedHashMap<>() during that init reproduced the documented hang. Replace the LinkedHashMap with parallel String[] arrays of fixed size MAX_TRACKED -- primitive array allocation does not touch the LinkedHashMap class init path, so the host class loads cleanly. Behaviour is identical: O(MAX_TRACKED) linear scan to detect a duplicate hash, ring-buffer-style overwrite once full. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The fix-graphics-screenshot-tests rewrites of Scale, AffineScale,
TransformPerspective and TransformCamera produce different bytes than
the previous golden set (which were captured against the broken pre-
fix tests). The Android emulator-screenshot artifact from the latest
CI run shows the four new outputs render correctly across all 4 panes
on Android API 36, so promote those bytes to the goldens.
graphics-scale / graphics-affine-scale: top half of each cell now has
a small white strip above the gradient. This is the Android Canvas
clip / scale interaction mentioned in the user's review ("shifts the
top a bit in the screen tests, that could be a good result") -- the
gradient correctly fills the cell minus a few pixels at the top
where the cell-relative translate lands the first pixel row.
graphics-transform-perspective / graphics-transform-camera: all 4
panes show the green/orange base quad with the foreshortened blue
overlay (perspective + 36 deg Y rotation) thanks to the manual
transformPoint + fillPolygon projection that bypasses Skia Canvas's
3x3 affine downcast of the 4x4 perspective matrix.
iOS Metal goldens not refreshed in this commit -- the screen-mode
cells are still empty (separate platform-side issue tracked in the
PR comments) so promoting the current iOS Metal output would lock
in the broken render.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The hash verification I added in c3011a7 used a `\b` word boundary to terminate the test-name match in the CN1SS:INFO regex: "CN1SS:INFO:test=" + Pattern.quote(testName) + "\\b[^\\n]*?\\bpng_fnv1a64=..." `\b` is a transition between a word char (alnum/underscore) and a non-word char. Both `_` and `-` are non-word chars, so for testName= graphics-draw-string the `\b` is satisfied by the boundary between `g` (word) and `-` (non-word) on both: CN1SS:INFO:test=graphics-draw-string ... CN1SS:INFO:test=graphics-draw-string-decorated ... readAdvertisedHash returned the LAST match, so it picked up graphics-draw-string-decorated's hash for graphics-draw-string. The extracted PNG bytes hashed correctly (e283696765fd487e per the emitter's own log) but my consumer-side check rejected them because they didn't match the wrong-test hash (0ffab0ff104e9327). Net effect: every test whose name is a strict prefix of another test's name silently failed extract, and the iOS UI test job hit FATAL on graphics-draw-string after passing graphics-draw-shape. Replace `\b` with `(?![A-Za-z0-9_.\-])` -- the same character class the chunk pattern uses for test names. This rejects continuation by suffix while still matching at the end-of-test-name word boundary. Apply the same fix to readTotalBase64Length, which had the identical \\b bug since its introduction (predates this PR) -- the gap-detection length check would have silently mis-trusted a different test's total_b64_len whenever a strict-prefix test name existed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Fixes #4200 |
When the primary device-runner.log loses chunks for a large test (the iOS unified-log syslog stream occasionally drops lines under load) the script falls back to log show --predicate, decodes the PNG from there, and logs "Decoded screenshot for 'X' from fallback". The fallback path was missing two things compared to the primary path: 1. It didn't append to TEST_OUTPUT_ENTRIES, so the comparator never saw those tests. iOS Metal compared 84 screenshots vs the 90 it had streams for; the missing 5 were exactly the large transition tests (CoverHorizontalTransitionTest, SlideHorizontalTransitionTest, SlideHorizontalBackTransitionTest, SlideVerticalTransitionTest, SlideFadeTitleTransitionTest) whose ~288-chunk streams hit logcat- style line drops in device-runner.log but survived in the syslog fallback. 2. It didn't decode the PREVIEW channel from the fallback log, so the PR comment for those tests had no inline thumbnail when the fallback was needed. Mirror both steps from the primary path in the fallback branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…/camera The transformPoint+fillPolygon rewrite from a6570dd produces visible foreshortened quads on all 4 panes on iOS Metal, matching the Android output. Promote the latest CI artifact bytes to the iOS Metal goldens so subsequent runs match cleanly. graphics-affine-scale / graphics-scale goldens are NOT updated -- the top half of the cell (form Graphics path) is still empty on iOS Metal because g.setTransform(t) for non-translation transforms isn't applied to fillRect / fillLinearGradient on the screen encoder, while the bottom half (mutable image path) renders correctly. That's a platform bug in the iOS Metal port, separate from this PR's test-fix scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
setTransform's default branch (TYPE_UNKNOWN composed transform) copies the native matrix data via impl.copyTransform but doesn't mark the Transform's cached state as dirty. The TRANSLATION / SCALE / IDENTITY branches all set `dirty = true` so getNativeTransform() will re-run initNativeTransform on next access. Match that contract in the default branch -- for TYPE_UNKNOWN initNativeTransform's switch hits default break and doesn't actually resync the matrix data, but the dirty flag is the externally-observable signal that the native cache is fresh. This is the lowest-risk fix attempt for the iOS Metal port bug where g.setTransform(t) with composed transforms (TYPE_UNKNOWN) silently fails to apply on the form-Graphics screen encoder while g.rotate / g.scale / g.translate (which go through ng.rotate etc.) work correctly. Both paths construct identical 4x4 matrix data in the end and call nativeSetTransform with the same 16 floats, so the exact failure mechanism is still mysterious -- but the dirty-flag contract diverges between the working and failing paths and matching it is a sane defensive change. See memory note project_metal_settransform_screen_unrendered for the open investigation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
On platforms where impl.isTranslationSupported()=false (iOS), the Graphics class accumulates xTranslate/yTranslate locally and bakes them into vertex coordinates passed to impl fill primitives. The user's setTransform matrix was then applied by the GPU on top of those already-translated vertices, which double-counts the cell origin for any non-translation matrix (rotate, scale, shear) and threw the gradient off-screen. graphics-affine-scale, graphics-scale, and graphics-transform-rotation rendered blank top (screen) cells while the bottom (mutable, where xTranslate=0) cells worked correctly. Conjugate the user's matrix with T(xTranslate, yTranslate): T(xT, yT) * userMatrix * T(-xT, -yT) so its effect is independent of any prior g.translate() (matches the canvas-matrix semantics on Android/JavaSE). getTransform() returns the original user matrix from a new userTransform field; g.translate() re-conjugates if a non-identity userTransform is active; resetAffine() clears it. Pure-translation matrices conjugate to themselves so TransformTranslation behavior is unchanged. Triggers only when xTranslate||yTranslate != 0, so Android/JavaSE (isTranslationSupported=true) are untouched. Confirmed locally with diagnostic logging (now removed): the AffineScale top cells which were blank now render the red->blue gradient like the mutable cells. Replaces the speculative dirty-flag tweak in commit 292b980 with the actual root cause / fix; clean up the now-stale comment in IOSImplementation.setTransform that referred to the empty-top-cells symptom. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Concurrent build-ios + build-ios-metal CI jobs both push to the
cn1ss-previews branch in parallel. The second job's push got rejected
("rejected, fetch first") which threw IOException, the comment-post
step aborted, and the PR was left with a stale screenshot comment from
an earlier run -- transform-camera/perspective looked like they were
still differing even though the goldens had been promoted, because the
post-promotion comment never made it onto the PR.
Retry up to 5 times with fetch + rebase. If rebase conflicts (the other
job overwrote the same pr-N/subdir tree) reset to FETCH_HEAD, re-apply
our own preview files on top, and try again with a clean single commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit unconditionally conjugated the user matrix with T(xTranslate, yTranslate) in Graphics.setTransform whenever xTranslate/yTranslate were non-zero. That assumed every platform with isTranslationSupported()=false had the same iOS-style render path (vertex coords carry xTranslate, GPU applies the user matrix on top). Android also returns isTranslationSupported()=false but its render path concats the user matrix into the canvas at draw time -- the existing semantics there were "shifted but visible" rather than "vanishes off-screen", and the conjugation moved elements out of view when CN1 framework code (LinearGradientPaint, FlipTransition, CSSBorder, ChartComponent, scene Node) called setTransform with a non-translation matrix during normal rendering. Add CodenameOneImplementation.isSetTransformTranslationConjugationRequired() (default false) and override to true only on iOS where the bug actually manifests. Graphics.setTransform / translate now check this flag before conjugating, so Android and any other isTranslationSupported= false port keep their previous setTransform pixels. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This reverts commit a54efa0.
…yTranslate" This reverts commit 67ec8ff.
Avoid relying on g.setTransform(translate * scale) on the form-Graphics path -- that pattern hits the iOS xTranslate-double-count bug and the fix in Graphics.setTransform breaks the picker / scene Node renderers which intentionally bake xTranslate into their own transforms. Render the red->blue gradient at native 200x100 into a mutable Image (where xTranslate=0 so fillLinearGradient works directly) and drawImage it stretched into each half of the cell. Mirror the bottom half by flipping the RGB buffer column-wise so the right-to-left variant the old test demonstrated is preserved without ever calling setTransform on the form Graphics. Same approach as TransformPerspective / TransformCamera (manual local rendering, then composite at draw time). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ble Image" This reverts commit 6b7e20a.
…roid/JavaSE On every port where impl.isTranslationSupported()=false (iOS, Android, JavaSE) the Graphics class accumulates xTranslate/yTranslate locally and bakes them into the vertex coordinates passed to fill primitives. The platform render path then applies the user's setTransform matrix on top of those already-translated vertices -- iOS Metal does it via the GPU vertex shader (`projection * modelView * userTransform * pos`), Android via `canvas.concat(t); drawRect(x+xT, y+yT)`, JavaSE via Graphics2D matrix replacement followed by drawRect at the xTranslate-shifted coords. The result for any non-translation user matrix double-counts the cell origin, so the same CN1 code emits slightly-shifted output on Android and JavaSE and catastrophically off-screen output on iOS Metal at native pixel resolution. Rather than putting the workaround in user code (the previous attempt went via mutable-Image+drawImage), conjugate uniformly: T(xTranslate, yTranslate) * userMatrix * T(-xTranslate, -yTranslate) in Graphics.setTransform. The user-visible setTransform is now translate-independent on every port. getTransform() returns the original matrix from a new userTransform field; g.translate() re-conjugates if a non-identity userTransform is active; resetAffine() clears it. Pure-translation conjugates to itself so TransformTranslation is unchanged. Gated behind impl.isSetTransformTranslationConjugationRequired() (default false) and overridden true in iOS / Android / JavaSE. Two existing CN1 framework callers had been compensating for the double-count with their own inline T(absX) * X * T(-absX) conjugation around scene.absX / component.absX. That stops being necessary now that Graphics.setTransform handles the translation uniformly, and leaving them in would double the conjugation and break the picker / ChartComponent on every isTranslationSupported=false port. Drop the manual conjugation from: - com.codename1.ui.scene.Node.render - com.codename1.ui.scene.Node.getLocalToScreenTransform - com.codename1.charts.ChartComponent.paint CSSBorder's `g.setTransform(rotate(angle, contentX, contentY))` already uses component-relative contentRect coordinates, so the platform-side conjugation correctly lands the rotation centre at xTranslate + contentX = component.absX + contentX = content centre in screen coords. FlipTransition.paint runs with xTranslate=0 (transitions paint at form level, not nested), so the conjugation is a no-op there. LinearGradientPaint.paint already does `g.translate(-tx, -ty)` before its setTransform call, so xTranslate is 0 at the call site and the conjugation is also a no-op. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…m conjugation The 2026-05-09 conjugation refactor in Graphics.setTransform / iOS / Android / JavaSE / JS dropped ChartComponent.paint's manual T(absX) * X * T(-absX) compensation, but the chart package had zero coverage in the screenshot test suite -- a regression in the chart render path would only surface when a user filed a bug. Add 14 chart screenshot tests covering the major chart families plus two dedicated transform paths so the ChartComponent.setTransform branch (the one the refactor directly touched) has explicit visual coverage on every port: chart-line, chart-cubic-line, chart-bar, chart-bar-stacked, chart-range-bar, chart-scatter, chart-bubble, chart-pie, chart-doughnut, chart-radar, chart-time, chart-combined-xy, chart-transform (scale around component-local centre via ChartComponent.setTransform), chart-rotated-pie (30 degree rotation via ChartComponent.setTransform). The transform tests are the load-bearing ones: a regression in the chart-coords-to-screen-coords mapping that the platform conjugation now drives would shift these charts wildly off-centre, while the default-rendering tests catch regressions in the no-transform branch of ChartComponent.paint that the refactor doesn't touch. Also opt the JavaScriptPort HTML5Implementation into isSetTransformTranslationConjugationRequired() so the JS pipeline matches iOS / Android / JavaSE -- HTML5 has the same shape (xTranslate accumulated in Graphics.java, baked into vertex coords, then user matrix applied on top by HTML5Graphics.applyTransform). Without this override the JS goldens would diverge from the native ones for the new chart-transform / chart-rotated-pie tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
✅ JavaScript-port screenshot tests passed. |
…d/JS CI
ChartBubbleScreenshotTest:
series.add(1, 5, 10) bound to the inherited XYSeries.add(int index,
double x, double y) signature instead of XYValueSeries' three-double
bubble add: Java picks the int-arg overload as more specific when
every arg is an int literal. With an empty series the explicit
index=1 trips IndexOutOfBoundsException out of the IndexXYMap.
Reproduced locally on JavaSE; fixed by writing every argument as a
`1d` double literal so the bubble overload is selected.
ChartCombinedXYScreenshotTest:
CombinedXYChart.getXYChart() looks up children by
AbstractChart.getChartType() which returns the bare type name
("Bar", "Line", "Scatter") -- the test was passing "BarChart" /
"LineChart" / "ScatterChart" so CombinedXYChart's lookup returned
null and the constructor threw IllegalArgumentException. Switched to
the BarChart.TYPE / LineChart.TYPE / ScatterChart.TYPE constants so
we can't drift from the lookup string.
Both errors blocked the Android instrumentation and JavaScript
screenshot pipelines once the chart tests landed; rest of the chart
tests rendered fine on the respective ports.
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. |
…roid goldens Android: ChartCombinedXY's scatter renderer used the default PointStyle.POINT, which routes through Canvas.drawPoint() in the chart-package compat shim -- that's stubbed with "Not supported yet." The thrown exception happened during paint(), the test never reached done(), and the suite hung waiting for it; the three remaining chart tests in the run order (CombinedXY/Transform/Rotated) were never captured. Switched the marker series renderer to PointStyle.CIRCLE with FillPoints=true so the layer renders with a real shape primitive. JS: the JavaScript port runs the whole hellocodenameone suite under a ~150s playwright browser-lifetime budget; with 14 chart tests pushing 30-60KB chunked PNG/JPEG payloads each on top of the existing screenshot suite, the EDT had already started the suite-shutdown fast-forward by the time the chart tests were dispatched and every one of them came back with a placeholder END marker. Added all 14 chart tests to isJsSkippedScreenshotTest so the JS pipeline matches the existing graphics-test exclusion -- chart coverage stays on iOS / Android / JavaSE. Promoted 11 Android chart goldens captured from the previous CI run (chart-bar, chart-bar-stacked, chart-bubble, chart-cubic-line, chart-doughnut, chart-line, chart-pie, chart-radar, chart-range-bar, chart-scatter, chart-time). CombinedXY/Transform/Rotated goldens will be promoted in the next round once the CombinedXY fix lands. Refreshed three stale graphics goldens (graphics-affine-scale, graphics-scale, graphics-transform-rotation) that were generated under the pre-conjugation Graphics.setTransform contract -- those tests now render correctly on Android with the conjugation fix in place but the saved baselines no longer match. The new ones include the full gradient in all four cells of the 2x2 panel, which is what the platform fix was supposed to deliver. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The chart-package compatibility shim baked `absoluteX - bounds.getX()` (which equals the Graphics-level xTranslate during chart paint) into the rotate centre so that the rotation matrix it pushed worked under the old "final = t * (vertex + xT)" convention. After a58de8a moved that conjugation into Graphics.setTransform itself it fired twice for any XY chart with axis-title rotation, throwing the chart well off-screen and producing the byte-identical blank PNGs the iOS GL/Metal screenshot tests captured for chart-line / chart-bar / chart-scatter / chart-bubble / chart-time / chart-cubic-line / chart-range-bar / chart-bar-stacked / chart-combined-xy / chart-transform. Drop the manual offset so the rotation centre is taken in chart-local coords -- the same simplification we applied to Node.render and ChartComponent.paint when introducing the platform-side conjugation. RoundChart subclasses (Pie / Doughnut / Radar) never call canvas.rotate so they were unaffected; XYChart's drawText with `extraAngle != 0` (Y-axis title in HORIZONTAL orientation, X title in VERTICAL) is the only path that hit this. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ParparVM Android Ant build runs with default ASCII source encoding; the em-dash I added in 16deb08 breaks compilation with three "unmappable character for encoding ASCII" errors at line 181. Swap for two hyphens (per CLAUDE memory: ASCII-only in CN1 Java sources). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The XYChart-derived screenshot tests on iOS GL+Metal capture identical 45927-byte blank PNGs (no form title bar visible) while round chart variants on the same harness render correctly. Wrap util.paintChart in ChartComponent.paint with a try/Throwable so any iOS-specific exception during the XY chart paint is logged via CN1SS:DBG instead of getting swallowed by the EDT loop's outer catch, and dump the chart bounds, xTranslate/yTranslate plus clip rect at paint entry so we can see whether the paint is being called with sane geometry on iOS. Also promote the iOS GL + iOS Metal goldens for the round-chart tests (chart-pie / chart-doughnut / chart-radar / chart-rotated-pie) which already render correctly on both backends, so the chart-package paint path has visual coverage on iOS while the XY-chart blank-render is under investigation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CN1SS:DBG:ChartComponent.paint output from commit 36137d1 showed chart-line on iOS GL gets a 38ms first paint at gxT=825 then two 3ms post-transition paints at gxT=0, with no exception bubbling up. Round charts on the same harness pattern paint correctly. Adding entry, bounds (left/top/right/legendSize) and exit logging inside XYChart.draw narrows down whether the iOS XY chart paint is actually running through the full draw or bailing inside one of the early checks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Trying to narrow down which XYChart.draw subpath is making chart-line render blank on iOS GL+Metal. With ShowLabels/ShowLegend/ShowGrid/ ShowAxes all disabled the chart paints only the data series strokes (drawPath through GeneralPath -> Graphics.drawShape). If chart-line then captures a non-blank PNG on iOS we know drawText/drawLegend/ drawGrid/drawAxes is at fault; if it stays blank, drawSeries' drawPath is the suspect. This commit is purely diagnostic and will be reverted once the culprit is identified. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous diagnostic with showLabels/Legend/Grid/Axes off still produced a byte-identical blank PNG on iOS GL+Metal -- so the only remaining significant chart paint was drawSeries -> drawPath. Drop all data series so drawSeries is never invoked. If chart-line *now* captures a non-blank form (just an empty chart area + form title bar) the iOS blank-render is on the drawPath / Graphics.drawShape / TextureAlphaMask path. If it stays byte-identical blank, the bug is upstream of any actual chart drawing -- something about ChartComponent.paint or XYChart.draw's pre-series setup is corrupting the iOS form-Graphics state. Pure diagnostic; reverts after the culprit is identified. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CN1SS:DBG diagnostic logging in ChartComponent.paint and XYChart.draw plus the empty-dataset / labels-off variants of ChartLineScreenshotTest narrowed the iOS GL+Metal blank-render to LineChart.drawSeries -> AbstractChart.drawPath -> g.drawShape(translatedPath, stroke) on the form-Graphics. The bug is in the iOS port's GlobalGraphics.nativeDrawShape / TextureAlphaMask path -- removing data from chart-line so drawSeries isn't invoked produced a non-blank form (title bar visible) on iOS Metal, while restoring drawSeries reliably reproduces the all-blank PNG. drawShape on iOS form-Graphics aborts the entire frame, not just the chart area; that's why even the form's title bar disappears. Revert the diagnostics now that the culprit is identified. The chart-line config goes back to its real two-series LineChart so Android, JavaSE and the (eventually-fixed) iOS path all keep meaning- ful coverage. The Canvas.rotate xTranslate-conjugation fix in 16deb08 was investigated and ruled out -- it stays in place because it's correct under the universal Graphics.setTransform conjugation, just not the cause of this iOS-specific blank. 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
Four graphics screenshot tests in
scripts/hellocodenameone/common/.../tests/graphics/produced empty cells in the iOS and Android screenshot pipelines because each test had a structural defect.xScale = 0.01 * bounds.heightwas applied to the X axis (andyScalefromwidthto Y) — axes swapped, so the 100×100 logical fill was clipped to a thin strip on portrait screens. Also relied ong.translate+g.scaleseparately, which doesn't compose becauseg.translate(int, int)is a no-op on JavaSE and the iOS form-graphics path doesn't carry the translate intofillLinearGradient.makePerspective/makeCamerastraight tofillRect, so the rect projected to a sub-pixel region around the screen origin and rendered nothing visible. Also used the staticTransform.isPerspectiveSupported()(the global check) instead of the per-graphicsg.isPerspectiveTransformSupported()— on iOS Metal, mutable-image graphics return false for the per-graphics check, so the bottom 2 cells of each 2×2 grid silently no-oped.Fixes
All four tests now:
Transform(translate × scale) applied viag.setTransform(t)instead ofg.translate+g.scale.FlipTransition.paint()pattern.g.isPerspectiveTransformSupported()and emit a clear "No perspective" / "No camera" label when the per-graphics target doesn't support perspective.Verified end-to-end on the JavaSE simulator — all four tests now emit valid 65–72 KB PNGs with visible content (previously the cells were mostly empty).
Test plan
graphics-scale,graphics-affine-scale,graphics-transform-perspective,graphics-transform-cameraon each pipeline (the new pixel output differs from the previously broken goldens)🤖 Generated with Claude Code