diff --git a/apps/skit/src/server.rs b/apps/skit/src/server.rs index 2bc66f67..75c8393f 100644 --- a/apps/skit/src/server.rs +++ b/apps/skit/src/server.rs @@ -1926,6 +1926,7 @@ async fn get_pipeline_handler( // Fetch current node states without holding the pipeline lock. let node_states = session.get_node_states().await.unwrap_or_default(); + let node_view_data = session.get_node_view_data().await.unwrap_or_default(); // Clone pipeline (short lock hold) and add runtime state to nodes. let mut api_pipeline = { @@ -1936,6 +1937,11 @@ async fn get_pipeline_handler( node.state = node_states.get(id).cloned(); } + // Attach resolved view data so clients have accurate positions on initial load. + if !node_view_data.is_empty() { + api_pipeline.view_data = Some(node_view_data); + } + info!("Fetched pipeline with states for session '{}' via HTTP", session_id); Ok(Json(api_pipeline)) } diff --git a/apps/skit/src/session.rs b/apps/skit/src/session.rs index ba94ec8c..159b2510 100644 --- a/apps/skit/src/session.rs +++ b/apps/skit/src/session.rs @@ -350,6 +350,19 @@ impl Session { pub async fn get_node_stats(&self) -> Result, String> { self.engine_handle.get_node_stats().await } + + /// Gets the current view data for all nodes in this session's pipeline. + /// + /// View data contains resolved runtime state that differs from the static + /// config params (e.g., compositor resolved layout with aspect-fit adjustments). + /// + /// # Errors + /// + /// Returns an error if the engine handle's oneshot channel fails to receive a response, + /// which typically indicates the engine actor has stopped or panicked. + pub async fn get_node_view_data(&self) -> Result, String> { + self.engine_handle.get_node_view_data().await + } } /// A thread-safe manager for all active sessions. diff --git a/apps/skit/src/websocket_handlers.rs b/apps/skit/src/websocket_handlers.rs index 9cf398a8..2ebfa92c 100644 --- a/apps/skit/src/websocket_handlers.rs +++ b/apps/skit/src/websocket_handlers.rs @@ -1028,6 +1028,7 @@ async fn handle_get_pipeline( } let node_states = session.get_node_states().await.unwrap_or_default(); + let node_view_data = session.get_node_view_data().await.unwrap_or_default(); // Clone pipeline (short lock hold) and add runtime state to nodes. let mut api_pipeline = { @@ -1038,6 +1039,11 @@ async fn handle_get_pipeline( node.state = node_states.get(id).cloned(); } + // Attach resolved view data so clients have accurate positions on initial load. + if !node_view_data.is_empty() { + api_pipeline.view_data = Some(node_view_data); + } + info!( session_id = %session_id, node_count = api_pipeline.nodes.len(), diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index ed9be8cf..f248aa36 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -9,6 +9,7 @@ //! contract exclusively uses JSON for consistency and TypeScript compatibility. use serde::{Deserialize, Serialize}; +use std::collections::HashMap; use ts_rs::TS; // YAML pipeline format compilation @@ -526,6 +527,12 @@ pub struct Pipeline { #[ts(type = "Record")] pub nodes: indexmap::IndexMap, pub connections: Vec, + /// Resolved per-node view data (e.g., compositor layout). + /// Only populated in API responses; absent from pipeline definitions. + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] + #[ts(type = "Record | null")] + pub view_data: Option>, } // Type aliases for backwards compatibility diff --git a/crates/api/src/yaml.rs b/crates/api/src/yaml.rs index 3265602e..86500178 100644 --- a/crates/api/src/yaml.rs +++ b/crates/api/src/yaml.rs @@ -177,7 +177,7 @@ fn compile_steps( nodes.insert(node_name, Node { kind: step.kind, params: step.params, state: None }); } - Pipeline { name, description, mode, nodes, connections } + Pipeline { name, description, mode, nodes, connections, view_data: None } } /// Known bidirectional node kinds that are allowed to participate in cycles. @@ -409,7 +409,7 @@ fn compile_dag( }) .collect(); - Ok(Pipeline { name, description, mode, nodes, connections }) + Ok(Pipeline { name, description, mode, nodes, connections, view_data: None }) } #[cfg(test)] diff --git a/crates/engine/benches/compositor_pipeline.rs b/crates/engine/benches/compositor_pipeline.rs index 5cb6d8e5..89db92d8 100644 --- a/crates/engine/benches/compositor_pipeline.rs +++ b/crates/engine/benches/compositor_pipeline.rs @@ -253,6 +253,7 @@ fn build_pipeline(width: u32, height: u32, fps: u32, frame_count: u32) -> stream mode: EngineMode::OneShot, nodes, connections, + view_data: None, } } diff --git a/ui/src/hooks/compositorDragResize.test.ts b/ui/src/hooks/compositorDragResize.test.ts new file mode 100644 index 00000000..e1b98845 --- /dev/null +++ b/ui/src/hooks/compositorDragResize.test.ts @@ -0,0 +1,134 @@ +// SPDX-FileCopyrightText: © 2025 StreamKit Contributors +// +// SPDX-License-Identifier: MPL-2.0 + +/** + * Unit tests for the zero-delta click guard in useCompositorDragResize. + * + * Verifies that a pointer-up at the exact same position as pointer-down + * (i.e. a click-to-select with zero movement) does NOT fire the config + * change callback, preventing stale client positions from overwriting + * the server's resolved layout. + */ + +import { renderHook, act } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; + +import type { DragResizeDeps } from './compositorDragResize'; +import { useCompositorDragResize } from './compositorDragResize'; +import type { LayerState } from './compositorLayerParsers'; + +/** Build a minimal layer for testing. */ +function makeLayer(id: string): LayerState { + return { + id, + x: 100, + y: 100, + width: 200, + height: 150, + opacity: 1, + zIndex: 0, + visible: true, + rotationDegrees: 0, + mirrorHorizontal: false, + mirrorVertical: false, + cropZoom: 1.0, + cropX: 0.5, + cropY: 0.5, + }; +} + +function makeDeps(overrides: Partial = {}): DragResizeDeps { + const layer = makeLayer('layer-1'); + return { + canvasWidth: 1280, + canvasHeight: 720, + dragStateRef: { current: null }, + layerRefs: { current: new Map() }, + layersRef: { current: [layer] }, + textOverlaysRef: { current: [] }, + imageOverlaysRef: { current: [] }, + setLayers: vi.fn(), + setTextOverlays: vi.fn(), + setImageOverlays: vi.fn(), + setSelectedLayerId: vi.fn(), + setIsDragging: vi.fn(), + findAnyLayer: (id: string) => { + if (id === 'layer-1') return { state: layer, kind: 'video' as const }; + return null; + }, + throttledConfigChange: vi.fn(), + commitOverlaysRef: { current: vi.fn() }, + snapGuideRefs: { current: { vertical: null, horizontal: null } }, + ...overrides, + }; +} + +describe('useCompositorDragResize zero-delta guard', () => { + let deps: DragResizeDeps; + + beforeEach(() => { + deps = makeDeps(); + }); + + it('should NOT fire throttledConfigChange on zero-delta click (video layer)', () => { + const { result } = renderHook(() => useCompositorDragResize(deps)); + + // Simulate pointer-down at (500, 300) + act(() => { + result.current.handleLayerPointerDown('layer-1', { + button: 0, + clientX: 500, + clientY: 300, + stopPropagation: vi.fn(), + preventDefault: vi.fn(), + } as unknown as React.PointerEvent); + }); + + // Verify drag state was set + expect(deps.dragStateRef.current).not.toBeNull(); + + // Simulate pointer-up at the exact same position (zero delta) + act(() => { + const pointerUpEvent = new PointerEvent('pointerup', { + clientX: 500, + clientY: 300, + }); + document.dispatchEvent(pointerUpEvent); + }); + + // throttledConfigChange must NOT have been called + expect(deps.throttledConfigChange).not.toHaveBeenCalled(); + + // setLayers must NOT have been called (no state update) + expect(deps.setLayers).not.toHaveBeenCalled(); + }); + + it('should fire throttledConfigChange on actual drag (video layer)', () => { + const { result } = renderHook(() => useCompositorDragResize(deps)); + + // Simulate pointer-down at (500, 300) + act(() => { + result.current.handleLayerPointerDown('layer-1', { + button: 0, + clientX: 500, + clientY: 300, + stopPropagation: vi.fn(), + preventDefault: vi.fn(), + } as unknown as React.PointerEvent); + }); + + // Simulate pointer-up at a DIFFERENT position (non-zero delta) + act(() => { + const pointerUpEvent = new PointerEvent('pointerup', { + clientX: 520, + clientY: 310, + }); + document.dispatchEvent(pointerUpEvent); + }); + + // throttledConfigChange SHOULD have been called + expect(deps.throttledConfigChange).toHaveBeenCalled(); + }); +}); diff --git a/ui/src/hooks/compositorDragResize.ts b/ui/src/hooks/compositorDragResize.ts index 17ab2b2b..e2f48f8b 100644 --- a/ui/src/hooks/compositorDragResize.ts +++ b/ui/src/hooks/compositorDragResize.ts @@ -173,43 +173,62 @@ export function useCompositorDragResize(deps: DragResizeDeps) { const updated = computeLayerFromPointer(state, e.clientX, e.clientY); setIsDragging(false); + // Pure click-to-select (zero movement) — don't commit to server. + // Committing here would send potentially stale config-parsed positions + // for ALL layers, overwriting the server's resolved layout. + const dx = e.clientX - state.startX; + const dy = e.clientY - state.startY; + const isZeroDelta = dx === 0 && dy === 0; + if (state.layerKind === 'video') { - setLayers((prev) => prev.map((l) => (l.id === updated.id ? updated : l))); - const newLayers = layersRef.current.map((l) => (l.id === updated.id ? updated : l)); - throttledConfigChange?.(newLayers); + if (!isZeroDelta) { + setLayers((prev) => prev.map((l) => (l.id === updated.id ? updated : l))); + const newLayers = layersRef.current.map((l) => (l.id === updated.id ? updated : l)); + throttledConfigChange?.(newLayers); + } } else if (state.layerKind === 'text') { - const isResize = state.type === 'resize'; - const origFontSize = state.origFontSize; - setTextOverlays((prev) => { - const next = prev.map((o) => { - if (o.id !== updated.id) return o; - const patch: Partial = { - x: updated.x, - y: updated.y, - width: updated.width, - height: updated.height, - }; - if (isResize && origFontSize != null && state.origLayer.width > 0) { - patch.fontSize = Math.max( - 8, - Math.round(origFontSize * (updated.width / state.origLayer.width)) - ); - } - return { ...o, ...patch }; + if (!isZeroDelta) { + const isResize = state.type === 'resize'; + const origFontSize = state.origFontSize; + setTextOverlays((prev) => { + const next = prev.map((o) => { + if (o.id !== updated.id) return o; + const patch: Partial = { + x: updated.x, + y: updated.y, + width: updated.width, + height: updated.height, + }; + if (isResize && origFontSize != null && state.origLayer.width > 0) { + patch.fontSize = Math.max( + 8, + Math.round(origFontSize * (updated.width / state.origLayer.width)) + ); + } + return { ...o, ...patch }; + }); + commitOverlaysRef.current(next, imageOverlaysRef.current); + return next; }); - commitOverlaysRef.current(next, imageOverlaysRef.current); - return next; - }); + } } else if (state.layerKind === 'image') { - setImageOverlays((prev) => { - const next = prev.map((o) => - o.id === updated.id - ? { ...o, x: updated.x, y: updated.y, width: updated.width, height: updated.height } - : o - ); - commitOverlaysRef.current(textOverlaysRef.current, next); - return next; - }); + if (!isZeroDelta) { + setImageOverlays((prev) => { + const next = prev.map((o) => + o.id === updated.id + ? { + ...o, + x: updated.x, + y: updated.y, + width: updated.width, + height: updated.height, + } + : o + ); + commitOverlaysRef.current(textOverlaysRef.current, next); + return next; + }); + } } dragStateRef.current = null; diff --git a/ui/src/hooks/compositorLayerParsers.test.ts b/ui/src/hooks/compositorLayerParsers.test.ts new file mode 100644 index 00000000..17f4d90d --- /dev/null +++ b/ui/src/hooks/compositorLayerParsers.test.ts @@ -0,0 +1,247 @@ +// SPDX-FileCopyrightText: © 2025 StreamKit Contributors +// +// SPDX-License-Identifier: MPL-2.0 + +/** + * Unit tests for mergeOverlayState in compositorLayerParsers. + * + * Focuses on the `preserveGeometry` (Monitor view) behaviour: when true, + * ALL server-resolved OverlayBase fields (position, opacity, rotation, etc.) + * plus runtime-only fields (measuredTextWidth, measuredTextHeight) must be + * kept from `current`, with only type-specific config fields (text, fontSize, + * fontName, color, dataBase64) taken from `parsed`. + */ + +import { describe, it, expect } from 'vitest'; + +import type { TextOverlayState, ImageOverlayState } from './compositorLayerParsers'; +import { mergeOverlayState } from './compositorLayerParsers'; + +// ── Helpers ───────────────────────────────────────────────────────────────── + +function makeTextOverlay(overrides: Partial = {}): TextOverlayState { + return { + id: 'text_0', + text: 'Hello', + x: 0, + y: 0, + width: 200, + height: 40, + color: [255, 255, 255, 255], + fontSize: 32, + fontName: 'dejavu-sans', + opacity: 1, + rotationDegrees: 0, + zIndex: 100, + mirrorHorizontal: false, + mirrorVertical: false, + visible: true, + ...overrides, + }; +} + +function makeImageOverlay(overrides: Partial = {}): ImageOverlayState { + return { + id: 'img_0', + dataBase64: 'abc123', + x: 0, + y: 0, + width: 200, + height: 200, + opacity: 1, + rotationDegrees: 0, + zIndex: 200, + mirrorHorizontal: false, + mirrorVertical: false, + visible: true, + ...overrides, + }; +} + +const textHasExtraChanges = (a: TextOverlayState, b: TextOverlayState) => + a.text !== b.text || + a.fontSize !== b.fontSize || + a.fontName !== b.fontName || + a.color.some((v, i) => v !== b.color[i]); + +// ── Tests ─────────────────────────────────────────────────────────────────── + +describe('mergeOverlayState', () => { + describe('preserveGeometry=false (Design view)', () => { + it('takes all fields from parsed, preserving only visible from existing', () => { + const current = [makeTextOverlay({ x: 100, y: 200, visible: false, opacity: 0.5 })]; + const parsed = [makeTextOverlay({ x: 0, y: 0, opacity: 1 })]; + + const result = mergeOverlayState(current, parsed, textHasExtraChanges, false); + + expect(result).not.toBe(current); + expect(result[0].x).toBe(0); // from parsed + expect(result[0].y).toBe(0); // from parsed + expect(result[0].visible).toBe(false); // from existing + expect(result[0].opacity).toBe(0.5); // from existing (because visible=false) + }); + }); + + describe('preserveGeometry=true (Monitor view)', () => { + it('preserves ALL OverlayBase fields from existing, not just x/y/w/h', () => { + const current = [ + makeTextOverlay({ + x: 100, + y: 200, + width: 300, + height: 50, + opacity: 0.8, + rotationDegrees: 45, + zIndex: 150, + mirrorHorizontal: true, + mirrorVertical: true, + visible: true, + }), + ]; + // Parsed has different values for all OverlayBase fields + const parsed = [ + makeTextOverlay({ + x: 0, + y: 0, + width: 200, + height: 40, + opacity: 1, + rotationDegrees: 0, + zIndex: 100, + mirrorHorizontal: false, + mirrorVertical: false, + }), + ]; + + const result = mergeOverlayState(current, parsed, textHasExtraChanges, true); + + // All OverlayBase fields should come from existing + expect(result[0].x).toBe(100); + expect(result[0].y).toBe(200); + expect(result[0].width).toBe(300); + expect(result[0].height).toBe(50); + expect(result[0].opacity).toBe(0.8); + expect(result[0].rotationDegrees).toBe(45); + expect(result[0].zIndex).toBe(150); + expect(result[0].mirrorHorizontal).toBe(true); + expect(result[0].mirrorVertical).toBe(true); + expect(result[0].visible).toBe(true); + }); + + it('takes type-specific config fields from parsed', () => { + const current = [makeTextOverlay({ text: 'Old', fontSize: 24, fontName: 'mono' })]; + const parsed = [makeTextOverlay({ text: 'New', fontSize: 48, fontName: 'dejavu-sans' })]; + + const result = mergeOverlayState(current, parsed, textHasExtraChanges, true); + + expect(result[0].text).toBe('New'); + expect(result[0].fontSize).toBe(48); + expect(result[0].fontName).toBe('dejavu-sans'); + }); + + it('preserves runtime-only fields like measuredTextWidth/measuredTextHeight', () => { + const current = [ + makeTextOverlay({ + x: 100, + y: 200, + width: 300, + height: 50, + measuredTextWidth: 280, + measuredTextHeight: 45, + }), + ]; + // Parsed never has measuredTextWidth/measuredTextHeight + const parsed = [makeTextOverlay({ text: 'Updated' })]; + + const result = mergeOverlayState(current, parsed, textHasExtraChanges, true); + + // Runtime fields should be preserved from existing + expect(result[0].measuredTextWidth).toBe(280); + expect(result[0].measuredTextHeight).toBe(45); + }); + + it('returns same reference when nothing changed', () => { + const overlay = makeTextOverlay({ x: 100, y: 200 }); + const current = [overlay]; + // Parsed has different OverlayBase fields (should be ignored) but same config + const parsed = [makeTextOverlay({ x: 0, y: 0, text: overlay.text })]; + + const result = mergeOverlayState(current, parsed, textHasExtraChanges, true); + + expect(result).toBe(current); // referential equality — no re-render + }); + + it('detects config-only changes via hasExtraChanges', () => { + const current = [makeTextOverlay({ text: 'Old' })]; + const parsed = [makeTextOverlay({ text: 'New' })]; + + const result = mergeOverlayState(current, parsed, textHasExtraChanges, true); + + expect(result).not.toBe(current); // new array due to text change + expect(result[0].text).toBe('New'); + }); + + it('works with image overlays (preserves OverlayBase, takes dataBase64)', () => { + const current = [ + makeImageOverlay({ + x: 50, + y: 50, + width: 400, + height: 300, + rotationDegrees: 90, + dataBase64: 'old-data', + }), + ]; + const parsed = [ + makeImageOverlay({ + x: 0, + y: 0, + width: 200, + height: 200, + rotationDegrees: 0, + dataBase64: 'new-data', + }), + ]; + + const imageHasExtraChanges = (a: ImageOverlayState, b: ImageOverlayState) => + a.dataBase64 !== b.dataBase64; + + const result = mergeOverlayState(current, parsed, imageHasExtraChanges, true); + + // OverlayBase from existing + expect(result[0].x).toBe(50); + expect(result[0].y).toBe(50); + expect(result[0].width).toBe(400); + expect(result[0].height).toBe(300); + expect(result[0].rotationDegrees).toBe(90); + // Config field from parsed + expect(result[0].dataBase64).toBe('new-data'); + }); + + it('handles new overlays (not in current) by using parsed values', () => { + const current: TextOverlayState[] = []; + const parsed = [makeTextOverlay({ id: 'text_new', text: 'Brand new' })]; + + const result = mergeOverlayState(current, parsed, textHasExtraChanges, true); + + expect(result.length).toBe(1); + expect(result[0].id).toBe('text_new'); + expect(result[0].text).toBe('Brand new'); + // Falls through to parsed since no existing match + expect(result[0].x).toBe(0); + }); + + it('handles removed overlays (in current but not in parsed)', () => { + const current = [ + makeTextOverlay({ id: 'text_0' }), + makeTextOverlay({ id: 'text_1', text: 'Second' }), + ]; + const parsed = [makeTextOverlay({ id: 'text_0' })]; + + const result = mergeOverlayState(current, parsed, textHasExtraChanges, true); + + expect(result.length).toBe(1); + expect(result[0].id).toBe('text_0'); + }); + }); +}); diff --git a/ui/src/hooks/compositorLayerParsers.ts b/ui/src/hooks/compositorLayerParsers.ts index be4f05d1..6be8a92a 100644 --- a/ui/src/hooks/compositorLayerParsers.ts +++ b/ui/src/hooks/compositorLayerParsers.ts @@ -319,18 +319,63 @@ export function serializeLayers(layers: LayerState[]): Record = new Set([ + 'id', + 'x', + 'y', + 'width', + 'height', + 'opacity', + 'rotationDegrees', + 'zIndex', + 'mirrorHorizontal', + 'mirrorVertical', + 'visible', +]); + +/** Extract type-specific config fields from a parsed overlay by removing + * all OverlayBase keys. The result contains only fields like `text`, + * `fontSize`, `fontName`, `color`, `dataBase64`, etc. */ +function pickConfigFields(parsed: T): Partial { + const config: Record = {}; + for (const key of Object.keys(parsed)) { + if (!OVERLAY_BASE_KEYS.has(key)) { + config[key] = (parsed as Record)[key]; + } + } + return config as Partial; +} + /** Merge parsed overlays with existing state, preserving client-side visibility. * Returns the same array reference if nothing changed (avoiding re-renders). * An optional `hasExtraChanges` comparator can detect changes in type-specific - * fields (e.g. `text`, `fontSize` for text overlays). */ + * fields (e.g. `text`, `fontSize` for text overlays). + * + * When `preserveGeometry` is true (Monitor view), ALL server-resolved fields + * are kept from `current` — not just positions but also opacity, rotation, + * z-index, mirror flags, and any runtime-only fields (e.g. measuredTextWidth). + * Only type-specific config fields (text, fontSize, color, dataBase64, …) are + * taken from `parsed`. This prevents config-derived values from clobbering the + * server's resolved layout that useServerLayoutSync applied. */ export function mergeOverlayState( current: T[], parsed: T[], - hasExtraChanges?: (a: T, b: T) => boolean + hasExtraChanges?: (a: T, b: T) => boolean, + preserveGeometry?: boolean ): T[] { const merged = parsed.map((p) => { const existing = current.find((o) => o.id === p.id); if (existing) { + if (preserveGeometry) { + // Monitor view: server is the source of truth for all OverlayBase + // fields. Start from `existing` (preserves server-resolved spatial + // values AND runtime-only fields like measuredTextWidth), then + // overlay only type-specific config fields from `parsed`. + return { ...existing, ...pickConfigFields(p) } as T; + } return { ...p, visible: existing.visible, diff --git a/ui/src/hooks/useCompositorLayers.monitor-flow.test.ts b/ui/src/hooks/useCompositorLayers.monitor-flow.test.ts new file mode 100644 index 00000000..703c3c4b --- /dev/null +++ b/ui/src/hooks/useCompositorLayers.monitor-flow.test.ts @@ -0,0 +1,559 @@ +// SPDX-FileCopyrightText: © 2025 StreamKit Contributors +// +// SPDX-License-Identifier: MPL-2.0 + +/** + * Integration tests for the Monitor view compositor data flow. + * + * These tests verify that server-driven layout (useServerLayoutSync) and + * config-driven state (the "sync from props" effect) interact correctly + * in Monitor view. They catch the class of bugs where: + * + * - A server params echo-back overwrites server-resolved positions + * - Runtime measurement fields (measuredTextWidth) are lost on re-merge + * - Focus/selection changes cause layer positions to revert + * + * Each test mounts the real useCompositorLayers hook with a sessionId + * (Monitor view) and drives the Zustand session store to simulate server + * view data arrivals, then exercises the "sync from props" path and + * asserts that server state is preserved. + */ + +import { renderHook, act } from '@testing-library/react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; + +import { useSessionStore } from '@/stores/sessionStore'; +import type { CompositorLayout } from '@/types/generated/compositor-types'; + +import type { UseCompositorLayersOptions } from './useCompositorLayers'; +import { useCompositorLayers } from './useCompositorLayers'; + +// ── Helpers ───────────────────────────────────────────────────────────────── + +const SESSION_ID = 'test-session'; +const NODE_ID = 'compositor'; + +/** Build a minimal params object with a single auto-layout layer (rect: null) + * and optionally a text overlay. */ +function makeParams(overrides: Record = {}): Record { + return { + width: 1280, + height: 720, + layers: { + in_0: { opacity: 1.0, z_index: 0 }, + }, + text_overlays: [], + image_overlays: [], + ...overrides, + }; +} + +/** Build a params object with a text overlay. */ +function makeParamsWithText( + textOverrides: Record = {}, + paramOverrides: Record = {} +): Record { + return makeParams({ + text_overlays: [ + { + id: 'text_0', + text: 'Hello', + color: [255, 255, 255, 255], + font_size: 32, + font_name: 'dejavu-sans', + rect: { x: 0, y: 0, width: 200, height: 40 }, + opacity: 1.0, + rotation_degrees: 0, + z_index: 100, + mirror_horizontal: false, + mirror_vertical: false, + ...textOverrides, + }, + ], + ...paramOverrides, + }); +} + +/** Build default hook options for Monitor view (has sessionId). */ +function monitorOptions( + overrides: Partial = {} +): UseCompositorLayersOptions { + return { + nodeId: NODE_ID, + sessionId: SESSION_ID, + canvasWidth: 1280, + canvasHeight: 720, + params: makeParams(), + onConfigChange: vi.fn(), + throttleMs: 100, + ...overrides, + }; +} + +/** Build a CompositorLayout representing server-resolved positions. */ +function makeServerLayout(overrides: Partial = {}): CompositorLayout { + return { + canvas_width: 1280, + canvas_height: 720, + layers: [ + { + id: 'in_0', + x: 160, + y: 0, + width: 960, + height: 720, + opacity: 1.0, + z_index: 0, + rotation_degrees: 0, + mirror_horizontal: false, + mirror_vertical: false, + crop_zoom: 1.0, + crop_x: 0.5, + crop_y: 0.5, + }, + ], + text_overlays: [], + image_overlays: [], + ...overrides, + }; +} + +/** Seed the Zustand store with a session so useServerLayoutSync finds it. */ +function seedStore() { + useSessionStore.getState().initSession(SESSION_ID, true); +} + +/** Push server view data into the store (simulates a WS view_data message). */ +function pushServerViewData(layout: CompositorLayout) { + useSessionStore.getState().updateNodeViewData(SESSION_ID, NODE_ID, layout); +} + +// ── Lifecycle ─────────────────────────────────────────────────────────────── + +afterEach(() => { + // Clean up store between tests + useSessionStore.getState().clearSession(SESSION_ID); +}); + +// ── Tests ─────────────────────────────────────────────────────────────────── + +describe('Monitor view data flow integration', () => { + it('server-resolved layer positions survive a params echo-back', () => { + seedStore(); + + // Layer in_0 has rect: null → parseLayers defaults to full canvas (0,0,1280,720). + // The server resolves it to an aspect-fit rect (160,0,960,720). + const opts = monitorOptions(); + const { result, rerender } = renderHook( + (props: UseCompositorLayersOptions) => useCompositorLayers(props), + { initialProps: opts } + ); + + // Initial state: parsed from params (full canvas fallback) + expect(result.current.layers[0].x).toBe(0); + expect(result.current.layers[0].width).toBe(1280); + + // Server view data arrives + act(() => pushServerViewData(makeServerLayout())); + + // Server-resolved positions applied + expect(result.current.layers[0].x).toBe(160); + expect(result.current.layers[0].width).toBe(960); + expect(result.current.layers[0].height).toBe(720); + + // Params echo-back: new reference, same content. + // This triggers the "sync from props" effect. + act(() => rerender({ ...opts, params: makeParams() })); + + // Server-resolved positions must survive the echo-back. + expect(result.current.layers[0].x).toBe(160); + expect(result.current.layers[0].width).toBe(960); + expect(result.current.layers[0].height).toBe(720); + }); + + it('server text overlay measurements survive a params echo-back', () => { + seedStore(); + + const opts = monitorOptions({ params: makeParamsWithText() }); + const { result, rerender } = renderHook( + (props: UseCompositorLayersOptions) => useCompositorLayers(props), + { initialProps: opts } + ); + + // Initial text overlay from params + expect(result.current.textOverlays[0].id).toBe('text_0'); + expect(result.current.textOverlays[0].measuredTextWidth).toBeUndefined(); + + // Server sends layout with text measurements + const layout = makeServerLayout({ + text_overlays: [ + { + id: 'text_0', + x: 50, + y: 100, + width: 280, + height: 45, + opacity: 1.0, + z_index: 100, + rotation_degrees: 0, + mirror_horizontal: false, + mirror_vertical: false, + measured_text_width: 275, + measured_text_height: 42, + }, + ], + }); + act(() => pushServerViewData(layout)); + + // Measurements applied + expect(result.current.textOverlays[0].measuredTextWidth).toBe(275); + expect(result.current.textOverlays[0].measuredTextHeight).toBe(42); + expect(result.current.textOverlays[0].x).toBe(50); + expect(result.current.textOverlays[0].y).toBe(100); + + // Params echo-back + act(() => rerender({ ...opts, params: makeParamsWithText() })); + + // Measurements and server positions must survive + expect(result.current.textOverlays[0].measuredTextWidth).toBe(275); + expect(result.current.textOverlays[0].measuredTextHeight).toBe(42); + expect(result.current.textOverlays[0].x).toBe(50); + expect(result.current.textOverlays[0].y).toBe(100); + }); + + it('config changes from params are picked up while preserving server geometry', () => { + seedStore(); + + const opts = monitorOptions({ params: makeParamsWithText() }); + const { result, rerender } = renderHook( + (props: UseCompositorLayersOptions) => useCompositorLayers(props), + { initialProps: opts } + ); + + // Server positions + const layout = makeServerLayout({ + text_overlays: [ + { + id: 'text_0', + x: 300, + y: 200, + width: 280, + height: 45, + opacity: 1.0, + z_index: 100, + rotation_degrees: 0, + mirror_horizontal: false, + mirror_vertical: false, + measured_text_width: 275, + measured_text_height: 42, + }, + ], + }); + act(() => pushServerViewData(layout)); + + expect(result.current.textOverlays[0].text).toBe('Hello'); + expect(result.current.textOverlays[0].x).toBe(300); + + // Params update with different text content (e.g. from another client) + act(() => + rerender({ + ...opts, + params: makeParamsWithText({ text: 'Updated text', font_size: 48 }), + }) + ); + + // Text content updated from params + expect(result.current.textOverlays[0].text).toBe('Updated text'); + expect(result.current.textOverlays[0].fontSize).toBe(48); + // Server position preserved + expect(result.current.textOverlays[0].x).toBe(300); + expect(result.current.textOverlays[0].y).toBe(200); + // Measurements preserved + expect(result.current.textOverlays[0].measuredTextWidth).toBe(275); + }); + + it('server-resolved opacity/rotation/zIndex survive params echo-back', () => { + seedStore(); + + const opts = monitorOptions(); + const { result, rerender } = renderHook( + (props: UseCompositorLayersOptions) => useCompositorLayers(props), + { initialProps: opts } + ); + + // Server resolves layer with specific opacity/rotation/z-index + const layout = makeServerLayout(); + layout.layers[0].opacity = 0.75; + layout.layers[0].rotation_degrees = 45; + layout.layers[0].z_index = 5; + act(() => pushServerViewData(layout)); + + expect(result.current.layers[0].opacity).toBe(0.75); + expect(result.current.layers[0].rotationDegrees).toBe(45); + expect(result.current.layers[0].zIndex).toBe(5); + + // Params echo-back (params still have default opacity=1, rotation=0, z_index=0) + act(() => rerender({ ...opts, params: makeParams() })); + + // Server-resolved values must survive + expect(result.current.layers[0].opacity).toBe(0.75); + expect(result.current.layers[0].rotationDegrees).toBe(45); + expect(result.current.layers[0].zIndex).toBe(5); + }); + + it('selecting different layers does not reset positions', () => { + seedStore(); + + const params = makeParams({ + layers: { + in_0: { opacity: 1.0, z_index: 0 }, + in_1: { + rect: { x: 0, y: 0, width: 320, height: 240 }, + opacity: 1.0, + z_index: 1, + }, + }, + }); + const opts = monitorOptions({ params }); + const { result } = renderHook( + (props: UseCompositorLayersOptions) => useCompositorLayers(props), + { initialProps: opts } + ); + + // Server resolves both layers at specific positions + const layout = makeServerLayout({ + layers: [ + { + id: 'in_0', + x: 160, + y: 0, + width: 960, + height: 720, + opacity: 1.0, + z_index: 0, + rotation_degrees: 0, + mirror_horizontal: false, + mirror_vertical: false, + crop_zoom: 1.0, + crop_x: 0.5, + crop_y: 0.5, + }, + { + id: 'in_1', + x: 800, + y: 400, + width: 320, + height: 240, + opacity: 1.0, + z_index: 1, + rotation_degrees: 0, + mirror_horizontal: false, + mirror_vertical: false, + crop_zoom: 1.0, + crop_x: 0.5, + crop_y: 0.5, + }, + ], + }); + act(() => pushServerViewData(layout)); + + // Verify server positions applied + const layer0 = result.current.layers.find((l) => l.id === 'in_0')!; + const layer1 = result.current.layers.find((l) => l.id === 'in_1')!; + expect(layer0.x).toBe(160); + expect(layer1.x).toBe(800); + + // Select layer 0 + act(() => result.current.selectLayer('in_0')); + expect(result.current.selectedLayerId).toBe('in_0'); + + // Switch to layer 1 + act(() => result.current.selectLayer('in_1')); + expect(result.current.selectedLayerId).toBe('in_1'); + + // Both layers should keep their server-resolved positions + const layer0After = result.current.layers.find((l) => l.id === 'in_0')!; + const layer1After = result.current.layers.find((l) => l.id === 'in_1')!; + expect(layer0After.x).toBe(160); + expect(layer0After.width).toBe(960); + expect(layer1After.x).toBe(800); + expect(layer1After.width).toBe(320); + }); + + it('selecting different layers does not reset text overlay size/position', () => { + seedStore(); + + const params = makeParams({ + layers: { + in_0: { opacity: 1.0, z_index: 0 }, + }, + text_overlays: [ + { + id: 'text_0', + text: 'Title', + color: [255, 255, 255, 255], + font_size: 32, + font_name: 'dejavu-sans', + rect: { x: 0, y: 0, width: 200, height: 40 }, + opacity: 1.0, + rotation_degrees: 0, + z_index: 100, + mirror_horizontal: false, + mirror_vertical: false, + }, + ], + }); + const opts = monitorOptions({ params }); + const { result } = renderHook( + (props: UseCompositorLayersOptions) => useCompositorLayers(props), + { initialProps: opts } + ); + + // Server resolves text overlay at a different position with measurements + const layout = makeServerLayout({ + text_overlays: [ + { + id: 'text_0', + x: 400, + y: 300, + width: 250, + height: 48, + opacity: 1.0, + z_index: 100, + rotation_degrees: 0, + mirror_horizontal: false, + mirror_vertical: false, + measured_text_width: 245, + measured_text_height: 44, + }, + ], + }); + act(() => pushServerViewData(layout)); + + // Verify server state + expect(result.current.textOverlays[0].x).toBe(400); + expect(result.current.textOverlays[0].y).toBe(300); + expect(result.current.textOverlays[0].measuredTextWidth).toBe(245); + + // Select the text overlay + act(() => result.current.selectLayer('text_0')); + expect(result.current.selectedLayerId).toBe('text_0'); + + // Switch focus to video layer + act(() => result.current.selectLayer('in_0')); + expect(result.current.selectedLayerId).toBe('in_0'); + + // Text overlay position and measurements must be unchanged + expect(result.current.textOverlays[0].x).toBe(400); + expect(result.current.textOverlays[0].y).toBe(300); + expect(result.current.textOverlays[0].width).toBe(250); + expect(result.current.textOverlays[0].measuredTextWidth).toBe(245); + expect(result.current.textOverlays[0].measuredTextHeight).toBe(44); + }); + + it('image overlay dataBase64 changes are picked up in Monitor view', () => { + seedStore(); + + const params = makeParams({ + image_overlays: [ + { + id: 'img_0', + data_base64: 'aW1hZ2UtZGF0YQ==', // "image-data" + rect: { x: 10, y: 20, width: 100, height: 80 }, + opacity: 1.0, + rotation_degrees: 0, + z_index: 50, + mirror_horizontal: false, + mirror_vertical: false, + }, + ], + }); + const opts = monitorOptions({ params }); + const { result, rerender } = renderHook( + (props: UseCompositorLayersOptions) => useCompositorLayers(props), + { initialProps: opts } + ); + + // Server resolves image at a different position + const layout = makeServerLayout({ + image_overlays: [ + { + id: 'img_0', + x: 500, + y: 300, + width: 100, + height: 80, + opacity: 0.9, + z_index: 50, + rotation_degrees: 0, + mirror_horizontal: false, + mirror_vertical: false, + }, + ], + }); + act(() => pushServerViewData(layout)); + + expect(result.current.imageOverlays[0].x).toBe(500); + expect(result.current.imageOverlays[0].y).toBe(300); + expect(result.current.imageOverlays[0].dataBase64).toBe('aW1hZ2UtZGF0YQ=='); + + // Another client changes the image data via params + const updatedParams = makeParams({ + image_overlays: [ + { + id: 'img_0', + data_base64: 'bmV3LWltYWdl', // "new-image" + rect: { x: 10, y: 20, width: 100, height: 80 }, + opacity: 1.0, + rotation_degrees: 0, + z_index: 50, + mirror_horizontal: false, + mirror_vertical: false, + }, + ], + }); + act(() => rerender({ ...opts, params: updatedParams })); + + // dataBase64 must be updated (config field) + expect(result.current.imageOverlays[0].dataBase64).toBe('bmV3LWltYWdl'); + // Server-resolved position must be preserved + expect(result.current.imageOverlays[0].x).toBe(500); + expect(result.current.imageOverlays[0].y).toBe(300); + // Server-resolved opacity must be preserved + expect(result.current.imageOverlays[0].opacity).toBe(0.9); + }); + + it('Design view (no sessionId) still uses parsed positions as source of truth', () => { + // This is the control test — Design view should NOT preserve existing geometry. + const opts: UseCompositorLayersOptions = { + nodeId: NODE_ID, + // No sessionId — Design view + canvasWidth: 1280, + canvasHeight: 720, + params: makeParamsWithText(), + onConfigChange: vi.fn(), + throttleMs: 100, + }; + + const { result, rerender } = renderHook( + (props: UseCompositorLayersOptions) => useCompositorLayers(props), + { initialProps: opts } + ); + + // Text overlay at parsed position + expect(result.current.textOverlays[0].x).toBe(0); + expect(result.current.textOverlays[0].y).toBe(0); + + // Params update with new position + act(() => + rerender({ + ...opts, + params: makeParamsWithText({ rect: { x: 100, y: 200, width: 300, height: 50 } }), + }) + ); + + // Design view: position should update from params (not preserved) + expect(result.current.textOverlays[0].x).toBe(100); + expect(result.current.textOverlays[0].y).toBe(200); + }); +}); diff --git a/ui/src/hooks/useCompositorLayers.ts b/ui/src/hooks/useCompositorLayers.ts index 19050b39..6a185432 100644 --- a/ui/src/hooks/useCompositorLayers.ts +++ b/ui/src/hooks/useCompositorLayers.ts @@ -161,6 +161,13 @@ export const useCompositorLayers = ( }, [layers]); // ── Sync from props ───────────────────────────────────────────────────── + // In Monitor view (sessionId is set), the server's view data is the source + // of truth for geometry (x, y, width, height). The "sync from props" effect + // must NOT overwrite server-resolved positions with config-parsed ones, + // otherwise a params echo-back from the server would clobber the accurate + // layout that useServerLayoutSync applied. + const isMonitorView = !!sessionId; + useEffect(() => { if (dragStateRef.current) return; const parsed = parseLayers(params, canvasWidth, canvasHeight); @@ -168,7 +175,8 @@ export const useCompositorLayers = ( const merged = mergeOverlayState( layersRef.current, parsed, - (a, b) => a.cropZoom !== b.cropZoom || a.cropX !== b.cropX || a.cropY !== b.cropY + (a, b) => a.cropZoom !== b.cropZoom || a.cropX !== b.cropX || a.cropY !== b.cropY, + isMonitorView ); if (merged !== layersRef.current) setLayers(merged); @@ -180,11 +188,19 @@ export const useCompositorLayers = ( a.text !== b.text || a.fontSize !== b.fontSize || a.fontName !== b.fontName || - a.color.some((v, i) => v !== b.color[i]) + a.color.some((v, i) => v !== b.color[i]), + isMonitorView + ) + ); + setImageOverlays((cur) => + mergeOverlayState( + cur, + parseImageOverlays(params), + (a, b) => a.dataBase64 !== b.dataBase64, + isMonitorView ) ); - setImageOverlays((cur) => mergeOverlayState(cur, parseImageOverlays(params))); - }, [params, canvasWidth, canvasHeight]); + }, [params, canvasWidth, canvasHeight, isMonitorView]); // ── Server-driven layout (Monitor view only) ─────────────────────────── useServerLayoutSync( diff --git a/ui/src/stores/sessionStore.edge-cases.test.ts b/ui/src/stores/sessionStore.edge-cases.test.ts index c536d5fe..03593945 100644 --- a/ui/src/stores/sessionStore.edge-cases.test.ts +++ b/ui/src/stores/sessionStore.edge-cases.test.ts @@ -229,6 +229,101 @@ describe('sessionStore edge cases', () => { }); }); + describe('setPipeline view_data extraction', () => { + it('should extract view_data into nodeViewData on initial load', () => { + const sessionId = TEST_SESSION_ID; + const pipeline: Pipeline = { + name: null, + description: null, + mode: 'dynamic', + nodes: { + compositor: { + kind: 'video::compositor', + params: { width: 1280, height: 720 }, + state: 'Running', + }, + }, + connections: [], + view_data: { + compositor: { layers: { in_0: { x: 0, y: 60, width: 1280, height: 600 } } }, + }, + }; + + useSessionStore.getState().setPipeline(sessionId, pipeline); + + const session = useSessionStore.getState().getSession(sessionId); + expect(session?.nodeViewData).toBeDefined(); + expect(session?.nodeViewData.compositor).toEqual({ + layers: { in_0: { x: 0, y: 60, width: 1280, height: 600 } }, + }); + }); + + it('should merge view_data with existing nodeViewData', () => { + const sessionId = TEST_SESSION_ID; + + // First pipeline sets initial view data + useSessionStore.getState().setPipeline(sessionId, { + name: null, + description: null, + mode: 'dynamic', + nodes: {}, + connections: [], + view_data: { nodeA: { key: 'original' } }, + }); + + // Second pipeline adds more view data + useSessionStore.getState().setPipeline(sessionId, { + name: null, + description: null, + mode: 'dynamic', + nodes: {}, + connections: [], + view_data: { nodeB: { key: 'new' } }, + }); + + const session = useSessionStore.getState().getSession(sessionId); + expect(session?.nodeViewData.nodeA).toEqual({ key: 'original' }); + expect(session?.nodeViewData.nodeB).toEqual({ key: 'new' }); + }); + + it('should handle null view_data gracefully', () => { + const sessionId = TEST_SESSION_ID; + const pipeline: Pipeline = { + name: null, + description: null, + mode: 'dynamic', + nodes: {}, + connections: [], + }; + + useSessionStore.getState().setPipeline(sessionId, pipeline); + + const session = useSessionStore.getState().getSession(sessionId); + expect(session?.nodeViewData).toEqual({}); + }); + + it('should extract view_data in batchSetPipelines', () => { + const pipelines = [ + { + sessionId: 'session-a', + pipeline: { + name: null, + description: null, + mode: 'dynamic' as const, + nodes: {}, + connections: [], + view_data: { comp: { layers: { in_0: { x: 10 } } } }, + }, + }, + ]; + + useSessionStore.getState().batchSetPipelines(pipelines); + + const session = useSessionStore.getState().getSession('session-a'); + expect(session?.nodeViewData.comp).toEqual({ layers: { in_0: { x: 10 } } }); + }); + }); + describe('Pipeline Updates with Missing Nodes', () => { beforeEach(() => { const pipeline: Pipeline = { diff --git a/ui/src/stores/sessionStore.ts b/ui/src/stores/sessionStore.ts index 6fa9f426..bc5cfb8d 100644 --- a/ui/src/stores/sessionStore.ts +++ b/ui/src/stores/sessionStore.ts @@ -108,11 +108,18 @@ export const useSessionStore = create((set, get) => ({ }); } + // Extract view data snapshot (e.g. compositor resolved layout) so + // useServerLayoutSync finds it immediately on mount. + const incomingViewData = + pipeline.view_data && typeof pipeline.view_data === 'object' + ? (pipeline.view_data as Record) + : {}; + newSessions.set(sessionId, { pipeline, nodeStates: session ? { ...session.nodeStates, ...nodeStates } : nodeStates, nodeStats: session?.nodeStats ?? {}, - nodeViewData: session?.nodeViewData ?? {}, + nodeViewData: { ...(session?.nodeViewData ?? {}), ...incomingViewData }, isConnected: session?.isConnected ?? false, }); return { sessions: newSessions }; @@ -326,11 +333,17 @@ export const useSessionStore = create((set, get) => ({ } } + // Extract view data snapshot so useServerLayoutSync finds it on mount. + const incomingViewData = + pipeline.view_data && typeof pipeline.view_data === 'object' + ? (pipeline.view_data as Record) + : {}; + newSessions.set(sessionId, { pipeline, nodeStates: session ? { ...session.nodeStates, ...nodeStates } : nodeStates, nodeStats: session?.nodeStats ?? {}, - nodeViewData: session?.nodeViewData ?? {}, + nodeViewData: { ...(session?.nodeViewData ?? {}), ...incomingViewData }, isConnected: session?.isConnected ?? false, }); } diff --git a/ui/src/types/generated/api-types.ts b/ui/src/types/generated/api-types.ts index bd286ca4..9974cbf8 100644 --- a/ui/src/types/generated/api-types.ts +++ b/ui/src/types/generated/api-types.ts @@ -351,7 +351,12 @@ export type Node = { kind: string, params: JsonValue, */ state: NodeState | null, }; -export type Pipeline = { name: string | null, description: string | null, mode: EngineMode, nodes: Record, connections: Array, }; +export type Pipeline = { name: string | null, description: string | null, mode: EngineMode, nodes: Record, connections: Array, +/** + * Resolved per-node view data (e.g., compositor layout). + * Only populated in API responses; absent from pipeline definitions. + */ +view_data?: Record | null, }; export type SamplePipeline = { id: string, name: string, description: string, yaml: string, is_system: boolean, mode: string, /**