[Windows] Reverse horizonal scroll direction in WindowEvent::MouseWheel
#2101
Conversation
aevyrie
changed the title
WindowEvent::MouseWheel
| @@ -1200,7 +1200,7 @@ unsafe fn public_window_callback_inner<T: 'static>( | |||
|
|
|||
| let value = (wparam >> 16) as i16; | |||
| let value = value as i32; | |||
| let value = value as f32 / winuser::WHEEL_DELTA as f32; | |||
| let value = -value as f32 / winuser::WHEEL_DELTA as f32; | |||
There was a problem hiding this comment.
https://docs.microsoft.com/en-us/windows/win32/api/windowsx/nf-windowsx-get_y_lparam
Use GET_Y_LPARAM instead of HIWORD to extract signed coordinate data. Negative screen coordinates may be returned on multiple monitor systems.
There was a problem hiding this comment.
@aloucks could you expand on the relevance of this please? I'm not familiar with the win32 api.
There was a problem hiding this comment.
Is the suggestion to use lparam instead of wparam?
There was a problem hiding this comment.
I think I misread part of https://docs.microsoft.com/en-us/windows/win32/inputdev/wm-mousehwheel and thought the negative value might be related. I think you can disregard my previous comment.
Have you checked to see how GLFW treats the mouse wheel direction? I think we should just use that prior art as reference and have the same behavior.
There was a problem hiding this comment.
There was a problem hiding this comment.
Awesome, if I read this correctly, this PR follows GLFW prior art.
The previous hscroll was based on a wrong precedence set by windows. The PR to fix this on windows is at rust-windowing#2101 With this PR we now match what GLFW does: https://github.com/glfw/glfw/blob/53d86c64d709ff52886580d338d9b3b2b1f27266/src/cocoa_window.m#L614:L627 This will remove the need for a workaround in my egui crate at https://github.com/emilk/egui/blob/master/egui-winit/src/lib.rs#L456-L460
The previous hscroll was based on a wrong precedence set by windows. The PR to fix this on windows is at rust-windowing#2101 With this PR we now match what GLFW does: https://github.com/glfw/glfw/blob/53d86c64d709ff52886580d338d9b3b2b1f27266/src/cocoa_window.m#L614:L627 This will remove the need for a workaround in my egui crate at https://github.com/emilk/egui/blob/master/egui-winit/src/lib.rs#L456-L460
|
I've cherry-picked the changes in this PR into #2105, so I suggest we close this PR in favor of that more all-encompassing PR (which also includes updated documentation). |
|
Sounds great. Thanks! |
cargo fmthas been run on this branchcargo docbuilds successfullyCHANGELOG.mdif knowledge of this change could be valuable to usersFixes #2099
Addresses part of downstream issue emilk/egui#356
On windows, the horizonal and vertical scrolling directions do not match when running the MRE in #2099. Considering vertical scroll is used in many places already, it seems sensible to swap horizontal scroll deltas on windows, like #1695.
This PR also changes the
mouse_wheelexample to demonstrate scroll behavior. In its current state, it only reports mouse device events, which don't seem to report horizontal scrolling on windows. I've changed this toWindowEvent::MouseWheel, which allows you to test x and y scrolling - which seems to be the intent of the example considering it already allows repositioning the window in x and y. I've also flipped the delta applied to the window, so it moves in the direction the page you are scrolling would move. This makes it easier to verify "natural" scrolling is working as expected.