Revamp `DeviceEvent::Button` to be consistent with the pointer event overhaul
- [ ] Tested on all platforms changed I have tested the change on Windows, but not on other platforms.
- [x] Added an entry to the
changelogmodule if knowledge of this change could be valuable to users - [x] Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
- [ ] Created or updated an example program if it would help users understand this functionality
I revamped DeviceEvent::Button to make it consistent with the overhauled pointer events, and to make it usable without having to implement platform-specific code to recognize the same buttons.
The API looks like this:
DeviceEvent::PointerButton {
button: DeviceButtonSource,
button_id: ButtonId,
state: ElementState,
}
pub enum DeviceButtonSource {
Mouse(MouseButton),
Touch { finger_id: FingerId, force: Option<Force> },
TabletTool { kind: TabletToolKind, button: TabletToolButton, data: TabletToolData },
Unknown,
}
pub fn DeviceButtonSource::to_mouse_button(&self) -> Option<MouseButton>;
Initially, I was just going to reuse ButtonSource for the for the button field instead of creating a new DeviceButtonSource enum, but I decided to create DeviceButtonSource because the Unknown variant has different semantics for device button inputs compared to window button inputs.
Two considerations:
- On Windows, what should the value be for
button_id? The raw input API sends a bitfield containing mouse button states, and there are specific down/up flags for the 5 supported mouse buttons. This means that there aren't any "raw" button identifiers for mouse buttons on Windows. Current version chooses to send a button "index" as the button id, butDeviceButtonSourcemakes that unnecessary. I've opted to always send0as the button id on Windows, since there are no benefits for any users to use the button index whenDeviceButtonSourceexists, it would only make their code less cross-platform compatible for no reason. - On X11, how should button inputs for scrolling events be handled? I think using
DeviceButtonSource::Unknownhere makes sense, and if an user wants to specifically detect X11 scroll button inputs, they can handle that themselves using the button id.
the Unknown variant has different semantics for device button inputs compared to window button inputs.
Could you expand on this? What are the differing semantics?
We talked about it in our maintainer meeting, I think we'd prefer:
DeviceEvent::PointerButton {
button: ButtonSource,
state: ElementState,
}
This would also sidestep your issue with button_id on Windows IIUC?
The semantic difference is that DeviceButtonSource::Unknown is for platform-specific pointer buttons, whereas ButtonSource::Unknown is used for typical pointer buttons. This means that DeviceButtonSource::Unknown cannot necessarily be converted into a generic MouseButton variant, whereas ButtonSource::Unknown can. On X11 for example there are scroll 'button' inputs, which there aren't any sensible ways to convert into MouseButton.
My motivation for having button_id was to still retain easy access to the specific platform button identifier that Winit received, if the user wishes to pass it forward to other platform API or libraries for whatever reason. I think it's fine to remove button_id though, since the user can just use the button source to find the corresponding platform identifier. It makes for nicer API too, and as you said avoids the problem on Windows with having to either send nothing or a dummy value as the button id.
On X11 for example there are scroll 'button' inputs, which there aren't any sensible ways to convert into
MouseButton.
Maybe these shouldn't emit a DeviceEvent::Button then. Or at least this doesn't seem to make any sense to me.
Maybe. I'm coming more from the perspective of DeviceEvent::Button being "raw", like just passing what is being received thru the platform APIs directly to the user to allow them to handle the inputs themselves.
Maybe. I'm coming more from the perspective of
DeviceEvent::Buttonbeing "raw", like just passing what is being received thru the platform APIs directly to the user to allow them to handle the inputs themselves.
We have touched on this and I think there is some sort of consensus that this is what it should be. Unfortunately that would require a much bigger revamp of DeviceEvent. Until then, we have to work with what we currently have.
I have updated the PR to the suggested API, and DeviceEvent::PointerButton is no longer emitted for X11 scroll button inputs. I also removed the ButtonId type alias, since it is no longer used anywhere.
FYI this will clash with #4324. I suggest waiting until that is merged since it was already approved.
Said PR got merged.
Updated the PR