Skip to content

fix(core): accept 65-point .cube LUTs, the size grading apps export - #6921

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/lut-size-65
Nov 1, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
fix/lut-size-65

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

What

media-use resolve --type lut and the color grader now accept 65-point .cube files, the other size grading apps export besides 17 and 33. A 65-point LUT used to stop with LUT_3D_SIZE 65 exceeds max 64.

Why

The cap was a bare default of 64 with no stated reason. The only size-dependent GPU cost is the packed texture, size * size pixels wide: 4225 for 65, well inside what headless Chrome and desktop GPUs allow.

Change

  • DEFAULT_MAX_CUBE_LUT_SIZE is 65 in core and in the media-use mirror (kept in step by the existing mirror tests).
  • grade-compare stops carrying its own literal 64 and uses the default.
  • The capability catalog and the media-effects guide report 65.

Test

New tests: a 65-point cube parses (core and media-use), 66 is rejected naming the max, the catalog reports 65. With both constants set back to 64, 6 of them fail; at 65 they pass 3 runs in a row.

Sizes above 65 are still refused, with the message naming the max.

fails without the fix: packages/core/src/colorLuts.test.ts › cube LUT parsing › accepts the 65-point cube that grading tools export

Texture limit

A 65-point cube packs into a texture 4225 wide, past MAX_TEXTURE_SIZE (4096) on older GPUs, where the upload fails silently and the grade renders black. Both upload sites (render and preview) now resample such a cube trilinearly to the largest size that fits (64 on a 4096 limit) before packing; cubes that fit are untouched.

fails without the fix: packages/core/src/runtime/colorGrading.test.ts › shrinks a LUT whose packed width exceeds the GPU texture limit
fails without the fix: packages/core/src/colorLuts.test.ts › fitCubeLutToTexture › resamples a cube whose packed row is wider than the texture limit, keeping an identity identity

@mintlify

mintlify Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
hyperframes 🟢 Ready View Preview Nov 1, 2026, 10:04 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed at 6cb7c8c0.

Blocker (process): this is a fix PR, and the body has no fails without the fix: line naming a test. The Test section says "6 of them fail" with the constants back at 64, but it doesn't say which ones. Please add a line like:

fails without the fix: packages/core/src/colorLuts.test.ts › cube LUT parsing › accepts the 65-point cube that grading tools export

(the media-use cube-validate.test.mjs 65-point test fits too). I'll approve once that's there.

Code: I found no problems.

  • Both DEFAULT_MAX_CUBE_LUT_SIZE constants, core and the media-use mirror, are now 65. The runtime (runtime/colorGrading.ts), the capability catalog and grade-compare all read that default, so nothing else still hard-codes 64 on the ingest path.
  • The packed texture for 65 is 4225×65 RGBA8, about 1.1 MB, which is fine for Chrome and SwiftShader.
  • New tests cover both edges: 65 is accepted and 66 is rejected with the max named.

Nit (not blocking): packages/cli/src/media-use/lib/cube-build.mjs still has MAX_SIZE = 64 for generated LUTs. That's a separate builder limit, so leaving it is fine, but you could raise it in this PR for consistency.

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

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

  • seqplainkeys-none-center-r0-nested-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true
  • seqplainkeys-none-center-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true
  • seqplainkeys-none-pct-r0-nested-z100: tracking 0.05, pressJump 0, drop 0, reload 0, render 0.02, renderKey -, undo false, teleport true / tracking 0.05, pressJump 0, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true / tracking 0.05, pressJump 0, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true

@somanshreddy somanshreddy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review at 6cb7c8c0: one hold-worthy finding. The 65-point LUT doesn't fit the packed texture on GPUs whose MAX_TEXTURE_SIZE is 4096. The body's fails without the fix: line is now present, and the cap change itself is consistent: core, the media-use copy, grade-compare (which now uses the default), and the docs/tests all agree on 65.

Hold: a 65-point LUT packs to a 4225-px-wide texture

