[PATCH 2/2] XKB: Add debug key actions for grabs & window tree

[Peter Hutterer]

On Tue, Jun 21, 2011 at 12:04:27PM +0100, Daniel Stone wrote:
> Hi,
> 
> On Mon, Jun 20, 2011 at 02:12:56PM +1000, Peter Hutterer wrote:
> > On Fri, Jun 17, 2011 at 05:36:09PM +0100, Daniel Stone wrote:
> > > On Thu, Jun 16, 2011 at 11:01:58AM +1000, Peter Hutterer wrote:
> > > > > +        if (dev->deviceGrab.grab->deviceMask)
> > > > > +            DebugF("      xi1 event mask 0x%lx\n",
> > > > > +                   dev->deviceGrab.grab->deviceMask);
> > > > 
> > > > only for implicit passive grabs, otherwise eventMask is the device's grab
> > > > mask see the massive comment above struct _GrabRec in inputstr.h
> > > 
> > > I see.  I've just changed the conditional to
> > > if (devGrab->implicitGrab && grab->deviceMask) - does that suit?
> > 
> > I think it's better to check the grab type and then print out eventMask for
> > core, and eventMask/deviceMask depending on implicit. Otherwise you'd still
> > print out out "core mask: blah" for XI non-implicit grabs.
> > 
> > if (grab type == core)
> >     print event mask
> > else if (grab type == xi)
> >     print (implicit) ? device mask : event mask
> 
> While looking at this, I found something that confused me further still.
> 
> In ActivatePassiveGrab, we set up the masks thusly:
>     tempGrab.eventMask = deliveryMask;
> 
>     /* get the XI and XI2 device mask */
>     inputMasks = wOtherInputMasks(win);
>     tempGrab.deviceMask = (inputMasks) ? inputMasks->inputEvents[dev->id]: 0;
> 
>     if (inputMasks)
>         memcpy(tempGrab.xi2mask, inputMasks->xi2mask,
>                sizeof(tempGrab.xi2mask));
> 
> So, on an XI1.x passive grab, eventMask will be the proper device mask
> we actually want, plus a core MotionNotifyMask (which seems harmless,
> but weird).  deviceMask will be the union of all XI1.x selections from
> all clients on that particular window, and xi2mask will be the union of
> all XI2.x selections from all clients on that particular window.
> 
> In DeliverGrabbedEvents, we try the following:
>     /* try core event */
>     if (sendCore && grab->grabtype == GRABTYPE_CORE)
>     {
>         rc = EventToCore(event, &core, &count);
>         [...]
>     }
> 
>     if (!deliveries)
>     {
>         rc = EventToXI2(event, &xi2);
>         if (rc == Success)
>         {
>             int evtype = xi2_get_type(xi2);
>             mask = grab->xi2mask[XIAllDevices][evtype/8] |
>                 grab->xi2mask[XIAllMasterDevices][evtype/8] |
>                 grab->xi2mask[thisDev->id][evtype/8];
>             [...]
>         }
>     }
> 
>     if (!deliveries)
>     {
>         rc = EventToXI(event, &xi, &count);
>         if (rc == Success)
>         {
>             /* try XI event */
>             if (grabinfo->fromPassiveGrab  &&
>                     grabinfo->implicitGrab)
>                 mask = grab->deviceMask;
>             else
>                 mask = grab->eventMask;
>             [...]
>         }
>     }
> 
> With the [...] being TryClientEvents and setting deliveries if it was
> successful.  So, if we have an XI1.x grab from one client on a window
> where another client has a XI2 event selection, it looks like we'll
> send an XI2 event to the XI1 client and never send an XI1 event.
> 
> Shouldn't both the XI2 and XI1 branches be testing for grabtype ==
> GRABTYPE_XI or GRABTYPE_XI2, like the core branch?
> 
> If that's true, then I'll push v2 with your suggested if/else change.

yes, you're right, that appears to be the case. should be quite simple to
knock up a test script to check, i think xinput's test-xi2 has basic code
for grabs so you could easily add an XI1 grab.

having said all that, I'm not sure at this point if we still need the
eventMask vs deviceMask. This stuff dates back quite a while IIRC, before
the introduction of GRABTYPE_FOO. I'm unsure at the moment if there can be a
case where we need _both_ eventMask and deviceMask set for correct delivery.

Cheers,
  Peter