-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Pen/Stylus support #3810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Pen/Stylus support #3810
Conversation
|
|
||
| #[derive(Clone, Copy, Debug, PartialEq)] | ||
| #[cfg_attr(feature = "serde", derive(Deserialize, Serialize))] | ||
| pub struct ToolState { |
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 would be glad for an alternative to the term "Tool" or "State".
Originally got the idea from Linux, because we need a term that applies to all sorts of different tools.
But honestly I think "Pen" would have been fine as well?
Maybe "Stylus" would fit all the possible tools:
- Pen
- Eraser
- Brush
- Pencil
- Airbrush
This aligns with the future Std type name and allows us to introduce `LazyCell`.
aac4f31 to
c3b84f8
Compare
| event: WindowEvent::CursorMoved { | ||
| device_id: mkdid(util::VIRTUAL_CORE_POINTER), | ||
| position: location.cast(), | ||
| r#type: CursorType::Mouse, | ||
| }, | ||
| }; | ||
| callback(&self.target, event); |
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.
Mouse cursor position changes when touch events are received.
Only the first concurrently active touch ID moves the mouse cursor.
This is a bit confusing to me, especially because I believe it doesn't map to other backends. Why do we tell the user that the mouse moves if the cursor moves with touch input?
Mouse input is cursor movement, but not all cursor movement is mouse input.
I'm really not sure what the purpose of this is.
| CursorLeft { device_id: DeviceId }, | ||
|
|
||
| /// A cursor button press has been received. | ||
| CursorInput { device_id: DeviceId, state: ElementState, button: CursorButton }, |
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.
At this point we should probably rename "Cursor" to "Pointer".
"Cursor" commonly refers to the visual representation of an input device on the screen.
But "Pointer" is a broader category that can include any device and is not specific to its visual representation on the screen.
I couldn't really find anything concrete on this online.
WDYT?
| let event = Event::WindowEvent { | ||
| window_id, | ||
| event: WindowEvent::CursorMoved { device_id, position }, | ||
| event: WindowEvent::CursorMoved { device_id, position, r#type: CursorType::Mouse }, |
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 don't know how to ask X11 what kind of device this is.
Right now it reports my pen as a mouse.
Same in CursorEntered and CursorLeft.
Same issue in DeviceEvents.
Interestingly MouseWheel contains the pen angles, but MouseMotion should probably be excluded for pens as well, unless we want to rename it to CursorMotion (or PointerMotion).
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.
Okay I figured this one out as well.
After digging around in Chromium for a while I found how they determined if this was a stylus or not.
After some digging, I also found that Glazier was doing something quite similar, in addition to detecting erasers (in a hacky way, but it works). There is a lot to learn from them when actually implementing the backends for Pens/Stylus's.
Additionally, after some more digging into X11, I found that there is an alternative to figuring out the type: XListInputDevices. That's right, using the old XInput version and no way to query individual devices. This exposes what they call "Device Types", which unfortunately don't include pens, but they let you recognize the drawing tablet, which works as well.
I found that Chromium was doing something similar for touchpads, but not for pens (and a comment in a much older revision explaining this as well). So maybe this will be useful for us in the future.
| let event = Event::WindowEvent { | ||
| window_id, | ||
| event: WindowEvent::CursorMoved { device_id: mkdid(pointer_id as _), position }, | ||
| event: WindowEvent::CursorMoved { | ||
| device_id: mkdid(pointer_id as _), | ||
| position, | ||
| r#type: CursorType::Mouse, | ||
| }, | ||
| }; |
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.
This is also a bit confusing to me.
AFAIU focusing the window is not a mouse event and the mouse didn't move.
| let Some(DeviceType::Mouse) = self | ||
| .devices | ||
| .borrow() | ||
| .get(&DeviceId(event.sourceid as xinput::DeviceId)) |
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.
This came as a surprise to me, but I had to spend quite some time to figure out why this wasn't working for me.
deviceId here uses the cursor, but sourceId actually uses the device that moved the cursor. Shouldn't we use that to send DeviceId to the user?
- This adds pen/stylus support. - Move `Force::Calibrated::altitude_angle` to `ToolAngle::altitude`. - `Force::normalized()` now takes a `Option<ToolAngle>` to calculate the perpendicular force. - Just introducing types, no implementation yet!
Add a new `CursorButton` and `ToolButton` type.
Fixed: device events are emitted regardless of cursor type.
|
Great work on this PR and the pointer event overhaul! May I ask what the status of this PR is? This is something I have wanted for quite some time. I made an implementation based on this PR (with some modifications) for wayland https://github.com/WhiteboardCX/winit/tree/tablet
If my changes are OK, I can also make a PR. I should have hardware to test all platforms, but implementing all of them will take some time. |
It's nominated for our next meeting, which should happen sometime in start January |
|
Thanks. In the meantime, I added support for windows to my fork and created a small demo based on vello. |
|
Does this PR need to be redone on top of #3876? I have a lot of free time right now and like the above poster I did a test implementation of exposing tablet events on Windows. I could do the same for linux, android, and web too if this PR's creator is busy. https://github.com/wareya/winit/tree/tablet |
|
Hey, I think my version of this was already on top of that commit https://github.com/WhiteboardCX/winit/tree/tablet I would be happy to work with you on this. In principle, I also have Mac and iPad hardware, so I could try adding support for these as well. At this point my fork probably needs some work again to run on top of the latest master branch, and it would be helpful if someone else went over my design decisions. |
|
Thanks for the interest. I unfortunately don't think we're equipped to review such work at the moment (I plan to post something about that soon), would it be okay if I maybe got back to you in a few months? |
|
Are you planning on doing work that would require us to rewrite whatever we come up with, like swapping out platform integrations? If not, I'm willing to do whatever you guys need to make reviewing easier (e.g. limiting its scope, or sticking to a basic design that won't need breakage to be extended later, limiting the platforms supported at first to ones that are unlikely to need changing, etc). |
|
Don't want to promise anything but it seems that after the backends are now split in seperate crates, the awaited "refactor" is done? |
Waiting for #3833.
This implements Pen/Stylus support inspired by the Web Pointer API.
CursorMovedandMouseInput(now renamed toCursorInput.DeviceEvent::MouseWheel,DeviceEvent::MouseMotion,WindowEvent::CursorMoved,WindowEvent::MouseInput, which I disabled, because users would think all these inputs are mouse. This is also in line with all the other backends, X11 and iOS were the only ones emitting any pen events.Force::Calibrated::altitude_anglewhen a pen is used. I removed this and disabled pen events as well, as this information is now part of theToolState, which can be used withForce::normalized()to get the old behavior back.Open questions:
DeviceEvent::MouseMotiontoDeviceEvent::CursorMotion?Follow-up:
Public API changes
Fixes #99.