fix(core): compile only the color grading stages a grade enables - #4884
miga-heygen wants to merge 3 commits into
Conversation
Every data-color-grading element drew through one fragment shader with every effect compiled in. An effect at 0 returns early, which is free on a GPU, but software GL and some ANGLE backends still pay for every stage on every pixel. A 1080p adjust-only grade took about 6s per draw on SwiftShader, so `hyperframes check` on one graded 3s clip went from 15s to 231s and read as a hang. Each optional stage now sits behind an #ifdef HF_STAGE_* guard. FRAGMENT_SHADER_STAGES maps each define to the grade condition that makes the stage change a pixel, and useMainShader compiles and caches the matching variant per WebGL context (LRU, sized to one Studio preview batch). Both draw sites bind the main program only through it. Output is byte-identical to the previous shader across every stage. The same check now takes 23s. Fixes #4881. Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com>
…ompile A variant the driver rejects is remembered and draws through the all-stages variant, which is the previous shader, instead of being recompiled every frame. If that fails too, the context stops compiling and reports the draw error. Adds tests for the animated-grade and look-preview selection paths. Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com>
Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com>
Edit accuracy: 852 passing here, 852 on the base branchThe gate passes. Quarantined, measured but not gated (2)
Unstable (2)
|
miga-heygen
left a comment
There was a problem hiding this comment.
Reviewed at e70c202a83875b2ba03d88647499e941f9fd4c50. No blockers. The parity between the stage predicates and the GLSL guards holds, and the cache lifecycle is correct. The only gaps are in test coverage.
Strengths
- Removing the
#ifdefguards from the newFRAGMENT_SHADERleaves text byte-identical to the old shader, and the vertex shader is unchanged. The fallback variant really is the previous shader, so a rejected variant cannot render differently. useMainShader(colorGrading.ts:3333,:3516) receives the samegradingandcompareobjects thatapplyUniformsgets. The variant cannot be keyed to a different grade from the uniforms it is drawn with.mainShaderFor(colorGrading.ts:1893) terminates in every case. If a variant fails, it falls back to all stages. If all stages fail, they are marked failed and the next lookup throws at:1902, whichdrawEntryturns intodrawError.
Verification, run on the head
- Compiling the variants: 83 variants compiled and linked in headless Chromium on SwiftShader (ANGLE/Vulkan), with no failures. The set was: no stages, all stages, each stage alone, and all stages but one. All-but-one catches a variable declared inside one guard and used under another.
- Old vs new pixels:
- The old shader and the new shader each drew 400 grades with random uniforms, at 96x54. 0 of 400 cases differed in
readPixels. - Each case defined the set of stages from my own reading of the GLSL comparisons (clamp, then
> 0; flags>= 0.5; secondary count), not fromFRAGMENT_SHADER_STAGES. - Amounts included
1e-6,5e-5(CRT under thestep(0.0001)threshold), values above 1, and negatives.
- The old shader and the new shader each drew 400 grades with random uniforms, at 96x54. 0 of 400 cases differed in
- Harness control: dropping one active stage from the derived set gave 52 of 60 mismatches, so the harness can see a missing stage.
- Predicate to uniform keys: every predicate reads the key whose
u_${key}uniform the GLSL compares.LUT(grading.lut !== null) is a superset ofu_lutEnabled, which is correct, as the PR notes. - Unit tests:
vitest run src/runtime/colorGrading.test.tspassed 88 of 88.tsc --noEmitis clean fortsconfig.jsonandtsconfig.runtime.json. - Cache mutation tests:
- Dropping the LRU re-insert on a hit fails the eviction test.
- Dropping
failedStageDefines.addfails both rejection tests.
Nits
-
The tests do not pin the GL resource cleanup or the cap bound. Each of these mutations still leaves 88 of 88 green:
- Deleting
gl.deleteProgram(evicted.program)atcolorGrading.ts:1918, which leaks a program per eviction. - Deleting the
deleteProgramloop indestroyMainProgramResourcesat:2131. - Changing
>=to>at:1916, which lets the cache hold 33.
One
deleteProgramspy assertion in the eviction test, and one on runtime teardown, would close all three. - Deleting
-
Note, no change asked: on a hardware GPU, a stage set first seen mid-playback compiles synchronously inside
drawEntry. An example is an animated effect rising above 0 for the first time. That is one hitch per stage set per context, where before there was none. This trade is fine for #4881, but worth knowing if Studio playback reports a stutter on the first glitch or bloom keyframe.
Verdict: COMMENT (approve-level; this account authored the PR, so it cannot approve it)
Reasoning: The real GLSL compiles in every guard combination, and the derived variants render pixel-identical to the old shader across 400 random grades. The cache evicts least-recently-used, memoizes failures and always terminates. The open items are test coverage only.
— Miga
Fixes #4881.
What was wrong
Every
data-color-gradingelement drew through one fragment shader with every effect compiled in. A stage set to 0 returns early, which costs nothing on a GPU. Software GL (SwiftShader, whichcheckuses when WebGL is unavailable or with--no-browser-gpu) and some ANGLE backends still pay for every stage on every pixel. So an adjust-only grade paid for ASCII, tape damage, crosshatch, glitch and the rest on each draw.Measured on 0.8.106 with software GL, one 3s 1080p clip, adjust-only grade:
hyperframes checkcheckdraws the grade about 60 times per run, at 3 to 4s each. On the reporter's Iris Xe with ANGLE/D3D11, that reads as a hang.The fix
#ifdef HF_STAGE_*, and a guarded function falls back to returning its input.FRAGMENT_SHADER_STAGESis one table mapping each define to the grade condition that makes the stage change a pixel. That condition mirrors the shader's ownamount > 0test.useMainShadercompiles the variant for the grade's active stages, caches it per WebGL context in least-recently-used order, sized to one full Studio look-preview batch (32). It is the only way either draw site (drawEntry,renderPreviewCandidate) binds the main program. Preview, render and Studio look previews therefore share one path.Equivalence and speed
readPixelsoutput in 28 of 28 cases. Each stage was tested alone with its sub-parameters, plus RGB curves, hue curves, secondaries, LUT, compare, a combination, and tape sub-parameters with tape damage at 0.uvdirectly differs from(uv * 2 - 1) * 0.5 + 0.5by one ulp, and that flips digital-glitch block boundaries.Tests
packages/core/src/runtime/colorGrading.test.ts, through the real runtime:Mutation-checked: all of these fail against the previous runtime. Deleting one table row fails the drift test. Swapping two predicates fails the per-stage test.
Verification
vitest run src/runtime/colorGrading.test.ts: 87 passed.tsc --noEmitforpackages/coreandtsconfig.runtime.json: clean.hyperframes checkon the 1080p adjust-only repro with software GL: 231s on 0.8.106, 23s with this branch. The ungraded baseline is 15s.Independent review
Ranked findings from an independent adversarial review of this branch:
Miga