Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling easeTo/flyTo while updating the bearing and rapidly zooming crashes zoom #9793

Closed
VigibotDev opened this issue Jun 16, 2020 · 19 comments
Labels

Comments

@VigibotDev
Copy link

mapbox-gl-js version:
Last

browser:
Any

Steps to Trigger Behavior

  1. easeTo or jumpTo at 10Hz (like any good GPS)
  2. Use the zoom (mouse or mobile)

Link to Demonstration

https://jsbin.com/

Expected Behavior

Manual Zoom is responsive at 10Hz refresh rate easeTo/jumpTo
Any platform any browsers

Actual Behavior

Manual Zoom is buggy at 10Hz refresh rate easeTo/jumpTo
Any platform any browsers

@ryanhamley
Copy link
Contributor

This report is not actionable on our end as is. Can you be more specific about what "buggy" means? It would also be very helpful to include an example of the issue you're reporting in JSBin or something similar. I am not seeing any issue with zoom when I try it. I'm going to close this for now. Feel free to update and reopen.

@VigibotDev
Copy link
Author

buggy = is not responsive, very hard to change and after few minutes completely freeze

@ryanhamley
Copy link
Contributor

I still don't see any issue with zooming. We need more specifics to go on. Do you see this on every map or just with certain styles/interactions/setups (if the latter, provide a JSBin example showing the specific circumstance that causes an issue)? What are the specs of your GPU? What system are you running on?

@VigibotDev
Copy link
Author

VigibotDev commented Jun 17, 2020

I use it on a robotics website : https://www.vigibot.com/ to show location of some robots (realtime websocket one page app).

You can select the 24/7 demo robot named "Bubot", real GPS @ 10Hz you can look the small GPS noise @ maximum zoom.

I move a basic Marker @ 10Hz, I reduced the easeTo() @ maximum 1Hz because @ 10Hz all users get the Pan/Zoom unusable/locked after few use (any browsers desktop / any mobiles devices)

My code could not be more minimalist :
With ease rate reduction it's okay. But there is still a bug inside mapbox code

// Init code (once) :

let osm = new mapboxgl.Map({
 container: "osm",
 style: "mapbox://styles/serveurperso/ckbf19amu3oro1ildtlu0sgjl",
 center: [gpsLon, gpsLat],
 bearing: gpsTrack,
 zoom: 15,
 attributionControl: false
});

let marker = new mapboxgl.Marker();

// -----------------------------------------------------


  // Data refresh code @ 20Hz (websocket receiver event) :
  gpsLat = rx.getValeur32(0);
  gpsLon = rx.getValeur32(1);
  gpsTrack = rx.getValeur8(6);

  // Refresh reduction to 10Hz
  if(gpsLat != oldGpsLat || gpsLon != oldGpsLon) {
   marker.setLngLat([gpsLon, gpsLat]);
   marker.addTo(osm);

   // Refresh reduction to 1Hz only if more more than a box
   if((Math.abs(gpsLat - oldGpsLat) > 0.00001 ||
      Math.abs(gpsLon - oldGpsLon) > 0.00001) &&
      Date.now() - lastGpsMove > 1000) {

    osm.easeTo({
     center: [gpsLon, gpsLat],
     bearing: gpsTrack
    });

    oldGpsLat = gpsLat;
    oldGpsLon = gpsLon;
    lastGpsMove = Date.now();
   }
  }



@ryanhamley
Copy link
Contributor

I think I'm still not 100% clear on what it is you're trying to do or experiencing. Let me clarify. On your website, users can move robots around and as the robot moves, you update the map with a marker to display the robot's position and bearing. By 10Hz, you mean that you're updating the marker 10 times per second. Is that correct? When I looked at your website, the map zoomed just fine and appeared to update fine as well. If you need to update your marker frequently or effectively animate it, we have some examples such as https://docs.mapbox.com/mapbox-gl-js/example/animate-marker/ that may be helpful. I think this may be a question better suited for Mapbox Support because I don't think it's a bug in GL JS.

@VigibotDev
Copy link
Author

The problem is not the marker. Always perfect update.
The problem is when I use easeTo() also at 10Hz, pan and zoom freeze.
The problem is easeTo (and flyTo) even jumpTo have the bug

@VigibotDev
Copy link
Author

Manual pan and zoom freeze when we use automatic pan/zoom @ more than few Hz.

@ryanhamley
Copy link
Contributor

I still am not seeing any issue with easeTo or flyTo on my computer using Chrome on MacOS Catalina. At this point, unless you can isolate the issue to a minimal, reproducible example on a site like JSBin (e.g. not your own website but something where we can easily see and interact with the code), I'm afraid there's not much we can do to help. I don't believe there's a bug in GL JS though so you may be able to get further help on working through a problem like this by contacting Mapbox support.

@VigibotDev
Copy link
Author

I make a basic minimal html file with sinus cosinus fake coordinate @ 10Hz...

@VigibotDev
Copy link
Author

VigibotDev commented Jun 18, 2020

Here is the minimal code to display the bug, same problem on last firefox/ last chrome mobile and desktop :

https://www.serveurperso.com/temp/test.html

Zoom working for about few sec... use F5 to reset and zoom work again, and stop again after few second.
I used gentle 500ms rate, at 10Hz it's unusable.
It's frustrating not being able to use mapbox GL for more arcade/realtime viewing:(

<link rel="stylesheet" href="https://api.mapbox.com/mapbox-gl-js/v1.11.0/mapbox-gl.css" type="text/css" />
<script src="https://api.mapbox.com/mapbox-gl-js/v1.11.0/mapbox-gl.js" type="text/javascript"></script>

