Maps: Always a delay/timeout when press on a SymbolLayer/ShapeSource

[arnaudambro]

Hi there,

I noticed that there is always a timeout between the moment a SymbolLayer is pressed and the moment the action connected to this event occurs.
Is there anything we can improve there ?

PS: I thought an issue of this subject was already opened there, but I didn't find any, sorry if I am mistaking

[arnaudambro]

Hello there, am I the only one to experience this ?
Like when I press on a SymbolLayer, I need to wait for almost a second before the onPress is called. Is it happening to me only ?
I just released a beta version of my app and the first feedback I have from my user is that it’s really annoying...

[Epick362]

I am experiencing the same thing on Android. Would really like this to be fixed

[fvieira]

Same here, makes for a really lousy experience...
I'm on Android, haven't tested iOS yet.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ferdicus]

valid issue, go away stalebot

[mfazekas]

I could not reproduce on 8.0.0-rc5.
Was using code bellow i saw no noticeable delay between the click and console.log.

Maybe you have a complex map? Or lot of symbols?!

import React from 'react';
import {
  MapView,
  ShapeSource,
  SymbolLayer,
  Camera,
  Images,
} from '@react-native-mapbox-gl/maps';

const features = {
  type: 'FeatureCollection',
  features: [
    {
      type: 'Feature',
      id: 'a-feature',
      properties: {
        icon: 'example',
        text: 'example-icon-and-label',
      },
      geometry: {
        type: 'Point',
        coordinates: [-74.00597, 40.71427],
      },
    },
    {
      type: 'Feature',
      id: 'b-feature',
      properties: {
        text: 'just-label',
      },
      geometry: {
        type: 'Point',
        coordinates: [-74.001097, 40.71527],
      },
    },
    {
      type: 'Feature',
      id: 'c-feature',
      properties: {
        icon: 'example',
      },
      geometry: {
        type: 'Point',
        coordinates: [-74.00697, 40.72427],
      },
    },
  ],
};


