fix(ui): viewer progress-image handoff: slow reveal, overlay stuck after socket drop, stale-image flash on quick re-generate - #9434
Open
lstein wants to merge 2 commits into
Conversation
lstein
requested review from
JPPhoto,
Pfannkuchensack,
blessedcoolant and
dunkeroni
as code owners
August 1, 2026 22:29
…king The image viewer holds the last progress preview on screen until the final image's onLoad fires. Two problems with that. The reveal was gated on a preload of imageDTO.image_url — the full-resolution PNG — so on a slow connection the stale latent preview stayed up for the entire multi-megabyte download. A 256px thumbnail is already generated for every image and is typically higher resolution than the preview it replaces. Gate on that instead; DndImage renders it via Chakra's fallbackSrc and swaps the full image in, in place, once it arrives. The preload also used the raw URL while DndImage requests useMediaUrl(...), which appends ?media_cookie_version=N. Different key, so the bytes were fetched twice (measured: 2 requests mismatched vs 1 matched). Route the preload through useMediaUrl so it is byte-identical. The reuse is the document's list of available images, keyed by URL rather than the HTTP cache, so it still holds in multiuser mode where images are served Cache-Control: private, no-store. Separately, the viewer's progress atoms are distinct stores from the global ones in services/events/stores, and only the latter were reset on socket lifecycle transitions. socket.io has no event replay, so a drop spanning the terminal queue_item_status_changed loses that event permanently and nothing is left to clear the opaque overlay covering the finished image — the reported "backgrounded the tab, came back, only a reload fixes it". Reset the viewer's atoms on connect/connect_error/disconnect too, matching setEventListeners. onLoadImage is not a guaranteed callback in any case: Chakra reports a failed load as onError, useImage only re-runs when src changes, the load can beat the terminal event, and an all-intermediate item never changes the selection. So the deferred clear also gets a backstop deadline. The armed flag and its timer live together in createDeferredClear — as separate state, a path that reset the flag but leaked the timer let a deadline outlive the generation that armed it and blank a later one's live preview. The backstop does not clear while other sessions still have previews, since nulling $progressImage tears down the whole overlay including multi-GPU tiles, and the reconnect reset only replaces the map when it holds something, because connect_error fires once per reconnection attempt. The terminal-status policy moves to a pure getTerminalProgressAction so the branchy decision is testable without a socket or a React tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…review Starting a new generation soon after the previous one finishes made the viewer flicker: the new previews would appear, then the previous generation's finished image would cover them for two seconds, then the previews resumed. Waiting between generations avoided it. The flash is the "reveal selected image" feature (invoke-ai#9217), which briefly hides the progress overlay so a mid-generation gallery click is visible. Its only guard against the auto-switch handoff was $isProgressImageResolving — a timing guard, and the timing loses: the auto-switch selection is dispatched only after onInvocationComplete's async DTO fetch, then waits for the thumbnail preload, and the next generation's first invocation_progress event slots into that window and resets the flag. By the time the handoff reaches the viewer it is indistinguishable from a user click, so the reveal fires over the live preview. Distinguish them by identity instead of timing: auto-switch records the image name in a small registry at dispatch, and the reveal effect consumes it on the selection's first render. Consumption happens on every rendered-image change, not only when the reveal conditions hold, because in the common (unraced) case the image renders with no progress showing and a leftover entry would suppress a genuine user selection of the same image later. Entries also expire after 30 seconds. Recording is unconditional but consumption requires the image to actually render, so a superseded auto-switch (two completions within one thumbnail-fetch window — routine with parallel multi-GPU sessions), a viewer unmounted by comparison mode, or a duplicate invocation_complete event would otherwise leave an immortal entry whose only future effect is to swallow a genuine click on that image — the very dead-click the reveal exists to prevent. The TTL is generous for the dispatch-to-render handoff it protects; expiring early merely readmits the 2-second flash on a very slow connection, which is the milder failure. The suppression branch still lowers $isTemporarilyShowingSelectedImage — the effect has already cancelled any running reveal's timer by that point, so returning with the atom raised would wedge the reveal on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lstein
force-pushed
the
fix/viewer-progress-image-handoff
branch
from
August 3, 2026 17:33
40a6a4c to
d86b6f6
Compare
Collaborator
|
@lstein Should this be held for v7? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
I have been working over a flaky Internet connection for the past few days, connecting to my home InvokeAI server over an internet connection that has 5-15% packet loss, depending on congestion. In this environment I observed that there is a long delay between the final VAE decode finishing and the viewer replacing the last progress preview with the finished image — and if the browser tab is backgrounded during a render, coming back often leaves the stale preview up permanently, fixable only by reloading the page.
Both are frontend bugs that are exacerbated by poor network latency.
While testing the fix I also hit a third, related race: starting a new generation soon after the previous one finishes made the viewer flash the previous generation's finished image over the new generation's live previews for two seconds. That fix is the second commit.
Why the reveal was slow
CurrentImagePreviewgated rendering behind an off-DOM preload ofimageDTO.image_url— the full-resolution PNG (/api/v1/images/i/{name}/full, often several MB).imageToRenderwas not set until that completed, and only the resultingonLoadcleared the overlay. So the stale latent preview stayed on screen for the entire full-resolution download.A 256px WEBP thumbnail is already generated for every image and was never used to shorten that wait — and it is typically higher resolution than the latent preview it replaces. The reveal is now gated on the thumbnail;
DndImagerenders it through Chakra's existingfallbackSrcand swaps the full image in, in place, once it finishes.The preload also used the raw
imageDTO.image_urlwhileDndImagerequestsuseMediaUrl(imageDTO.image_url), which appends?media_cookie_version=N(bumped to 1 on every authenticated app load). Different key, so the same bytes were fetched twice. Measured against a local server: 2 requests when the URLs differ, 1 when they match. Note the reuse is the document's list of available images, which is keyed by URL and is not the HTTP cache — so it still holds in multiuser mode, where images are servedCache-Control: private, no-store.Why the preview could stick forever
The viewer's progress atoms (
$progressEvent,$progressImage,$progressData) are a separate set of stores from the global ones inservices/events/stores.ts.setEventListenersalready resets the global stores onconnect/connect_error/disconnect; the viewer's had no such handler.python-socketio has no Connection State Recovery or event replay, and the app has no polling, no
visibilitychangehandling and norefetchOnFocus. So when the socket drops while the tab is backgrounded and the terminalqueue_item_status_changedlands in that gap, the event is gone for good — nothing is left to clear an overlay that is opaque (bg="base.900") and absolutely positioned over the already-loaded final image. Hence "only a reload fixes it".The viewer's atoms are now reset on the same socket lifecycle transitions.
Backstop for the deferred clear
onLoadImageis not a guaranteed callback:onLoad/onErroroff the DOM<img>wheneverfallbackSrcis supplied and fires them from its own internal preloader, so a failed load reportsonErrorand never clears.useImageonly re-runs whensrcchanges, so re-rolling onto the sameimage_nameproduces noonLoad.Any of those previously wedged the overlay until reload, so the armed clear now has a deadline. The armed flag and its timer live together in
createDeferredClear— as two independent pieces of state, a path that reset the flag but leaked the timer let a deadline outlive the generation that armed it and blank a later generation's live preview.Two refinements on that: the backstop does not clear while other sessions still have previews (nulling
$progressImagetears down the whole overlay, multi-GPU tiles included), and the reconnect reset only replaces the progress map when it actually holds something, sinceconnect_errorfires roughly once a second while the server is down.Why the previous image flashed over the next generation's previews (second commit)
Generating again quickly after a completion produced: new previews appear, then the previous generation's finished image covers them for two seconds, then the previews resume. Waiting a few seconds between generations avoided it.
The flash is the "reveal selected image" feature from #9217, which briefly hides the progress overlay when the rendered gallery image changes mid-generation, so a user's gallery click is not invisible under the opaque overlay. Its only guard against the auto-switch handoff was
$isProgressImageResolving— a timing guard, and the timing loses. The auto-switch selection is dispatched only afteronInvocationComplete's async DTO fetch, then waits for the thumbnail preload before rendering; the next generation's firstinvocation_progressevent slots into that window and resets the flag (it must — leaving the deferred clear armed is what used to blank a later generation's preview). By the time the handoff reaches the viewer it is indistinguishable from a user click, so the reveal fires over the live preview.The fix distinguishes the two by identity instead of timing: auto-switch records the image name in a small registry (
features/gallery/store/autoSwitchedImages.ts) at dispatch, and the reveal effect consumes the entry on the selection's first render — a consumed entry suppresses the reveal, while genuine gallery clicks (never recorded) reveal exactly as before.Two details that matter:
invocation_completeevent would otherwise leave an immortal entry whose only future effect is to swallow one genuine click. The TTL is generous for the dispatch-to-render window it protects; expiring early merely readmits the two-second flash on a very slow connection, which is the milder failure.An adversarial review of this mechanism turned up one pre-existing, out-of-scope observation worth recording: the duplicate-completion guard in
onInvocationCompleteonly skips the node-state upsert —addImagesToGallery(including the optimistic board-total increments) re-runs on duplicate events.Tests
The terminal-status policy moves into a pure
getTerminalProgressAction, and the timer intocreateDeferredClear, so both are testable without a socket or a React tree (this directory has no DOM test environment). 16 unit tests cover the decision matrix and the arm/disarm/re-arm lifecycle; mutation-checking confirms 2 of them fail if the leaked-timer bug is reintroduced. The remaining assertions incontext.test.tsare wiring checks in the style of the existingCurrentVideoPreview.test.ts. The auto-switch registry gets 7 more unit tests covering consume-once semantics, the pending bound, and TTL expiry (the factory takes an injectable clock).Verification
For the current head:
lint:tsc,lint:eslint,lint:prettier,lint:dpdmall clean; full suite 1743 tests passing (lint:knipand the production build left to CI).Not yet exercised against a live backend. End-to-end checks still worth doing before merge:
/fullis requested once, not twice).Follow-up (not in this PR)
The canvas staging area (
StagingArea/state.ts) has the same shape:$progressDatais never cleared on socket disconnect, and its progress image is only hidden onceonImageLoadedfires fromDndImage, giving it the same load-error wedge. It partially self-heals via thelistAllQueueItemsrefetch, so it is left out here to keep the diff reviewable.🤖 Generated with Claude Code