state: support querying whether virtual modifiers are active

[bluetech]

This is an old patch that I thought would be more visible in a PR. See the commit message for an explanation. It makes it possible for library users to query virtual modifiers directly (e.g. Alt, NumLock) instead of only the 8 core mods.

This is not safe to apply:

1. The major issue is that this will not work for xkbcommon-x11 -- the event that feeds the state to this library, XkbStateNotify, only sends real mods and not the virtual ones. For Wayland and evdev there is no such problem.

   We can work around this by mapping back real mods -> vmods, but this is lossy (either over-matching or under-matching, depending on the strategy) and not very nice.

2. This is backward-compatible, but not forward-compatible, i.e. if a program starts using "Hyper" for instance, it will fail silently (i.e. never active) in older versions. I also changed the definitions of a few mod names in xkbcommon-names.h, which would cause the same problem if compiled on new version but run on old one.

3. If a user does any sort of exclusive matching on the modifiers, this will suddenly fail, since new mods are added. I patched the are_matching functions to avoid it, but I prefer not to, and also most [all?] users use serialize_mods instead of these functions.

4. There are probably other unforeseen consequences...

However, I think if I can solve the first problem this will be a really nice thing to have, as users could finally forget about the obscure ModX mods and how they are mapped and use the nice names (which are also more precise).

[fooishbar]

This was, as you can probably guess from the comments and the code itself, my intention. But I never really managed to resolve a lot of you mentioned. Unfortunately I don't have any bright ideas on the subject either ...

[gatispaeglis]

We can work around this by mapping back real mods -> vmods, but this is lossy

Perhaps it is better not to try to unify API between X11 and Wayland / evdev. X11 clients can do something like this: https://codereview.qt-project.org/#/c/253377/ . It is an experimental work-in-progress patch for some other issue, but the relevant parts for this topic are QXcbKeyboard::modifiers() and modMaskToIndex(uint mask). See also where modMaskToIndex() is called.

For Wayland / evdev use case you could add xkb_state_mod_index_is_active2(), which uses virtual names. Having a separate function would avoid compatibility concerns. I don't know if that would make libxkbcommon internal code too complex.

There are probably other unforeseen consequences...

One thing that I do not understand is why default xmodmap output on Ubuntu has this confusing Mod (with 'us' keyboard layout):

mod1 Alt_L (0x40), Meta_L (0xcd)

The only way how to know if the modifier in the 'state' is Alt or Meta is to look at the pressed keys. But keeping track of the pressed keys does not sound super reliable, probably WM global shortcuts or keyboard grabbing can make the things go out of sync. Without distinguishing the actual keysym, the API would report 2 modifiers as being active, right?

