Cancel unloaded tile request on zooming in across multiple zoom

[pramilk]

Launch Checklist

Currently on zooming in across multiple zoom level, tile request which were previously invoked does not get cancelled.
So, if user does a fast (or has a slow network) zoom in from LOD 5 to 8. Then network panel shows that the request for 5,6,7 zoom tiles in the viewport does not get cancelled (Tiles outside the view do get cancelled).

This change will keep the tiles which have completed loading while zoom-in, but will cancel can unloaded tiles.
Zoom out did not had this bug. Have added a test for both zoom-in and zoom out scenarios.

• Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
• Briefly describe the changes in this PR.
• Link to related issues.
• Include before/after visuals or gifs if this PR includes visual changes.
• Write tests for all new functionality.
• Document any changes to public APIs.
• Post benchmark scores.
• Add an entry to CHANGELOG.md under the ## main section.

[HarelM]

Is it possible that this is by design? So that if a higher zoom level fails we will still get the next zoom level that succeeded?
i.e. if we zoom from 5 to 8 but the tiles at 8 fail to load we will still have the tiles that were successfully for zoom 7.
I'm not saying this is the expected behavior, I'm trying to see if there are possible problems with this proposal...

[pramilk]

Is it possible that this is by design? So that if a higher zoom level fails we will still get the next zoom level that succeeded? i.e. if we zoom from 5 to 8 but the tiles at 8 fail to load we will still have the tiles that were successfully for zoom 7. I'm not saying this is the expected behavior, I'm trying to see if there are possible problems with this proposal...

I think its a bug and not by design for 3 reason

1. In your example if zoom tile 8, fails then as per current code logic, it will fetch parent tile. So it will make fresh request to get parent 7 zoom tiles. There are bunch of unit test cases already in place for it.
2. Zoom out already cancels all the pending tiles. Zoom-in was not cancelling it.
3. Even if 1 above, does not work then control has LOD 5 tiles to show.

[pramilk]

Please let me know, if any other information is needed for this PR.

[HarelM]

This is a breaking change, isn't it?
I would consider adding that to the change log, otherwise approved.

[pramilk]

This is a breaking change, isn't it? I would consider adding that to the change log, otherwise approved.

Updated Changelog with [Breaking] tag.

[bdon]

FYI, I think the previous behavior was by design. Showing intermediate LOD tiles will make the progressive loading of map features feel perceptibly "faster" even if loading only the final LOD takes less time end-to-end. there are less jarring transitions between map appearance states with the old behavior.

If your interaction is with an API e.g. Mapbox where the vast majority of tile requests are edge cached and fast (~60ms) then I think the previous behavior might have a better end user experience. The new behavior is definitely better for bring-your-own-tile situations where requests can take more time.

I prefer the new behavior over the old one, but am sympathetic to certain use cases wanting an opt-in flag back to the old behavior (could demand a style spec change)

[ChrisLoer]

Showing intermediate LOD tiles will make the progressive loading of map features feel perceptibly "faster" even if loading only the final LOD takes less time end-to-end. there are less jarring transitions between map appearance states with the old behavior.

While I didn't work directly on this behavior in Mapbox GL, this description matches my understanding of what we were going for with continuing to load lower-zoom tiles. You could think of it as something like -- if network latency means you're two zoom levels behind during a 5-second zoom operation from z3 to z15, you can show a pretty intelligible map the whole time. If you're aborting intermediate tiles, the z3 tile becomes useless pretty early on and for most of the 5 seconds you're not showing anything intelligible, until the z15 tile arrives. If your network connection is kind of on the edge of being able to load tiles fast enough while you zoom, you get a kind of hard-to-predict behavior where sometimes you get a sprinkling of tiles at intermediate zooms, but you don't know which ones will or won't show up.

That said, if you're loading really data-heavy tiles (or have a slow network connection), the effect is not great -- you still don't see anything during your 5-second zoom operation, but then once you arrive at z15, you sit around waiting for the map to update with the in-flight lower zoom tiles before you get the z15 tiles you actually want.

[bdon]

Maybe there's a compromise method such as canceling every even-numbered zoom level loading in progress, to get some of the progressive appearance while still feeling faster on slow connections.

[pramilk]

There are 4 classes of issues and not just network speed that need to be considered.

1. Network speed or tile latency
2. Slow GPU - About 45% of all uses are on some form of super slow GPU like swiftshader. They get no more then 1/3-1/2 the FPS then a user with dedicated GPU.
3. Multiple tile sources - Scenarios where there are multiple tile sources, this issue get really execrated.
4. Tile size - For large vector tile, we may not want to spend all that time specially on slower machines. By the time these tiles are processed, they might not even be required as user has zoom passed the view.

[HarelM]

Maybe the middle ground here is to have this configurable? So one can decide between bandwidth and responsiveness?
I would suggest to bring this up in our next TSC meeting.

[HarelM]

This was discussed today in the TSC meeting.
There were no real inputs in the meeting though.
The more I think about it the more I believe a configuration flag is the right approach to solve this as some products will want to have the performance benefit and some will want to have a smoother user experiece.
I would recommend opening an issue and continue the discussion there as this is a merged PR.

[DanielForniessoria-TomTom]

Hi @HarelM @pramilk , could this be causing perceived performance issues?
When zooming in through multiple zoom levels, older MapLibre versions give you the appearance that the details are coming in. The new version seems to wait until the final zoom level tiles load, which makes you wait for longer without map details improving. Smells like a regression to me.
Can we please add some map init option to decide what behaviour to apply? I think this is causing performance perception issues for us, so it should have some priority.
In a way the improvement here hasn't really been an improvement for us at all.

paris-zoom-blurry.mov

[HarelM]

@DanielForniessoria-TomTom feel free to open an issue to describe the problem and/or PR to solve this.
As I said, I think a config in the map init should do it.
Note that it should keep the current behavior in order to avoid a breaking change version.
We can discuss if we want to change the default for next breaking change version.

[DanielForniessoria-TomTom]

@HarelM I reverted the change locally and tested again, see video with the result.
I like the idea to have this behaviour configurable, but I'm really inclined that the default should it be like it was before. The behaviour is seriously better. I can also zoom around quickly into areas and I can see the definition of the map coming together progressively without having to wait for the latest tiles to appear in one go.
We might want to discuss if we want MapLibre optimized by default for faster machines/connections or slower machines/connections. Machines and connections won't be getting any slower, so optimizing things by default for slow resources seems like a losing game to me.
If it were up to me, I'd revert this change now and think later to make the zoom-in-tile-cancelling behaviour opt-in.

paris-zoom-in-no-tile-cancel.mov

[HarelM]

This change was introduced in a breaking change version, we can't ignore this concept when it doesn't serve us, otherwise, what's the point of following semver.
I don't oppose to change it back as part of #3834 and until then introduce a flag in the map config (and keep it later on).
In any case, here is not where we should discuss it as this PR was already merged...