Hi Peter, et al,
On 09/04/2014 08:31 AM, Peter Hutterer wrote:
>
> This patchset adds support for two features:
> * disabling the touchpad when a mouse is plugged in
> * allow top-software buttons to work when the touchpad is disabled
>
> Builds on the patchset sent out here:
> http://lists.freedesktop.org/archives/wayland-devel/2014-September/017032.html
Patches 1-2 look good to me, they are:
Reviewed-by: Hans de Goede <hdego...@redhat.com>
For patch 3, esp this bit:
+static void
+tp_device_added(struct evdev_device *device,
+ struct evdev_device *added_device)
+{
+ struct tp_dispatch *tp = (struct tp_dispatch*)device->dispatch;
+
+ if (tp->sendevents.current_mode !=
+ LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_EXTERNAL_MOUSE)
+ return;
+
+ if (added_device->tags & EVDEV_TAG_EXTERNAL_MOUSE)
+ tp_suspend(tp, device);
+}
Combined with Bastien's comments, has made me think that we need to treat
tp->sendevents.current_mode as a bitmask rather then an enum. I see that
the values for the libinput_config_send_events_mode enum already are bitmask
values, rather then incremental integer values.
So what I'm thinking is e.g. adding a
LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_TRACKPOINT = (1 << 3)
to the enum, make sure that the bitmasks for various tags match their
LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_foo counterparts, and then we can
change the above into:
+static void
+tp_device_added(struct evdev_device *device,
+ struct evdev_device *added_device)
+{
+ struct tp_dispatch *tp = (struct tp_dispatch*)device->dispatch;
+
+ if (added_device->tags & tp->sendevents.current_mode)
+ tp_suspend(tp, device);
+}
And then libinput users can do things like:
libinput_device_config_send_events_set_mode(dev,
LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_EXTERNAL_MOUSE |
LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_TRACKPOINT)
And we can express all the weird combinations Bastien has indicated he
may want, without needing any additional complexity inside libinput.
Note if we do this we should probably rename sendevents.current_mode,
and we need to sanity check what gets passed into
libinput_device_config_send_events_set_mode(0 a bit harder so that
an unconditional enable / disable cannot be mixed with one of the
LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_foo options.
Patch 4 is:
Reviewed-by: Hans de Goede <hdego...@redhat.com>
Patch 5 I'm not all that enthusiastic about, I see 2 issues with it:
1) Only rerouting the top buttons to the trackpoint when the touchpad
is disabled means the origin of the buttons becomes inconsistent
over time. I would like to suggest to always send the top buttons
events through the trackpoint if there is a trackpoint, this is something
which we will need anyways for middle button scrolling on the trackpoint.
2) Extending the top button area to the entire pad when disabled is
a clever hack. A bit to clever IMHO, this means that the behavior of
e.g. clicking in the center of the pad changes from registering a
left click to registering a middle click when the pad gets "disabled"
again this is inconsistent, and breaches the principle of least
surprise. A user can likely understand clicking outside of the top button
area not working when the touchpad is disabled. OTOH changing which button
we send when the touchpad gets disabled will leave the user wondering what
is going on.
I'm willing to write (a) patch(es) for this, since I need 1) anyways. Please
let me know if you plan to work on this, or want me to pick it up tomorrow
morning (CET).
Regards,
Hans
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
