Opened 5 years ago

Closed 3 years ago

#3269 closed enhancement (fixed)

Custom multitouch trackpad bindings

Reported by: Tristan Croll Owned by: Tristan Croll
Priority: moderate Milestone:
Component: UI Version:
Keywords: Cc: Tom Goddard
Blocked By: Blocking:
Notify when closed: Platform: all
Project: ChimeraX

Description

This has been on the to-do list for a while, and I was recently reminded by Oli Clarke that I really need to improve support for the Mac trackpad (in particular) with ISOLDE/Clipper. I vaguely recall you saying that you'd added support for modifier keys, but I'm looking at bundles/mouse_modes/trackpad.py and I can't see anything in MultitouchTrackpad - am I looking in the right place?

In any case, I can probably muddle through for now by subclassing MultitouchTrackpad and running with it, but it would be nice to think about a more official way to customise it (perhaps a (modifier, gesture): method dict and a way to assign methods to it (e.g. ("alt", "pinch"): contour_selected_volume)?

Another question: you have two-finger swipe currently mapped to rotation rather than scroll. That's fine, but would it be problematic if modifier keys caused that gesture to revert to scrolling? That would allow me to put things almost entirely inline with Clipper's mouse/Windows touchpad behaviour: ctrl-scroll selects the map to contour; alt-scroll contours it; shift-scroll adjusts the thickness of the clipping slab. Contouring and clipping thickness could equally be mapped to pinching gestures, but that would feel quite strange for iterating through maps.

Attachments (3)

mousmodes.diff (15.3 KB ) - added by Tristan Croll 5 years ago.
Added by email2trac
mousemodes.diff (32.5 KB ) - added by Tristan Croll 5 years ago.
Added by email2trac
mousemodes-1.diff (35.1 KB ) - added by Tristan Croll 5 years ago.
Added by email2trac

Download all attachments as: .zip

Change History (19)

in reply to:  1 ; comment:1 by goddard@…, 5 years ago

I have not added any modifier key support for multi-touch trackpad modes.  Also I haven't added any ability to customize those modes.  I agree that is needed and have many times thought about doing it but have not had time.

I think it is fine if modifier keys change behavior of multitouch.

Since the current multitouch code was written with no thought at all given to customizing or allowing the user to change the behaviors it may be quite hard for you to hack your way around that.  Instead you should think about how to enhance the current multitouch implementation that would allow the customization you want, implement the changes in the mouse_modes bundle and I will put it in.

comment:2 by Eric Pettersen, 5 years ago

Component: UnassignedUI

comment:3 by Tristan Croll, 5 years ago

Owner: changed from Tom Goddard to Tristan Croll

OK, will see what I can do.

comment:4 by Tristan Croll, 5 years ago

Here's the plan I'm playing with:

  • extend the base MouseMode class to have an on_touch method.
  • modify MultitouchTrackpad so it becomes just a generator of events - that is, it calculates two-fingered swipe, pinch and twist or three-fingered swipe values, and emits a TouchEvent (an extended subclass of MouseEvent) (with the two-fingered swipe calculated as both a translation and a scroll, so the MouseMode can use the one that makes most sense to it)
  • a MouseMode will then register with MouseModes to handle touches in the same way as currently happens for standard mouse bindings - e.g. session.ui.mouse_modes.bind_mouse_mode("two-finger swipe", "alt", mode), with the available categories being "two-finger swipe" (encompassing both swipe and scroll so you don't end up with two modes fighting over the same swipe), "three-finger swipe", "pinch" and "twist".
  • My initial thought was to decompose two-finger motions into translate, pinch and twist and trigger the corresponding MouseMode for all that pass a threshold, but perhaps it would be better if, say, pinch and twist suppress the swipe term. Probably best to just try it and see?

The other issue that I've noticed after really playing with the existing implementation is that the three-fingered translation interacts weirdly with the Mac system-default binding (where a three-fingered upward swipe goes to Mission Control, and right/left switches between full-screen apps). If they're enabled, then it's really hit-and miss what happens - if the initial move direction is up, left or right then the OS binding takes precedence; if it's diagonal then ChimeraX gets it - but if you then move horizontally/vertically within the first second or so the OS steals it again. While I quite like the 3-fingered translate approach, I think it's going to be jarring for anyone who's really used to the default Mac scheme - perhaps it would be a good idea to plan for this to be optional, and offer an alternative (some modifier key + two-finger swipe).

comment:5 by Tristan Croll, 5 years ago

Cc: Tom Goddard added

Tom - realised I forgot to add you to CC when I reassigned. See my last comment - would be good to get your feedback!

comment:6 by Tom Goddard, 5 years ago

That plan sounds reasonable. Hard to know what might go wrong until you try it. Decomposing the two-finger motions into translate, pinch and twist seems good. Not sure if the current code allows for simultaneous pinch and twist -- I suspect it chooses the dominant motion, although you don't need to lift your fingers to change the type of motion.

The 3-finger touch events are a pain, as you note macOS has a bunch of default behaviors for those depending on which direction you swipe in. I tried hard to find a way to have ChimeraX take precedence and suppress the system behaviors but believe it is not possible. Since I like to use 3-finger drag and I don't use those macOS 3-finger swipe modes I disable them in macOS preferences under Trackpad. Few users will do that and will just be frustrated.

I also have 4-finger drag which currently zooms. Would be nice to handle that too.

Your scheme sounds reasonable to me. I added MouseMode callbacks vr_press(), vr_motion(), vr_release() to handle VR controller events although the MouseModes class does not dispatch those events -- instead the VR bundle directliy uses them. Having the mouse modes event dispatch them so the bindings can be done by the same mousemode command may be good. Instead I have a "vr button" command to assign the VR "mouse" modes.

As far as timing goes, this won't go into the ChimeraX 1.0, too late for that. Our daily builds are all on the release branch and we won't be back to regular development builds until that goes out in a couple weeks. So I don't expect to be able to try new assignable multitouch until the release is out.

in reply to:  7 ; comment:7 by Tristan Croll, 5 years ago

Before I take this further, could you take a quick look at this and see 
if the framework makes sense to you? Right now touchpad actions won't do 
anything except trigger a print of the computed action values to the 
log. Here's the current scenario:

An action on the touchpad will trigger exactly one of the following:
- if two fingers and they're moving in opposite directions, both pinch 
and twist values will be returned
- if two fingers and they're moving in the same direction, both 
two-finger swipe (dx, dy) and scroll (calculated from dy) will be 
returned
- if three fingers, three-finger swipe (dx,dy) will be returned
- if four fingers, four-finger swipe (dx,dy) will be returned

The idea from here would be that a given mousemode can register itself 
for pinch, twist, (2-finger swipe/scroll), 3-finger swipe or four-finger 
swipe with its choice of modifier keys, in essentially the same way as 
the current approach for normal mouse actions.

On 2020-05-21 23:08, ChimeraX wrote:

mousmodes.diff

by Tristan Croll, 5 years ago

Attachment: mousmodes.diff added

Added by email2trac

in reply to:  9 comment:8 by goddard@…, 5 years ago

I'll take a look this afternoon.  Have meetings until 2 pm today.

in reply to:  10 ; comment:9 by Tristan Croll, 5 years ago

One thing that's still a little weird: if I start a fresh session and 
load a model, often the very first 2-finger swipe will be interpreted as 
a scroll. If I lift my fingers and swipe again, the ChimeraX touchpad 
code takes over and doesn't seem to go back.

On 2020-06-04 17:11, ChimeraX wrote:

comment:10 by Tom Goddard, 5 years ago

Ok, tried your code. Seems reasonable.

I also have seen that the first two finger drag zooms instead of rotates. This happens after opening the first model by clicking on a file history thumbnail. It does not happen when the graphics is shown and the open command is used to open the file. I put in some debugging code and we simply don't get any touch events on that first two-finger drag. I suspect that is because the graphics window was just created and raised by the Qt stack widget that holds the file history html view and the graphics pane. Somehow it is not ready to get multitouch events. Probably there is some work-around to convince Qt to give the first touch events. At any rate you can report this as a separate ticket or it will simply be lost, buried as comment 11 of a ticket on a different subject. If you make a separate ticket please copy and paste this comment too.

in reply to:  12 comment:11 by Tristan Croll, 5 years ago

OK, this replicates all the two- and three-finger behaviour of the
original implementation, and my logging says it's correctly applying the
modifier keys (although I haven't tried actually registering a
multitouch+modifier mode yet). I haven't yet done the 4-finger swipe
(which in the original triggered whatever MouseMode was currently mapped
to the mouse wheel). If you want to keep that behaviour, I think the
simplest way to do so would be to create a minimal MouseMode to catch
the 4-finger swipe event and pass it on.

One other point: the two-finger twist and pinch (zoom and z-rotate by 
default) actions are currently allowed to happen simultaneously, but are 
mutually exclusive with the two-fingered swipe (xy-rotate by default). 
It would be trivial to allow all three at once, but I'm not sure if this 
would be advisable in the general case. When using them for the default 
actions it would be fine, but could get messy with modifier keys mapping 
to other modes.


On 2020-06-05 00:27, ChimeraX wrote:

mousemodes.diff

by Tristan Croll, 5 years ago

Attachment: mousemodes.diff added

Added by email2trac

in reply to:  14 ; comment:12 by goddard@…, 5 years ago

Do all default multitouch modes (2, 3, and 4 finger) work identically with your new code as in current ChimeraX?  If so I can check in your changes.

in reply to:  15 comment:13 by Tristan Croll, 5 years ago

All except the 4-finger mode appear functionally identical to me. Will 
add the 4-finger mode taking the approach I suggested (creating a new 
minimal MouseMode to calculate and pass on the wheel value). Might be 
able to do it this evening, otherwise tomorrow.

On 2020-06-09 18:18, ChimeraX wrote:

in reply to:  16 ; comment:14 by goddard@…, 5 years ago

Good thanks!  Sound like a good approach.  Whenever it is ready I can check it in.  Just want to make sure it is working since people will be using the daily builds once we restart those after 1.0 is out perhaps at the end of this week.

in reply to:  17 ; comment:15 by Tristan Croll, 5 years ago

OK, this should now replicate all prior behaviour.

On 2020-06-09 18:46, ChimeraX wrote:

mousemodes-1.diff

by Tristan Croll, 5 years ago

Attachment: mousemodes-1.diff added

Added by email2trac

comment:16 by Tristan Croll, 3 years ago

Resolution: fixed
Status: assignedclosed

Sorry for not closing this at the time...

Note: See TracTickets for help on using tickets.