Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ui/bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@
"@emotion/react": "^11.14.0",
"@emotion/styled": "^11.14.1",
"@moq/hang": "^0.2.0",
"@moq/signals": "^0.1.3",
"@moq/publish": "^0.2.3",
"@moq/signals": "^0.1.3",
"@moq/watch": "^0.2.3",
"@radix-ui/react-alert-dialog": "^1.1.15",
"@radix-ui/react-checkbox": "^1.3.3",
Expand All @@ -78,6 +78,7 @@
"@uiw/react-codemirror": "^4.25.8",
"@xyflow/react": "^12.10.1",
"codemirror": "^6.0.2",
"jotai": "^2.18.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 CONTRIBUTING.md says 'Zustand for global state' but Jotai is now added alongside it

CONTRIBUTING.md states "Zustand for global state, React Query for server state" as a TypeScript convention. This PR introduces Jotai as a third state management library. While the Jotai usage is well-scoped (per-compositor-node atom families for component-level state, distinct from Zustand's global session/pipeline state), having three state libraries adds cognitive overhead for contributors. The PR's compositorAtoms.ts module doc describes the planned evolution toward per-field atoms, which would further leverage Jotai's strengths. Consider updating CONTRIBUTING.md to reflect the new convention (e.g., "Zustand for global state, Jotai for component-scoped state, React Query for server state").

Staging: Open in Devin

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion. I'd defer this to the maintainer's preference — updating CONTRIBUTING.md to document the convention (e.g. "Zustand for global state, Jotai for component-scoped high-frequency state, React Query for server state") makes sense but should be a deliberate decision rather than a side-effect of this PR.

"js-yaml": "^4.1.1",
"lodash-es": "^4.17.23",
"lucide-react": "^0.577.0",
Expand Down
66 changes: 35 additions & 31 deletions ui/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: MPL-2.0

import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { Provider as JotaiProvider } from 'jotai';
import React, { useState, useEffect } from 'react';
import { BrowserRouter, Routes, Route, Navigate } from 'react-router-dom';

Expand All @@ -14,6 +15,7 @@ import { ToastProvider } from './context/ToastContext';
import Layout from './Layout';
import { fetchAuthMe } from './services/auth';
import { initializePermissions } from './services/permissions';
import { jotaiStore } from './stores/jotaiStore';
import { ensureSchemasLoaded } from './stores/schemaStore';
import { getBasePathname } from './utils/baseHref';
import { getLogger } from './utils/logger';
Expand Down Expand Up @@ -98,39 +100,41 @@ const App: React.FC = () => {

return (
<ErrorBoundary>
<QueryClientProvider client={queryClient}>
<ThemeProvider>
<ToastProvider>
<TooltipProvider delayDuration={300} skipDelayDuration={200}>
<BrowserRouter basename={getBasePathname()}>
<Routes>
<Route
path="/login"
element={<LoginView onLoggedIn={() => setRequiresLogin(false)} />}
/>
<Route
path="/"
element={requiresLogin ? <Navigate to="/login" replace /> : <Layout />}
>
<Route index element={<Navigate to="/design" replace />} />
<Route path="design" element={<DesignView />} />
<Route path="monitor" element={<MonitorView />} />
<Route path="convert" element={<ConvertView />} />
<Route path="stream" element={<StreamView />} />
<Route path="admin" element={<Navigate to="/admin/plugins" replace />} />
<JotaiProvider store={jotaiStore}>
<QueryClientProvider client={queryClient}>
<ThemeProvider>
<ToastProvider>
<TooltipProvider delayDuration={300} skipDelayDuration={200}>
<BrowserRouter basename={getBasePathname()}>
<Routes>
<Route
path="admin/plugins"
element={<Navigate to="/admin/plugins/installed" replace />}
path="/login"
element={<LoginView onLoggedIn={() => setRequiresLogin(false)} />}
/>
<Route path="admin/plugins/:tab" element={<PluginsView />} />
<Route path="admin/tokens" element={<TokensView />} />
</Route>
</Routes>
</BrowserRouter>
</TooltipProvider>
</ToastProvider>
</ThemeProvider>
</QueryClientProvider>
<Route
path="/"
element={requiresLogin ? <Navigate to="/login" replace /> : <Layout />}
>
<Route index element={<Navigate to="/design" replace />} />
<Route path="design" element={<DesignView />} />
<Route path="monitor" element={<MonitorView />} />
<Route path="convert" element={<ConvertView />} />
<Route path="stream" element={<StreamView />} />
<Route path="admin" element={<Navigate to="/admin/plugins" replace />} />
<Route
path="admin/plugins"
element={<Navigate to="/admin/plugins/installed" replace />}
/>
<Route path="admin/plugins/:tab" element={<PluginsView />} />
<Route path="admin/tokens" element={<TokensView />} />
</Route>
</Routes>
</BrowserRouter>
</TooltipProvider>
</ToastProvider>
</ThemeProvider>
</QueryClientProvider>
</JotaiProvider>
</ErrorBoundary>
);
};
Expand Down
76 changes: 4 additions & 72 deletions ui/src/components/CompositorCanvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const noopResizeStart = (() => {}) as (
// ── Main canvas ─────────────────────────────────────────────────────────────

export interface CompositorCanvasProps {
nodeId: string;
canvasWidth: number;
canvasHeight: number;
layers: LayerState[];
Expand All @@ -61,78 +62,9 @@ export interface CompositorCanvasProps {
disabled?: boolean;
}

/**
* Custom comparator for CompositorCanvas memo.
*
* Video layers use zero-render DOM updates for opacity/rotation during
* slider drags, so we only compare geometry fields for those.
* Text and image overlays update via React state (not the zero-render path),
* so we use reference equality — a new array reference means content changed.
*
* Known limitation: server-pushed opacity/rotation changes from other clients
* won't trigger a canvas re-render until the next geometry change. This is
* acceptable for the single-client case (echo-backs carry the same values).
* The Jotai migration will resolve this by giving each field its own atom.
*/
function areCanvasPropsEqual(prev: CompositorCanvasProps, next: CompositorCanvasProps): boolean {
// Scalar props
if (prev.canvasWidth !== next.canvasWidth) return false;
if (prev.canvasHeight !== next.canvasHeight) return false;
if (prev.selectedLayerId !== next.selectedLayerId) return false;
if (prev.disabled !== next.disabled) return false;

// Callback identity (stable via useCallback in parent)
if (prev.onSelectLayer !== next.onSelectLayer) return false;
if (prev.onLayerPointerDown !== next.onLayerPointerDown) return false;
if (prev.onResizePointerDown !== next.onResizePointerDown) return false;
if (prev.onTextFocusRequest !== next.onTextFocusRequest) return false;
if (prev.onLayerContextMenu !== next.onLayerContextMenu) return false;

// Ref identity (stable MutableRefObjects)
if (prev.layerRefs !== next.layerRefs) return false;
if (prev.snapGuideRefs !== next.snapGuideRefs) return false;

// Video layers: compare geometry only, skip appearance (opacity, rotation, mirror)
// since those use the zero-render DOM path
if (!layerArrayGeometryEqual(prev.layers, next.layers)) return false;

// Text/image overlays: use reference equality since they update via React state
// and content changes (text, font, color, opacity, rotation) need to re-render
if (prev.textOverlays !== next.textOverlays) return false;
if (prev.imageOverlays !== next.imageOverlays) return false;

return true;
}

/**
* Compare layer arrays by geometry + mirror fields, skipping opacity/rotation
* (those use the zero-render DOM path during slider drags).
* Mirror is included because updateLayerMirror updates via React state, not DOM.
*/
function layerArrayGeometryEqual(a: readonly LayerState[], b: readonly LayerState[]): boolean {
if (a.length !== b.length) return false;
for (let i = 0; i < a.length; i++) {
const la = a[i];
const lb = b[i];
if (
la.id !== lb.id ||
la.x !== lb.x ||
la.y !== lb.y ||
la.width !== lb.width ||
la.height !== lb.height ||
la.zIndex !== lb.zIndex ||
la.visible !== lb.visible ||
la.mirrorHorizontal !== lb.mirrorHorizontal ||
la.mirrorVertical !== lb.mirrorVertical
) {
return false;
}
}
return true;
}

export const CompositorCanvas: React.FC<CompositorCanvasProps> = React.memo(
({
nodeId,
canvasWidth,
canvasHeight,
layers,
Expand Down Expand Up @@ -257,6 +189,7 @@ export const CompositorCanvas: React.FC<CompositorCanvasProps> = React.memo(
{layers.map((layer, i) => (
<VideoLayer
key={layer.id}
nodeId={nodeId}
layer={layer}
index={i}
isSelected={selectedLayerId === layer.id}
Expand Down Expand Up @@ -305,8 +238,7 @@ export const CompositorCanvas: React.FC<CompositorCanvasProps> = React.memo(
</CanvasInner>
</CanvasOuter>
);
},
areCanvasPropsEqual
}
);

CompositorCanvas.displayName = 'CompositorCanvas';
29 changes: 26 additions & 3 deletions ui/src/components/compositorCanvasLayers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import type {
ResizeHandle,
} from '@/hooks/useCompositorLayers';
import { friendlyLabel } from '@/nodes/compositorNodeParts';
import { compositorLayerOpacityAtom, compositorLayerRotationAtom } from '@/stores/compositorAtoms';
import { jotaiStore } from '@/stores/jotaiStore';

import {
LayerBox,
Expand Down Expand Up @@ -123,13 +125,34 @@ ResizeHandles.displayName = 'ResizeHandles';
// ── Video input layer ───────────────────────────────────────────────────────

export const VideoLayer: React.FC<{
nodeId: string;
layer: LayerState;
index: number;
isSelected: boolean;
onPointerDown: (layerId: string, e: React.PointerEvent) => void;
onResizeStart: (layerId: string, handle: ResizeHandle, e: React.PointerEvent) => void;
layerRef: (el: HTMLDivElement | null) => void;
}> = React.memo(({ layer, index, isSelected, onPointerDown, onResizeStart, layerRef }) => {
}> = React.memo(({ nodeId, layer, index, isSelected, onPointerDown, onResizeStart, layerRef }) => {
// Read opacity/rotation from per-layer atoms so this component
// re-renders only when THIS layer's appearance changes, not when
// any layer in the array changes.
const atomKey = `${nodeId}:${layer.id}`;
const opacity = jotaiStore.get(compositorLayerOpacityAtom(atomKey));
const rotationDegrees = jotaiStore.get(compositorLayerRotationAtom(atomKey));
const [, forceRender] = useState(0);
useEffect(() => {
const unsub1 = jotaiStore.sub(compositorLayerOpacityAtom(atomKey), () => {
forceRender((c) => c + 1);
});
const unsub2 = jotaiStore.sub(compositorLayerRotationAtom(atomKey), () => {
forceRender((c) => c + 1);
});
return () => {
unsub1();
unsub2();
};
}, [atomKey]);

const handlePointerDown = useCallback(
(e: React.PointerEvent) => {
onPointerDown(layer.id, e);
Expand All @@ -148,9 +171,9 @@ export const VideoLayer: React.FC<{
aria-label={`Video layer: ${layer.id}`}
style={layerBoxStyle(layer.x, layer.y, layer.width, layer.height, {
visible: layer.visible,
opacity: layer.opacity,
opacity,
zIndex: layer.zIndex,
rotationDegrees: layer.rotationDegrees,
rotationDegrees,
mirrorHorizontal: layer.mirrorHorizontal,
mirrorVertical: layer.mirrorVertical,
borderColor,
Expand Down
Loading
Loading