feat: generic progress events #6715
Conversation
Some processes have steps, like denoising or a tiled spandel. Denoising has its own step callback but we don't have any generic way to signal progress. Processes like a tiled spandrel run show indeterminate progress in the client. This change introduces a new event to handle this: `InvocationGenericProgressEvent` A simplified helper is added to the invocation API so nodes can easily emit progress as they do their thing.
psychedelicious
requested review from
blessedcoolant,
brandonrising and
hipsterusername
as code owners
github-actions
bot
added
api
python
|
If we want to implement this change, once the details are stabilized I can play around with the frontend implementation. I don't anticipate it being difficult. |
|
Good idea 🤘 |
- Merged `InvocationGenericProgressEvent` and `InvocationDenoiseProgressEvent` into single `InvocationProgressEvent` - Simplified API - message is required, percentage and image are optional, no steps/total steps - Added helper to build a `ProgressImage` - Added field validation to `ProgressImage` width and height - Added `ProgressImage` to `invocation_api.py` - Updated `InvocationContext` utils
Minor changes to use the new progress event. Only additional feature is if the progress has a message, it is displayed as a tooltip on the progress bar.
|
I've revised this PR a bit, simplifying the API and updating the frontend to use the new progress events. Currently it's only the two spandrel nodes that signal progress with the new API. For the autoscale node, which does multiple spandrel runs, the progress bar resets for each. Its message looks like this: "Processing image (iteration 2, tile 4/25)". While there is a message attached to each progress event, it's not yet used in the UI. I'd like to get this core implementation merged first, then iterate on the presentation. I tried displaying the messages as a tooltip on the progress bar but it's not particularly discoverable. Maybe it should go on the invoke button tooltip? Not sure. When that presentation is sorted, I'll update the UI's event handling to present model events as well as invocation start and complete events. With this new flexible API, I'd like to explore some changes:
|
| is_canceled=self.is_canceled, | ||
| ) | ||
|
|
||
| def signal_progress( |
There was a problem hiding this comment.
Maybe this should called update_status instead?
Summary
We don't have a way to signal progress in a generic way, so things like tiled spandrel are just an indeterminate progress bar in the UI. The UX is poor.
This PR is a first go at rectifying this. It adds a separate "generic" progress event, that accepts:
name: str: used to group progress events (along with the invocation's ID)step: int | None: the current step (omit for indeterminate)total_steps: int | None: the total steps (omit for indeterminate)message: str | None: a message to displayimage: ProgressImage | None = None: an image to displayNo frontend implementation yet as I'm still unsure of the direction of this PR.
See code comments for discussion.
Related Issues / Discussions
Dunno if there are any.
QA Instructions
You can test this PR by opening the network monitor in your browser's dev tools, filtering for websockets and reviewing the events while using spandrel. Start with an image > 512x512 else you only get one step.
Merge Plan
n/a
Checklist