class BugReportExample extends React.Component {
  render() {
    return (
      <>
        <MapView style={{flex: 1}}>
          <Camera centerCoordinate={[-74.00597, 40.71427]} zoomLevel={14} />
          <Images
            images={{
              example:
                'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEYAAABGEAYAAAAhvj7HAAAABmJLR0T///////8JWPfcAAAACXBIWXMAAABIAAAASABGyWs+AAAACXZwQWcAAABGAAAARgBq8Lz7AAAE4UlEQVR42u2dTWhcVRiG7512tBNI2kxptU1ji7/0T8WFRNKo7UKjJI2ltNFFoW4EF/4tbJtV68aNuBJdNcGCCFIXUiWFklARhohC/SEVU9G0o4RWdGw7JZMwmRkX71uQYpJL7jnnOzP3ezYvZ/Odc97vyz1nzv1JECiKoiiKoiiKoiiKoiiK4h2h9ABcU6sFQRAErWitmIRm+6kt0OZ2aHoUWu6EFvPQwiz1FHRmQwgn/5aen7JEUBgrX4V2/wodPAutXaDWzGilAj2+C/r0JPtvl/ZBuQUkJvUo9PE10NFOswURV0c+gna9z/GulvYtMcDwsA/aM82/8EE/CiOqVgegvVs4n2ekfW04YOzWK9BLL/qReFOaXwXddkLa57qFl+6HoEfO+5FYV3r4DOe/RToP3sPN4mpobo8fCZTS3A1oyw3pvHgHjFk3S8MabMmJveehH+vfkc6TODTiZT8SUy+6Tuy8JyXVMS+119H64wmpcdQnU2u5dN/tumfnBcPN3CNonT4HDftdj6POmYMM8/wp9YD0gKzBXwE/+XFpbxQd2CidV+PwnOGQHwY3qm79xXYerS9JmEjYjdbwrO3+ks1wN/3ea6sH63erMYHeLFqn9G6uE3Yvx93zzyumI1srGG7G1qI1x918OGarP+W/VD+Epk+gcKpfmopseUnqfBuqheKW1EHojqrxyHYHfrTJbnxlYY52mI5ofEnigdJ2tK7+aNsSJQqr9mNpunYybiRLV5gOLRSveKzXVCRLBbPvFVdWKFHYVzQVydiShKUoWINWdQPDn3PoijI/X0DCA3xY/epSAxkumEwGrelpKWeUhcjkUTAzS76VYHhJan1B1hBlYbKx9zKGCya7UsoKJQrZdNwIhgum+R4pK5QoNN8ZN4Lhgkl/LWWFEoX0m3EjGC6Y8k4pK5QolGOf/BoumOJvUlYoUSj+EDeC4YIp/CllhRKFQuwHrEyfwzDetPG7pIoJmjbjHKb081IjGL7ClPgZjeA1MU+U/6F2BloqxI1krGB45MwBDZUFXFHmZWgT8xN7y2Dp5uPJ3U79UBbh00OmIlkqmLHvHTmhRGJshfQIFgWb4NEmP16/SKqO5Ezn1fIjmscu242vLMxbJekRRIZvDTwIrXzsx19cUrQyQf83mc6rtSsMX2/go5p9+kkup/S10f+LpiM7epEt7EHr4gHoXftt95tMfufzSBuvoWBqp0334Ow7vXz3l18dGF/mqt9ksf1hFMp47HtG8+Hscx+YyPnlaA2856rfZDBwn+1CEYObsQ5obq8fm8R61Ry/hpF6Sjqv1uELb7fTgG/9SEDd6PPQFuc/m8U+WcY38fj5j7YuqXHUJ20T8O96xnXPYgVzE0x8aoZG8NdUbU56XJ7xFaSNP5envpMekDfwUlvkGv2BJ0uA1B7lIP3Q54sWg5u5HdDDs34k0JUe6ef8d0nnoW6Bgdueg+bP+pFYU3qpzPnpfzkxDU+O+S23Xrary/xIfFSt8H8J9BQ4n2elfU0MvHSvh3Z9Ah35zI/CuKkjeY5vnOO9X9o35RZ4zrMT2n0v9PgQr0ivGy6MFuhgO/s7xv7fkPbBNEn9n493oJW5Ddr6EjTLV0mbN0PT30DLT0KLE9DCJPSfd6GlkM/M/iU9P0VRFEVRFEVRFEVRFKUx+Bf7EbtqCt8N3QAAACV0RVh0Y3JlYXRlLWRhdGUAMjAwOC0wMy0xNFQxOToyOTo0Mi0wNDowMD++m78AAAAldEVYdG1vZGlmeS1kYXRlADIwMDgtMDMtMTRUMTk6Mjk6NDItMDQ6MDBgD+2LAAAAAElFTkSuQmCC',
            }}
          />
          <ShapeSource
            id={'shape-source-id-0'}
            shape={features}
            onPress={(info) => console.log('Feature pressed', info)}>
            <SymbolLayer
              id="symbol-id"
              style={{
                iconImage: ['get', 'icon'],
              }}
            />
          </ShapeSource>
        </MapView>
      </>
    );
  }
}

[arnaudambro]

@mfazekas I could reproduce your example - not with the base 64 image, it was crashing the app, I replaced with the same but jpg -, and it has the same behaviour as in my app.

I wrapped the Map in a TouchableWithoutFeedback to capture the pressing, and to calculate the timeout between the pressing and the feature feedback : it's almost always around 350ms.

import React from 'react';
import MapboxGL from '@react-native-mapbox-gl/maps';
import {View, TouchableWithoutFeedback} from 'react-native';
MapboxGL.setAccessToken(YOUR_TOKEN);
const features = {
  type: 'FeatureCollection',
  features: [
    {
      type: 'Feature',
      id: 'a-feature',
      properties: {
        icon: 'example',
        text: 'example-icon-and-label',
      },
      geometry: {
        type: 'Point',
        coordinates: [-74.00597, 40.71427],
       },
    },
    {
      type: 'Feature',
      id: 'b-feature',
      properties: {
        text: 'just-label',
      },
      geometry: {
        type: 'Point',
        coordinates: [-74.001097, 40.71527],
      },
    },
    {
      type: 'Feature',
      id: 'c-feature',
      properties: {
        icon: 'example',
      },
      geometry: {
        type: 'Point',
        coordinates: [-74.00697, 40.72427],
      },
    },
  ],
};

