Repository navigation
fix(core): grade an image that appears after its scene starts hidden - #6012
Conversation
A graded <img> was only picked up if it was visible when grading scanned the page. An image in a layer revealed later by a timeline, or in a sub-composition whose host starts later, stayed ungraded in snapshot and render. Each draw pass now adopts graded media that has come into view.
Edit accuracy: accurate 271 (base branch 271), smooth 177 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
vanceingalls
left a comment
There was a problem hiding this comment.
Reviewed at 0dcb6184. I reviewed a2eaaec1 in full, then the two later commits on their own: 3e884f94 (rename only) and 0dcb6184 (adoption limited to media hidden at the last scan). There are no other reviews.
What holds
- Every path that reveals the layer adopts the image, on both surfaces:
- Render and snapshot:
seekand the render-frame seek callcolorGrading.redraw()afterseekTimelineAndAdapters(init.ts:4233,:4293). So the image is adopted after the timeline has set its ancestor's visibility, and beforewaitForActiveLuts(frameCapture.ts:2904). - Playback: the tick calls
redrawAnimated()(init.ts:5157).
- Render and snapshot:
hiddenAtRefreshgets filled whenever an image is first seen hidden.createColorGradingRuntimeends with arefresh(), and the observer runsrefresh()again on every insert, so a page that loads with a hidden layer, or a sub-composition inlined later, both record their hidden media.- Entries are removed from the set on every path that should drop them:
upsertremoves the element, so a livesetGrading,setSourceVisibilityor a successful adoption takes it out.removeElementremoves it, so turning a grade off live drops it, and the new test covers that.- Adoption removes it before trying
upsert, so media whose WebGL setup fails isn't retried every frame. It is retried at the nextrefresh(). That also closes the gap I raised at3e884f94, wherefailedWebGlSetupwas never cleared.
- Images visible from the start are unchanged. They never enter the set, and per-frame work is now just a walk over the set, so the full-document query I flagged earlier is gone.
- Verified locally at
0dcb6184:- core
colorGrading.test.ts103/103; - the init color-grading test passes. Locally, jsdom can't load the
node:fsimport, so I swapped that one line forprocess.getBuiltinModulewhile running it; lateVisibleImageGrading.integration.test.ts3/3 in a real render (ffmpeg 7.0.2, software GPU). With basecolorGrading.tsrebuilt into core, all three fail.
- core
- LUT probe: I copied the integration test with a
.cubeLUT as the only grade. It passes 3/3 at this head and fails on base, so the LUT is loaded before the reveal frame is captured. - Mutation checks (each turns a test red):
- dropping adoption from
redraw(); - dropping adoption from
redrawAnimated(); refresh()never recording hidden media;removeElementkeeping the set entry, which turns the new live-off test red.
- dropping adoption from
Non-blocking
-
One line has no test: the
hiddenAtRefresh.deleteat the top ofupsert(colorGrading.ts:3895). Without it, a livesetGradingon an image while it is hidden is replaced by the saved attribute grade when the image is revealed. With the line, the live grade stays: my probe passes at head and fails when the line is removed. The probe:- hide the scene;
- call
setGrading(image, liveGrade); - show the scene and call
redraw(); - remove the attribute and call
refresh(); - expect
isGraded.
That probe would pin the line.
CI: the eight required checks passed at 0dcb6184 in CI run 39564691110: Build, Render on windows-latest, Semantic PR title, Test, Test: runtime contract, Tests on windows-latest, Typecheck and regression. A second pull_request run on the same head (39564747650) skipped its jobs at change detection.
Verdict: APPROVE
Reasoning: Media first seen hidden is adopted on the first seek or playback tick that shows it, so a layer revealed by visibility, a late sub-composition host and a timed insert are graded from their first frame, in a real render and with LUTs. A grade turned off live stays off, and images visible from the start never reach the new path.
— Via
What a user sees
A color-graded
<img>(set withmedia-treatment/data-color-grading) rendered ungraded insnapshotandrenderwhen it was hidden at the moment grading first scanned the page:visibility:hiddenand a timeline reveals later (tl.set(layer, { visibility: "visible" }))Reported on 0.8.181. The same grade worked when the scene started at 0 s, and when the layer was shown with
opacityinstead ofvisibility.Why
refresh()inpackages/core/src/runtime/colorGrading.tsdecides which media gets graded, and keeps an element only if its computed visibility is visible right then. After the first reveal pass it only re-runs on a DOM child-list ordata-color-gradingchange. An ancestor'svisibilityflipping (a timelineset, a host's clip window opening) is neither, so the image was never adopted. The per-seekredraw()that snapshot and render call before every capture only draws media that is already tracked. Tracked entries already follow visibility on every draw (syncSourceAppearance); only adoption was missing.It sometimes looked fine: grading any other element inserts its canvas, which fires the observer and rescues hidden images by accident. The tests use one project per shape for this reason.
Fix
refresh()now records the graded media it skipped because it was hidden.redraw()andredrawAnimated()(the seek path and the playback tick) first re-check only that list and adopt whatever has come into view, using the same visibility rule asrefresh(). That rule now has one owner,isShownForColorGrading. An element leaves the list as soon as anything tries to set it up or removes it on purpose, so:allocates color grading only for the active timed mediatest);Tests
packages/producer/src/services/render/lateVisibleImageGrading.integration.test.ts: a real render of the three shapes, one project each. On main all three fail at the reveal frame (level 128, the ungraded source grey); with the fix all three pass, 3 runs in a row.packages/core/src/runtime/colorGrading.test.ts: an image whose ancestor starts hidden is graded once a seek (redraw) or a playback tick (redrawAnimated) shows it; both fail on main. A grade switched off live stays off, whether the image is shown or still hidden; these fail if adoption scans every graded element or if removal leaves an element on the waiting list. 103/103 pass, 3 runs.packages/core/src/runtime: 2165/2165 pass.snapshoton the same fixtures: before, the revealed layer and the late host measured 126 (ungraded); after, 63 (graded).Independent pre-review
No blockers, no majors. Its minor findings are addressed on this head: a live grade switched off came back on the next frame (fixed, with a test), a failed setup blocked an element for good (that skip list is gone), and the per-frame document scan (gone).
refresh,setSourceVisibility,setGrading, context lossredraw/redrawAnimatedcaller and the render-seek orderinginit.ts,player.ts,frameCapture.ts,screenshotService.tsNot exercised: a real Chrome benchmark with many graded elements, Studio playback end to end, a real WebGL context loss, a render with a LUT, layered capture, Linux.