Winit: Tracking Issue: Implement `DeviceEvent::ModifiersChanged`

[goddessfreya]

This issue tracks the implementation of DeviceEvent::ModifiersChanged that was discussed in #1124. PRs implementing this change should be made against the master branch.

• [x] Public API changed
• [ ] Implemented on all platforms
  
  
  
  • [x] Windows - #1344
  
  
  • [x] macOS - #1268
  
  
  • [ ] iOS
  
  
  • [ ] WASM
  
  
  • [x] Linux X11 - #1132
  
  
  • [x] Linux Wayland -#1132
  
  
• [ ] Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
• [ ] Updated feature matrix, if new features were added or implemented.

[goddessfreya]

Until it is implemented on all platforms, I've hidden ModifiersChanged. See https://github.com/rust-windowing/winit/pull/1152

[chrisduerr]

ModifiersChanged on macOS has been implemented in #1268.

[murarth]

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]

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

[Osspial]

@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]

@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.

[shika-blyat]

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

[goddessfreya]

@shika-blyat I believe @Osspial was?

[Osspial]

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]

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]

@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.

[shika-blyat]

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

This issue should be closed now no ?

[ryanisaacg]

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