`

[bam80]

We started to hit this in implementation of global shortcuts for KWin compositor.
As a result we have to apply ugly workarounds in our code:
https://lxr.kde.org/source/kde/workspace/kwin/src/xkb.cpp#0493

Any chance this issue could be reinforced?

[orki]

Here's a pure client-side hack that queries the entire keymap to find mapping between real modifiers and virtual modifiers using xkbcommon. This is part of my daily driver on Fedora 34 Plasma Wayland session. Is there a way to make this more efficient?

typedef struct {
    struct xkb_state *state;
    int failed;
    xkb_mod_mask_t used_mods;
    xkb_mod_mask_t shift, control, capsLock, numLock, alt, super, meta, hyper;

    /* Combination modifiers to try */
    int try_shift;
    xkb_keycode_t shift_keycode;
} modifier_mapping_algorithm_t;

/* Algorithm for mapping virtual modifiers to real modifiers:
 *   1. create new state
 *   2. for each key in keymap
 *      a. send key down to state
 *      b. if it affected exactly one bit in modifier map
 *         i) get keysym
 *         ii) if keysym matches one of the known modifiers, save it for that modifier
 *         iii) if modifier is latched, send key up and key down to toggle again
 *      c. send key up to reset the state
 *   3. if shift key found in step 2, run step 2 with all shift+key for each key
 *   4. if shift, control, alt and super are not all found, declare failure
 *   5. if failure, use static mapping from xkbcommon-names.h
 *
 * Step 3 is needed because many popular keymaps map meta to alt+shift.
 *
 * We could do better by constructing a system of linear equations, but it should not be
 * needed in any sane system. We could also use this algorithm with X11, but X11
 * provides XkbVirtualModsToReal which is guaranteed to be accurate, while this
 * algorithm is only a heuristic.
 *
 * We don't touch level3 or level5 modifiers.
 */
static void modifier_mapping_algorithm( struct xkb_keymap *keymap, xkb_keycode_t key, void *data ) {
    ( void )keymap;
    modifier_mapping_algorithm_t *algorithm = ( modifier_mapping_algorithm_t * )data;
    if ( algorithm->failed )
        return;

    if ( algorithm->try_shift ) {
        if ( key == algorithm->shift_keycode ) return;
        xkb_state_update_key( algorithm->state, algorithm->shift_keycode, XKB_KEY_DOWN );
    }

    enum xkb_state_component changed_type = xkb_state_update_key( algorithm->state, key, XKB_KEY_DOWN );
    if ( changed_type & ( XKB_STATE_MODS_DEPRESSED | XKB_STATE_MODS_LATCHED | XKB_STATE_MODS_LOCKED ) ) {
        xkb_mod_mask_t mods = xkb_state_serialize_mods( algorithm->state,
                                                        algorithm->try_shift ? XKB_STATE_MODS_EFFECTIVE : ( XKB_STATE_MODS_DEPRESSED | XKB_STATE_MODS_LATCHED | XKB_STATE_MODS_LOCKED ) );

        const xkb_keysym_t *keysyms;
        int num_keysyms = xkb_state_key_get_syms( algorithm->state, key, &keysyms );
        /* We can handle exactly one keysym with exactly one bit set in the implementation
         * below; with a lot more gymnastics, we could set up an 8x8 linear system and solve
         * for each modifier in case there are some modifiers that are only present in
         * combination with others, but it is not worth the effort. */
        if ( num_keysyms == 1 && mods && ( mods & ( mods-1 ) ) == 0 ) {
#define S2( k, a )                                                      \
            do {                                                        \
                if ( keysyms[0] == XKB_KEY_##k##_L || keysyms[0] == XKB_KEY_##k##_R ) { \
                    if ( !algorithm->a )                                \
                        algorithm->a = mods;                            \
                    else if ( algorithm->a != mods )                    \
                        algorithm->failed = 1;                          \
                }                                                       \
            } while ( 0 )
#define S1( k, a ) if ( ( keysyms[0] == XKB_KEY_##k ) && !algorithm->a ) algorithm->a = mods
            S2( Shift, shift );
            S2( Control, control );
            S1( Caps_Lock, capsLock );
            S1( Shift_Lock, numLock );
            S2( Alt, alt );
            S2( Super, super );
            S2( Meta, meta );
            S2( Hyper, hyper );
#undef S1
#undef S2
        }
        if ( !algorithm->shift_keycode && ( keysyms[0] == XKB_KEY_Shift_L || keysyms[0] == XKB_KEY_Shift_R ) )
            algorithm->shift_keycode = key;

        /* If this is a lock, then up and down to remove lock state*/
        if ( changed_type & XKB_STATE_MODS_LOCKED ) { /* What should we do for LATCHED here? */
            xkb_state_update_key( algorithm->state, key, XKB_KEY_UP );
            xkb_state_update_key( algorithm->state, key, XKB_KEY_DOWN );
        }
    }
    xkb_state_update_key( algorithm->state, key, XKB_KEY_UP );

    if ( algorithm->try_shift ) {
        xkb_state_update_key( algorithm->state, algorithm->shift_keycode, XKB_KEY_UP );
    }
}

static int local_modifier_mapping(struct xkbkeymap *keymap) {
    modifier_mapping_algorithm_t algorithm;

    algorithm.failed = 0;
    algorithm.used_mods = 0;
    algorithm.shift = algorithm.control = algorithm.capsLock = algorithm.numLock = algorithm.alt = algorithm.super = algorithm.meta = algorithm.hyper = 0;
    algorithm.try_shift = 0;
    algorithm.shift_keycode = 0;

    algorithm.state = xkb_state_new( keymap );
    if ( algorithm.state != NULL )
    {
        xkb_keymap_key_for_each( keymap, &modifier_mapping_algorithm, &algorithm );
        if ( !algorithm.shift_keycode )
            algorithm.failed = 1;

        if ( !( algorithm.shift && algorithm.control && algorithm.alt && algorithm.super && algorithm.meta && algorithm.hyper )
             && !algorithm.failed  ) {
            algorithm.try_shift = 1;
            xkb_keymap_key_for_each( keymap, &modifier_mapping_algorithm, &algorithm );
        }
        xkb_state_unref( algorithm.state );
        if ( !algorithm.failed && !( algorithm.shift && algorithm.control && algorithm.alt && algorithm.super ) )
            algorithm.failed = 1;     /* must have found at least those 4 modifiers */
    }

    /* My code to use the found modifier map goes here, but is not shown as it is nor relevant to this bug report. */
    return !algorithm.failed;
}

[wismill]

I have been playing with this and I found 2 interesting cases with SetMods(modifiers=modMapMods).

Using modmap | vmodmap for modMapMods

This is the new implementation in the current PR. By default we have something like:

// layout=us, no option
xkb_compat "default" {
    interpret Alt_L+Any {
        virtualModifier= Alt;
        action = SetMods(modifiers=modMapMods);
    };
    interpret Meta_L+Any {
        virtualModifier= Meta;
        action = SetMods(modifiers=modMapMods);
    };
};
xkb_symbols "default" {
    key <LALT> { [ Alt_L, Meta_L ] };
    modifier_map Mod1 { Alt_L, Alt_R, Meta_L, Meta_R };
};

So for <LALT> we have:

• modmap: Mod1
• vmodmap: Alt, Meta

Pressing <LALT> results in:

• Old implementation: depressed modifiers set to Mod1, because we used modmap.
• New implementation: depressed modifiers set to Mod1+Alt+Meta, because we added the used modmap | vmodmap.

Pressing <LFSH>, <LALT> results in:

• Old implementation: depressed modifiers set to Shift+Mod1.
• New implementation: depressed modifiers set to Shift+Mod1+Alt+Meta.

We would rather want respectively Mod1+Alt (i.e. without Meta) and Shift+Mod1+Meta (i.e. without Alt) in the new implementation.

Using modmap | mask(virtualModifier) for modMapMods

Let’s imagine we use the virtualModifier field of the interpret statement instead of vmodmap, in order to set the modifiers when modifiers=modMapMods. We have an interesting issue in the following case:

// layout=ru, options=grp:alts_toggle
// NOTE: Use ru instead of us layout to avoid our local test/data to force +level3(ralt_switch_for_alts_toggle)
// NOTE: recent version of xkeyboard config changed PC_LALT_LEVEL2 and PC_RALT_LEVEL2
xkb_compat "default" {
    interpret Alt_L+Any {
        virtualModifier= Alt;
        action = SetMods(modifiers=modMapMods);
    };
};
xkb_symbols "default" {
    key <LALT>               {
        type= "PC_RALT_LEVEL2",
        virtualMods= LAlt,
        symbols[Group1]= [ Alt_L, ISO_Prev_Group ]
    };
    key <RALT>               {
        type= "PC_LALT_LEVEL2",
        virtualMods= RAlt,
        symbols[Group1]= [ Alt_R,  ISO_Next_Group ]
    };
    modifier_map Mod1 { Alt_L, Alt_R, Meta_L, Meta_R };
	modifier_map Mod5 { <LVL3>, <MDSW> };
};

So for <LALT> we have:

• modmap: Mod1
• vmodmap: LAlt
  So for <RALT> we have:
• modmap: Mod1
• vmodmap: RAlt

Pressing <LALT> results in:

• Old implementation: depressed modifiers set to Mod1, because we used modmap.
• New implementation: depressed modifiers set to Mod1+Alt, because we added the used modmap | mask(virtualModifier).

Pressing <LALT>, <RALT> results in:

• Old implementation: depressed modifiers set to Mod1, keysym: ISO_Next_Group.
• New implementation: depressed modifiers set to Mod1+Alt, keysym: Alt_R.

We would rather want ISO_Next_Group in the new implementation. The issue is that we need Mod1+LAlt in order to match the proper level, but instead we get Mod1+Alt.

Conclusion

I think modifying the current behaviour is dangerous, because we will have lots of corner cases.
I wrote a new MR #353 to explore another implementation.