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

Tiles with raster-fade-duration: 0 inappropriately held for underzooming #2445

Closed
ChrisLoer opened this issue Apr 25, 2023 · 1 comment · Fixed by #2455
Closed

Tiles with raster-fade-duration: 0 inappropriately held for underzooming #2445

ChrisLoer opened this issue Apr 25, 2023 · 1 comment · Fixed by #2455
Labels
bug Something isn't working good first issue Good for newcomers PR is more than welcomed Extra attention is needed

Comments

@ChrisLoer
Copy link
Contributor

When you zoom out on a raster layer, MapLibre holds on to underzoomed child tiles for the "raster fade duration" so that it can apply a cross fade. In the case where the fade-duration is 0, it shouldn't hold onto them at all once the parent is loaded, but its logic to do that incorrectly always retains when fade-duration is 0:

if (!tile || tile.fadeEndTime && tile.fadeEndTime <= browser.now()) continue;

The tiles will stay in the pyramid potentially indefinitely, although only while in the viewport and within the "max underzooming" range. Here's an example of a satellite layer with higher-zoom (cloudy) tiles persistently drawn where we should have lower zoom tiles:

image

At Felt, we're working around this by using raster-fade-duration: 1 which isn't visibly different but avoids the logic error.

I believe the correct logic should be:

if (!tile || !tile.fadeEndTime || tile.fadeEndTime <= browser.now()) continue;

a.k.a. "don't hold for fading, if (1) the tile is already out of the pyramid, (2) the tile doesn't have a fade duration set, or (3) the tile has a fade duration set, but it's expired"

cc @ibesora

@HarelM
Copy link
Collaborator

HarelM commented Apr 27, 2023

Javascript ! operation is tricky as I'm sure you know. 0, null and undefined all return true if you place a ! operation before them, in both proposals this will not work, I think - you'll need to explicitly check for undefined i.e. fadeEndTime === undefined.
In any case, this looks like a bug that would love a PR :-)

@HarelM HarelM added bug Something isn't working good first issue Good for newcomers PR is more than welcomed Extra attention is needed labels Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers PR is more than welcomed Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants