GNOME Bugzilla – Bug 710769
Implement cursor movement speed as a setting
Last modified: 2018-08-17 19:45:10 UTC
Cursor movement speed should be available as a setting to the user. It is important for the user to be able to specify how fast they would like the cursor to move compared to their head movement. If the GNOME mouse settings can accomplish this, it may not need to be integrated directly into the code, but the capability needs to exist somewhere.
Created attachment 302073 [details] [review] Adds ability to change cursor speed. Author: Keith Carriere Added a variable in the config file for cursor speed, and updated the nose.py plugin to handle the config variable while calculating cursor movement
Review of attachment 302073 [details] [review]: This patch worked and applied no problem. HOwever, when I pushed the commit to master on Gnome, it was rejected because the author's git email was incorrectly configured. Please fix and resubmit.
Review of attachment 302073 [details] [review]: I left a few in-line comments, mostly revolving around possible issues from adding the new key to the configuration file. Also, as mentioned by Heidi, the email for the commit author is set to `localhost.localdomain` (the default one). You can set your email in Git by following these directions: https://help.github.com/articles/setting-your-email-in-git/. While the instructions are from GitHub, they apply to any Git repository. You are then going to need to update the author of this path, which you can do with `git commit --amend --author "Your Name <your-email@example.com>" as explained at http://stackoverflow.com/q/750172/359284. ::: src/mousetrap/mousetrap.yaml @@ +52,3 @@ mousetrap.plugins.nose.NoseJoystickPlugin: threshold: 5 + speed: 0.3 For compatibility reasons, are we sure we want to slow down the cursor movement within this patch? That wasn't mentioned in the commit information, but maybe it should be. ::: src/mousetrap/plugins/nose.py @@ +46,3 @@ self._config = config self._threshold = config[self]['threshold'] + self._speed = config[self]['speed'] Right now if someone is using a custom configuration file, this will trigger a `KeyError` because the `speed` key most likely does not exist in their configuration. It might be better to use `config[self].get('speed', 1)` instead, to avoid the unexpected error and provide a reasonable fall back.
Maybe this should be a feature of Pointer? That way all plugins gain this feature. Something like this: class Pointer: def __init__(...): self._movement_multiplier = config[self]['movement-multiplier'] def move(self, dx, dy): m = self._movement_multiplier (x, y) = self.get_position() (x, y) = (m*dx + x, m*dy + y) self.set_position( (x, y) ) This is off the top of my head (i.e., not tested, not clean).
Created attachment 305496 [details] [review] Adds pointer speed control to Pointer Does not integrate with existing plugins. Just makes it possible.
Created attachment 305711 [details] [review] Adds pointer speed control to Pointer This one provides the new Pointer.move() method and integrates it with NoseJoystickPluging. So now it works out of the box.
Review of attachment 305711 [details] [review]: I left a few in-line comments, there appears to be an unexpected (also untested by me) bug. ::: src/mousetrap/plugins/nose.py @@ +62,3 @@ + def _calculate_pointer_delta(self, nose_location_in_image): + (x, y) = nose_location_in_image + (x0, y0) = self._initial_nose_location_in_image Might want to rename `x0` and `y0` to `initial_x` and `initial_y`, or another set of variables that give it a bit of context (these are the initially detected points, not the points that were just detected). @@ +75,3 @@ + def _diff(self, a, b): + d = a - b + return 0 if d < self._minimum_change_to_move_pointer else d I think this might not work the same if the delta is negative (moving left or up), as `d` will end up being negative. We previously used the equivalent of `abs(d)` to work around this. At the expense of an extra line or two, it might also be better to split this up so those not familiar with Python's version of a ternary condition can still understand what is going on. if abs(difference) < self._minimum_change_to_move_pointer: return 0 return difference ::: src/mousetrap/tests/test_gui.py @@ +59,3 @@ + self.pointer.get_position = mock.MagicMock(return_value=(2, 2)) + self.pointer.set_position = mock.Mock() + self.pointer.get_position = mock.MagicMock(return_value=(100, 50)) Do we actually want to handle requested sub-pixel pointer movements? I'm not sure if someone would expect this to only move the pointer by a single pixel in one direction (under the default conditions). While I can see this possibly being useful for higher precise tracking methods, the rounding that happens isn't documented and sub-pixel movement is not supported by GDK [1]. We may want to enforce that values passed into `move` are integers, and then document the rounding for non-integer move multipliers. [1]: https://developer.gnome.org/gdk3/stable/GdkDevice.html#gdk-device-warp
Created attachment 305924 [details] [review] Add control of pointer speed This one is more focused, and does not try to make too many significant changes to the logic of NoseJoystickPlugin.
Review of attachment 305924 [details] [review]: The patch looks good to me.
Review of attachment 305924 [details] [review]: The new check to ensure integer types are being used for deltas (dx, dy) are choking under Python3. Don't know how I missed this before. I haven't had time to debug yet. Here is how to reproduce it. First, make sure you have uninstalled any previous install of mousetrap. Then... $ PYTHON=$(which python3) ./autogen.sh $ make && make check && make distcheck $ sudo make install $ mousetrap # Loading /usr/lib/python3.4/site-packages/mousetrap/mousetrap.yaml VIDEOIO ERROR: V4L/V4L2: VIDIOC_S_CROP VIDEOIO ERROR: V4L/V4L2: VIDIOC_S_CROP Failed to load OpenCL runtime Traceback (most recent call last):
+ Trace 235217
self._fire(self.CALLBACK_RUN)
callback(**self.__arguments)
app.pointer.move(delta)
raise Exception(_('delta elements must be integer: found (%s, %s)' % (type(dx), type(dy))))
As you can see, I added a bit of code to print the types that were being detected (that code isn't in the patch, probably will be in the next patch).
mousetrap is not under active development anymore since 2015. Its codebase has been archived: https://gitlab.gnome.org/Archive/mousetrap/commits/master Closing this report as WONTFIX as part of Bugzilla Housekeeping to reflect reality. Please feel free to reopen this ticket (or rather transfer the project to GNOME Gitlab, as GNOME Bugzilla is deprecated) if anyone takes the responsibility for active development again.