feat(ui): redesign followups #5368
Merged
Conversation
This file contains 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
Fixes issue where translations overflowed due to hardcoded widths.
This uses the previous implementation of the memoization function in reselect. It's possible for the new weakmap-based memoization to cause memory leaks in certain scenarios, so we will avoid it for now.
Far more efficient than the crude redux incrementor thing.
More efficient selectors, memoized/stable references to objects, lazy popover/menu rendering.
This can cause stuttering when nodes are being moved in and out of the viewport. I think it's better to improve rendering/perf in other ways.
- disable listening when not needed - use useMemo for gridlines
Includes vite v5 - only change needed is to set .mts for vite config files.
This greatly reduces the weight of the event handlers.
Need an extra ref to pass to the InvSelect component.
Manually hook into pubsub to eliminate extraneous rerenders on hook change
Need to make the menu not lazy. A better solution is to refactor how the settings works, rendering it in a different part of the component tree
This drastically reduces the computation needed when moving the cursor. It also correctly separates ephemeral interaction state from redux, where it is not needed. Also removed some unused canvas state.
Flattens the `nodes` slice. May offer minor perf improvements in addition to just being cleaner.
This reduces top-level rerenders when zooming in and out on workflow editor
- Store workflow in nanostore as singleton instead of building for each consumer - Debounce the build (already was indirectly debounced) - When the workflow is needed, imperatively grab it from the nanostores, instead of letting react handle it via reactivity
This reduces rerenders when the user presses a modifier key.
This provides a small performance improvement, on the order of a few ms per interaction.
This ensures the context menus don't get cut off when the window size is very small.
Closes #5292 The special handling for single prompt is totally extraneous and caused a bug.
psychedelicious
requested review from
maryhipp and
hipsterusername
as code owners
hipsterusername
approved these changes
Jan 1, 2024
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.
What type of PR is this? (check all applicable)
Have you discussed this change with the InvokeAI team?
Description
This PR is a followup to #5270, incorporating many fixes, enhancements and optimizations.
Minor fixes and enhancements
Performance improvements
Thanks to the peeps on discord for reporting and investigating performance issues on canvas and workflow editor.
I did some deeper analysis in firefox and chrome to identify the areas of the code that we have the ability to change.
Unsurprisingly, most performance issues I identified were related to our growing redux store and high-frequency state changes. Said state changes are mostly due to user interaction - like moving the mouse around.
I've shifted state around or out of redux in a number of spots to keep things ephemeral state separate from redux. I also optimized a number of components and selectors that were too generous with what they selected.
Rerenders are greatly reduced, and the necessary rerenders are faster. The workflow editor and canvas are now substantially more performant overall.
Workflow building function and zod
The workflow building function was being called multiple times when workflow editor state changed. The function was debounced by 300ms, because it is pretty slow. Another inefficiency meant that this was called once per consumer (4 in the app right now). Calling it 4x usually incurred a major GC, making the user impact even greater. For large workflows, we are talking hundreds of ms, even over one second, of blocking - and that's on my fairly high-end machine.
The workflow building function uses zod to parse nodes and edges into a runtime-validated workflow object. The workflow nodes and edges are modeled as zod unions.
zod parses unions by testing the object being parsed against each union member until it hits a match. Each node in a workflow needs to be parsed against a union of all possible nodes (currently ~130 core nodes). A failed parse take much longer than a successful parse, so this process can take a while. A discriminated union would be much faster, but we can't use that for our models (the discriminator must be a literal, but ours are necessarily just strings).
I implemented a zod-less version of the workflow builder func that's 1 to 3 orders of magnitude faster than the zod version and fixed the logic so that this is only called once. Muuuuuch better.
(A zod-based workflow building function is still used when enqueuing, because we really don't want bad workflows ending up in the db.)
TODO: Workflow editor selector inefficiencies
The other big hit in the workflow editor is related to how nodes get their state from redux. Each node uses a number of hooks with carefully memoized selectors. This was done to reduce rerenders, but there is still a performance cost to a memoized selector.
I made one minor improvement by using a different memoization function for the args of each selector, but theres still this flat cost, per selector, per tick.
It may be more performant to just pass the node state around in props, or track each node state in eg nanostores/refs. Redux would be the source of truth either way. The only way to really tell is to actually implement it, but it's not a small change, so this will have to wait.
Not fixed
The model manager and canvas bounding box are not fixed. I'll tackle those tomorrow.
Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Fire it up and have a whirl around canvas and workflow editor. Should be noticeably snappier.
Merge Plan
This PR may be merged when approved.
Added/updated tests?