Skip to content

Commit 8dae5ef

Browse files
fix(core): make grading writes previewable and honor computed properties (#5157)
* fix(core): write only authored hue curves, so a single hue curve passes lint * fix(cli): apply over a grading saved with empty hue curves instead of refusing it * fix(core): read grading properties from computed style, and stop advising an inline start * fix(cli): preserve validation during legacy hue repair * fix(cli): preview grading writes without apply
1 parent 202d5b5 commit 8dae5ef

10 files changed

Lines changed: 203 additions & 22 deletions

File tree

‎docs/packages/cli.mdx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ npx hyperframes media-treatment --selector '#hero' --analyze --json
435435
npx hyperframes media-treatment --selector '#hero' \
436436
--grading '{"preset":"skin-soft","intensity":0.6}' --apply
437437
npx hyperframes media-treatment --file compositions/scene.html --selector 'video' \
438-
--grading '{"preset":"warm-daylight"}' --apply --dry-run --json
438+
--grading '{"preset":"warm-daylight"}' --dry-run --json
439439
npx hyperframes media-treatment --selector '#hero' --clear
440440
```
441441

@@ -455,6 +455,8 @@ npx hyperframes media-treatment --selector '#hero' --clear
455455
| `--dry-run` | Validate and report without writing |
456456
| `--json` | Agent-readable JSON |
457457

458+
`--dry-run` accepts `--grading` without `--apply` and writes nothing. JSON includes `attribute`, the exact normalized `value` that apply would write (or `null` to remove it), and `lint`, the color-grading lint verdict and findings for the proposed HTML.
459+
458460
The command writes `data-color-grading` — the same contract Studio, preview, and
459461
render all read, so the two can never disagree. It does not recognise subjects
460462
or isolate part of an image; the effect covers the whole media layer.

‎docs/reference/color-grading.mdx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,10 @@ can tween directly:
5050
--hf-color-grading-lut-intensity
5151
```
5252

53-
Start the value at identity in both the payload and inline style, then animate
54-
it on the registered timeline:
53+
Once set, a property overrides the payload's value for that control, whether it
54+
comes from the element's inline style, a stylesheet or a parent. Set it only on
55+
media you animate, starting at the value the tween begins from; leave it unset
56+
for a static grade. Then animate it on the registered timeline:
5557

5658
```js
5759
tl.to(

‎packages/cli/src/commands/media-treatment.test.ts‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "nod
22
import { tmpdir } from "node:os";
33
import { basename, join } from "node:path";
44
import { runCommand } from "citty";
5+
import { parseHTML } from "linkedom";
56
import { describe, expect, it, vi } from "vitest";
67
import {
78
HF_COLOR_GRADING_ACTIVE_EFFECT_KEYS,
@@ -116,6 +117,16 @@ describe("applyMediaTreatmentToHtml", () => {
116117
});
117118
});
118119

120+
it("warns that an animated property overrides the payload, instead of asking for an inline start", () => {
121+
const detail = getMediaTreatmentCapabilityDetail("blur");
122+
expect(detail).toMatchObject({
123+
animation: {
124+
rules: expect.arrayContaining([expect.stringContaining("overrides the payload's value")]),
125+
},
126+
});
127+
expect(JSON.stringify(detail)).not.toContain("Author the initial value inline");
128+
});
129+
119130
it("rejects unknown capability lookups", () => {
120131
expect(() => getMediaTreatmentCapabilityDetail("make-it-cinematic")).toThrow(
121132
/Unknown media-treatment capability/,
@@ -294,6 +305,51 @@ describe("applyMediaTreatmentToHtml", () => {
294305
expect(repeated.after).toEqual(repeated.before);
295306
});
296307

308+
it("applies over a grading an earlier version wrote with empty hue curves, and writes it valid", () => {
309+
const broken = JSON.stringify({
310+
hueCurves: {
311+
hueVsHue: [
312+
[0, 0],
313+
[120, 20],
314+
[240, 0],
315+
],
316+
hueVsSaturation: [],
317+
hueVsLuma: [],
318+
},
319+
});
320+
const html = VIDEO.replace('id="hero"', `id="hero" data-color-grading='${broken}'`);
321+
322+
const patched = applyMediaTreatmentToHtml(html, {
323+
selector: "#hero",
324+
grading: { adjust: { exposure: 0.1 } },
325+
});
326+
327+
expect(patched.after).toMatchObject({
328+
adjust: { exposure: 0.1 },
329+
hueCurves: {
330+
hueVsHue: [
331+
[0, 0],
332+
[120, 20],
333+
[240, 0],
334+
],
335+
},
336+
});
337+
expect(patched.after).not.toHaveProperty("hueCurves.hueVsSaturation");
338+
});
339+
340+
it("rejects unknown empty hue curves instead of healing them", () => {
341+
const html = VIDEO.replace(
342+
'id="hero"',
343+
`id="hero" data-color-grading='{"hueCurves":{"bogus":[]}}'`,
344+
);
345+
expect(() =>
346+
applyMediaTreatmentToHtml(html, {
347+
selector: "#hero",
348+
grading: { adjust: { exposure: 0.1 } },
349+
}),
350+
).toThrow(/hueCurves.*unsupported key.*bogus/);
351+
});
352+
297353
it("preserves unresolved variable references for runtime resolution", () => {
298354
const wholeGrade = applyMediaTreatmentToHtml(VIDEO, {
299355
selector: "#hero",
@@ -368,6 +424,38 @@ describe("applyMediaTreatmentToHtml", () => {
368424
).toThrow(/effects.*dithering/i);
369425
});
370426

427+
it("previews the exact grading attribute and lint verdict without --apply or writes", async () => {
428+
const project = mkdtempSync(join(tmpdir(), "hf-media-dry-run-"));
429+
const file = join(project, "index.html");
430+
const grading = '{"preset":"warm-daylight","intensity":0.8}';
431+
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
432+
writeFileSync(file, VIDEO);
433+
const args = ["--project", project, "--selector", "#hero", "--grading", grading, "--json"];
434+
try {
435+
await runCommand(mediaTreatmentCommand, { rawArgs: [...args, "--dry-run"] });
436+
const preview = JSON.parse(String(log.mock.calls.at(-1)?.[0]));
437+
expect(preview).toMatchObject({
438+
ok: true,
439+
dryRun: true,
440+
changed: true,
441+
attribute: "data-color-grading",
442+
lint: { ok: true, findings: [] },
443+
});
444+
expect(readFileSync(file, "utf8")).toBe(VIDEO);
445+
await runCommand(mediaTreatmentCommand, { rawArgs: [...args, "--apply"] });
446+
const applied = JSON.parse(String(log.mock.calls.at(-1)?.[0]));
447+
const written = parseHTML(readFileSync(file, "utf8"))
448+
.document.querySelector("#hero")
449+
?.getAttribute("data-color-grading");
450+
expect(preview.value).toBe(written);
451+
expect(preview.value).toBe(applied.value);
452+
expect(preview.lint).toEqual(applied.lint);
453+
} finally {
454+
log.mockRestore();
455+
rmSync(project, { recursive: true, force: true });
456+
}
457+
});
458+
371459
it("requires --apply for --grading while keeping --clear explicit", async () => {
372460
const project = mkdtempSync(join(tmpdir(), "hf-media-treatment-"));
373461
const file = join(project, "index.html");

‎packages/cli/src/commands/media-treatment.ts‎

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { existsSync, readFileSync, writeFileSync } from "node:fs";
22
import { join, relative, resolve } from "node:path";
33
import {
44
HF_COLOR_GRADING_ATTR,
5+
HF_COLOR_GRADING_HUE_CURVE_KEYS,
56
getHfColorGradingCapabilities,
67
hasHfColorGradingAuthoredValues,
78
isPathInside,
@@ -20,6 +21,7 @@ import {
2021
import { rewriteAssetPath } from "@hyperframes/parsers/asset-paths";
2122
import { patchElementInHtml } from "@hyperframes/studio-server/source-mutation";
2223
import { defineCommand } from "citty";
24+
import { lintHyperframeHtml } from "@hyperframes/lint";
2325
import { parseHTML } from "linkedom";
2426
import type { Example } from "./_examples.js";
2527
import { c } from "../ui/colors.js";
@@ -91,7 +93,7 @@ export function getMediaTreatmentCapabilityDetail(id: string): unknown {
9193
initial: `style="${property.name}: <start>"`,
9294
tween: `timeline.to("<selector>", { "${property.name}": <end>, duration: <seconds> })`,
9395
rules: [
94-
"Author the initial value inline on the media element.",
96+
"Once set (inline, in a stylesheet or on a parent), the property overrides the payload's value for this control: set it only on media you animate, starting at the tween's first value.",
9597
"Use finite keyframes on a paused timeline registered in window.__timelines.",
9698
"Do not use a frame-zero set, timers, random values, or onUpdate callbacks.",
9799
],
@@ -317,7 +319,7 @@ export const examples: Example[] = [
317319
],
318320
[
319321
"Preview the exact mutation without writing",
320-
`hyperframes media-treatment --file compositions/scene.html --selector 'video' --grading '{"preset":"warm-daylight"}' --apply --dry-run --json`,
322+
`hyperframes media-treatment --file compositions/scene.html --selector 'video' --grading '{"preset":"warm-daylight"}' --dry-run --json`,
321323
],
322324
[
323325
"Measure one local media source before choosing a correction",
@@ -385,13 +387,28 @@ function mergeGradingPatch(current: unknown, patch: unknown): unknown {
385387
return merged;
386388
}
387389

390+
function withoutEmptyHueCurves(grading: unknown): unknown {
391+
if (!isRecord(grading) || !isRecord(grading.hueCurves)) return grading;
392+
const hueCurves = Object.fromEntries(
393+
Object.entries(grading.hueCurves).filter(
394+
([key, curve]) =>
395+
!HF_COLOR_GRADING_HUE_CURVE_KEYS.some((knownKey) => knownKey === key) ||
396+
!Array.isArray(curve) ||
397+
curve.length > 0,
398+
),
399+
);
400+
return { ...grading, hueCurves };
401+
}
402+
388403
function serializeGradingPatch(before: unknown, patch: unknown): string | null {
389404
assertKnownGradingShape(patch);
390405
if (isColorGradingVariableRef(before) && isRecord(patch)) {
391406
throw new Error("Cannot merge a grading patch into an unresolved whole-grade variable");
392407
}
393408
const current =
394-
typeof before === "string" && !isColorGradingVariableRef(before) ? { preset: before } : before;
409+
typeof before === "string" && !isColorGradingVariableRef(before)
410+
? { preset: before }
411+
: withoutEmptyHueCurves(before);
395412
const grading = mergeGradingPatch(current, patch);
396413
assertKnownGradingShape(grading);
397414
if (containsColorGradingVariableRef(grading)) {
@@ -508,15 +525,20 @@ export function resolveMediaTreatmentSource(
508525
return asset.resolved;
509526
}
510527

511-
function parseGrading(raw: string | undefined, apply: boolean, clear: boolean): unknown {
528+
function parseGrading(
529+
raw: string | undefined,
530+
apply: boolean,
531+
clear: boolean,
532+
dryRun: boolean,
533+
): unknown {
512534
if (clear) {
513535
if (raw !== undefined || apply) {
514536
throw new Error("Use either --apply with --grading or --clear, not both");
515537
}
516538
return undefined;
517539
}
518-
if (!apply) {
519-
if (raw !== undefined) throw new Error("--grading requires --apply");
540+
if (!apply && !dryRun) {
541+
if (raw !== undefined) throw new Error("--grading requires --apply or --dry-run");
520542
throw new Error("Use --apply with --grading <json> or --clear");
521543
}
522544
if (raw === undefined) throw new Error("--apply requires --grading <json>");
@@ -613,20 +635,23 @@ function analyzeTarget(args: MediaTreatmentCommandArgs) {
613635
};
614636
}
615637

616-
function prepareMutation(args: MediaTreatmentCommandArgs) {
638+
async function prepareMutation(args: MediaTreatmentCommandArgs) {
617639
const { project, filePath } = resolveMutationFile(args);
618640
const selector = readOptionalString(args.selector);
619641
if (!selector) throw new Error("--selector is required");
620642
const clear = args.clear === true;
621643
const apply = args.apply === true;
644+
const dryRun = args["dry-run"] === true;
622645
const selectorIndex = parseSelectorIndex(readOptionalString(args["selector-index"]));
623646
const result = applyMediaTreatmentToHtml(readFileSync(filePath, "utf8"), {
624647
selector,
625648
selectorIndex,
626-
grading: parseGrading(readOptionalString(args.grading), apply, clear),
649+
grading: parseGrading(readOptionalString(args.grading), apply, clear, dryRun),
627650
clear,
628651
});
629-
const dryRun = args["dry-run"] === true;
652+
const lintResult = await lintHyperframeHtml(result.html);
653+
const findings = lintResult.findings.filter(({ code }) => code.startsWith("color_grading_"));
654+
const lint = { ok: findings.every(({ severity }) => severity !== "error"), findings };
630655
if (result.changed && !dryRun) writeFileSync(filePath, result.html);
631656

632657
const action: "clear" | "apply" = result.value === null ? "clear" : "apply";
@@ -645,6 +670,9 @@ function prepareMutation(args: MediaTreatmentCommandArgs) {
645670
dryRun,
646671
before: result.before,
647672
after: result.after,
673+
attribute: HF_COLOR_GRADING_ATTR,
674+
value: result.value,
675+
lint,
648676
},
649677
};
650678
}
@@ -667,8 +695,8 @@ function printAnalysis(args: MediaTreatmentCommandArgs): void {
667695
console.log(` suggested patch: ${JSON.stringify(result.suggestedPatch)}`);
668696
}
669697

670-
function printMutation(args: MediaTreatmentCommandArgs): void {
671-
const { action, result, selector, payload } = prepareMutation(args);
698+
async function printMutation(args: MediaTreatmentCommandArgs): Promise<void> {
699+
const { action, result, selector, payload } = await prepareMutation(args);
672700
if (args.json === true) {
673701
console.log(JSON.stringify(withMeta(payload), null, 2));
674702
return;
@@ -731,19 +759,20 @@ export const mediaTreatmentCommand = defineCommand({
731759
clear: { type: "boolean", description: "Remove color grading from the target", default: false },
732760
"dry-run": {
733761
type: "boolean",
734-
description: "Validate and report without writing",
762+
description:
763+
"Preview the normalized grading attribute and color-grading lint verdict without writing",
735764
default: false,
736765
},
737766
json: { type: "boolean", description: "Output an agent-friendly JSON result", default: false },
738767
},
739-
run({ args }) {
768+
async run({ args }) {
740769
try {
741770
if (isCapabilityQuery(args)) return runCapabilityQuery(args);
742771
if (args.analyze === true) {
743772
printAnalysis(args);
744773
return;
745774
}
746-
printMutation(args);
775+
await printMutation(args);
747776
} catch (error) {
748777
printFailure(error, args.json === true);
749778
}

‎packages/core/src/colorGrading.test.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,36 @@ describe("color grading", () => {
818818
expect(serialized).toContain('"secondaries"');
819819
});
820820

821+
it("writes only the hue curves that were authored, so lint accepts a single hue curve", async () => {
822+
const grading = normalizeHfColorGrading({
823+
hueCurves: {
824+
hueVsHue: [
825+
[0, 0],
826+
[120, 20],
827+
[240, 0],
828+
],
829+
},
830+
});
831+
const serialized = serializeHfColorGrading(grading);
832+
expect(JSON.parse(serialized).hueCurves).toEqual({
833+
hueVsHue: [
834+
[0, 0],
835+
[120, 20],
836+
[240, 0],
837+
],
838+
});
839+
840+
const lint = await lintHyperframeHtml(`
841+
<html><body>
842+
<div id="root" data-composition-id="c1" data-start="0" data-width="1920" data-height="1080" data-duration="1">
843+
<img class="clip" data-start="0" data-duration="1" src="media.jpg" data-color-grading='${serialized}'>
844+
</div>
845+
<script>window.__timelines = {};</script>
846+
</body></html>
847+
`);
848+
expect(lint.findings.filter((finding) => finding.severity === "error")).toEqual([]);
849+
});
850+
821851
it("round-trips advanced grading byte-identically", () => {
822852
const grading = normalizeHfColorGrading({
823853
wheels: { shadows: { hue: 205, amount: 0.08 } },

‎packages/core/src/colorGrading.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1504,6 +1504,15 @@ export function hasHfColorGradingSecondaryValues(
15041504
);
15051505
}
15061506

1507+
function authoredHueCurves(curves: NormalizedHfColorGradingHueCurves): HfColorGradingHueCurves {
1508+
return Object.fromEntries(
1509+
HF_COLOR_GRADING_HUE_CURVE_KEYS.filter((key) => curves[key].length > 0).map((key) => [
1510+
key,
1511+
curves[key],
1512+
]),
1513+
);
1514+
}
1515+
15071516
export function serializeHfColorGrading(
15081517
grading: NormalizedHfColorGrading | HfColorGrading | null,
15091518
): string {
@@ -1522,7 +1531,9 @@ export function serializeHfColorGrading(
15221531
...serializable,
15231532
...(hasWheelGrade(wheels) ? { wheels } : {}),
15241533
...(hasHfColorGradingRgbCurveValues(curves) ? { curves } : {}),
1525-
...(hasHfColorGradingHueCurveValues(hueCurves) ? { hueCurves } : {}),
1534+
...(hasHfColorGradingHueCurveValues(hueCurves)
1535+
? { hueCurves: authoredHueCurves(hueCurves) }
1536+
: {}),
15261537
// Preserve a keyed secondary even before it changes pixels so Studio and
15271538
// CLI can author its qualifier and correction in separate transactions.
15281539
...(secondaries.length > 0 ? { secondaries } : {}),

‎packages/core/src/runtime/colorGrading.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,23 @@ describe("createColorGradingRuntime", () => {
586586
expect(lastUniform1f).toHaveBeenCalledWith("u_exposure", 0.35);
587587
});
588588

589+
it("reads a grading property set in a stylesheet, not only inline", () => {
590+
const style = document.createElement("style");
591+
style.textContent = "#plate { --hf-color-grading-blur: 0.6; }";
592+
document.head.appendChild(style);
593+
const video = makeDrawableVideo();
594+
video.id = "plate";
595+
video.setAttribute(
596+
HF_COLOR_GRADING_ATTR,
597+
serializeHfColorGrading({ adjust: { saturation: -0.5 } }),
598+
);
599+
stubCubeLutFetch();
600+
startRuntimeWithVideo(video);
601+
602+
if (!lastUniform1f) throw new Error("Expected WebGL uniform calls");
603+
expect(lastUniform1f).toHaveBeenCalledWith("u_blur", 0.6);
604+
});
605+
589606
it("samples seek-derived grading values from inline CSS properties on every redraw", () => {
590607
const video = makeDrawableVideo();
591608
video.setAttribute(

0 commit comments

Comments
 (0)