On Fri, Oct 26, 2012 at 04:04:21PM -0700, Keith Packard wrote: > Peter Hutterer <[email protected]> writes: > > > All these need review lovin'. > > > > Pull request is on top of my for-keith branch > > from last Friday. > > http://lists.freedesktop.org/archives/xorg-devel/2012-October/034064.html > > > > The following changes since commit c5396ec05a5c6cab6608ba677f703c5227b1de13: > > > > xf86: Fix build against recent Linux kernel (2012-10-19 13:12:33 +1000) > > > > are available in the git repository at: > > > > git://people.freedesktop.org/~whot/xserver unreviewed > > > > for you to fetch changes up to c97467d449c30da7529bdcc68c3ed344828a6baa: > > > > dix: don't filter RawEvents if the grab window is not the root window > > (#53897) (2012-10-25 15:54:02 +1000) > > > > ---------------------------------------------------------------- > > Peter Hutterer (7): > > dix: don't allow disabling XTest devices > > I'd say the error values for the two code paths should be the same. I'd > suggest that BadMatch might be a slightly better value, but I'm easy if > you prefer BadAccess.
We've had BadAccess for VCP/VCK for a while now, so I'm hesitant to change it for little benefit. Plus, the error codes for properties are a bit of a desaster, I should've added specific error codes for the various errors that can actually happen. For XChangeDeviceControl, we actually have it documented that BadMatch is a valid error code (and from my reading it appears to be the best choice here). > Also, the comment in DeviceSetProperty should be updated to reflect that > the xtest devices are also not subject to disabling. done, thanks. > I did verify that these are the only relevant code paths. > > Mostly-happy-with: Keith Packard <[email protected]> > > > xkb: ProcesssPointerEvent must work on the VCP if it gets the VCP > > Yeah, this looked right when I looked at it a while ago. > > Reviewed-by: Keith Packard <[email protected]> > > > Xi: set xChangeDeviceControlReply.status to Success by default > > Reviewed-by: Keith Packard <[email protected]> > > > Xi: don't deliver TouchEnd to a client waiting for TouchBegin (#55738) > > Following the state changes in exevents.c was entertaining, but there > doesn't seem to be any way to get past LISTENER_AWAITING_BEGIN without > delivering a TouchBegin event. Therefore, this simple patch seems > correct to me. > > Reviewed-by: Keith Packard <[email protected]> > > > dix: fix zaphod screen scrossing (#54654) > > Reviewed-by: Keith Packard <[email protected]> > > > xfree86: remove unused variable sigstate > > Reviewed-by: Keith Packard <[email protected]> > > > dix: don't filter RawEvents if the grab window is not the root window > > (#53897) > > I'd have to say that 'FilterRawEvents' is a pretty miserable function; > the semantics of its return value are muddled due to how it gets > invoked, in particular, it "knows" that an event may have been delivered > through the grab and that it must, therefore, suppress an extra event in > that case. Ick. > > I have the feeling that it should be using the return value from > DeliverGrabbedEvent to help decide whether to deliver another event, > something like: > > if (deliveries && SameClient(grab, client)) > don't deliver another event > > Then, the question is 2.0 vs 2.1; for the 2.0 case a grab always > prevents delivery > > else if (2.0 && grab) > don't deliver an event > > else > deliver an event > > Just sticking these tests right inside DeliverRawEvent would keep all of > the conditions together, instead of having some split out to > FilterRawEvent. Keeping the Xi version check in a separate function > makes good sense though; that's really the only complexity in > FilterRawEvents at present. I'll have a look again and see if I can clean this up. One day. > (I'm hoping I got the semantics of raw events right; the protocol spec > seems a lot less ambiguous than the code does...) if there's any ambiguity in the spec let me know and I can amend them. I know what was _intended_, so we can fix the spec to be understood by people now directly involved in the development. A luxury that we don't have on all extensions... fwiw, the semantics are fairly simple: XI 2.0: raw events except if some other client has a grab XI 2.1: raw events at all times The code is more complicated largely to avoid duplicate raw events. Cheers, Peter _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
