From 4f614a96dcc04285f1a0d0b06187b2ba02562ec9 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sat, 3 Oct 2026 19:22:55 +0300 Subject: [PATCH] fix(ui): generate by unit with stop/errors/project guard, edit rollback, selection after remove, split selected clip, failed feature unsubscribed, project.video controls, preview starts at the cut Co-Authored-By: Claude Sonnet 5.5 --- app/ui/features/generate/index.js | 64 ++++++++++++++----------- app/ui/features/generate/render.test.js | 14 ++++++ app/ui/features/inspector/index.js | 14 +++++- app/ui/features/preview/index.js | 2 +- app/ui/features/timeline/index.js | 13 ++--- app/ui/features/timeline/lanes.js | 4 +- app/ui/shell/features.js | 7 ++- app/ui/shell/features.test.js | 9 ++++ app/ui/shell/project.js | 9 +++- app/ui/shell/tests/project.test.js | 20 ++++++++ 10 files changed, 115 insertions(+), 41 deletions(-) create mode 100644 app/ui/features/generate/render.test.js diff --git a/app/ui/features/generate/index.js b/app/ui/features/generate/index.js index f57ef2da..a63b7b9c 100644 --- a/app/ui/features/generate/index.js +++ b/app/ui/features/generate/index.js @@ -1,57 +1,67 @@ -// Generate / Selected: queue the plan's scenes one after another and put each result on its clip. +// Generate / Selected / Stop: queue generated units one after another and put each result on its clips. import { composer as c } from "../../composer/composer.js"; -import { isGenerative, genUnitId, effFrames, effFps } from "../../shell/scenes.js"; +import { isGenerative, genUnitId, unitRoot, effFrames, effFps } from "../../shell/scenes.js"; import { buildInputs } from "./inputs.js"; const expand = (body) => fetch("/funpack/api/prompt/expand", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ ...body, seed: Math.floor(Math.random() * 2 ** 31) || 1 }) }).then((r) => (r.ok ? r.json() : null)); +/** The render a scene gets from its unit's one clip: a cut half plays from where its half begins. */ +export const renderFor = (sc, p, media, unitSec) => ({ media, inSec: (sc.cut_offset_frames || 0) / effFps(sc, p), + ...((sc.frames_mode || "project") === "project" ? { durationSec: unitSec } : {}) }); + export default { id: "generate", mount: "timeline.actions", needs: ["project", "pipeline", "generate"], setup({ host, app }) { const p = app.project, g = app.generate; - let busy = false; + const tell = (text) => c.toast.warn({ text }); + g.on("say", tell); g.on("warn", tell); // the pipeline check's refusals: said where the person is looking + let busy = false, stopped = false; - const record = (pid, id, image) => { - if (!image || !p.project || p.project.id !== pid) return; - p.edit((pr) => { - const sc = pr.scenes.find((s) => s.id === id); - if (!sc) return false; - (pr.scene_renders ||= {})[id] = { media: { filename: image.filename, subfolder: image.subfolder || "", type: image.type || "output" }, - durationSec: effFrames(sc, pr) / effFps(sc, pr), inSec: 0 }; - return true; - }); - }; + const record = (pid, unit, image) => p.project && p.project.id === pid && p.edit((pr) => { + const group = pr.scenes.filter((s) => genUnitId(s) === unit); + if (!group.length) return false; + const secs = group.reduce((t, s) => t + effFrames(s, pr) / effFps(s, pr), 0); + const media = { filename: image.filename, subfolder: image.subfolder || "", type: image.type || "output" }; + group.forEach((s) => { (pr.scene_renders ||= {})[s.id] = renderFor(s, pr, media, secs); }); + return true; + }); - async function runAll(scenes) { + async function runUnits(units) { if (busy) return; - busy = true; draw(); - const pid = p.project.id, seen = new Set(); + busy = true; stopped = false; draw(); + const pid = p.project.id; try { - for (const sc of scenes) { - if (!isGenerative(sc) || sc.excluded || seen.has(genUnitId(sc))) continue; - seen.add(genUnitId(sc)); - const { inputs, unwired } = await buildInputs({ project: p.project, scene: sc, slots: app.pipeline.slots(), expand }); - if (unwired) c.toast.warn({ text: `${unwired} reference(s) did not fit this pipeline and are not used.` }); + for (const unit of units) { + if (stopped) break; + if (!p.project || p.project.id !== pid) { tell("Stopped: another project was opened."); break; } + const root = unitRoot(p.project, unit); + if (!root || root.excluded || !isGenerative(root)) continue; // removed, left out, or not made by the model + const { inputs, unwired } = await buildInputs({ project: p.project, scene: root, slots: app.pipeline.slots(), expand }); + if (stopped) break; + if (unwired) tell(`${unwired} reference(s) did not fit this pipeline and are not used.`); const done = g.waitForTerminal(); // listening before the run starts, so a fast one is not missed - if (!(await g.generate({ sceneId: sc.id, projectId: pid, inputs }))) { done.cancel(); break; } + if (!(await g.generate({ sceneId: root.id, projectId: pid, inputs }))) { done.cancel(); break; } const end = await done; - if (end !== g.DONE) break; + if (end === g.CANCELLED) break; const images = g.run.state.images; - record(pid, sc.id, images[images.length - 1]); + if (end !== g.DONE) { tell("Generation failed. The log has ComfyUI's message."); break; } + if (!images.length) { tell("ComfyUI finished without a result (a cached run makes none). Change the prompt or seed and try again."); break; } + record(pid, unit, images[images.length - 1]); } } finally { busy = false; draw(); } } + const unitsOf = (scenes) => [...new Set(scenes.map(genUnitId))]; let nodes = []; function draw() { const open = p.project; const go = c.button.sm({ label: busy ? "Generating…" : "▶ Generate", tone: "primary", disabled: busy || !open, - onClick: () => runAll(p.scenes) }); - const one = c.button.sm({ label: "Selected", disabled: busy || !p.selected, onClick: () => runAll([p.selected]) }); - const stop = c.button.sm({ label: "■ Stop", tone: "danger", disabled: !busy, onClick: () => g.cancel() }); + onClick: () => runUnits(unitsOf(p.scenes)) }); + const one = c.button.sm({ label: "Selected", disabled: busy || !p.selected, onClick: () => runUnits(unitsOf([p.selected])) }); + const stop = c.button.sm({ label: "■ Stop", tone: "danger", disabled: !busy, onClick: () => { stopped = true; g.cancel(); } }); const next = [go, one, stop].map((b) => b.node); if (nodes.length) nodes.forEach((n, i) => n.replaceWith(next[i])); else host.append(...next); // in place: keeps its spot in the row nodes = next; diff --git a/app/ui/features/generate/render.test.js b/app/ui/features/generate/render.test.js new file mode 100644 index 00000000..09167e8d --- /dev/null +++ b/app/ui/features/generate/render.test.js @@ -0,0 +1,14 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { renderFor } from "./index.js"; +import { split } from "../../shell/edits.js"; + +test("both halves of a split unit get the one clip, the second from its cut", () => { + const p = { num_frames_per_scene: 97, frame_rate: 24, scenes: [{ id: "a", text: "cat" }] }; + split(p, "a"); + const [a, b] = p.scenes; + const media = { filename: "x.mp4" }; + assert.equal(renderFor(a, p, media, 4).inSec, 0); + assert.ok(renderFor(b, p, media, 4).inSec > 0); + assert.equal(b.gen_unit_id, a.gen_unit_id); +}); diff --git a/app/ui/features/inspector/index.js b/app/ui/features/inspector/index.js index 7af78737..f8d10616 100644 --- a/app/ui/features/inspector/index.js +++ b/app/ui/features/inspector/index.js @@ -17,7 +17,16 @@ const sceneRows = (p) => { ]; }; -const projectRows = (p) => { +// What the pipeline lets the project decide (width, length, steps...): one control per slot input that declares the role. +const videoRows = (p, slots) => (slots || []).flatMap((slot) => (slot.roles || []).filter((r) => r.at === "project.video" && r.input).map((role) => { + const now = p.video[role.input] ?? (slot.inputs || {})[role.input]; + const set = (v) => p.setVideo(role.input, v); + const label = role.label || role.input; + const control = typeof now === "number" ? c.number.md({ label, value: now, onChange: set }) : c.input.md({ label, value: now ?? "", onCommit: set }); + return c.settingsRow.default({ label, control }); +})); + +const projectRows = (p, slots) => { const open = p.project; if (!open) return [c.emptyState.default({ icon: "▭", title: "No project", hint: "Open or create one." })]; const num = (label, key, fallback, min) => c.settingsRow.default({ label, control: c.number.md({ label, min, value: open[key] || fallback, @@ -25,6 +34,7 @@ const projectRows = (p) => { return [ c.label.section({ text: "Project" }), num("Frames per scene", "num_frames_per_scene", 97, 9), num("Frame rate", "frame_rate", 25, 1), + ...videoRows(p, slots), c.label.section({ text: "Negative prompt" }), c.textarea.md({ label: "Negative prompt", value: p.negative, rows: 3, autoGrow: true, onInput: (v) => p.setNegative(v) }), ]; @@ -45,7 +55,7 @@ export default { const next = `${tab}|${p.project && p.project.id}|${p.selectedId}`; if (!force && next === key) return; key = next; - body.set((tab === "scene" ? sceneRows : projectRows)(p)); + body.set((tab === "scene" ? sceneRows : projectRows)(p, app.pipeline && app.pipeline.slots())); } draw(true); return app.on((what) => draw(what === "open")); diff --git a/app/ui/features/preview/index.js b/app/ui/features/preview/index.js index 547cc194..635b6b34 100644 --- a/app/ui/features/preview/index.js +++ b/app/ui/features/preview/index.js @@ -13,7 +13,7 @@ export default { let shown = ""; const draw = () => { const sc = p.selected, media = sc && p.project && (p.project.scene_renders || {})[sc.id]; - const src = media && media.media ? viewUrl(media.media) : ""; + const src = media && media.media ? `${viewUrl(media.media)}#t=${media.inSec || 0}` : ""; // a split clip starts where its half does if (src === shown) return; // a keystroke elsewhere must not restart the video shown = src; viewer.setSource(src || null, "video", src ? media.media : null); diff --git a/app/ui/features/timeline/index.js b/app/ui/features/timeline/index.js index decb143c..0eb171c0 100644 --- a/app/ui/features/timeline/index.js +++ b/app/ui/features/timeline/index.js @@ -42,15 +42,16 @@ export default { const btn = (label, onClick, disabled = false, title) => c.button.sm({ label, onClick, disabled, title }); function bar() { const open = p.project, ids = open ? sel.ids : []; - const splitAt = () => { - const seg = segments(open).find((s) => s.kind === "scene" && at >= s.start && at < s.start + s.dur) || segments(open).find((s) => s.id === sel.focus); - if (!seg || !seg.scene) return; - edit((pr) => { const r = edits.split(pr, seg.id, framesAt(seg.scene, pr, Math.max(0, at - seg.start))); return r; }); + const splitAt = () => { // the selected clip, at the playhead if it is over it, else in the middle + const seg = segments(open).find((s) => s.id === sel.focus && s.kind === "scene"); + if (!seg) return; + const inside = at > seg.start && at < seg.start + seg.dur; + edit((pr) => edits.split(pr, seg.id, inside ? framesAt(seg.scene, pr, at - seg.start) : undefined)); }; toolbar = swap(toolbar, [ c.text.sm({ text: `${clock(at)} / ${clock(open ? totalSeconds(open) : 0)}` }), - btn("＋ Add", () => edit((pr) => edits.addScene(pr)), !open), - btn("Split", splitAt, !ids.length, "Split the selected clip at the playhead (S)"), + btn("＋ Add", () => { const sc = edit((pr) => edits.addScene(pr)); if (sc) p.select(sc.id); }, !open), + btn("Split", splitAt, !ids.length, "Split the selected clip at the playhead"), btn("Remove", () => ids.forEach((id) => edit((pr) => edits.removeScene(pr, id))), !ids.length), ids.length > 1 ? c.chip.neutral({ label: `${ids.length} selected` }) : c.text.sm({ text: "" }), ]); diff --git a/app/ui/features/timeline/lanes.js b/app/ui/features/timeline/lanes.js index 8b74a4cf..9b0072f2 100644 --- a/app/ui/features/timeline/lanes.js +++ b/app/ui/features/timeline/lanes.js @@ -1,5 +1,5 @@ // The project as lanes of clips: what the timeline stage draws. Pure. -import { segments, isVideoClip, effFps, effFrames } from "../../shell/scenes.js"; +import { segments, isVideoClip, effFps } from "../../shell/scenes.js"; const first = (t) => ((t || "").split("\n")[0] || "").slice(0, 80); @@ -24,4 +24,4 @@ export function videoLane(p, picked, focus, actions = () => []) { } /** Frames at `sec` into scene `sc`, for Split. */ -export const framesAt = (sc, p, sec) => Math.round(sec * effFps(sc, p)) || effFrames(sc, p) / 2; +export const framesAt = (sc, p, sec) => Math.round(sec * effFps(sc, p)); diff --git a/app/ui/shell/features.js b/app/ui/shell/features.js index d0c23304..a212aa30 100644 --- a/app/ui/shell/features.js +++ b/app/ui/shell/features.js @@ -10,10 +10,13 @@ function mountOne(hub, has, app) { const lacking = (hub.needs || []).filter((name) => !has(name)); if (lacking.length) throw new Error(`needs ${lacking.join(", ")}`); const before = [...host.childNodes]; + const heard = []; // what the feature subscribed to, so a failed one can be unsubscribed + const mine = { ...app, on: (fn) => { const off = app.on(fn); heard.push(off); return off; } }; try { - return { id: hub.id, teardown: hub.setup({ host, app }) }; - } catch (err) { // whatever it put on the page goes with it + return { id: hub.id, teardown: hub.setup({ host, app: mine }) }; + } catch (err) { // whatever it put on the page and listened to goes with it [...host.childNodes].filter((n) => !before.includes(n)).forEach((n) => n.remove()); + heard.forEach((off) => off()); throw err; } } diff --git a/app/ui/shell/features.test.js b/app/ui/shell/features.test.js index c3284376..2f0be131 100644 --- a/app/ui/shell/features.test.js +++ b/app/ui/shell/features.test.js @@ -25,3 +25,12 @@ test("what a feature put on the page before it threw is taken away", async () => await run(["throws"]); assert.equal(h.childNodes.length, 0); }); + +test("a feature that subscribed and then threw stops hearing the app", async () => { + _reset(); offer("a", host()); + const heard = new Set(); + const app = { on: (fn) => { heard.add(fn); return () => heard.delete(fn); } }; + hubs.throws.setup = ({ app: a }) => { a.on(() => {}); throw new Error("late"); }; + await loadFeatures(["throws"], { load: async (n) => ({ default: hubs[n] }), app, has: () => true }); + assert.equal(heard.size, 0); +}); diff --git a/app/ui/shell/project.js b/app/ui/shell/project.js index 02f6831c..0b90942a 100644 --- a/app/ui/shell/project.js +++ b/app/ui/shell/project.js @@ -301,8 +301,15 @@ export function createProject({ onChange, onError, onOpen } = {}) { if (!project) return false; const before = JSON.stringify(project); remember(); - const result = fn(project); + let result; + try { result = fn(project); } catch (err) { // a half-made edit must not stay in memory to be saved by the next one + Object.keys(project).forEach((k) => delete project[k]); + Object.assign(project, JSON.parse(before)); + past.pop(); + throw err; + } if (!result || JSON.stringify(project) === before) { past.pop(); return result; } + if (!sceneAt(selected)) selected = ((project.scenes || [])[0] || {}).id || null; // the selected scene was removed scheduleSave(); changed(); return result; diff --git a/app/ui/shell/tests/project.test.js b/app/ui/shell/tests/project.test.js index 20e4a0d0..98a0b59a 100644 --- a/app/ui/shell/tests/project.test.js +++ b/app/ui/shell/tests/project.test.js @@ -609,3 +609,23 @@ test("taking an edit back saves it, and tells whoever draws it", async () => { await p.flush(); assert.equal(sent.at(-1).scenes.at(-1).text, "", "the version saved was not the one on screen"); }); + +test("an edit that throws leaves the project as it was, and the next edit does not save it", async () => { + const { sent } = server(); + const p = createProject({}); + await p.start(); + p.project.scenes.push({ id: "a", text: "keep" }); + assert.throws(() => p.edit((pr) => { pr.scenes[0].text = "half"; throw new Error("boom"); }), /boom/); + assert.equal(p.project.scenes[0].text, "keep"); + assert.equal(p.canUndo, false); +}); + +test("removing the selected scene selects another", async () => { + server(); + const p = createProject({}); + await p.start(); + p.project.scenes.push({ id: "a" }, { id: "b" }); + p.select("a"); + p.edit((pr) => { pr.scenes.shift(); return true; }); + assert.equal(p.selectedId, "b"); +});