class BugReportExample extends React.Component {
  onStartPressing = () => {
    console.log('start pressing');
    this.startPressing = Date.now();
  };

  onFeatureFeedback = (info) => {
    console.log(
      `Feature feedback ${Date.now() - this.startPressing}ms later`,
      info,
    );
  };

  render() {
    return (
      <TouchableWithoutFeedback onPress={this.onStartPressing}>
        <View style={{flex: 1}}>
          <MapboxGL.MapView style={{flex: 1}}>
            <MapboxGL.Camera
              centerCoordinate={[-74.001097, 40.71527]}
              zoomLevel={14}
            />
            <MapboxGL.Images
              images={{
                example: require('./cbimage.jpg'),
              }}
            />
            <MapboxGL.ShapeSource
              id={'shape-source-id-0'}
              shape={features}
              onPress={this.onFeatureFeedback}>
              <MapboxGL.SymbolLayer
                id="symbol-id"
                style={{
                  iconImage: ['get', 'icon'],
                }}
              />
            </MapboxGL.ShapeSource>
          </MapboxGL.MapView>
        </View>
      </TouchableWithoutFeedback>
    );
  }
}

export default BugReportExample;

[arnaudambro]

I think that this is clearly a timeout.
I don't know if it is due to Mapbox SDK or the react-native SDK, so I don't know who can resolve the issue, but anyway, I don't think this issue should be closed until it's resolved, what do you think @mfazekas ?

[mfazekas]

@arnaudambro Thanks for the repro and the concrete timeout you were measuring. Please also comment on the platform you've measured this ios/android and simulator/device.

[arnaudambro]

The test I did was on iPhone 8 Simulator with iOS 13.2.2

[kelleyvanevert]

Same here, I'm noticing 200-300 ms of delay between a wrapper touchable and the shapesource onpress. My setup:

• Physical Android 10 (Mi A2)
• @react-native-mapbox-gl/[email protected]
• RN 0.61.5

It would be really awsome if we can circumvent or fix this issue somehow. Mapbox has been super great in terms of features and performance, but this issue really is a ux thorn :P

[kelleyvanevert]

Browsing through the code though, it doesn't seem like @react-native-mapbox-gl/map adds any delay or timeout, really only wraps around mapbox' native event.

ShapeSource.js maps its onPress prop to onMapboxShapeSourcePress, which by some indirection comes through RCTMGLShapeSourceManager.java / customEvents SHAPE_SOURCE_LAYER_CLICK, through makeShapeSourceEvent(), on RCTMGLShapeSource.java's onPress method, which is called in RCTMGLMapView.java line 592 in onMapClick, which is a listener added to MapboxMap at line 518, and that's it.

That being said, on line 519 addOnMapLongClickListener is used as well, which tempts me to think that the delay is just a natural result of the common technique of waiting to see whether the click is a long click or not.

So now I'm wondering, can we configure the MapboxMap instance in such a way as to tell it we're only interested in short clicks, or fire both? A quick investigation hasn't led to any fruitful results yet (I'm not really at home in java world), but I think this might be the path forward?

[kelleyvanevert]

...maybe, for example,

standardGestureListener.setIsLongpressEnabled(false);

after line 125 in Mapbox SDK's MapGestureDetector.java (in the method initializeGestureListeners) would do the trick.

Not that that would be good engineering, just trying to figure out the chain of causality. Properly done, the MapView component would probably add a new prop similar to the existing rotateEnabled and pitchEnabled etc.

These props find their way to the native gesture detectors like so: mMap.getGesturesManager().getRotateGestureDetector().interrupt() (line 1011 in RCTMGLMapView.java in @react-native-mapbox-gl/maps), so maybe a new prop longPressEnabled could map to something like:

mMap.getGesturesManager().getStandardGestureDetector().setIsLongpressEnabled(longPressEnabled)

