Skip to content

fix(core): grade an image that appears after its scene starts hidden - #6012

Merged
miguel-heygen merged 3 commits into
mainfrom
fix/img-grade-late-visible
Oct 21, 2026
Merged

miguel-heygen merged 3 commits into
mainfrom
fix/img-grade-late-visible

Conversation

@miguel-heygen

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

Copy link
Copy Markdown
Collaborator

What a user sees

A color-graded <img> (set with media-treatment / data-color-grading) rendered ungraded in snapshot and render when it was hidden at the moment grading first scanned the page:

  • an image in a layer that starts visibility:hidden and a timeline reveals later (tl.set(layer, { visibility: "visible" }))
  • an image in a sub-composition whose host starts later than 0 s
  • a full-frame still inserted mid-timeline inside a timed wrapper

Reported on 0.8.181. The same grade worked when the scene started at 0 s, and when the layer was shown with opacity instead of visibility.

Why

refresh() in packages/core/src/runtime/colorGrading.ts decides 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 or data-color-grading change. An ancestor's visibility flipping (a timeline set, a host's clip window opening) is neither, so the image was never adopted. The per-seek redraw() 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() and redrawAnimated() (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 as refresh(). 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:

  • a grade Studio switches off live stays off, even while the saved attribute still grades;
  • media the runtime hides or un-grades itself is never re-adopted;
  • a page without WebGL does not retry context creation every frame (caught by the existing allocates color grading only for the active timed media test);
  • no per-frame document scan: only media still waiting to be shown is checked.

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.
  • CLI snapshot on 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).

Audited How
Adoption against refresh, setSourceVisibility, setGrading, context loss code read plus jsdom probes
Every redraw / redrawAnimated caller and the render-seek ordering init.ts, player.ts, frameCapture.ts, screenshotService.ts
Each new test fails when its part of the fix is removed mutations, one per test

Not 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.

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.
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 177 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)

Unstable (1)

  • seqfastkeys-none-px-r0-root-z100: tracking 7.76, pressJump 0.05, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true / tracking 0.07, pressJump 0.05, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0.05, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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: seek and the render-frame seek call colorGrading.redraw() after seekTimelineAndAdapters (init.ts:4233, :4293). So the image is adopted after the timeline has set its ancestor's visibility, and before waitForActiveLuts (frameCapture.ts:2904).
    • Playback: the tick calls redrawAnimated() (init.ts:5157).
  • hiddenAtRefresh gets filled whenever an image is first seen hidden. createColorGradingRuntime ends with a refresh(), and the observer runs refresh() 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:
    • upsert removes the element, so a live setGrading, setSourceVisibility or a successful adoption takes it out.
    • removeElement removes 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 next refresh(). That also closes the gap I raised at 3e884f94, where failedWebGlSetup was 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.ts 103/103;
    • the init color-grading test passes. Locally, jsdom can't load the node:fs import, so I swapped that one line for process.getBuiltinModule while running it;
    • lateVisibleImageGrading.integration.test.ts 3/3 in a real render (ffmpeg 7.0.2, software GPU). With base colorGrading.ts rebuilt into core, all three fail.
  • LUT probe: I copied the integration test with a .cube LUT 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;
    • removeElement keeping the set entry, which turns the new live-off test red.

Non-blocking

  • One line has no test: the hiddenAtRefresh.delete at the top of upsert (colorGrading.ts:3895). Without it, a live setGrading on 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:

    1. hide the scene;
    2. call setGrading(image, liveGrade);
    3. show the scene and call redraw();
    4. remove the attribute and call refresh();
    5. 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

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 21, 2026
Merged via the queue into main with commit 0874518 Oct 21, 2026
123 checks passed
@miguel-heygen
miguel-heygen deleted the fix/img-grade-late-visible branch October 21, 2026 11:58
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.

2 participants