Refactor CustomSource

[stepankuzmin]

This PR makes 3 changes:

1. Drops the prepareTile as it was complicating more than helping
2. Adds the clearTiles method that allows an implementation to remove all existing tiles in SourceCaches
3. Denotes the "missing" vs "empty" tile behavior in CustomSource by the undefined vs null value returned by the loadTile. More context is below.

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 returns null

loadTile returns undefined

Related to #11608
Closes #11687 #12159

Launch Checklist

• briefly describe the changes in this PR
• manually test the debug page
• apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'

[ansis]

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.

[stepankuzmin]

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?

[ansis]

Another option would be to catch rejected promises and treat those as errors. Error/rejection == missing tile. Undefined/null === blank tile.

[stepankuzmin]

@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?

[mourner]

Love how this simplifies the code significantly!

[rreusser]

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.

unloading.mov

[rreusser]

I really like the changes!! 👏 I think it's definitely headed in the right direction, but I observed what appears to be a bug in the debug page, a possible bug in tile unloading, and had a question about a possible alternative API design.

[SergeiMelman]

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 :)

[stepankuzmin]

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 #12241

[stepankuzmin]

Good catch, @SergeiMelman!

I've updated the JSDoc for loadTile.

[rreusser]

This looks good to me, thanks for the changes! My strategy now is to use an LRU cache to store canvases by z/x/y and separately use a Set to track whether particular tiles need to be redraw. This happens since there are two cases:

1. the tile's canvas is cached and can be re-used as-is
2. the tile's canvas is cached but needs to be repainted

I think this is reasonable and matches expectations, and I'm able to use it to accomplish the rendering I was after!

(Also, "raster-fade-duration": 1 solved my other problem regarding the retained tiles, thanks for tracking down the source of that issue!)