From 012e9510ff7d2fe57b7626ba4e7e8b9b61bd88f2 Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Fri, 22 Nov 2019 13:45:57 -0500 Subject: [PATCH 1/2] update rendering after addImage and removeImage 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 | 24 ++++++++ src/source/tile.js | 24 ++++++++ src/source/tile_cache.js | 18 ++++++ src/source/worker_tile.js | 4 +- src/style/style.js | 41 +++++++++++++- .../image-add-remove-add/expected.png | Bin 0 -> 193 bytes .../image-add-remove-add/style.json | 52 ++++++++++++++++++ 7 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 test/integration/render-tests/runtime-styling/image-add-remove-add/expected.png create mode 100644 test/integration/render-tests/runtime-styling/image-add-remove-add/style.json diff --git a/src/source/source_cache.js b/src/source/source_cache.js index d602cec1eb7..1091843cc0f 100644 --- a/src/source/source_cache.js +++ b/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) { + const tile = this._tiles[tileKey]; + if (tile) { + tile.setDependencies(namespace, dependencies); + } + } + + /** + * Reloads all tiles that depend on the given keys. + */ + reloadTilesForDependencies(namespaces: Array, keys: Array) { + 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; diff --git a/src/source/tile.js b/src/source/tile.js index e714007e2e1..ad26b13adce 100644 --- a/src/source/tile.js +++ b/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) { + const index = {}; + for (const dep of dependencies) { + index[dep] = true; + } + this.dependencies[namespace] = index; + } + + hasDependency(namespaces: Array, keys: Array) { + 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; diff --git a/src/source/tile_cache.js b/src/source/tile_cache.js index 678105fa1f3..a78c002eb7c 100644 --- a/src/source/tile_cache.js +++ b/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; diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index 17cd7f11b43..1aa8a5c5c68 100644 --- a/src/source/worker_tile.js +++ b/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; diff --git a/src/style/style.js b/src/style/style.js index 6ab17885fb9..a80c91c9b30 100644 --- a/src/style/style.js +++ b/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, removedIds: Array) { 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}, callback: Callback<{[string]: StyleImage}>) { + getImages(mapId: string, params: {icons: Array, 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}}, callback: Callback<{[string]: {[number]: ?StyleGlyph}}>) { diff --git a/test/integration/render-tests/runtime-styling/image-add-remove-add/expected.png b/test/integration/render-tests/runtime-styling/image-add-remove-add/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..94d4b5440aa365a3a66a168d23703ceec0e16765 GIT binary patch literal 193 zcmeAS@N?(olHy`uVBq!ia0vp^4j|0I1|(Ny7TyC=9iA?ZAr*{I4<6(^V8C<0fz@Qz z@o#x=dH%LEy;JdfL=1W|X=de*gmQjyk2yy?QBcgL-R<&|EIKC>L_5S){MgML@5C Date: Wed, 27 Nov 2019 09:49:38 -0500 Subject: [PATCH 2/2] add pattern test --- .../pattern-add-remove-add/expected.png | Bin 0 -> 4533 bytes .../pattern-add-remove-add/style.json | 52 ++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 test/integration/render-tests/runtime-styling/pattern-add-remove-add/expected.png create mode 100644 test/integration/render-tests/runtime-styling/pattern-add-remove-add/style.json diff --git a/test/integration/render-tests/runtime-styling/pattern-add-remove-add/expected.png b/test/integration/render-tests/runtime-styling/pattern-add-remove-add/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..8325b5ef81a69b4c867cad07d44c6a4f68480c85 GIT binary patch literal 4533 zcmcJTc|6o>+rY`jDH{8p#K+$D=LZwVi|@XWntQI^9erN{R;M*ulR2m8>NT^X=OU9av*xA)1&E z8visvV?(wrMp%uH2GtiJmzMNhPI6zv5Y|=n$giKYYy9aUK2}VGUE!2_y7EyXO~_M4 zWgwkX6iKA%VeR$I zUabrBas(bfF<4aWeztpN{2{U{8mLa0^^)}VGAamLwkVWmOd=2^fkQEy?wltJzdZyh zG0h3{&!5L|nm#G9aGgoU5F*2-)aCdRG>M%2?~96cV_#G?*Ih`4mYApYNUCCDtJyXV zL`nH(x`bIoGO~-Dg6zU8#J(l1pO34iu3YZzX{!Sf)>n4@TgFL`;Hb)jcAeYHL|Yy1 zs8E+QqgyaoS=b5fGUeO#I{*^78I3G>i!gvJ7_7T4H1D__D|6E+!>D z?V58JbE$zd(8&^4HhA9om#wesaPf-LXSCLwmw&qA{^C~FM^)truB7}zLZ2g&t?fa1 zr{^U~)vH#1E5B@E>|uZZo3QfyiAE2{?JqEE)x>nn#kjmn5TQnv3Q!K;5AQk;;!=F} zRaAs%8xsRop3z5~UwIaz*qC@?N#VmaCaHNWi^T2QVIGcJl$kVG4A@k=Gofo ziblds9$@<>ok1BY7aTQ!GHFdh ztX&j_DvA-rcKUb_VHN*1IkpD<9pzd);)Y~YzzxacQhYkamX}RAapgs&jH+bj&f%fq zV-=Vd7Ad}Hp18axJaHOvWV4cCHcoW*%F;4)0N3fWn!eIMZ`$LUhtA(nE~d1@OFR5A6@dnX+3bHsQFz-t z2OZNfC`euB;`X+wlUyy~wo=*KJ_jUQmBn50~wG4F#z_!rPZx8`A2iA(GqrINjo0m z&Hr&JHhlaSy0Y3Ac}Tag~x_y{c~}Z`G;GvbRfoV!{SWs@Y^jwhi9< z*4mP_}nswFuC;1x-wO)7`oe;SxD*Acw7Ko#)nF;W>q}x z0%~f8>2~(IEnl-9>7E8SNygy6e#e$BgK&ZA!{egTq4c`g^I*xs5QfVf;+gk$XXUG- z!h^T{{Ku*-ajsx7kFCzS5MMPjX^Yh!ku~+sDL1}8BHnL!edLC|nPp9s+{I7hj55BM zip!v<&lT8`gY&}7$(dS@!Ge|Hv}L7(f*%Vqs+dw`i=6&|J2$uh77Ak|g*F@ldbFJJ zvVR#j1Ij;JuU5%FyL(4-X6KRsBnFc$Mib)&iV1WiHXdn7&saE5KhLP(9$NGRy6v#O+g)GaqoTl*=qrLwU46581P=Zx=aC zS6cVwD)(Q-N1K!Pmn*G}4n5$>CigU&2XdprJ8xVFT}Ii>@EY?jmV(WGk%N{R^YKB= zM{|aUmLCs9%8`H}W^cDGe;$t48r=jWY7}8t_|Vl zJ5pVOj|NiG-CLi%BU|h`Vxch@>}L>H@5tX|$(h-8W#W?#K+2$6b^p4B^mh6Vlwa1%N@n5IeD~Pe8h0QEJ=QtMlW~4wDOIUA zIGpuSeHr(5?3S}v;9eYvnsjw@U8L{!pWWFmEo~=2lH-EVLDlt52VE0d(1MDJ^^t-4 zzZ5*Ia=j3UPoQoiDWrCq_|(+Z+zOlD#}@adW>Mu9NX}iFkKY0{QHaC)a?k#{Ic}1g zzB5W{Z|?(Dga_YjLn)0O?vK?K-MM=tl{lv#sp-HUmJlj>zjMX}?lkuAm@tA;JT*|0 z!k-ON2GrMD zU?HD&f?_L<&EQiX%IK*bH58ThP7Mu5rng{DAH7*jQdCiX6#^84(X!SD^=ZOgp`N)D z9~Zg_^q`y!!&d495Z))ZM7kIGIkK(t{F=zFQ<82+`$(mbGg4q&Lw)X6lpx&dyeD_g z2SH)9KT~dskZAO^YkNC!TZtpd#=n3Z!lFBKWl0KH zC>d9%nODi$Mmi0u3HnV4!4ld5AM&9`kkc3+1E*oKinLzFYR)y zVt5)JHyWvVmUVjL*A9o5ZtShI-YRquHdj4*gUcR`PETw9T@tXu z;yyqYem%N-0+f)Wy|4Y#0^b!*t^PCsf9DUnLvr1dCvFC@DLLg9=WyX7=>1ozeuke; zBeZg#5ug0zIxvE_V3exsYqoFdnY|xM>k*c&EI>{LrX$fDL9&0d0Z1Oxt6 zah;KXfE+0?{oqDS=IdkWobw09@8xiAK5TcgV!2Wfub5*nA-cKG1jQyv;HlJN_`~>$ zc+G~cSPyV07oMaIScG+H$U?Yl{MzJLmM+b3vAzozpBUN-@FU0HRCE@oD!`xVy?&iT z?5XmV;vld4vO>}~x)tXL4?f&wSkpE7FcT4Dgf zoG;$Z1#oQlgMG_Et}yTA&i@T}0F!_*`7^Bpl091$tWpxR6o9c}OOH*Lz@B11?TgZj*l}!xAe5JhN ziAU$axN8d-NyfINk@cz|r)HD?(CafcTR08-L@rK7=^-KHQ1_)!18CVq2y_)f#g zJhmT(C%-836%73|*3&$TVYi6}Iy&O6R;+aboO^d`LSa*zvU%iYXE~9gvvLIfj)CZ* LjkU@(?IZpN-U^?Q literal 0 HcmV?d00001 diff --git a/test/integration/render-tests/runtime-styling/pattern-add-remove-add/style.json b/test/integration/render-tests/runtime-styling/pattern-add-remove-add/style.json new file mode 100644 index 00000000000..35610cc1bd4 --- /dev/null +++ b/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": [] +}