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

Consider dropping DeviceEvent in favor of an external library #193

Closed
Ralith opened this issue May 28, 2017 · 15 comments
Closed

Consider dropping DeviceEvent in favor of an external library #193

Ralith opened this issue May 28, 2017 · 15 comments

Comments

@Ralith
Copy link
Contributor

Ralith commented May 28, 2017

In #164, among other changes I introduced DeviceEvent to represent raw input events not associated with any particular window. This allowed applications to access much-needed high resolution unfiltered sensor data e.g. from mice, allowing for interactions such as high-fidelity first-person camera control. However, as the events are not window-related, it is somewhat at odds with the need to limit winit's scope to the task of handling windows.

Further research has since made it apparent to me that raw input can be handled entirely by an external crate without winit's cooperation on Linux and Windows, by creating an independent X or Wayland connection and (so long as winit does not indiscriminately consume events for windows not its own) HWND_MESSAGE window respectively. Such a crate would also be better positioned to handle firmly out-of-scope, but widely required, functionality such as game controller and joystick input.

If/when such a crate is written, winit's scope could therefore be narrowed and more clearly defined by the wholesale removal of non-GUI-related events, replaced with a recommendation to use the external crate. This strikes me as an ideal factoring of the problem.

I'm not sure whether it's appropriate to remove DeviceEvent in the mean time--I don't think it's had time to see much use in the wild yet, so future breakage could be limited by acting quickly, but the information it exposes is important and there is not yet any good alternative crate with which to access it.

@Ralith
Copy link
Contributor Author

Ralith commented May 30, 2017

I have put together a prototype of such a crate supporting raw mouse, keyboard, joystick, and gamepad input on Linux+X11, with allowances for the extension of support to other platforms.

One remaining question is how these libraries might be composed on a single thread in a user-friendly and efficient manner.

@elinorbgr
Copy link
Contributor

Note that on wayland, a client obtaining raw device events is voluntarily forbidden for security reasons (these mechanism are typically used by keyloggers and other similar malwares), and the clients must specifically ask the compositor for such access, in a way linked to its window (for example ask a pointer grab for a specific surface).

For the record, there is a protocol extension (link) in the works for providing access to high-precision relative pointer events, but it cannot be decoupled from surfaces as they define the focus. A client cannot receive input events if it is not focused.

So decoupling the input from the windowing would not work under wayland, unless the user is willing to configure udev to allow the winit apps direct access to the devices.

@Ralith
Copy link
Contributor Author

Ralith commented May 30, 2017

That is frustrating. I don't suppose it's possible to get wayland to give you that high-precision data in association with focus on a surface managed by another client, with that client's cooperation?

@elinorbgr
Copy link
Contributor

That would be up to the wayland server to allow such a thing, but I don't know of any work being done to allow that, as there are mechanisms being developed to provide a client directly with this high-precision input.

A work-around would be to have your lib ask to be given a wayland display as input during initialization in case of a wayland context (with is what mesa does to setup EGL contextes for example). Once you have this display you can basically do whatever you want.

@Ralith
Copy link
Contributor Author

Ralith commented May 30, 2017

Does sharing a wayland display with winit entail sharing the same event queue? If so, how might they avoid inadvertently consuming eachothers' events? My prototype is able to support X11 input well because it is, as far as the X11 server is concerned, a totally independent client.

@elinorbgr
Copy link
Contributor

Nope, wayland includes a mechanism to have more than one event queue, and to precisely decide where each event must be dispatched (a same event can also be sent to more than one event queue if necessary). It also gracefully handles managing the various event queues from different threads.

@Ralith
Copy link
Contributor Author

Ralith commented May 30, 2017

Okay, maybe this is still practicable then, if winit's willing to provide access to the underlying handles. I'd also need to be able to track window creation and destruction so as to maintain the request for data, but presumably wayland has something similar to X11's map/unmap events to support that.

The complexity involved in this case still somewhat weakens the case against having support for raw mouse input (at least) built into winit, although it concerns only one backend, and there's still going to be demand for an external library for non-windowing-system-related input such as game controllers.

@elinorbgr
Copy link
Contributor

Given the content of the protocol extension, tracking window creation/deletion may not be needed. It's just that the events will only be received if the focus is on one of the winit windows, but there is no need to explicitly map the cursor events to any window.

Also, winit does already expose access to the wayland display already (as mesa needs it for example).

@Ralith
Copy link
Contributor Author

Ralith commented May 30, 2017

