Fix an issue with regional guidance and multiple quick-queued generations after moving bbox #8613
Conversation
After moving the canvas bbox we still handed out the previous regional-guidance mask because only two parts of the system knew anything had changed. The adapter’s cache key doesn’t include the bbox, so the next few graph builds reused the stale mask from before the move; if the user queued several runs back‑to‑back, every background enqueue except the last skipped rerasterizing altogether because another raster job was still in flight. The fix makes the canvas manager invalidate each region adapter’s cached mask whenever the bbox (or a related setting) changes, and—if a reraster is already running—queues up and waits instead of bailing. Now the first run after a bbox edit forces a new mask, and rapid-fire enqueues just wait their turn, so every queued generation gets the correct regional prompt.
kyhavlov
requested review from
blessedcoolant,
hipsterusername,
maryhipp and
psychedelicious
as code owners
kyhavlov
changed the title
lstein
requested review from
Copilot
and removed request for
maryhipp and
psychedelicious
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where regional guidance was ignored during the first few quick-queued generations after moving the bbox on the canvas. The root cause was that regional guidance mask cache entries were not invalidated when the bbox changed, and concurrent rasterization requests would bail out instead of waiting, causing stale masks to be used.
Key changes:
- Invalidate regional guidance mask caches when bbox changes from canvas interactions
- Replace assertion failure with waiting logic when rasterization is already in progress
- Track cache keys in adapters to enable targeted cache invalidation
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| CanvasStateApiModule.ts | Added waitForRasterizationToFinish method and cache invalidation call in setGenerationBbox |
| CanvasManager.ts | Added invalidateRegionalGuidanceRasterCache to invalidate all regional guidance adapter caches |
| CanvasEntityObjectRenderer.ts | Added rasterCacheKeys tracking, changed concurrent rasterization handling from assertion to waiting, and implemented invalidateRasterCache |
| CanvasEntityAdapterBase.ts | Added invalidateRasterCache method that delegates to the renderer |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
invokeai/frontend/web/src/features/controlLayers/konva/CanvasStateApiModule.ts
Show resolved
Hide resolved
…anvasStateApiModule.ts Fixes race condition identified during copilot review. Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
invokeai/frontend/web/src/features/controlLayers/konva/CanvasStateApiModule.ts
Show resolved
Hide resolved
lstein
left a comment
lstein
left a comment
There was a problem hiding this comment.
I was able to reproduce the original problem, and the proposed fix seems to work. Code review by copilot detected a race issue, and its suggested change was accepted and committed.
…tateApiModule.ts Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...eai/frontend/web/src/features/controlLayers/konva/CanvasEntity/CanvasEntityObjectRenderer.ts
Show resolved
Hide resolved
invokeai/frontend/web/src/features/controlLayers/konva/CanvasEntity/CanvasEntityAdapterBase.ts
Show resolved
Hide resolved
...eai/frontend/web/src/features/controlLayers/konva/CanvasEntity/CanvasEntityObjectRenderer.ts
Outdated
Show resolved
Hide resolved
...eai/frontend/web/src/features/controlLayers/konva/CanvasEntity/CanvasEntityObjectRenderer.ts
Show resolved
Hide resolved
invokeai/frontend/web/src/features/controlLayers/konva/CanvasStateApiModule.ts
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
2 participants
The way this issue manifested for me was that every time i would move the bbox in the canvas and generate, the first couple generations would ignore the regional guidance completely, and then after those first couple, everything worked as expected and regional guidance was included. I looked into it a bit more and realized it only happened when i quickly queued up a few generations at once by clicking rapidly - if I clicked 3 times, the first 2 would ignore the regional layers and the 3rd image generation worked correctly.
What seems to be happening is:
After moving the canvas bbox, Invoke still handed out the previous regional-guidance mask because only two parts of the system knew anything had changed. The adapter’s cache key doesn’t include the bbox, so the next few graph builds reused the stale mask from before the move; if the user queued several runs back‑to‑back, every background enqueue except the last skipped rerasterizing altogether because another raster job was still in flight.
The fix makes the canvas manager invalidate each region adapter’s cached mask whenever the bbox (or a related setting) changes, and, if a reraster is already running, queues up and waits instead of bailing. Now the first run after a bbox edit forces a new mask, and rapid-fire enqueues just wait their turn, so every queued generation gets the correct regional prompt.
I don't know if this is the best way to fix this, but I figured it'd be helpful to post the bug investigation info and a fix that at least worked for me. I did use Codex to help investigate/write the fix, but I looked through the changes and associated code and nothing seems obviously wrong to me, at least.