Commit dddffd51bee9a4d8a2fbe5246b3422690b84338b

Pierre Le Marre 2025-05-05T13:22:57

state: Fix virtual modifiers with non-real mod mapping Currently there are 2 issues with the handling of virtual modifiers in the keyboard state: 1. We assume that the input modifiers masks encode the indexes of all the modifiers of the keymap, but this is true only for the *real* modifiers (at least in xkbcommon and X11). Indeed, since the virtual modifiers *indexes* are implementation-specific, the input modifier masks merely *encode* the modifiers via their *mapping*. Consider the following keymap: ```c xkb_keymap { xkb_compat { virtual_modifiers M1 = 0x100; }; xkb_types { virtual_modifiers M2 = 0x200; }; }; ``` Now to illustrate, consider the following 2 implementation variants of libxkbcommon (assuming indexes 0-7 are the usual real modifiers): 1. Process `xkb_compat` then `xkb_types`. M1 and M2 have the respective indexes 8 and 9 and map to themselves (with the current assumption about mask denotation). 2. Process `xkb_types` then `xkb_compat`. M1 and M2 have the respective indexes 9 and 8 and map to each other. With the current `xkb_state_update_mask`, implementation 2 will swap M1 and M2 (compared to impl. 1) at each update! Indeed, we can see that `xkb_state_serialize_mods` doesn’t roundtrip via `xkb_state_update_mask`. 2. We assume that modifier masks use only bits denoting modifiers in the keymap, but when parsing the keymap we accept explicit virtual modifiers mapping of arbitrary values. E.g. if `M1` is the only virtual modifier and it is defined by: ```c virtual_modifiers M1 = 0x80000000; // 1 << (32 - 1) ``` then the 32th bit of a modifier mask input does *not* denote the 32th virtual modifier of the keymap, but merely the encoding of the mapping of `M1`. So when calling `xkb_state_update_mask`, we may discard some bits of the modifiers masks and end up with an incorrect state. These 2 issues may break interoperability with other implementations of XKB (e.g. kbvm) and make pure virtual modifiers handling fragile. We introduce the notion of *canonical state modifier mask*: the mask with the smallest population count that denotes all bits used to encode the modifiers in the keyboard state. It is equal to the bitwise OR of real modifiers mask and all the virtual modifiers mappings. This commit fixes the 2 issues by making *weaker* assumptions about the input modifier masks: 1. Modifiers may map to arbitrary values, not only real modifiers. 2. Input modifier masks merely encode modifiers via their *mapping*: - *real* modifiers map to themselves; - *virtual* modifiers map to the bitwise OR of their *explicit* mapping (via `virtual_modifiers`) and their *implicit* mapping (via keys’ real and virtual modmaps). - modifiers indexes are implementation-specific. Since the implementation before this commit also resolved virtual modifiers to their mappings, we continue doing so, but using only the bits that are *not* set in the canonical state modifier mask, so that we enable roundtrip of `xkb_state_serialize_mods` via `xkb_state_update_mask`. 3. Input modifier masks do not denote modifiers indexes (apart from real modifiers), so it is safe to discard only the bits that are not set in the canonical state modifier mask.