From c569760a7a7c96df6540d53bbfe4670225ae140e Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Wed, 28 Oct 2026 10:51:17 -0700 Subject: [PATCH] fix(core): grade groups resolve per composition; freeze frames keep their lane (#6685) Co-authored-by: Claude Opus 5.5 --- packages/core/src/colorGroups.test.ts | 25 +++++++++++++++++++ packages/core/src/colorGroups.ts | 18 ++++++++++--- .../engine/src/services/videoFrameInjector.ts | 8 +++++- .../src/helpers/freezeFrame.test.ts | 8 ++++++ .../studio-server/src/helpers/freezeFrame.ts | 2 +- 5 files changed, 56 insertions(+), 5 deletions(-) diff --git a/packages/core/src/colorGroups.test.ts b/packages/core/src/colorGroups.test.ts index b9a5f0c154..43dd98b920 100644 --- a/packages/core/src/colorGroups.test.ts +++ b/packages/core/src/colorGroups.test.ts @@ -113,3 +113,28 @@ describe("ensureColorGroupInertStyle", () => { expect(document.head.querySelectorAll("style")).toHaveLength(1); }); }); + +describe("group ids are scoped to their composition", () => { + it("pairs a member with the group in its own composition before one elsewhere with the same id", () => { + document.body.innerHTML = ` +
+ + +
+
+ + +
+ `; + expect(effectiveColorGrading(document.getElementById("first")!, document)).toBe(LANE_GRADE); + expect(effectiveColorGrading(document.getElementById("second")!, document)).toBe(OWN_GRADE); + }); + + it("falls back to the document when the member's composition declares no such group", () => { + document.body.innerHTML = ` + +
+ `; + expect(effectiveColorGrading(document.getElementById("v")!, document)).toBe(LANE_GRADE); + }); +}); diff --git a/packages/core/src/colorGroups.ts b/packages/core/src/colorGroups.ts index 68ce44e9a9..0a21a7a6ef 100644 --- a/packages/core/src/colorGroups.ts +++ b/packages/core/src/colorGroups.ts @@ -27,10 +27,22 @@ export function colorGroupOf(el: Element): string | null { } /** - * The `` for an id, or null. Tag-checked so an unrelated + * The `` for an id, or null. Ids are unique only per + * composition file, so a member's own composition is searched first, as audio + * groups pair per composition; then the document. Tag-checked so an unrelated * element sharing the id is never read as a lane's grade. */ -export function resolveColorGroupElement(root: GroupLookupRoot, groupId: string): Element | null { +export function resolveColorGroupElement( + root: GroupLookupRoot, + groupId: string, + member?: Element, +): Element | null { + const scope = + typeof member?.closest === "function" ? member.closest("[data-composition-id]") : null; + const scoped = scope + ? Array.from(scope.getElementsByTagName(HF_COLOR_GROUP_TAG)).find((g) => g.id === groupId) + : undefined; + if (scoped) return scoped; const el = root?.getElementById(groupId) ?? null; return el?.tagName.toLowerCase() === HF_COLOR_GROUP_TAG ? el : null; } @@ -44,7 +56,7 @@ export function effectiveColorGrading(el: Element, root: GroupLookupRoot): strin if (own !== null) return own; const groupId = colorGroupOf(el); if (!groupId) return null; - return resolveColorGroupElement(root, groupId)?.getAttribute(HF_COLOR_GRADING_ATTR) ?? null; + return resolveColorGroupElement(root, groupId, el)?.getAttribute(HF_COLOR_GRADING_ATTR) ?? null; } /** diff --git a/packages/engine/src/services/videoFrameInjector.ts b/packages/engine/src/services/videoFrameInjector.ts index f944187091..88d82b2333 100644 --- a/packages/engine/src/services/videoFrameInjector.ts +++ b/packages/engine/src/services/videoFrameInjector.ts @@ -384,7 +384,13 @@ export async function measureVideoShownBoxes( const fullSize = new Set(); const gradedThroughGroup = (video: Element): boolean => { const groupId = video.getAttribute(groupAttr); - const group = groupId ? document.getElementById(groupId) : null; + if (!groupId) return false; + // the member's own composition first: ids are unique only per composition file + const scope = video.closest("[data-composition-id]"); + const group = + (scope && + Array.from(scope.getElementsByTagName(groupTag)).find((g) => g.id === groupId)) || + document.getElementById(groupId); return group?.localName === groupTag && group.hasAttribute(gradingAttr); }; const keepsFullSize = (video: Element): boolean => { diff --git a/packages/studio-server/src/helpers/freezeFrame.test.ts b/packages/studio-server/src/helpers/freezeFrame.test.ts index 116de769b9..aaae1b77b5 100644 --- a/packages/studio-server/src/helpers/freezeFrame.test.ts +++ b/packages/studio-server/src/helpers/freezeFrame.test.ts @@ -163,6 +163,14 @@ describe("applyFreezeFrameToHtml", () => { ).toBe("11"); }); + it("keeps a lane clip's grade group on the still, so the still is graded with its lane", () => { + const graded = applyFreezeFrameToHtml( + project.replace('id="talk"', 'id="talk" data-color-group="cam-a"'), + { target: { id: "talk" }, playhead: 3.2, imageSrc: "assets/freeze/talk-3200.png" }, + ); + expect(at(graded?.html ?? "", "talk-freeze")?.getAttribute("data-color-group")).toBe("cam-a"); + }); + it("returns null when the playhead is not inside the video", () => { expect( applyFreezeFrameToHtml(project, { target: { id: "talk" }, playhead: 9, imageSrc: "x.png" }), diff --git a/packages/studio-server/src/helpers/freezeFrame.ts b/packages/studio-server/src/helpers/freezeFrame.ts index 4bd8af25f2..a7045a25b0 100644 --- a/packages/studio-server/src/helpers/freezeFrame.ts +++ b/packages/studio-server/src/helpers/freezeFrame.ts @@ -138,7 +138,7 @@ function growRootToFit(document: Document): void { if (end > rootDuration + EPSILON) root.setAttribute("data-duration", String(round3(end))); } -const COPIED_PICTURE_ATTRS = ["class", "style", "data-color-grading"]; +const COPIED_PICTURE_ATTRS = ["class", "style", "data-color-grading", "data-color-group"]; function buildStill( document: Document,