Hi,
On 11/20/2014 07:19 AM, Peter Hutterer wrote:
On Wed, Nov 19, 2014 at 11:50:13AM +0100, Hans de Goede wrote:
[...]
+
+ switch (tp->model) {
+ case MODEL_SYNAPTICS:
+ edge_width = width * .07;
+ edge_height = height * .07;
+ break;
+ case MODEL_ALPS:
+ edge_width = width * .15;
+ edge_height = height * .15;
+ break;
+ case MODEL_APPLETOUCH:
+ case MODEL_UNIBODY_MACBOOK:
the unibody macbooks already had a clickpad, so this isn't needed
+ edge_width = width * .085;
+ edge_height = height * .085;
+ break;
+ default:
+ edge_width = width * .04;
+ edge_height = height * .054;
+ }
fwiw, there is a bit of history there and many oddities are based on a
the edges in synaptics being 'wrong'. the kernel exports the synaptics
dimensions as the coordinates produced by 'an average finger', but it's not
hard to go beyond min/max. iirc for alps and other devices the edges are the
actual edges (and for the T440-style synaptics pads that we manually fixed
up). Hence the need for different edge zones on synaptics.
And the default numbers come from the synaptics user interface guide.
Typical bezel limits: x 1472–5472 y 1408–4448
Typical edge margins: x 1632–5312 y 1568–4288
i.e. 4% and 5.4% are interpolated from those
so in short, we don't need MODEL_SYNAPTICS, it's covered by the defaults.
that just leaves APPLETOUCH and ALPS, I wonder if these have resolutions
set (the synaptics code pre-dates resolution support in the kernel). if so,
just defining a sensible size for the edge is enough (10mm?). or based on
the diagonal.
As also mentioned in a private mail not all alps devices have resolution set,
so at least for some devices we will need to take a percentage of width / height
(no idea why you mention diagonal here).
pls see comments in the other email
static int
@@ -1096,6 +1133,9 @@ tp_init(struct tp_dispatch *tp,
if (tp_init_scroll(tp) != 0)
return -1;
+ if (tp_edge_scroll_init(tp, device) != 0)
+ return -1;
tp_scroll_init() -> tp_edge_scroll_init()
+
device->seat_caps |= EVDEV_DEVICE_POINTER;
return 0;
@@ -1239,6 +1279,7 @@ evdev_mt_touchpad_create(struct evdev_device *device)
tp->model = tp_get_model(device);
+ device->dispatch = &tp->base;
I guess this is needed for some check or another? if so, can we either pass
in the data that's needed, or alternatively make it consistent that the
create method sets device->dispatch for the fallback interface as well.
This is needed because the initial value for tp->scroll.mode is set
by calling tp_scroll_config_scroll_mode_get_default_mode(device).
yeah, I think we need to wrap this differently. The current approach works,
but it's a bit of a cardhouse. specifically, in a failure case the order of
events is:
evdev_mt_touchpad_create() sets device->dispatch but returns NULL
evdev_configure_device() takes the NULL and overwrites device->dispatch
evdev_configure_device() calls evdev_device_destroy() which frees
device->dispatch
so again, it works right now but if we ever rearrange
evdev_configure_device() we'll be dealing with a freed pointer here. and the
failure case isn't easy to test for.
simple way to split this is to have an inline that takes the tp_dispatch and
is called from get_default_mode() and tp_init_touch().
Ok, fixed as suggested for V3.
Regards,
Hans
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
