update rendering after addImage and removeImage (#9016)

fix #9004

A tile needs to be reparsed in order for it to use the new image.
Reloading all tiles whenever an image is added would be expensive. For
example:
- load a map
- add an image and use it in a geojson overlay

You don't want adding that image to make the basemap tiles be reloaded.

To get around that we track all the image ids used by each tile and only
reload tiles that depend on images that changed.

src/source/source_cache.js

@@ -849,6 +849,30 @@ class SourceCache extends Evented {
         sourceLayer = sourceLayer || '_geojsonTileLayer';
         return this._state.getState(sourceLayer, feature);
     }
+
+    /**
+     * Sets the set of keys that the tile depends on. This allows tiles to
+     * be reloaded when their dependencies change.
+     */
+    setDependencies(tileKey: string | number, namespace: string, dependencies: Array<string>) {
+        const tile = this._tiles[tileKey];
+        if (tile) {
+            tile.setDependencies(namespace, dependencies);
+        }
+    }
+
+    /**
+     * Reloads all tiles that depend on the given keys.
+     */
+    reloadTilesForDependencies(namespaces: Array<string>, keys: Array<string>) {
+        for (const id in this._tiles) {
+            const tile = this._tiles[id];
+            if (tile.hasDependency(namespaces, keys)) {
+                this._reloadTile(id, 'reloading');
+            }
+        }
+        this._cache.filter(tile => !tile.hasDependency(namespaces, keys));
+    }
 }
 
 SourceCache.maxOverzooming = 10;

src/source/tile.js

@@ -97,6 +97,7 @@ class Tile {
     symbolFadeHoldUntil: ?number;
     hasSymbolBuckets: boolean;
     hasRTLText: boolean;
+    dependencies: Object;
 
     /**
      * @param {OverscaledTileID} tileID
@@ -112,6 +113,7 @@ class Tile {
         this.queryPadding = 0;
         this.hasSymbolBuckets = false;
         this.hasRTLText = false;
+        this.dependencies = {};
 
         // Counts the number of times a response was already expired when
         // received. We're using this to add a delay when making a new request
@@ -493,6 +495,28 @@ class Tile {
     setHoldDuration(duration: number) {
         this.symbolFadeHoldUntil = browser.now() + duration;
     }
+
+    setDependencies(namespace: string, dependencies: Array<string>) {
+        const index = {};
+        for (const dep of dependencies) {
+            index[dep] = true;
+        }
+        this.dependencies[namespace] = index;
+    }
+
+    hasDependency(namespaces: Array<string>, keys: Array<string>) {
+        for (const namespace of namespaces) {
+            const dependencies = this.dependencies[namespace];
+            if (dependencies) {
+                for (const key of keys) {
+                    if (dependencies[key]) {
+                        return true;
+                    }
+                }
+            }
+        }
+        return false;
+    }
 }
 
 export default Tile;

src/source/tile_cache.js

@@ -179,6 +179,24 @@ class TileCache {
 
         return this;
     }
+
+    /**
+     * Remove entries that do not pass a filter function. Used for removing
+     * stale tiles from the cache.
+     */
+    filter(filterFn: (tile: Tile) => boolean) {
+        const removed = [];
+        for (const key in this.data) {
+            for (const entry of this.data[key]) {
+                if (!filterFn(entry.value)) {
+                    removed.push(entry);
+                }
+            }
+        }
+        for (const r of removed) {
+            this.remove(r.value.tileID, r);
+        }
+    }
 }
 
 export default TileCache;

src/source/worker_tile.js

