Clean up initial map load, disable zoom animation#168
Conversation
Maybe we could just parse the url first, and then choose the bbox given by the parsed points as initial map view. Even without actually calculating the route first. |
Yes, I think this is the only way to avoid this when points are specified. (For the old GH maps the world wide view still loads but shows up only very quickly and so it is not really disruptive.) |
For this PR it is kind of the same, but unfortunately we see the world view a bit longer compared to GH maps 1 and it is therefore more distracting. Strangely, I see a blurred 'Deutschland' label when I use a route in Germany, as if some intermediate tiles between the world view and the route bounding box were loaded. At least this happens for a longer route like this: But for a shorter one like this: it seems fine and I do not even get to see the world view. For the
I think you are right. First I thought we could just estimate the bounding box from the url points before we send the route query. Unfortunately it is hard to get a good estimation for this in all cases. It turns out that the tricky thing is that if we do not get a good estimate for the bounding box from the url points and then update the map view again when the routing request returns we are at risk of loading even more tiles than we would load if we did not try to estimate the bounding box at all. This is because the higher level tiles are larger than e.g. the world view tiles, because they show far more details. I tried delaying the map rendering until the route request returns using Mapbox here: You can compare this here: Compared to Note that my implementations are more of a hack to try this out. For example we won't see a map at all if the route request fails, e.g. if one of the points cannot be found. We would have to implement this more thoroughly and probably this is also related to #167. I think we need the following logic:
But unfortunately, even with this approach there is a problem: When we add the custom model editor we have to deal with requests that take a lot longer than what we are used to right now. This might mean that delaying the map rendering until the initial route request returns will no longer be an option. Or we need to show some kind of loading animation for this case and maybe show the map for a preliminary bounding box that is based on the url points for such slow requests. |
|
Thanks for this!
Ah, indeed this is problematic and also the implementation seems to be rather tricky.
Yes, sounds good. This would also (often) reduce the required animation time and might reduce flickering even when enabled for fast returning routes? (And for the most routes the initially loaded map tiles won't change due to the route geometry.) Another possibility is to store the current bbox in the URL. And if none exists in the URL we load the bbox from /info. And if we calculate a route and auto zoom to it we attach this bbox to the URL or update the old. This way one could share the URL with an arbitrary bbox (could be seen as an advantage :) ). |
I'm not sure. Like I said, first I thought it is a good idea to use the url points as a first estimation, but when I tried this I found that even when the final bounding box only changed a bit there was still some kind of flickering effect and many more tiles were loaded. I just meant for slow routing request we might have no other option if we don't want to show the map at all until the route was calculated. But this would make the implementation a lot more complicated indeed.
Yes, this could be a very good solution indeed. Would you keep the url in sync with the camera position all the time? Qwant Maps does this too. They store the zoom level and center point, which might be the easiest option, but I also wonder how this works when sharing urls between devices with different screen sizes: https://www.qwant.com/maps/#map=13.24/47.2111331/7.2227028 And yes, this way it would be possible to share a location even without calculating a route. For GH maps 1 we were able to do this with a single marker: https://graphhopper.com/maps/?point=51.404036%2C13.521234. For maps 2 this does not work as well, because the map is zoomed out to the world view currently: https://graphhopper.com/maps2/?point=51.404036%2C13.521234 |
I just pushed this here: 5aff626 and you can try it here: Actually, now it does not look so bad to me anymore?! |
|
... and to complete this little collection here is the version where we estimate the bbox from the url points using OpenLayers: 844d7d5 |
The center and zoom should solve this (?) and would be probably better than my proposed bbox. One question then is when we store a new history state (for back button vs. up-to-date URL).
Nice 👍 But indeed it loads further tiles and cancels the older ones. Wouldn't have this expected for most cases. btw: Unfortunately it does not fix the problem for the mapbox version that when you open the URL in a background tab it does not update to the bbox of the route compared to the situation when you open it in a foreground tab. (the OL version does not have this problem) |
Not necessarily, because the visible map area depends not only on the center and zoom, but also on the map size (in pixels). So it could happen that the zoom level that is calculated on a larger screen is too large (too far zoomed in) to show the entire route on a smaller screen.
Yes, good question, but maybe we can just not create a history state when we zoom/pan the map, but do create a history state whenever we calculate a new route?
As long as the older ones are canceled it should be ok (?). Can you notice a difference in performance for all these approaches and also Mapbox vs. OpenLayers?
When I right-click and choose 'Open Link in New Tab' it works, but not when I use the middle mouse button of my laptop: No idea why, but this could be something unrelated? I also don't know what the difference between using the middle mouse button and choosing 'Open Link in New Tab' is. The tab is opened in the 'background' in both cases. And yes, for the OL version it works for both Mapbox and OL. This 'estimate bbox from url points' approach seems like the easiest thing we can do right now and at least it is an improvement over the current master? But yes, it is still not ideal. The next best thing is probably storing the camera state in the URL. |
Ah, ok. Now I understand what you mean. Yes, indeed. So the bbox is better in this regard as we can say that the specified bbox should be taken as the minimum bbox.
Yes, still we would have to update the URL as otherwise the shared URL is something different. Or we have a separate "share" button like before. (probably unavoidable due to the custom model anyway)
It is a bit an overhead we could avoid but yes this should be fine.
between the mapbox versions: not really.
It also occurs in master and is unrelated but I thought that it was related to how master handles the bbox and hoped it was fixed here.
Yes and yes :) |
Yes, better in this regard, but also a bit more complicated, i.e. we need to calculate the bbox all the time using the center point, zoom level and current screen width/height. But that should be doable.
Yes, but this has always been like this. When we calculate a route in GH maps 1, and then change the camera position and share the url, our updated camera position won't be included.
Yes, probably. This is related to: #25
Ok, my main motivation for this PR was to get rid of any overhead, like the additional zoom to the info bbox and the zoom animation so we can rule out these factors to be the cause of the slow map loading. If OpenLayers is still faster with these changes that is an argument against Mapbox.
Do the rasters look ugly only in your Browser or also on your phone?
I don't know why the route rendering is so slow. |
Yes, I think the reason is that mapbox allows the partial zoom
Feel free to merge this as this is certainly an improvement over master. |
I thought on my iPhone it's not as bad as on my Laptop. Maybe because of different screen resolutions and maybe this means we could improve this also for Desktop still. But the issue with the partial zooming will remain anyway (I think).
I opened #171 now. Will leave this here as is for later reference. Closing for now. |
2 participants
Instead of actually fixing #162, which seems not so trivial, I thought we could just disable the 'zoom to info bbox' feature entirely for now. The info bbox is always equal to the entire planet view anyway, except for local development with smaller maps. We could re-consider this for example in #117, but right now the initial page load seems a lot more important, especially because we also still need to decide which map library we should continue working with (#159).
Therefore, I disabled the viewport update that is triggered by the
/infoendpoint here. I also disabled the viewport transition animation to reduce the initial map flickering as much as possible. Reloading a url that includes points still does not work perfecty though, because for raster tiles we still see a very quick view transition from the world view to the route bbox. To get rid of this maybe we even have to delay the map loading until we received the route response(?).