Oh, I see! That's not so bad after all, then. There's still the question of composition--polling is easy, but winit currently provides no mechanism to wait on events other than its own. Real futures-rs support could be a solution, or users could simply be expected to run winit on a dedicated thread xor rely on polling for non-winit events.

@tomaka
Copy link
Contributor

tomaka commented May 30, 2017

Real futures-rs support could be a solution

I'm opposed to this.

@MaikKlein
Copy link

MaikKlein commented May 30, 2017

Shouldn't winit then drop DeviceEvent and add the specific events to WindowEvent? For example a mouse motion event could be added WindowEvent::MouseMotion(i32, i32).

@LPGhatguy
Copy link
Contributor

I understand the desire to factor out things like gamepad input into a separate library.

I don't like the idea of having to use another crate to get precise mouse deltas. I feel like all of the current WindowEvent and DeviceEvent types should just be rolled into a single Event that exposes as much data as possible. It would have events like CursorMoved and MouseMoved to differentiate between GUI-related and device-related events.

The line between 'device' events and 'window' events is further blurred by devices like keyboards, which can convey input in a number of different ways (keys, characters, composition events) that don't necessarily need to map to the 'GUI' vs 'non-GUI' paradigm.

@Ralith
Copy link
Contributor Author

Ralith commented May 31, 2017

@MaikKlein The proposal was to remove the raw input events entirely, retaining only device add/removal, which cannot be added to WindowEvent because there is no WindowId associated with them.

@LPGhatguy

I don't like the idea of having to use another crate to get precise mouse deltas.

Given the incredibly common need for raw mouse motion and the low implementation cost, I can see the case for retaining support for that specific event.

I feel like all of the current WindowEvent and DeviceEvent types should just be rolled into a single Event that exposes as much data as possible. It would have events like CursorMoved and MouseMoved to differentiate between GUI-related and device-related events.

Whether the event enums should be flattened is out of scope of this discussion.

The line between 'device' events and 'window' events is further blurred by devices like keyboards, which can convey input in a number of different ways (keys, characters, composition events) that don't necessarily need to map to the 'GUI' vs 'non-GUI' paradigm.

The window-related-ness of keyboard input in the current implementation is well defined: keyboard events delivered by the windowing system to a particular window, generally due to being focused by a pointer associated with a particular virtual keyboard device, are window-related. Keyboard events gathered directly from the physical devices irrespective of focus, association with pointers, or any other factors are not.

There's a separate question of whether you process those events in terms of scancodes or semantic operations plus complex text input, but that's more or less orthogonal. Only window-related keyboard events need to be handled by an application attempting to play well with the host system and the user's expectations.

@tomaka
Copy link
Contributor

tomaka commented Jul 1, 2017

Per discussion on IRC, since raw input requires knowledge of the window on at least wayland, then it's not practical to use an external crate and winit should support raw input like a monolithic crate.

@tomaka tomaka closed this as completed Jul 1, 2017
@MaikKlein
Copy link

MaikKlein commented Jul 1, 2017

@tomaka Will the API change for https://docs.rs/winit/0.7.1/winit/enum.Event.html? For example will DeviceEvent move into WindowEvent? Or will it stay the way it currently is?

madsmtm added a commit to madsmtm/winit that referenced this issue Jun 11, 2022
* refactor(windows): `begin_resize_drag` now similar to gtk's (rust-windowing#200)

* refactor(windows): `begin_resize_drag` now similart to gtk's

* fix

