Created attachment 330653 [details] [review]
backends: Re-add support for edge scrolling with some touchpads

Add support for setting edge-scrolling separately from two-finger
scrolling. We now have 2 separate boolean settings for those, with the
Mouse panel in gnome-control-center allowing to set only one of those at
a time, but nothing precludes both being set in the configuration.

We need to handle:
- two-finger-scrolling-enabled and edge-scrolling-enabled settings both
  being set.
- those 2 settings being change out-of-order
- two-finger-scrolling being set on a device that doesn't support it
- edge-scrolling-enabled on a device that doesn't support it

And the combinations of one touchpad supporting just one of edge
scrolling and two-finger scrolling and another vice-versa.

*** Bug 768071 has been marked as a duplicate of this bug. ***

Review of attachment 330653 [details] [review]:

::: configure.ac
@@ +67,1 @@
    $CLUTTER_PACKAGE >= 1.25.6

Looks like you haven't pulled in a while :-)

::: src/backends/native/meta-input-settings-native.c
@@ +170,3 @@
+  /* Don't set edge scrolling if two-finger scrolling is enabled and available */
+  if (current & LIBINPUT_CONFIG_SCROLL_2FG &&
+      supported & LIBINPUT_CONFIG_SCROLL_2FG)

I double checked with Carlos, and:
 - get_method() will return a single value from the enum
 - set_method() will only be successful if the requested
   method is supported

So the following should be equivalent:
  if (current == LIBINPUT_CONFIG_SCROLL_2FG)
    return;

@@ +205,3 @@
+
+  if (two_finger_scroll_enabled &&
+      supported & LIBINPUT_CONFIG_SCROLL_2FG)

There's already an early return if 2fg scrolling is enabled but not supported, so this condition can be reduced to two_finger_scroll_enabled, no?

@@ +209,3 @@
+      scroll_method = LIBINPUT_CONFIG_SCROLL_2FG;
+    }
+  else if (!(current & LIBINPUT_CONFIG_SCROLL_EDGE))

or current != ...

::: src/backends/x11/meta-input-settings-x11.c
@@ +210,3 @@
   available = get_property (device, "libinput Scroll Methods Available",
                             XA_INTEGER, 8, 3);
+  defaults = get_property (device, "libinput Scroll Methods Enable",

Is this correct? I'm only seeing "libinput Scroll Method Enabled" in list-props ...

@@ +223,3 @@
     }
+
+  change_property (device, "libinput Scroll Method Enabled",

If the above is a typo and this is the same property that we retrieve in defaults, this call could be moved into the above if block

Created attachment 332211 [details] [review]
backends: Re-add support for edge scrolling with some touchpads

Add support for setting edge-scrolling separately from two-finger
scrolling. We now have 2 separate boolean settings for those, with the
Mouse panel in gnome-control-center allowing to set only one of those at
a time, but nothing precludes both being set in the configuration.

We need to handle:
- two-finger-scrolling-enabled and edge-scrolling-enabled settings both
  being set.
- those 2 settings being change out-of-order
- two-finger-scrolling being set on a device that doesn't support it
- edge-scrolling-enabled on a device that doesn't support it

And the combinations of one touchpad supporting just one of edge
scrolling and two-finger scrolling and another vice-versa.

Review of attachment 332211 [details] [review]:

LGTM now, though I'd like Carlos to double-check before pushing.

Review of attachment 332211 [details] [review]:

Looks good! I still find kinda unfortunate that edge/2fg scroll are separate settings, but there seems to be a clear precedence between those. I won't let that hold this patch.

::: src/backends/x11/meta-input-settings-x11.c
@@ +237,3 @@
+                                               gboolean                      two_finger_scroll_enabled)
+{
+  guchar values[3] = { 0 }; /* 2fg, edge, button. The last value is unused */

Now that we have this in a couple of places, IMO would be great to have an enum so reading/settings values becomes more readable. Can be done in a later commit though.

Attachment 332211 [details] pushed as 36cd717 - backends: Re-add support for edge scrolling with some touchpads

*** Bug 760814 has been marked as a duplicate of this bug. ***

*** Bug 765619 has been marked as a duplicate of this bug. ***