[PATCH 1/4] Port Wayland clients to new xkbcommon API

[Kristian Høgsberg]

On Tue, May 8, 2012 at 3:14 PM, Kristian Høgsberg <hoegsberg at gmail.com> wrote:
> On Tue, May 8, 2012 at 12:26 PM, Daniel Stone <daniel at fooishbar.org> wrote:
>> Hi,
>>
>> On 7 May 2012 21:01, Kristian Høgsberg <hoegsberg at gmail.com> wrote:
>>> On Tue, May 01, 2012 at 08:37:09PM +0100, Daniel Stone wrote:
>>>> +     if (xkb_state_mod_name_is_active(xkb_state, "Mod1",
>>>> +                                      (XKB_STATE_DEPRESSED | XKB_STATE_LATCHED)))
>>>> +             modifiers |= MOD_ALT;
>>>> +
>>>> +     if ((modifiers & MOD_CTRL) && (modifiers & MOD_SHIFT) &&
>>>
>>> I'm still not convinced this API is practical.  Testing a specific
>>> combination of modifiers is such a common operation and it just can't
>>> be this awkward.  The original code tests that shift and control are
>>> pressed, but it really should test modifers == SHIFT | CONTROL, that
>>> is, that those and *only* those modifiers are down.  To do that we
>>> need to get the modifier masks up front (in display_create for
>>> window.c), and then create the modifier mask we want to test against:
>>>
>>> [...]
>>>
>>> or we can define MOD_CTRL in window.h and then translate the
>>> serialized mask to those values:
>>> [...]
>>>
>>> which is what we had before.  Plus a MOD_OTHER if something else we
>>> don't know about is down.  So maybe it's ok if we just wrap it in the
>>> toolkits...  At the end of the day, we'll just hard code shift, ctrl
>>> and alt again, just a layer up in the toolkits.
>>
>> Thanks, this is actually really useful feedback.  I'm going to wedge
>> something like this in:
>>    enum xkb_state_match_type {
>>        XKB_STATE_MATCH_ALL_NON_EXCL, /* all are down */
>>        XKB_STATE_MATCH_ALL_EXCL, /* all are down & nothing else */
>>        XKB_STATE_MATCH_ANY, /* at least one is down */
>>    };
>>    if (xkb_state_mod_indices_are_down(state,
>> XKB_STATE_MATCH_ALL_EXCL, ctrl_index, alt_index, XKB_MOD_INVALID))
>>        fprintf(stderr, "Ctrl + Alt and nothing else are down! Hooray!\n");
>>
>> And the same for mod names rather than indices.  Would that solve your concerns?
>
> I think that I'd be fine with just get_mod_index (from name) and
> serialize_mods.  If you want to keep the mod_name_is_active variants
> and want to add the more complex matching function above, that's fine.
>  I just think xkb_state_serialise_mods() promoted along with mod
> indexes and bit math is much simpler and would like to see it promoted
> to something that not discouraged ("This function should not be used
> in regular clients;").  And spelled with a z, pretty please!

Ok, I pushed an edited version of this patch that works with the
current xkbcommon API.  We just lookup the modifers at xkb init time
and convert the serialized mask to a bitmask of hardcoded modifers.  I
applied the remaining patches in this series too.

Now alt-tabbing away and then back to weston is broken as the active
client never sees the alt-release.  When we alt tab back, we have a
stuck modifier.  So now we need that modifiers event.  Something like
this:

    <event name="modifiers">
      <arg name="serial" type="uint"/>
      <arg name="time" type="uint"/>
      <arg name="pressed" type="uint"/>
      <arg name="latched" type="uint"/>
      <arg name="locked" type="uint"/>
    </event>

sent before keyboard_enter, before pointer_enter, and when modifiers
change we send it to client that only have pointer focus but not
keyboard focus.  Sounds right?

Kristian