Fix #109525: Improved Win32 Repeat Key Filtering #109991

Harley Acheson · 2023-07-12T00:59:59+02:00

Harley Acheson commented

2023-07-12 00:59:59 +02:00

Allows Win32 key repeat filtering to support multiple simultaneously
repeating keys, as can happen with modifiers. Removes
m_keycode_last_repeat_key and instead checks current down status.

We detect repeating keys and attempt to filter out repeating modifier keypresses, like Shift, Alt, Ctrl. But we don't currently filter properly when there are multiple keys repeating at once.

We are currently storing the last key pressed, and then comparing the next key against that to determine if we are repeating. However this doesn't work when we have multiple keys repeating at once, for example when holding down combinations of Shift, Ctrl, and Alt. These repeated keydown events don't come in the expect order and every time a different one comes in we fail to filter it. This results in the majority of events not being filtered when they involve multiple modifiers together.

To fix this the m_keycode_last_repeat_key is removed. Instead key repeat detection just uses the state as it is in the thead's event queue. If the incoming event is key_down we use GetKeyState to see if that key is already known to be down. If yes it is repeating.

Note that an earlier version of the PR included functionality that has been moved out into #110020 to make it easier to track these changes separately.

Allows Win32 key repeat filtering to support multiple simultaneously repeating keys, as can happen with modifiers. Removes m_keycode_last_repeat_key and instead checks current down status. --- We detect repeating keys and attempt to filter out repeating modifier keypresses, like Shift, Alt, Ctrl. But we don't currently filter properly when there are multiple keys repeating at once. We are currently storing the last key pressed, and then comparing the next key against that to determine if we are repeating. However this doesn't work when we have multiple keys repeating at once, for example when holding down combinations of Shift, Ctrl, and Alt. These repeated keydown events don't come in the expect order and every time a different one comes in we fail to filter it. This results in the majority of events not being filtered when they involve multiple modifiers together. To fix this the `m_keycode_last_repeat_key` is **removed**. Instead key repeat detection just uses the state as it is in the thead's event queue. If the incoming event is key_down we use `GetKeyState` to see if that key is already known to be down. If yes it is repeating. Note that an earlier version of the PR included functionality that has been moved out into #110020 to make it easier to track these changes separately.

👍 1 ❤️ 1 🎉 2 🚀 1

Harley Acheson referenced this pull request

2023-07-12 01:01:09 +02:00

Regression: Pressing multiple modifier keys at the same time locks mouse click #109525

Campbell Barton approved these changes 2023-07-12 01:29:27 +02:00

Campbell Barton commented

2023-07-12 01:31:50 +02:00

This looks good, minor requests:

Split the Alt-Tab on Windows-10 (22H2) removal into a separate commit - to make it simpler to bisect if the error ever returns (other applications also run into this from checking on-line, so it's possible the problem persists depending on the version of windows).
Include some details in the commit log (that m_keycode_last_repeat_key is removed & why).
The commit subject mentions refactoring repeat events, but I'm still unclear on what was failing exactly, and what this change fixes - although I can guess it's related to repeat events not working properly.

This looks good, minor requests: - Split the `Alt-Tab on Windows-10 (22H2)` removal into a separate commit - to make it simpler to bisect if the error ever returns (other applications also run into this from checking on-line, so it's possible the problem persists depending on the version of windows). - Include some details in the commit log (that `m_keycode_last_repeat_key` is removed & why). - The commit subject mentions refactoring repeat events, but I'm still unclear on what was failing exactly, and what this change fixes - although I can guess it's related to repeat events not working properly.

Harley Acheson referenced this pull request

2023-07-12 19:12:34 +02:00

Fix #40059: Use Modifier Keys Between Windows on Win32 #110020

Harley Acheson force-pushed WinModifiers from 9ab65e73df to 1759164f28

2023-07-12 20:39:51 +02:00

Compare

Harley Acheson changed title from ~~Fix #109525: Refactor of Win32 Repeat and Multi-Window Modifiers~~ to Fix #109525: Improved Win32 Repeat Key Filtering

2023-07-12 20:40:36 +02:00

Harley Acheson merged commit 65e8b21a95 into main

2023-07-12 20:55:06 +02:00

Harley Acheson deleted branch WinModifiers

2023-07-12 20:55:08 +02:00

Harley Acheson referenced this issue from a commit

2023-07-12 20:55:10 +02:00

Fix #109525: Improved Win32 Repeat Key Filtering

Colin Marmond referenced this issue from a commit

2023-07-12 22:05:32 +02:00

Fix #109525: Improved Win32 Repeat Key Filtering

~~Harley Acheson referenced this pull request 2023-07-13 01:45:03 +02:00~~