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

macOS: Repeated key's ReceivedCharacter cancelled by previous key's keyUp event #1488

Closed
jayache80 opened this issue Mar 1, 2020 · 2 comments
Closed

macOS: Repeated key's ReceivedCharacter cancelled by previous key's keyUp event #1488

jayache80 opened this issue Mar 1, 2020 · 2 comments
Labels
B - bug Dang, that shouldn't have happened C - needs investigation Issue must be confirmed and researched DS - macos

Comments

@jayache80
Copy link
Contributor

jayache80 commented Mar 1, 2020 •
edited
Loading

On macOS only, with some combinations of repeated keys, the action of releasing the first repeated key will cause the remaining repeated key to no longer call ReceivedCharacter.

Steps to reproduce:

  1. Hold down j until it repeats
  2. While still holding down j, hold down k until that repeats. (j's ReceivedCharacters will stop, superseded by k's ReceivedCharacters, as expected.)
  3. Release j while still holding k. This causes k's ReceivedCharacters to stop.

Expected behavior:

  1. Hold down j until it repeats
  2. While still holding down j, hold down k until that repeats. (j's ReceivedCharacters will stop, superseded by k's ReceivedCharacters, as expected.)
  3. Release j while still holding k. k's ReceivedCharacters should continue until k is released.

This causes much frustration to, say, vim and less users with "lazy fingers" while trying to scroll in a terminal emulator utilizing winit.

Note: This happens, strangely, only with some key combinations. For example, trying the above steps with a and s will not reproduce this problem.

This is likely to be the cause of alacritty/alacritty#2788
jayache80 added a commit to jayache80/winit that referenced this issue Mar 1, 2020
@jayache80
macOS: Only check is_repeat to send ReceivedCharacter. Fixes rust-win…
13764bb 
…dowing#1488

Checking state.is_key_down is redundant, since we are handling keyDown.
Moreover, state.is_key_down can be affected by a previous repeated key's
keyUp.
@kchibisov
Copy link
Member

@chrisduerr it'll be nice if you can test it, since IIRC you don't have this issue on your macOS machine, so checking that it doesn't break anything could be a good check.

@jayache80 jayache80 mentioned this issue Mar 1, 2020
Merged
8 tasks
@ryanisaacg ryanisaacg added DS - macos C - needs investigation Issue must be confirmed and researched B - bug Dang, that shouldn't have happened labels Mar 3, 2020
@maroider maroider mentioned this issue Dec 20, 2020
Closed
@jayache80 jayache80 mentioned this issue Jul 9, 2021
Closed
@jayache80
Copy link
Contributor Author

I reopened #1489 to fix this issue. It would've also been fixed by #1449 but that has yet to be ironed out.

jayache80 added a commit to jayache80/winit that referenced this issue Jul 11, 2021
@jayache80
macOS: Remove is_key_down from ViewState
f6ad9d7 
Fixes rust-windowing#1488.

`is_key_down` was only set to true when `insertText` was called.
Therefore `is_key_down` was actually meant to store whether the most
recently pressed key generated an `insertText` event, at which, winit
produces a `ReceivedCharacter`. The issue is that `insertText` is *not*
called for "key repeat", but winit wants to send `ReceivedCharacter`
events for "key repeat" too. To solve this, the `is_key_down` variable
was then checked in the `key_down` function to determine whether it was
valid to produce repeated `ReceivedCharacter` events during "key
repeat". However this check is not needed since `ReceivedCharacter` must
always be called if the key event has a non-empty `characters` property.

Furthermore `is_key_down` didnt actually store whether the previously
pressed character had an `insertText` event, because it was incorrectly
set to false on every "key up". This meant that if two keys were pressed
consecutively and then the first was released, then `is_key_down` was
set to false even if the most recent keypress did actually produce an
`insertText`.
jayache80 added a commit to jayache80/winit that referenced this issue Jul 13, 2021
@jayache80
macOS: Remove is_key_down from ViewState
86cc66f 
Fixes rust-windowing#1488.

`is_key_down` was only set to true when `insertText` was called.
Therefore `is_key_down` was actually meant to store whether the most
recently pressed key generated an `insertText` event, at which, winit
produces a `ReceivedCharacter`. The issue is that `insertText` is *not*
called for "key repeat", but winit wants to send `ReceivedCharacter`
events for "key repeat" too. To solve this, the `is_key_down` variable
was then checked in the `key_down` function to determine whether it was
valid to produce repeated `ReceivedCharacter` events during "key
repeat". However this check is not needed since `ReceivedCharacter` must
always be called if the key event has a non-empty `characters` property.

Furthermore `is_key_down` didn't actually store whether the previously
pressed character had an `insertText` event, because it was incorrectly
set to false on every "key up". This meant that if two keys were pressed
consecutively and then the first was released, then `is_key_down` was
set to false even if the most recent keypress did actually produce an
`insertText`.

Update changelog.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
B - bug Dang, that shouldn't have happened C - needs investigation Issue must be confirmed and researched DS - macos
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ryanisaacg @jayache80 @kchibisov