[kmscon-devel] About keyboard shortcuts and consumed modifiers

[Ran Benita]

On Mon, Mar 11, 2013 at 12:01:37AM +0100, David Herrmann wrote:
> Hi Ran
> 
> On Sun, Mar 10, 2013 at 11:30 PM, Ran Benita <ran234 at gmail.com> wrote:
> > On Sun, Mar 10, 2013 at 04:27:07PM +0100, David Herrmann wrote:
> >> Hi Ran
> >>
> >> On Sun, Mar 10, 2013 at 11:21 AM, Ran Benita <ran234 at gmail.com> wrote:
> >> > I noticed the last commit added a nice feature for dynamically
> >> > increasing/decreasing the font size with a shortcut. The default is
> >> > <Ctrl><+> and <Ctrl><->.
> >> >
> >> > When I tried it I noticed that you need to use <Ctrl><Shift><=> to get
> >> > the effect of <Ctrl><+>, because on this keymap the <+> is shifted. I
> >> > think though that the expected behavior is that you don't actually need
> >> > to use Shift there.. I wrote some explanation on this a while back, I
> >> > thought it might intrest you:
> >> > http://cgit.freedesktop.org/libxkbcommon/tree/xkbcommon/xkbcommon.h?id=bb620df7aa98c129#n1323
> >>
> >> I am aware of the problem, but didn't have the time to fix it. Thanks
> >> for pointing me to the xkb_*_consumed_*() helpers.
> >
> > Well in a German layout (if you use that) I see that + is not shifted,
> > so you might have missed that.
> 
> Yes, indeed.
> 
> >> So to fix this bug, I basically want to call
> >> xkb_state_mod_mask_remove_consumed() on the keycode+modifiers for each
> >> key-event and pass this as "effective_mods" or something like this
> >> along, right? And I should call this _after_ xkb_state_update_key(),
> >> right?
> >
> > I might have confused two separate issues in my message.
> >
> > First to solve the issue with the + and =, I don't see a better solution
> > than just adding two bindings, one one + and one with =. This boils down
> > to the fact that eventually you are comparing the keysyms. Solving it in
> > another way, like probing the keymap levels etc., would just be more
> > complicated (but I haven't thought of it too much). Of course for
> > letters (e.g. <Ctrl><A> vs. <Ctrl><a>) it's easier, because you just
> > convert to lower or upper case. But with + and = it's completely
> > arbitrary.
> 
> I think I understand that now. So you are basically saying the
> ctrl+'-' shortcut is just _wrong_ on your layout because you want
> ctrl+'='. Even if I fix the modifier-comparison, you still have to
> press "Shift", but that's not what you want, right?

Yes.

> I could fix that by comparing all keysyms of all the levels of the
> single key. However, I think that's overkill and we might run into
> several other problems here. Keymaps can be weird and what happens if
> the user has "+" and "-" on the same key in different levels? That
> just gets nasty.
> We would have to first check the active level, then the other
> levels... Argh, no, I don't want to go that way. If someone has the
> time to investigate into that, I'd appreciate it. But I think that's
> indeed a waste of time.
> 
> I think the fix here is that you simply have to change the shortcut
> via --grab-zoom-out="<ctrl>equal,<ctrl>KP_Equal" right?
> A default <ctrl>minus seems legit to me. I cannot expect to find
> shortcuts that are easy and simple on all layouts. And at least with
> correct modifier-comparison, I still allow users to use these
> shortcuts (with the drawback that they have tohold Shift or whatever).
> 
> Or what do you think is the right fix for this specific problem?
> (apart from the mod-comparison fix)

Yes, figuring out "automatically" probably can be done, but it's what I
wanted to avoid. I guess what I wanted, in essence, is to bind keys to
actions (many possible combinations to one action), rather to bind actions
to keys (one action <-> one combination). This seems a bit more common,
but it works better in config files than on the command line.

> > Second, about the consumed modifiers. I suggest being explicit, i.e. add
> > a 'consumed_mods' field to the event, and do the comparison as described
> > in the mod_index_is_consumed comment. The function to calculate this
> > mask should be similar to shl_xkb_get_mods (or maybe just make this one
> > return the two masks at once). Then you can just use the
> > xkb_state_mod_index_is_consumed() function. The remove_consumed() function
> > wouldn't work with a mask of SHL_*_MASK, because those may be different
> > than the xkbcommon modifier indexes which it expects.
> 
> Maybe I wasn't clear enough, but that's what I meant. Extending
> shl_xkb_get_mods() to return both fields. I was coing for all_mods and
> all_but_consumed_mods. But your way "all_mods and consumed_mods" is
> maybe the better choice.
> Bit-operations are cheap anyway...
> 
> >> I think that's the easiest way. Any comments?
> >
> > Hope I haven't confused you too much :)
> 
> I still wonder how to support all the other modifiers that keymaps may
> advertise. I currently hardcode the standard modifiers, but what if a
> keymap also provides MOD_FOO?
> If the user adds "<mod_foo>Keysym" as shortcut, I currently return an
> error because the modifier is unknown. However, I should instead look
> it up in the layout and use it if available. But how do I communicate
> the xkb_mod_index_t between uterm-input and any input-listener? And
> what happens when the layout is changed during runtime? Hm.. That
> requires some thought...
> Anayway, not directly related to the problem.

Daniel designed the API with the notion that the application uses the
xkb_state directly without a mediator. But I'm assuming that you don't
want such tight coupling, and the 'key event with modifier mask' idiom
is common enough... So what I'd say is just add them as you need them :)
If someone asks for a modifier it's easy to add.

> One more thing. Lets look only at the modifiers. The code is:
> 
> @code
> (state_modifiers & ~consumed_modifiers & significant_modifiers) ==
> shortcut_modifiers
> @endcode
> 
> This still fails with the following example:
>   state_mods = CTRL | SHIFT
>   consumed_mods = SHIFT
>   significant_mods = CTRL | SHIFT
>   shortcut_mods = CTRL | SHIFT
> 
> In this case, the SHIFT is consumed but we also require it as part of
> the shortcut. But the shift is cleared by "~consumed_mods" and the
> comparison will return false.
> 
> So I recommend changing this to:
> 
> @code
> (state_modifiers & ~consumed_modifiers & significant_modifiers) ==
> (shortcut_modifiers & ~consumed_mods)
> @endcode
> 
> This will remove consumed mods also from the shortcut.

Ah, good point. I remember I had considered it, but I can't recall why
I had decided against it! Of course it generally "shouldn't happen"..
Only thing I could find now is here:
https://developer.gnome.org/gdk/stable/gdk-Keyboard-Handling.html#gdk-keymap-translate-keyboard-state
But GTK is a mess here (they keep changing how they handle this stuff
about once a year), And I don't think their reason is relevant for us.
So you should do it as you say, and if I can remember again I'll be
back :)

Ran