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.

Sign up for GitHub

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

Jump to bottom

Tracking Issue: Implement DeviceEvent::ModifiersChanged #1151

Closed
5 of 10 tasks
goddessfreya opened this issue Sep 8, 2019 · 15 comments
Closed
5 of 10 tasks

Tracking Issue: Implement DeviceEvent::ModifiersChanged #1151

goddessfreya opened this issue Sep 8, 2019 · 15 comments
Labels
D - average Likely as difficult as most tasks here DS - ios DS - macos DS - web DS - windows H - good first issue Ideal for new contributors H - help wanted Someone please save us P - high Vital to have S - api Design and usability S - enhancement Wouldn't this be the coolest? S - meta Project governance

Comments

@goddessfreya
Copy link
Contributor

goddessfreya commented Sep 8, 2019 •
edited
Loading

This issue tracks the implementation of DeviceEvent::ModifiersChanged that was discussed in #1124. PRs implementing this change should be made against the master branch.
@goddessfreya goddessfreya added S - enhancement Wouldn't this be the coolest? H - help wanted Someone please save us DS - macos DS - windows DS - x11 DS - wayland S - api Design and usability DS - ios D - average Likely as difficult as most tasks here P - high Vital to have H - good first issue Ideal for new contributors labels Sep 8, 2019
@goddessfreya
Copy link
Contributor Author

Until it is implemented on all platforms, I've hidden ModifiersChanged. See #1152

@chrisduerr
Copy link
Contributor

ModifiersChanged on macOS has been implemented in #1268.

@murarth
Copy link
Contributor

murarth commented Nov 9, 2019 •
edited
Loading

I should note that PR #1262 (which has not yet been merged) moves the ModifiersChanged event from WindowEvent to DeviceEvent. This better conforms to how this event is generated from X11 and Wayland events. If there's any issue with implementing ModifiersChanged as a DeviceEvent on macOS, please let me know.

@chrisduerr
Copy link
Contributor

@murarth I just used queue_event, which takes an Event. So changing it from WindowEvent to DeviceEvent will not be a problem.

@Osspial
Copy link
Contributor

Osspial commented Dec 2, 2019 •
edited
Loading

@murarth I was looking over the X11 implementation while getting the Windows implementation going, and I noticed that ModifiersChanged is only dispatched in response to keyboard events. Don't we need to check if the modifiers have been changed before dispatching any event, in case the user unfocused the window, pressed or released a modifier, than switched focus back to Winit?

@murarth
Copy link
Contributor

murarth commented Dec 2, 2019

@Osspial I don't believe that's an issue with the current state of the implementation. The X11 code uses XInput device events, which are received regardless of window focus, to track modifier state and issue ModifiersChanged events. Also, as noted above, ModifiersChanged was moved to the DeviceEvent enum, so that the event can be issued to applications globally and regardless of whether a window has focus.

@murarth murarth changed the title Tracking Issue: Implement WindowEvent::ModifiersChanged Tracking Issue: Implement DeviceEvent::ModifiersChanged Dec 5, 2019
@shika-blyat
Copy link

What is the state of this issue for Windows ? Anyone working on it ?

@goddessfreya
Copy link
Contributor Author

@shika-blyat I believe @Osspial was?

@Osspial
Copy link
Contributor

Osspial commented Dec 27, 2019

I was waiting for the redraw-requested-2.0 branch to get merged onto master, since that changed the event loop code in a way that would've made this change difficult to merge. Since that's been merged I'll get the windows implementation up soon.

@Osspial
Copy link
Contributor

Osspial commented Dec 27, 2019

Is there any particular reason ModifiersChanged is defined as a struct variant (ModifiersChanged { modifiers: ModifiersState }) rather than a tuple variant (ModifiersChanged(ModifiersState))? I can't see a reason to force the field to be named, since there isn't any ambiguity to what ModifersState should refer to.

@murarth
Copy link
Contributor

murarth commented Dec 28, 2019

@Osspial No reason. I don't remember why I chose to make it a struct variant. As long as there's only one field, I have no objection to using a tuple variant.

@Osspial Osspial mentioned this issue Dec 28, 2019
Merged
5 tasks
@shika-blyat
Copy link

shika-blyat commented Dec 31, 2019 •
edited
Loading

EDIT:
Nevermind i forgot it needs to be done on multiple platforms

This issue should be closed now no ?

@gin-ahirsch gin-ahirsch mentioned this issue Jan 2, 2020
Closed
@ryanisaacg ryanisaacg mentioned this issue Jan 22, 2020
Merged
4 tasks
@ryanisaacg
Copy link
Contributor

I think I'll wait on the web implementation until #1381 is merged, just for convenience's sake.

@ColonelThirtyTwo
Copy link

@ryanisaacg Do you have an implementation for web? I noticed a note on the ModifiersChanged documentation that it hasn't been implemented yet.

@ryanisaacg
Copy link
Contributor

I don't think I left any WIP branches lying around; I haven't worked on winit for a while.

@niklaskorz niklaskorz mentioned this issue Jan 3, 2022
Closed
@emilk emilk mentioned this issue Jan 4, 2022
Open
@stevenhuyn stevenhuyn mentioned this issue Jun 7, 2022
Merged
kchibisov added a commit that referenced this issue May 28, 2023
@maroider @ArturKovacs
@kchibisov @daxpedda
Overhaul the Keyboard API
a870147 
Overhaul the keyboard API in winit to mimic the W3C specification
to achieve better crossplatform parity. The `KeyboardInput` event
is now uses `KeyEvent` which consists of:

  - `physical_key` - a cross platform way to refer to scancodes;
  - `logical_key`  - keysym value, which shows your key respecting the
                     layout;
  - `text`         - the text produced by this keypress;
  - `location`     - the location of the key on the keyboard;
  - `repeat`       - whether the key was produced by the repeat.

And also a `platform_specific` field which encapsulates extra
information on desktop platforms, like key without modifiers
and text with all modifiers.

The `Modifiers` were also slightly reworked as in, the information
whether the left or right modifier is pressed is now also exposed
on platforms where it could be queried reliably. The support was
also added for the web and orbital platforms finishing the API
change.

This change made the `OptionAsAlt` API on macOS redundant thus it
was removed all together.

Co-Authored-By: Artúr Kovács <kovacs.artur.barnabas@gmail.com>
Co-Authored-By: Kirill Chibisov <contact@kchibisov.com>
Co-Authored-By: daxpedda <daxpedda@gmail.com>
Fixes: #2631.
Fixes: #2055.
Fixes: #2032.
Fixes: #1904.
Fixes: #1810.
Fixes: #1700.
Fixes: #1443.
Fixes: #1343.
Fixes: #1208.
Fixes: #1151.
Fixes: #812.
Fixes: #600.
Fixes: #361.
Fixes: #343.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
D - average Likely as difficult as most tasks here DS - ios DS - macos DS - web DS - windows H - good first issue Ideal for new contributors H - help wanted Someone please save us P - high Vital to have S - api Design and usability S - enhancement Wouldn't this be the coolest? S - meta Project governance
Milestone
No milestone
Development

No branches or pull requests
7 participants
@ColonelThirtyTwo @murarth @ryanisaacg @chrisduerr @Osspial @goddessfreya @shika-blyat