Stop the workspace row rebuilding itself for no visible change - #9440
Open
VykosMolt wants to merge 1 commit into
Open
Stop the workspace row rebuilding itself for no visible change#9440VykosMolt wants to merge 1 commit into
VykosMolt wants to merge 1 commit into
Conversation
A Repeater compares its model by identity, not by contents: hand it a new array and it destroys every delegate and builds them again, however alike the two arrays are. The row's model was `root.workspaceIds()`, a function call in a binding that allocated a fresh array every time it ran, and it re-ran whenever Hyprland's workspace list changed. `columns:` called the same function a second time. Hyprland's workspace list changes when a workspace is created or destroyed, which is what happens the moment you touch an empty one. Moving between workspaces that hold windows changes nothing and is smooth. Pressing SUPER+4 from an occupied workspace created 4 and rebuilt the whole row, and leaving 4 empty destroyed it and rebuilt the row again. That is why the stutter came and went rather than being there every time. The waste is worse than it first looks. The row always offers 1 through 5, so creating workspace 4 does not change what it draws at all: the ids are [1,2,3,4,5] before and after. Every one of those rebuilds produced an identical row. It only genuinely changes when a workspace outside 1-5 appears or goes away. The ids now live in a property that is replaced only when the ids really differ, so the array's identity survives a change that does not alter the row. The delegate's `workspaceById()` went with it. Each delegate scanned the whole workspace list to find its own workspace, which made drawing the row O(n^2) every time it was rebuilt. The map is now built once per change. The model math moved to WorkspacesModel.js, matching MenuModel.js and NightlightModel.js, so it can be unit tested under node. It also holds a workspace list more defensively than the inline version did: a null entry in Hyprland.workspaces.values used to throw reading `.id`, and a non-numeric id used to be pushed into the row.
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.
A
Repeatercompares its model by identity, not by contents: hand it a new array and it destroys every delegate and builds them again, however alike the two arrays are. The row's model isroot.workspaceIds(), a function call in a binding that allocates a fresh array every time it runs, and it re-runs whenever Hyprland's workspace list changes.columns:calls the same function a second time.Hyprland's workspace list changes when a workspace is created or destroyed, which is what happens the moment you touch an empty one. Moving between workspaces that hold windows changes nothing and is smooth. Pressing SUPER+4 from an occupied workspace creates 4 and rebuilds the whole row, and leaving 4 empty destroys it and rebuilds the row again. That is why the stutter comes and goes rather than being there every time.
The waste is worse than it first looks. The row always offers 1 through 5, so creating workspace 4 does not change what it draws at all — the ids are
[1,2,3,4,5]before and after. Every one of those rebuilds produces an identical row. It only genuinely changes when a workspace outside 1-5 appears or goes away.The ids now live in a property that is replaced only when the ids really differ, so the array's identity survives a change that does not alter the row.
The delegate's
workspaceById()goes with it. Each delegate scanned the whole workspace list to find its own workspace, which made drawing the row O(n²) every time it was rebuilt. The map is built once per change instead.The model math moved to
WorkspacesModel.js, matchingMenuModel.jsandNightlightModel.js, so it can be unit tested under node.Testing
workspaces-model-test.shis new. Under node it checks the row always offering 1-5, a workspace outside that range joining and leaving, ids out of range and junk entries being ignored, and then the case this is about:stableIdsreturns the same array object when creating or destroying a workspace inside 1-5, and a different one when the row genuinely changes. Same length with different ids must not be mistaken for unchanged, and a first run with nothing to compare against still returns the new array.Against the QML it checks that the widget assigns through
stableIds, that neithermodel:norcolumns:allocates a fresh array in a binding any more, and that the delegate reads the prebuilt map rather than scanning. The whole file fails on quattro, whereWorkspacesModel.jsdoes not exist.While moving the model math out I made it hold the workspace list more defensively than the inline version did: a
nullentry inHyprland.workspaces.valuesused to throw reading.id, and a non-numeric id used to be pushed into the row. Both are covered../test/shell— 218 of 222 files pass; the four failures (config-test,runtime-smoke-test,snapper-test,unowned-system-paths-test) fail identically on unmodified quattro on this machine.This touches the same file as #9419, which changes how the click is dispatched. They are independent changes to different parts of the widget; whichever lands second needs a trivial rebase.
This comes out of https://github.com/VykosMolt/omarchy-desktop, where I have been running the Quattro shell as a plain Arch session and fixing what turned up.