DeviceIds contain no data on most platforms #338
Comments
|
Having just browsed the Windows source, I can now tell you this isn't as bad as it first appears because the only places this is used is in keyboard and mouse input, and thanks to the distinction between those events it's actually impossible to confuse these. So the DeviceId, while useless, also isn't really needed. |
Xaeroxe
changed the title
|
Just found this is also an issue for OSX. |
Xaeroxe
changed the title
|
After even further scouring this is literally only used for the X11 backend. DeviceId is just a dummy value for every other platform. |
|
So, I'd like to propose that we restructure events. Since 99% of the time DeviceEvents are just raw mouse input we should label them as such, then create a new event for non-mouse devices that will only be emitted on X11 (and maybe other platforms if support for that is ever added) Further, we should remove most of the DeviceIds from the current event structure, as they currently only serve to confuse people and basically just lie about the library's capabilities. |
|
|
|
Device ids can be useful when using multiseat AFAIK. |
|
@torkleyy Sure, but that's only meaningful if the device id actually carries any data. It doesn't 99% of the time, throughout most of winit it's just dead characters. |
|
So what is it that you're suggesting? |
|
Key:
R DeviceEvent::Added R DeviceEvent::Removed R WindowEvent::AxisMotion M WindowEvent Remove all DeviceIds from this enum, as they carry 0 useful data M winit::Touch remove the DeviceId as it's also useless M DeviceEvent::Button and DeviceEvent::Motion Either start emitting these for devices other than the mouse on platforms other than X11, or just remove it entirely. Having this behave as it does today is dangerous as it can lead developers to believe they have support for arbitrary input devices on all platforms when in fact they are far from it. R DeviceEvent::Key - Not needed when the window isn't focused R DeviceEvent::Text - Not needed when the window isn't focused M WindowEvent::MouseMoved rename this to CursorMoved. A WindowEvent::MouseMoved largely replaces today's DeviceEvent::Motion, but includes semantics for this being the mouse. Also includes both x and y, rather than just one axis. |
|
The proposed changes include significant regressions.
The purpose of DeviceId is to distinguish between different keyboards and mice (for example), not to distinguish keyboards from mice.
I'm not sure how you came to this conclusion. They are used on the X11 backend, and are necessary for good hotplugging support.
These are expected to be necessary to access non-mouse/kb devices on Wayland; upstream plans preclude the normal, winit-independent linux input device methods.
Physical mouse (as opposed to cursor) motion is not associated with a window on most platforms. This is why the On the whole, your reasoning here seems to be "Windows support for these recently introduced features is lacking, therefore they should be removed entirely." If Windows support concerns you to that extent, I suggest completing it instead. If you are only concerned about confusion but are not actually interested in the features, documenting their completeness would probably be appropriate. While Windows may account for "96% of gamers," winit is not only a game library, and I believe its users are not nearly so concentrated. |
It fails entirely in this regard as for every platform that isn't X11 it's Eq implementation is literally just
Great! But right now we have so many other places these could be used but they just aren't. Finish the job, don't do it at all, or indicate in documentation somehow this is very incomplete. I'm going to call a straw man fallacy on the windows portion of your comment. This started as a Windows issue but the scope has changed to encompass literally everything that isn't X11. You on the other hand seem entirely concerned with the Linux experience and have paid no heed to the impact on other platforms. |
winit is not a finished library. Refer to #252 for detailed discussion of platform support consistency. Contributions to improve and maintain documentation about this would certainly be valuable. Removing all incompletely supported features would not leave very much. |
My bad. I missed their usage when I did my search.
I understand that. By the time I got done with my list the DeviceEvent enum was so sparse that it made more sense to just drop it and integrate the last remaining event into WindowEvent. If we keep DeviceEvent then it makes sense to keep it there. |
To be clear, I absolutely am concerned for the quality of winit's API across all major platforms. I cannot personally supply implementations for a given feature on every single platform, but I want to be sure we produce a rich and usable API that is useful and realistically implementable in most places. To my knowledge, based on discussions with @retep998, these features make perfect sense on Windows, and do not represent a disproportionate implementation challenge. Some of the proposed changes seem reasonable to me. Wholesale removal of device IDs and events are not one of them, but e.g. the proposed renamings seem like a clarity boon. |
|
A lot of my complaints are resolved by #344 . Before we can close this issue we should
|
|
See also #212 for prior musing on arbitrary input device support. |
Fixes rust-windowing#467 All variants other than Text have been implemented. While Text can be implemented using ToUnicode, that doesn't play nice with dead keys, IME, etc. Most of the mouse DeviceEvents were already implemented, but due to the flags that were used when registering for raw input events, they only worked when the window was in the foreground. This is also a step forward for rust-windowing#338, as DeviceIds are no longer useless on Windows. On DeviceEvents, the DeviceId contains that device's handle. While that handle could ostensibly be used by developers to query device information, my actual reason for choosing it is because it's simply a very easy way to handle this. As a fun bonus, this enabled me to create this method: DevideIdExt::get_persistent_identifier() -> Option<String> Using this gives you a unique identifier for the device that persists across replugs/reboots/etc., so it's ideal for something like device-specific configuration. There's a notable caveat to the new DeviceIds, which is that the value will always be 0 for a WindowEvent. There doesn't seem to be any straightforward way around this limitation. I was concerned that multi-window applications would receive n copies of every DeviceEvent, but Windows only sends them to one window per application. Lastly, there's a chance that these additions will cause antivirus/etc. software to detect winit applications as keyloggers. I don't know how likely that is to actually happen to people, but if it does become an issue, the raw input code is neatly sequestered and would be easy to make optional during compilation.
Fixes rust-windowing#467 All variants other than Text have been implemented. While Text can be implemented using ToUnicode, that doesn't play nice with dead keys, IME, etc. Most of the mouse DeviceEvents were already implemented, but due to the flags that were used when registering for raw input events, they only worked when the window was in the foreground. This is also a step forward for rust-windowing#338, as DeviceIds are no longer useless on Windows. On DeviceEvents, the DeviceId contains that device's handle. While that handle could ostensibly be used by developers to query device information, my actual reason for choosing it is because it's simply a very easy way to handle this. As a fun bonus, this enabled me to create this method: DevideIdExt::get_persistent_identifier() -> Option<String> Using this gives you a unique identifier for the device that persists across replugs/reboots/etc., so it's ideal for something like device-specific configuration. There's a notable caveat to the new DeviceIds, which is that the value will always be 0 for a WindowEvent. There doesn't seem to be any straightforward way around this limitation. I was concerned that multi-window applications would receive n copies of every DeviceEvent, but Windows only sends them to one window per application. Lastly, there's a chance that these additions will cause antivirus/etc. software to detect winit applications as keyloggers. I don't know how likely that is to actually happen to people, but if it does become an issue, the raw input code is neatly sequestered and would be easy to make optional during compilation.
Fixes rust-windowing#467 All variants other than Text have been implemented. While Text can be implemented using ToUnicode, that doesn't play nice with dead keys, IME, etc. Most of the mouse DeviceEvents were already implemented, but due to the flags that were used when registering for raw input events, they only worked when the window was in the foreground. This is also a step forward for rust-windowing#338, as DeviceIds are no longer useless on Windows. On DeviceEvents, the DeviceId contains that device's handle. While that handle could ostensibly be used by developers to query device information, my actual reason for choosing it is because it's simply a very easy way to handle this. As a fun bonus, this enabled me to create this method: DevideIdExt::get_persistent_identifier() -> Option<String> Using this gives you a unique identifier for the device that persists across replugs/reboots/etc., so it's ideal for something like device-specific configuration. There's a notable caveat to the new DeviceIds, which is that the value will always be 0 for a WindowEvent. There doesn't seem to be any straightforward way around this limitation. I was concerned that multi-window applications would receive n copies of every DeviceEvent, but Windows only sends them to one window per application. Lastly, there's a chance that these additions will cause antivirus/etc. software to detect winit applications as keyloggers. I don't know how likely that is to actually happen to people, but if it does become an issue, the raw input code is neatly sequestered and would be easy to make optional during compilation.
Fixes rust-windowing#467 All variants other than Text have been implemented. While Text can be implemented using ToUnicode, that doesn't play nice with dead keys, IME, etc. Most of the mouse DeviceEvents were already implemented, but due to the flags that were used when registering for raw input events, they only worked when the window was in the foreground. This is also a step forward for rust-windowing#338, as DeviceIds are no longer useless on Windows. On DeviceEvents, the DeviceId contains that device's handle. While that handle could ostensibly be used by developers to query device information, my actual reason for choosing it is because it's simply a very easy way to handle this. As a fun bonus, this enabled me to create this method: DevideIdExt::get_persistent_identifier() -> Option<String> Using this gives you a unique identifier for the device that persists across replugs/reboots/etc., so it's ideal for something like device-specific configuration. There's a notable caveat to the new DeviceIds, which is that the value will always be 0 for a WindowEvent. There doesn't seem to be any straightforward way around this limitation. I was concerned that multi-window applications would receive n copies of every DeviceEvent, but Windows only sends them to one window per application. Lastly, there's a chance that these additions will cause antivirus/etc. software to detect winit applications as keyloggers. I don't know how likely that is to actually happen to people, but if it does become an issue, the raw input code is neatly sequestered and would be easy to make optional during compilation.
Fixes #467 All variants other than Text have been implemented. While Text can be implemented using ToUnicode, that doesn't play nice with dead keys, IME, etc. Most of the mouse DeviceEvents were already implemented, but due to the flags that were used when registering for raw input events, they only worked when the window was in the foreground. This is also a step forward for #338, as DeviceIds are no longer useless on Windows. On DeviceEvents, the DeviceId contains that device's handle. While that handle could ostensibly be used by developers to query device information, my actual reason for choosing it is because it's simply a very easy way to handle this. As a fun bonus, this enabled me to create this method: DevideIdExt::get_persistent_identifier() -> Option<String> Using this gives you a unique identifier for the device that persists across replugs/reboots/etc., so it's ideal for something like device-specific configuration. There's a notable caveat to the new DeviceIds, which is that the value will always be 0 for a WindowEvent. There doesn't seem to be any straightforward way around this limitation. I was concerned that multi-window applications would receive n copies of every DeviceEvent, but Windows only sends them to one window per application. Lastly, there's a chance that these additions will cause antivirus/etc. software to detect winit applications as keyloggers. I don't know how likely that is to actually happen to people, but if it does become an issue, the raw input code is neatly sequestered and would be easy to make optional during compilation.
|
|
francesca64
added
S - api
|
Thanks @francesca64 ! |
Xaeroxe
changed the title
|
I should note that there doesn't seem to be any solution to this on macOS, and I'm also not optimistic about a solution for |
|
I'm content closing this then
…On Sat, Jun 2, 2018, 21:03 Francesca Frangipane ***@***.***> wrote:
I should note that there doesn't seem to be any solution to this on macOS,
and I'm also not optimistic about a solution for WindowEvent on Windows.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#338 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AF5Uci_cMkCcqdq1V9-DbWgiUScM8LOcks5t41ISgaJpZM4QQa_Z>
.
|
* fix(Windows): fix a deadlock in `WindowState` * add change file
This needs to be fixed: https://github.com/tomaka/winit/blob/master/src/platform/windows/mod.rs#L22
Since Windows makes up 96% of gamers at the moment this effectively makes the DeviceId useless.EDIT: winit is not exclusively a gaming library.
The text was updated successfully, but these errors were encountered: