Repository navigation
perf(core): a render frame with no mover to blur skips capture blur - #7038
vanceingalls wants to merge 2 commits into
Conversation
5a7dfe7 to
251823b
Compare
Edit accuracy: accurate 267 (base branch 267), smooth 89 of thoseThe gate passes. Quarantined, measured but not gated (8)
Unstable (1)
|
A render frame decides from its own pose reads: when the compositor keeps no mover for the frame, the root is unwrapped and the capture is skipped. The decision follows the reads (the same `kept` test the sharp phase used), so a shake between two scan steps still blurs and a sub-pixel mover draws plain. wrap() stays before the reads. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
…s and set order Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
251823b to
4eaa255
Compare
vanceingalls
changed the title
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 1 nit · head 4eaa255
The skip really is exact. It is the old path's own kept from compositor.plan(), and with nothing kept the old sharp phase was drawImage(capture) with no trails. The skip runs before the composite is armed, so the engine never gets a handshake.
- nit (PR body): Limits says software GPU sees "no gain". But the Linux
spacex-launchpairs went 62.2 / 62.2 → 63.1 / 64.4, about 1.5–3.5 % slower, and that was before the scroll fix added reads. Hosted renders use software GPU, so please call it a small loss on mixed films there, not neutral.
Scope
Read index.ts end to end (start/unwrap/finishJob/arm/finish, paintCaptureMotionBlur, armPageComposite) plus all three test diffs.
Checked:
- A job in flight is revealed and settled by
unwrap(). A queued preview request is released instead of restarted, which is better than the oldfinishJobre-entry. - Iframes are already the
iframe-in-rootdefect. CSS transitions killed by the extra moves are already thecss_transition_usedlint warning. The CSS adapter re-readsgetAnimations()on the resync. report().framesandmeanMshave no engine or CLI reader.
Locally, the blur and GSAP step suites pass (287 tests). All CI on this head is green.
— Claude (Genesis)
|
Correction to the Results section (added after the review request). The timings above use draft quality and 4 workers. I then ran the Desktop's export protocol (standard quality, 8 workers, one warm-up, median of 3 rotated rounds, the
So under that protocol this change is neutral to roughly 10% slower on the capture-tier films, not 20-50% faster. I am isolating which setting flips it (quality, worker count, or the load) and will update this PR before it merges. The correctness results are unaffected. Please do not merge on the speedup numbers above. |
|
Follow-up on the correction above: the speed claim is unresolved, not disproved. I isolated quality and worker count with old and new interleaved on What I am confident of: correctness (no failed frames, no frame further from a blur-off render than |
|
Quiet-machine rerun: no speedup under the Desktop's export settings. Please do not merge as it stands. Protocol: standard quality, 8 workers, Chrome 156, hardware GPU, Medians, Subtle as a multiple of blur off:
Strong shows the same shape ( Likely cause (not yet proven): a frame drawn plain pays Moving this to draft until the design is reworked or the PR is closed. |
|
Closing. At Desktop export settings (standard quality, 8 workers) on a quiet machine this gave no speedup, and the scroll-offset part shipped as #7190. The skip-capture-on-plain-frames idea can come back if a measurement on a real film shows a gain. |
What
With capture blur on, every render frame goes through the engine's composite handshake (a forced paint and two page calls), a full
drawElementImageof the root and the pose reads, even when no mover would be blurred on that frame. Now a render frame whose pose reads keep no mover is drawn as the plain page: the root is unwrapped right after the reads, so the engine's composite step is never armed and no capture runs. Preview, the filter tier and frames that keep a mover are unchanged.How
start()keepsmain's order:wrap(), then the reads, thencompositor.plan(). Right after the plan,if (noMoverToBlur(kept, engineMode)) { unwrap(); return Promise.resolve(); }. The test is the old path's ownkept, so a frame is skipped exactly when the old path would have drawn it sharp. Nothing is predicted from the scan, so there is no new way to draw a blurred frame sharp.wrap()has to stay before the reads. A trial that moved it after them failed three existing tests, because the root's silent resync seek then lands afterholdFrame's restore. Onlyunwrap()follows the reads, and its resync is a same-time silent seek. Two new real-GSAP tests pin that this extra seek neither re-fires a call nor reorders same-time sets.Chrome drops the
scrollTop/scrollLeftof scrolled containers when the root moves in or out of thelayoutsubtreecanvas. Onmainthe root stays wrapped, but a root that now alternates would lose them, and GSAP's same-time resync cannot put them back. Sodiscover()collects the scroll containers once per scan andmoveRootThenResyncsaves and restores their offsets around the move.What this replaces on this branch
An earlier revision decided before the reads, from a hash of every changing element's box at each scan step (
stillAt). It skipped the reads too, but any move that leaves and returns to its box between two scan steps rendered sharp: a 0.06 s shake in a 20 s film lost 3 frames (smear up to 43 px), a 0.3 s shake in a 120 s film 10 frames. No frame of the 12 capture-tier launch films hit it (0 of 12,990), but nothing prevented it. Deciding after the reads removes the blind spot and is also faster: it saves the capture on every frame without a mover to blur, not only on frames where no box moves.Results
Mac, Chrome 156, hardware GPU, 4 workers, draft quality,
mainbuild against this branch, two alternated pairs. Wall seconds, old to new:onUpdateon frames drawn plain: 9.8 / 9.8 to 5.2 / 5.1scrollTopand holds: 9.9 / 9.8 to 4.1 / 4.4k3-promo: 18.4 / 18.4 to 15.0 / 15.2spacex-launch: 38.5 / 37.5 to 25.3 / 24.6website-to-hyperframes: 43.5 / 39.9 to 22.1 / 24.8figma-launch: 65.8 / 62.5 to 45.6 / 47.1failedFrames=0in every render. Compared per frame with themainoutput, no frame of the constructed films is below 56 dB (the calls film is above 84 dB on the frames with calls), and on real films the new output is closer to a blur-off render thanmain's on 114 (k3-promo), 319 (spacex-launch), 257 (website) and 797 (figma) frames, and further by more than 1 dB on at most 15, all at noise level or identical frames (101 to 99 dB).maindraws the frames it calls sharp through the capture, which is slightly less faithful than the plain page.Linux container (Chrome 156, software GPU), same protocol, before the scroll fix: the 20 s shake film 16.4 / 16.4 to 4.5 / 4.5, the calls film 8.9 / 8.8 to 5.0 / 5.0, but
k3-promo41.6 / 41.1 to 39.3 / 39.1 andspacex-launch62.2 / 62.2 to 63.1 / 64.4.Limits
k3-promoandspacex-launch. Hosted renders are software-GPU renders.wrap(), the 8 pose reads andunwrap(), with two silent same-time seeks and two moves of the root.report().framesandmeanMsnow count only frames that went through capture. Nothing in the engine reads them for decisions.main, which vary per worker.websiteandfigmatimings and the Linux numbers were taken before the scroll fix, which adds one read per scroll container on a root move.Tests
The blur folder and the GSAP step tests pass (287 tests). New: a shake that falls between two scan steps stays blurred; a mover moving under a pixel across the shutter draws plain; a frame with no mover skips the capture (with a positive control that a moving frame draws); a frame left in flight is settled and its movers revealed; preview is unchanged; scroll offsets survive a wrap, an unwrap and a wrap again; and two real-GSAP tests for the same-time silent seek. The engine pixel test and the producer
motionBlurVideointegration test run in CI.🤖 Generated with Claude Code