From b7fe69a05ae83cd7b57685a277514cf0bc546345 Mon Sep 17 00:00:00 2001 From: dualchimerra Date: Sun, 27 Sep 2026 18:20:27 +0200 Subject: [PATCH] fix(web): smoother image viewing and live monitor updates - ImagePreviewModal renders through a portal to . It was mounted inside queue cards; finished cards have opacity .72, so the lightbox came out translucent, could sit under later cards and forced the whole page to be composited behind the full-size image every frame. Also locks body scroll, binds keydown once, and preloads the prev/next image. - ZoomableImage keeps the current image painted while the next one loads (no blank flash when flipping), decodes async, refits in a layout effect. - useMonitorProgress is now a shared per-task store: Topbar, Sidebar, Queue, QueueDetail and MonitorDashboard used to each fetch the full multi-MB snapshot and merge every 1s delta into their own 50k-point arrays. - TaskSampleStrip and the dashboard's SampleViewer are memoized, so the per-second training ticks no longer re-render them (or an open lightbox). - MonitorDashboard: chart series are memoized; fixes the learning-rate chart being rendered twice and the "d" chart floating outside a card - it now shows in its own card only when the optimizer reports d. Co-Authored-By: Claude Opus 5.5 --- .../web/src/components/ImagePreviewModal.tsx | 44 +++++- .../web/src/components/MonitorDashboard.tsx | 51 ++++--- studio/web/src/components/TaskSampleStrip.tsx | 12 +- studio/web/src/components/ZoomableImage.tsx | 10 +- studio/web/src/lib/useEventStream.ts | 21 +++ studio/web/src/lib/useMonitorProgress.ts | 126 ++++++++++++------ studio/web/src/lib/useZoomPan.ts | 7 +- studio/web/src/pages/QueueDetail.tsx | 5 +- 8 files changed, 195 insertions(+), 81 deletions(-) diff --git a/studio/web/src/components/ImagePreviewModal.tsx b/studio/web/src/components/ImagePreviewModal.tsx index 9be8804..c834d21 100644 --- a/studio/web/src/components/ImagePreviewModal.tsx +++ b/studio/web/src/components/ImagePreviewModal.tsx @@ -1,4 +1,5 @@ -import { useEffect } from 'react' +import { useEffect, useRef } from 'react' +import { createPortal } from 'react-dom' import ZoomableImage from './ZoomableImage' @@ -23,6 +24,9 @@ interface Props { onAccept?: () => void onDelete?: () => void shortcutHint?: string + /** URLs to warm up in the background (usually the prev/next image), so flipping + * with the arrows shows an already-decoded image instead of loading from scratch. */ + preload?: string[] } export default function ImagePreviewModal({ @@ -41,9 +45,15 @@ export default function ImagePreviewModal({ onAccept, onDelete, shortcutHint, + preload, }: Props) { + // Handlers live in a ref so the keydown listener binds once: callers pass fresh + // inline closures on every render (and some re-render on every training tick). + const keysRef = useRef({ hasPrev, hasNext, onPrev, onNext, onClose, onAccept, onDelete }) + keysRef.current = { hasPrev, hasNext, onPrev, onNext, onClose, onAccept, onDelete } useEffect(() => { const handler = (e: KeyboardEvent) => { + const { hasPrev, hasNext, onPrev, onNext, onClose, onAccept, onDelete } = keysRef.current if (e.key === 'Escape') { e.preventDefault() onClose() @@ -63,11 +73,35 @@ export default function ImagePreviewModal({ } window.addEventListener('keydown', handler) return () => window.removeEventListener('keydown', handler) - }, [hasPrev, hasNext, onPrev, onNext, onClose, onAccept, onDelete]) + }, []) + + // The page behind stays put while the lightbox is open (no scroll-through on wheel / touch). + useEffect(() => { + const prev = document.body.style.overflow + document.body.style.overflow = 'hidden' + return () => { document.body.style.overflow = prev } + }, []) + + // Warm up the neighbours; keyed by the joined URLs so a re-render with the same list is a no-op. + const preloadKey = (preload ?? []).filter(Boolean).join('\n') + useEffect(() => { + if (!preloadKey) return + const t = window.setTimeout(() => { + for (const u of preloadKey.split('\n')) { + const img = new Image() + img.decoding = 'async' + img.src = u + } + }, 150) + return () => window.clearTimeout(t) + }, [preloadKey]) const counter = index != null && total != null ? `${index + 1} / ${total}` : null - return ( + // Portaled to : callers mount this deep inside cards, and an ancestor with opacity / + // transform (e.g. a finished queue card at opacity .72) would otherwise make the fixed + // overlay translucent, trap it under later siblings and composite the page behind every frame. + return createPortal(
{shortcutHint}
} )} - + , + document.body, ) } @@ -152,6 +187,7 @@ function SplitPane({ src={src} alt={label ?? altFallback ?? 'preview'} onClick={(e) => e.stopPropagation()} + decoding="async" className="max-w-full max-h-full object-contain" /> diff --git a/studio/web/src/components/MonitorDashboard.tsx b/studio/web/src/components/MonitorDashboard.tsx index b77fdea..e06c4b5 100644 --- a/studio/web/src/components/MonitorDashboard.tsx +++ b/studio/web/src/components/MonitorDashboard.tsx @@ -4,7 +4,7 @@ * Data source: GET /api/state?task_id=N fetches a downsampled snapshot + SSE monitor_progress * goes through the useMonitorProgress hook for delta merging (PR #37's incremental protocol). */ -import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' +import { memo, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { api, type EvalJobInfo, type EvalMetricResult, type EvalMetricState, type LoraCkpt, type MonitorState } from '../api/client' import { evalProgressFromResults } from '../lib/useEvalProgress' import { useMonitorProgress } from '../lib/useMonitorProgress' @@ -840,7 +840,7 @@ function sampleMarks(s: { path: string; step?: number }): { step: number | null; } } -function SampleViewer({ samples, taskId }: { +const SampleViewer = memo(function SampleViewer({ samples, taskId }: { samples: Array<{ path: string; step?: number }> taskId: number }) { @@ -956,7 +956,7 @@ function SampleViewer({ samples, taskId }: { key={fullUrl} src={fullUrl} alt="sample preview" - loading="lazy" + decoding="async" onClick={() => setZoomOpen(true)} className="absolute inset-0 w-full h-full object-contain cursor-zoom-in" /> @@ -985,11 +985,14 @@ function SampleViewer({ samples, taskId }: { onClose={() => setZoomOpen(false)} onPrev={() => setActive((i) => Math.max(0, i - 1))} onNext={() => setActive((i) => Math.min(list.length - 1, i + 1))} + preload={[list[active - 1], list[active + 1]] + .filter((x): x is { path: string; step?: number } => !!x) + .map((x) => api.sampleImageUrl(x.path.split(/[\\/]/).pop() ?? x.path, taskId))} /> )} ) -} +}) // ── Main Component ───────────────────────────────────────────────────────── @@ -1056,6 +1059,15 @@ export default function MonitorDashboard({ taskId }: { taskId: number }) { const vramTotal = state?.vram_total_gb const vramTone = vram && vramTotal ? (vram / vramTotal > 0.85 ? 'warn' : 'ok') as 'ok' | 'warn' : undefined + // Full raw series (no more slice(-60)) -- SeriesChart downsamples evenly to 600 points internally when rendering. + // Memoized: the arrays run up to 50k points and the dashboard re-renders on every SSE delta, so the + // charts only rebuild when their own series actually changed. + const lossSeries = useMemo(() => losses.map((l) => ({ step: l.step, value: l.loss })), [losses]) + const lrSeries = useMemo(() => lrHistory.map((l) => ({ step: l.step, value: l.lr })), [lrHistory]) + const dSeries = useMemo(() => optimizerMetricsHistory + .filter((m) => typeof m.d === 'number') + .map((m) => ({ step: m.step, value: m.d as number })), [optimizerMetricsHistory]) + if (!state && !connected) { return (
@@ -1064,13 +1076,6 @@ export default function MonitorDashboard({ taskId }: { taskId: number }) { ) } - // Full raw series (no more slice(-60)) -- SeriesChart downsamples evenly to 600 points internally when rendering - const lrSeries = lrHistory.map((l) => ({ step: l.step, value: l.lr })) - const dSeries = optimizerMetricsHistory - .map((m) => ({ step: m.step, d: m.d })) - .filter((m): m is { step: number; d: number } => typeof m.d === 'number') - .map((m) => ({ step: m.step, value: m.d })) - return (
{/* Connection status + progress */} @@ -1165,7 +1170,7 @@ export default function MonitorDashboard({ taskId }: { taskId: number }) {
({ step: l.step, value: l.loss }))} + data={lossSeries} rawColor="color-mix(in srgb, var(--fg-secondary) 35%, transparent)" smoothColor="var(--accent)" fillColor="var(--accent-soft)" @@ -1190,19 +1195,11 @@ export default function MonitorDashboard({ taskId }: { taskId: number }) { />
-
-
- learning rate - -
- + {/* d value (Prodigy / D-Adaptation etc.) -- only when the optimizer reports it. */} + {dSeries.length > 0 && ( +
+
+ d
-
+ )} + + ) } diff --git a/studio/web/src/components/TaskSampleStrip.tsx b/studio/web/src/components/TaskSampleStrip.tsx index 9df8720..8c8db4f 100644 --- a/studio/web/src/components/TaskSampleStrip.tsx +++ b/studio/web/src/components/TaskSampleStrip.tsx @@ -15,7 +15,7 @@ * when the delta actually contains appended_samples (training emits an image every few dozen steps, * no need to jitter on every single delta). */ -import { useCallback, useEffect, useRef, useState } from 'react' +import { memo, useCallback, useEffect, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' import { api, type TaskSample } from '../api/client' @@ -40,7 +40,11 @@ function shortMarks(s: TaskSample): string { return parts.join(' · ') } -export default function TaskSampleStrip({ taskId, live = false }: { +// Memoized: the queue page re-renders on every monitor_progress delta of the running task; +// the strip (and its open lightbox) only depends on taskId / live. +export default memo(TaskSampleStrip) + +function TaskSampleStrip({ taskId, live = false }: { taskId: number /** running task -> subscribes to monitor_progress, auto-appends new images. */ live?: boolean @@ -159,6 +163,7 @@ export default function TaskSampleStrip({ taskId, live = false }: { src={api.sampleImageUrl(s.filename, taskId, THUMB_REQ)} alt="" loading="lazy" + decoding="async" /> {caption && {caption}} @@ -180,6 +185,9 @@ export default function TaskSampleStrip({ taskId, live = false }: { onClose={() => setZoomIdx(null)} onPrev={() => setZoomIdx((i) => Math.max(0, (i ?? 0) - 1))} onNext={() => setZoomIdx((i) => Math.min(items.length - 1, (i ?? 0) + 1))} + preload={[items[zoomIdx - 1], items[zoomIdx + 1]] + .filter((x): x is TaskSample => !!x) + .map((x) => api.sampleImageUrl(x.filename, taskId))} /> )} diff --git a/studio/web/src/components/ZoomableImage.tsx b/studio/web/src/components/ZoomableImage.tsx index ba1e698..78c963e 100644 --- a/studio/web/src/components/ZoomableImage.tsx +++ b/studio/web/src/components/ZoomableImage.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react' +import { useLayoutEffect, useState } from 'react' import { useTranslation } from 'react-i18next' import { useZoomPan } from '../lib/useZoomPan' @@ -36,13 +36,17 @@ export default function ZoomableImage({ readout?: boolean }) { const { t } = useTranslation() + // Not reset when src changes: the browser keeps painting the current image until the + // next one has loaded and decoded, so flipping through a sequence never flashes an + // empty viewport. onLoad then stores the new size and we refit before the frame paints. const [nat, setNat] = useState<{ w: number; h: number } | null>(null) - useEffect(() => { setNat(null) }, [src]) const zp = useZoomPan({ contentW: nat?.w ?? 0, contentH: nat?.h ?? 0, primaryButtonPans: true, }) + const { fit } = zp + useLayoutEffect(() => { if (nat) fit() }, [nat, fit]) return (
{ const w = e.currentTarget.naturalWidth const h = e.currentTarget.naturalHeight @@ -75,6 +80,7 @@ export default function ZoomableImage({ transformOrigin: '0 0', maxWidth: 'none', maxHeight: 'none', + willChange: 'transform', visibility: nat ? 'visible' : 'hidden', }} /> diff --git a/studio/web/src/lib/useEventStream.ts b/studio/web/src/lib/useEventStream.ts index e98708d..74820e7 100644 --- a/studio/web/src/lib/useEventStream.ts +++ b/studio/web/src/lib/useEventStream.ts @@ -58,6 +58,27 @@ function _maybeClose(): void { } } +/** + * Non-hook subscription to the same shared stream, for module-level stores that + * outlive any single component. onOpen fires on every (re)connect, and right away + * when the shared connection is already open. Returns the unsubscribe function. + */ +export function subscribeEventStream(onEvent: Listener, onOpen?: OpenListener): () => void { + if (typeof EventSource === 'undefined') return () => {} + const openHandler: OpenListener = () => onOpen?.() + _listeners.add(onEvent) + _openListeners.add(openHandler) + _ensureOpen() + if (_es && _es.readyState === EventSource.OPEN) { + try { onOpen?.() } catch { /* ignore */ } + } + return () => { + _listeners.delete(onEvent) + _openListeners.delete(openHandler) + _maybeClose() + } +} + /** * Subscribes to the /api/events SSE stream. The callback fires once per event. * Reconnects automatically on disconnect (this is just EventSource's built-in behavior). diff --git a/studio/web/src/lib/useMonitorProgress.ts b/studio/web/src/lib/useMonitorProgress.ts index 91f2754..e3bfde8 100644 --- a/studio/web/src/lib/useMonitorProgress.ts +++ b/studio/web/src/lib/useMonitorProgress.ts @@ -15,9 +15,9 @@ * cross-task views, which don't subscribe when nothing is running). Changing * taskId clears state and refetches the snapshot. */ -import { useCallback, useEffect, useRef, useState } from 'react' +import { useCallback, useSyncExternalStore } from 'react' import { api, type MonitorState } from '../api/client' -import { useEventStream } from './useEventStream' +import { subscribeEventStream } from './useEventStream' interface MonitorProgressDelta { step?: number @@ -100,53 +100,93 @@ export interface MonitorProgress { refetch: () => Promise } -export function useMonitorProgress(taskId: number | null): MonitorProgress { - const [state, setState] = useState(null) - const [connected, setConnected] = useState(false) - const lastUpdateRef = useRef(0) - // Store taskId in a ref for the event handler's closure, to avoid resubscribing SSE on every taskId change - const taskIdRef = useRef(taskId) - taskIdRef.current = taskId - - const refetch = useCallback(async () => { - const tid = taskIdRef.current - if (tid == null) return +// ── shared per-task store ─────────────────────────────────────────────────── +// Topbar, Sidebar, the queue page, the task page and its dashboard all watch the +// same running task at once. Each used to fetch its own full snapshot (tens of +// thousands of loss points, several MB of JSON) and merge every delta into its own +// copy; now there is one entry per task: one snapshot fetch, one merge per delta, +// and every subscriber reads the same object. Topbar / Sidebar stay mounted across +// pages, so the entry survives navigation while the task runs. + +interface Snap { state: MonitorState | null; connected: boolean } +interface Entry { + snap: Snap + lastUpdate: number + subs: Set<() => void> + inflight: Promise | null + unsubscribe: () => void +} + +const _entries = new Map() +const IDLE: Snap = { state: null, connected: false } + +function _set(e: Entry, next: Partial): void { + e.snap = { ...e.snap, ...next } + for (const cb of e.subs) cb() +} + +function _refetch(tid: number): Promise { + const e = _entries.get(tid) + if (!e) return Promise.resolve() + if (e.inflight) return e.inflight + e.inflight = (async () => { try { const s = await api.getMonitorState(tid) - setState(s) - setConnected(true) - lastUpdateRef.current = Date.now() + e.lastUpdate = Date.now() + _set(e, { state: s, connected: true }) } catch { - if (Date.now() - lastUpdateRef.current > 5000) setConnected(false) + if (Date.now() - e.lastUpdate > 5000) _set(e, { connected: false }) + } finally { + e.inflight = null } - }, []) - - // taskId changes -> clear state + refetch - useEffect(() => { - setState(null) - if (taskId == null) { - setConnected(false) - return + })() + return e.inflight +} + +function _acquire(tid: number): Entry { + const existing = _entries.get(tid) + if (existing) return existing + const entry: Entry = { snap: IDLE, lastUpdate: 0, subs: new Set(), inflight: null, unsubscribe: () => {} } + _entries.set(tid, entry) + entry.unsubscribe = subscribeEventStream((evt) => { + if (evt.type !== 'monitor_progress') return + if (String(evt.task_id) !== String(tid)) return + const delta = evt.delta as MonitorProgressDelta | undefined + if (!delta) return + entry.lastUpdate = Date.now() + _set(entry, { state: mergeDelta(entry.snap.state, delta), connected: true }) + }, () => { void _refetch(tid) }) + void _refetch(tid) + return entry +} + +function _release(tid: number): void { + const e = _entries.get(tid) + if (!e || e.subs.size > 0) return + e.unsubscribe() + _entries.delete(tid) +} + +export function useMonitorProgress(taskId: number | null): MonitorProgress { + const subscribe = useCallback((cb: () => void) => { + if (taskId == null) return () => {} + const e = _acquire(taskId) + e.subs.add(cb) + return () => { + e.subs.delete(cb) + _release(taskId) } - void refetch() - }, [taskId, refetch]) - - useEventStream( - (evt) => { - const tid = taskIdRef.current - if (tid == null) return - if (evt.type !== 'monitor_progress') return - if (String(evt.task_id) !== String(tid)) return - const delta = evt.delta as MonitorProgressDelta | undefined - if (!delta) return - setState((prev) => mergeDelta(prev, delta)) - setConnected(true) - lastUpdateRef.current = Date.now() - }, - { onOpen: () => void refetch() }, + }, [taskId]) + const getSnap = useCallback( + () => (taskId == null ? IDLE : _entries.get(taskId)?.snap ?? IDLE), + [taskId], ) - - return { state, connected, refetch } + const snap = useSyncExternalStore(subscribe, getSnap, getSnap) + const refetch = useCallback( + () => (taskId == null ? Promise.resolve() : _refetch(taskId)), + [taskId], + ) + return { state: snap.state, connected: snap.connected, refetch } } // exposed for tests diff --git a/studio/web/src/lib/useZoomPan.ts b/studio/web/src/lib/useZoomPan.ts index 9dadafb..e94a578 100644 --- a/studio/web/src/lib/useZoomPan.ts +++ b/studio/web/src/lib/useZoomPan.ts @@ -28,7 +28,7 @@ * the pan/zoom hot path never goes through React rendering; only zoomPct * (for the readout) goes through state */ -import { useCallback, useEffect, useRef, useState } from 'react' +import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react' export interface ZoomPanView { scale: number @@ -140,8 +140,9 @@ export function useZoomPan({ } }, []) - // Auto-fit when switching images / on initial readiness - useEffect(() => { + // Auto-fit when switching images / on initial readiness. Layout effect: the new + // transform lands before the browser paints, so the image never shows a frame at the old scale. + useLayoutEffect(() => { fit() }, [fit]) diff --git a/studio/web/src/pages/QueueDetail.tsx b/studio/web/src/pages/QueueDetail.tsx index 0632182..d6c0f0c 100644 --- a/studio/web/src/pages/QueueDetail.tsx +++ b/studio/web/src/pages/QueueDetail.tsx @@ -665,7 +665,7 @@ function SamplesCard({ taskId, live, onRender, canRender }: { taskId: number; li
{shown.map((s, i) => ( ))} @@ -683,6 +683,9 @@ function SamplesCard({ taskId, live, onRender, canRender }: { taskId: number; li onClose={() => setZoom(null)} onPrev={() => setZoom((z) => (z != null && z > 0 ? z - 1 : z))} onNext={() => setZoom((z) => (z != null && z < items.length - 1 ? z + 1 : z))} + preload={[items[zoom - 1], items[zoom + 1]] + .filter((x): x is TaskSample => !!x) + .map((x) => api.sampleImageUrl(x.filename, taskId))} /> )}