[framework] make hit slop based on device pointer kind for drag/pan/scale gestures by jonahwilliams · Pull Request #64267 · flutter/flutter

jonahwilliams · 2020-08-20T18:26:20Z

Description

Currently the framework uses fairly large "hit slop" values to disambiguate taps from drags/pans. This makes sense on touch devices where the interaction is not very precise, on mice however it can feel as if the UI is lagging. This is immediately noticeable on our infra dashboard, where it takes almost half of a grid square of drag before the actual drag kicks in.

One potential solution is to always use smaller constants depending on whether the interaction is mouse or touch based. The only reasonable choice is to use the pointer device kind and not target platform - same platform can have different input sources. This requires exposing the pointer device kind in a few new places in several of the gesture detectors, and using the enum to compute the correct hit slop from an expanded set of constants.

This almost works, however there are a few places (notably ListViews) which uses the touch hit slop as a default value in scroll physics. It does not seem like it will be easy to disambiguate a user provided scroll physics constant from the default and/or adjust it somehow - this might require significant changes to scroll physics which I have left out of this PR.

This PR does not adjust:

kTouchSlop used in scroll_physics.dart's minFlingDistance
kTouchSlop used in PrimaryPointerGestureRecognizer/LongPressGestureRecognizer

Before:

After:

packages/flutter/lib/src/gestures/constants.dart

jonahwilliams · 2020-08-20T18:26:53Z

packages/flutter/lib/src/gestures/constants.dart

