Fix #5049 follow-ups: clipped_badge rounded clip + picker capture stability#5056
Merged
Merged
Conversation
…bility Two follow-up fixes from #5049's "left on the table" list: 1. clipped_badge.svg's rounded clip-path rendered as a sharp-cornered square on every port. The transcoded gradient-fill recipe ran pushClip -> setClip(__p) -> paint.paint -> popClip, but setClip REPLACES the current clip instead of intersecting, so the outer rounded clipPath (set by the enclosing element's clip-path attr) was wiped before the gradient ran. Switch the inner clamp to clipRect when __p.isRectangle() at runtime -- clipRect intersects, so the rounded outer clip survives; non-rectangular __p keeps setClip(__p) because the framework doesn't expose a shape-shape intersect on the public Graphics API and bounding-rect clipping would let the gradient bleed past the shape (visible regression on gradient_circle.svg which has no outer clip-path). Updates the linearGradientEmitsPaint codegen test to lock the new branch in. 2. LightweightPickerButtonsScreenshotTest's screenshots drifted from the goldens because (a) picker.setDate while the popup is showing rebuilds each spinner's ListModel and snaps scrollY but never revalidates the popup container -- so the wheels reach the right scroll position but the surrounding tick rows are still laid out for the pre-setDate model, and (b) the existing 400 ms settle timer ran on the EDT but never traversed enough paint cycles to guarantee the CAMetalLayer front buffer matched on iOS Metal (same symptom DualAppearanceBaseTest hit in ddf03de). Add an explicit form.revalidate() + repaint() after setDate to drive the layout PopupButtonActionListener does for button-driven setDates (the test bypasses that listener since it calls setDate directly), and pump three Display.callSerially hops before emitCurrentFormScreenshot so at least three EDT paint cycles land between the settle timer and cn1_captureView's afterScreenUpdates:NO grab. The matching screenshot goldens (SVGStatic.png on all three ports plus LightweightPickerButtons*.png) need to be refreshed from the CI run that lands this PR -- the SVGStatic goldens currently encode the bug (clipped_badge as a square), and the picker goldens were captured under the old non-deterministic timing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
|
Compared 116 screenshots: 116 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 47 screenshots: 47 matched. |
…act) The first attempt at fixing clipped_badge.svg switched the inner setClip(__p) to a runtime clipRect when __p.isRectangle(). That preserved the outer rounded clip in theory, but Android's clipRect-on-a-rounded-clip path runs through ShapeUtil.intersection, and the CI run on this branch showed the result rendered as a star/spike pattern -- the rounded clip survived but the interior of the badge was XOR'd against an unexplained diamond. The cleaner fix doesn't need that code path at all. Replace the runtime branch with a codegen-time decision: track the depth of the active clip-path stack in clipPathDepth (incremented when emitNode enters a clip-path block, popped on the way out). When emitting a gradient fill, skip the inner pushClip/setClip(__p)/popClip wrapping entirely if clipPathDepth > 0 -- the outer clip-path is already constraining the gradient to the visible region. Every SVG authoring tool we've seen builds clipPath as a subset of the element it clips, so the outer is strictly more restrictive and the inner clamp is redundant. Outside a clip-path block we keep the original setClip(__p) clamp because LinearGradientPaint.paint rasterises bands wider than __p's bounding box and would otherwise bleed past non-rectangular shapes (gradient_circle.svg). Updates linearGradientEmitsPaint to assert the standalone gradient still emits the inner clip, and adds a new gradientInsideClipPathSkipsInnerClip test that exercises the clipped_badge structure and asserts exactly one setClip( call (the outer clipPath) reaches the emitted source. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 114 screenshots: 114 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 116 screenshots: 116 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
CI run 26534422667 (Android Default: 8) / 26534422711 (iOS UI builds) on this branch produced the four screenshots that drifted off the pre-fix goldens: - scripts/android/screenshots/SVGStatic.png -- clipped_badge cell now renders the rounded outline + gradient fill (was a sharp- cornered square pre-fix). - scripts/ios/screenshots/SVGStatic.png -- same fix on iOS GL. - scripts/ios/screenshots-metal/SVGStatic.png -- same fix on iOS Metal. - scripts/ios/screenshots-metal/LightweightPickerButtons.png -- iOS Metal-only refresh from the picker capture-stability changes; the three extra Display.callSerially hops + post-setDate revalidate shifted a few subpixel rows of the spinner enough to fail the channel-delta tolerance on Metal (Android + iOS GL still matched their existing goldens). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
shai-almog
added a commit
that referenced
this pull request
May 29, 2026
- Section heading "Sizing in millimeters is the important knob" shortened to "Sizing in millimeters". - The "Explicit non-coverage in v1" sentence was wrong: text and clip-path are supported in this release and visible in the static-SVG fixture screenshot we ship in the post. Rewritten to call out that text and clipPath landed via PR #5056, show what is supported (<text> / <tspan> with single-style fills and transforms; <clipPath> via clip-path="url(#id)" against rect / circle / path clip shapes), and accurately list what is still not covered (filter primitives, mask with alpha, radialGradient, CSS-in-SVG with selector style rules).
shai-almog
added a commit
that referenced
this pull request
May 30, 2026
- Section heading "Sizing in millimeters is the important knob" shortened to "Sizing in millimeters". - The "Explicit non-coverage in v1" sentence was wrong: text and clip-path are supported in this release and visible in the static-SVG fixture screenshot we ship in the post. Rewritten to call out that text and clipPath landed via PR #5056, show what is supported (<text> / <tspan> with single-style fills and transforms; <clipPath> via clip-path="url(#id)" against rect / circle / path clip shapes), and accurately list what is still not covered (filter primitives, mask with alpha, radialGradient, CSS-in-SVG with selector style rules).
shai-almog
added a commit
that referenced
this pull request
May 30, 2026
- Section heading "Sizing in millimeters is the important knob" shortened to "Sizing in millimeters". - The "Explicit non-coverage in v1" sentence was wrong: text and clip-path are supported in this release and visible in the static-SVG fixture screenshot we ship in the post. Rewritten to call out that text and clipPath landed via PR #5056, show what is supported (<text> / <tspan> with single-style fills and transforms; <clipPath> via clip-path="url(#id)" against rect / circle / path clip shapes), and accurately list what is still not covered (filter primitives, mask with alpha, radialGradient, CSS-in-SVG with selector style rules).
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
Two follow-ups to #5049's "left on the table" list. Both originate in the
transcoder / test layer rather than the iOS / Android ports.
Clipped-badge rounded clip-path (
scripts/hellocodenameone/common/src/main/css/clipped_badge.svg) rendered as a sharp-cornered square on every port — Android, iOS GL, iOS Metal.setClip(__p)to confineLinearGradientPaint.paint's bounding-rect fill, butsetClipREPLACES the current clip. The outer roundedclipPathset by the enclosing<rect clip-path="url(#rounded)">was wiped before the gradient ran.__p.isRectangle()at runtime, switch the inner clamp toclipRect(which intersects). Non-rectangular__pkeepssetClip(__p)because the framework doesn't expose a shape-shape intersect on the public Graphics API, and a bounding-rect clip would let the gradient bleed past the shape outline (would regressgradient_circle.svg, which has no outerclip-path).linearGradientEmitsPaintcodegen test.LightweightPickerButtonsScreenshotTest screenshots drifted from the goldens.
picker.setDatewhile the popup is showing rebuilds each spinner'sListModeland snapsscrollY, but never revalidates the popup container — so the wheels reach the right scroll position but the surrounding tick rows are still laid out for the pre-setDatemodel.PopupButtonActionListenercallsrevalidate()+repaint()for button-drivensetDatepaths, but the screenshot test callssetDatedirectly and bypasses that listener.DualAppearanceBaseTestinddf03deaf).form.revalidate()+repaint()aftersetDate, and pump threeDisplay.callSeriallyhops beforeemitCurrentFormScreenshotso at least three EDT paint cycles land between the settle timer andcn1_captureView'safterScreenUpdates:NOgrab.Goldens
The matching screenshot goldens still encode the pre-fix behaviour and need to be refreshed from this PR's CI run:
scripts/{android,ios,ios-metal}/screenshots/SVGStatic.png— currently shows the badge as a square; after this PR the rounded clipPath is honoured.scripts/{android,ios,ios-metal}/screenshots/LightweightPickerButtons*.png— captured under the old non-deterministic timing.Test plan
mvn -pl svg-transcoder test(transcoder unit tests including the updated gradient-emission test) passes locally.SVGStatic.png: clipped_badge cell shows a rounded square; rest of the SVG grid unchanged.🤖 Generated with Claude Code