Skip to content

fix(screenshot-diff): cap the 1% aspect tolerance at the rounding bound - #1382

Closed
latekvo wants to merge 7 commits into
mainfrom
fix/screenshot-diff-aspect-rounding-tolerance
Closed

latekvo wants to merge 7 commits into
mainfrom
fix/screenshot-diff-aspect-rounding-tolerance

Conversation

@latekvo

@latekvo latekvo commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Closes #807

The 1% aspect tolerance let full-res captures from two different iPhones (16 vs 16 Pro / Pro Max, 0.29% / 0.22% apart) be resampled and diffed as one screen. The tolerance is now capped at 2 / shortest side, the most that integer rounding of rescales can move the aspect, so pairs further apart than that become a dimension mismatch while real 1.0x vs 0.5/0.3/0.25x pairs still normalize.

Pairs inside the rounding bound still normalize: they cannot be told apart from a rescale by dimensions alone (i16 Pro vs Pro Max, i16 vs 16 Plus, and more once one side is a scaled capture, since the bound grows as the image shrinks).

On a foldable, a foreign baseline near a panel aspect now hard-fails; the posture hint is dropped when both sizes map to one posture, instead of naming the cover panel twice and saying to fold.

No docs update needed.

Verification

Probe: diffPngFiles on real PNGs, with same-device downscales made by real sips -Z (the path downscalePngInPlace uses) and full-res cross-device pairs that sit within 1% aspect.

pair main this PR
21 same-device 1.0 vs sips 0.25/0.3/0.5 (7 devices) NORMALIZED NORMALIZED
i16 1179x2556 vs i16 Pro 1206x2622 (0.285%) NORMALIZED MISMATCH
i16 vs i16 Pro Max 1320x2868 (0.220%) NORMALIZED MISMATCH
i16 vs i16e 1170x2532 (0.177%) NORMALIZED MISMATCH
i16 Pro vs i16e / Pro Max vs i16e NORMALIZED MISMATCH
Pixel 9 1080x2424 vs Pixel 7 1080x2400 (0.990%) NORMALIZED MISMATCH
i16 Pro vs i16 Pro Max (0.064%) NORMALIZED NORMALIZED

The last row cannot be decided from dimensions: 1206x2622 is exactly a floor-rounded 0.914x rescale of 1320x2868. Images with a shortest side under 200 px keep the 1% bound, as before (100x200 vs 101x200 normalizes, vs 102x200 mismatches).

A sweep over 29,796 rescale pairs (13 device sizes, scales 0.05-1.0, round/floor/ceil) found 0 pairs that normalize on main and are rejected here. Reverting only the return line brings back both failures in the unit-level repro.

A second sweep modelled each producer's rounding: sim-server f32 round (iOS sim and Android), Math.round (Chromium, Vega, Harmony) and measured sips -Z (physical iOS, tvOS). It covered 80 device sizes, scaled-vs-scaled pairs (such as the 0.5 baseline vs 0.25 live-capture fallback) and double rescales, and found 0 same-screen pairs rejected here. iPhone 16 Pro Max simulator captures at 7 scales came out at exactly round(W*s). Mutants 2 -> 1 (now killed by the two-rescale test) and 2 -> 3/4, min -> max are all caught.

Checks: npx tsc --build, the test typecheck, vitest on the screenshot-diff, foldable and flow snapshot/visual test files, and prettier + eslint on the changed files all pass.

…band

A uniform rescale of one framebuffer only drifts in aspect by integer
rounding, which stays under 2 / the shortest side. Capping the 1% tolerance
at that bound rejects iPhone 16 vs 16 Pro / Pro Max captures (0.29% / 0.22%
apart) as a dimension mismatch, while real 1.0x vs 0.3x/0.25x pairs still
normalize.

Fixes #807
@coderabbitai

coderabbitai Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Two rounded rescales of one screen can sit past 1 / shortest side, so a
1 / shortest bound would reject them; nothing caught that. Also bring the
tolerance comment and an older test name in line with the capped bound.
With the tighter aspect bound, a foreign baseline near a panel's aspect
(e.g. an iPad Pro 11 vs the Duo cover panel) now hard-fails, and the hint
named the cover panel for both sizes while still saying to fold.
@latekvo latekvo changed the title fix(screenshot-diff): reject cross-device pairs inside the 1% aspect band fix(screenshot-diff): cap the 1% aspect tolerance at the rounding bound Oct 24, 2026
@latekvo
latekvo marked this pull request as ready for review October 24, 2026 14:52
@latekvo
latekvo marked this pull request as draft October 24, 2026 14:52
@latekvo

latekvo commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

Superseded by #1403, folded into #1753: it removes the aspect-ratio tolerance this PR capped, so #807's pair (1179x2556 vs 1206x2622) is now a dimension_mismatch.

@latekvo latekvo closed this Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

screenshot-diff: the 1% aspect tolerance silently normalizes screenshots taken on two different devices

1 participant