UIComponent: Optimize mouseMove and dragMouse methods #156
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
The old API is difficult to work with because it will continue to render floating components even if they've been removed from the component tree. Additionally it can CME if components are removed from the floating list during rendering, further complicating the workarounds required. This new API fixes the issue by tracking when components are removed/added from/to the tree and updating its internal floating list accordingly. It also allows setting the floating state at any time, even before the component has a parent, another thing the old API did not support. The order in which floating components appear also differs in the new API. While the old API showed floating components in the order in which they were set to be floating, this often isn't all too useful when the order in which components are added/removed to/from the tree is not particularily well defined. As such, the new API choses to instead order floating components in exactly the same way as they appear in the component tree (pre-order tree traversal, i.e. first parent, then children). This results in consistent ordering and is generally the order you want for nested floating components to behave in a useful way. This has been implemented as a new, completely separate API instead of an ElementaVersion primarily to easy migration (the new API can be used even with Windows still on older ElementaVersions; both APIs can be used at the same time) but also because there isn't anything reasonable the old-API methods in `Window` could do in the new version, they really should have been internal to begin with.
Intended to be used in almost all places where `animationFrame` was used before (only major exception being animated constraints, which will receive a different replacement). Unlike `animationFrame`, which runs at a fixed rate of (by default) 244 times per second, the [UpdateFunc] API is variable rate, meaning it'll be called exactly once per frame with the time that passed since the last frame. This allows it to match the true framerate exactly, animations won't slow down under high load, and it won't waste tons of CPU time on traversing the entire tree potentially mutliple times each frame. The UpdateFunc API notably also allows modifying the component hierarchy and accessing layout information directly from within a UpdateFunc call, both of which are quite tricky to do correctly from `animationFrame`.
Previously these would both just traverse the entire tree and evaluate the constraints of all components. In Essential, only components which can be dragged care about drag events, so most components don't, and since we primarily use the `hoveredState` from unstable Elementa, virtually no components need the mouseEnter/Leave listeners (which is what mouseMove is for). This commit thusly optimizes the two methods to only traverse the narrow subtree which has components that might be interested. For simplicity, this is a best-effort optimization (e.g. it may still visit components which have had such listeners at some point but no longer do now); it should however be completely correct in that it will behave idential to how it behaved prior to this commit, just consume less CPU where easily possible. Only exception are the `lastDraggedMouseX/Y` properties which are impossible to accurately emulate without traversing every component. They should really have been private and only in Window to begin with... This commit compromises by accepting that their behaivor may be different now if set manually or if dragMouse is called manually, but still preserves the rough meaning if it's merely read from in e.g. `mouseRelease`. The exact behavior of these properties was sufficiently unexpected that hopefully no one will have tried to rely on their exact behavior to begin with. Note that the Flags class and tracking introduced in this commit also support `Effect`s, despite neither of the two methods being available in `Effect`s. This is because we'll also use the same mechanism with `animationFrame` in the future.
RedEpicness
approved these changes
Mar 2, 2025
Sychic
approved these changes
Mar 2, 2025
| put(Window::class.java, Flags(0u)) | ||
| } | ||
| fun initialFor(cls: Class<*>): Flags = cache.getOrPut(cls) { | ||
| flagsBasedOnOverrides(cls) + initialFor(cls.superclass) |
There was a problem hiding this comment.
Wouldn't it make more sense to add the flags from overrides onto the initial flag set?
There was a problem hiding this comment.
That's literally what this does? I don't understand.
There was a problem hiding this comment.
I'm just being pedantic since if the left side is the first one and the right side is the other one, then it would imply that in this case, the override is the first one and the initial flags are the second.
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.
Previously these would both just traverse the entire tree and evaluate the constraints of all components.
In Essential, only components which can be dragged care about drag events, so most components don't, and since we primarily use the
hoveredStatefrom unstable Elementa, virtually no components need the mouseEnter/Leave listeners (which is what mouseMove is for).This commit thusly optimizes the two methods to only traverse the narrow subtree which has components that might be interested. For simplicity, this is a best-effort optimization (e.g. it may still visit components which have had such listeners at some point but no longer do now); it should however be completely correct in that it will behave idential to how it behaved prior to this commit, just consume less CPU where easily possible.
Only exception are the
lastDraggedMouseX/Yproperties which are impossible to accurately emulate without traversing every component. They should really have been private and only in Window to begin with... This commit compromises by accepting that their behaivor may be different now if set manually or if dragMouse is called manually, but still preserves the rough meaning if it's merely read from in e.g.mouseRelease. The exact behavior of these properties was sufficiently unexpected that hopefully no one will have tried to rely on their exact behavior to begin with.Note that the Flags class and tracking introduced in this commit also support
Effects, despite neither of the two methods being available inEffects. This is because we'll also use the same mechanism withanimationFramein the future.(technically depends on #155 to avoid merge conflicts (well, that didn't work) but is otherwise independent; the first three commits are from those PRs and have already been reviewed, this PR is the last commit only)