Skip to content

perf(core): mounting a scene no longer restyles it to look for raster hints - #6300

Merged
miguel-heygen merged 6 commits into
mainfrom
perf/raster-hints-skip-unhinted
Oct 25, 2026
Merged

miguel-heygen merged 6 commits into
mainfrom
perf/raster-hints-skip-unhinted

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

What

Mounting a scene in a small preview no longer forces the browser to restyle the whole scene just to look for will-change hints. On a page with no hints, the raster-hints pass now reads no computed style at all.

Why

Below 1x, previewRasterHints drops pure will-change: transform hints so a small preview does not raster every hinted layer at full size. To decide that, it ran getComputedStyle on every element of every added subtree (followTree). When the added nodes included a <style> or <link>, it ran it on every element of the page (markLayers). Each read on a freshly inserted subtree forces a style recalc of that subtree, in the same task as the insertion.

Scene parking (#6189) mounts each upcoming scene's DOM 10 s ahead while playing, so a big scene arrives as one subtree. On a 50 s 3840x2160 film with 7 scenes (1,595 layers in the largest), each mount of a large scene was a single 123 to 131 ms frame, and those frames were the film's last dropped frames. The film declares will-change nowhere, so every one of those reads came back auto.

How

  • What can carry a hint. will-change is not inherited, so only an element whose inline style or a stylesheet rule sets it can carry one. mayHint checks exactly that without reading computed style: the element is marked, or its own style.getPropertyValue("will-change") is set, or it matches the selectors of the style rules that declare will-change (including @media/@supports/@layer, imported and adopted sheets; the runtime's own sheet is left out). markLayers and followTree call fits only where mayHint holds.

  • Marks have one writer. setMark writes and clears every mark and keeps the set of marked elements, so "is anything marked" is exact and free. Marks on removed elements are dropped from the set, and an element followTree visits that arrives carrying the mark (re-attached, or copied with cloneNode) is adopted back into it, so its hint is still given back when a child starts to need it.

  • Attributes are watched only when they can matter. With nothing marked and no rule giving a hint, no attribute or inline style change can make or break a mark, so the observer follows added nodes only. On this film that drops about 1,500 attribute records a second per SVG attribute the scene player animates (fill, stroke, stroke-width, filter seed/radius/values), each of which used to run followTree, and the 8,642-node walk when perf(core): the preview mounts and draws only the scenes near the playhead #6189 stamps data-duration on a mounted scene. As soon as a mark is written or a hint rule arrives, attributes are watched as before, so unmarking stays exact. A layer hinted inline that only later becomes droppable through an attribute change keeps its hint, which is the browser's own behaviour.

  • Holders are walked only when something is marked.

  • Rules are read once per page check. The rule selector is re-read where the page is already re-checked: when a <style> or <link> is added or a linked sheet loads. A rule edited through CSSOM in between is not seen, the same limit the file already has for marks; a hint it misses keeps rastering at full size and paints the same. Cross-origin sheets and selectors that do not parse fall back to every element, as do rules nested in a style rule (relative selectors). The selector is validated against an empty fragment, never against the page.

  • Package size budget. The producer bundles the runtime, so its packed budget rises by the 336 bytes this change adds (unpacked by 897): on main at aae32e4 it packs at 773,560 bytes and on this branch at 773,896. main sits at the edge of that budget, and the previous head failed verify:packed-manifests by those bytes.

Measured

The film

Studio-like playback of that film in <hyperframes-player> at 1x for the whole 50 s, headless Chrome 152 on an Apple silicon Mac with GPU compositing, GPU rasterization and hardware video decode. Base is main with #6189 at its current head merged on top. Three builds in rotating order, three runs each, trace on, each run started with the host's 1-minute load under 10. "Off" is the same base with the hint pass disabled, the floor for a film with no hints.

before after off
film frames missed, of 1,201 (runtime clock) 2, 1, 1 1, 0, 0 1, 0, 0
longest frame of main-thread work 106.8, 107.6, 110.9 ms 62.1, 57.7, 60.4 ms 54.2, 56.7, 52.3 ms
median main-thread work per frame 3.6, 3.4, 3.6 ms 3.3, 3.5, 3.3 ms 3.4, 3.2, 3.2 ms
p95 main-thread work per frame 8.3, 7.5, 8.4 ms 9.0, 7.5, 7.2 ms 7.4, 7.4, 7.4 ms
frames over 8 ms of main-thread work 132, 60, 182 234, 52, 47 53, 54, 52

The misses before were at 13.17 to 13.25 s, where #6189 mounts the 1,595-layer scene 10 s before it starts; the single miss after and off was at 20.54/20.58 s (the next mount). The longest frame after stays about 5 ms above off: the remaining per-node work at a mount is the inline-hint check on the 8,642 added elements. Frames over 8 ms spread widely between runs of the same build (47 to 234 after here); over the earlier 15 pairs in both orders they did not follow this change.

The 48-layer fixture from #5268

48 will-change: transform lyric layers animated by GSAP, 528 px player at device scale 2, SwiftShader (the perf runner's browser). Three paired runs, main against this branch:

main this PR
largest raster / shown size, 528 px and 1100 px 1.00, 1.00 1.00, 1.00
will-change layers left 0 0
compositor layers 4 to 5 4 to 5
main-thread busy during 8 s of playback 5.5, 5.3, 5.5 % 5.7, 5.6, 5.9 %

With the swap turned off on this branch, the same probe reads 48 hinted layers, 52 to 53 compositor layers and a 3.64x raster, the numbers #5268 measured before its fix. Frames drawn with missing tiles were not measured: that needs a constrained tile budget the perf runner does not set.

Test plan

  • Unit tests added/updated

  • Manual testing performed

  • Documentation updated (if applicable)

  • Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug

  • previewRasterHints.test.ts: mounting a 50-layer scene that brings its own <style> while the preview is small reads no computed style and makes no page-wide query (fails on main with 53 style reads, and with the selector validated against the page). A layer that only a rule inside @media hints, added later, is still marked and a plain neighbour is not (fails with grouped rules not collected). Three added-node batches read the stylesheets at most once (fails with a per-batch read), and a hint rule in a <style> that arrives while the preview is small marks a layer added after it (fails without the re-read in markLayers). Mounting a hint-free scene walks no holders, and attribute and style writes on a page nothing hints do no work (fails when attributes are always watched); forgetting to clear a mark from the set fails two existing tests, and deciding from rules alone fails four. A marked layer removed and re-attached, and a copy of a marked layer added after the original is removed, both get their hint back when a child turns position: fixed (both fail without the adoption in followTree).

  • test:browser-raster (player perf shard) passes 3 runs in a row. Ignoring inline hints fails 4 existing tests; ignoring rule hints fails 2. The 14 existing tests pass unchanged, including the holder, stylesheet-load and batching cases.

  • The test file passes 3 runs in a row. Full packages/core/src/runtime suite: 98 files, 2,431 tests, exit 0. Pre-commit lint, format, typecheck pass; check-comment-citations and comment-ratchet pass.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE · 0 blocker · 0 important · 1 nit · head b228a7e7

The selector can only miss in ways that keep a hint the old code would have dropped, so a miss costs perf and never correctness. Marked elements always match through the runtime's own [data-hf-preview-raster] rule, so unmarking is unchanged. unmarkHolders still visits every added element, and a cross-origin sheet, a selector that does not parse, or a nested rule falls back to *.

  • nit previewRasterHints.ts:86: [style*="will-change"] is case-sensitive, so an inline style="WILL-CHANGE: transform" written by hand is no longer considered. [style*="will-change" i] fixes it. The same kind of miss happens for @scope (.x) { :scope { will-change: transform } }: markLayers' document.querySelectorAll(":scope") matches only <html>. Both only cost the optimization.
Scope

Read the full diff and the whole module. Checked how rule collection handles @media/@supports/@layer/@import/adopted sheets, nesting, and pseudo-element selectors (fits never read pseudo hints, so nothing is lost there). I did not run the tests locally because bun is not available here. CI was still running at review time.

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 157 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

@miguel-heygen
miguel-heygen force-pushed the perf/raster-hints-skip-unhinted branch 2 times, most recently from 60128a6 to 7f8eb9c Compare October 25, 2026 11:28

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES · 1 blocker · 0 important · 0 nits · b228a7e → 7f8eb9c

Reading mark state from the marked Set instead of the attribute breaks it for a marked layer that is detached and later re-attached. ↓ inline. I reproduced it: the repro passes on b228a7e and fails on this head.

Scope

Compared b228a7e with 7f8eb9c for previewRasterHints.ts and its test; after the rebase, those two files are the PR's whole diff. The PR's test file passes locally (20/20). The inline-hint check now reads style.getPropertyValue, which resolves the case-sensitivity nit from my earlier review.


function forgetRemovedMarks(records: MutationRecord[]): void {
if (!records.some((record) => record.removedNodes.length)) return;
for (const element of marked) if (!element.isConnected) marked.delete(element);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocker: a marked layer that is detached and re-attached in a later batch keeps data-hf-preview-raster but is no longer in marked, so nothing restores its hint when it stops qualifying. forgetRemovedMarks drops it from the Set while the attribute stays on the node. On re-attach, fits sees isMarked = false, reads will-change: auto through our own !important rule, and returns false, and writeMark(false) is a no-op because the Set already says unmarked. From then on, followStyle skips the layer, and if this was the only mark, watch() stops observing attributes altogether. Result: a later position: fixed child, or the layer's own z-index going back to auto, paints without the containing block or stacking context the hint provides. Main and b228a7e read the attribute, so they re-check the layer.

Repro (fails on 7f8eb9c, passes on b228a7e):

const words = layer("position: absolute; will-change: transform");
const child = layer("", words);
setPreviewRasterScale(0.5);
words.remove(); await Promise.resolve();
document.body.append(words); await Promise.resolve();
child.style.position = "fixed"; await Promise.resolve();
expect(marked(words)).toBe(false); // stays true

Fix: re-adopt the attribute when an element is followed, e.g. at the top of the followTree loop: if (element.hasAttribute(PREVIEW_RASTER_ATTR)) marked.add(element);. watch() then re-arms attributes as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 935faf2, as you suggested: followTree adopts any element it visits that already carries data-hf-preview-raster back into marked (adoptCopiedMark), so fits reads it as marked, followStyle follows it and watch() re-arms attributes. Your repro is now a test ("gives the hint back to a marked layer re-attached after removal once a child needs it"). The same gap existed for a cloneNode(true) copy of a marked layer added after the original is removed, so that has a test too. Both fail without the adoption. Core runtime suite 2,431 green, test:browser-raster 3/3.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES · 1 blocker · 0 important · 0 nits · 7f8eb9c → 935faf2

✅ The re-attach blocker is fixed: adoptCopiedMark picks the mark back up from the attribute. My repro and the PR's 22 tests pass locally (24/24).

New blocker: CI fails on this head. In the Build job, verify:packed-manifests reports @hyperframes/producer packed size 776219 bytes is over its budget of 768210 by more than 1% (the limit is 775,892). The producer packs the modular core runtime, so this PR's runtime growth on top of current main pushes it past the 1% tolerance. Main alone (aae32e43e) is green. The Studio edit-accuracy gate failure only follows from Build: its shards were skipped. Fix: raise the producer budget in scripts/package-size-budgets.json and give the reason in the PR body, as the check asks, then re-run CI.

@miguel-heygen
miguel-heygen force-pushed the perf/raster-hints-skip-unhinted branch from 935faf2 to 32984da Compare October 25, 2026 12:20
…ster-hint fix adds

The producer bundles the runtime. On main (aae32e4) it packs at 773560 bytes
(2821768 unpacked) and this branch at 773896 (2822665): packed rises by 336, unpacked by 897.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE · 0 resolved, 0 open, 0 new · 935faf2 → 87c82d4

✅ CI blocker fixed: the producer budget rises by the 336 packed / 897 unpacked bytes this change adds, the reason is in the PR body, and CI is green on this head. After the rebase, previewRasterHints.ts and its test are byte-identical to 935faf2, which I verified earlier (24/24 locally).

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 25, 2026
Merged via the queue into main with commit 89af290 Oct 25, 2026
84 of 85 checks passed
@miguel-heygen
miguel-heygen deleted the perf/raster-hints-skip-unhinted branch October 25, 2026 13:09
@heygengenesis heygengenesis Bot mentioned this pull request Oct 25, 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.

1 participant