On Sat, Aug 15, 2015 at 03:51:57PM +0200, Ming-ting Yao Wei wrote:
> This patch is a simple ellipsis size thumb detection. By using MAJOR*MINOR we
> can get what correspond to the pressure value, and we can find a suitable
> pressure threshold that can separate thumbs from fingers.
>
> Signed-off-by: Yao Wei <[email protected]>
>
> ---
> src/evdev-mt-touchpad.c | 39 ++++++++++++++++++++++++++++++++++++++-
> src/evdev-mt-touchpad.h | 7 +++++++
> 2 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index 872da98..2096dee 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -319,6 +319,14 @@ tp_process_absolute(struct tp_dispatch *tp,
> t->dirty = true;
> tp->queued |= TOUCHPAD_EVENT_MOTION;
> break;
> + case ABS_MT_TOUCH_MAJOR:
> + t->ellipsis.major = e->value;
> + /* falls through, minor = major when major changes */
no, you're not guaranteed to get a ABS_MT_TOUCH_MINOR, e.g. if the value
doesn't change. when that happens, you'd now be overwriting the minor value
with the wrong one.
> + case ABS_MT_TOUCH_MINOR:
> + t->ellipsis.minor = e->value;
> + t->dirty = true;
> + tp->queued |= TOUCHPAD_EVENT_MOTION;
> + break;
> }
> }
>
> @@ -697,8 +705,18 @@ tp_thumb_detect(struct tp_dispatch *tp, struct tp_touch
> *t, uint64_t time)
> * A finger that remains at the very bottom of the touchpad becomes
> * a thumb.
> */
> - if (t->pressure > tp->thumb.threshold)
> + if (tp->thumb.pressure_detection && t->pressure > tp->thumb.threshold)
> t->thumb.state = THUMB_STATE_YES;
> + else if (tp->thumb.ellipsis_detection) {
> + log_debug(tp_libinput_context(tp),
> + "ellipsis: major %d, minor %d => %d, threshold is
> %d\n",
> + t->ellipsis.major,
> + t->ellipsis.minor,
> + t->ellipsis.major * t->ellipsis.minor,
> + tp->thumb.threshold);
please remove such messages before patch submission.
> + if (t->ellipsis.major * t->ellipsis.minor > tp->thumb.threshold)
> + t->thumb.state = THUMB_STATE_YES;
> + }
> else if (t->point.y > tp->thumb.lower_thumb_line &&
> tp->scroll.method != LIBINPUT_CONFIG_SCROLL_EDGE &&
> t->thumb.first_touch_time + THUMB_MOVE_TIMEOUT < time)
> @@ -1783,6 +1801,25 @@ tp_init_thumb(struct tp_dispatch *tp)
> tp->thumb.upper_thumb_line = ymax - yres * 15;
> tp->thumb.lower_thumb_line = ymax - yres * 8;
>
> + abs = libevdev_get_abs_info(device->evdev, ABS_MT_TOUCH_MAJOR);
> + if (!abs)
> + goto pressure;
> +
> + /* Use ABS_MT_TOUCH_MAJOR and ABS_MT_TOUCH_MINOR as pressure
> + * detection.
> + * Use MacBookPro11,1 (13" Retina, Mid-2014) as reference,
> + * The ABS_MT_TOUCH_MAJOR * ABS_MT_TOUCH_MINOR should not
> + * exceed 350000. However, this need a test if higher res
> + * touchpads exhibit higher threshold value */
> +
> + tp->thumb.detect_thumbs = true;
> + tp->thumb.ellipsis_detection = true;
if elipsis and thumb detection are exclusive, it'd be better to have a
"mode" field that is set to THUMB_MODE_NONE, THUMB_MODE_PRESSURE, etc. that
way you can dropi detect_thumbs too.
> + tp->thumb.threshold = 350000.0;
I'd like some more explanation in how you arrived at 350000 please. it's a
pretty magic number, there should be some way of understanding why that
number and not something else.
> +
> + goto out; /* skip pressure detection */
yikes. not the best use of goto, please use helper functions instead here.
Cheers,
Peter
> +
> +pressure:
> +
> abs = libevdev_get_abs_info(device->evdev, ABS_MT_PRESSURE);
> if (!abs)
> goto out;
> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> index 3bd8425..110f824 100644
> --- a/src/evdev-mt-touchpad.h
> +++ b/src/evdev-mt-touchpad.h
> @@ -215,6 +215,11 @@ struct tp_touch {
> uint64_t first_touch_time;
> struct device_coords initial;
> } thumb;
> +
> + struct {
> + int major;
> + int minor;
> + } ellipsis;
> };
>
> struct tp_dispatch {
> @@ -347,6 +352,8 @@ struct tp_dispatch {
>
> struct {
> bool detect_thumbs;
> + bool pressure_detection;
> + bool ellipsis_detection;
> int threshold;
> int upper_thumb_line;
> int lower_thumb_line;
> --
> 2.5.0
>
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