@@ -145,7 +145,7 @@ class WorkerTile {
 
         const icons = Object.keys(options.iconDependencies);
         if (icons.length) {
-            actor.send('getImages', {icons}, (err, result) => {
+            actor.send('getImages', {icons, source: this.source, tileID: this.tileID, type: 'icons'}, (err, result) => {
                 if (!error) {
                     error = err;
                     iconMap = result;
@@ -158,7 +158,7 @@ class WorkerTile {
 
         const patterns = Object.keys(options.patternDependencies);
         if (patterns.length) {
-            actor.send('getImages', {icons: patterns}, (err, result) => {
+            actor.send('getImages', {icons: patterns, source: this.source, tileID: this.tileID, type: 'patterns'}, (err, result) => {
                 if (!error) {
                     error = err;
                     patternMap = result;

src/style/style.js

@@ -63,6 +63,7 @@ import type {
 } from '../style-spec/types';
 import type {CustomLayerInterface} from './style_layer/custom_style_layer';
 import type {Validator} from './validate_style';
+import type {OverscaledTileID} from '../source/tile_id';
 
 const supportedDiffOperations = pick(diffOperations, [
     'addLayer',
@@ -119,6 +120,7 @@ class Style extends Evented {
     _updatedSources: {[string]: 'clear' | 'reload'};
     _updatedLayers: {[string]: true};
     _removedLayers: {[string]: StyleLayer};
+    _changedImages: {[string]: true};
     _updatedPaintProps: {[layer: string]: true};
     _layerOrderChanged: boolean;
 
@@ -380,6 +382,8 @@ class Style extends Evented {
                 }
             }
 
+            this._updateTilesForChangedImages();
+
             for (const id in this._updatedPaintProps) {
                 this._layers[id].updateTransitions(parameters);
             }
@@ -411,6 +415,19 @@ class Style extends Evented {
 
     }
 
+    /*
+     * Apply any queued image changes.
+     */
+    _updateTilesForChangedImages() {
+        const changedImages = Object.keys(this._changedImages);
+        if (changedImages.length) {
+            for (const name in this.sourceCaches) {
+                this.sourceCaches[name].reloadTilesForDependencies(['icons', 'patterns'], changedImages);
+            }
+            this._changedImages = {};
+        }
+    }
+
     _updateWorkerLayers(updatedIds: Array<string>, removedIds: Array<string>) {
         this.dispatcher.broadcast('updateLayers', {
             layers: this._serializeLayers(updatedIds),
@@ -426,6 +443,8 @@ class Style extends Evented {
 
         this._updatedSources = {};
         this._updatedPaintProps = {};
+
+        this._changedImages = {};
     }
 
     /**
@@ -477,6 +496,8 @@ class Style extends Evented {
             return this.fire(new ErrorEvent(new Error('An image with this name already exists.')));
         }
         this.imageManager.addImage(id, image);
+        this._changedImages[id] = true;
+        this._changed = true;
         this.fire(new Event('data', {dataType: 'style'}));
     }
 
@@ -493,6 +514,8 @@ class Style extends Evented {
             return this.fire(new ErrorEvent(new Error('No image with this name exists.')));
         }
         this.imageManager.removeImage(id);
+        this._changedImages[id] = true;
+        this._changed = true;
         this.fire(new Event('data', {dataType: 'style'}));
     }
 
@@ -1271,8 +1294,24 @@ class Style extends Evented {
 
     // Callbacks from web workers
 
-    getImages(mapId: string, params: {icons: Array<string>}, callback: Callback<{[string]: StyleImage}>) {
+    getImages(mapId: string, params: {icons: Array<string>, source: string, tileID: OverscaledTileID, type: string}, callback: Callback<{[string]: StyleImage}>) {
+
         this.imageManager.getImages(params.icons, callback);
+
+        // Apply queued image changes before setting the tile's dependencies so that the tile
+        // is not reloaded unecessarily. Without this forced update the reload could happen in cases
+        // like this one:
+        // - icons contains "my-image"
+        // - imageManager.getImages(...) triggers `onstyleimagemissing`
+        // - the user adds "my-image" within the callback
+        // - addImage adds "my-image" to this._changedImages
+        // - the next frame triggers a reload of this tile even though it already has the latest version
+        this._updateTilesForChangedImages();
+
+        const sourceCache = this.sourceCaches[params.source];
+        if (sourceCache) {
+            sourceCache.setDependencies(params.tileID.key, params.type, params.icons);
+        }
     }
 
     getGlyphs(mapId: string, params: {stacks: {[string]: Array<number>}}, callback: Callback<{[string]: {[number]: ?StyleGlyph}}>) {

test/integration/render-tests/runtime-styling/image-add-remove-add/style.json

@@ -0,0 +1,52 @@
+{
+  "version": 8,
+  "metadata": {
+    "test": {
+      "width": 64,
+      "height": 64,
+      "operations": [
+        [
+          "addImage",
+          "marker",
+          "./sprites/1x.png"
+        ],
+        [
+          "addLayer",
+          {
+            "id": "geometry",
+            "type": "symbol",
+            "source": "geometry",
+            "layout": {
+              "icon-image": "marker"
+            }
+          }
+        ],
+        [
+          "wait"
+        ],
+        [
+            "removeImage",
+            "marker"
+        ],
+        [
+            "addImage",
+            "marker",
+            "./sprites/dark.png"
+        ],
+        [
+            "wait"
+        ]
+      ]
+    }
+  },
+  "sources": {
+    "geometry": {
+      "type": "geojson",
+      "data": {
+        "type": "Point",
+        "coordinates": [0, 0]
+      }
+    }
+  },
+  "layers": []
+}

test/integration/render-tests/runtime-styling/pattern-add-remove-add/style.json

@@ -0,0 +1,52 @@
+{
+  "version": 8,
+  "metadata": {
+    "test": {
+      "width": 64,
+      "height": 64,
+      "operations": [
+        [
+          "addImage",
+          "marker",
+            "./sprites/dark.png"
+        ],
+        [
+          "addLayer",
+          {
+            "id": "geometry",
+            "type": "fill",
+            "source": "geometry",
+            "paint": {
+              "fill-pattern": "marker"
+            }
+          }
+        ],
+        [
+          "wait"
+        ],
+        [
+            "removeImage",
+            "marker"
+        ],
+        [
+            "addImage",
+            "marker",
+          "./sprites/1x.png"
+        ],
+        [
+            "wait"
+        ]
+      ]
+    }
+  },
+  "sources": {
+    "geometry": {
+      "type": "geojson",
+      "data": {
+        "type": "Polygon",
+        "coordinates": [[[-180, 80], [180, 80], [180, -80], [-180, -80]]]
+      }
+    }
+  },
+  "layers": []
+}