Skip to content
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

Richer input events #164

Merged
merged 1 commit into from
May 7, 2017
Merged

Richer input events #164

merged 1 commit into from
May 7, 2017

Conversation

Ralith
Copy link
Contributor

@Ralith Ralith commented Apr 22, 2017

This expands input events to represent sub-pixel mouse positions, devices responsible for generating events, and raw device-oriented events. The X11 back end is refactored to make full use of the new expressiveness. Other backends have had new functionality minimally stubbed out, save for the macos backend which already supports sub-pixel mouse positions.

  • This improves winit's usability for e.g. controlling 3D cameras or first-person characters, where the raw high-resolution sensor data from analog axes is more important than the coordinates of the pixel under the cursor.
  • Increasing the ScanCode size from 8 to 32 bits enables winit to usefully report a wide array of more esoteric inputs such as media keys. The use of XInput2 to handle raw device key events takes full advantage of this on the X11 backend.
  • Library users can now distinguish between inputs on different keyboards and mice.

Future work that should be addressed in a separate PR:

  • Keyboard input via XInput2, Xkb, and libxkbcommon on X11, enabling text input via device events on X11, unusual key (e.g. media key) inputs via window events, more accurate reporting of keyboard device IDs in window events, and accurate reporting of shifted keysyms.
  • Provide access to persistent device IDs (e.g. USB vendor+product+serial) where available so that bindings can be persisted reliably.

Fixes #70 and #133 on linux and provides API groundwork for them to be fixed on other platforms.
Fixes #107, undue CPU use of EventsLoop::run_forever on X11, and misattribution of events to windows on X11 when multiple windows are created.
Provides at least the groundwork for #99 by passing through unhandled analog axis data.
Includes some of the missing fixes from #108

@mitchmindtree
Copy link
Contributor

I like where this is going!

It looks like some of the DeviceEvent and WindowEvent variants might double up? E.g. what is the distinction between WindowEvent::KeyboardInput and DeviceEvent::Key?

Minor nitpick - I think the devid const should be DEVICE_ID (I believe all-caps snake case is standard rust style for consts?)

@mitchmindtree
Copy link
Contributor

Re-reading the description I noticed it mentions that these are events not associated with any window. Does this mean that they are events that occur when no window is focused?

Perhaps the WindowEvent::KeyboardInput fields should be abstracted into a KeyboardInput struct so they can be re-used for the DeviceEvent::Key variant?

@Ralith
Copy link
Contributor Author

Ralith commented Apr 23, 2017

Device events occur regardless of input focus. I've added a note to this effect to the docs. Semantically, they represent a stream of sensor readings from the physical hardware, unfiltered by high-level notions like cursors and input focus.

I've factored out the common elements of KeyboardInput and tweaked the constant use.

@tomaka
Copy link
Contributor

tomaka commented Apr 23, 2017

The API changes look good. As usual I don't know enough about X11 to know whether the implementation changes are good as well, so I'm going to trust you.

I think that on Windows you won't get any DeviceEvent, only WindowEvents. Is this intended by the API, or is it just not implemented?

@Ralith
Copy link
Contributor Author

Ralith commented Apr 23, 2017

I tried to keep the refactored X11 implementation as close as possible to the existing one where it made sense to do so. I think the most significant discretionary change I made was to remove the code to generate artificial analog scroll events from non-emulated scroll-wheel button presses, because in my testing the kernel input system already did this internally, so the code was dead. It'd be nice to have someone else review it just because it's a pretty big set of changes, though.

I did not attempt to implement any of the advanced features supported by the new API on any platform other than X11, partly because I thought it prudent to get the API hashed out first but mostly because I'm not really qualified to do so. That said, discussion with WindowsBunny on IRC (not sure what his github tag is) has made it very clear that Windows does provide very similar data; see the documentation on raw input. My hope is that the new functionality will eventually be filled out by those that are able to do so, as needs arise. In the mean time, the new enums and trivial device IDs should be harmless.

This expands input events to represent sub-pixel mouse positions, devices responsible for generating events, and raw
device-oriented events. The X11 back end is refactored to make full use of the new expressiveness. Other backends have
had new functionality minimally stubbed out, save for the macos backend which already supports sub-pixel mouse
positions.
@Ralith
Copy link
Contributor Author

Ralith commented Apr 23, 2017

I also removed some strange special-cased keysym remapping which I assume was intended to work around the very anemic nature of XKeycodeToKeysym but which was unfortunately not safe. I'm not sure it ever did anything useful in practice (it may have made numlock sort-of work), and the whole area needs to be replaced with the XInput2/Xkb/xkbcommon scheme I've alluded to above anyway.

@quininer
Copy link

quininer commented Apr 23, 2017

I see it removing XIM support, do you have a plan to re-support it?

@Ralith
Copy link
Contributor Author

Ralith commented Apr 23, 2017

XIM support is retained in this PR, and has in fact had a couple bugs fixed. I think we should consider removing it in the long run since I think most people use ibus input methods these days, but I'm not an authority on that matter. If/when I proceed to replace the keyboard handling in this PR with a more modern Xkb-based scheme, XIM will probably have to be removed but XCompose should be supported, which will provide for dead keys at the very least.

@quininer
Copy link

quininer commented Apr 24, 2017

@Ralith

I'm sorry I did not notice src/platform/linux/x11/mod.rs because it was folded.
As far as I am concerned, most people use fcitx. but I guess it also works for fcitx.

This is good news, thanks for your work.

@Ralith
Copy link
Contributor Author

Ralith commented May 7, 2017

It doesn't seem like any reviewers are coming out of the woodwork. What's necessary for a merge?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Duplicated keyboard events with dead keys in X11 Relative mouse motion
4 participants