Experiment with an improved event design #1374
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,7 @@ use winapi::{ | |
| use crate::{ | ||
| dpi::{PhysicalDelta, PhysicalPosition, PhysicalSize, UnitlessDelta}, | ||
| event::{ | ||
| Event, Force, KeyEvent, LogicalKey, ModifiersState, PointerButton, | ||
| PointerId, RawKeyEvent, RawKeyboardEvent, RawPointerButtonEvent, RawPointerEvent, | ||
| WindowEvent, PointerTiltEvent, | ||
| }, | ||
|
|
@@ -1050,15 +1050,15 @@ unsafe extern "system" fn public_window_callback<T: 'static>( | |
|
|
||
| subclass_input.send_event(Event::WindowEvent( | ||
| WindowId(window).into(), | ||
| WindowEvent::ScrollStarted, | ||
| )); | ||
| subclass_input.send_event(Event::WindowEvent( | ||
| WindowId(window).into(), | ||
| WindowEvent::ScrollLines(UnitlessDelta::new(0.0, value)), | ||
| )); | ||
| subclass_input.send_event(Event::WindowEvent( | ||
| WindowId(window).into(), | ||
| WindowEvent::ScrollEnded, | ||
| )); | ||
|
|
||
| 0 | ||
|
|
@@ -1073,15 +1073,15 @@ unsafe extern "system" fn public_window_callback<T: 'static>( | |
|
|
||
| subclass_input.send_event(Event::WindowEvent( | ||
| WindowId(window).into(), | ||
| WindowEvent::ScrollStarted, | ||
| )); | ||
| subclass_input.send_event(Event::WindowEvent( | ||
| WindowId(window).into(), | ||
| WindowEvent::ScrollLines(UnitlessDelta::new(value, 0.0)), | ||
| )); | ||
| subclass_input.send_event(Event::WindowEvent( | ||
| WindowId(window).into(), | ||
| WindowEvent::ScrollEnded, | ||
| )); | ||
|
|
||
| 0 | ||
|
|
@@ -1460,7 +1460,7 @@ unsafe extern "system" fn public_window_callback<T: 'static>( | |
|
|
||
| let x = position.x as f64 + x.fract(); | ||
| let y = position.y as f64 + y.fract(); | ||
| let mut position = PhysicalPosition::new(x, y); | ||
|
|
||
| let pi = std::f64::consts::PI; | ||
| let deg_to_twist = |deg: u32| (deg as f64 / 360.0) * pi; | ||
|
|
@@ -1471,14 +1471,16 @@ unsafe extern "system" fn public_window_callback<T: 'static>( | |
| let pointer_id: PointerId; | ||
|
|
||
| let mut buttons_down = ButtonsDown::empty(); | ||
|
|
||
| match pointer_info.pointerType { | ||
| winuser::PT_TOUCH => { | ||
| pointer_id = PointerId::TouchId(TouchId(pointer_info.pointerId).into()); | ||
|
|
||
| buttons_down.set( | ||
| PointerButton::TOUCH_CONTACT.into_flags(), | ||
| pointer_info.pointerFlags & winuser::POINTER_FLAG_INCONTACT != 0 | ||
| ); | ||
|
|
||
| let mut touch_info = mem::MaybeUninit::uninit(); | ||
| if let Some(GetPointerTouchInfo) = *GET_POINTER_TOUCH_INFO { | ||
| let get_touch_info_result = GetPointerTouchInfo( | ||
|
|
@@ -1502,6 +1504,12 @@ unsafe extern "system" fn public_window_callback<T: 'static>( | |
| } | ||
| winuser::PT_PEN => { | ||
| pointer_id = PointerId::PenId(PenId(pointer_info.pointerId).into()); | ||
|
|
||
| buttons_down.set( | ||
| PointerButton::PEN_CONTACT.into_flags(), | ||
| pointer_info.pointerFlags & winuser::POINTER_FLAG_INCONTACT != 0 | ||
| ); | ||
|
|
||
| let mut pen_info = mem::MaybeUninit::uninit(); | ||
| if let Some(GetPointerPenInfo) = *GET_POINTER_PEN_INFO { | ||
| let get_pen_info_result = GetPointerPenInfo( | ||
|
|
@@ -1533,6 +1541,38 @@ unsafe extern "system" fn public_window_callback<T: 'static>( | |
| } | ||
| } | ||
| } | ||
| winuser::PT_MOUSE | | ||
| winuser::PT_TOUCHPAD => { | ||
| pointer_id = PointerId::MOUSE_ID; | ||
|
|
||
| // There can be some rounding errors when doing the mouse hiMetric math, | ||
| // and it doesn't make a whole lot of sense for a traditional mouse | ||
| // pointer to be on a non-integer point, so round off any fractional | ||
| // values that might've cropped up. | ||
| position.x = position.x.round(); | ||
| position.y = position.y.round(); | ||
|
Comment on lines
+1549
to
+1553
There was a problem hiding this comment.
I think this is a mistake. High-precision mouse pointers are widely supported and are useful for a number of purposes, e.g. in art applications. See prior discussion in #1375. There was a problem hiding this comment. High-precision mouse pointers are typically from pen/touch devices, which this rounding does not effect. If there are any relatively widely-used devices that this change would negatively effect I'd be open to reverting this, but otherwise I think it's an appropriate action to take for standard pixel-aligned mouse devices. There was a problem hiding this comment. Touchpads are a touch device. I'm not willing to assume without evidence that Windows always discards high-precision data from mice, either. There was a problem hiding this comment. Do you mean that touchpads are mouses actually? IIRC touchpads do have high precision data, but are treated as pointers on at least Wayland. So yeah, removing fractional part is likely not a good thing to do and I don't see much issue with it? Also, even mouse on Wayland will have fractional part by default, so I'm not sure that things should diverge here. clients can always round themselves if they don't care, I guess. There was a problem hiding this comment. Sorry, the terminology gets a bit confusing here. I mean that touchpads are high-precision input devices driven by a finger which can, should, and on non-Windows backends do have that precision propagated to the application. Even conventional mice can be, contrary to the comment above. There's no reason a conventional mouse should be treated differently either, as you observe. Silently discarding perfectly good data is bad behavior. There was a problem hiding this comment.
I'd like to emphasize that high-precision pointers absolutely can be useful. For example, any time you're interacting with a vector or zoomed-out raster graphics canvas, or operating a continuous UI control like a slider. There was a problem hiding this comment.
Ah, true there, I have actually noticed that on some KDE sliders where I can barely adjust them but the cursor doesn't actually move... There was a problem hiding this comment.
I have a laptop that uses the Windows Precision Touchpad drivers. When using the touchpad, Windows does not dispatch sub-pixel pointer movement events, and each movement event's position is within a
Are you on a Windows system? If so, could you check whether or not diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs
index 3702d352..b1d7ae8c 100644
--- a/src/platform_impl/windows/event_loop.rs
+++ b/src/platform_impl/windows/event_loop.rs
@@ -1549,8 +1549,8 @@ unsafe extern "system" fn public_window_callback<T: 'static>(
// and it doesn't make a whole lot of sense for a traditional mouse
// pointer to be on a non-integer point, so round off any fractional
// values that might've cropped up.
- position.x = position.x.round();
- position.y = position.y.round();
+ // position.x = position.x.round();
+ // position.y = position.y.round();
buttons_down.set(
PointerButton::MOUSE_LEFT.into_flags(),There was a problem hiding this comment. If it is ultimately determined that windows genuinely never reports non-integer mouse/touchpad coordinates, then at the very least the comment should reflect that, rather than claiming there's no use case for higher precision data. That way if a future change makes higher-precision data available there won't be cause for confusion as to whether it should be discarded. There was a problem hiding this comment.
I'm not, linux with a custom mouse driver because my mouse is... unique. |
||
|
|
||
| buttons_down.set( | ||
| PointerButton::MOUSE_LEFT.into_flags(), | ||
| pointer_info.pointerFlags & winuser::POINTER_FLAG_FIRSTBUTTON != 0, | ||
| ); | ||
| buttons_down.set( | ||
| PointerButton::MOUSE_RIGHT.into_flags(), | ||
| pointer_info.pointerFlags & winuser::POINTER_FLAG_SECONDBUTTON != 0, | ||
| ); | ||
| buttons_down.set( | ||
| PointerButton::MOUSE_MIDDLE.into_flags(), | ||
| pointer_info.pointerFlags & winuser::POINTER_FLAG_THIRDBUTTON != 0, | ||
| ); | ||
| buttons_down.set( | ||
| PointerButton::MOUSE_X1.into_flags(), | ||
| pointer_info.pointerFlags & winuser::POINTER_FLAG_FOURTHBUTTON != 0, | ||
| ); | ||
| buttons_down.set( | ||
| PointerButton::MOUSE_X2.into_flags(), | ||
| pointer_info.pointerFlags & winuser::POINTER_FLAG_FIFTHBUTTON != 0, | ||
| ); | ||
| } | ||
| _ => return 0, | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also wonder the idea behind event for start/stop scrolling, seems like events for ScrooLine and ScrollPixels are already enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We exposed a
TouchPhasefield on the oldMouseWheelevent andScrollStarted/ScrollEndedallow us to expose that functionality in the new API.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like the naming/docs are pretty confusing about those. The docs are saying that they are about touchscreen, in reality they are used for some mouse/touchpad scroll as well. Also old API is using
TouchPhase::Ended, when you've removed all your fingers from the touch surface, so using scroll here is just confusing. If we take into account the otherPointerevents handling of touch is just more confusing at this point, unless you want to force users to track touch phase themself.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that implying that the scroll phase and the touchscreen touch phase are related is confusing, why is why this redesign doesn't do that.
TouchPhaseinMouseWheelwas originally exposed in order to implement overscrolling on macOS, and this design should let us continue to do that. The exact API guarantees around whenScrollStartedandScrollEndedget emitted should be specified in the docs, though.