feat(studio): add media treatment inspector#2755
Conversation
There was a problem hiding this comment.
Pull request overview
Adds “media treatment” tooling to the Studio flat inspector so selected media can be graded, given shader effects, and augmented with Registry overlay components—while keeping persistence routed through existing edit/registry install paths.
Changes:
- Introduces color grading preview batching + animated hover previews via the preview iframe runtime bridge.
- Adds new flat inspector sections for Grade presets, Effects (families + controls), and Registry-driven Overlays install.
- Extends block installation to support explicit overlay placement (start/duration/track) and refactors install/insert logic with targeted tests.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/studio/src/utils/blockInstaller.ts | Refactors Registry install + sub-comp insertion; adds placement duration/optional track support for overlays. |
| packages/studio/src/utils/blockInstaller.test.ts | Covers explicit duration/track placement behavior for Registry components. |
| packages/studio/src/hooks/useBlockHandlers.ts | Adds handleAddMediaOverlay to install overlays into the selected media’s composition/timing. |
| packages/studio/src/components/StudioRightPanel.tsx | Wires an optional onAddMediaOverlay handler through the right panel to the property panel. |
| packages/studio/src/components/editor/useColorGradingPreviews.ts | New hook for preset/effect preview batching, retries, caching, and animated hover preview frames. |
| packages/studio/src/components/editor/useColorGradingController.ts | Integrates preview capabilities into the existing grading controller API/state. |
| packages/studio/src/components/editor/useColorGradingController.test.ts | Adds tests for preview batching, effect-family requests, preview-only behavior, and animated hover previews. |
| packages/studio/src/components/editor/propertyPanelTypes.ts | Adds MediaOverlayPlacement + AddMediaOverlayHandler types for overlay installs. |
| packages/studio/src/components/editor/propertyPanelPresetPreview.ts | New shared helper to standardize preview/commit pointer/focus handlers for preset cards. |
| packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx | Adds a shared responsive grid class constant for preview card layouts. |
| packages/studio/src/components/editor/propertyPanelFlatOverlaysSection.tsx | New Overlays section that filters tagged Registry components and previews poster/video on hover. |
| packages/studio/src/components/editor/propertyPanelFlatOverlaysSection.test.tsx | Tests overlay filtering, placement derivation, and add-overlay delegation. |
| packages/studio/src/components/editor/propertyPanelFlatEffectsSection.tsx | New Effects section (families, previews, presets, palette + per-effect controls). |
| packages/studio/src/components/editor/propertyPanelFlatEffectsSection.test.tsx | Tests effect families, preview callbacks, applying/removing/resetting effects, and palette behavior. |
| packages/studio/src/components/editor/propertyPanelFlatEffectSpecs.ts | Defines effect families, control specs, and defaults used by the Effects UI. |
| packages/studio/src/components/editor/propertyPanelFlatEffectControl.tsx | Renders per-effect slider/toggle/select controls with correct defaulting/reset behavior. |
| packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.tsx | Switches Grade UI to preset cards w/ runtime previews; renames “Strength” → “Amount”; removes legacy mini effects sliders. |
| packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.test.tsx | Updates tests for preset-card preview/commit behavior and removes legacy effects assertions. |
| packages/studio/src/components/editor/PropertyPanelFlat.tsx | Adds new accordion groups for Effects and Overlays; wires preview/overlay handlers; scrolls newly opened group into view. |
| packages/studio/src/components/editor/PropertyPanel.test.tsx | Updates fixed-header/open-body test expectations to account for the new Effects group. |
| packages/studio/src/App.tsx | Wires handleAddMediaOverlay from block handlers into the right panel. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const applyPreset = (presetId: string) => { | ||
| const next = normalizeHfColorGrading({ preset: presetId, intensity: 1, lut: grading.lut }); | ||
| if (next) onCommitColorGrading(next); | ||
| useEffect(() => onRequestPresetPreviews(), [onRequestPresetPreviews]); |
| useEffect(() => { | ||
| if (!catalogOpen) return; | ||
| const group = EFFECT_GROUPS.find((candidate) => candidate.label === catalogGroup); | ||
| if (!group) return; | ||
| onRequestEffectPreviews(group.effects.map((effect) => effect.key)); | ||
| if (group.presets?.length) onRequestPresetPreviews(); | ||
| }, [catalogGroup, catalogOpen, onRequestEffectPreviews, onRequestPresetPreviews]); |
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed exact head 9b2a1a753a8a159b44b6537e28f6f3be489496c5.
Blocking
packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.tsx:271-274
The Grade preset path rebuilds the entire normalized grading object from only { preset, lut }:
const next = normalizeHfColorGrading({ preset: presetId, lut: grading.lut });That silently drops independently configured effects and palette. The new inspector presents Grade and Effects as composable sibling sections, so this makes the result order-dependent: apply Blur (or a palette) in Effects, then choose a Grade preset, and the prior treatment disappears.
I confirmed this directly against Core: normalizing { preset: "bright-pop", effects: { blur: 0.5 } }, then taking the current preset resolver path for "deep-contrast", changes blur from 0.5 to 0.
Please preserve the currently active Effects/Palette while applying the Grade-owned preset fields (or expose a Core helper that performs that scoped operation), and add a regression starting with a nonzero effect/palette, clicking a Grade preset, and asserting those values survive while the preset changes. Please also audit the Grade reset path for the same whole-object replacement shape.
Important
packages/studio/src/components/editor/propertyPanelFlatEffectSpecs.ts:79-285
This new 200-line Studio registry re-declares effect keys, ranges, defaults/control shape, and palette support that Core already publishes through getHfColorGradingCapabilities(). The values match today, but Studio never consumes that capability API, so future Core additions/range changes can produce a valid serialized treatment that the inspector renders incorrectly or cannot edit. Keep UI-only labels/grouping/formatters here, but derive the canonical keys, bounds, defaults, and palette support from Core. This is a new Studio SSOT seam, separate from the already-reported lint/CLI key copies in earlier PRs.
Ponytail review
propertyPanelFlatEffectSpecs.ts:79-285— join compact presentation metadata against Core capabilities instead of restating the treatment contract — net: -90 lines possible.
Copilot's two preview-generation findings are already covered and not repeated here. Resource lifecycles and the block-installer extraction otherwise looked clean.
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Additive review at 9b2a1a75, weighted to Ular's focus (duplication / deletable / over-complex / core+registry contract match). @Copilot commented (redundant preset-preview renders on mount and on effect-family switch); @magi is reviewing in parallel. Not repeating those or the #2751-#2754 findings.
Good — consumes the shared contracts instead of re-implementing them. The inspector reads getHfColorGradingCapabilities() + core normalize/serialize; blockInstaller.ts delegates the actual install to the existing POST /api/projects/:id/registry/install route (:67) and reuses the existing Studio edit helpers (saveProjectFilesWithHistory, collectHtmlIds, extendRootDurationInSource) for composition insertion — no duplicate install pipeline. DEFAULT_EFFECTS derives from normalizeHfColorGrading("neutral") rather than hard-coding (effectSpecs.ts:283).
Verified — no range bug in the effect sliders (the thing most likely to be one). The bloom master uses max: 300 while core clamps bloom to {0,3}; I traced the mapping — effectsSection.tsx:304/312 displays value*100 and commits next/100, and sub-controls use sliderValue = value*scale / next/scale (effectControl.tsx:34-48), so bloom stores 0–3 and bloomRadius (scale 1, {1,100}) stores its raw range. All match core; the sliders don't overrun the runtime clamp.
Additive (duplication / drift) — effectSpecs.ts is the third consumer that shadows core's effect contract, and the drift surface here is ranges + option counts, not just keys. Same systemic shape as the #2752 lint rule (Magi's blocker) and the #2753 CLI GRADING_KEYS/LUT_KEYS — both already being fixed — so I'm flagging only what's new to this file: effectSpecs.ts hand-codes numeric ranges (bloomRadius {1,100}, bloom master 300 = core-max × 100) and select-option lists (ASCII_STYLES = 8 for core asciiStyle {0,7}; SCREEN_SHAPES = 5 for monoScreenShape {0,4}) that getHfColorGradingCapabilities() already owns. All correct today, but they drift silently if core retunes a range or adds a style. The genuinely UI-specific bits (labels, masterFormat, groupings, master relationships) are legit and should stay. One concrete gap worth closing now regardless of the shared-registry work: the effect keys are satisfies-typed for membership but there's no completeness check, so a new core active effect wouldn't error — it'd just be silently missing from the inspector. An exhaustiveness assertion against HF_COLOR_GRADING_ACTIVE_EFFECT_KEYS (and deriving the select-option counts from the core max) would make this the point where the stack's contract is enforced rather than re-copied. (Inline below.)
Verdict: COMMENT — Ready on the merits; the effectSpecs drift is latent (verified correct today) and best folded into the same core-registry consumption the earlier PRs are getting. Solid, contract-respecting inspector. (Deferring the stamp to the maintainer.)
Review by Jerrai (hyperframes)
| }, | ||
| ]; | ||
|
|
||
| export const EFFECT_SPECS = EFFECT_GROUPS.flatMap((group) => group.effects); |
There was a problem hiding this comment.
The effect keys in EFFECT_GROUPS are satisfies-typed against HfColorGradingActiveEffectKey, so a typo'd key won't compile — but there's no completeness check, so a new core active effect would not error here; it'd just be silently absent from the inspector. A one-line exhaustiveness assertion where EFFECT_SPECS is derived — e.g. verify EFFECT_SPECS.map(e => e.key) covers HF_COLOR_GRADING_ACTIVE_EFFECT_KEYS — closes that gap.
Same note applies to the hard-coded numeric ranges (bloomRadius {1,100}, bloom master 300) and select-option counts (ASCII_STYLES=8, SCREEN_SHAPES=5): all match core today (verified), but they shadow getHfColorGradingCapabilities() and would drift if core retunes. When the shared core-registry work from #2752/#2753 lands, deriving these here (and asserting the option counts against the core asciiStyle/monoScreenShape maxes) makes this the enforcement point rather than a third copy.
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Follow-up to my earlier COMMENT, correcting my verdict: @magi's CHANGES_REQUESTED blocker is real — I reproduced it in source. My prior "ready on the merits" was scoped to the areas I'd audited (effectSpecs / install / slider ranges), not the preset-apply path.
Confirmed — Grade-preset data loss at propertyPanelFlatColorGradingSection.tsx:272. resolvePreset rebuilds grading from only { preset: presetId, lut: grading.lut }, so normalizeHfColorGrading refills adjust/details/effects from the preset — for a Grade preset that zeroes the user's independently-set Effects and Palette (matches Magi's blur 0.5 → 0; it's core's rawX[key] ?? presetX[key] semantics). The diff shows the path is pre-existing (it only dropped the old intensity: 1), but #2755 is the PR that adds independent Effects + Palette to the inspector — so this change is what turns a latent reconstruct into live data loss. In scope.
Additive — the correct shape is already in the same file; it's a one-line consistency break. resolvePreset is the only commit path in this section that doesn't spread the current grading. Every sibling preserves state via { ...grading, … }: intensity (:278), lut (:282), details (:313), adjust (:493). The preset path should match — keep effects + palette, merge the preset's adjust/details onto the current grading — plus Magi's requested regression, and the same audit on the reset path.
Updated verdict: COMMENT, concurring with @magi's REQUEST_CHANGES. My effectSpecs / SSOT + range notes stand as latent cleanups; this preset data loss is the blocker.
Review by Jerrai (hyperframes)
| useEffect(() => onRequestPresetPreviews(), [onRequestPresetPreviews]); | ||
|
|
||
| const resolvePreset = (presetId: string) => { | ||
| const next = normalizeHfColorGrading({ preset: presetId, lut: grading.lut }); |
There was a problem hiding this comment.
This rebuilds grading from only { preset, lut }, so normalizeHfColorGrading refills adjust/details/effects from the preset — zeroing the user's independently-set Effects and Palette on any Grade-preset click (Magi's blocker; blur 0.5 → 0).
It's the only commit in this section that doesn't spread the current grading — intensity (:278), lut (:282), details (:313), and adjust (:493) all use { ...grading, … }. Match that here: preserve effects + palette and merge the preset's adjust/details onto grading instead of reconstructing. (Pre-existing pattern, but this PR is what adds the independent Effects/Palette it now erases.)
9b2a1a7 to
a473c41
Compare
6b7bb56 to
9b79f5b
Compare
What
Adds selected-media Grade, Effects, and Overlays controls to the flat Studio inspector, with responsive visual grids, shader-rendered look/effect previews, selected-video hover playback, adjustable effect controls, LUT upload, compare, and one-click overlay installation.
Why
Users need to see treatments on the selected media before applying them, while retaining precise manual control and the same persisted contract agents use.
How
The inspector consumes Core capability metadata and the existing runtime bridge. Hover is preview-only; click persists through the existing edit path. Preview batching, cancellation, caching, and synchronous latest-state tracking avoid stale commits and unnecessary renderer work.
Test plan
Stack 5/6. Base: #2754. The separate workspace-layout cleanup is PR 6.