Support cross-fading between old & new tiles of the same zoom level

[wagewarbler]

These changes resolve #3595.

maplibre-gl-js tracks both parent and children tiles so that when a user zooms in/out, tiles of the most recent previous zoom level are retained and kept on the map until the current layer of tiles populates. This creates a more seamless experience when navigating the layer.

Maintaining just parents and children was sufficient to cover zoom changes, but with the introduction of the setTiles method in #3208, it also then became desirable to maintain the most recent batch of tiles at the same zoom level. This PR adds a "sibling" tile cache.

Demo of map performance before the change (note the labels you can see in the underlying basemap during transitions):

Demo of map performance after the change (no labels seen during transitions, older tiles kept until new ones appear):

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 84.09091% with 14 lines in your changes missing coverage. Please review.

Project coverage is 86.57%. Comparing base (4abf018) to head (8e7aa0c).
Report is 828 commits behind head on main.

Files with missing lines	Patch %	Lines
src/source/source_cache.ts	84.52%	11 Missing and 2 partials ⚠️
src/render/draw_raster.ts	75.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4072      +/-   ##
==========================================
+ Coverage   86.43%   86.57%   +0.14%     
==========================================
  Files         241      241              
  Lines       32535    32565      +30     
  Branches     2001     1976      -25     
==========================================
+ Hits        28121    28194      +73     
+ Misses       3458     3429      -29     
+ Partials      956      942      -14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[HarelM]

Is this logic relevant only to raster source or to vector too?

Cc: @msbarry as you are looking into this area as well.
The more I read the more I think source cache is doing "too much" and should be split to different types/classes/strategies acording to the source it manages...

[wagewarbler]

Is this logic relevant only to raster source or to vector too?

Very possibly since I've seen similar behavior in vector tiles. It's on my list to better understand the parallels there since we're working on animation capability for both raster imagery and vector tiles.

Cc: @msbarry as you are looking into this area as well. The more I read the more I think source cache is doing "too much" and should be split to different types/classes/strategies acording to the source it manages...

I'm sure there are people who have a more complete view of the library as a whole, but my 2 cents here is that the files modified in this PR, and much of the associated logic is ripe for a refactor. Most of it doesn't appear to have been modified since well before Maplibre-gl split off. Selfishly I'd like to see this change incorporated prior to such an effort, but I'm happy to work with whatever makes the most sense here.

[msbarry]

Got it! As long as there are some comments in source_cache explaining what sibling tiles are and why they are needed we should be able to include in future refactors.

[HarelM]

Is it somehow possible to take a look at this issue while we are changing the logic here:

• Map makes unnecessary repeated requests against a source's tile #4074

Seems like it touches the same code and since you changed it I would hope you fully understand it and it should be easy to fix the above linked issue...?

[HarelM]

Thanks for taking the time to open this PR.
I added some minor comments.

[wagewarbler]

Got it! As long as there are some comments in source_cache explaining what sibling tiles are and why they are needed we should be able to include in future refactors.

Matching the trend in source_cache of throwing more comments on the methods, I did the same in this commit. Additionally, please feel free to tag me on any refactors in the future. Happy to help however I can.

[HarelM]

@wagewarbler any inputs about the issue by any chance? #4074

[wagewarbler]

@wagewarbler any inputs about the issue by any chance? #4074

So far only that I've also observed this behavior and I have some high-level guesses that require a little more digging. Will comment on the issue next! Thanks for the latest round of feedback, had one other thing come up but will circle back soon.

[HarelM]

Are the following coverage missing lines related to this PR or are they considered missing because you moved the logic into a method?

[HarelM]

I found the answer, it was moved.
The following are not covered though:

Seems like an edge case, so it would be nice to add a test that cover this flow, but not mandatory.
Let me know what you decide to do.

[wagewarbler]

Seems like an edge case, so it would be nice to add a test that cover this flow, but not mandatory. Let me know what you decide to do.

Not a problem. I've added a test to cover it.

[HarelM]

Are you sure? I don't think the coverage report have changed...
Am I missing anything?

[wagewarbler]

Are you sure? I don't think the coverage report have changed... Am I missing anything?

Strange, maybe I'm missing something. I've added this test which I've run locally and executes & passes with the expected results (the bottom one):

I could certainly make the test more rigorous, but seems like overkill. In its current form it should be sufficiently executing code within the if statement, otherwise we'd end up with the original length 4 array.

[wagewarbler]

Actually, I think I see what's happening. The tile cache doesn't permit adding duplicate tiles, so this logic is unnecessary. I can go ahead and remove the test and the if statement.