mapbox-gl-js
mapbox-gl-js copied to clipboard
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
![Screen Shot 2022-07-08 at 15 41 51](https://user-images.githubusercontent.com/533564/177997992-4dffb3f1-1018-43cf-bf72-5ed15db4de36.png)
loadTile
or prepareTile
returns undefined
![Screen Shot 2022-07-08 at 15 41 57](https://user-images.githubusercontent.com/533564/177998030-81e722f9-5a27-43e6-ac10-b74e6ab6d329.png)
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
.