Attachment 86455 Details for Bug 67563 – evdev: Only track one pending event

[patch] evdev: Only track one pending event

0001-evdev-Only-track-one-pending-event.patch (text/plain), 15.56 KB, created by Neil Roberts on 2013-09-24 15:00:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Neil Roberts

Created: 2013-09-24 15:00:01 UTC

Size: 15.56 KB

patch

obsolete

>From 0572b3adc670065b7529db7f8acaa19f8974c258 Mon Sep 17 00:00:00 2001
>From: Neil Roberts <neil@linux.intel.com>
>Date: Tue, 24 Sep 2013 12:09:03 +0100
>Subject: [PATCH] evdev: Only track one pending event
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>Instead of having a mask of pending events there is now an enum with a
>single value to represent the one pending event. The event gets
>flushed explicitly as part of the handling code for each event type
>rather than in the outer event reading loop. The pending event is used
>so that we can combine multiple motion events into one and to make
>sure that we have recieved the latest position before sending a touch
>up or down event. This should fix the following problems with the old
>approach:
>
>â¢ If you release a finger and press it down again quickly you could
>  get the up and down events in the same batch. However the pending
>  events were always processed in the order down then up so it would
>  end up notifying two down events and then an up. The pending event
>  is now always flushed when there is a new up or down event so they
>  will always be in the right order.
>
>â¢ When it got a slot event it would immediately change the slot number
>  and then set the pending event. Then when it flushed the events it
>  would use the new slot number to flush the old pending event so the
>  events could have the wrong finger. The pending event is now
>  immediately flushed when a slot event is received so it will have
>  the right finger.
>
>â¢ If you get more than 32 events in one read then it was resetting the
>  pending events before processing the next batch in
>  evdev_process_events. If four fingers were pressed down at once then
>  it ended up with more than 32 events and the sync message would be
>  in the second batch. The pending flag for the last finger was
>  getting cleared so it never got emitted. In this patch the pending
>  event is no longer reset after reading nor is it explicitly flushed.
>  Instead it is flushed when we receive a EV_SYN event or a different
>  pending event needs to replace it.
>
>The touchpad handling code was trying to use the pending event
>mechanism to notify the relative motion events. I'm not sure why it
>was doing this because it looks the event would effectively get
>emitted as soon as the touchpad_process function is finished anyway
>and it wasn't accumulating the values. Instead I've just changed it to
>emit the event directly.
>
>https://bugs.freedesktop.org/show_bug.cgi?id=67563
>---
> src/evdev-touchpad.c |   7 +-
> src/evdev.c          | 246 ++++++++++++++++++++++++---------------------------
> src/evdev.h          |  19 ++--
> 3 files changed, 129 insertions(+), 143 deletions(-)
>
>diff --git a/src/evdev-touchpad.c b/src/evdev-touchpad.c
>index aa9df62..69f913a 100644
>--- a/src/evdev-touchpad.c
>+++ b/src/evdev-touchpad.c
>@@ -490,10 +490,9 @@ touchpad_update_state(struct touchpad_dispatch *touchpad, uint32_t time)
> 		filter_motion(touchpad, &dx, &dy, time);
> 
> 		if (touchpad->finger_state == TOUCHPAD_FINGERS_ONE) {
>-			touchpad->device->rel.dx = wl_fixed_from_double(dx);
>-			touchpad->device->rel.dy = wl_fixed_from_double(dy);
>-			touchpad->device->pending_events |=
>-				EVDEV_RELATIVE_MOTION | EVDEV_SYN_OR_SLOT;
>+			notify_motion(touchpad->device->seat, time,
>+				      wl_fixed_from_double(dx),
>+				      wl_fixed_from_double(dy));
> 		} else if (touchpad->finger_state == TOUCHPAD_FINGERS_TWO) {
> 			if (dx != 0.0)
> 				notify_axis(touchpad->device->seat,
>diff --git a/src/evdev.c b/src/evdev.c
>index 3ccdd3c..c1e11d3 100644
>--- a/src/evdev.c
>+++ b/src/evdev.c
>@@ -28,6 +28,7 @@
> #include <unistd.h>
> #include <fcntl.h>
> #include <mtdev.h>
>+#include <assert.h>
> 
> #include "compositor.h"
> #include "evdev.h"
>@@ -64,6 +65,85 @@ evdev_led_update(struct evdev_device *device, enum weston_led leds)
> 	(void)i; /* no, we really don't care about the return value */
> }
> 
>+static void
>+transform_absolute(struct evdev_device *device, int32_t *x, int32_t *y)
>+{
>+       if (!device->abs.apply_calibration) {
>+               *x = device->abs.x;
>+               *y = device->abs.y;
>+               return;
>+       } else {
>+               *x = device->abs.x * device->abs.calibration[0] +
>+                       device->abs.y * device->abs.calibration[1] +
>+                       device->abs.calibration[2];
>+
>+               *y = device->abs.x * device->abs.calibration[3] +
>+                       device->abs.y * device->abs.calibration[4] +
>+                       device->abs.calibration[5];
>+       }
>+}
>+
>+static void
>+evdev_flush_pending_event(struct evdev_device *device, uint32_t time)
>+{
>+	struct weston_seat *master = device->seat;
>+	wl_fixed_t x, y;
>+	int32_t cx, cy;
>+	int slot;
>+
>+	slot = device->mt.slot;
>+
>+	switch (device->pending_event) {
>+	case EVDEV_NONE:
>+		return;
>+	case EVDEV_RELATIVE_MOTION:
>+		notify_motion(master, time, device->rel.dx, device->rel.dy);
>+		device->rel.dx = 0;
>+		device->rel.dy = 0;
>+		goto handled;
>+	case EVDEV_ABSOLUTE_MT_DOWN:
>+		weston_output_transform_coordinate(device->output,
>+						   device->mt.slots[slot].x,
>+						   device->mt.slots[slot].y,
>+						   &x, &y);
>+		notify_touch(master, time,
>+			     slot, x, y, WL_TOUCH_DOWN);
>+		goto handled;
>+	case EVDEV_ABSOLUTE_MT_MOTION:
>+		weston_output_transform_coordinate(device->output,
>+						   device->mt.slots[slot].x,
>+						   device->mt.slots[slot].y,
>+						   &x, &y);
>+		notify_touch(master, time,
>+			     slot, x, y, WL_TOUCH_MOTION);
>+		goto handled;
>+	case EVDEV_ABSOLUTE_MT_UP:
>+		notify_touch(master, time, slot, 0, 0,
>+			     WL_TOUCH_UP);
>+		goto handled;
>+	case EVDEV_ABSOLUTE_MOTION:
>+		transform_absolute(device, &cx, &cy);
>+		weston_output_transform_coordinate(device->output,
>+						   cx, cy, &x, &y);
>+
>+		if (device->caps & EVDEV_TOUCH) {
>+			if (master->num_tp == 0)
>+				notify_touch(master, time, 0,
>+					     x, y, WL_TOUCH_DOWN);
>+			else
>+				notify_touch(master, time, 0,
>+					     x, y, WL_TOUCH_MOTION);
>+		} else
>+			notify_motion_absolute(master, time, x, y);
>+		goto handled;
>+	}
>+
>+	assert(0 && "Unknown pending event type");
>+
>+handled:
>+	device->pending_event = EVDEV_NONE;
>+}
>+
> static inline void
> evdev_process_key(struct evdev_device *device, struct input_event *e, int time)
> {
>@@ -71,6 +151,8 @@ evdev_process_key(struct evdev_device *device, struct input_event *e, int time)
> 	if (e->value == 2)
> 		return;
> 
>+	evdev_flush_pending_event(device, time);
>+
> 	switch (e->code) {
> 	case BTN_LEFT:
> 	case BTN_RIGHT:
>@@ -102,33 +184,40 @@ evdev_process_key(struct evdev_device *device, struct input_event *e, int time)
> }
> 
> static void
>-evdev_process_touch(struct evdev_device *device, struct input_event *e)
>+evdev_process_touch(struct evdev_device *device,
>+		    struct input_event *e,
>+		    uint32_t time)
> {
> 	const int screen_width = device->output->current_mode->width;
> 	const int screen_height = device->output->current_mode->height;
> 
> 	switch (e->code) {
> 	case ABS_MT_SLOT:
>+		evdev_flush_pending_event(device, time);
> 		device->mt.slot = e->value;
>-		device->pending_events |= EVDEV_SYN_OR_SLOT;
> 		break;
> 	case ABS_MT_TRACKING_ID:
>+		if (device->pending_event != EVDEV_NONE &&
>+		    device->pending_event != EVDEV_ABSOLUTE_MT_MOTION)
>+			evdev_flush_pending_event(device, time);
> 		if (e->value >= 0)
>-			device->pending_events |= EVDEV_ABSOLUTE_MT_DOWN;
>+			device->pending_event = EVDEV_ABSOLUTE_MT_DOWN;
> 		else
>-			device->pending_events |= EVDEV_ABSOLUTE_MT_UP;
>+			device->pending_event = EVDEV_ABSOLUTE_MT_UP;
> 		break;
> 	case ABS_MT_POSITION_X:
>-		device->mt.x[device->mt.slot] =
>+		device->mt.slots[device->mt.slot].x =
> 			(e->value - device->abs.min_x) * screen_width /
> 			(device->abs.max_x - device->abs.min_x);
>-		device->pending_events |= EVDEV_ABSOLUTE_MT_MOTION;
>+		if (device->pending_event == EVDEV_NONE)
>+			device->pending_event = EVDEV_ABSOLUTE_MT_MOTION;
> 		break;
> 	case ABS_MT_POSITION_Y:
>-		device->mt.y[device->mt.slot] =
>+		device->mt.slots[device->mt.slot].y =
> 			(e->value - device->abs.min_y) * screen_height /
> 			(device->abs.max_y - device->abs.min_y);
>-		device->pending_events |= EVDEV_ABSOLUTE_MT_MOTION;
>+		if (device->pending_event == EVDEV_NONE)
>+			device->pending_event = EVDEV_ABSOLUTE_MT_MOTION;
> 		break;
> 	}
> }
>@@ -145,13 +234,15 @@ evdev_process_absolute_motion(struct evdev_device *device,
> 		device->abs.x =
> 			(e->value - device->abs.min_x) * screen_width /
> 			(device->abs.max_x - device->abs.min_x);
>-		device->pending_events |= EVDEV_ABSOLUTE_MOTION;
>+		if (device->pending_event == EVDEV_NONE)
>+			device->pending_event = EVDEV_ABSOLUTE_MOTION;
> 		break;
> 	case ABS_Y:
> 		device->abs.y =
> 			(e->value - device->abs.min_y) * screen_height /
> 			(device->abs.max_y - device->abs.min_y);
>-		device->pending_events |= EVDEV_ABSOLUTE_MOTION;
>+		if (device->pending_event == EVDEV_NONE)
>+			device->pending_event = EVDEV_ABSOLUTE_MOTION;
> 		break;
> 	}
> }
>@@ -162,14 +253,19 @@ evdev_process_relative(struct evdev_device *device,
> {
> 	switch (e->code) {
> 	case REL_X:
>+		if (device->pending_event != EVDEV_RELATIVE_MOTION)
>+			evdev_flush_pending_event(device, time);
> 		device->rel.dx += wl_fixed_from_int(e->value);
>-		device->pending_events |= EVDEV_RELATIVE_MOTION;
>+		device->pending_event = EVDEV_RELATIVE_MOTION;
> 		break;
> 	case REL_Y:
>+		if (device->pending_event != EVDEV_RELATIVE_MOTION)
>+			evdev_flush_pending_event(device, time);
> 		device->rel.dy += wl_fixed_from_int(e->value);
>-		device->pending_events |= EVDEV_RELATIVE_MOTION;
>+		device->pending_event = EVDEV_RELATIVE_MOTION;
> 		break;
> 	case REL_WHEEL:
>+		evdev_flush_pending_event(device, time);
> 		switch (e->value) {
> 		case -1:
> 			/* Scroll down */
>@@ -185,6 +281,7 @@ evdev_process_relative(struct evdev_device *device,
> 		}
> 		break;
> 	case REL_HWHEEL:
>+		evdev_flush_pending_event(device, time);
> 		switch (e->value) {
> 		case -1:
> 			/* Scroll left */
>@@ -203,119 +300,17 @@ evdev_process_relative(struct evdev_device *device,
> }
> 
> static inline void
>-evdev_process_absolute(struct evdev_device *device, struct input_event *e)
>+evdev_process_absolute(struct evdev_device *device,
>+		       struct input_event *e,
>+		       uint32_t time)
> {
> 	if (device->is_mt) {
>-		evdev_process_touch(device, e);
>+		evdev_process_touch(device, e, time);
> 	} else {
> 		evdev_process_absolute_motion(device, e);
> 	}
> }
> 
>-static int
>-is_motion_event(struct input_event *e)
>-{
>-	switch (e->type) {
>-	case EV_REL:
>-		switch (e->code) {
>-		case REL_X:
>-		case REL_Y:
>-			return 1;
>-		}
>-		break;
>-	case EV_ABS:
>-		switch (e->code) {
>-		case ABS_X:
>-		case ABS_Y:
>-		case ABS_MT_POSITION_X:
>-		case ABS_MT_POSITION_Y:
>-			return 1;
>-		}
>-	}
>-
>-	return 0;
>-}
>-
>-static void
>-transform_absolute(struct evdev_device *device, int32_t *x, int32_t *y)
>-{
>-       if (!device->abs.apply_calibration) {
>-               *x = device->abs.x;
>-               *y = device->abs.y;
>-               return;
>-       } else {
>-               *x = device->abs.x * device->abs.calibration[0] +
>-                       device->abs.y * device->abs.calibration[1] +
>-                       device->abs.calibration[2];
>-
>-               *y = device->abs.x * device->abs.calibration[3] +
>-                       device->abs.y * device->abs.calibration[4] +
>-                       device->abs.calibration[5];
>-       }
>-}
>-
>-static void
>-evdev_flush_motion(struct evdev_device *device, uint32_t time)
>-{
>-       struct weston_seat *master = device->seat;
>-       wl_fixed_t x, y;
>-       int32_t cx, cy;
>-       int slot;
>-
>-       if (!(device->pending_events & EVDEV_SYN_OR_SLOT))
>-		return;
>-
>-	slot = device->mt.slot;
>-	device->pending_events &= ~EVDEV_SYN_OR_SLOT;
>-	if (device->pending_events & EVDEV_RELATIVE_MOTION) {
>-		notify_motion(master, time, device->rel.dx, device->rel.dy);
>-		device->pending_events &= ~EVDEV_RELATIVE_MOTION;
>-		device->rel.dx = 0;
>-		device->rel.dy = 0;
>-	}
>-	if (device->pending_events & EVDEV_ABSOLUTE_MT_DOWN) {
>-		weston_output_transform_coordinate(device->output,
>-						   device->mt.x[slot],
>-						   device->mt.y[slot],
>-						   &x, &y);
>-		notify_touch(master, time,
>-			     device->mt.slot, x, y, WL_TOUCH_DOWN);
>-		device->pending_events &= ~EVDEV_ABSOLUTE_MT_DOWN;
>-		device->pending_events &= ~EVDEV_ABSOLUTE_MT_MOTION;
>-	}
>-	if (device->pending_events & EVDEV_ABSOLUTE_MT_MOTION) {
>-		weston_output_transform_coordinate(device->output,
>-						   device->mt.x[slot],
>-						   device->mt.y[slot],
>-						   &x, &y);
>-		notify_touch(master, time,
>-			     device->mt.slot, x, y, WL_TOUCH_MOTION);
>-		device->pending_events &= ~EVDEV_ABSOLUTE_MT_DOWN;
>-		device->pending_events &= ~EVDEV_ABSOLUTE_MT_MOTION;
>-	}
>-	if (device->pending_events & EVDEV_ABSOLUTE_MT_UP) {
>-		notify_touch(master, time, device->mt.slot, 0, 0,
>-			     WL_TOUCH_UP);
>-               device->pending_events &= ~EVDEV_ABSOLUTE_MT_UP;
>-       }
>-       if (device->pending_events & EVDEV_ABSOLUTE_MOTION) {
>-               transform_absolute(device, &cx, &cy);
>-               weston_output_transform_coordinate(device->output,
>-                                                  cx, cy, &x, &y);
>-
>-               if (device->caps & EVDEV_TOUCH) {
>-                       if (master->num_tp == 0)
>-				notify_touch(master, time, 0,
>-					     x, y, WL_TOUCH_DOWN);
>-			else
>-				notify_touch(master, time, 0,
>-					     x, y, WL_TOUCH_MOTION);
>-		} else
>-			notify_motion_absolute(master, time, x, y);
>-		device->pending_events &= ~EVDEV_ABSOLUTE_MOTION;
>-	}
>-}
>-
> static void
> fallback_process(struct evdev_dispatch *dispatch,
> 		 struct evdev_device *device,
>@@ -327,13 +322,13 @@ fallback_process(struct evdev_dispatch *dispatch,
> 		evdev_process_relative(device, event, time);
> 		break;
> 	case EV_ABS:
>-		evdev_process_absolute(device, event);
>+		evdev_process_absolute(device, event, time);
> 		break;
> 	case EV_KEY:
> 		evdev_process_key(device, event, time);
> 		break;
> 	case EV_SYN:
>-		device->pending_events |= EVDEV_SYN_OR_SLOT;
>+		evdev_flush_pending_event(device, time);
> 		break;
> 	}
> }
>@@ -369,23 +364,13 @@ evdev_process_events(struct evdev_device *device,
> 	struct input_event *e, *end;
> 	uint32_t time = 0;
> 
>-	device->pending_events = 0;
>-
> 	e = ev;
> 	end = e + count;
> 	for (e = ev; e < end; e++) {
> 		time = e->time.tv_sec * 1000 + e->time.tv_usec / 1000;
> 
>-		/* we try to minimize the amount of notifications to be
>-		 * forwarded to the compositor, so we accumulate motion
>-		 * events and send as a bunch */
>-		if (!is_motion_event(e))
>-			evdev_flush_motion(device, time);
>-
> 		dispatch->interface->process(dispatch, device, e, time);
> 	}
>-
>-	evdev_flush_motion(device, time);
> }
> 
> static int
>@@ -601,6 +586,7 @@ evdev_device_create(struct weston_seat *seat, const char *path, int device_fd)
> 	device->rel.dy = 0;
> 	device->dispatch = NULL;
> 	device->fd = device_fd;
>+	device->pending_event = EVDEV_NONE;
> 	wl_list_init(&device->link);
> 
> 	ioctl(device->fd, EVIOCGNAME(sizeof(devname)), devname);
>diff --git a/src/evdev.h b/src/evdev.h
>index 7edcce6..6d91620 100644
>--- a/src/evdev.h
>+++ b/src/evdev.h
>@@ -31,12 +31,12 @@
> #define MAX_SLOTS 16
> 
> enum evdev_event_type {
>-	EVDEV_ABSOLUTE_MOTION = (1 << 0),
>-	EVDEV_ABSOLUTE_MT_DOWN = (1 << 1),
>-	EVDEV_ABSOLUTE_MT_MOTION = (1 << 2),
>-	EVDEV_ABSOLUTE_MT_UP = (1 << 3),
>-	EVDEV_RELATIVE_MOTION = (1 << 4),
>-	EVDEV_SYN_OR_SLOT = (1 << 5),
>+	EVDEV_NONE,
>+	EVDEV_ABSOLUTE_MOTION,
>+	EVDEV_ABSOLUTE_MT_DOWN,
>+	EVDEV_ABSOLUTE_MT_MOTION,
>+	EVDEV_ABSOLUTE_MT_UP,
>+	EVDEV_RELATIVE_MOTION,
> };
> 
> enum evdev_device_capability {
>@@ -66,8 +66,9 @@ struct evdev_device {
> 
> 	struct {
> 		int slot;
>-		int32_t x[MAX_SLOTS];
>-		int32_t y[MAX_SLOTS];
>+		struct {
>+			int32_t x, y;
>+		} slots[MAX_SLOTS];
> 	} mt;
> 	struct mtdev *mtdev;
> 
>@@ -75,7 +76,7 @@ struct evdev_device {
> 		wl_fixed_t dx, dy;
> 	} rel;
> 
>-	enum evdev_event_type pending_events;
>+	enum evdev_event_type pending_event;
> 	enum evdev_device_capability caps;
> 
> 	int is_mt;
>-- 
>1.8.3.1
>

Actions: View | Splinter Review

Attachments on bug 67563: 84292 | 86180 | 86455

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.