fix(ui): always enqueue with fresh staging area bbox #6184
Merged
Conversation
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
maryhipp
requested review from
psychedelicious,
blessedcoolant and
hipsterusername
as code owners
maryhipp
changed the title
|
@psychedelicious I also considered clearing it programmatically in the canvas slice when a cancel request got fired but this felt like it would cover more cases and I couldn't think of a reason that we wouldn't want to always set it fresh. |
...frontend/web/src/app/store/middleware/listenerMiddleware/listeners/enqueueRequestedCanvas.ts
Outdated
Show resolved
Hide resolved
|
This was a bit of a doozy, the flow is pretty tangled. I've pushed fixes for a few edge cases, including the one initially addressed by the PR. Details in the commit message. The reason it's tangled is described in #6190 . Essentially, we are doing a lot of manual client-side tracking of batches and generations, and that all has to be carefully synced. Not going to make the deeper change suggested in that issue now. |
psychedelicious
approved these changes
Apr 10, 2024
hipsterusername
approved these changes
Apr 10, 2024
This reverts commit fae51da.
Handful of intertwined fixes. - Create and use helper function to reset staging area. - Clear staging area when queue items are canceled, failed, cleared, etc. Fixes a bug where the bbox ends up offset and images are put into the wrong spot. - Fix a number of similar bugs where canvas would "forget" it had pending generations, but they continued to generate. Canvas needs to track batches that should be displayed in it using `state.canvas.batchIds`, and this was getting cleared without actually canceling those batches. - Disable the `discard current image` button on canvas if there is only one image. Prevents accidentally canceling all canvas batches if you spam the button.
psychedelicious
force-pushed
the
maryhipp/fix-canvas-bbox-on-cancel
branch
from
b05d555 to
f08de5f
Compare
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
When a canvas invocation was canceled, the next invocation would use the previous "staging area", including the bounding box settings. This change will make sure that every requested enqueue uses the current bounding box.
Related Issues / Discussions
Closes #5969
QA Instructions
Put an image on canvas
Set bounding box
Invoke
Cancel before it can finish
Move bounding box
Invoke again
Previously, the second invocation would use the bounding box from invocation #1. With this fix, it should use the bounding box from invocation #2 as expected.
Merge Plan
Checklist