packCubeLutToRgba8 (packages/core/src/colorLuts.ts:206-209) lays the cube out as width = size * size by height = size. So:

  • At 64, that's 4096 × 64: exactly the 4096 MAX_TEXTURE_SIZE that a lot of WebGL1 hardware reports (older Intel iGPUs, many Android GPUs, older iOS). The old cap of 64 may have been the largest size that packs in one row on those devices; I couldn't find a stated reason in 7c1443849.
  • At 65, it's 4225 × 65. On a 4096-limit GPU, uploadLutTexture's gl.texImage2D (runtime/colorGrading.ts:2099) fails with INVALID_VALUE. WebGL reports that via getError, it doesn't throw, so the try/catch in uploadEntryLut (:2062) never sees it. entry.lut is then recorded as ready, and sampleLut reads from an incomplete texture: black / (0,0,0,1). The graded video goes black or badly off-colour in the preview on those machines, with no error and no fallback.
  • Not affected: the CPU bake path (colorGradingBake.ts:349) has no texture limit. Desktop GPUs reporting 8192+ (and SwiftShader in headless capture) are fine too, which is likely why nothing in CI or a typical dev box shows it.

What clears it (any one):

  1. Pack so neither side exceeds 4096. For example, tile the size slices in a grid: 65 slices as 9×8 tiles of 65×65 gives 585×520. The shader's sampleLut x/y math would change to match.
  2. Or check gl.getParameter(gl.MAX_TEXTURE_SIZE) before upload, and when size*size exceeds it, resample the cube to the largest size that fits (64), or surface a clear LUT error instead of a silent black picture. Add a test with a stubbed MAX_TEXTURE_SIZE of 4096.
  3. Or show that every target client reports ≥ 8192, e.g. from a telemetry query, and note it in the body.

Non-blocking

  • cube-build.mjs still caps the LUTs it generates at 64 (as Genesis noted). That's fine, and it's arguably correct given the above.

Verified / not verified

  • Verified by reading at this head: the packing math, the upload path and the missing getError check. The fails without the fix: test is named and covers 65-accept / 66-refuse.
  • Not verified: I didn't run it on a 4096-limit device. The black-picture outcome follows from WebGL's spec behaviour for an oversize texImage2D, not from a repro.

— Somu

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed at 2b33db95. Approving.

  • The body now has fails without the fix: lines. All three named tests fail without the change: the 65-point parse test needs the max at 65, and the two resize tests need fitCubeLutToTexture in place.
  • The resize looks correct. Grid point i samples the source at i/(size-1)·(N-1), so the corners of the color cube stay exactly where they were. The index order (b, g, r) matches packCubeLutToRgba8, and title and domain carry over. An identity cube stays an identity to within 1e-5, and the test that checks values between source points covers the interpolation.
  • Both upload paths resize before packing and record the resized size and textureWidth, so the shader samples with the right numbers. Each path keeps its src cache (entry.lut?.src, renderer.lut?.src), so the 64³ resize runs once per LUT, not once per frame or preview.
  • An unreadable or tiny MAX_TEXTURE_SIZE (NaN, 0) gives the cube back unchanged, the same as before.

Nits (not blocking):

  • The "Why" section still says 4225 is "well inside what headless Chrome and desktop GPUs allow", which contradicts the new "Texture limit" section. Please update it before merge, since it ends up in the squash message.
  • packages/cli/src/media-use/lib/cube-build.mjs still caps generated cubes at 64 (from last round).

@somanshreddy somanshreddy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-review @ 2b33db95: my blocker is fixed; dismissing my CR (5729103676)

Blocker from 6cb7c8c0 is closed. A 65-point LUT packed as size²×size was 4225 px wide, which exceeds a 4096 MAX_TEXTURE_SIZE. That caused a silent INVALID_VALUE and a black frame. fitCubeLutToTexture now trilinearly resamples to floor(sqrt(MAX_TEXTURE_SIZE)), so 64 at 4096 packs to exactly 4096. It runs on both GPU upload paths: uploadEntryLut for render and uploadPreviewLut for preview. The fitted size and textureWidth are what get stored for sampling, while domain and title are kept. An unreadable or zero limit (NaN, 0, or null on a lost context) falls back to the unchanged cube, which is the old behaviour. The only other packer, colorGradingBake.compileLut, runs on the CPU with no texture, so it isn't affected. colorLuts.test.ts and runtime/colorGrading.test.ts pass at head (139 tests).

