Review fixes round 2: only Upscale and Interpolate jobs wait for each other, a queued one stops at once; tour numbers the parts on screen and Help sees it running
Tour and docs wording match the UI names. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tw9fjVsBg3ixjnT1Fgdy12
This commit is contained in:
@@ -70,7 +70,7 @@ export default {
|
||||
{ id: "exclude", label: "Toggle Exclude", disabled: !sel.focus }],
|
||||
View: () => [{ id: "refresh", label: "Refresh Preview" }, { id: "reset", label: "Reset Layout" }],
|
||||
Help: () => {
|
||||
const running = Boolean(app.tourOn?.());
|
||||
const running = app.has.has("tour.running");
|
||||
return [...(app.has.has("tour") ? [{ id: "tour", label: running ? "Restart tour" : "Start tour" }, { id: "faq", label: "Skip to FAQ" },
|
||||
...(running ? [{ id: "exit", label: "Exit tour" }] : []), { separator: true }] : []), { id: "welcome", label: "Welcome window…" }];
|
||||
},
|
||||
|
||||
@@ -13,7 +13,7 @@ const STEPS = [
|
||||
{ at: "button:⏱ Sampler", side: "top", title: "Sampler", text: "Steps, sampler, scheduler, seed and the common switches, one click from the timeline. While a run goes, the step count shows to its left." },
|
||||
{ at: "button:◆ Composer", side: "top", title: "Composer", text: "Write the whole story in one box, cut at the cut word; keep templates, $variables and shortcuts; chat with the prompt enhancer." },
|
||||
{ at: "button:+ Add", side: "top", title: "Add and edit", text: "+ Add opens one menu: a scene, a video clip or audio track from the bin, an effect or transition, a text or picture overlay. Next to it: Split, Remove, Export, Save to media bin and Separate audio." },
|
||||
{ at: "button:⧉ Render", side: "top", title: "Render", text: "Joins the whole timeline into one file, with the separated sound mixed in. Export downloads just the picked clip." },
|
||||
{ at: "button:⧉ Render", side: "top", title: "Render", text: "Joins the whole timeline into one file, with the separated sound mixed in. Export downloads the picked clips." },
|
||||
{ at: "main", side: "bottom", title: "FAQ", text: "Nothing happens? Check the ComfyUI chip at the top right and the Log window. Wrong look? Settings ▸ Appearance. Lost a layout? View ▸ Reset Layout. Lost a project? File ▸ Open recent. That is the tour: Help ▸ Start tour runs it again." },
|
||||
];
|
||||
|
||||
@@ -23,26 +23,28 @@ export default {
|
||||
needs: ["project", "dom"],
|
||||
setup({ app }) {
|
||||
let at = -1, pop = null;
|
||||
const stop = () => { if (pop) { const p = pop; pop = null; p.close(); } at = -1; };
|
||||
const stop = () => { if (pop) { const p = pop; pop = null; p.close(); } at = -1; app.has.delete("tour.running"); }; // a flag the Help menu reads: features see copies of app, the Set is shared
|
||||
const shown = (i) => { const el = app.dom.landmark(STEPS[i].at); return el && el.getBoundingClientRect().width ? el : null; };
|
||||
function show(i) {
|
||||
if (pop) { const p = pop; pop = null; p.close(); }
|
||||
let target;
|
||||
while (i >= 0 && i < STEPS.length && !((target = app.dom.landmark(STEPS[i].at)) && target.getBoundingClientRect().width)) i += i >= at ? 1 : -1; // a part that is hidden right now is skipped
|
||||
while (i >= 0 && i < STEPS.length && !(target = shown(i))) i += i >= at ? 1 : -1; // a part that is hidden right now is skipped
|
||||
if (i < 0 || i >= STEPS.length) return stop();
|
||||
at = i;
|
||||
app.has.add("tour.running");
|
||||
const seen = STEPS.map((_, k) => k).filter((k) => k === i || shown(k)), place = seen.indexOf(i) + 1; // numbered among the parts on screen, so the count has no gaps
|
||||
const s = STEPS[i], last = i === STEPS.length - 1;
|
||||
const body = c.region.stack({ gap: "sm", children: [c.label.section({ text: `${i + 1} / ${STEPS.length} · ${s.title}` }), c.text.sm({ text: s.text }),
|
||||
const body = c.region.stack({ gap: "sm", children: [c.label.section({ text: `${place} / ${seen.length} · ${s.title}` }), c.text.sm({ text: s.text }),
|
||||
c.toolbar.default({ items: [c.button.sm({ label: "Back", tone: "ghost", disabled: i === 0, onClick: () => show(i - 1) }), c.button.sm({ label: last ? "Done" : "Next", tone: "primary", onClick: () => (last ? stop() : show(i + 1)) })],
|
||||
trailing: [c.button.sm({ label: "Exit tour", tone: "ghost", onClick: stop })] })] });
|
||||
pop = c.popover.anchored({ anchor: target, body, side: s.side, align: "center", gap: 10, closeOnOutside: false, onClose: () => { if (pop) { pop = null; at = -1; } } });
|
||||
pop = c.popover.anchored({ anchor: target, body, side: s.side, align: "center", gap: 10, closeOnOutside: false, onClose: () => { if (pop) { pop = null; at = -1; app.has.delete("tour.running"); } } });
|
||||
}
|
||||
app.has.add("tour");
|
||||
app.tourOn = () => pop !== null; // Help shows "Exit tour" only while one is running
|
||||
const off = app.on((what) => {
|
||||
if (what === "tour.start") show(0);
|
||||
else if (what === "tour.faq") show(STEPS.length - 1);
|
||||
else if (what === "tour.stop") stop();
|
||||
});
|
||||
return () => { off(); app.has.delete("tour"); delete app.tourOn; };
|
||||
return () => { off(); app.has.delete("tour"); app.has.delete("tour.running"); };
|
||||
},
|
||||
};
|
||||
|
||||
+19
-5
@@ -8,6 +8,7 @@ from collections import OrderedDict
|
||||
|
||||
MAX_JOBS = 50 # finished jobs are kept this long for the poll that reads them
|
||||
|
||||
_waiting: set = set() # ids queued behind another job
|
||||
_jobs: "OrderedDict[str, dict]" = OrderedDict()
|
||||
_stop: set = set() # ids asked to stop; the job notices between chunks
|
||||
_can_stop: set = set() # ids whose work checks for it
|
||||
@@ -23,6 +24,8 @@ def stop(job: str) -> bool:
|
||||
if job not in _can_stop or not entry or entry.get("state") not in ("queued", "running"):
|
||||
return False
|
||||
_stop.add(job)
|
||||
if entry.get("state") == "queued" and job in _waiting:
|
||||
_jobs[job] = {"state": "stopped"} # still waiting for its turn: it ends now, not when the one ahead does
|
||||
return True
|
||||
|
||||
|
||||
@@ -49,15 +52,23 @@ def get(job: str) -> dict | None:
|
||||
return _jobs.get(job)
|
||||
|
||||
|
||||
_turns: "weakref.WeakKeyDictionary" = weakref.WeakKeyDictionary() # one lock per event loop: one job at a time (they share the GPU, the loaded effects and ffmpeg)
|
||||
_turns: "weakref.WeakKeyDictionary" = weakref.WeakKeyDictionary() # one lock per event loop
|
||||
|
||||
|
||||
async def run(job: str, fn, *args, error, stoppable: bool = False) -> None:
|
||||
async def run(job: str, fn, *args, error, stoppable: bool = False, exclusive: bool = False) -> None:
|
||||
"""Run the blocking `fn(*args)` off the event loop and record how it ended. `error` is the
|
||||
exception class whose message is meant for the person; anything else is reported plainly.
|
||||
Jobs run one after the other (a second one waits as "queued"): two effects of one kind share a buffer."""
|
||||
turn = _turns.setdefault(asyncio.get_running_loop(), asyncio.Lock())
|
||||
async with turn:
|
||||
`exclusive`: the work uses a GPU effect that keeps one buffer per kind (Upscale, Interpolate), so such jobs
|
||||
run one after the other and a second waits as "queued". Other jobs (the render) never wait for them."""
|
||||
turn = _turns.setdefault(asyncio.get_running_loop(), asyncio.Lock()) if exclusive else None
|
||||
if turn is not None and turn.locked():
|
||||
_waiting.add(job)
|
||||
try:
|
||||
if turn is not None:
|
||||
await turn.acquire()
|
||||
finally:
|
||||
_waiting.discard(job)
|
||||
try:
|
||||
if job in _stop: # stopped while it was still queued
|
||||
_stop.discard(job)
|
||||
_can_stop.discard(job)
|
||||
@@ -76,3 +87,6 @@ async def run(job: str, fn, *args, error, stoppable: bool = False) -> None:
|
||||
finally:
|
||||
_stop.discard(job)
|
||||
_can_stop.discard(job)
|
||||
finally:
|
||||
if turn is not None:
|
||||
turn.release()
|
||||
|
||||
@@ -23,8 +23,8 @@ Start ComfyUI as usual and open `/fun/` on the same address, for example `http:/
|
||||
| Start from | State |
|
||||
|---|---|
|
||||
| MiniMax H3 · Reference to Video, MiniMax H3 · Fast & Simple R2V | The model FunUI is built and tested around. |
|
||||
| Stable Diffusion 1.5 · Text to Image | Experimental. |
|
||||
| LTX 2.3 · Image to Video (from References), LTX 2.3 · Video to Video (direct) | Tested on a real model, with LoRAs and Noise merging. Other enhancements are not tested on LTX yet; please report problems in [Issues](https://github.com/digital-garbage/FunUI/issues). |
|
||||
| Stable Diffusion 1.5 · Text to Image (held as a clip) | Experimental. |
|
||||
| LTX 2.3 · Image to Video, from References, LTX 2.3 · Video to Video, direct | Tested on a real model, with LoRAs and Noise merging. Other enhancements are not tested on LTX yet; please report problems in [Issues](https://github.com/digital-garbage/FunUI/issues). |
|
||||
|
||||
## 4. Pick your files
|
||||
|
||||
|
||||
@@ -43,7 +43,7 @@ def routes(table, base, web):
|
||||
except ValueError as exc:
|
||||
return bad(str(exc))
|
||||
job = jobs.new(stoppable=True)
|
||||
asyncio.create_task(jobs.run(job, nodes.interpolate, media, body.get("options") or {}, error=nodes.InterpolateError, stoppable=True))
|
||||
asyncio.create_task(jobs.run(job, nodes.interpolate, media, body.get("options") or {}, error=nodes.InterpolateError, stoppable=True, exclusive=True))
|
||||
return web.json_response({"job_id": job})
|
||||
|
||||
@table.post(base + "/jobs/{job}/stop")
|
||||
|
||||
@@ -102,7 +102,7 @@ def routes(table, base, web):
|
||||
if not isinstance(media, dict) or not isinstance(media.get("filename"), str) or not media["filename"] or not isinstance(model, str) or not model:
|
||||
return bad("Send {media: {filename, subfolder, type}, model, rtx_options}.")
|
||||
job = jobs.new(stoppable=True)
|
||||
asyncio.create_task(jobs.run(job, upscale.run, media, model, body.get("rtx_options") or "{}", error=(RuntimeError, ValueError, files.ClipError), stoppable=True))
|
||||
asyncio.create_task(jobs.run(job, upscale.run, media, model, body.get("rtx_options") or "{}", error=(RuntimeError, ValueError, files.ClipError), stoppable=True, exclusive=True))
|
||||
return web.json_response({"job_id": job})
|
||||
|
||||
@table.post(base + "/jobs/{job}/stop")
|
||||
|
||||
@@ -235,7 +235,7 @@ def test_a_stop_that_lands_after_the_last_chunk_still_removes_the_file(comfy):
|
||||
assert not dst.exists()
|
||||
|
||||
|
||||
def test_jobs_run_one_after_the_other_and_the_second_waits_as_queued():
|
||||
def test_exclusive_jobs_run_one_after_the_other_and_the_second_waits_as_queued():
|
||||
import asyncio
|
||||
import threading
|
||||
gate, order = threading.Event(), []
|
||||
@@ -252,10 +252,10 @@ def test_jobs_run_one_after_the_other_and_the_second_waits_as_queued():
|
||||
|
||||
async def go():
|
||||
a, b = jobs.new(), jobs.new()
|
||||
ta = asyncio.create_task(jobs.run(a, first, error=RuntimeError))
|
||||
ta = asyncio.create_task(jobs.run(a, first, error=RuntimeError, exclusive=True))
|
||||
while "first start" not in order:
|
||||
await asyncio.sleep(0.01)
|
||||
tb = asyncio.create_task(jobs.run(b, second, error=RuntimeError))
|
||||
tb = asyncio.create_task(jobs.run(b, second, error=RuntimeError, exclusive=True))
|
||||
await asyncio.sleep(0.1)
|
||||
waiting = (jobs.get(a)["state"], jobs.get(b)["state"], list(order))
|
||||
gate.set()
|
||||
@@ -265,3 +265,39 @@ def test_jobs_run_one_after_the_other_and_the_second_waits_as_queued():
|
||||
waiting, order, last = asyncio.run(go())
|
||||
assert waiting == ("running", "queued", ["first start"])
|
||||
assert order == ["first start", "first end", "second start"] and last == "done"
|
||||
|
||||
|
||||
def test_a_render_never_waits_for_an_exclusive_job_and_a_queued_one_can_be_stopped():
|
||||
import asyncio
|
||||
import threading
|
||||
gate, ran = threading.Event(), []
|
||||
|
||||
def long_effect(interrupted):
|
||||
gate.wait(5)
|
||||
return {}
|
||||
|
||||
def render():
|
||||
ran.append("render")
|
||||
return {}
|
||||
|
||||
def effect(interrupted):
|
||||
ran.append("effect")
|
||||
return {}
|
||||
|
||||
async def go():
|
||||
a, b, r = jobs.new(stoppable=True), jobs.new(stoppable=True), jobs.new()
|
||||
ta = asyncio.create_task(jobs.run(a, long_effect, error=RuntimeError, stoppable=True, exclusive=True))
|
||||
await asyncio.sleep(0.05)
|
||||
tb = asyncio.create_task(jobs.run(b, effect, error=RuntimeError, stoppable=True, exclusive=True))
|
||||
await asyncio.sleep(0.05)
|
||||
await jobs.run(r, render, error=RuntimeError) # not held up by the effect running
|
||||
seen = (jobs.get(r)["state"], jobs.get(b)["state"])
|
||||
assert jobs.stop(b) is True
|
||||
stopped_now = jobs.get(b)["state"] # ends at once, not when the one ahead ends
|
||||
gate.set()
|
||||
await asyncio.gather(ta, tb)
|
||||
return seen, stopped_now, jobs.get(b)["state"]
|
||||
|
||||
seen, stopped_now, last = asyncio.run(go())
|
||||
assert seen == ("done", "queued") and stopped_now == "stopped" and last == "stopped"
|
||||
assert ran == ["render"] # the stopped one never started
|
||||
|
||||
Reference in New Issue
Block a user