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

Mouse device events #192

Closed
wants to merge 4 commits into from
Closed

Conversation

Ralith
Copy link
Contributor

@Ralith Ralith commented May 28, 2017

This makes the API more intuitive for common use-cases and allows us to better propagate platform knowledge of motion semantics.

My original design for DeviceEvent from #164 attempted to be very abstract in the interest of supporting the broadest possible range of input device types, at the cost of being somewhat less obvious. Thanks in large part to discussion with @LPGhatguy, I now believe this was unnecessary. In particular, I believe that the set of input device types supported by major platforms' windowing APIs is very small, and hence merits richer, more semantic event types to improve winit's ease-of-use.

Generic events are retained to ensure we never need to discard data, but I think we should in general try to add handling for every device type we encounter in practice. On the flip side, I think we should be careful to keep the scope of input devices handled as narrow as possible, exposing only events that cannot reasonably be accessed without winit's cooperation.

Do these changes make sense to people?

I also took the opportunity to normalize event naming slightly.

@Ralith
Copy link
Contributor Author

Ralith commented May 28, 2017

Note that I don't go to any special effort in the X11 backend to ensure that the raw input data presented as mouse events are in fact relative pointing device motion, because I haven't been able to identify any cases where any other type of data can possibly arrive on the conventional axes.

@Ralith
Copy link
Contributor Author

Ralith commented Jul 1, 2017

Since Wayland is likely to require winit's involvement to get controller input, part of the motivation of this PR--that the set of device classes whose input must pass through winit is small--no longer seems to hold. There will always be generic events, not just as a fallback but as a routine part of the API. That said, there's still a usability case for special-casing mouse motion.

Ralith added 3 commits July 2, 2017 20:34
This makes the API more intuitive for common use-cases and allows us
to better propagate platform knowledge of motion semantics.
@LPGhatguy
Copy link
Contributor

I think we should rename WindowEvent::MouseMoved to WindowEvent::CursorMoved to reduce confusion between the cursor moving and the mouse reporting that it was moved.

Other than that, LGTM; I'm willing to do the Windows implementation and have it written against an old version of winit right now.

@Ralith Ralith force-pushed the mouse-device-events branch from da49125 to c475fc6 Compare July 3, 2017 03:43
This emphasizes the difference between motion of the host GUI cursor,
as used for clicking on things, and raw mouse(-like) input data, as
used for first-person controls.
@elinorbgr
Copy link
Contributor

What's the state of this, relating to the notion of device events that is currently in winit?

@Xaeroxe
Copy link
Contributor

Xaeroxe commented Nov 6, 2017

@Ralith I really like this PR, so I think I'm going to add support for the missing platforms to this, fix up the OSX build, fix the conflicts then open a PR with these commits. Alternatively I can also open a PR against your fork if you'd prefer to just merge my commits into this PR.

@Ralith
Copy link
Contributor Author

Ralith commented Nov 6, 2017

Feel free to make a new PR if you like; saves on coordination overhead, and I think you have stronger motivation to see changes along these lines merged than I presently do.

@Xaeroxe Xaeroxe mentioned this pull request Nov 6, 2017
@Ralith
Copy link
Contributor Author

Ralith commented Nov 7, 2017

Closing in favor of #344.

@Ralith Ralith closed this Nov 7, 2017
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.

4 participants