Remove cursor caching #19629
Remove cursor caching #19629
Conversation
janhohenheim
added
C-Bug
There was a problem hiding this comment.
Important to note Window is modified to set cursor position when a user moves the mouse, so this may be triggered every frame for some kinds of games. I don't know the cost of the calls made here so I can't say if that's a problem or not.
Came up in discord https://discord.com/channels/691052431525675048/1253771396832821270/1383504960469209239
Potential perf aside removing caching looks good
janhohenheim
added
S-Ready-For-Final-Review
|
A more sophisticated version could split the |
|
I just realized a side effect of this change is that if you set CursorGrabMode::Locked then any time you mutate the window, it will try to re-lock the cursor. I can't think of a realistic situation where this would break things because its so rare to change window properties on the web but I feel like there are could be edge cases where this would result in something strange. For example resizing the window would spam attempts to lock the cursor but because there aren't triggered by a click, it would probably would just spam console errors but not actually do anything. |
|
@jf908 I can check. I'm planning on splitting the Window struct afterwards anyways, so any issue like that will be addressed by the followup PR :) |
|
Bad news: due to #8949, unlocking your cursor on web now becomes extremely icky, as Bevy just tries and tries again to lock the cursor, resulting in it staying locked even when trying to free it! Screencast.From.2025-06-16.05-11-26.mp4This means that for cursor caching to be gone, we really do need working change detection |
janhohenheim
removed
the
S-Ready-For-Final-Review
# Objective - Fixes #19627 - Tackles part of #19644 - Supersedes #19629 - `Window` has become a very very very big component - As such, our change detection does not *really* work on it, as e.g. moving the mouse will cause a change for the entire window - We circumvented this with a cache - But, some things *shouldn't* be cached as they can be changed from outside the user's control, notably the cursor grab mode on web - So, we need to disable the cache for that - But because change detection is broken, that would result in the cursor grab mode being set every frame the mouse is moved - That is usually *not* what a dev wants, as it forces the cursor to be locked even when the end-user is trying to free the cursor on the browser - the cache in this situation is invalid due to #8949 ## Solution - Split `Window` into multiple components, each with working change detection - Disable caching of the cursor grab mode - This will only attempt to force the grab mode when the `CursorOptions` were touched by the user, which is *much* rarer than simply moving the mouse. - If this PR is merged, I'll do the exact same for the other constituents of `Window` as a follow-up ## Testing - Ran all the changed examples
Objective
Solution
Changed<Window>Testing
I ran the window_settings.rs, changed the window settings and the cursor capturing to verify that everything still works the same.