[PATCH libinput 4/4] touchpad: change manual calculations of dimensions to helper functions
Hans de Goede
hdegoede at redhat.com
Fri Jul 15 08:58:06 UTC 2016
Hi,
Patch 1-3 look good to me and are:
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
This one needs some work, see comments online.
As mentioned before I'm going on vacation, so this will likely
be me last wayland-devel mail for the coming 2 weeks.
On 15-07-16 09:50, Peter Hutterer wrote:
> Wherever we use an absolute size in mm on the touchpad, switch to the new
> helper functions. In a few cases we only need one coordinate so just leave the
> other one as 0 in those cases.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> src/evdev-mt-touchpad-buttons.c | 80 +++++++++++++++++++------------------
> src/evdev-mt-touchpad-edge-scroll.c | 17 ++++----
> src/evdev-mt-touchpad.c | 33 ++++++++++-----
> 3 files changed, 72 insertions(+), 58 deletions(-)
>
> diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
> index 85a355f..811d121 100644
> --- a/src/evdev-mt-touchpad-buttons.c
> +++ b/src/evdev-mt-touchpad-buttons.c
> @@ -541,33 +541,29 @@ static void
> tp_init_softbuttons(struct tp_dispatch *tp,
> struct evdev_device *device)
> {
> - int width, height;
> - const struct input_absinfo *absinfo_x, *absinfo_y;
> - struct device_coords offset;
> - int xres, yres;
> + double width, height;
> + struct device_coords edges;
> int mb_le, mb_re; /* middle button left/right edge */
> + struct phys_coords mm = { 0.0, 0.0 };
>
> - absinfo_x = device->abs.absinfo_x;
> - absinfo_y = device->abs.absinfo_y;
> -
> - offset.x = absinfo_x->minimum,
> - offset.y = absinfo_y->minimum,
> - xres = absinfo_x->resolution;
> - yres = absinfo_y->resolution;
> - width = device->abs.dimensions.x;
> - height = device->abs.dimensions.y;
> + evdev_device_get_size(device, &width, &height);
>
> /* button height: 10mm or 15% or the touchpad height,
> whichever is smaller */
> - if ((height * 0.15)/yres > 10) {
> - tp->buttons.bottom_area.top_edge =
> - absinfo_y->maximum - 10 * yres;
> + if (height * 0.15 > 10) {
> + mm.y = height - 10;
> + edges = evdev_device_mm_to_units(device, &mm);
This evdev_device_mm_to_units() call seems redundant.
> } else {
> - tp->buttons.bottom_area.top_edge = height * .85 + offset.y;
> + mm.y = height * 0.85;
> + edges = evdev_device_mm_to_units(device, &mm);
Idem.
> }
>
> + mm.x = width * 0.5;
> + edges = evdev_device_mm_to_units(device, &mm);
This will override the result of the previous call.
> + tp->buttons.bottom_area.top_edge = edges.y;
> + tp->buttons.bottom_area.rightbutton_left_edge = edges.x;
> +
> tp->buttons.bottom_area.middlebutton_left_edge = INT_MAX;
> - tp->buttons.bottom_area.rightbutton_left_edge = width/2 + offset.x;
>
> /* if middlebutton emulation is enabled, don't init a software area */
> if (device->middlebutton.want_enabled)
> @@ -584,14 +580,21 @@ tp_init_softbuttons(struct tp_dispatch *tp,
> * All Dell touchpads appear to have a middle marker.
> */
> if (tp->device->model_flags & EVDEV_MODEL_DELL_TOUCHPAD) {
> - const int MIDDLE_BUTTON_WIDTH = 10; /* mm */
> - int half_width = MIDDLE_BUTTON_WIDTH/2 * xres; /* units */
> + mm.x = width/2 - 5; /* 10mm wide */
> + edges = evdev_device_mm_to_units(device, &mm);
> + mb_le = edges.x;
>
> - mb_le = offset.x + width/2 - half_width;
> - mb_re = offset.x + width/2 + half_width;
> + mm.x = width/2 + 5; /* 10mm wide */
> + edges = evdev_device_mm_to_units(device, &mm);
> + mb_le = edges.y;
This looks wrong, you're not setting mb_re and you're using
edges.y here. I guess this is missing a test case in the test
suite ?
> } else {
> - mb_le = offset.x + width * 0.375;
> - mb_re = offset.x + width * 0.625;
> + mm.x = width * 0.375;
> + edges = evdev_device_mm_to_units(device, &mm);
> + mb_le = edges.x;
> +
> + mm.x = width * 0.625;
> + edges = evdev_device_mm_to_units(device, &mm);
> + mb_re = edges.x;
> }
>
> tp->buttons.bottom_area.middlebutton_left_edge = mb_le;
> @@ -603,18 +606,7 @@ tp_init_top_softbuttons(struct tp_dispatch *tp,
> struct evdev_device *device,
> double topbutton_size_mult)
> {
> - int width;
> - const struct input_absinfo *absinfo_x, *absinfo_y;
> - struct device_coords offset;
> - int yres;
> -
> - absinfo_x = device->abs.absinfo_x;
> - absinfo_y = device->abs.absinfo_y;
> -
> - offset.x = absinfo_x->minimum,
> - offset.y = absinfo_y->minimum;
> - yres = absinfo_y->resolution;
> - width = device->abs.dimensions.x;
> + struct device_coords edges;
>
> if (tp->buttons.has_topbuttons) {
> /* T440s has the top button line 5mm from the top, event
> @@ -622,10 +614,20 @@ tp_init_top_softbuttons(struct tp_dispatch *tp,
> top - which maps to 15%. We allow the caller to enlarge the
> area using a multiplier for the touchpad disabled case. */
> double topsize_mm = 10 * topbutton_size_mult;
> + struct phys_coords mm;
> + double width, height;
>
> - tp->buttons.top_area.bottom_edge = offset.y + topsize_mm * yres;
> - tp->buttons.top_area.rightbutton_left_edge = width * .58 + offset.x;
> - tp->buttons.top_area.leftbutton_right_edge = width * .42 + offset.x;
> + evdev_device_get_size(device, &width, &height);
> +
> + mm.x = width * 0.58;
> + mm.y = topsize_mm;
> + edges = evdev_device_mm_to_units(device, &mm);
> + tp->buttons.top_area.bottom_edge = edges.y;
> + tp->buttons.top_area.rightbutton_left_edge = edges.x;
> +
> + mm.x = width * 0.42;
> + edges = evdev_device_mm_to_units(device, &mm);
> + tp->buttons.top_area.leftbutton_right_edge = edges.x;
> } else {
> tp->buttons.top_area.bottom_edge = INT_MIN;
> }
> diff --git a/src/evdev-mt-touchpad-edge-scroll.c b/src/evdev-mt-touchpad-edge-scroll.c
> index 610744a..7675885 100644
> --- a/src/evdev-mt-touchpad-edge-scroll.c
> +++ b/src/evdev-mt-touchpad-edge-scroll.c
> @@ -285,28 +285,29 @@ int
> tp_edge_scroll_init(struct tp_dispatch *tp, struct evdev_device *device)
> {
> struct tp_touch *t;
> - int edge_width, edge_height;
> double width, height;
> bool want_horiz_scroll = true;
> + struct device_coords edges;
> + struct phys_coords mm = { 0.0, 0.0 };
>
> + evdev_device_get_size(device, &width, &height);
> /* Touchpads smaller than 50mm are not tall enough to have a
> horizontal scroll area, it takes too much space away. But
> clickpads have enough space here anyway because of the
> software button area (and all these tiny clickpads were built
> when software buttons were a thing, e.g. Lenovo *20 series)
> */
> - if (!tp->buttons.is_clickpad) {
> - evdev_device_get_size(device, &width, &height);
> + if (!tp->buttons.is_clickpad)
> want_horiz_scroll = (height >= 50);
> - }
>
> /* 7mm edge size */
> - edge_width = device->abs.absinfo_x->resolution * 7;
> - edge_height = device->abs.absinfo_y->resolution * 7;
> + mm.x = width - 7;
> + mm.y = height - 7;
> + edges = evdev_device_mm_to_units(device, &mm);
>
> - tp->scroll.right_edge = device->abs.absinfo_x->maximum - edge_width;
> + tp->scroll.right_edge = edges.x;
> if (want_horiz_scroll)
> - tp->scroll.bottom_edge = device->abs.absinfo_y->maximum - edge_height;
> + tp->scroll.bottom_edge = edges.y;
> else
> tp->scroll.bottom_edge = INT_MAX;
>
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index a7b5a87..eff09f9 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -1993,25 +1993,32 @@ static int
> tp_init_palmdetect(struct tp_dispatch *tp,
> struct evdev_device *device)
> {
> - int width;
> + double width, height;
> + struct phys_coords mm = { 0.0, 0.0 };
> + struct device_coords edges;
>
> tp->palm.right_edge = INT_MAX;
> tp->palm.left_edge = INT_MIN;
>
> - width = device->abs.dimensions.x;
> -
> /* Wacom doesn't have internal touchpads */
> if (device->model_flags & EVDEV_MODEL_WACOM_TOUCHPAD)
> return 0;
>
> + evdev_device_get_size(device, &width, &height);
> +
> /* Enable palm detection on touchpads >= 70 mm. Anything smaller
> probably won't need it, until we find out it does */
> - if (width/device->abs.absinfo_x->resolution < 70)
> + if (width < 70.0)
> return 0;
>
> /* palm edges are 5% of the width on each side */
> - tp->palm.right_edge = device->abs.absinfo_x->maximum - width * 0.05;
> - tp->palm.left_edge = device->abs.absinfo_x->minimum + width * 0.05;
> + mm.x = width * 0.05;
> + edges = evdev_device_mm_to_units(device, &mm);
> + tp->palm.left_edge = edges.x;
> +
> + mm.x = width * 0.95;
> + edges = evdev_device_mm_to_units(device, &mm);
> + tp->palm.right_edge = edges.x;
>
> tp->palm.monitor_trackpoint = true;
>
> @@ -2038,8 +2045,9 @@ tp_init_thumb(struct tp_dispatch *tp)
> struct evdev_device *device = tp->device;
> const struct input_absinfo *abs;
> double w = 0.0, h = 0.0;
> + struct device_coords edges;
> + struct phys_coords mm = { 0.0, 0.0 };
> int xres, yres;
> - int ymax;
> double threshold;
>
> if (!tp->buttons.is_clickpad)
> @@ -2057,10 +2065,13 @@ tp_init_thumb(struct tp_dispatch *tp)
>
> /* detect thumbs by pressure in the bottom 15mm, detect thumbs by
> * lingering in the bottom 8mm */
> - ymax = tp->device->abs.absinfo_y->maximum;
> - yres = tp->device->abs.absinfo_y->resolution;
> - tp->thumb.upper_thumb_line = ymax - yres * 15;
> - tp->thumb.lower_thumb_line = ymax - yres * 8;
> + mm.y = h * 0.85;
> + edges = evdev_device_mm_to_units(device, &mm);
> + tp->thumb.upper_thumb_line = edges.y;
> +
> + mm.y = h * 0.92;
> + edges = evdev_device_mm_to_units(device, &mm);
> + tp->thumb.lower_thumb_line = edges.y;
>
> abs = libevdev_get_abs_info(device->evdev, ABS_MT_PRESSURE);
> if (!abs)
>
Regards,
Hans
More information about the wayland-devel
mailing list