GNOME Bugzilla – Bug 778119
Scale relative motion deltas with monitor scale
Last modified: 2017-05-06 23:25:00 UTC
Scale the relative pointer and tablet tool in relative mode with the monitor scale, to make the same physical motion travel the same distance over a properly scaled GUI.
Created attachment 344832 [details] [review] clutter: Add API for filtering relative motion events Add an API that allows the owner of the clutter context to alter the relative motion events (so far relative pointer events).
Created attachment 344833 [details] [review] clutter: Also filter relative tablet tool motions Pass the relative motion from tablet tools through the same filter mechanism as used for the relative pointer motions.
Created attachment 344834 [details] [review] backends/native: Scale relative input motions with monitor scale To allow for more natural pointer movements from relative pointer devices (e.g. mouse, touchpad, tablet tool in relative mode, etc), scale the relative motion from libinput with the scale of the monitor. In effect, this means that the pointer movement is twice as fast (physical movement vs numbers of pixels passed) as before, but it also means that the same physical movement crosses the distance in a GUI no matter if it is on a HiDPI monitor or not.
Review of attachment 344834 [details] [review]: ::: src/backends/native/meta-backend-native.c @@ +260,3 @@ + meta_backend_get_monitor_manager (backend); + int monitor; + MetaMonitorInfo *monitor_info; this should be converted to MetaLogicalMonitor @@ +287,3 @@ clutter_evdev_set_pointer_constrain_callback (manager, pointer_constrain_callback, NULL, NULL); + clutter_evdev_set_relative_motion_filter (manager, relative_motion_filter, NULL); might as well make use of the user_data argument and pass in the MetaMonitorManager instance. same for the existing constrain callback really
Review of attachment 344834 [details] [review]: ::: src/backends/native/meta-backend-native.c @@ +249,3 @@ +static void +relative_motion_filter (ClutterInputDevice *device, + guint32 time, the time argument is unused, maybe it shouldn't be in the API?
(In reply to Rui Matos from comment #4) > Review of attachment 344834 [details] [review] [review]: > > ::: src/backends/native/meta-backend-native.c > @@ +260,3 @@ > + meta_backend_get_monitor_manager (backend); > + int monitor; > + MetaMonitorInfo *monitor_info; > > this should be converted to MetaLogicalMonitor This is for the gnome-3-22 branch, but maybe we shouldn't mess with gnome-3.22. > > @@ +287,3 @@ > clutter_evdev_set_pointer_constrain_callback (manager, > pointer_constrain_callback, > NULL, NULL); > + clutter_evdev_set_relative_motion_filter (manager, > relative_motion_filter, NULL); > > might as well make use of the user_data argument and pass in the > MetaMonitorManager instance. same for the existing constrain callback really
Review of attachment 344832 [details] [review]: Looks correct. I however wonder, couldn't this be put together with the pointer constrain function? I mean, both are about taking some coordinates and maybe transforming those into something else. Both this and pointer constraints should probably also apply to tablets configured as relative.
Review of attachment 344834 [details] [review]: Besides the s/MetaMonitorInfo/MetaLogicalMonitor/ stuff (why didn't these patches target master btw?), I think we need to bisect motion in the case it crosses monitors, and apply the corresponding scale*distance for each of those. Otherwise quick pointer motions come across as too fast or too slow depending on the monitor they're crossing into.
Given I have the hw to test this, do you mind me continuing on these patches?
(In reply to Carlos Garnacho from comment #7) > Review of attachment 344832 [details] [review] [review]: > > Looks correct. I however wonder, couldn't this be put together with the > pointer constrain function? I mean, both are about taking some coordinates > and maybe transforming those into something else. > > Both this and pointer constraints should probably also apply to tablets > configured as relative. We constrain both absolute and relative pointer events, but we can only filter a dx/dy for relative ones. (In reply to Carlos Garnacho from comment #8) > Review of attachment 344834 [details] [review] [review]: > > Besides the s/MetaMonitorInfo/MetaLogicalMonitor/ stuff (why didn't these > patches target master btw?), I did the same patch both on top of s/MetaMonitorInfo/MetaLogicalMonitor and not, and choose the one people could test more easily (the reason this came up was because of libinput pointer acceleration having issue on hidpi, and that it would be good with something like this to test) > I think we need to bisect motion in the case it > crosses monitors, and apply the corresponding scale*distance for each of > those. Otherwise quick pointer motions come across as too fast or too slow > depending on the monitor they're crossing into. True. (In reply to Carlos Garnacho from comment #9) > Given I have the hw to test this, do you mind me continuing on these patches? I don't mind, go ahead! I pushed a rebased master branch version of the above patches to https://github.com/jadahl/mutter/commits/wip/dpi-scale-motion
Created attachment 348650 [details] [review] backends/native: Scale relative input motions with monitor scale To allow for more natural pointer movements from relative pointer devices (e.g. mouse, touchpad, tablet tool in relative mode, etc), scale the relative motion from libinput with the scale of the monitor. In effect, this means that the pointer movement is twice as fast (physical movement vs numbers of pixels passed) as before, but it also means that the same physical movement crosses the distance in a GUI no matter if it is on a HiDPI monitor or not.
Created attachment 348651 [details] [review] backends: Refactor MetaScreenDirection guessing into separate function Make it a helper MetaLogicalMonitor API, and use it on the MetaMonitorManager.
Created attachment 348652 [details] [review] backends: Add meta_logical_monitor_get_relative_direction() function This function tries all directions between two given logical monitors and returns TRUE (plus the direction) if they are arranged together, or returns FALSE if they are not.
Created attachment 348653 [details] [review] backends/native: Apply per-output scales when moving across outputs Quick motions can come across as too fast (or slow) if it crosses outputs with different scales. If this happens, check both source/dest logical monitors and try to apply the relative distance * scale that applies to each of them.
Review of attachment 348653 [details] [review]: Did you ever consider using the line intersection vector math method used by pointer barriers and pointer confinements? They do pretty much the same thing (find borders of rectangles and cut vectors) except they cut the motion vector off instead of scaling it. It'd work more or less by finding the target logical monitor, calculate the length of the vector within the target and scale it accordingly, and do the same with the source logical monitor. That is if the motion doesn't travel through more than one logical monitor (e.g. crosses the corner of one in a three triple head configuration). Using the vector math solution here would be trivial; it'd just mean calculate the length of the vector within every logical monitor and scale accordingly. See meta_line2_intersects_with() in meta-border.c.
Review of attachment 348650 [details] [review]: ::: src/backends/native/meta-backend-native.c @@ +294,3 @@ clutter_evdev_set_pointer_constrain_callback (manager, pointer_constrain_callback, NULL, NULL); + clutter_evdev_set_relative_motion_filter (manager, relative_motion_filter, NULL); I think Rui's comment was valid here; i.e. pass manager as user_data here.
(In reply to Jonas Ådahl from comment #15) > Review of attachment 348653 [details] [review] [review]: > > Did you ever consider using the line intersection vector math method used by > pointer barriers and pointer confinements? They do pretty much the same > thing (find borders of rectangles and cut vectors) except they cut the > motion vector off instead of scaling it. Nah, math is fun :P. > > It'd work more or less by finding the target logical monitor, calculate the > length of the vector within the target and scale it accordingly, and do the > same with the source logical monitor. > > That is if the motion doesn't travel through more than one logical monitor > (e.g. crosses the corner of one in a three triple head configuration). Using > the vector math solution here would be trivial; it'd just mean calculate the > length of the vector within every logical monitor and scale accordingly. Right, I didn't think of L-shaped setups. > > See meta_line2_intersects_with() in meta-border.c. Yup, indeed seems I can use that. (In reply to Jonas Ådahl from comment #16) > Review of attachment 348650 [details] [review] [review]: > > ::: src/backends/native/meta-backend-native.c > @@ +294,3 @@ > clutter_evdev_set_pointer_constrain_callback (manager, > pointer_constrain_callback, > NULL, NULL); > + clutter_evdev_set_relative_motion_filter (manager, > relative_motion_filter, NULL); > > I think Rui's comment was valid here; i.e. pass manager as user_data here. Oops, right, forgot to locally fix that one.
Created attachment 348674 [details] [review] backends/native: Scale relative input motions with monitor scale To allow for more natural pointer movements from relative pointer devices (e.g. mouse, touchpad, tablet tool in relative mode, etc), scale the relative motion from libinput with the scale of the monitor. In effect, this means that the pointer movement is twice as fast (physical movement vs numbers of pixels passed) as before, but it also means that the same physical movement crosses the distance in a GUI no matter if it is on a HiDPI monitor or not.
Created attachment 348675 [details] [review] backends/native: Apply per-output scales when moving across outputs Quick motions can come across as too fast (or slow) if it crosses outputs with different scales. If this happens, check both source/dest logical monitors and try to apply the relative distance * scale that applies to each of them.
(In reply to Carlos Garnacho from comment #12) > Created attachment 348651 [details] [review] [review] > backends: Refactor MetaScreenDirection guessing into separate function > > Make it a helper MetaLogicalMonitor API, and use it on the > MetaMonitorManager. This one is not strictly needed anymore, still makes some sense as a minor refactor IMHO. (In reply to Rui Matos from comment #5) > Review of attachment 344834 [details] [review] [review]: > > ::: src/backends/native/meta-backend-native.c > @@ +249,3 @@ > +static void > +relative_motion_filter (ClutterInputDevice *device, > + guint32 time, > > the time argument is unused, maybe it shouldn't be in the API? I also did this locally fwiw, not much of a big change, so I'm not resubmitting the first two patches.
Review of attachment 348651 [details] [review]: Name suggestion (I couldn't understand what "has direction" means): meta_logical_monitor_is_neighbour()/is_adjecent_to()
Review of attachment 348674 [details] [review]: Looks good to me.
Review of attachment 348675 [details] [review]: ::: src/backends/native/meta-backend-native.c @@ +277,3 @@ + motion = (MetaLine2) {{ x, y }, + { x + (dx * cur->scale), + y + (dy * cur->scale) }}; Quite unimportant, but the "style" of defining the lines etc differ from elsewhere. @@ +309,3 @@ + dy -= intersection.y - motion.a.y; + x = intersection.x + ((dx > 0) ? 1 : -1); + y = intersection.y + ((dy > 0) ? 1 : -1); This looks a bit odd, and I think it might create odd mini "jumps" when moving slowly across two monitors. I guess what you are doing here is avoid re-intersecting at the same place next time, but can't you just do this by setting the new x/y to intersection.x/y +/- FLOAT_EPSILON? I dealt with a similar thing clamp_to_border() and used something similar, except that it had to not be rounded away as a wl_fixed_t.
(In reply to Jonas Ådahl from comment #21) > Review of attachment 348651 [details] [review] [review]: > > Name suggestion (I couldn't understand what "has direction" means): > meta_logical_monitor_is_neighbour()/is_adjecent_to() I like the first, used the american spelling though (which seems already used in api). (In reply to Jonas Ådahl from comment #23) > Review of attachment 348675 [details] [review] [review]: > > ::: src/backends/native/meta-backend-native.c > @@ +277,3 @@ > + motion = (MetaLine2) {{ x, y }, > + { x + (dx * cur->scale), > + y + (dy * cur->scale) }}; > > Quite unimportant, but the "style" of defining the lines etc differ from > elsewhere. Fixed > > @@ +309,3 @@ > + dy -= intersection.y - motion.a.y; > + x = intersection.x + ((dx > 0) ? 1 : -1); > + y = intersection.y + ((dy > 0) ? 1 : -1); > > This looks a bit odd, and I think it might create odd mini "jumps" when > moving slowly across two monitors. > > I guess what you are doing here is avoid re-intersecting at the same place > next time, but can't you just do this by setting the new x/y to > intersection.x/y +/- FLOAT_EPSILON? I dealt with a similar thing > clamp_to_border() and used something similar, except that it had to not be > rounded away as a wl_fixed_t. seems off by more than that... I made it check the last direction, and avoid checking intersections on the border it just came from, so I can just set the intersection coordinates as the new motion starting point.
Pushed after having fixed all review comments. Attachment 344832 [details] pushed as 8f691c2 - clutter: Add API for filtering relative motion events Attachment 344833 [details] pushed as df45c50 - clutter: Also filter relative tablet tool motions Attachment 348651 [details] pushed as efae039 - backends: Refactor MetaScreenDirection guessing into separate function Attachment 348674 [details] pushed as e60dfd5 - backends/native: Scale relative input motions with monitor scale Attachment 348675 [details] pushed as 420311b - backends/native: Apply per-output scales when moving across outputs
Would it be possible to backport this to the 3.22 branch and cut a release, so we can get it in Fedora 25? It seems like a bit too large of a change to just carry as a downstream patch...thanks!
Is this functionality available in Gnome 3.24? If not, how can I get it in Gnome 3.24?
(In reply to Kanishk Dudeja from comment #27) > Is this functionality available in Gnome 3.24? > > If not, how can I get it in Gnome 3.24? It was included in mutter 3.24.1.
Thanks for the reply Jonas :).
I just installed Ubuntu Gnome 17.04 and upgraded to Gnome 3.24.1 with Mutter 3.24.1. This doesn't look to have changed anything. Can anything help on checking the following things: 1) In the Wayland session, how do i check if the libinput driver is being used or if the synaptics driver is being used? 2) In the X session, how do i check if the libinput driver is being used or if the synaptics driver is being used?
Wayland always uses libinput, synaptics is an xorg-only driver and cannot be used in a wayland compositor. Easiest way to check under X is to run xinput list-props "device name" and check the prefix for the driver property names.