Repository navigation
fix: the preview preloads the coming scene's images, and check stops failing on image swaps - #5128
Conversation
…s by whether they still decode
…s restore real timers
Edit accuracy: accurate 2059 (base branch 2059), smooth 1463 of thoseThe gate passes. Quarantined, measured but not gated (0) |
There was a problem hiding this comment.
Reviewed at exact head 905473da3749f2b2d9a28d8fae64ef93b9c2badc. packages/core/src/runtime/init.ts:388-396,2782-2786,3868-3889 scopes eager promotion to Studio-marked images in the approaching clip and shares it with paused seeks; authored lazy images and nested later clips retain their timing. packages/cli/src/utils/checkBrowser.ts:374-411,414-441 checks aborted image requests after scrubbing while retaining HTTP errors and other network failures.
A Chromium reproduction using actual base/head bundled runtimes, a production-positioned head script, and a synthetic 320px PNG delayed by 1.5 seconds observed the later clip’s first visible frame with image incomplete (width 0) before and complete (width 320) after, in 3/3 runs per revision. This confirms the mechanism, not the PR body’s exact Studio fixture or screenshots; those hosted screenshots were not independently inspected. Focused runtime tests passed 32/32; all 11 required checks passed at this head. The focused CLI suite could not collect locally because of unbuilt transitive workspace packages.
Verdict: APPROVE
Reasoning: The first-frame preload and own-scrub cancellation paths have no verified correctness regression in the reviewed diff.
— Review by tai (pr-review)
What
Two image-loading fixes from user reports:
loading="lazy", and the runtime hides a scene that has not started withdisplay: none. Chrome never fetches a lazy image that has no layout box, so the 2 s look-ahead marked the next scene as coming up but its images still did not load: on the scene's first frame the image was missing. The runtime now switches Studio's lazy images in a clip to eager the moment that clip comes within the look-ahead, which is what a paused seek onto a scene already did (both now share one helper). Only the clip's own images switch: a clip nested inside it waits for its own look-ahead, so a chapter's later scenes stay unloaded. Images the author markedloading="lazy"themselves are left alone.hyperframes checkstops reporting image loads its own scrubbing cancelled. Check samples the timeline 8 times a second without waiting, by design. In a film that swaps an<img>source through numbered frames, each sample cancels the previous frame's load, and check reported every cancelled request asrequest_failed ... net::ERR_ABORTED, failing the run. After the sampling, check asks the page about each aborted image: it is reported only when an<img>still shows that URL and its decode fails. A load still pending after 5 s (a deferred lazy image) is not counted as a failure. One a later swap replaced, or that has since loaded, is dropped. A missing file still fails the check, as before (missing_local_asset/http_error). An aborted image that is not an<img>source (a CSS background, an SVG<image>) is no longer reported, since an abort means the page cancelled it, not that it is missing.Why
img.decode()on such an image waited forever.checkfailed every film with an image sequence of non-trivial frame size, though nothing was wrong with the project.Test plan
entry.test.ts: a Studio lazy image in a clip within the look-ahead is eager while the clip is still hidden; one further away stays lazy until a seek brings it near; one in a clip nested inside the coming clip stays lazy; an authored lazy image stays lazy. Fails on main (lazywhereeageris expected).checkBrowser.test.ts: the filter keeps an aborted image only when the page reports it broken, and every non-image failure; a test through the wholecheckpath reports an aborted image still shown and broken, and drops one swapped away and one still shown that decodes; another (fake timers, no real wait) pins that a reload still pending at the 5 s cap is not reported. Fails on main.loading=lazy(3/3)loading=eager(3/3)hyperframes checkon a 4 s film swapping one<img>through 60 PNG frames at 30 fps:request_failed ... net::ERR_ABORTEDrequest_failed ... net::ERR_ABORTED<img>pointing at a file that does not existmissing_local_asset), exit 1Before / After
The preview of a 4 s composition whose second scene (in normal flow) starts at 2 s and holds one 1280×720 image, played from 0 and captured on the first frame at or after 2 s (read at t = 2.00–2.01 s, screenshot done by t ≈ 2.2–2.3 s). The image is served 1.5 s late to stand in for a real network. 3 runs per side, same result each time.
Before
main (
156e87766c7): scene 2 has started and its image has not loaded (complete: false,loading=lazy).After
This branch (
905473da374): the same frame shows the image (complete: true, 1280 px wide,loading=eager), fetched during the look-ahead.Not in this PR
decode()on every image of the preview will still wait on images of scenes far beyond the look-ahead: keeping those unloaded is the preview's memory saving. Tools that need every image should load the capture document (?hf-capture=1), which serves no lazy images.validatecommand keeps its own request listener unchanged. It seeks only 5 times with a 150 ms settle each, not a dense no-settle pass, so it is much less likely to cancel its own loads.