inside RCTMGLMapView.java

Any thoughts?

I haven't worked on java code in years and years, but I think I might give this a try. (First I'd have to figure out how I can started developing on the java code of @react-native-mapbox-gl/maps in the first place, haha. Never done this before.)

If someone can help out figuring this out, that would be _amazing_! :D :D

[mfazekas]

@kelleyvanevert sure i think it would be great to try. First just test if mMap.getGesturesManager().setIsLongpressEnabled(false) does fix the delay issue or not.

I'd expect it to not. I'd say this delay is more related to recognizing double taps, as taps are recognized at end of touch, and then it's easy to tell if it was a long or short one, but hard to tell if it will be a double tap or not.

[kelleyvanevert]

Ah @mfazekas you're totally right, it's probably the double tap detection instead of long press. In which case ... I'm not exactly sure how we'd solve it?

I'll definitely try it out, but it'll take me some time. Because I have to get initially set up for java development etc., I'll probably not get around to it today.

[mfazekas]

@kelleyvanevert it can be as simple as react-native run-android
https://reactnative.dev/docs/running-on-device#3-run-your-app

But yes you probably need Android studio installed

[kelleyvanevert]

Well, that was surprisingly easier than I thought, haha. And indeed turning off long press detection doesn't solve the problem. Also, turning off double taps via uiSettings.setDoubleTapGesturesEnabled(false); doesn't do the trick, the double tap waiting is probably still done inside com.mapbox.android.gestures.StandardGestureDetector.SimpleStandardOnGestureListener (which can be found here I think).

Not really sure how to proceed :|

[kelleyvanevert]

I would want to execute gestureDetector.setOnDoubleTapListener(null) inside https://github.com/mapbox/mapbox-gestures-android/blob/master/library/src/main/java/com/mapbox/android/gestures/StandardGestureDetector.java, but that dependency is not included directly, so that's a bit over my head :P

[mfazekas]

@kelleyvanevert you can also try disabling quick zoom:

https://docs.mapbox.com/android/api/map-sdk/8.0.1/com/mapbox/mapboxsdk/maps/UiSettings.html#isQuickZoomGesturesEnabled--

[kelleyvanevert]

No, that doesn't have any effect. I think that setting is only used in the scale gesture listening logic in MapGestureDetector.java in the Mapbox SDK, unless there are indirect links that I'm unaware of. (I think it relates to the double-tap(-then-move) scale gesturing thing.)

Similarly to disabling the double tap gesture via UiSettings, we still aren't _pre-empting_ the underlying gesture detection's waiting for a double tap. The UI settings only decide whether and how those registered events are handled, when they are fired, it seems.

[kelleyvanevert]

I have a strong hunch that adding this line of code:

   public StandardGestureDetector(Context context, AndroidGesturesManager androidGesturesManager) {
     super(context, androidGesturesManager);

     this.gestureDetector = new GestureDetectorCompat(context, innerListener);
+    this.gestureDetector.setOnDoubleTapListener(null);
   }

