diff --git a/app/boot.js b/app/boot.js index a9146afa..a8ad4810 100644 --- a/app/boot.js +++ b/app/boot.js @@ -8,7 +8,7 @@ import { fetchManifest } from "./shell/manifest.js"; import { mountAll } from "./shell/panels.js"; import { all as allValues } from "./shell/values.js"; import { composer } from "./composer/composer.js"; -import { createRun, viewUrl } from "./shell/run.js"; +import { createRun } from "./shell/run.js"; import { clientId, connect, runningFor, finishedFor } from "./shell/client.js"; import { wire } from "./shell/session.js"; import { check } from "./shell/pipeline.js"; @@ -25,12 +25,6 @@ async function start() { }); const session = wire({ run, page, check, id, runningFor, finishedFor }); - run.subscribe((state) => { - page.transport.draw(state); - const last = state.images[state.images.length - 1]; - if (last) page.viewer.setSource(viewUrl(last), kindOf(last)); - }); - let manifest; try { manifest = await fetchManifest(); @@ -63,9 +57,4 @@ async function start() { }; } -/** Video and image results arrive the same way and cannot be shown the same way. */ -function kindOf(image) { - return /\.(mp4|webm|mov|mkv)$/i.test(image.filename || "") ? "video" : "image"; -} - start(); diff --git a/app/shell/session.js b/app/shell/session.js index 76b0d3d2..a5caa3bb 100644 --- a/app/shell/session.js +++ b/app/shell/session.js @@ -1,5 +1,10 @@ // The two things the page does around a run: start one, and take back over one // that was already going. + +import { viewUrl } from "./run.js"; + +/** Video and image results arrive the same way and cannot be shown the same way. */ +const kindOf = (image) => (/\.(mp4|webm|mov|mkv)$/i.test(image.filename || "") ? "video" : "image"); // // Kept out of boot.js because boot.js runs on import and needs a document, so // nothing in it could be driven by a test -- and both of the faults that lived @@ -82,6 +87,16 @@ export function createGenerator({ run, transport, check, ready }) { * it. */ export function wire({ run, page, check, id, runningFor, finishedFor }) { + // One subscription draws everything a run affects. It lives here rather than + // beside it in boot.js because subscribe() delivers the CURRENT state at + // once, and doing that after the button was deliberately disabled handed it + // straight back -- live-looking, and doing nothing when pressed. + run.subscribe((state) => { + page.transport.draw(state); + const last = state.images[state.images.length - 1]; + if (last && page.viewer) page.viewer.setSource(viewUrl(last), kindOf(last)); + }); + // The socket opens at load, not at Generate. A generation queued before a // reload keeps running on the server, and a page that only listens once it // starts one of its own hears nothing about it. @@ -90,12 +105,10 @@ export function wire({ run, page, check, id, runningFor, finishedFor }) { // Not awaited by the caller, but the button waits on it. Until this settles // nobody can know whether this page already has a run, and starting another // one on the strength of "phase is idle" is starting one on a guess. - page.transport.generate.setDisabled(true); - page.transport.say("Looking for a run already in progress"); + page.transport.hold("Looking for a run already in progress"); const ready = reattach(run, id, { runningFor, finishedFor }).then((adopted) => { - page.transport.say(null, run.state); - page.transport.draw(run.state); + page.transport.release(run.state); return adopted; }); diff --git a/app/shell/tests/session.test.js b/app/shell/tests/session.test.js index 944c7d21..de442a0a 100644 --- a/app/shell/tests/session.test.js +++ b/app/shell/tests/session.test.js @@ -50,11 +50,26 @@ function fakeRun(phase = "idle") { }, started, release: () => release(), - state: { phase }, + state: { phase, promptId: null, progress: null, images: [], audio: [], error: null, node: null }, listened: 0, listen() { this.listened += 1; }, + // The real run delivers the CURRENT state on subscribe, at once. Leaving + // that off is how a fake stays tidier than the thing it stands in for: the + // synchronous first delivery is exactly what handed a deliberately disabled + // button straight back, and a fake without it could not show that. + subscribers: new Set(), + subscribe(fn) { + this.subscribers.add(fn); + try { fn(this.state); } catch { /* as the real one does */ } + return () => this.subscribers.delete(fn); + }, + announce() { for (const fn of this.subscribers) fn(this.state); }, seen: () => [], - adopt(id) { this.state = { ...this.state, phase: "running", promptId: id }; return true; }, + adopt(id) { + this.state = { ...this.state, phase: "running", promptId: id, images: [] }; + this.announce(); + return true; + }, async start(prompt) { started.push(prompt); this.state = { ...this.state, phase: "queued" }; @@ -311,3 +326,46 @@ test("a queue that cannot be reached still gives the button back", async () => { assert.equal(t.disabled, false); assert.equal(t.text, "Ready"); }); + +test("a state delivered while the page is still looking does not hand Generate back", () => { + // subscribe() delivers the current state at once, and every later change + // redraws. Each of those draws decides the button from the run's phase alone, + // so an idle run -- which is what a page has while it is still finding out + // whether it has a run at all -- re-enabled a button that had just been + // deliberately disabled. Live-looking, and doing nothing when pressed. + const t = transport(); + const run = fakeRun(); + wire({ + run, page: { transport: t }, check: plan(), id: "me", + runningFor: () => new Promise(() => {}), // never answers + finishedFor: async () => null, + }); + + assert.equal(t.disabled, true); + run.announce(); // any redraw at all + assert.equal(t.disabled, true, "a redraw handed the button back mid-lookup"); + t.draw(run.state); + assert.equal(t.disabled, true, "drawing the idle run handed the button back"); +}); + +test("the result of a run is shown as it arrives", async () => { + // The subscription draws the transport AND the preview. Moving it had to + // carry both, and the viewer is the half with nothing else watching it. + const shown = []; + const t = transport(); + const run = fakeRun(); + wire({ + run, + page: { transport: t, viewer: { setSource: (src, kind) => shown.push([src, kind]) } }, + check: plan(), id: "me", + runningFor: async () => null, finishedFor: async () => null, + }); + + run.state = { ...run.state, phase: "done", images: [{ filename: "a.png", subfolder: "", type: "output" }] }; + run.announce(); + assert.deepEqual(shown, [["/view?filename=a.png&subfolder=&type=output", "image"]]); + + run.state = { ...run.state, images: [{ filename: "clip.mp4", subfolder: "", type: "output" }] }; + run.announce(); + assert.equal(shown[1][1], "video", "a video result was handed to an "); +}); diff --git a/app/shell/transport.js b/app/shell/transport.js index 25e6b292..3141e34f 100644 --- a/app/shell/transport.js +++ b/app/shell/transport.js @@ -37,6 +37,13 @@ export function createTransport({ onGenerate, onCancel } = {}) { // run says "Ready", which wiped the reason a moment after it appeared. let note = null; + // Held for a reason that is not a run: the page is still working out whether + // it already has one. Kept as a flag rather than left to whoever disabled the + // button, because draw() runs on every state change and each of those would + // otherwise hand the button back -- which is exactly what a subscription + // delivering the current state did, one line after the button was disabled. + let held = false; + // Hidden by attribute rather than removed: the row's shape then does not jump // as a run starts and finishes, and there is one node to find in a test. const hide = (handle, yes) => { @@ -51,7 +58,7 @@ export function createTransport({ onGenerate, onCancel } = {}) { // says outranks a note about an attempt that never started. if (state.phase !== IDLE) note = null; const working = WORKING.has(state.phase); - generate.setDisabled(working); + generate.setDisabled(working || held); generate.setBusy(working); hide(cancel, !working); @@ -72,7 +79,21 @@ export function createTransport({ onGenerate, onCancel } = {}) { status.setText(note || describe(state || { phase: IDLE, images: [] })); } - return { node: bar.node, draw, say, generate, cancel, progress, status }; + /** Stop offering Generate for a reason of the page's own, and say why. */ + function hold(reason) { + held = Boolean(reason); + if (reason) say(reason); + generate.setDisabled(true); + } + + /** Offer it again. The run's own state decides from here. */ + function release(state) { + held = false; + say(null, state); + draw(state); + } + + return { node: bar.node, draw, say, hold, release, generate, cancel, progress, status }; } /** One line saying where the run is. The only place these words are decided. */