[PATCH] dix: deliver attached slave device's keyboard events to the master's focus

[Peter Hutterer]

On Tue, Jul 16, 2013 at 01:32:19AM -0400, Jasper St. Pierre wrote:
> POn Tue, Jul 16, 2013 at 1:23 AM, Peter Hutterer
> <peter.hutterer at who-t.net>wrote:
> 
> > Tricky one.
> >
> > The protocol spec specifically states:
> > - if the SD is attached to a master keyboard, it sends events to this
> >   keyboard's focus window (if applicable) and/or changes the modifier state
> >   of this keyboard.
> >
> > Except that the actual behavior of the keyboard since 1.7 was that SDs have
> > a separate keyboard focus whether attached or not. That focus by default is
> > PointerRoot, unless set otherwise. So fixing this means we may actually
> > break applications - if there are any that use this, I don't actually know.
> >
> 
> I will say that I was code reviewing an XI2 port of Dolphin. I thought the
> behavior that selecting for events on slave devices worked without keyboard
> focus, but I suppose it makes sense for floating devices.
> 
> The developer of the branch claimed he was one of the few explicitly using
> slave devices and MPX. Whether or not these setups are used in "the real
> world" is questionable, but it's actually a convenient way to capture the
> keyboard in the background that doesn't involve calling XQueryKeymap every
> frame, which is what they do now, so it might be worth it to just keep the
> behavior and document it.
> 
> You can see the respective code for your curiosity as well at:
> 
> https://github.com/Max-E/dolphin-emu/blob/xorg_xinput2/Source/Core/InputCommon/Src/ControllerInterface/Xlib/Xinput2/Xinput2.cpp#L52

I understand, and that was in part why the devices are accessible separatly
- so you can really tell which keyboard has which physical state. problem is
that it requires a bit of work because unless you're handling the focus for
all devices all it takes is one other client to set the focus to a specific
window, or None, etc. and you lose the event stream.

quick search didn't show any focus handling in this file, but I didn't look
at the code in detail.

Cheers,
   Peter

> 
> 
> > I'm actually contemplating changing the spec to reflect the server behavior
> > we've had since 1.7 than fixing this, but here's the patch anyway.
> >
> > To make the behavior consistent we'd need to extend XISetFocus to return
> > BadDevice when trying to set a
> >
> 
> ---
> >  dix/events.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/dix/events.c b/dix/events.c
> > index ed3138d..825cb8b 100644
> > --- a/dix/events.c
> > +++ b/dix/events.c
> > @@ -4039,13 +4039,17 @@ CheckDeviceGrabs(DeviceIntPtr device, DeviceEvent
> > *event, WindowPtr ancestor)
> >  void
> >  DeliverFocusedEvent(DeviceIntPtr keybd, InternalEvent *event, WindowPtr
> > window)
> >  {
> > -    DeviceIntPtr ptr;
> > -    WindowPtr focus = keybd->focus->win;
> > +    DeviceIntPtr ptr, kbd;
> > +    WindowPtr focus;
> >      BOOL sendCore = (IsMaster(keybd) && keybd->coreEvents);
> >      xEvent *core = NULL, *xE = NULL, *xi2 = NULL;
> >      int count, rc;
> >      int deliveries = 0;
> >
> > +    kbd = GetMaster(keybd, KEYBOARD_OR_FLOAT);
> > +    BUG_RETURN(!kbd->focus);
> > +    focus = kbd->focus->win;
> > +
> >      if (focus == FollowKeyboardWin)
> >          focus = inputInfo.keyboard->focus->win;
> >      if (!focus)
> > --
> > 1.8.2.1
> >
> >
> 
> 
> -- 
>   Jasper