Screenshot test for clip under scale+translate (#4907)#4917
Closed
shai-almog wants to merge 3 commits into
Closed
Conversation
Repro for issue #4907 (glitchy metal drawing). The issue reports that a magnifier-style render path -- set a small clip rect, then apply g.scale(scale, scale) + g.translate(-ax, -ay) and draw a much larger surface -- correctly clips on the legacy iOS pipeline / JavaSE / Android but leaks outside the clip rect under iOS Metal. The new ClipUnderScaleTranslate test runs the AbstractGraphicsScreenshotTest 2x2 (AA off/on, form-graphics vs mutable-image) and renders the exact sequence from ddyer0's snippet: g.pushClip(); g.clipRect(centred small rect); g.scale(2.0f, 2.0f); g.translate(-ax, -ay); g.fillRect(<covers entire post-transform cell>); // should clip to rect g.translate(ax, ay); g.scale(0.5f, 0.5f); g.popClip(); A navy outline of the expected clip rect is drawn before the clipped fill so the screenshot diff has a stable anchor; a tiny green sentinel is drawn after popClip so a broken pop/transform-reset is also caught. On a working pipeline the cell shows a centred red square inside the navy outline; on the buggy pipeline the red leaks outside (worst case: the entire cell is red). Splitting the form-graphics path from the mutable-image path lets the diff narrow the suspect target without extra instrumentation. Wired into Cn1ssDeviceRunner after Clip and into the HTML5 skip list to match the other graphics screenshot tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
JavaScript port screenshot updatesCompared 16 screenshots: 15 matched, 1 missing reference.
|
…4907) The previous version drew a solid red fillRect covering the entire post-transform area, with a navy outline of the expected clip rect. When the clip is honoured the red square sits exactly on the navy outline, so a small bleed at the edges is hard to distinguish from normal antialiasing -- on the JavaSE simulator the result looked correct but it was hard to tell whether the test would catch a subtle leak. Switch to a 5-patch pattern sized in source (pre-scale) coordinates: - Central red patch sized 1/scale * clipW by 1/scale * clipH so that, under the 2x scale, it exactly fills the clip rect. - Four side patches (green N, blue S, orange W, magenta E) placed one patch-width / height away from the centre. With the 2x scale they end up one full clipW / clipH off-centre, entirely outside the clip rect under a correct clip and invisible. A correct render shows only a red square inside a black clip-rect outline (drawn last, on top of the fill so its position is stable). A bleed shows a coloured patch outside the outline, and the colour identifies which post-transform direction leaked. The clip rect is now 1/5 of the cell (was 1/3) so there is plenty of gray buffer around it to make even a thin bleed visible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
Android screenshot updatesCompared 106 screenshots: 105 matched, 1 missing reference.
Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
iOS Metal screenshot updatesCompared 106 screenshots: 105 matched, 1 missing reference.
Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
iOS screenshot updatesCompared 106 screenshots: 105 matched, 1 missing reference.
Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
…#4907) The previous version picked ax = clipCenterX and ay = clipCenterY, which made the magnifier-origin the fixed point of the transform rather than the clip-rect centre. After g.scale(s); g.translate(-ax, -ay) a source point p ends up at native screen coord s * (p - (ax, ay)) (Graphics adds xTranslate/yTranslate before the impl applies the scale; every active port has isTranslationSupported() == false), so the old math sent the centre patch and all four side patches to screen positions outside the cell on every port. The CI output for the previous PR showed empty cells on iOS GL and iOS Metal -- no red, no leak signal, just the clip outline and the corner sentinel. Set ax = clipCenterX / scale and ay = clipCenterY / scale instead. With that choice the transform's fixed point is the clip-rect centre, and a source patch centred at (clipCenterX, clipCenterY) of size (clipW / scale, clipH / scale) maps exactly onto the clip rect on screen. The four side patches placed one source-patch-size off-centre land in the cell rows / columns immediately adjacent to the clip rect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closed
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
ClipUnderScaleTranslatescreenshot test that reproduces the magnifier-style render path from issue glitchy metal drawing #4907 (set a small clip, theng.scale(2,2)+g.translate(-ax,-ay), then a fillRect covering the whole post-transform cell).popClipcatches broken pop/transform-reset.AbstractGraphicsScreenshotTest2×2 grid so the form-graphics path and the mutable-image path are captured separately, narrowing the suspect target if only one cell fails.This is purely a test addition -- no framework code is touched. We're letting CI render the test on each platform to see whether iOS Metal currently clips correctly or leaks outside the rect.
Test plan
🤖 Generated with Claude Code