qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
18.16k stars 39.11k forks source link

[Bug] Permissive hold doesn't enable combos on another layer before tapping term is met #24250

Open lunyx opened 2 months ago

lunyx commented 2 months ago

Describe the Bug

Let's suppose we have the following two layers:

Layer 1:

Layer 2:

Keyboard Used

zsa Voyager

Link to product page (if applicable)

https://www.zsa.io/voyager

Operating System

Windows

qmk doctor Output

No response

Is AutoHotKey / Karabiner installed

Other keyboard-related software installed

No response

Additional Context

No response

sigprof commented 2 months ago

Since #8591 combos are handled at a very early stage of the key event processing — before the tapping code which handles the LT/MT and other dual role keys. Because of that the combo code may not see the final layer state when some LT keys had just been pressed, but the tap detection did not complete yet. So in your case the combo code sees the layer 1 keycodes for keys 2 and 3 and just passes them through to the tapping code, and then the tapping code queues those events while waiting for the tap/hold decision to complete. After the tap/hold decision is complete (which may happen when the first of keys 2 or 3 is released due to your permissive hold configuration) the tapping code processes the queued events, and at this time the layer state is updated according to the LT hold, but the combo code does not receive those events again, so they are handled as plain key presses on layer 2.

Not sure whether this is really fixable; for your use case you may want some kind of “late combos” which are handled after the tapping code, but there is nothing like that in the core.

anderso commented 2 months ago

A workaround is making the combo produce a custom keycode that is handled in process_record_user, where you then can emit the desired keycode based on the current layer state. This probably has various limitations but will work for the most common use case of tapping a regular key.

I think this is a non-obvious issue that many(?) users hit. It's annoying to design a layout based on layers and combos, and only after implementing it and using it for a while you figure out why you increasingly often mistype certain combos, as your speed increases. Speaking from experience 😄

Maybe this could be documented somewhere?

lunyx commented 2 months ago

So I'm not familiar with the implementation details, but by layer 1 key codes, do you mean the codes for the assigned keys on layer 1, or is there a separate key code for the individual physical keys themselves? I'm guessing that the complexity here it that the COMBO_TERM is processed ahead of time and produces a different key code depending on if it is a combo or not. At that point, you would be losing context of the layer change. I suppose that also means that if you had a combo set up on the same keys on layer 1 that you have on layer 2, that would also result in the layer 1 combo taking effect rather than the individual keys on layer 2 in my example, which would also be undesired behavior, in my opinion.

Maybe what would be needed is to pass down the original key codes in addition to the resolved key codes so that it can be further processed after layer resolution if permissive hold is enabled and the tapping term hasn't been reached.

Also, for some additional information, my actual use case, not the simplified one here, is that my combo is outputting a macro that has <= or >= on it.