mapbox / mapbox-gl-native

Interactive, thoroughly customizable maps in native Android, iOS, macOS, Node.js, and Qt applications, powered by vector tiles and OpenGL
https://mapbox.com/mobile
Other
4.36k stars 1.33k forks source link

Gesture delay while zooming in/out #10102

Closed paulVulog closed 6 years ago

paulVulog commented 6 years ago

Hi I just updated from 5.1.3 to 5.1.4 and the zoom function gesture performance are worst than before.

It takes around 500ms delay for the map to start the zoom after the gesture. It was not the case before ( tested with nexus5x and galaxyS5 ). Moreover I am no more able to completely zoom out. At some point I am blocked and I need to start a new gesture to zoom out again.

Same issue regqrding the delay with the 5.2.0-SNAPSHOT. (but less delay - around 200ms); Edit: The zoom seems not blocked with the snapshot version. The zoom level is blocked at some point with a single gesture on the snapshot version too.

Using the same devices with the mapbox demo app (version 5.1.3) I get no delay at all during the gesture.

However, regarding this closed issue #9413, it's definitely better. I can now zoom and drag at the same time ;)

This issue is originated from : #10026

tobrun commented 6 years ago

Thank you for writing up this ticket @paulVulog. First want to clarify there isn't any difference in the gesture performance (in which I mean the time needed to execute a gesture). The difference you are noticing is the change in touch slop (the barrier the touch interaction need to cross before executing the gesture). The code related to this can be found here. I'm unable to revert that value back to half as this wasn't working correctly with the issue the PR was solving. I'm now looking into finding a value in between that provides a better UX for the issue in OP + that works well with the issue #10026 was solving.

tobrun commented 6 years ago

in #10134 I have fine tuned the gesture variables. Are you able to test this? Once merged, this will be available in the 5.2.0-SNAPSHOT build.

tobrun commented 6 years ago

@paulVulog re. the zoom level is blocked at some point with a single gesture on the snapshot version too. I haven't been able to reproduce this, can you share the steps needed to reproduce it?

paulVulog commented 6 years ago

Hi @tobrun,

for the first issue (delay) I don't know if the snapshot already contains the last fix, but it's definitely better than the 5.1.4 version. (the delay is now acceptable !)

For the second issue, here are my findings (version 5.1.4 -> Nexus 5x (Oreo)):

First, let's split the zoom behavior in three gestures :

I will link my findings to the three listeners :

MapboxMap.setOnCameraMoveCancelListener() 
MapboxMap.setOnCameraMoveStartedListener() 
MapboxMap.setOnCameraIdleListener() 

1 - The Tap & Drag gesture : The result is perfect. below is a log during the gesture: 11:55:13.283 / MapboxMap.setOnCameraMoveStartedListener() / zoom = 11,000000 11:55:17.848 / MapboxMap.setOnCameraIdleListener() / zoom = 7,229478

2 - The Double Tap The first gesture return: 14:12:25.370 / MapboxMap.setOnCameraMoveStartedListener() / zoom = 12,635269 14:12:25.686 / MapboxMap.setOnCameraIdleListener() / zoom = 12,001105 Then, successive Double Tap gesture don't trigger the methods (no logs) but perform the map zoom out in the MapView. So it looks perfect in the UI but the methods workflow is broken.

3 - The Pinch gesture I think I understand what's appends. It seems that if my two fingers are too close:

paulVulog commented 6 years ago

Hi ! any update @tobrun ?

tobrun commented 6 years ago

Sorry for the delay on this.

re.

1) Tap & Drag: will keep this "gesture" in mind with future changes to the gesture system 2) Double Tap: I couldn't reproduce that behaviour, every time I'm seeing logs as:

  10-24 02:50:41.402 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 02:50:41.412 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.422 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.452 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.472 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.492 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.512 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.532 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.552 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.572 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.593 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.613 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.633 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.643 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.673 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.693 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.713 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 02:50:41.713 24275-24275/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle

3) Having fingers close isn't an issue with the SDK but with the Android scale gesture detector: I looked into this here in case you are interested in the additional background.

@paulVulog : could you please add steps for a reproducible test for The zoom level is blocked at some point with a single gesture. I haven't been able to reproduce this and would love to get this resolved so we can ship a fix with 5.1.5.

paulVulog commented 6 years ago

Hi @tobrun,

Regarding the pinch gesture issue, this is exactly what you are targetting : https://github.com/mapbox/mapbox-gl-native/issues/8403 This is what I call :

The zoom level is blocked at some point with a single gesture

So at the end it is an Android issue and we can do almost nothing :( Works fine on Android before Nougat. https://issuetracker.google.com/issues/37131665


Regarding the Double tap gesture, I was not clear enough.

The Double Tap to zoom in works fine ( one finger gesture ) However, the gesture to zoom out ( double tap with two fingers ) is not working as expected and the camera callback doesn't return the camera changes.

Let me know if I am not clear enough. Thanks !

tobrun commented 6 years ago

@paulVulog thank you for the clarification, for the 5.2.0 release the double tap zoom out doesn't seem to be broken:

10-24 07:20:09.245 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 07:20:09.274 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:09.608 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:09.609 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle
10-24 07:20:15.255 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 07:20:15.272 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:15.603 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:15.604 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle
10-24 07:20:16.752 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 07:20:16.798 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:16.813 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:16.859 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle
10-24 07:20:19.762 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 07:20:19.824 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:20.060 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:20.117 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle
10-24 07:20:20.844 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 07:20:20.940 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:21.259 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:21.260 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle
10-24 07:20:22.510 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 07:20:22.607 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:22.884 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:22.884 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle
10-24 07:20:23.442 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 07:20:23.485 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveCanceled
10-24 07:20:23.494 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:23.823 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 07:20:23.826 13650-13650/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle

Will look into reproducing with the 5.1.4 codebase.

tobrun commented 6 years ago

With the 5.1.4 codebase I'm seeing output as:

10-24 16:25:20.717 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 16:25:20.817 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle
10-24 16:25:23.390 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 16:25:23.480 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveCanceled
10-24 16:25:23.490 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 16:25:23.520 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle
10-24 16:25:25.832 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 16:25:25.932 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle
10-24 16:25:28.565 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 16:25:28.595 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 16:25:28.645 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle
10-24 16:25:31.027 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMoveStarted: REASON_API_GESTURE
10-24 16:25:31.117 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 16:25:31.127 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 16:25:31.147 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 16:25:31.177 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 16:25:31.187 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraMove
10-24 16:25:31.408 18690-18690/com.mapbox.mapboxsdk.testapp E/CameraPositionActivity: OnCameraIdle

So this indicates to me that everything is working as expected.

We are currently working on something else related to gestures (adding velocity), feel free to look into the PR here and provide feedback.

Long term goal is to rewrite gestures from scratch in https://github.com/mapbox/mapbox-gl-native/issues/10016.

Thank you for reaching out but closing as not reproducible or not actionable (scalegesturedetector issue).