+double computeHitSlop(PointerDeviceKind kind) {
+  switch (kind) {
+    case PointerDeviceKind.mouse:
+    case PointerDeviceKind.stylus:


unsure if stylus should be treated like mouse or touch, probably the latter to be safe.

goderbauer · 2020-08-24T17:52:00Z

packages/flutter/lib/src/gestures/recognizer.dart

      final bool isPostAcceptSlopPastTolerance =
          _gestureAccepted &&
-          postAcceptSlopTolerance != null &&
+           postAcceptSlopTolerance != null &&


nit: revert this?

goderbauer · 2020-08-24T17:53:45Z

/cc @gspencergoog @dkwingsmt for desktop and gesture stuff.

goderbauer · 2020-08-24T17:54:36Z

packages/flutter/lib/src/gestures/events.dart

+double computeHitSlop(PointerDeviceKind kind) {
+  switch (kind) {
+    case PointerDeviceKind.mouse:
+    case PointerDeviceKind.stylus:


Unsure whether styles is more a mouse or touch device...

yeah, I have no idea really. The only stylus I've ever used was the one that came with the Nintendo DS. In that case, it was more of a precision assist for the existing touch screen. I could imagine for a stylus like that it would not be disambiguated from a touch event.

lol, I see you asked the same question above...

And people often use stylii in place of mice when they use it on a drawing surface. I think of stylii as being much more precise than touch, so I'd go with the more precise slop on them.

Good to know!

jonahwilliams · 2020-08-24T18:31:01Z

Of course the other question is should the mouse slop be smaller? We could always file an issue and ask for feedback

jonahwilliams · 2020-09-03T19:49:25Z

Friendly ping @gspencergoog @dkwingsmt

gspencergoog · 2020-09-03T20:51:17Z

packages/flutter/lib/src/gestures/constants.dart

 const Duration kJumpTapTimeout = Duration(milliseconds: 500);
+
+/// Like [kTouchSlop], but for mouse pointers.
+const double kMouseHitSlop = 1.0; // Logical pixels;


Should we be using the word "Mouse" in these? I think something like "kPrecisePointerHitSlop" is more correct, since it wouldn't have to be a mouse (trackpads would have this set too, for instance). We use "precise" as terminology elsewhere (but we also use mouse some places too).

Renamed to follow the precisePointer pattern

gspencergoog · 2020-09-03T20:52:52Z

packages/flutter/lib/src/gestures/events.dart

+double computeHitSlop(PointerDeviceKind kind) {
+  switch (kind) {
+    case PointerDeviceKind.mouse:
+    case PointerDeviceKind.stylus:


And people often use stylii in place of mice when they use it on a drawing surface. I think of stylii as being much more precise than touch, so I'd go with the more precise slop on them.

gspencergoog · 2020-09-03T20:53:46Z

packages/flutter/lib/src/gestures/monodrag.dart

       super(debugOwner: debugOwner, kind: kind);

-  static VelocityTracker _defaultBuilder(PointerEvent ev) => VelocityTracker();
+  static VelocityTracker _defaultBuilder(PointerEvent ev) => VelocityTracker(ev.kind);


nit: it'd be nice if the variable was called event instead of just ev.

gspencergoog · 2020-09-03T20:54:50Z

packages/flutter/lib/src/widgets/scroll_configuration.dart

      case TargetPlatform.iOS:
      case TargetPlatform.macOS:
-        return (PointerEvent ev) => IOSScrollViewFlingVelocityTracker();
+        return (PointerEvent ev) => IOSScrollViewFlingVelocityTracker(ev.kind);


Can we rename ev to event here (and below) too?

gspencergoog

jamesblasco · 2020-09-13T17:06:23Z

Hello @jonahwilliams. I am using VelocityTracker for a package and this is a breaking change for me.

Looking at the code it looks like VelocityTracker doesn't depend in PointerDeviceKind to calculate the velocity no?
Could I use VelocityTracker(PointerDeviceKind.unknown) directly?

jonahwilliams · 2020-09-13T22:24:22Z

The pointer device type is used to configure the amount of touch/drag/pan slop. If you use unknown you will always have touch level low precision.

orischwartz · 2020-09-16T18:01:42Z

This change broke builds involving any library that used VelocityTracker() without the required parameter.

Any chance this can be modified to take an optional parameter defaulting to PointerDeviceKind.touch?

Here's an example of a popular plugin that broke when building against beta channel: akshathjain/sliding_up_panel#193

viplifes · 2020-09-17T12:26:34Z

@TahaTesser This change broke another popular plugin jamesblasco/modal_bottom_sheet#69

goderbauer · 2020-09-17T16:43:19Z

To avoid being broken by changes in Flutter, consider adding the tests of your packages to https://github.com/flutter/tests as per policy here: https://flutter.dev/docs/resources/compatibility.

yunior123 · 2020-09-17T18:31:20Z

I am using flutter with the mp_chart library and this is the error I am getting
/opt/hostedtoolcache/flutter/1.22.0-12.1.pre-beta/x64/.pub-cache/hosted/pub.dartlang.org/optimized_gesture_detector-0.0.5/lib/scale.dart:229:55: Error: Too few positional arguments: 1 required, 0 given.

_velocityTrackers[event.pointer] = VelocityTracker();

Any idea on how to solve it?

[framework] make slop based on device pointer kind

d529140

flutter-dashboard bot added framework flutter/packages/flutter repository. See also f: labels. work in progress; do not review labels Aug 20, 2020

googlebot added the cla: yes label Aug 20, 2020

jonahwilliams commented Aug 20, 2020

View reviewed changes

packages/flutter/lib/src/gestures/constants.dart Outdated Show resolved Hide resolved

jonahwilliams commented Aug 20, 2020

View reviewed changes

jonahwilliams added 3 commits August 21, 2020 10:29

revert changes to longpress

ecf0d63

Add simple unit test

fb1925d

Merge branch 'master' of github.com:flutter/flutter into sloppy_touch

889d3ab

jonahwilliams changed the title ~~[framework] make hit slop based on device pointer kind~~ [framework] make hit slop based on device pointer kind for drag/pan/scale gestures Aug 21, 2020

jonahwilliams marked this pull request as ready for review August 21, 2020 17:48

jonahwilliams removed the work in progress; do not review label Aug 21, 2020

fix benchmarks

3f1eeb8

goderbauer added the a: desktop Running on desktop label Aug 24, 2020

goderbauer reviewed Aug 24, 2020

View reviewed changes

goderbauer added the f: gestures flutter/packages/flutter/gestures repository. label Aug 24, 2020

goderbauer reviewed Aug 24, 2020

View reviewed changes

Update recognizer.dart

8b10831

gspencergoog requested changes Sep 3, 2020

View reviewed changes

jonahwilliams added 2 commits September 3, 2020 14:20

Merge branch 'master' of github.com:flutter/flutter into sloppy_touch

e225de7

address review comments

d38d397

jonahwilliams requested a review from gspencergoog September 3, 2020 21:40

gspencergoog approved these changes Sep 3, 2020

View reviewed changes

jonahwilliams added 2 commits September 8, 2020 13:16

Merge branch 'master' of github.com:flutter/flutter into sloppy_touch

e477d57

fix newly added test

f9eb9f8

jonahwilliams merged commit 2989881 into flutter:master Sep 8, 2020

jonahwilliams deleted the sloppy_touch branch September 8, 2020 22:53

jonahwilliams mentioned this pull request Sep 29, 2020

GestureRecognizers are too "sloppy" for accurate pointing devices #36381

Closed

mrbech mentioned this pull request Oct 11, 2020

Stylus tap gesture registering as drag/pan #67869

Closed

Conversation

jonahwilliams commented Aug 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

goderbauer commented Aug 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Aug 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Aug 24, 2020

Uh oh!

jonahwilliams commented Sep 3, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gspencergoog left a comment

Choose a reason for hiding this comment

Uh oh!

jamesblasco commented Sep 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonahwilliams commented Sep 13, 2020

Uh oh!

orischwartz commented Sep 16, 2020

Uh oh!

viplifes commented Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

goderbauer commented Sep 17, 2020

Uh oh!

yunior123 commented Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jonahwilliams commented Aug 20, 2020 •

edited

Loading

jonahwilliams Aug 24, 2020 •

edited

Loading

jamesblasco commented Sep 13, 2020 •

edited

Loading

viplifes commented Sep 17, 2020 •

edited

Loading

yunior123 commented Sep 17, 2020 •

edited

Loading