would solve the issue, because skimming over (a version of) the code that implements GestureDetectorCompat, I think that if we make sure that there's no double tap listener, then it won't wait to detect double taps at all and fire an event directly (see line 279). And indeed the innerListener inside StandardGestureDetector is an instance of a doubletaplistener, so the double tap listeneer has been registered. (At least I think so, I'm not entirely sure how Java thinks about instanceof then the instance is a declared implementation of an interface instead of a subclass...)

...so I spent the last 2 hours trying to add this line of code, but I've been mired in configuration problems because this is a transitive dependency :/ The closest I think I got was adding the library to my android project, and then changing this line of code in the build.gradle file of @react-native-mapbox-gl/maps:

-    implementation 'com.mapbox.mapboxsdk:mapbox-android-gestures:0.6.0'
+    implementation project(':mapbox-gestures-android')

..but that didn't get my edited version of the code to run instead of the automatically included .jar version. So I'm thinking maybe I have to pull in the mapbox sdk itself in that way and override the gestures library? Unless it's some Gradle cache thing of course... Basically I'm just stumbling around haha.

So yeah 🤷

[kelleyvanevert]

Alright, so I was able to "fix" it, but it was a bit tricky due to the layers and private-ness of certain fields. First there's the RCTMGLMapView class in @react-native-mapbox-gl/maps, which uses the MapboxMap class in the Mapbox SDK, which uses the AndroidGesturesManager class in mapbox-gestures-android, which in turn uses StandardGestureDetector in that library, and _that_ class has a _private_ instance of GestureDetectorCompat whose listener needed some tweaking.

Luckily though, the MapboxMap allows you to provide your own instance of AndroidGesturesManager, which we're able to subclass ourselves, and then with some subclassing trickery we can override the double tap timeout behavior _within_ @react-native-mapbox-gl/maps!

Here's an overview of how I patched the code:

In RCTMGLMapView.java, override the AndroidGesturesManager instance that the MapboxMap uses to a custom subclass:

     @Override
     public void onMapReady(final MapboxMap mapboxMap) {
         mMap = mapboxMap;

+        mMap.setGesturesManager(new CustomAndroidGesturesManager(mContext), true, true);
         mMap.setStyle(new Style.Builder().fromUrl(mStyleURL));

The only point of this subclass is to override the StandardGestureDetector class, which we also have to subclass to satisfy the class system. Here's the CustomAndroidGesturesManager. (Not a diff, but the diff format lets me place emphasis on the changes made in the subclass.)

 public class CustomAndroidGesturesManager extends AndroidGesturesManager {
+    private final CustomStandardGestureDetector customStandardGestureDetector;

     public CustomAndroidGesturesManager(Context context) {
         super(context)
+        this.customStandardGestureDetector = new CustomStandardGestureDetector(context, this);

+        List<BaseGesture> detectors = this.getDetectors();
+        for (int i = 0; i < detectors.size(); i++) {
+            if (detectors.get(i) instanceof StandardGestureDetector) {
+                detectors.set(i, this.customStandardGestureDetector);
+            }
+        }
     }

     @Override
     public void setStandardGestureListener(StandardGestureDetector.StandardOnGestureListener listener) {
!        customStandardGestureDetector.setListener(listener);
     }

     @Override
     public void removeStandardGestureListener() {
!        customStandardGestureDetector.removeListener();
     }

     @Override
     public StandardGestureDetector getStandardGestureDetector() {
!        return customStandardGestureDetector;
     }
 }

And here's the CustomStandardGestureDetector, which we need in order to override the underlying GestureDetectorCompat and our inner listener. (I would have reset the listener used by the existing gesture detector, but the detector is private.) It turns out that using .setOnDoubleTapListener(null) on it does not work, but that's OK because we can also just override the behavior of the inner listener.

public class CustomStandardGestureDetector extends StandardGestureDetector {
    private final GestureDetectorCompat customGestureDetector;

    public CustomStandardGestureDetector(Context context, AndroidGesturesManager androidGesturesManager) {
        super(context, androidGesturesManager);
        this.customGestureDetector = new GestureDetectorCompat(context, this.customInnerListener);
    }

    // These two are overridden from BaseGesture, which is necessary because
    //  they're protected, and we want to use them in our subclassed gesture manager
    @Override
    protected void setListener(StandardGestureDetector.StandardOnGestureListener l) {
        super.setListener(l)
    }
    @Override
    protected void removeListener() {
        super.removeListener();
    }

    // This is where we override the listening behavior to
    //  simply treat single taps as "confirmed" single taps
    final StandardOnGestureListener customInnerListener = new StandardOnGestureListener() {

        @Override
        public boolean onSingleTapUp(MotionEvent e) {
            if (canExecute(GESTURE_TYPE_SINGLE_TAP_UP) && listener.onSingleTapUp(e)){
                return canExecute(GESTURE_TYPE_SINGLE_TAP_CONFIRMED) && listener.onSingleTapConfirmed(e);
            } else {
                return false;
            }
        }

        @Override
        public boolean onSingleTapConfirmed(MotionEvent e) {
            return true; // I guess we'll do this?
        }

        // (and then just copy over the rest here)
    };

    @Override
    protected boolean analyzeEvent(@NonNull MotionEvent motionEvent) {
        return customGestureDetector.onTouchEvent(motionEvent);
    }

    @Override
    public boolean isLongpressEnabled() {
        return customGestureDetector.isLongpressEnabled();
    }

    @Override
    public void setIsLongpressEnabled(boolean enabled) {
        customGestureDetector.setIsLongpressEnabled(enabled);
    }
}

Finally, measuring the results:

• With the (default) double tap delay, the time difference between the ShapeSource's onPress and a surrounding TouchableWithoutFeedback onPress is on average 270 ms, on my phone and within the app I'm building, and ranging between 100 and 400 ms.

• With the custom patch that overrides / removes the double tap delay, the time difference drops to an average 6 ms.

Interestingly though, although the numbers seem a super clear win, and the UX does feel better, I can't say the UX improved as dramatically as I had hoped. The delay between the press handler being called and the map rerendering the symbol layer and then the animated sheet effect within my app, is the other half of the total delay, so really I've only improved the experience by 50% haha.

_A fun (though probably undesireable) side-effect of my "overridden inner listener behavior", is that double tap zoom actually still works, except it _also_ triggers the single tap, which in my app means selecting a resource. This is just a case of tweaking the inner listener though, returning the right booleans to indicate whether the events were handled._

[mfazekas]

@karlguillotte wow, impressive fight, with a win in the end! 🥇

I think we can add an option to disable doubleTap on map, and then use the above solution, to get rid of that delay. We'll also need that option to iOS.

[kelleyvanevert]

Haha thanks, yeah this was an interesting fight indeed. The solution is not beautiful, but it does the trick. Shall I draft a PR then? What do you think, something like doubleTapEnabled on the MapView component, to align with zoomEnabled etc?

As for iOS, I haven't even started making sure my app's code works well there, so I've no idea of the state of this thing there.. also I have literally zero experience in Objective-C, so I'd be out of my depth :D

[mfazekas]

I think doubleTapEnabled sounds fine.

On ios at worst something like this should work:
https://medium.com/@mihai/disabling-the-click-delay-in-uiwebview-5cd6b18a8c19

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[arnaudambro]

it's not a wontfix bot

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ferdicus]

@kelleyvanevert, it's been a while since you've been active on this issue.
I just want to inquire if you're still working on this?

I'd like to slap a few labels on this issue and make sure, that it's still relevant

Thanks 🙇

[kelleyvanevert]

Hey!

This was a performance issue for my app, and although still relevant, after this initial investigation I never put in the time to work it into a PR, because overall it was eclipsed by more pressing work to be done on the app ;)

