From 1ffef9a262ca13ada04d53d93d6f8bfbbfa83f26 Mon Sep 17 00:00:00 2001 From: ukimsanov Date: Sat, 25 Jul 2026 23:16:50 -0700 Subject: [PATCH] fix(core): address grading review findings --- packages/core/src/colorGrading.test.ts | 19 ++++ packages/core/src/colorGrading.ts | 95 +++++++++++-------- packages/core/src/colorGradingCurves.ts | 61 +++++++----- packages/core/src/colorLuts.ts | 8 +- packages/core/src/index.ts | 2 - packages/core/src/runtime/colorGrading.ts | 59 +++++++++--- .../parsers/src/colorGradingContract.test.ts | 7 ++ packages/parsers/src/colorGradingContract.ts | 65 +++++++++++-- 8 files changed, 220 insertions(+), 96 deletions(-) diff --git a/packages/core/src/colorGrading.test.ts b/packages/core/src/colorGrading.test.ts index cc9b6a056..3840acb7d 100644 --- a/packages/core/src/colorGrading.test.ts +++ b/packages/core/src/colorGrading.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { COLOR_GRADING_ADVANCED_LIMITS } from "@hyperframes/parsers/color-grading-contract"; import { HF_COLOR_GRADING_COLOR_SPACE, HF_COLOR_GRADING_ACTIVE_EFFECT_KEYS, @@ -177,6 +178,24 @@ describe("color grading", () => { ]), ); expect(capabilities.secondaries.max).toBe(4); + expect(capabilities.secondaries).toMatchObject({ + hue: { + center: { + min: COLOR_GRADING_ADVANCED_LIMITS.hueDegrees.min, + maxExclusive: COLOR_GRADING_ADVANCED_LIMITS.hueDegrees.max, + }, + range: COLOR_GRADING_ADVANCED_LIMITS.secondaryHueRange, + softness: COLOR_GRADING_ADVANCED_LIMITS.secondaryHueSoftness, + rangePlusSoftnessMax: COLOR_GRADING_ADVANCED_LIMITS.secondaryHueCombinedMax, + }, + saturation: { + softness: COLOR_GRADING_ADVANCED_LIMITS.secondarySoftRangeSoftness, + }, + correction: { + hueShift: COLOR_GRADING_ADVANCED_LIMITS.secondaryHueShift, + saturation: COLOR_GRADING_ADVANCED_LIMITS.signedUnit, + }, + }); }); it("merges manual adjustments over preset values", () => { diff --git a/packages/core/src/colorGrading.ts b/packages/core/src/colorGrading.ts index 3b6b29218..f84b141d0 100644 --- a/packages/core/src/colorGrading.ts +++ b/packages/core/src/colorGrading.ts @@ -2,6 +2,7 @@ import { DEFAULT_MAX_CUBE_LUT_SIZE } from "./colorLuts"; import type { HfColorCurvePoint, HfHueCurvePoint } from "./colorGradingCurves"; import { COLOR_GRADING_ADJUST_KEYS, + COLOR_GRADING_ADVANCED_LIMITS, COLOR_GRADING_COLOR_SPACE, COLOR_GRADING_CONTRACT_VERSION, COLOR_GRADING_CURVE_KEYS, @@ -16,11 +17,7 @@ import { } from "@hyperframes/parsers/color-grading-contract"; export type { HfColorCurvePoint, HfHueCurvePoint } from "./colorGradingCurves"; -export { - compileHfColorCurve, - compileHfHueCurve, - HF_COLOR_CURVE_MAX_POINTS, -} from "./colorGradingCurves"; +export { HF_COLOR_CURVE_MAX_POINTS } from "./colorGradingCurves"; export const HF_COLOR_GRADING_ATTR = "data-color-grading"; @@ -650,8 +647,8 @@ const TONAL_WHEEL_DEFAULT: Required = { level: 0, }; const WHEEL_LIMITS = { - amount: { min: 0, max: 1 }, - level: { min: -1, max: 1 }, + amount: COLOR_GRADING_ADVANCED_LIMITS.unit, + level: COLOR_GRADING_ADVANCED_LIMITS.signedUnit, }; const RGB_CURVE_IDENTITY: readonly HfColorCurvePoint[] = [ [0, 0], @@ -659,9 +656,9 @@ const RGB_CURVE_IDENTITY: readonly HfColorCurvePoint[] = [ ]; const HUE_CURVE_IDENTITY: readonly HfHueCurvePoint[] = []; const HUE_CURVE_LIMITS: Record = { - hueVsHue: { min: -180, max: 180 }, - hueVsSaturation: { min: -1, max: 1 }, - hueVsLuma: { min: -1, max: 1 }, + hueVsHue: COLOR_GRADING_ADVANCED_LIMITS.secondaryHueShift, + hueVsSaturation: COLOR_GRADING_ADVANCED_LIMITS.signedUnit, + hueVsLuma: COLOR_GRADING_ADVANCED_LIMITS.signedUnit, }; const SECONDARY_HUE_DEFAULT: Required = { center: 0, @@ -683,7 +680,7 @@ const DETAIL_LIMITS: Record > = { @@ -979,7 +976,13 @@ export function getHfColorGradingCapabilities() { wheels: { zones: HF_COLOR_GRADING_WHEEL_KEYS, controls: { - hue: { identity: 0, unit: "degrees", wrap: true, min: 0, maxExclusive: 360 }, + hue: { + identity: 0, + unit: "degrees", + wrap: true, + min: COLOR_GRADING_ADVANCED_LIMITS.hueDegrees.min, + maxExclusive: COLOR_GRADING_ADVANCED_LIMITS.hueDegrees.max, + }, amount: { identity: 0, ...WHEEL_LIMITS.amount }, level: { identity: 0, ...WHEEL_LIMITS.level }, }, @@ -1005,29 +1008,34 @@ export function getHfColorGradingCapabilities() { secondaries: { max: COLOR_GRADING_MAX_SECONDARIES, hue: { - center: { min: 0, maxExclusive: 360, unit: "degrees", wrap: true }, - range: { min: 0, max: 180, unit: "degrees" }, - softness: { min: 0, max: 180, unit: "degrees" }, - rangePlusSoftnessMax: 180, + center: { + min: COLOR_GRADING_ADVANCED_LIMITS.hueDegrees.min, + maxExclusive: COLOR_GRADING_ADVANCED_LIMITS.hueDegrees.max, + unit: "degrees", + wrap: true, + }, + range: { ...COLOR_GRADING_ADVANCED_LIMITS.secondaryHueRange, unit: "degrees" }, + softness: { ...COLOR_GRADING_ADVANCED_LIMITS.secondaryHueSoftness, unit: "degrees" }, + rangePlusSoftnessMax: COLOR_GRADING_ADVANCED_LIMITS.secondaryHueCombinedMax, }, saturation: { min: { ...UNIT_LIMIT }, max: { ...UNIT_LIMIT }, relation: "min < max", - softness: { min: 0, max: 0.5 }, + softness: { ...COLOR_GRADING_ADVANCED_LIMITS.secondarySoftRangeSoftness }, }, luma: { min: { ...UNIT_LIMIT }, max: { ...UNIT_LIMIT }, relation: "min < max", - softness: { min: 0, max: 0.5 }, + softness: { ...COLOR_GRADING_ADVANCED_LIMITS.secondarySoftRangeSoftness }, }, correction: { - hueShift: { min: -180, max: 180, unit: "degrees" }, - saturation: { min: -1, max: 1 }, - luma: { min: -1, max: 1 }, - temperature: { min: -1, max: 1 }, - tint: { min: -1, max: 1 }, + hueShift: { ...COLOR_GRADING_ADVANCED_LIMITS.secondaryHueShift, unit: "degrees" }, + saturation: { ...COLOR_GRADING_ADVANCED_LIMITS.signedUnit }, + luma: { ...COLOR_GRADING_ADVANCED_LIMITS.signedUnit }, + temperature: { ...COLOR_GRADING_ADVANCED_LIMITS.signedUnit }, + tint: { ...COLOR_GRADING_ADVANCED_LIMITS.signedUnit }, }, }, effectFamilies: EFFECT_FAMILIES, @@ -1136,6 +1144,11 @@ function hasDuplicateCurveInputs(points: readonly HfColorCurvePoint[]): boolean return points.some((point, index) => index > 0 && point[0] === points[index - 1]?.[0]); } +function completeCurveEndpoints(points: HfColorCurvePoint[]): void { + if ((points[0]?.[0] ?? 0) > 0) points.unshift([0, 0]); + if ((points.at(-1)?.[0] ?? 1) < 1) points.push([1, 1]); +} + function normalizeCurve(value: unknown): readonly HfColorCurvePoint[] { if (!Array.isArray(value) || value.length < 2) return RGB_CURVE_IDENTITY; const points: HfColorCurvePoint[] = []; @@ -1146,8 +1159,7 @@ function normalizeCurve(value: unknown): readonly HfColorCurvePoint[] { } points.sort((a, b) => a[0] - b[0]); if (hasDuplicateCurveInputs(points)) return RGB_CURVE_IDENTITY; - if (points[0]![0] > 0) points.unshift([0, 0]); - if (points[points.length - 1]![0] < 1) points.push([1, 1]); + completeCurveEndpoints(points); if (points.length > COLOR_GRADING_MAX_CURVE_POINTS) return RGB_CURVE_IDENTITY; return points; } @@ -1219,25 +1231,25 @@ function normalizeSoftRange(value: unknown): Required { return { min: Math.min(first, second), max: Math.max(first, second), - softness: readLimitedValue(range.softness ?? SECONDARY_SOFT_RANGE_DEFAULT.softness, { - min: 0, - max: 0.5, - }), + softness: readLimitedValue( + range.softness ?? SECONDARY_SOFT_RANGE_DEFAULT.softness, + COLOR_GRADING_ADVANCED_LIMITS.secondarySoftRangeSoftness, + ), }; } function normalizeSecondaryHue(value: unknown): Required { const hue = isRecord(value) ? value : {}; - const range = readLimitedValue(hue.range ?? SECONDARY_HUE_DEFAULT.range, { - min: 0, - max: 180, - }); + const range = readLimitedValue( + hue.range ?? SECONDARY_HUE_DEFAULT.range, + COLOR_GRADING_ADVANCED_LIMITS.secondaryHueRange, + ); return { center: wrapDegrees(hue.center, SECONDARY_HUE_DEFAULT.center), range, softness: readLimitedValue(hue.softness ?? SECONDARY_HUE_DEFAULT.softness, { - min: 0, - max: 180 - range, + min: COLOR_GRADING_ADVANCED_LIMITS.secondaryHueSoftness.min, + max: COLOR_GRADING_ADVANCED_LIMITS.secondaryHueCombinedMax - range, }), }; } @@ -1246,11 +1258,14 @@ function normalizeSecondaryCorrection( value: Record, ): Required { return { - hueShift: readLimitedValue(value.hueShift ?? 0, { min: -180, max: 180 }), - saturation: readLimitedValue(value.saturation ?? 0, { min: -1, max: 1 }), - luma: readLimitedValue(value.luma ?? 0, { min: -1, max: 1 }), - temperature: readLimitedValue(value.temperature ?? 0, { min: -1, max: 1 }), - tint: readLimitedValue(value.tint ?? 0, { min: -1, max: 1 }), + hueShift: readLimitedValue( + value.hueShift ?? 0, + COLOR_GRADING_ADVANCED_LIMITS.secondaryHueShift, + ), + saturation: readLimitedValue(value.saturation ?? 0, COLOR_GRADING_ADVANCED_LIMITS.signedUnit), + luma: readLimitedValue(value.luma ?? 0, COLOR_GRADING_ADVANCED_LIMITS.signedUnit), + temperature: readLimitedValue(value.temperature ?? 0, COLOR_GRADING_ADVANCED_LIMITS.signedUnit), + tint: readLimitedValue(value.tint ?? 0, COLOR_GRADING_ADVANCED_LIMITS.signedUnit), }; } diff --git a/packages/core/src/colorGradingCurves.ts b/packages/core/src/colorGradingCurves.ts index 580918e36..cbf322fb9 100644 --- a/packages/core/src/colorGradingCurves.ts +++ b/packages/core/src/colorGradingCurves.ts @@ -25,29 +25,43 @@ function endpointSlope( return slope; } +function valueAt(values: readonly T[], index: number): T { + const value = values[index]; + if (value === undefined) throw new RangeError("Color curve points must be contiguous"); + return value; +} + function pointSlopes(points: readonly HfColorCurvePoint[]): number[] { - const spans = points.slice(1).map(([x], index) => x - points[index]![0]); - const slopes = spans.map((span, index) => (points[index + 1]![1] - points[index]![1]) / span); - if (points.length === 2) return [slopes[0]!, slopes[0]!]; + const spans: number[] = []; + const slopes: number[] = []; + let previous = valueAt(points, 0); + for (const point of points.slice(1)) { + const span = point[0] - previous[0]; + spans.push(span); + slopes.push((point[1] - previous[1]) / span); + previous = point; + } + const firstSlope = valueAt(slopes, 0); + if (points.length === 2) return [firstSlope, firstSlope]; const result = new Array(points.length); - result[0] = endpointSlope(spans[0]!, spans[1]!, slopes[0]!, slopes[1]!); + result[0] = endpointSlope(valueAt(spans, 0), valueAt(spans, 1), firstSlope, valueAt(slopes, 1)); result[result.length - 1] = endpointSlope( - spans[spans.length - 1]!, - spans[spans.length - 2]!, - slopes[slopes.length - 1]!, - slopes[slopes.length - 2]!, + valueAt(spans, spans.length - 1), + valueAt(spans, spans.length - 2), + valueAt(slopes, slopes.length - 1), + valueAt(slopes, slopes.length - 2), ); for (let index = 1; index < points.length - 1; index += 1) { - const before = slopes[index - 1]!; - const after = slopes[index]!; + const before = valueAt(slopes, index - 1); + const after = valueAt(slopes, index); if (before === 0 || after === 0 || Math.sign(before) !== Math.sign(after)) { result[index] = 0; continue; } - const beforeSpan = spans[index - 1]!; - const afterSpan = spans[index]!; + const beforeSpan = valueAt(spans, index - 1); + const afterSpan = valueAt(spans, index); const beforeWeight = 2 * afterSpan + beforeSpan; const afterWeight = afterSpan + 2 * beforeSpan; result[index] = (beforeWeight + afterWeight) / (beforeWeight / before + afterWeight / after); @@ -111,7 +125,6 @@ function compileCurveSamples( outputMin: number, outputMax: number, ): Float32Array { - validateCurvePoints(points, size); validateOutputRange(points, outputMin, outputMax); const tangents = pointSlopes(points); @@ -119,17 +132,17 @@ function compileCurveSamples( let segment = 0; for (let index = 0; index < size; index += 1) { const input = inputAt(index); - while (segment < points.length - 2 && input > points[segment + 1]![0]) { + while (segment < points.length - 2 && input > valueAt(points, segment + 1)[0]) { segment += 1; } - const start = points[segment]!; - const end = points[segment + 1]!; + const start = valueAt(points, segment); + const end = valueAt(points, segment + 1); const output = interpolateCurveSegment( input, start, end, - tangents[segment]!, - tangents[segment + 1]!, + valueAt(tangents, segment), + valueAt(tangents, segment + 1), ); samples[index] = Math.min(outputMax, Math.max(outputMin, output)); } @@ -141,7 +154,7 @@ export function compileHfColorCurve( points: readonly HfColorCurvePoint[], size = HF_COLOR_CURVE_LUT_SIZE, ): Float32Array { - if (points.length < 2) throw new RangeError("A color curve requires at least two points"); + validateCurvePoints(points, size); if (points.length > HF_COLOR_CURVE_MAX_POINTS) { throw new RangeError(`A color curve supports at most ${HF_COLOR_CURVE_MAX_POINTS} points`); } @@ -177,11 +190,7 @@ export function compileHfHueCurve( } const before = sorted.slice(-2).map(([hue, delta]) => [hue - 360, delta] as const); const after = sorted.slice(0, 2).map(([hue, delta]) => [hue + 360, delta] as const); - return compileCurveSamples( - [...before, ...sorted, ...after], - size, - (index) => (index / size) * 360, - outputMin, - outputMax, - ); + const periodic = [...before, ...sorted, ...after]; + validateCurvePoints(periodic, size); + return compileCurveSamples(periodic, size, (index) => (index / size) * 360, outputMin, outputMax); } diff --git a/packages/core/src/colorLuts.ts b/packages/core/src/colorLuts.ts index 36cf554e1..6f5bd4cf9 100644 --- a/packages/core/src/colorLuts.ts +++ b/packages/core/src/colorLuts.ts @@ -199,7 +199,7 @@ function clampUnit(value: number): number { return Math.min(1, Math.max(0, value)); } -function toByte(value: number): number { +export function unitFloatToByte(value: number): number { return Math.round(clampUnit(value) * 255); } @@ -214,9 +214,9 @@ export function packCubeLutToRgba8(lut: CubeLut3D): PackedCubeLut2D { for (let r = 0; r < size; r++) { const lutIndex = ((b * size + g) * size + r) * 3; const pixelIndex = (g * width + b * size + r) * 4; - packed[pixelIndex] = toByte(lut.data[lutIndex] ?? 0); - packed[pixelIndex + 1] = toByte(lut.data[lutIndex + 1] ?? 0); - packed[pixelIndex + 2] = toByte(lut.data[lutIndex + 2] ?? 0); + packed[pixelIndex] = unitFloatToByte(lut.data[lutIndex] ?? 0); + packed[pixelIndex + 1] = unitFloatToByte(lut.data[lutIndex + 1] ?? 0); + packed[pixelIndex + 2] = unitFloatToByte(lut.data[lutIndex + 2] ?? 0); packed[pixelIndex + 3] = 255; } } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 25ae577db..b23ea509f 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -184,8 +184,6 @@ export { HF_COLOR_GRADING_PRESETS, HF_COLOR_GRADING_TOP_LEVEL_KEYS, HF_COLOR_GRADING_WHEEL_KEYS, - compileHfColorCurve, - compileHfHueCurve, getHfColorGradingCapabilities, hasHfColorGradingAuthoredValues, isHfColorGradingActive, diff --git a/packages/core/src/runtime/colorGrading.ts b/packages/core/src/runtime/colorGrading.ts index dd92ea0fe..318f1a18f 100644 --- a/packages/core/src/runtime/colorGrading.ts +++ b/packages/core/src/runtime/colorGrading.ts @@ -34,6 +34,7 @@ import { DEFAULT_MAX_CUBE_LUT_SIZE, packCubeLutToRgba8, parseCubeLut, + unitFloatToByte, type CubeLut3D, type CubeLutVec3, } from "../colorLuts"; @@ -337,7 +338,12 @@ const DEFAULT_COMPARE: RuntimeColorGradingCompareState = { const DEFAULT_EFFECT_PALETTE = ["#000000", "#ffffff"] as const; const DEFAULT_ART_PALETTE = ["#1a1a1a", "#f5f5dc"] as const; const ADVANCED_TEXTURE_HEIGHT = 3; +const ADVANCED_CONFIG_ROW = ADVANCED_TEXTURE_HEIGHT - 1; const ADVANCED_SECONDARY_TEXELS = 5; +const ADVANCED_TEXTURE_WIDTH_GLSL = `${HF_COLOR_CURVE_LUT_SIZE}.0`; +const ADVANCED_TEXTURE_MAX_X_GLSL = `${HF_COLOR_CURVE_LUT_SIZE - 1}.0`; +const ADVANCED_TEXTURE_HEIGHT_GLSL = `${ADVANCED_TEXTURE_HEIGHT}.0`; +const ADVANCED_CONFIG_Y_GLSL = `${(ADVANCED_CONFIG_ROW + 0.5) / ADVANCED_TEXTURE_HEIGHT}`; function readColorGradingAttribute(element: Element): ResolvedHfColorGrading | null { const raw = element.getAttribute(HF_COLOR_GRADING_ATTR); @@ -980,16 +986,16 @@ const FRAGMENT_SHADER = [ " return color.z * mix(vec3(1.0), clamp(bands - 1.0, 0.0, 1.0), color.y);", "}", "vec4 sampleAdvancedCurve(float coordinate, float row){", - " float position = clamp(coordinate, 0.0, 1.0) * 1023.0;", + ` float position = clamp(coordinate, 0.0, 1.0) * ${ADVANCED_TEXTURE_MAX_X_GLSL};`, " float lower = floor(position);", - " float upper = min(lower + 1.0, 1023.0);", - " float y = (row + 0.5) / 3.0;", - " vec4 before = texture2D(u_advanced, vec2((lower + 0.5) / 1024.0, y));", - " vec4 after = texture2D(u_advanced, vec2((upper + 0.5) / 1024.0, y));", + ` float upper = min(lower + 1.0, ${ADVANCED_TEXTURE_MAX_X_GLSL});`, + ` float y = (row + 0.5) / ${ADVANCED_TEXTURE_HEIGHT_GLSL};`, + ` vec4 before = texture2D(u_advanced, vec2((lower + 0.5) / ${ADVANCED_TEXTURE_WIDTH_GLSL}, y));`, + ` vec4 after = texture2D(u_advanced, vec2((upper + 0.5) / ${ADVANCED_TEXTURE_WIDTH_GLSL}, y));`, " return mix(before, after, position - lower);", "}", "vec4 advancedConfig(float index){", - " return texture2D(u_advanced, vec2((index + 0.5) / 1024.0, 0.8333333));", + ` return texture2D(u_advanced, vec2((index + 0.5) / ${ADVANCED_TEXTURE_WIDTH_GLSL}, ${ADVANCED_CONFIG_Y_GLSL}));`, "}", "vec3 wheelDirection(float hue){", " vec3 direction = hsvToRgb(vec3(fract(hue), 1.0, 1.0));", @@ -2741,12 +2747,38 @@ function buildAdvancedTextureData( ): Float32Array { const data = new Float32Array(HF_COLOR_CURVE_LUT_SIZE * ADVANCED_TEXTURE_HEIGHT * 4); writeAdvancedCurveRows(data, curves, hueCurves); - secondaries - .slice(0, 4) - .forEach((secondary, index) => writeAdvancedSecondary(data, secondary, index)); + let index = 0; + for (const secondary of secondaries) { + if (!secondary.enabled) continue; + writeAdvancedSecondary(data, secondary, index); + index += 1; + } return data; } +const ADVANCED_SIGNATURES = new WeakMap< + NormalizedHfColorGradingCurves, + { + hueCurves: NormalizedHfColorGradingHueCurves; + secondaries: readonly NormalizedHfColorGradingSecondary[]; + signature: string; + } +>(); + +function advancedTextureSignature( + curves: NormalizedHfColorGradingCurves, + hueCurves: NormalizedHfColorGradingHueCurves, + secondaries: readonly NormalizedHfColorGradingSecondary[], +): string { + const cached = ADVANCED_SIGNATURES.get(curves); + if (cached?.hueCurves === hueCurves && cached.secondaries === secondaries) { + return cached.signature; + } + const signature = JSON.stringify([curves, hueCurves, secondaries]); + ADVANCED_SIGNATURES.set(curves, { hueCurves, secondaries, signature }); + return signature; +} + function ensureAdvancedTexture( gl: WebGLRenderingContext, program: ProgramInfo, @@ -2754,12 +2786,12 @@ function ensureAdvancedTexture( secondaries: readonly NormalizedHfColorGradingSecondary[], ): void { const { curves, hueCurves } = grading; - const signature = JSON.stringify([curves, hueCurves, secondaries]); + const signature = advancedTextureSignature(curves, hueCurves, secondaries); if (program.advancedSignature === signature) return; const pixels = Uint8Array.from( buildAdvancedTextureData(curves, hueCurves, secondaries), - (value) => Math.round(Math.min(1, Math.max(0, value)) * 255), + unitFloatToByte, ); gl.activeTexture(gl.TEXTURE5); gl.bindTexture(gl.TEXTURE_2D, program.advancedTexture); @@ -2830,14 +2862,13 @@ function applyUniforms( ); gl.uniform1f(program.lutIntensity, grading.lut?.intensity ?? 0); const { curves, hueCurves, secondaries } = grading; - const enabledSecondaries = secondaries.filter((secondary) => secondary.enabled); const rgbCurvesEnabled = hasHfColorGradingRgbCurveValues(curves); const hueCurvesEnabled = hasHfColorGradingHueCurveValues(hueCurves); const secondaryCount = hasHfColorGradingSecondaryValues(secondaries) - ? enabledSecondaries.length + ? secondaries.reduce((count, secondary) => count + Number(secondary.enabled), 0) : 0; if (rgbCurvesEnabled || hueCurvesEnabled || secondaryCount > 0) { - ensureAdvancedTexture(gl, program, grading, enabledSecondaries); + ensureAdvancedTexture(gl, program, grading, secondaries); } setWheelUniform(gl, program.shadowWheel, grading.wheels.shadows); setWheelUniform(gl, program.midtoneWheel, grading.wheels.midtones); diff --git a/packages/parsers/src/colorGradingContract.test.ts b/packages/parsers/src/colorGradingContract.test.ts index c0eebe013..7625ea92b 100644 --- a/packages/parsers/src/colorGradingContract.test.ts +++ b/packages/parsers/src/colorGradingContract.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { COLOR_GRADING_ADJUST_KEYS, + COLOR_GRADING_ADVANCED_LIMITS, COLOR_GRADING_DETAIL_KEYS, COLOR_GRADING_EFFECT_KEYS, COLOR_GRADING_HUE_CURVE_KEYS, @@ -20,6 +21,12 @@ describe("color grading contract", () => { expect(COLOR_GRADING_WHEEL_KEYS).toEqual(["shadows", "midtones", "highlights"]); expect(COLOR_GRADING_HUE_CURVE_KEYS).toContain("hueVsSaturation"); expect(COLOR_GRADING_LUT_KEYS).toEqual(["src", "intensity"]); + expect(COLOR_GRADING_ADVANCED_LIMITS).toMatchObject({ + hueDegrees: { min: 0, max: 360, inclusiveMax: false }, + secondaryHueRange: { min: 0, max: 180 }, + secondarySoftRangeSoftness: { min: 0, max: 0.5 }, + secondaryHueShift: { min: -180, max: 180 }, + }); }); it("accepts the complete current contract and variable references", () => { diff --git a/packages/parsers/src/colorGradingContract.ts b/packages/parsers/src/colorGradingContract.ts index ad3e8543e..a5a147105 100644 --- a/packages/parsers/src/colorGradingContract.ts +++ b/packages/parsers/src/colorGradingContract.ts @@ -2,6 +2,16 @@ export const COLOR_GRADING_CONTRACT_VERSION = 2; export const COLOR_GRADING_COLOR_SPACE = "rec709"; export const COLOR_GRADING_MAX_CURVE_POINTS = 16; export const COLOR_GRADING_MAX_SECONDARIES = 4; +export const COLOR_GRADING_ADVANCED_LIMITS = { + hueDegrees: { min: 0, max: 360, inclusiveMax: false }, + unit: { min: 0, max: 1 }, + signedUnit: { min: -1, max: 1 }, + secondaryHueRange: { min: 0, max: 180 }, + secondaryHueSoftness: { min: 0, max: 180 }, + secondaryHueCombinedMax: 180, + secondarySoftRangeSoftness: { min: 0, max: 0.5 }, + secondaryHueShift: { min: -180, max: 180 }, +} as const; export const COLOR_GRADING_TOP_LEVEL_KEYS = [ "enabled", @@ -130,8 +140,8 @@ export const COLOR_GRADING_LUT_KEYS = ["src", "intensity"] as const; type NumericLimit = Readonly<{ min: number; max: number }>; -const UNIT_LIMIT: NumericLimit = { min: 0, max: 1 }; -const SIGNED_UNIT_LIMIT: NumericLimit = { min: -1, max: 1 }; +const UNIT_LIMIT: NumericLimit = COLOR_GRADING_ADVANCED_LIMITS.unit; +const SIGNED_UNIT_LIMIT: NumericLimit = COLOR_GRADING_ADVANCED_LIMITS.signedUnit; const EFFECT_LIMIT_OVERRIDES: Readonly> = { asciiStyle: { min: 0, max: 7 }, bloom: { min: 0, max: 3 }, @@ -449,7 +459,14 @@ function validateWheels(value: unknown, issues: ColorGradingContractIssue[]): vo const path = `wheels.${wheel}`; const controls = validateObject(wheels[wheel], path, COLOR_GRADING_WHEEL_CONTROL_KEYS, issues); if (!controls) continue; - validateNumericField(controls, "hue", path, { min: 0, max: 360 }, issues, false); + validateNumericField( + controls, + "hue", + path, + COLOR_GRADING_ADVANCED_LIMITS.hueDegrees, + issues, + COLOR_GRADING_ADVANCED_LIMITS.hueDegrees.inclusiveMax, + ); validateNumericField(controls, "amount", path, UNIT_LIMIT, issues); validateNumericField(controls, "level", path, SIGNED_UNIT_LIMIT, issues); } @@ -489,7 +506,13 @@ function validateSoftRange( if (!range) return; validateNumericField(range, "min", path, UNIT_LIMIT, issues); validateNumericField(range, "max", path, UNIT_LIMIT, issues); - validateNumericField(range, "softness", path, { min: 0, max: 0.5 }, issues); + validateNumericField( + range, + "softness", + path, + COLOR_GRADING_ADVANCED_LIMITS.secondarySoftRangeSoftness, + issues, + ); if (typeof range.min === "number" && typeof range.max === "number" && range.min >= range.max) { issues.push({ path, message: "min must be smaller than max" }); } @@ -502,15 +525,31 @@ function validateSecondaryHue( ): void { const hue = validateObject(value, path, COLOR_GRADING_HUE_RANGE_KEYS, issues); if (!hue) return; - validateNumericField(hue, "center", path, { min: 0, max: 360 }, issues, false); - validateNumericField(hue, "range", path, { min: 0, max: 180 }, issues); - validateNumericField(hue, "softness", path, { min: 0, max: 180 }, issues); + validateNumericField( + hue, + "center", + path, + COLOR_GRADING_ADVANCED_LIMITS.hueDegrees, + issues, + COLOR_GRADING_ADVANCED_LIMITS.hueDegrees.inclusiveMax, + ); + validateNumericField(hue, "range", path, COLOR_GRADING_ADVANCED_LIMITS.secondaryHueRange, issues); + validateNumericField( + hue, + "softness", + path, + COLOR_GRADING_ADVANCED_LIMITS.secondaryHueSoftness, + issues, + ); if ( typeof hue.range === "number" && typeof hue.softness === "number" && - hue.range + hue.softness > 180 + hue.range + hue.softness > COLOR_GRADING_ADVANCED_LIMITS.secondaryHueCombinedMax ) { - issues.push({ path, message: "range plus softness must not exceed 180 degrees" }); + issues.push({ + path, + message: `range plus softness must not exceed ${COLOR_GRADING_ADVANCED_LIMITS.secondaryHueCombinedMax} degrees`, + }); } } @@ -533,7 +572,13 @@ function validateSecondaryCorrection( ): void { const correction = validateObject(value, path, COLOR_GRADING_SECONDARY_CORRECTION_KEYS, issues); if (!correction) return; - validateNumericField(correction, "hueShift", path, { min: -180, max: 180 }, issues); + validateNumericField( + correction, + "hueShift", + path, + COLOR_GRADING_ADVANCED_LIMITS.secondaryHueShift, + issues, + ); for (const key of ["saturation", "luma", "temperature", "tint"] as const) { validateNumericField(correction, key, path, SIGNED_UNIT_LIMIT, issues); }