On Thu, Aug 06, 2009 at 03:11:16AM +0200, Peter Hutterer wrote:
> On Wed, Aug 05, 2009 at 09:18:55PM +0300, [email protected] wrote:
> > From: Tiago Vignatti <[email protected]>
> >
> > The server was processing ET_RawMotion type when the cursor was wrapping to
> > another screen and getting wrong valuator values. This fix such issue
> > considering only ET_Motion, ET_KeyPress, ET_KeyRelease, ET_ButtonPress and
> > ET_ButtonRelease types when the cursor detects a new screen.
> >
> > Signed-off-by: Tiago Vignatti <[email protected]>
> > ---
> > mi/mieq.c | 24 ++++++++++++++++--------
> > 1 files changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/mi/mieq.c b/mi/mieq.c
> > index 6ec2dba..b52ed84 100644
> > --- a/mi/mieq.c
> > +++ b/mi/mieq.c
> > @@ -367,14 +367,22 @@ mieqProcessDeviceEvent(DeviceIntPtr dev,
> > /* Custom event handler */
> > handler = miEventQueue.handlers[event->any.type];
> >
> > - if (dev && screen && screen != DequeueScreen(dev) && !handler) {
> > - /* Assumption - screen switching can only occur on motion events.
> > */
> > - DequeueScreen(dev) = screen;
> > - x = event->device.root_x;
> > - y = event->device.root_y;
> > - NewCurrentScreen (dev, DequeueScreen(dev), x, y);
> > - }
> > - else {
> > + switch (event->any.type) {
> > + /* Catch events that include valuator information and check if they
> > + * are changing the screen */
> > + case ET_Motion:
> > + case ET_KeyPress:
> > + case ET_KeyRelease:
> > + case ET_ButtonPress:
> > + case ET_ButtonRelease:
> > + if (dev && screen && screen != DequeueScreen(dev) && !handler)
> > {
> > + DequeueScreen(dev) = screen;
> > + x = event->device.root_x;
> > + y = event->device.root_y;
> > + NewCurrentScreen (dev, DequeueScreen(dev), x, y);
> > + break;
> > + }
>
> please always add a comment when an intentional fallthrough is used in a
> switch statement.
I agree with you Peter. The readability of this chunk (and all
mieqProcessInputEvents) is pretty bad :(
> anyway, having a return instead of a break after the dequeue stuff means we
> wouldn't need the large default block.
yeah, return there makes more sense. But I guess you meant we indeed need the
default block, but not for catch that kinds of events (ET_Motion, ET_KeyPress,
etc).
> This whole thing looks odd though. If the screen switch happens on a button
> or key release event, the event isn't processed and we'd end up with a stuck
> button or key.
agreed.
> I think the return is only supposed to happen on a ET_Motion, with the other
> events falling through to the default case.
What about this patched attached? This fix the cursor warping screens and keep
processing those kind of events (it doesn't return on ET_Motion as you stated).
Tiago
>From 537a0a954dea9d2dd8260090822acc1010070326 Mon Sep 17 00:00:00 2001
From: Tiago Vignatti <[email protected]>
Date: Wed, 5 Aug 2009 21:02:29 +0300
Subject: [PATCH] mi: fix cursor warping screens
The server was processing ET_RawMotion type when the cursor was wrapping to
another screen and getting wrong valuator values. This fix such issue
considering only ET_Motion, ET_KeyPress, ET_KeyRelease, ET_ButtonPress and
ET_ButtonRelease types when the cursor detects a new screen, keeping the
"normal" processing of device events.
Signed-off-by: Tiago Vignatti <[email protected]>
---
mi/mieq.c | 24 ++++++++++++++++--------
1 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/mi/mieq.c b/mi/mieq.c
index 6ec2dba..da110c3 100644
--- a/mi/mieq.c
+++ b/mi/mieq.c
@@ -367,14 +367,22 @@ mieqProcessDeviceEvent(DeviceIntPtr dev,
/* Custom event handler */
handler = miEventQueue.handlers[event->any.type];
- if (dev && screen && screen != DequeueScreen(dev) && !handler) {
- /* Assumption - screen switching can only occur on motion events. */
- DequeueScreen(dev) = screen;
- x = event->device.root_x;
- y = event->device.root_y;
- NewCurrentScreen (dev, DequeueScreen(dev), x, y);
- }
- else {
+ switch (event->any.type) {
+ /* Catch events that include valuator information and check if they
+ * are changing the screen */
+ case ET_Motion:
+ case ET_KeyPress:
+ case ET_KeyRelease:
+ case ET_ButtonPress:
+ case ET_ButtonRelease:
+ if (dev && screen && screen != DequeueScreen(dev) && !handler) {
+ DequeueScreen(dev) = screen;
+ x = event->device.root_x;
+ y = event->device.root_y;
+ NewCurrentScreen (dev, DequeueScreen(dev), x, y);
+ }
+ /* No break here. Intentional fallthrough */
+ default:
master = CopyGetMasterEvent(dev, event, &mevent);
if (master)
--
1.5.6.3
_______________________________________________
xorg-devel mailing list
[email protected]
http://lists.x.org/mailman/listinfo/xorg-devel
