Skip to content

Fixed duplicate key events on macOS#764

Closed
nolanderc wants to merge 2 commits into
rust-windowing:winit-legacyfrom
nolanderc:master
Closed

Fixed duplicate key events on macOS#764
nolanderc wants to merge 2 commits into
rust-windowing:winit-legacyfrom
nolanderc:master

Conversation

@nolanderc
Copy link
Copy Markdown

Fixes the issue described in #734 using the suggested solution.

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created an example program if it would help users understand this functionality

@francesca64 francesca64 added the DS - appkit Affects the AppKit/macOS backend label Jan 17, 2019
@francesca64
Copy link
Copy Markdown
Member

Thanks for the PR! For me, this change prevents me from getting Cmd+Period key release events, so this doesn't look like the right solution.

Also, pinging @sodiumjoe since you wrote that code.

The way we currently handle Cmd weirdness isn't too great. I tried to come up with something more idiomatic while working on EventLoop 2.0:

@nolanderc if you try running cargo run --example window on that branch, do things work correctly for you?

@nolanderc
Copy link
Copy Markdown
Author

I can confirm that the changes work for me. This is the output of cargo run --example window:

WindowEvent { window_id: WindowId(Id(140591133379936)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 55, state: Pressed, virtual_keycode: Some(RWin), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: true } } } }
WindowEvent { window_id: WindowId(Id(140591133379936)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 47, state: Pressed, virtual_keycode: Some(Period), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: true } } } }
WindowEvent { window_id: WindowId(Id(140591133379936)), event: ReceivedCharacter('v') }
WindowEvent { window_id: WindowId(Id(140591133379936)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 47, state: Released, virtual_keycode: Some(Period), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: true } } } }
WindowEvent { window_id: WindowId(Id(140591133379936)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 55, state: Released, virtual_keycode: Some(RWin), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } } } }

However, I am using a Dvorak layout (as they were in #734). When I run the same example using Qwerty I get this instead:

WindowEvent { window_id: WindowId(Id(140509843650128)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 55, state: Pressed, virtual_keycode: Some(RWin), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: true } } } }
WindowEvent { window_id: WindowId(Id(140509843650128)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 47, state: Released, virtual_keycode: Some(Period), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: true } } } }
WindowEvent { window_id: WindowId(Id(140509843650128)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 55, state: Released, virtual_keycode: Some(RWin), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } } } }

I can replicate the behaviour of Qwerty using Dvorak by pressing Command + E (Command + Period in Dvorak):

WindowEvent { window_id: WindowId(Id(140586055237328)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 55, state: Pressed, virtual_keycode: Some(RWin), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: true } } } }
WindowEvent { window_id: WindowId(Id(140586055237328)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 14, state: Released, virtual_keycode: Some(E), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: true } } } }
WindowEvent { window_id: WindowId(Id(140586055237328)), event: KeyboardInput { device_id: DeviceId(DeviceId), input: KeyboardInput { scancode: 55, state: Released, virtual_keycode: Some(RWin), modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } } } }

This leads me to think that these issues might be related to #752 and #755. When checking if Command + Period was pressed we are incorrectly assuming a Qwerty layout where . corresponds to the scancode 47. However, in Dvorak . has scancode 14. As far as I can tell, this assumption is also present in EventsLoop 2.0.

Additionally (assuming a Qwerty layout), pressing Command + Period does not send a ReceivedCharacter event, unlike Command + Comma or Command + A.

@nolanderc
Copy link
Copy Markdown
Author

I did some more testing. Querying Cocoa for the actual character being pressed instead of the scancode resolves the issue with both Dvorak and Qwerty.

This works because we are not interested in the physical key being pressed, but the symbolic meaning of Command + Period.

@Osspial Osspial changed the base branch from master to winit-legacy June 13, 2019 05:35
@goddessfreya goddessfreya changed the base branch from winit-legacy to master July 26, 2019 07:26
@goddessfreya goddessfreya changed the base branch from master to winit-legacy July 26, 2019 07:26
@goddessfreya
Copy link
Copy Markdown
Contributor

Hey, @nolanderc, this PR has fallen out of date. Can you rebase this against master and file a new PR? That way we can get some of our MacOS testers testing it. Anyways, I'm closing this PR. Thanks.

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

Labels

DS - appkit Affects the AppKit/macOS backend

Development

Successfully merging this pull request may close these issues.

3 participants