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

[Jasper St. Pierre]

Yeah, there's no focus handling in there, and that's what I meant -- it
selects for events from slave devices on the root window. I was just
surprised that worked.


On Tue, Jul 16, 2013 at 2:03 AM, Peter Hutterer <peter.hutterer at who-t.net>wrote:

> 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
>



-- 
  Jasper
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20130716/7871ddb1/attachment.html>