<div id="osm" style="width: 300px; height: 230px;"></div>

<script type="text/javascript">

let gpsLat = 48.8906828;
let gpsLon = 2.2552074;
let gpsTrack = 0;
let i = 0

mapboxgl.accessToken = "pk.eyJ1Ijoic2VydmV1cnBlcnNvIiwiYSI6ImNrYmRvY2NoajBkdG8zMG5vNTBuNjQ3anQifQ.C4myOcX-Kj2ntykVQSMf7A";

let osm = new mapboxgl.Map({
 container: "osm",
 style: "mapbox://styles/serveurperso/ckbf19amu3oro1ildtlu0sgjl",
 center: [gpsLon, gpsLat],
 bearing: gpsTrack,
 zoom: 16,
 attributionControl: false
});

let marker = new mapboxgl.Marker();

setInterval(function() {
 gpsLat = 48.8906828 + Math.sin(i / 5) / 1000;
 gpsLon = 2.2552074 + Math.cos(i / 5) / 1000;
 gpsTrack = Math.sin(i / 2) * 10;

 marker.setLngLat([gpsLon, gpsLat]);
 marker.addTo(osm);

 osm.easeTo({
  center: [gpsLon, gpsLat],
  bearing: gpsTrack
 });

 oldGpsLat = gpsLat;
 oldGpsLon = gpsLon;
 lastGpsMove = Date.now();

 i++;
}, 500);

</script>

@ryanhamley
Copy link
Contributor

Great, thanks! I'll take a look

@ryanhamley
Copy link
Contributor

OK that was very helpful. I do see the error you're reporting and in fact after doing some research, I see that the same error was reported awhile back in #7523 but we were never able to isolate a test case. I managed to simplify your test case even more and make a live example of it. If you zoom in and out rapidly on that page, you should trigger the error within a few seconds (I seemed to have slightly more difficulty triggering it on JSBin than I did locally for some reason). The map will continue updating its position but zooming will no longer be possible and you should see this._onEaseFrame is not a function in the console.

What's interesting is that this only seems to happen when easeTo is updating the bearing. If we update only the center, I can't seem to reproduce the error. The error is caused by this function:

mapbox-gl-js/src/ui/camera.js

Lines 1209 to 1217 in 1299199

_renderFrameCallback() {
const t = Math.min((browser.now() - this._easeStart) / this._easeOptions.duration, 1);
this._onEaseFrame(this._easeOptions.easing(t));
if (t < 1) {
this._easeFrameId = this._requestRenderFrame(this._renderFrameCallback);
} else {
this.stop();
}
}

specifically line 1211. While this still points to an instance of Map (which is still an instance of Camera as expected) when this error occurs, that instance does not have _onEaseFrame defined for some reason. I'm a bit stumped why this is happening. But it is a bug so I'm reopening this. Thanks for your patience @serveurperso!

@ryanhamley ryanhamley reopened this Jun 19, 2020
@ryanhamley ryanhamley changed the title Manual Zoom is buggy at 10Hz refresh rate easeTo/jumpTo Calling easeTo/flyTo while updating the bearing and rapidly zooming crashes zoom Jun 19, 2020
@leigeber
Copy link

For what it's worth we noticed a bunch of production errors for "this._onEaseFrame is not a function". On our map a UI control that only calls zoomIn is throwing the error when rapidly clicked. No easing, bearing, etc in play at all.

@ryanhamley
Copy link
Contributor

@leigeber Are you able to isolate an example that reproduces this bug? That would be super helpful in figuring out what's wrong.

@leigeber
Copy link

@ryanhamley After digging we do actually have an easeTo call deep in the code with bearing and pitch set to 0 tied to the zoomend event. Easy enough to check if the ease is needed to resolve on our end. Sounds like what you were describing above, sorry for the false alarm.

@andycalder
Copy link
Contributor

andycalder commented Jul 20, 2020

@ryanhamley @serveurperso This was an interesting bug to track down but I think I found the problem.

  • camera.easeTo() immediately calls camera._stop() -> handlers.stop()
  • handlers.stop() initiates a bearing snap to north animation if -7 deg < bearing < 7 deg (default).
  • Hence a single user call to camera.easeTo() can actually initiate two easeTo animations.
  • Both get added to the render task queue and camera._easeFrameId gets incremented twice. The first _easeFrameId is lost and the corresponding ease can never be removed from the queue.
  • A _renderFrameCallback() remains in the queue even after delete this._onEaseFrame has been called.

The offending code is here:

if (shouldSnapToNorth(this._map.getBearing())) {
this._map.resetNorth();
}

The error does not occur when you comment out line 510, or set bearingSnap: 0 when initializing the map.

@andycalder
Copy link
Contributor

I think this can be closed since the fix has been merged?

@VigibotDev
Copy link
Author

VigibotDev commented Aug 15, 2020

I can increase public frequency to 10Hz on my live website (vigibot.com) to make a test.
There is modification todo on my website to get the version or it's published on the main URL ?
Thanks.

Edit:
Work on PC v1.12, now testing on mobile

@VigibotDev
Copy link
Author

VigibotDev commented Aug 15, 2020

The remaining problem is the MOBILE user action priority for pinch/zoom (and drag) is lower than the EaseTo refresh.
@ 10Hz there is no temporal space to zoom in/out on mobile:(
But there is no more original bug, I added a checkbox to enable/disable "follow the coordinate", this workaround the zoom user action priority problem...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants