On Sun, Dec 21, 2008 at 10:14:53PM +0100, Thomas Jaeger wrote: > This turned out to be a little bit trickier than I initially thought, > since buttonsDown counts the number of physical buttons that are down, > before they are mapped to logical buttons. > > The thing I'm unsure about (as should be evident from the patch), is > where to stick the declaration for the new function AllButtonsAreUp. > > Thanks, > Tom > > Thomas Jaeger wrote: > > I can't see any reason why we would treat buttons > 5 differently. This > > patch simplifies client code by eliminating the need to call XGrabDevice > > after a button has been pressed and prevents race conditions that could > > result from that.
Following up on that: Both the core protocol and the XI protocol spec state that a passive grab is to be released when all buttons are logically up, regardless of modifiers. This is where the magic number 5 comes in. Core allows for 5 buttons, XI allows for more. This puts us in a difficult position. Core requires us to release the grab if, say, only button 6 is down. XI requires us to maintain the grab. I think we need a more complex solution than the one you proposed: test whether the passive grab is a core or an XI grab and release depending on this condition. Something like if ((grab->coreGrab && !button->state) && ... || (!grab->coreGrab && AllButtonsAreUp(b)) Keith, any opinions on this? Cheers, Peter > From 86c2f7e8c37532287772aae1c2554da5fd5eda86 Mon Sep 17 00:00:00 2001 > From: Thomas Jaeger <thjae...@gmail.com> > Date: Sat, 20 Dec 2008 16:17:02 +0100 > Subject: [PATCH] Don't release grabs unless all buttons are up > > Previously, only buttons <= 5 would count here. > --- > Xi/exevents.c | 2 +- > dix/devices.c | 18 ++++++++++++++++++ > dix/events.c | 2 +- > include/inputstr.h | 3 +++ > 4 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/Xi/exevents.c b/Xi/exevents.c > index 083bb2f..07b7341 100644 > --- a/Xi/exevents.c > +++ b/Xi/exevents.c > @@ -1069,7 +1069,7 @@ ProcessOtherEvent(xEventPtr xE, DeviceIntPtr device, > int count) > xE->u.u.detail = key; > return; > } > - if (!b->state && device->deviceGrab.fromPassiveGrab) > + if (device->deviceGrab.fromPassiveGrab && AllButtonsAreUp(b)) > deactivateDeviceGrab = TRUE; > } > > diff --git a/dix/devices.c b/dix/devices.c > index d89f6c3..10dca2e 100644 > --- a/dix/devices.c > +++ b/dix/devices.c > @@ -1996,6 +1996,24 @@ NoteLedState(DeviceIntPtr keybd, int led, Bool on) > ctrl->leds &= ~((Leds)1 << (led - 1)); > } > > +Bool > +AllButtonsAreUp(ButtonClassPtr butc) > +{ > + int key; > + int missingButtons = butc->buttonsDown; > + > + for (key = 0; missingButtons && key < butc->numButtons; key++) > + { > + int bit = 1 << (key & 7); > + if (butc->down[key >> 3] & bit) { > + if (butc->map[key]) > + return False; > + missingButtons--; > + } > + } > + return True; > +} > + > _X_EXPORT int > Ones(unsigned long mask) /* HACKMEM 169 */ > { > diff --git a/dix/events.c b/dix/events.c > index d7618c2..0105e82 100644 > --- a/dix/events.c > +++ b/dix/events.c > @@ -3846,7 +3846,7 @@ ProcessPointerEvent (xEvent *xE, DeviceIntPtr mouse, > int count) > if (xE->u.u.detail == 0) > return; > filters[mouse->id][Motion_Filter(butc)] = MotionNotify; > - if (!butc->state && mouse->deviceGrab.fromPassiveGrab) > + if (mouse->deviceGrab.fromPassiveGrab && AllButtonsAreUp(butc)) > deactivateGrab = TRUE; > break; > default: > diff --git a/include/inputstr.h b/include/inputstr.h > index 9591d2f..a94ea01 100644 > --- a/include/inputstr.h > +++ b/include/inputstr.h > @@ -200,6 +200,9 @@ typedef struct _ButtonClassRec { > #endif > } ButtonClassRec, *ButtonClassPtr; > > +extern Bool AllButtonsAreUp( > + ButtonClassPtr /*butc*/); > + > typedef struct _FocusClassRec { > WindowPtr win; /* May be set to a int constant (e.g. > PointerRootWin)! */ > int revert; > -- > 1.5.6.3 _______________________________________________ xorg mailing list xorg@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/xorg