Repository navigation
fix(check): stop false sweep_static and text-overlap alarms on compositions that are fine - #5147
Conversation
…a-no-timeline for stills
Edit accuracy: accurate 2059 (base branch 2059), smooth 1569 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
…text overlap on glyphs
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
… words and one-side borders
jrusso1020
left a comment
There was a problem hiding this comment.
Approving at 78ee279a.
Motion signature. The new paint channels sit in the shared classifier, so sweep_static and keepsMoving keep counting the same things, as the header comment requires. Border and outline colors are signed only where a side is actually drawn. That gate matters: if I sign them unconditionally (a mutation), two tests fail, because Blink's currentColor would count every text color change twice. Video time goes through the same isOptedOut skip as every other channel. Audio is kept after \u001f and only reaches detectSweepStatic, through seenPart. keepsMoving never sees it. __hyperframesLayoutGeometry has one consumer (checkBrowser.collectLayoutGeometry → layoutStateSignatures), so the appended audio part can't leak into another comparison.
Glyph overlap. The re-measure runs only for pairs whose content areas already overlap by a fifth, and it shares inkRect with the clip check.
Ran.
- Real Chromium 153:
motion-signature.browser.chromium,layout-audit.chromiumandcheck.test.tsall pass, 131/131. - Mutations killed 6 of 7. Each of these fails a test:
- removing
contentPaintChannel - dropping the glyph re-check
- signing border/outline colors unconditionally
- removing the
isOptedOutskip for media - dropping the audio-only warning
- not applying
capitalize
- removing
- The one survivor: ignoring the 0.1 s liveness bucket for video time. It's harmless as written.
Should-fix, non-blocking. A line clipped at its top by an overflow:hidden ancestor loses a real collision that main reports. glyphRects runs inkRect on block.rects, but those come from visibleTextClientRects, which has already cut each rect to its overflow-clipping ancestors. inkRect derives its scale from rect.height. A top-clipped rect therefore gets a smaller scale, and the glyph box is placed lower than the letters really are. I ran a probe with HELLO at top 100 and WORLD at top 160, both 120px Arial, where WORLD sits in an overflow:hidden box spanning 180–300. The visible glyphs collide at 180–203 against heights of about 86 and 83, roughly 27%. Main reports content_overlap. This branch reports nothing, because its computed glyph box starts at about 194. Cases clipped at the bottom, or not clipped, agree with main. This shape is common for a line held part-way through a mask reveal. Fix: compute the glyph box from the unclipped Range rect, then intersect it with the clips. visibleTextClientRects could take an optional per-rect map applied before clipping.
CI. The red "Studio: timeline viewport gate" is skipped on main pushes, and its log isn't available until the run ends, so I couldn't confirm the #5109 overscan cause from the log. This PR touches only CLI and docs, so I'm not treating it as this PR's.
— Rames
What
hyperframes checkstops raising two false alarms on compositions that are fine:sweep_static) on compositions that do animate, and it tells authors of real stills how to say so.content_overlap) on words that wrap at a tight line-height, where no glyphs touch.sweep_static
The guard compares a fingerprint of everything visible at each seek sample. That fingerprint saw only geometry, opacity, font axes, clip-path, text, controls, generated content and media pixels. So a composition whose only motion was one of these failed
check:filter(blur) tweencolortween<audio>has no box)The shared motion classifier (
motion-signature.browser.js) now also signs:filter,backdrop-filter,background-color,background-image,background-position,box-shadow, plus each border side's and the outline's color where that side or outline is actually drawn. Blink computes those colors ascurrentColoreven when nothing is drawn, so signing them unconditionally would count every text color change twice, including on content the browser skips.color,text-shadow, and SVGfillandstrokecolor.currentTimeof every<video>under the composition root, bucketed to 0.1 s for liveness. A playing video is a moving picture even when its pixels cannot be read. Media on adata-layout-ignorelayer is skipped, like every other channel.Both
sweep_staticandkeepsMovingread this classifier, so they keep counting the same things as motion.Audio is handled apart, and this is a deliberate trade. Audio time shows the timeline ran, but it is not a picture. Counting it as motion would let a broken animation (tweens on a timeline the runtime never registered) pass whenever a soundtrack plays, and would hide a frozen picture from
keepsMoving. So:keepsMovingnever counts audio time.sweep_staticis a warning, not a failure: "Only the audio advanced under seek; nothing on screen moved." An audio-only composition no longer failscheck. A broken animation with music is still called out, as a warning instead of an error, and--strictstill fails it.A composition that really is still (a title with an empty timeline, or the blank template before anything is animated) still fails, because nothing changes. The fix hint for both tiers now says what to do: "If the composition is meant to be still, add
data-no-timelineto the element withdata-composition-id."SVG stroke drawing (
stroke-dasharray/stroke-dashoffset) is left to #3914, which covers it in the same classifier.content_overlap
The check measured overlap on each line's text rect, which is the font's whole content area (ascent plus descent). At
line-heightbelow about 0.95, the content areas of stacked lines overlap while the letters do not. Inline-block words wrapping in a headline atline-height: 0.8therefore failed. The flex/grid exemption did not cover them, since this is normal inline flow.For a pair whose content areas overlap by more than a fifth, the check now measures again on the glyph extent (the font's actual ascent and descent, from the canvas
measureTextthe clip check already uses) and flags only when the glyph boxes still overlap by more than a fifth of the smaller glyph area.text-transformis applied before measuring (uppercase,lowercase, and nowcapitalize, which uppercases the first character of each word the way Chromium draws it, so-aceis measured as-Acewhile3goand_gostay as written), so title-case words are measured at cap height. The glyph-extent math is shared with the clip check (inkRect), and pairs that do not overlap are never measured.A trade to know about. An opaque box covering another block's words, with no letters touching, no longer counts toward this finding at that sample. On the
vignelliexample one caption box sits over the tail of a label for part of its hold. The overlap is still reported, but it is held for less time, so it drops from error to warning. Text covered by a box is whattext_occludedreports, notcontent_overlap.Proof
hyperframes checkfrom this branch's build, on the reported repro fixtures:sweep_staticwarning and thedata-no-timelinehintdata-no-timelinedata-no-timelinedata-no-timelineline-height: 0.8content_overlapx2)Checks
display:nonevideo, in both samplerscontent-visibility: hiddenhost does not count, with no border or with one border side drawn in its own colordata-layout-ignorelayer is ignoredline-height: 0.8(no finding); the same words at0.4, where the glyphs collide; two headings 60 px apart whose letters collide; capitalized words 60 px apart whose capitals collide, including words led by punctuation, and digit-led words that stay lowercase and collide on their descenders; two headings on the same spot (each a finding).checkcases: an audio-only advance is one warning with the hint and the report stays ok; any on-screen change gives no finding; the existingsweep_staticerror namesdata-no-timeline.docs/packages/cli.mdx) and the CLI skill's check reference describe the new motion channels, the audio warning anddata-no-timeline.