* feat(linux): skipping taskbar will now also skip pager (rust-windowing#198)

* refactor(linux): clean dummy device_id (rust-windowing#195)

* refactor(linux): clean dummy device_id

* fmt

* feat(linux): allow resizing undecorated window using touch (rust-windowing#199)

* refactor(windows): only skip taskbar if needed when `set_visible` is called (rust-windowing#196)

* fix: increase borderless resizing inset (rust-windowing#202)

* fix: increase borderless resizing inset

* update some comments

* Replace winapi with windows crate bindings shared with WRY (rust-windowing#206)

* fix(deps): update rust crate libayatana-appindicator to 0.1.6 (rust-windowing#190)

Co-authored-by: Renovate Bot <[email protected]>

* Add Windows crate and webview2-com-sys bindings

* Initial port to webview2-com-sys

* Finish conversion and remove winapi

* Fix renamed lint warning

* Fix all match arms referencing const variables

* Put back the assert instead of expect

* Point to the published version of webview2-com-sys

* Cleanup slightly weird BOOL handling

* Replace mem::zeroed with Default::default

* Add a summary in .changes

* Remove extra projects not in config.json

* Fix clippy warnings

* Update to 32-bit compatible webview2-com-sys

* Better fix for merge conflict

* Fix clippy errors on Windows

* Use path prefix to prevent variable shadowing

* Fix Windows clippy warnings with nightly toolchain

* Fix Linux nightly/stable clippy warnings

* Fix macOS nightly/stable clippy warnings

* Put back public *mut libc::c_void for consistency

* Re-run cargo fmt

* Move call_default_window_proc to util mod

* Remove unnecessary util::to_wstring calls

* Don't repeat LRESULT expression in match arms

* Replace bitwise operations with util functions

* Cleanup more bit mask & shift with util fns

* Prefer from conversions instead of as cast

* Implement get_xbutton_wparam

* Use *mut libc::c_void for return types

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Renovate Bot <[email protected]>

* fix(keyboard): add mapping for space key on Windows (rust-windowing#209)

* fix(keyboard): add mapping for space key on Windows

* change file

* feat: impl Clone for EventLoopWindowTarget (rust-windowing#211)

* chore: add `on_issue_closed.yml` (rust-windowing#214)

* Update tray dependency version (rust-windowing#217)

* Delete on_issue_closed.yml (rust-windowing#221)

* refactor(linux): event loop (rust-windowing#233)

* Use crossbeam::channel

* Fix crossbeam channel import

* Add check on poll event

* Fix deadlock when unregistering shortcut on Linux (rust-windowing#230)

* Add fullscreen monitor selection support on Linux (rust-windowing#235)

* Add fullscreen monitor support on Linux

* Add change file

* Remove todo on videomode

* Fix clippy

* Update to 2021 edition (rust-windowing#236)

* Update to 2021 edition

* Fix clippy

* Add run_return on Linux (rust-windowing#237)

* Add run_return on Linux

* Add main context

* Add run_return trait on Linux (rust-windowing#238)

* Fix: rust-windowing#239 Update webview2-com and windows crates (rust-windowing#240)

* Replace webivew2-com-sys with prebuilt windows

* Use windows utility instead of direct GetLastError

* Bump windows version and add changelog

* Run cargo fmt

* Restore inverted matches macro

* Scope constants in match arms

* Fix inverted null check

* Update src/platform_impl/windows/util.rs

Co-authored-by: Amr Bashir <[email protected]>

* Use env_logger instead of simple_logger (rust-windowing#241)

* Use env_logger instead of simple_logger

* Make clippy happy

* Cherry pick commits to next (rust-windowing#244)

* feat(macos): Add `unhide_application` method, closes rust-windowing#182 (rust-windowing#231)

* feat(macos): Add `unhide_application` method

* Update src/platform/macos.rs

Co-authored-by: Amr Bashir <[email protected]>

* Reanme to `show_application()`

* Remove broken doc link

Co-authored-by: Amr Bashir <[email protected]>

* feat: Allow more strings to parse to keycode (rust-windowing#229)

* feat: support accelerator key strings `,` `-` `.` `Space` `Tab` and `F13`-`F24` (rust-windowing#228)

* feat(macOS): support more accelerator key strings

* Move function keys together

* Add `,` `-` `.` `Space` `F20-F24` for Windows

* Remove support for accelerators not found in `winapi`

* Add `,` `-` `.` `Space` `F13-F24` for Linux

* Update .changes

* Add the rest for Windows

* Add the rest for Linux

* Add the rest on macOS

* Update accelerator-strings.md

* Fix git comments

Co-authored-by: Kasper <[email protected]>
Co-authored-by: Amr Bashir <[email protected]>

* Add redraw events on Linux (rust-windowing#245)

* Add redraw events on Linux

* Update doc of RequestRedraw Event

* Add change file

* Fix missing menu bar on borderless window (rust-windowing#247)

Credit goes to irh's work on winit commit f2de847

* refactor: improve `set_skip_taskbar` impl on Windows (rust-windowing#250)

* fix: emit errors on parsing an invalid accelerator for string, closes rust-windowing#135 (rust-windowing#252)

* chore: update comment

* fix(linux): fix focus events not firing properly (rust-windowing#253)

* fix(linux): fix focus events not firing properly

* add changelog

* chore: update focus events error message

* chore: fmt

* fix: revert windows-rs 0.28 version bump

* fix(linux): fix native menu items (rust-windowing#256)

* chore: remove examples commited by accident

* Update `ReceivedImeText` (rust-windowing#251)

* Allow receiving text without Ime on Windows

* Avoid panic todo

* Receive text without ime on mac

* Fix CursorMoved event on Linux

* Add ReceivedImeText on Linux

This only add Simple IME from GTK for now. We should add the actual IME
from system in the future.

* Fix redraw event that causes inifinite loop (rust-windowing#260)

* Fix redraw event that causes inifinite loop

* Refactor event loop

* Remove unused function

* Add doc comment on linux's run_return

* Ignore doc test on run_return

* Add non blocking iteration on Linux (rust-windowing#261)

* Docs: SystemTrayExtWindows::remove() is gone (rust-windowing#262)

Fix docs following rust-windowing#153

* Fix busy loop on Linux (rust-windowing#265)

* Update windows crate to 0.29.0 (rust-windowing#266)

* Update to windows 0.29.0

* Add change description

* Remove clippy check (rust-windowing#267)

* refactor(windows): align util function with win32 names

* chore: update PR template

* fix(linux): fire resized & moved events on min/maximize, closes rust-windowing#219 (rust-windowing#254)

* feat(linux): implement `raw_window_handle()` (rust-windowing#269)

* chore(deps): update to raw-window-handle 0.4

* add linux raw-window-handle support

* update macos/ios/android

* fix ios

* Fix core-video-sys dependency (rust-windowing#274)

* The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. (rust-windowing#275)

However, this does not work on macOS 10.7 (where AppKit does not link to ColorSync internally). Instead of relying on this, we should just link to ApplicationServices directly.

* Fix some invalid msg_send! usage (rust-windowing#276)

* Revert "Fix some invalid msg_send! usage (rust-windowing#276)" (rust-windowing#277)

This reverts commit a3a2e0cfc49ddfa8cdf65cf9870fb8e3d45b4bc0.

* Revert "The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. (rust-windowing#275)" (rust-windowing#279)

This reverts commit 6f9c468f26ddb60e29be2139397bfaf3b30eab1e.

* The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. (rust-windowing#280)

However, this does not work on macOS 10.7 (where AppKit does not link to ColorSync internally). Instead of relying on this, we should just link to ApplicationServices directly.

Co-authored-by: madsmtm <[email protected]>

* Fix some invalid msg_send! usage (rust-windowing#278)

Co-authored-by: madsmtm <[email protected]>

* Add exit code to ControlFlow::Exit (rust-windowing#281)

* Add exit code to ControlFlow::Exit

* Cargo fmt

* Add change files

Co-authored-by:  multisn8 <[email protected]>

* Add new_any_thread to Unix event loop (rust-windowing#282)

* Update windows crate to 0.30.0 (rust-windowing#283)

* Update windows crate to 0.30.0

* Simplify new-type usage

* Fix boxing in GWL_USERDATA

* Make sure everyone is using Get/SetWindowLongPtrW

* build the system_tray module when "ayatana" feature is enabled (rust-windowing#285)

Without those cfg feature checks, the "ayatana" feature does
actually not enable anything.

* Fix click events missing whe tray has menu (rust-windowing#291)

* Fix click events missing whe tray has menu

* Add change file

* Fix crash when tray has no menu (rust-windowing#294)

* chore: update pull request commit exmple

* fix(windows): send correct position for system tray events, closes rust-windowing#295 (rust-windowing#300)

* fix(windows): revert maximized state handling to winit impl, closes rust-windowing#193 (rust-windowing#299)

* fix(windows): revet maximized state handling to winit impl, closes rust-windowing#193

* add chanefile [skip ci]

* fix: `MenuItem::Quit` on Windows (rust-windowing#303)

* fix: `MenuItem::Close` on Windows

* use `PostQuitMessage` instead

Co-authored-by: amrbashir <[email protected]>

* feat: v1 audit by Radically Open Security (rust-windowing#304)

* Update to gtk 0.15 (rust-windowing#288)

* Update to gtk 0.15

* Fix picky none on set_geometry_hint

* Fix CursorMoved position

Co-authored-by: Amr Bashir <[email protected]>
Co-authored-by: Bill Avery <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Renovate Bot <[email protected]>
Co-authored-by: Lucas Fernandes Nogueira <[email protected]>
Co-authored-by: Kasper <[email protected]>
Co-authored-by: amrbashir <[email protected]>
Co-authored-by: Jay Oster <[email protected]>
Co-authored-by: madsmtm <[email protected]>
Co-authored-by: multisn8 <[email protected]>
Co-authored-by: Aurélien Jacobs <[email protected]>
Co-authored-by: Lucas Fernandes Nogueira <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants