Repository navigation
[Merged by Bors] - fix cursor grab issue - #7010
VitalyAnkh wants to merge 2 commits into
Conversation
|
Oh no this doesn't solve the problem... The crash only happens on x11 platform for me, and it's always OK on wayland platform. This change doesn't make a difference. |
b11cc85 to
dec2611
Compare
|
This should work now. |
|
Sorry, I missed your PR, I just saw the original issue title and went "oh right, we only fixed that at update time, not at creation time" I can't test on Linux, I'll trust you if you say it's just an issue of it needed to being delayed. Fallback should still be added for Windows and macOS support. |
|
The fallback logic should be already in our codebase: bevy/crates/bevy_winit/src/lib.rs Line 143 in 1aeaafa bevy::window::Window::create_window invokes bevy::window::Window::set_cursor_grab which is actually a wrapper ofbevy_winit::change_window in the above.
This also eliminates code duplicate 😃. |
| } | ||
|
|
||
| /// Map [`bevy_window::CursorGrabMode`] to [`winit::window::CursorGrabMode`]. | ||
| pub fn convert_cursor_grab_mode(mode: CursorGrabMode) -> winit::window::CursorGrabMode { |
There was a problem hiding this comment.
This is non-blocking but I'm going to Chesterton-fence you here: What purpose did this serve? I notice it is marked pub so technically this is breaking change. Though such a conversion method defined as a free function is really weird.
There was a problem hiding this comment.
This is only for bevy crates' internal usage. I introduced this function for bevy_window abstracts its own CursorGrabMode struct because bevy_window doesn't want to depend on winit, then bevy_winit should convert bevy_window's CursorGrabMode to winit::window::CursorGrabMode. I think this function shouldn't be pub but I just followed these conversion functions' signature above. Anyway, we could remove it now and it shouldn't break much code since it's not intended for end users in the beginning. If this breaks the semantic version, please help us find a way to get of it... Sorry for this mistake.
There was a problem hiding this comment.
Don't worry. This is perfectly fine.
There was a problem hiding this comment.
This isn't a public module so this isn't a breaking change.
| height: 300., | ||
| present_mode: PresentMode::AutoVsync, | ||
| always_on_top: true, | ||
| cursor_grab_mode: CursorGrabMode::Confined, |
There was a problem hiding this comment.
Does the example here need to be changed? Feels like this just adds noise to the example without an explanation why users should set this. If it should, we should document what this is actually doing so that people learning from the example can improve their understanding.
There was a problem hiding this comment.
I just want to show users they could set cursor grab mode directly. But they will do this without guidance since this field is already in WindowDescriptor. I will remove this change. Thanks!
| ); | ||
| // Do not set the grab mode on window creation if it's none, this can fail on mobile | ||
| if window_descriptor.cursor_grab_mode != CursorGrabMode::None { | ||
| window.set_cursor_grab_mode(window_descriptor.cursor_grab_mode); |
There was a problem hiding this comment.
I think I would prefer if this still logged an error here, so the user gets some feedback rather than silently failing
There was a problem hiding this comment.
Sorry for the late reply. I'm experiencing covid-19 this week.
Like the fallback will be done in bevy_winit::change_window, the errors, if any, will also be logged there:
bevy/crates/bevy_winit/src/lib.rs
Line 150 in 1aeaafa
There was a problem hiding this comment.
oh gotcha nvm, my brain was in windows as entities mode and I forgot we had window commands
IceSentry
left a comment
There was a problem hiding this comment.
I didn't experience the bug, so can't confirm if it's fixed, but the code makes sense to me and mouse grabbing still works as expected for me.
|
bors r+ |
# Objective - Set the cursor grab mode after the window is built, fix #7007, clean some conversion code. ## Solution - Set the cursor grab mode after the window is built.
|
Pull request successfully merged into main. Build succeeded:
|
# Objective - Set the cursor grab mode after the window is built, fix bevyengine#7007, clean some conversion code. ## Solution - Set the cursor grab mode after the window is built.
# Objective - Set the cursor grab mode after the window is built, fix bevyengine#7007, clean some conversion code. ## Solution - Set the cursor grab mode after the window is built.
Objective
Solution