Should-fix: the new runtime test passes without the fix when run in the full suite. I mutated both call sites to const fitted = lut;:

  • vitest run src/runtime/colorGrading.test.ts -t "shrinks a LUT" on its own fails, as it should.
  • The whole file still passes, 124/124.

The cause is that the module-level LUT_CACHE is keyed by href. An earlier test already loaded /looks/test.cube as the 2-point IDENTITY_2, so this test reuses that cached cube instead of IDENTITY_3. A 2-point cube packs to [4, 2], the same dimensions the test expects from fitting IDENTITY_3 to a limit of 4. So in CI the test asserts nothing about the fix.

  • Fix: give the test a unique src (e.g. /looks/too-wide.cube) and pick dimensions that can't collide with a 2-point cube. For example, use a 4-point cube with maxTextureSize: 9, which should produce [9, 3].

The fitCubeLutToTexture unit tests in colorLuts.test.ts do pin the resampling itself, so this is not a blocker.

Nit (also raised by Genesis): the body's "Why" still says a 4225-px texture fits on any GPU. That contradicts the new "Texture limit" section, and the body becomes the squash message.

— Somu

@somanshreddy
somanshreddy dismissed their stale review November 1, 2026 20:13

Blocker fixed at 2b33db9: LUT is resampled to fit MAX_TEXTURE_SIZE before upload. See re-review.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit 9284e69 Nov 1, 2026
148 of 149 checks passed
@miguel-heygen
miguel-heygen deleted the fix/lut-size-65 branch November 1, 2026 21:02

@jrusso1020 jrusso1020 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.

Post-merge advisory review (this PR is already merged into main). Findings flagged for follow-up, not gating.

Reviewed at 2b33db95ac17529c082068a3f2a477b300ebb0e5.

No new correctness finding. fitCubeLutToTexture preserves endpoints, axis order, domain and title while fitting packed width to the GPU limit (packages/core/src/colorLuts.ts:228-261). Both runtime upload paths use the fitted size and packed dimensions (runtime/colorGrading.ts:2055-2132); CPU baking has no GPU texture limit. The core parser, standalone validator, capabilities and grade-compare default agree on 65. The body's fails without the fix: lines name real tests.

Existing test-hardening follow-up, as Somu already reported: the new runtime regression reuses /looks/test.cube, which an earlier test caches as a 2-point cube (runtime/colorGrading.test.ts:582-597). I corroborated the false positive: replacing both resize calls with const fitted = lut leaves all 124 original runtime tests passing. Two independent controls with unique 4-point LUT URLs and MAX_TEXTURE_SIZE=9 fail under that mutation and pass at this head; they exercise both render and preview uploads and assert [9,3] plus u_lutSize=3. Give the shipped regression a unique URL and distinguishable dimensions, and keep a render-path witness too. The helper unit tests already protect resampling, so this is test hardening, not a new production blocker.

Validation: core build passed; vitest run src/colorLuts.test.ts src/colorGrading.test.ts src/runtime/colorGrading.test.ts passed 176 tests; standalone cube-validate.test.mjs passed 9. In addition to the two upload witnesses, twelve further controls passed for nonidentity interpolation across all axes, byte packing within rounding tolerance, GPU size boundaries and unreadable-limit fallback. No physical 4096-limit GPU or live grading session was exercised. Existing negative-cache behavior is unchanged by this diff.

Need: yes, accept exported 65-point LUTs without oversize GPU uploads · Size: ok · Reuse: shared parser limit, packer and one fitting helper for both uploads · Simpler: already minimal · Structure: fine
Risk: medium — color resampling spans ingest, preview and render contracts.

Verdict: COMMENT (post-merge: Ready)
Reasoning: The earlier oversize-texture defect is fixed on both upload paths. No additional correctness defect found; the existing cache-sensitive regression needs the focused test hardening above.

— hf-review

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.

3 participants