Denote the "missing" vs "empty" tile behavior in `CustomSource` using the `undefined` vs `null` return value
This PR denotes the "missing" vs "empty" tile behavior in CustomSource by the undefined vs null value returned by the loadTile and prepareTile implementations.
Before the fix, we draw nothing (an empty tile) if loadTile throws or returns no data (blank space on the first screenshot). After the fix, if loadTile returns undefined, such tiles are marked as errored, and a map draws an overscaled parent tile if there is any (red tile on the second screenshot).
If prepareTile returns null, such tiles are marked as loading, and a map draws nothing. (blank space on the first screenshot). If prepareTile returns undefined, a map draws an overscaled parent tile if there is any (red tile on the second screenshot).
loadTile or prepareTile returns null
loadTile or prepareTile returns undefined
Related to #11608
Launch Checklist
- [x] briefly describe the changes in this PR
- [x] manually test the debug page
- [x] apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
Documenting conversation from elsewhere: Erroring the tile will cause the parent tile to be shown, if it is available. If we need this in some cases but not in other we'll need to support both a way to error the tile and a way to make it empty.
After some feedback, it looks like we should support both ways — to render nothing and to show an overscaled parent tile in the tile’s space.
I see two possible solutions: we can either add a boolean flag to the CustomSource constructor or use the undefined vs null returned value to denote the behavior.
// If the implementation returned `undefined` as tile data,
// mark the tile as `errored` to indicate that we have no data for it.
// A map will render an overscaled parent tile in the tile’s space.
if (data === undefined) {
tile.state = 'errored';
return callback(null);
}
// If the implementation returned `null` as tile data,
// mark the tile as `loaded` and use an an empty image as tile data.
// A map will render nothing in the tile’s space.
if (data === null) {
const emptyImage = {width: this.tileSize, height: this.tileSize, data: null};
this.loadTileData(tile, (emptyImage: any));
tile.state = 'loaded';
return callback(null);
}
@ansis @mourner, what do you think of these options?
Another option would be to catch rejected promises and treat those as errors. Error/rejection == missing tile. Undefined/null === blank tile.
@ansis yes, that's possible too. It's probably a matter of taste, but since the use-case implies that the user might want to return a missing tile explicitly, I'd prefer not to use throw to denote the missing tile and go like this: Undefined/Error/rejection == missing tile. Null === blank tile. Is that okay?
I'm not sure if it's related to this PR or not, but this is an odd behavior I observe on the custom-source debug page, in which high-zoom tiles continue to be displayed as you zoom out and until you zoom in elsewhere. I wonder if this is related to needing to manually implement your own unload function or something? I do not observe this behavior for regular tiles on the debug/debug.html page with tile debug turned on.
https://user-images.githubusercontent.com/572717/189183238-37122528-893e-4cc3-8714-944d28a6105f.mov
A little comment about mapboxgl.CustomSourceInterface<T> in version "2.10.0"
This T is expected to be HTMLImageElement | ImageData | ImageBitmap as it is declared in
export type AnySourceData =
......
| CustomSourceInterface<HTMLImageElement | ImageData | ImageBitmap>;
but raster is checked as
function isRaster(data: any): boolean {
return data instanceof window.ImageData ||
data instanceof window.ImageBitmap ||
data instanceof window.HTMLCanvasElement;
}
HTMLCanvasElement is what I am after, but I have to tweak return types as I use TS.
Thank you for the CustomSource in general :)
Hey @rreusser,
…high-zoom tiles continue to be displayed as you zoom out and until you zoom in elsewhere.
I did some digging, and it's a bug related to the retaining raster tiles when raster-fade-duration is set to 0 https://github.com/mapbox/mapbox-gl-js/issues/12241
Good catch, @SergeiMelman!
I've updated the JSDoc for loadTile.