xkbcommon / libxkbcommon

keymap handling library for toolkits and window systems
https://xkbcommon.org
Other
286 stars 125 forks source link

Add XKB_LED_NAME_COMPOSE and XKB_LED_NAME_KANA #502

Closed C0rn3j closed 2 months ago

C0rn3j commented 2 months ago

The end goal is to be able to press Num Lock, Caps Lock, Scroll Lock, Compose or Kana and have either of the 5 LEDs on my QMK keyboard light up.

I could use a bit of help here.

This is a continuation of https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/990 and is directly related to the current WIP for kwin for adding the same two LED keys.

HID spec defines five LEDs, but only the first 3 are currently supported by XKB and most of the Linux tooling:

image

I am not familiar with XKB, where else do I need to put the necessary definitions?

wismill commented 2 months ago

Thanks for your contribution. LGTM, these names have been stable for 17 years in xkeyboard-config. But since this is an area of XKB in which I have no experience, I am going to put this on hold while I investigate it. Thanks for the links.

wismill commented 2 months ago

@C0rn3j I converted to a draft and removed the “WIP” prefix because it’s clearer in Github’s interface; feel free to use the “ready for review” button when you find it relevant.

wismill commented 2 months ago

I think I got it. For the current use case, the name is a just an indirect way to access the LED index in the keycodes XKB file. The corresponding compat entry is more complex but irrelevant here.

@C0rn3j could you add a changelog entry for the API?

C0rn3j commented 2 months ago

Oh, so I have by chance used the existing correct names? Neat!

I have not tested this (nor libinput) with my kwin PR yet, I need to first prepare my keyboard FW and figure out how to swap out kwin, but I suppose this PR works as is anyway, even if subsequent additions will be needed later.

I have added the changelog fragment.


    indicator 1  = "Caps Lock";
    indicator 2  = "Num Lock";

On a side note, I find it curious that Num and Caps is swapped in the X components, it's equally wrong here and in evdev, so it ends up working. I see you have found my post there while I was writing my reply.

I also wonder what's causing Scroll Lock to break in both X and Wayland, but that's for another issue.

wismill commented 2 months ago

On a side note, I find it curious that Num and Caps is swapped in the X components, it's equally wrong here and in evdev, so it ends up working.

@C0rn3j I am actually wondering the same! Do the compositor or toolkit fixes it?

wismill commented 2 months ago

I think I understand now: LED index in XKB is only an implementation detail at least on Wayland, where the XKB LED state is queried by the LED’s name. Maybe the index is used by old drivers though, as you mentioned in xf86-input-evdev. So I guess we will never invert the indexes of CapsLock and NumLock, because it will require to sync all the drivers relying on the indexes and 1) nobody cares 2) nobody would take the risk for the satisfaction of having the same USB HID order. By the way the XKB stuff predates USB!

whot commented 2 months ago

I think this is correct though as for the XKB state I'm a bit unclear myself.

wismill commented 2 months ago

I investigated a bit some code bases:

So its seems that the correct mapping depends very much on the type of session and the driver used. At this point it looks easier to fix in xkeyboard-config rather than in the drivers.

We should really encourage the use of XKB_LED_* in xkbcommon, in order to query the LED state by its name, not the index defined in the keymap. So this MR is a good step.

Last point: xkeyboard-config should really have some comments about these mappings and names. I am going to send a MR there to inform indicators indexes and names must stay stable.

wismill commented 2 months ago

Added comment about the origin of the names and squashed.

C0rn3j commented 2 months ago

I knew about mutter and possibly wlroots needing similar treatment too, but would not have guessed gtk and xf86-input-libinput, I'll most likely get to them too, since I already started kicking the can.

Thanks a lot for your input and investigation!