Maybe we can keep it open for a while longer though. The app is reaching completion, which will free up some time to get back to performance issues like these :)

[ferdicus]

Hey!

This was a performance issue for my app, and although still relevant, after this initial investigation I never put in the time to work it into a PR, because overall it was eclipsed by more pressing work to be done on the app ;)

Maybe we can keep it open for a while longer though. The app is reaching completion, which will free up some time to get back to performance issues like these :)

That'd be great, thanks 🙇

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[tr3v3r]

As a workaround, it's possible to reduce press feedback time using panResponder and queryRenderedFeaturesAtPoint method. Works much more faster

 const map = useRef(null);
  return (
    <View
      onResponderStart={async (event) => {    

        const {locationX, locationY} = event.nativeEvent;
        let locX = locationX;
        let locY = locationY;

        if (!isIOS) {
          locX = locationX * PixelRatio.get();
          locY = locationY * PixelRatio.get();
        }

        const {features} = await map.current.queryRenderedFeaturesAtPoint(
          [locX, locY],
          null,
          ['singlePoint'], // symbol layer id
        );

        onPress(features[0] || null);
      }}
      onStartShouldSetResponder={() => true}
      style={styles.container}>
      <MapboxGL.MapView
        ref={map}
   ...

Hope this helps ;)

Info about PixelRatio.get -> this comment

[range-of-motion]

I'd just like to note that I would also benefit from a solution to this 🙂

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.