From 518b97a0c9b517b0ffd99ab7b4667137631da6a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20S=C3=A1ros?= Date: Mon, 7 Sep 2026 16:51:47 +0200 Subject: [PATCH 1/2] chore(ui-scripts,regression-test): ignore whole-pixel shifts in visual regression diffs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suite failed on screenshots that were pixel-identical to their baseline but translated by about a pixel. When a layout box rounds one device pixel differently the whole painted subtree moves, so a direct comparison lights up every edge in the image: on a real baseline, a 1px translation produces 2825 differing pixels. That is not a visual regression, and the suite should not report one. visual-diff now retries the comparison at small integer offsets before calling a screenshot changed, behind --max-shift (default 1, 0 restores exact matching). Scoring happens over the region the two images share, which also covers the related case of a screenshot one pixel taller with identical content. A pixel-count tolerance cannot do this job: the shift above and a genuine 40x40 recolour differ by less than a factor of two, so any threshold loose enough to absorb the first would hide the second. viewportHeight goes 800 to 2000 to cut the stitching that produces those shifts. capture: 'fullPage' scrolls the viewport down the document and stitches the slices, and the stitch is the least reproducible part of the capture. Measured against the baselines branch, this takes the suite from 58 stitch operations to 37, and from 17 of 32 pages captured in one pass to 28. The viewport change alters layout, so the first run shows a cascade of changed rows; merging refreshes the baselines. ๐Ÿค– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) --- .../lib/__node_tests__/visual-diff.test.ts | 78 ++++++++++++++++++ .../ui-scripts/lib/commands/visual-diff.ts | 81 ++++++++++++++++++- regression-test/cypress.config.ts | 9 ++- 3 files changed, 166 insertions(+), 2 deletions(-) diff --git a/packages/ui-scripts/lib/__node_tests__/visual-diff.test.ts b/packages/ui-scripts/lib/__node_tests__/visual-diff.test.ts index 02af8da185..a0d3c45e63 100644 --- a/packages/ui-scripts/lib/__node_tests__/visual-diff.test.ts +++ b/packages/ui-scripts/lib/__node_tests__/visual-diff.test.ts @@ -23,6 +23,7 @@ */ import { describe, it, expect } from 'vitest' +import { PNG } from 'pngjs' import { badgeFor, thumb, @@ -30,6 +31,7 @@ import { sourceLinkFor, appUrlFor, dilateMask, + matchesWhenShifted, esc, normalizeA11y, normalizeImpact, @@ -272,6 +274,82 @@ describe('dilateMask', () => { }) }) +// An opaque white image with the given rectangles painted solid black. Hard +// edges, so a one-pixel move registers as a real difference instead of being +// written off as antialiasing by pixelmatch's `includeAA: false`. +function image( + w: number, + h: number, + rects: Array<{ x: number; y: number; w: number; h: number }> +) { + const png = new PNG({ width: w, height: h }) + png.data.fill(255) + for (const r of rects) { + for (let y = r.y; y < r.y + r.h; y++) { + for (let x = r.x; x < r.x + r.w; x++) { + const i = (y * w + x) * 4 + png.data[i] = 0 + png.data[i + 1] = 0 + png.data[i + 2] = 0 + } + } + } + return png +} + +describe('matchesWhenShifted', () => { + // Big enough that a 1px move is unambiguous, and far enough from the edges to + // shift in any direction without clipping. + const box = [{ x: 6, y: 6, w: 8, h: 8 }] + const baseline = image(24, 24, box) + const matches = (actual: PNG, maxShift = 1) => + matchesWhenShifted(baseline, actual, 0.1, maxShift) + + it('matches an identical image', () => { + expect(matches(image(24, 24, box))).toBe(true) + }) + + it('matches a one-pixel horizontal shift in either direction', () => { + expect(matches(image(24, 24, [{ x: 7, y: 6, w: 8, h: 8 }]))).toBe(true) + expect(matches(image(24, 24, [{ x: 5, y: 6, w: 8, h: 8 }]))).toBe(true) + }) + + it('matches a one-pixel vertical shift', () => { + expect(matches(image(24, 24, [{ x: 6, y: 7, w: 8, h: 8 }]))).toBe(true) + }) + + it('matches a diagonal shift', () => { + expect(matches(image(24, 24, [{ x: 5, y: 7, w: 8, h: 8 }]))).toBe(true) + }) + + it('matches when the actual is a pixel taller but otherwise identical', () => { + expect(matches(image(24, 25, box))).toBe(true) + }) + + it('rejects a shift larger than the budget', () => { + expect(matches(image(24, 24, [{ x: 9, y: 6, w: 8, h: 8 }]))).toBe(false) + }) + + it('matches that larger shift once the budget allows it', () => { + expect(matches(image(24, 24, [{ x: 9, y: 6, w: 8, h: 8 }]), 3)).toBe(true) + }) + + it('rejects a real change that no shift can explain', () => { + const recolored = image(24, 24, box) + const i = (10 * 24 + 10) * 4 + recolored.data[i] = 255 + recolored.data[i + 1] = 0 + recolored.data[i + 2] = 0 + expect(matches(recolored)).toBe(false) + }) + + it('considers only the identity offset when maxShift is 0', () => { + const shifted = image(24, 24, [{ x: 7, y: 6, w: 8, h: 8 }]) + expect(matches(shifted, 0)).toBe(false) + expect(matches(image(24, 24, box), 0)).toBe(true) + }) +}) + describe('esc', () => { it('escapes HTML-significant characters', () => { expect(esc('a & b')).toBe( diff --git a/packages/ui-scripts/lib/commands/visual-diff.ts b/packages/ui-scripts/lib/commands/visual-diff.ts index 76c5b1abdd..00e16f9674 100644 --- a/packages/ui-scripts/lib/commands/visual-diff.ts +++ b/packages/ui-scripts/lib/commands/visual-diff.ts @@ -48,6 +48,7 @@ type Args = { baselineDir: string outputDir: string threshold: number + maxShift: number failOnMissingBaseline: boolean prNumber?: string prUrl?: string @@ -225,6 +226,64 @@ function diffMask(baseline: PNG, actual: PNG, threshold: number) { } } +// Copy a w*h window out of `src` starting at (sx, sy). Callers clamp the window +// to the source bounds first. +function crop(src: PNG, sx: number, sy: number, w: number, h: number): PNG { + const out = new PNG({ width: w, height: h }) + PNG.bitblt(src, out, sx, sy, w, h, 0, 0) + return out +} + +// Is `actual` pixel-identical to `baseline` once shifted by up to `maxShift`? +// +// When a layout box rounds one device pixel differently, the whole painted +// subtree moves โ€” glyphs, borders, all of it. The image is unchanged, just +// translated, but a direct comparison lights up every edge in it: a one-pixel +// shift routinely produces several thousand differing pixels. A component that +// moved a pixel and is otherwise identical is not a visual regression, so this +// is the check that says so. +// +// A pixel-count tolerance cannot do this job. The shift above and a genuine +// 40x40 recolor land in the same order of magnitude, so any threshold loose +// enough to absorb the first also hides the second. +// +// Each offset is scored over the region the two images share, so the band that +// shifts in from outside is never counted. (0, 0) is included deliberately: the +// caller's comparison *pads* mismatched sizes, while this one *crops* to the +// overlap, which is what lets "one pixel taller, same content" pass. +/** @internal โ€” exported only for tests; not part of the package's public API. */ +export function matchesWhenShifted( + baseline: PNG, + actual: PNG, + threshold: number, + maxShift: number +): boolean { + for (let dy = -maxShift; dy <= maxShift; dy++) { + for (let dx = -maxShift; dx <= maxShift; dx++) { + // A positive dx means the content sits that many pixels further right + // than in the baseline, so actual (x, y) lines up with baseline + // (x - dx, y - dy). + const x0 = Math.max(0, dx) + const y0 = Math.max(0, dy) + const w = Math.min(actual.width, baseline.width + dx) - x0 + const h = Math.min(actual.height, baseline.height + dy) - y0 + if (w <= 0 || h <= 0) continue + + // No output buffer โ€” only the count matters here. + const numDiff = pixelmatch( + crop(actual, x0, y0, w, h).data, + crop(baseline, x0 - dx, y0 - dy, w, h).data, + undefined, + w, + h, + { threshold, includeAA: false } + ) + if (numDiff === 0) return true + } + } + return false +} + // How much unchanged pixels are dimmed in the diff image so the changed pixels // stand out. DESAT blends each pixel toward its own grayscale (0 = keep color, // 1 = fully gray); DIM then scales brightness (0.5 = half). @@ -1431,6 +1490,7 @@ function run(args: Args): number { baselineDir, outputDir, threshold, + maxShift, failOnMissingBaseline } = args @@ -1471,8 +1531,21 @@ function run(args: Args): number { actual: padded } = diffMask(baseline, actual, threshold) - const status: Status = + // Straight comparison first, so identical screenshots cost nothing extra; + // the realignment check below only runs on one that already failed it. The + // size guard keeps a real layout change from being shifted away โ€” only a + // delta within the shift budget is a rounding artifact. + let status: Status = numDiff === 0 && !sizeMismatch ? 'unchanged' : 'changed' + if ( + status === 'changed' && + maxShift > 0 && + Math.abs(baseline.width - actual.width) <= maxShift && + Math.abs(baseline.height - actual.height) <= maxShift && + matchesWhenShifted(baseline, actual, threshold, maxShift) + ) { + status = 'unchanged' + } if (status === 'changed') { const highlight = highlightImage(padded, changed, width, height) @@ -1580,6 +1653,12 @@ export default { describe: 'pixelmatch color threshold (0-1)', default: 0.1 }, + 'max-shift': { + type: 'number', + describe: + 'Treat a screenshot as unchanged when it matches its baseline exactly after being shifted by up to this many pixels. Absorbs whole-pixel layout rounding, which moves a component without altering it. 0 requires an exact match.', + default: 1 + }, 'fail-on-missing-baseline': { type: 'boolean', describe: 'Exit non-zero if actual screenshots have no matching baseline', diff --git a/regression-test/cypress.config.ts b/regression-test/cypress.config.ts index b96443f7ac..732598dcd6 100644 --- a/regression-test/cypress.config.ts +++ b/regression-test/cypress.config.ts @@ -40,7 +40,14 @@ export default defineConfig({ screenshotOnRunFailure: false, e2e: { viewportWidth: 1280, - viewportHeight: 800, + // Deliberately taller than a real browser window. `capture: 'fullPage'` + // scrolls the viewport down the document and stitches the slices together, + // and the stitch is the least reproducible part of the capture. Measured + // against the baselines on the `visual-baselines` branch, raising this from + // 800 takes the suite from 58 stitch operations to 37, and from 17 of 32 + // pages captured in one pass to 28. Only `min-height: 100vh` on the body + // depends on this value. + viewportHeight: 2000, setupNodeEvents(on, config) { on('before:run', () => { writeFileSync(META_FILE, '{}') From a1d1cc6940b7ecee4cd8fe5c497898d3e454097f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20S=C3=A1ros?= Date: Wed, 30 Sep 2026 14:47:43 +0200 Subject: [PATCH 2/2] chore(ui-scripts,regression-test): simplify shift-detection comments and revert viewport change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on #2710. - Trim the `matchesWhenShifted` and status-check comments, and state that the match covers a shift in any direction, diagonals included. - Report a realigned screenshot as layout shifted in `summary.json` and the HTML report instead of silently calling it unchanged. - Revert `viewportHeight` to 800. The setting does not reach the browser in CI: captures are floored at 720px with both 800 and 2000, so the claimed stitch reduction never happened. ๐Ÿค– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) --- .../ui-scripts/lib/commands/visual-diff.ts | 42 +++++++++---------- regression-test/cypress.config.ts | 9 +--- 2 files changed, 20 insertions(+), 31 deletions(-) diff --git a/packages/ui-scripts/lib/commands/visual-diff.ts b/packages/ui-scripts/lib/commands/visual-diff.ts index 00e16f9674..cdcd7c2658 100644 --- a/packages/ui-scripts/lib/commands/visual-diff.ts +++ b/packages/ui-scripts/lib/commands/visual-diff.ts @@ -41,6 +41,8 @@ type Result = { status: Status numDiff?: number sizeMismatch?: boolean + // 'unchanged' only because the actual matched the baseline after realignment. + layoutShifted?: boolean } type Args = { @@ -226,31 +228,22 @@ function diffMask(baseline: PNG, actual: PNG, threshold: number) { } } -// Copy a w*h window out of `src` starting at (sx, sy). Callers clamp the window -// to the source bounds first. +// Copy a w*h window out of `src` at (sx, sy). Callers clamp to the source bounds. function crop(src: PNG, sx: number, sy: number, w: number, h: number): PNG { const out = new PNG({ width: w, height: h }) PNG.bitblt(src, out, sx, sy, w, h, 0, 0) return out } -// Is `actual` pixel-identical to `baseline` once shifted by up to `maxShift`? +// Is `actual` pixel-identical to `baseline` after being shifted up to +// `maxShift` pixels in any direction, diagonals included? // -// When a layout box rounds one device pixel differently, the whole painted -// subtree moves โ€” glyphs, borders, all of it. The image is unchanged, just -// translated, but a direct comparison lights up every edge in it: a one-pixel -// shift routinely produces several thousand differing pixels. A component that -// moved a pixel and is otherwise identical is not a visual regression, so this -// is the check that says so. +// A one-pixel rounding moves the whole painted subtree and lights up every edge +// in it โ€” thousands of pixels, the same order of magnitude as a genuine small +// recolor, so a pixel-count tolerance cannot separate the two. // -// A pixel-count tolerance cannot do this job. The shift above and a genuine -// 40x40 recolor land in the same order of magnitude, so any threshold loose -// enough to absorb the first also hides the second. -// -// Each offset is scored over the region the two images share, so the band that -// shifts in from outside is never counted. (0, 0) is included deliberately: the -// caller's comparison *pads* mismatched sizes, while this one *crops* to the -// overlap, which is what lets "one pixel taller, same content" pass. +// (0, 0) is included because this comparison crops to the overlap while the +// caller's pads, which is what lets "one pixel taller, same content" pass. /** @internal โ€” exported only for tests; not part of the package's public API. */ export function matchesWhenShifted( baseline: PNG, @@ -955,6 +948,8 @@ function row( ? `
${r.numDiff} pixels differ${ r.sizeMismatch ? ' ยท size mismatch' : '' }
` + : r.layoutShifted + ? `
layout shifted ยท ${r.numDiff} pixels differ before realignment
` : '' const source = sourceLinkFor(r.name, meta, sourceBaseUrl) const hasBoth = r.status === 'changed' || r.status === 'unchanged' @@ -1531,12 +1526,12 @@ function run(args: Args): number { actual: padded } = diffMask(baseline, actual, threshold) - // Straight comparison first, so identical screenshots cost nothing extra; - // the realignment check below only runs on one that already failed it. The - // size guard keeps a real layout change from being shifted away โ€” only a - // delta within the shift budget is a rounding artifact. + // Is the change just a layout shift? Checked only after the straight + // comparison fails; the size guard stops a real layout change from being + // shifted away. let status: Status = numDiff === 0 && !sizeMismatch ? 'unchanged' : 'changed' + let layoutShifted = false if ( status === 'changed' && maxShift > 0 && @@ -1545,6 +1540,7 @@ function run(args: Args): number { matchesWhenShifted(baseline, actual, threshold, maxShift) ) { status = 'unchanged' + layoutShifted = true } if (status === 'changed') { @@ -1553,7 +1549,7 @@ function run(args: Args): number { writeFileSync(join(outputDir, 'diff', name), PNG.sync.write(highlight)) } - results.push({ name, status, numDiff, sizeMismatch }) + results.push({ name, status, numDiff, sizeMismatch, layoutShifted }) } let a11y: A11y | null = null @@ -1656,7 +1652,7 @@ export default { 'max-shift': { type: 'number', describe: - 'Treat a screenshot as unchanged when it matches its baseline exactly after being shifted by up to this many pixels. Absorbs whole-pixel layout rounding, which moves a component without altering it. 0 requires an exact match.', + 'Treat a screenshot as unchanged when it matches its baseline exactly after being shifted up to this many pixels in any direction. Absorbs whole-pixel layout rounding. 0 requires an exact match.', default: 1 }, 'fail-on-missing-baseline': { diff --git a/regression-test/cypress.config.ts b/regression-test/cypress.config.ts index 732598dcd6..b96443f7ac 100644 --- a/regression-test/cypress.config.ts +++ b/regression-test/cypress.config.ts @@ -40,14 +40,7 @@ export default defineConfig({ screenshotOnRunFailure: false, e2e: { viewportWidth: 1280, - // Deliberately taller than a real browser window. `capture: 'fullPage'` - // scrolls the viewport down the document and stitches the slices together, - // and the stitch is the least reproducible part of the capture. Measured - // against the baselines on the `visual-baselines` branch, raising this from - // 800 takes the suite from 58 stitch operations to 37, and from 17 of 32 - // pages captured in one pass to 28. Only `min-height: 100vh` on the body - // depends on this value. - viewportHeight: 2000, + viewportHeight: 800, setupNodeEvents(on, config) { on('before:run', () => { writeFileSync(META_FILE, '{}')