From b91e1a893d81c1888a34d6622f6aa649a2fc63a1 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Fri, 3 Jan 2020 18:59:24 +0100 Subject: [PATCH 01/10] Do not abort and dispose of tiles --- src/ol/Tile.js | 7 ---- src/ol/TileCache.js | 12 +----- src/ol/TileQueue.js | 13 +----- src/ol/TileState.js | 3 +- src/ol/renderer/canvas/VectorTileLayer.js | 6 +-- src/ol/source/UrlTile.js | 2 +- test/spec/ol/imagetile.test.js | 17 -------- test/spec/ol/source/tileimage.test.js | 13 ------ test/spec/ol/tilecache.test.js | 2 - test/spec/ol/tilequeue.test.js | 24 ++--------- test/spec/ol/vectorrendertile.test.js | 51 +---------------------- 11 files changed, 11 insertions(+), 139 deletions(-) diff --git a/src/ol/Tile.js b/src/ol/Tile.js index 89c7a28e54..e0a393854c 100644 --- a/src/ol/Tile.js +++ b/src/ol/Tile.js @@ -144,13 +144,6 @@ class Tile extends EventTarget { this.dispatchEvent(EventType.CHANGE); } - /** - * @inheritDoc - */ - disposeInternal() { - this.setState(TileState.ABORT); - } - /** * @return {string} Key. */ diff --git a/src/ol/TileCache.js b/src/ol/TileCache.js index e71cfd4bc3..d1527388f4 100644 --- a/src/ol/TileCache.js +++ b/src/ol/TileCache.js @@ -6,15 +6,6 @@ import {fromKey, getKey} from './tilecoord.js'; class TileCache extends LRUCache { - /** - * @param {number=} opt_highWaterMark High water mark. - */ - constructor(opt_highWaterMark) { - - super(opt_highWaterMark); - - } - /** * @param {!Object} usedTiles Used tiles. */ @@ -24,7 +15,7 @@ class TileCache extends LRUCache { if (tile.getKey() in usedTiles) { break; } else { - this.pop().dispose(); + this.pop(); } } } @@ -42,7 +33,6 @@ class TileCache extends LRUCache { this.forEach(function(tile) { if (tile.tileCoord[0] !== z) { this.remove(getKey(tile.tileCoord)); - tile.dispose(); } }.bind(this)); } diff --git a/src/ol/TileQueue.js b/src/ol/TileQueue.js index 3bf20be359..994a8eeb1d 100644 --- a/src/ol/TileQueue.js +++ b/src/ol/TileQueue.js @@ -84,8 +84,7 @@ class TileQueue extends PriorityQueue { handleTileChange(event) { const tile = /** @type {import("./Tile.js").default} */ (event.target); const state = tile.getState(); - if (tile.hifi && state === TileState.LOADED || state === TileState.ERROR || - state === TileState.EMPTY || state === TileState.ABORT) { + if (tile.hifi && state === TileState.LOADED || state === TileState.ERROR || state === TileState.EMPTY) { tile.removeEventListener(EventType.CHANGE, this.boundHandleTileChange_); const tileKey = tile.getKey(); if (tileKey in this.tilesLoadingKeys_) { @@ -102,27 +101,19 @@ class TileQueue extends PriorityQueue { */ loadMoreTiles(maxTotalLoading, maxNewLoads) { let newLoads = 0; - let abortedTiles = false; let state, tile, tileKey; while (this.tilesLoading_ < maxTotalLoading && newLoads < maxNewLoads && this.getCount() > 0) { tile = /** @type {import("./Tile.js").default} */ (this.dequeue()[0]); tileKey = tile.getKey(); state = tile.getState(); - if (state === TileState.ABORT) { - abortedTiles = true; - } else if (state === TileState.IDLE && !(tileKey in this.tilesLoadingKeys_)) { + if (state === TileState.IDLE && !(tileKey in this.tilesLoadingKeys_)) { this.tilesLoadingKeys_[tileKey] = true; ++this.tilesLoading_; ++newLoads; tile.load(); } } - if (newLoads === 0 && abortedTiles) { - // Do not stop the render loop when all wanted tiles were aborted due to - // a small, saturated tile cache. - this.tileChangeCallback_(); - } } } diff --git a/src/ol/TileState.js b/src/ol/TileState.js index f350815140..61dd890c5b 100644 --- a/src/ol/TileState.js +++ b/src/ol/TileState.js @@ -14,6 +14,5 @@ export default { * @type {number} */ ERROR: 3, - EMPTY: 4, - ABORT: 5 + EMPTY: 4 }; diff --git a/src/ol/renderer/canvas/VectorTileLayer.js b/src/ol/renderer/canvas/VectorTileLayer.js index 785f0b3d3d..834725493e 100644 --- a/src/ol/renderer/canvas/VectorTileLayer.js +++ b/src/ol/renderer/canvas/VectorTileLayer.js @@ -117,8 +117,7 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer { let render; const tileUid = getUid(tile); const state = tile.getState(); - if (((state === TileState.LOADED && tile.hifi) || - state === TileState.ERROR || state === TileState.ABORT) && + if (((state === TileState.LOADED && tile.hifi) || state === TileState.ERROR) && tileUid in this.tileListenerKeys_) { unlistenByKey(this.tileListenerKeys_[tileUid]); delete this.tileListenerKeys_[tileUid]; @@ -468,9 +467,6 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer { const clipZs = []; for (let i = tiles.length - 1; i >= 0; --i) { const tile = /** @type {import("../../VectorRenderTile.js").default} */ (tiles[i]); - if (tile.getState() == TileState.ABORT) { - continue; - } const tileCoord = tile.tileCoord; const tileExtent = tileGrid.getTileCoordExtent(tile.wrappedTileCoord); const worldOffset = tileGrid.getTileCoordExtent(tileCoord, this.tmpExtent)[0] - tileExtent[0]; diff --git a/src/ol/source/UrlTile.js b/src/ol/source/UrlTile.js index 0707b15eac..ebc3f5a8ed 100644 --- a/src/ol/source/UrlTile.js +++ b/src/ol/source/UrlTile.js @@ -139,7 +139,7 @@ class UrlTile extends TileSource { } else if (uid in this.tileLoadingKeys_) { delete this.tileLoadingKeys_[uid]; type = tileState == TileState.ERROR ? TileEventType.TILELOADERROR : - (tileState == TileState.LOADED || tileState == TileState.ABORT) ? + tileState == TileState.LOADED ? TileEventType.TILELOADEND : undefined; } if (type != undefined) { diff --git a/test/spec/ol/imagetile.test.js b/test/spec/ol/imagetile.test.js index eb35ed9f18..7925182e0a 100644 --- a/test/spec/ol/imagetile.test.js +++ b/test/spec/ol/imagetile.test.js @@ -83,21 +83,4 @@ describe('ol.ImageTile', function() { }); - describe('dispose', function() { - - it('sets image src to a blank image data uri', function() { - const tileCoord = [0, 0, 0]; - const state = TileState.IDLE; - const src = 'spec/ol/data/osm-0-0-0.png'; - const tileLoadFunction = defaultImageLoadFunction; - const tile = new ImageTile(tileCoord, state, src, null, tileLoadFunction); - tile.load(); - expect(tile.getState()).to.be(TileState.LOADING); - tile.dispose(); - expect(tile.getState()).to.be(TileState.ABORT); - expect(tile.getImage().src).to.be(ImageTile.blankImageUrl); - }); - - }); - }); diff --git a/test/spec/ol/source/tileimage.test.js b/test/spec/ol/source/tileimage.test.js index 1879db57ac..cb9b85884c 100644 --- a/test/spec/ol/source/tileimage.test.js +++ b/test/spec/ol/source/tileimage.test.js @@ -218,19 +218,6 @@ describe('ol.source.TileImage', function() { const tile = source.getTile(0, 0, 0, 1, getProjection('EPSG:3857')); tile.load(); }); - - it('dispatches tileloadend events for aborted tiles', function() { - source.setTileLoadFunction(function() {}); - const startSpy = sinon.spy(); - source.on('tileloadstart', startSpy); - const endSpy = sinon.spy(); - source.on('tileloadend', endSpy); - const tile = source.getTile(0, 0, 0, 1, getProjection('EPSG:3857')); - tile.load(); - tile.dispose(); - expect(startSpy.callCount).to.be(1); - expect(endSpy.callCount).to.be(1); - }); }); }); diff --git a/test/spec/ol/tilecache.test.js b/test/spec/ol/tilecache.test.js index c20ac068d8..ce92e0a145 100644 --- a/test/spec/ol/tilecache.test.js +++ b/test/spec/ol/tilecache.test.js @@ -32,8 +32,6 @@ describe('ol.TileCache', function() { '2/1/0', '2/0/0' ]); - - expect(tiles[0].dispose.calledOnce).to.be(true); }); }); }); diff --git a/test/spec/ol/tilequeue.test.js b/test/spec/ol/tilequeue.test.js index c92e02a34a..5136ac8e01 100644 --- a/test/spec/ol/tilequeue.test.js +++ b/test/spec/ol/tilequeue.test.js @@ -89,20 +89,6 @@ describe('ol.TileQueue', function() { }); - it('calls #tileChangeCallback_ when all wanted tiles are aborted', function() { - const tileChangeCallback = sinon.spy(); - const queue = new TileQueue(noop, tileChangeCallback); - const numTiles = 20; - for (let i = 0; i < numTiles; ++i) { - const tile = createImageTile(); - tile.state = TileState.ABORT; - queue.enqueue([tile]); - } - const maxLoading = numTiles / 2; - queue.loadMoreTiles(maxLoading, maxLoading); - expect(tileChangeCallback.callCount).to.be(1); - }); - }); describe('heapify', function() { @@ -144,7 +130,7 @@ describe('ol.TileQueue', function() { describe('tile change event', function() { const noop = function() {}; - it('abort queued tiles', function() { + it('loaded tiles', function() { const tq = new TileQueue(noop, noop); const tile = createImageTile(); expect(tile.hasListener('change')).to.be(false); @@ -152,12 +138,11 @@ describe('ol.TileQueue', function() { tq.enqueue([tile]); expect(tile.hasListener('change')).to.be(true); - tile.dispose(); + tile.setState(TileState.LOADED); expect(tile.hasListener('change')).to.be(false); - expect(tile.getState()).to.eql(5); // ABORT }); - it('abort loading tiles', function() { + it('error tiles', function() { const tq = new TileQueue(noop, noop); const tile = createImageTile(noop); @@ -166,10 +151,9 @@ describe('ol.TileQueue', function() { expect(tq.getTilesLoading()).to.eql(1); expect(tile.getState()).to.eql(1); // LOADING - tile.dispose(); + tile.setState(TileState.ERROR); expect(tq.getTilesLoading()).to.eql(0); expect(tile.hasListener('change')).to.be(false); - expect(tile.getState()).to.eql(5); // ABORT }); diff --git a/test/spec/ol/vectorrendertile.test.js b/test/spec/ol/vectorrendertile.test.js index 783d9acf2d..355432be34 100644 --- a/test/spec/ol/vectorrendertile.test.js +++ b/test/spec/ol/vectorrendertile.test.js @@ -1,9 +1,8 @@ import TileState from '../../../src/ol/TileState.js'; import {defaultLoadFunction} from '../../../src/ol/source/VectorTile.js'; import VectorTileSource from '../../../src/ol/source/VectorTile.js'; -import {listen, listenOnce, unlistenByKey} from '../../../src/ol/events.js'; +import {listen, unlistenByKey} from '../../../src/ol/events.js'; import GeoJSON from '../../../src/ol/format/GeoJSON.js'; -import {createXYZ} from '../../../src/ol/tilegrid.js'; import TileGrid from '../../../src/ol/tilegrid/TileGrid.js'; import EventType from '../../../src/ol/events/EventType.js'; @@ -95,52 +94,4 @@ describe('ol.VectorRenderTile', function() { }); }); - it('#dispose() while loading', function() { - const source = new VectorTileSource({ - format: new GeoJSON(), - url: 'spec/ol/data/point.json', - tileGrid: createXYZ() - }); - source.getTileGridForProjection = function() { - return createXYZ({tileSize: 512}); - }; - const tile = source.getTile(0, 0, 0, 1, source.getProjection()); - - tile.load(); - expect(tile.getState()).to.be(TileState.LOADING); - tile.dispose(); - expect(source.sourceTilesByTileKey_[tile.getKey()]).to.be(undefined); - expect(tile.getState()).to.be(TileState.ABORT); - }); - - it('#dispose() when source tiles are loaded', function(done) { - const source = new VectorTileSource({ - format: new GeoJSON(), - url: 'spec/ol/data/point.json?{z}/{x}/{y}', - tileGrid: createXYZ() - }); - source.getTileGridForProjection = function() { - return createXYZ({tileSize: 512}); - }; - const tile = source.getTile(0, 0, 0, 1, source.getProjection()); - - tile.load(); - listenOnce(tile, 'change', function() { - expect(tile.getState()).to.be(TileState.LOADED); - expect(tile.loadingSourceTiles).to.be(0); - const sourceTiles = source.getSourceTiles(1, source.getProjection(), tile); - expect(sourceTiles.length).to.be(4); - for (let i = 0, ii = sourceTiles.length; i < ii; ++i) { - expect(sourceTiles[i].consumers).to.be(1); - } - tile.dispose(); - expect(tile.getState()).to.be(TileState.ABORT); - for (let i = 0, ii = sourceTiles.length; i < ii; ++i) { - expect(sourceTiles[i].consumers).to.be(0); - expect(sourceTiles[i].getState()).to.be(TileState.ABORT); - } - done(); - }); - }); - }); From ae336f0a1bbbe36d0cc3fa2e15f7b30540812c63 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 5 Jan 2020 00:13:05 +0100 Subject: [PATCH 02/10] Remove disposeInternal of ImageTile and reproj/Tile --- src/ol/ImageTile.js | 14 -------------- src/ol/reproj/Tile.js | 10 ---------- 2 files changed, 24 deletions(-) diff --git a/src/ol/ImageTile.js b/src/ol/ImageTile.js index 995f32657e..3e0b95253a 100644 --- a/src/ol/ImageTile.js +++ b/src/ol/ImageTile.js @@ -58,20 +58,6 @@ class ImageTile extends Tile { } - /** - * @inheritDoc - */ - disposeInternal() { - if (this.state == TileState.LOADING) { - this.unlistenImage_(); - this.image_ = getBlankImage(); - } - if (this.interimTile) { - this.interimTile.dispose(); - } - super.disposeInternal(); - } - /** * Get the HTML image element for this tile (may be a Canvas, Image, or Video). * @return {HTMLCanvasElement|HTMLImageElement|HTMLVideoElement} Image. diff --git a/src/ol/reproj/Tile.js b/src/ol/reproj/Tile.js index 909b125e7e..73d508d203 100644 --- a/src/ol/reproj/Tile.js +++ b/src/ol/reproj/Tile.js @@ -203,16 +203,6 @@ class ReprojTile extends Tile { } } - /** - * @inheritDoc - */ - disposeInternal() { - if (this.state == TileState.LOADING) { - this.unlistenSources_(); - } - super.disposeInternal(); - } - /** * Get the HTML Canvas element for this tile. * @return {HTMLCanvasElement} Canvas. From da6eed850ccddfefa8be0c3571b83368b7c3d955 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 5 Jan 2020 11:24:46 +0100 Subject: [PATCH 03/10] Do not lock label cache entries --- src/ol/VectorRenderTile.js | 6 --- src/ol/render/canvas/Executor.js | 13 +---- src/ol/render/canvas/ExecutorGroup.js | 22 +------- src/ol/render/canvas/LabelCache.js | 53 +------------------ src/ol/render/canvas/TextBuilder.js | 3 +- src/ol/renderer/canvas/VectorLayer.js | 3 -- src/ol/renderer/canvas/VectorTileLayer.js | 6 --- test/spec/ol/render/canvas/labelcache.test.js | 18 +------ 8 files changed, 7 insertions(+), 117 deletions(-) diff --git a/src/ol/VectorRenderTile.js b/src/ol/VectorRenderTile.js index a22a38991a..1bff7afcc3 100644 --- a/src/ol/VectorRenderTile.js +++ b/src/ol/VectorRenderTile.js @@ -127,12 +127,6 @@ class VectorRenderTile extends Tile { canvas.width = 0; canvas.height = 0; } - for (const key in this.executorGroups) { - const executorGroups = this.executorGroups[key]; - for (let i = 0, ii = executorGroups.length; i < ii; ++i) { - executorGroups[i].disposeInternal(); - } - } super.disposeInternal(); } diff --git a/src/ol/render/canvas/Executor.js b/src/ol/render/canvas/Executor.js index ebe8946f57..5ec354c5b0 100644 --- a/src/ol/render/canvas/Executor.js +++ b/src/ol/render/canvas/Executor.js @@ -18,7 +18,6 @@ import { } from '../../transform.js'; import {createCanvasContext2D} from '../../dom.js'; import {labelCache, defaultTextAlign, measureTextHeight, measureAndCacheTextWidth, measureTextWidths} from '../canvas.js'; -import Disposable from '../../Disposable.js'; import RBush from 'rbush/rbush.js'; @@ -52,7 +51,7 @@ const p3 = []; const p4 = []; -class Executor extends Disposable { +class Executor { /** * @param {number} resolution Resolution. * @param {number} pixelRatio Pixel ratio. @@ -60,7 +59,6 @@ class Executor extends Disposable { * @param {SerializableInstructions} instructions The serializable instructions */ constructor(resolution, pixelRatio, overlaps, instructions) { - super(); /** * @protected @@ -156,15 +154,6 @@ class Executor extends Disposable { this.widths_ = {}; } - /** - * @inheritDoc - */ - disposeInternal() { - labelCache.release(this); - super.disposeInternal(); - } - - /** * @param {string} text Text. * @param {string} textKey Text style key. diff --git a/src/ol/render/canvas/ExecutorGroup.js b/src/ol/render/canvas/ExecutorGroup.js index 852179f346..871456ba62 100644 --- a/src/ol/render/canvas/ExecutorGroup.js +++ b/src/ol/render/canvas/ExecutorGroup.js @@ -10,7 +10,6 @@ import {isEmpty} from '../../obj.js'; import BuilderType from './BuilderType.js'; import {create as createTransform, compose as composeTransform} from '../../transform.js'; import Executor from './Executor.js'; -import Disposable from '../../Disposable.js'; /** * @const @@ -26,7 +25,7 @@ const ORDER = [ ]; -class ExecutorGroup extends Disposable { +class ExecutorGroup { /** * @param {import("../../extent.js").Extent} maxExtent Max extent for clipping. When a * `maxExtent` was set on the Buillder for this executor group, the same `maxExtent` @@ -40,7 +39,6 @@ class ExecutorGroup extends Disposable { * @param {number=} opt_renderBuffer Optional rendering buffer. */ constructor(maxExtent, resolution, pixelRatio, overlaps, allInstructions, opt_renderBuffer) { - super(); /** * @private @@ -128,24 +126,6 @@ class ExecutorGroup extends Disposable { } } - /** - * @inheritDoc - */ - disposeInternal() { - for (const z in this.executorsByZIndex_) { - const executors = this.executorsByZIndex_[z]; - for (const key in executors) { - executors[key].disposeInternal(); - } - } - if (this.hitDetectionContext_) { - const canvas = this.hitDetectionContext_.canvas; - canvas.width = 0; - canvas.height = 0; - } - - super.disposeInternal(); - } /** * @param {Array} executors Executors. diff --git a/src/ol/render/canvas/LabelCache.js b/src/ol/render/canvas/LabelCache.js index dfa800428e..37de689738 100644 --- a/src/ol/render/canvas/LabelCache.js +++ b/src/ol/render/canvas/LabelCache.js @@ -1,4 +1,3 @@ -import {getUid} from '../../util.js'; import LRUCache from '../../structs/LRUCache.js'; /** @@ -11,59 +10,11 @@ import LRUCache from '../../structs/LRUCache.js'; */ class LabelCache extends LRUCache { - /** - * @inheritDoc - */ - constructor(opt_highWaterMark) { - super(opt_highWaterMark); - this.consumers = {}; - } - - clear() { - this.consumers = {}; - super.clear(); - } - - /** - * @override - * @param {string} key Label key. - * @param {import("./Executor.js").default} consumer Label consumer. - * @return {HTMLCanvasElement} Label. - */ - get(key, consumer) { - const canvas = super.get(key); - const consumerId = getUid(consumer); - if (!(consumerId in this.consumers)) { - this.consumers[consumerId] = {}; - } - this.consumers[consumerId][key] = true; - return canvas; - } - - prune() { - outer: + expireCache() { while (this.canExpireCache()) { - const key = this.peekLastKey(); - for (const consumerId in this.consumers) { - if (key in this.consumers[consumerId]) { - break outer; - } - } - const canvas = this.pop(); - canvas.width = 0; - canvas.height = 0; - for (const consumerId in this.consumers) { - delete this.consumers[consumerId][key]; - } + this.pop(); } } - - /** - * @param {import("./Executor.js").default} consumer Label consumer. - */ - release(consumer) { - delete this.consumers[getUid(consumer)]; - } } export default LabelCache; diff --git a/src/ol/render/canvas/TextBuilder.js b/src/ol/render/canvas/TextBuilder.js index 40c760256e..641801dba5 100644 --- a/src/ol/render/canvas/TextBuilder.js +++ b/src/ol/render/canvas/TextBuilder.js @@ -131,8 +131,6 @@ class CanvasTextBuilder extends CanvasBuilder { * @type {string} */ this.strokeKey_ = ''; - - labelCache.prune(); } /** @@ -140,6 +138,7 @@ class CanvasTextBuilder extends CanvasBuilder { */ finish() { const instructions = super.finish(); + labelCache.expireCache(); instructions.textStates = this.textStates; instructions.fillStates = this.fillStates; instructions.strokeStates = this.strokeStates; diff --git a/src/ol/renderer/canvas/VectorLayer.js b/src/ol/renderer/canvas/VectorLayer.js index b451a01b88..8fd84962ac 100644 --- a/src/ol/renderer/canvas/VectorLayer.js +++ b/src/ol/renderer/canvas/VectorLayer.js @@ -384,9 +384,6 @@ class CanvasVectorLayerRenderer extends CanvasLayerRenderer { return true; } - if (this.replayGroup_) { - this.replayGroup_.dispose(); - } this.replayGroup_ = null; this.dirty_ = false; diff --git a/src/ol/renderer/canvas/VectorTileLayer.js b/src/ol/renderer/canvas/VectorTileLayer.js index 834725493e..9c0974fa91 100644 --- a/src/ol/renderer/canvas/VectorTileLayer.js +++ b/src/ol/renderer/canvas/VectorTileLayer.js @@ -218,12 +218,6 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer { const sourceTiles = source.getSourceTiles(pixelRatio, projection, tile); const layerUid = getUid(layer); - const executorGroups = tile.executorGroups[layerUid]; - if (executorGroups) { - for (let i = 0, ii = executorGroups.length; i < ii; ++i) { - executorGroups[i].dispose(); - } - } delete tile.hitDetectionImageData[layerUid]; tile.executorGroups[layerUid] = []; for (let t = 0, tt = sourceTiles.length; t < tt; ++t) { diff --git a/test/spec/ol/render/canvas/labelcache.test.js b/test/spec/ol/render/canvas/labelcache.test.js index fa2fcd8f79..d6c5fb8531 100644 --- a/test/spec/ol/render/canvas/labelcache.test.js +++ b/test/spec/ol/render/canvas/labelcache.test.js @@ -2,25 +2,11 @@ import LabelCache from '../../../../../src/ol/render/canvas/LabelCache.js'; describe('ol.render.canvas.LabelCache', function() { - it('#prune()', function() { + it('#expireCache()', function() { const labelCache = new LabelCache(1); labelCache.set('key1', document.createElement('canvas')); labelCache.set('key2', document.createElement('canvas')); - labelCache.prune(); - expect(labelCache.getCount()).to.be(1); - }); - - it('#prune() leaves used labels untouched until consumer is released', function() { - const labelCache = new LabelCache(1); - labelCache.set('key1', document.createElement('canvas')); - labelCache.set('key2', document.createElement('canvas')); - const consumer = {}; - labelCache.get('key1', consumer); - labelCache.get('key2', consumer); - labelCache.prune(); - expect(labelCache.getCount()).to.be(2); - labelCache.release(consumer); - labelCache.prune(); + labelCache.expireCache(); expect(labelCache.getCount()).to.be(1); }); From 6affeb0beb754f733f8dde6373f3e8dcc40baa34 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 5 Jan 2020 11:52:45 +0100 Subject: [PATCH 04/10] Do not dispose VectorRenderTiles --- src/ol/VectorRenderTile.js | 30 +----------------------------- src/ol/source/VectorTile.js | 12 ++++-------- 2 files changed, 5 insertions(+), 37 deletions(-) diff --git a/src/ol/VectorRenderTile.js b/src/ol/VectorRenderTile.js index 1bff7afcc3..7425990573 100644 --- a/src/ol/VectorRenderTile.js +++ b/src/ol/VectorRenderTile.js @@ -4,7 +4,6 @@ import {getUid} from './util.js'; import Tile from './Tile.js'; import {createCanvasContext2D} from './dom.js'; -import {unlistenByKey} from './events.js'; /** @@ -29,10 +28,8 @@ class VectorRenderTile extends Tile { * @param {import("./tilegrid/TileGrid.js").default} sourceTileGrid Tile grid of the source. * @param {function(VectorRenderTile):Array} getSourceTiles Function * to get an source tiles for this tile. - * @param {function(VectorRenderTile):void} removeSourceTiles Function to remove this tile from its - * source tiles's consumer count. */ - constructor(tileCoord, state, urlTileCoord, sourceTileGrid, getSourceTiles, removeSourceTiles) { + constructor(tileCoord, state, urlTileCoord, sourceTileGrid, getSourceTiles) { super(tileCoord, state, {transition: 0}); @@ -81,22 +78,12 @@ class VectorRenderTile extends Tile { */ this.getSourceTiles = getSourceTiles.bind(this, this); - /** - * @type {!function(import("./VectorRenderTile.js").default):void} - */ - this.removeSourceTiles_ = removeSourceTiles; - /** * @private * @type {import("./tilegrid/TileGrid.js").default} */ this.sourceTileGrid_ = sourceTileGrid; - /** - * @type {Array} - */ - this.sourceTileListenerKeys = []; - /** * z of the source tiles of the last getSourceTiles call. * @type {number} @@ -115,21 +102,6 @@ class VectorRenderTile extends Tile { this.wrappedTileCoord = urlTileCoord; } - /** - * @inheritDoc - */ - disposeInternal() { - this.sourceTileListenerKeys.forEach(unlistenByKey); - this.sourceTileListenerKeys.length = 0; - this.removeSourceTiles_(this); - for (const key in this.context_) { - const canvas = this.context_[key].canvas; - canvas.width = 0; - canvas.height = 0; - } - super.disposeInternal(); - } - /** * @param {import("./layer/Layer.js").default} layer Layer. * @return {CanvasRenderingContext2D} The rendering context. diff --git a/src/ol/source/VectorTile.js b/src/ol/source/VectorTile.js index bd430a81d4..9cb28efb14 100644 --- a/src/ol/source/VectorTile.js +++ b/src/ol/source/VectorTile.js @@ -12,8 +12,7 @@ import {createXYZ, extentFromProjection, createForProjection} from '../tilegrid. import {buffer as bufferExtent, getIntersection, intersects} from '../extent.js'; import EventType from '../events/EventType.js'; import {loadFeaturesXhr} from '../featureloader.js'; -import {equals, remove} from '../array.js'; -import {listen, unlistenByKey} from '../events.js'; +import {equals} from '../array.js'; /** * @typedef {Object} Options @@ -294,13 +293,12 @@ class VectorTile extends UrlTile { } if (sourceTile.getState() !== TileState.EMPTY && tile.getState() === TileState.IDLE) { tile.loadingSourceTiles++; - const key = listen(sourceTile, EventType.CHANGE, function() { + sourceTile.addEventListener(EventType.CHANGE, function listenChange() { const state = sourceTile.getState(); const sourceTileKey = sourceTile.getKey(); if (state === TileState.LOADED || state === TileState.ERROR) { if (state === TileState.LOADED) { - remove(tile.sourceTileListenerKeys, key); - unlistenByKey(key); + sourceTile.removeEventListener(EventType.CHANGE, listenChange); tile.loadingSourceTiles--; delete tile.errorSourceTileKeys[sourceTileKey]; } else if (state === TileState.ERROR) { @@ -314,7 +312,6 @@ class VectorTile extends UrlTile { } } }); - tile.sourceTileListenerKeys.push(key); } }.bind(this)); if (!covered) { @@ -411,8 +408,7 @@ class VectorTile extends UrlTile { empty ? TileState.EMPTY : TileState.IDLE, urlTileCoord, this.tileGrid, - this.getSourceTiles.bind(this, pixelRatio, projection), - this.removeSourceTiles.bind(this)); + this.getSourceTiles.bind(this, pixelRatio, projection)); newTile.key = key; if (tile) { From bec747e513dca160b04c409ca5af95e1a6cd1024 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 5 Jan 2020 11:54:01 +0100 Subject: [PATCH 05/10] Remove unused argument and member --- src/ol/VectorRenderTile.js | 9 +-------- src/ol/source/VectorTile.js | 1 - test/spec/ol/renderer/canvas/vectortilelayer.test.js | 5 ++--- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/ol/VectorRenderTile.js b/src/ol/VectorRenderTile.js index 7425990573..0384fc3566 100644 --- a/src/ol/VectorRenderTile.js +++ b/src/ol/VectorRenderTile.js @@ -25,11 +25,10 @@ class VectorRenderTile extends Tile { * @param {import("./tilecoord.js").TileCoord} tileCoord Tile coordinate. * @param {import("./TileState.js").default} state State. * @param {import("./tilecoord.js").TileCoord} urlTileCoord Wrapped tile coordinate for source urls. - * @param {import("./tilegrid/TileGrid.js").default} sourceTileGrid Tile grid of the source. * @param {function(VectorRenderTile):Array} getSourceTiles Function * to get an source tiles for this tile. */ - constructor(tileCoord, state, urlTileCoord, sourceTileGrid, getSourceTiles) { + constructor(tileCoord, state, urlTileCoord, getSourceTiles) { super(tileCoord, state, {transition: 0}); @@ -78,12 +77,6 @@ class VectorRenderTile extends Tile { */ this.getSourceTiles = getSourceTiles.bind(this, this); - /** - * @private - * @type {import("./tilegrid/TileGrid.js").default} - */ - this.sourceTileGrid_ = sourceTileGrid; - /** * z of the source tiles of the last getSourceTiles call. * @type {number} diff --git a/src/ol/source/VectorTile.js b/src/ol/source/VectorTile.js index 9cb28efb14..abd76c21db 100644 --- a/src/ol/source/VectorTile.js +++ b/src/ol/source/VectorTile.js @@ -407,7 +407,6 @@ class VectorTile extends UrlTile { tileCoord, empty ? TileState.EMPTY : TileState.IDLE, urlTileCoord, - this.tileGrid, this.getSourceTiles.bind(this, pixelRatio, projection)); newTile.key = key; diff --git a/test/spec/ol/renderer/canvas/vectortilelayer.test.js b/test/spec/ol/renderer/canvas/vectortilelayer.test.js index eb3193223b..570a693f6d 100644 --- a/test/spec/ol/renderer/canvas/vectortilelayer.test.js +++ b/test/spec/ol/renderer/canvas/vectortilelayer.test.js @@ -255,11 +255,10 @@ describe('ol.renderer.canvas.VectorTileLayer', function() { sourceTile.getImage = function() { return document.createElement('canvas'); }; - const tile = new VectorRenderTile([0, 0, 0], 1, [0, 0, 0], createXYZ(), + const tile = new VectorRenderTile([0, 0, 0], 1, [0, 0, 0], function() { return sourceTile; - }, - function() {}); + }); tile.transition_ = 0; tile.setState(TileState.LOADED); layer.getSource().getTile = function() { From ae1ee192f3114c9c664731743cb8f36c742fbd96 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 5 Jan 2020 12:09:07 +0100 Subject: [PATCH 06/10] Avoid misleading bind argument --- src/ol/VectorRenderTile.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ol/VectorRenderTile.js b/src/ol/VectorRenderTile.js index 0384fc3566..bb2bd38f5d 100644 --- a/src/ol/VectorRenderTile.js +++ b/src/ol/VectorRenderTile.js @@ -26,7 +26,7 @@ class VectorRenderTile extends Tile { * @param {import("./TileState.js").default} state State. * @param {import("./tilecoord.js").TileCoord} urlTileCoord Wrapped tile coordinate for source urls. * @param {function(VectorRenderTile):Array} getSourceTiles Function - * to get an source tiles for this tile. + * to get source tiles for this tile. */ constructor(tileCoord, state, urlTileCoord, getSourceTiles) { @@ -75,7 +75,7 @@ class VectorRenderTile extends Tile { /** * @type {!function():Array} */ - this.getSourceTiles = getSourceTiles.bind(this, this); + this.getSourceTiles = getSourceTiles.bind(undefined, this); /** * z of the source tiles of the last getSourceTiles call. From 2875685b3c350f15f0e11eb37d196c82fe0b6809 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 5 Jan 2020 12:25:44 +0100 Subject: [PATCH 07/10] Use TileCache instead of custom structure --- src/ol/VectorTile.js | 5 --- src/ol/source/VectorTile.js | 31 ++++++------------- .../renderer/canvas/vectortilelayer.test.js | 2 +- 3 files changed, 10 insertions(+), 28 deletions(-) diff --git a/src/ol/VectorTile.js b/src/ol/VectorTile.js index 7c79cbc36f..ecf13ed8ea 100644 --- a/src/ol/VectorTile.js +++ b/src/ol/VectorTile.js @@ -18,11 +18,6 @@ class VectorTile extends Tile { super(tileCoord, state, opt_options); - /** - * @type {number} - */ - this.consumers = 0; - /** * Extent of this tile; set by the source. * @type {import("./extent.js").Extent} diff --git a/src/ol/source/VectorTile.js b/src/ol/source/VectorTile.js index abd76c21db..e2ec40c495 100644 --- a/src/ol/source/VectorTile.js +++ b/src/ol/source/VectorTile.js @@ -13,6 +13,7 @@ import {buffer as bufferExtent, getIntersection, intersects} from '../extent.js' import EventType from '../events/EventType.js'; import {loadFeaturesXhr} from '../featureloader.js'; import {equals} from '../array.js'; +import TileCache from '../TileCache.js'; /** * @typedef {Object} Options @@ -139,9 +140,9 @@ class VectorTile extends UrlTile { /** * @private - * @type {Object} + * @type {TileCache} */ - this.sourceTileByKey_ = {}; + this.sourceTileCache = new TileCache(this.tileCache.highWaterMark); /** * @private @@ -227,7 +228,7 @@ class VectorTile extends UrlTile { */ clear() { this.tileCache.clear(); - this.sourceTileByKey_ = {}; + this.sourceTileCache.clear(); this.sourceTilesByTileKey_ = {}; } @@ -269,8 +270,8 @@ class VectorTile extends UrlTile { const tileUrl = this.tileUrlFunction(sourceTileCoord, pixelRatio, projection); let sourceTile; if (tileUrl !== undefined) { - if (tileUrl in this.sourceTileByKey_) { - sourceTile = this.sourceTileByKey_[tileUrl]; + if (this.sourceTileCache.containsKey(tileUrl)) { + sourceTile = this.sourceTileCache.get(tileUrl); const state = sourceTile.getState(); if (state === TileState.LOADED || state === TileState.ERROR || state === TileState.EMPTY) { sourceTiles.push(sourceTile); @@ -282,7 +283,7 @@ class VectorTile extends UrlTile { sourceTile.extent = sourceTileGrid.getTileCoordExtent(sourceTileCoord); sourceTile.projection = projection; sourceTile.resolution = sourceTileGrid.getResolution(sourceTileCoord[0]); - this.sourceTileByKey_[tileUrl] = sourceTile; + this.sourceTileCache.set(tileUrl, sourceTile); sourceTile.addEventListener(EventType.CHANGE, this.handleTileChange.bind(this)); sourceTile.load(); } @@ -333,6 +334,7 @@ class VectorTile extends UrlTile { this.addSourceTiles(tile, sourceTiles); } } + this.sourceTileCache.expireCache({}); return sourceTiles; } @@ -342,28 +344,13 @@ class VectorTile extends UrlTile { */ addSourceTiles(tile, sourceTiles) { this.sourceTilesByTileKey_[tile.getKey()] = sourceTiles; - for (let i = 0, ii = sourceTiles.length; i < ii; ++i) { - sourceTiles[i].consumers++; - } } /** * @param {VectorRenderTile} tile Tile. */ removeSourceTiles(tile) { - const tileKey = tile.getKey(); - if (tileKey in this.sourceTilesByTileKey_) { - const sourceTiles = this.sourceTilesByTileKey_[tileKey]; - for (let i = 0, ii = sourceTiles.length; i < ii; ++i) { - const sourceTile = sourceTiles[i]; - sourceTile.consumers--; - if (sourceTile.consumers === 0) { - sourceTile.dispose(); - delete this.sourceTileByKey_[sourceTile.getKey()]; - } - } - } - delete this.sourceTilesByTileKey_[tileKey]; + delete this.sourceTilesByTileKey_[tile.getKey()]; } /** diff --git a/test/spec/ol/renderer/canvas/vectortilelayer.test.js b/test/spec/ol/renderer/canvas/vectortilelayer.test.js index 570a693f6d..aaeb91d0ca 100644 --- a/test/spec/ol/renderer/canvas/vectortilelayer.test.js +++ b/test/spec/ol/renderer/canvas/vectortilelayer.test.js @@ -315,7 +315,7 @@ describe('ol.renderer.canvas.VectorTileLayer', function() { tileClass: TileClass, tileGrid: createXYZ() }); - source.sourceTileByKey_[sourceTile.getKey()] = sourceTile; + source.sourceTileCache.set('0/0/0.mvt', sourceTile); source.sourceTilesByTileKey_[sourceTile.getKey()] = [sourceTile]; executorGroup = {}; source.getTile = function() { From 46d98201c3898fcb04a2e6ddb89ed45beb4abbfe Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 5 Jan 2020 21:07:38 +0100 Subject: [PATCH 08/10] Store source tiles on render tile instead of source --- src/ol/VectorRenderTile.js | 5 ++++ src/ol/source/VectorTile.js | 27 ++----------------- .../renderer/canvas/vectortilelayer.test.js | 2 +- 3 files changed, 8 insertions(+), 26 deletions(-) diff --git a/src/ol/VectorRenderTile.js b/src/ol/VectorRenderTile.js index bb2bd38f5d..1b05c5659b 100644 --- a/src/ol/VectorRenderTile.js +++ b/src/ol/VectorRenderTile.js @@ -67,6 +67,11 @@ class VectorRenderTile extends Tile { */ this.replayState_ = {}; + /** + * @type {Array} + */ + this.sourceTiles = null; + /** * @type {number} */ diff --git a/src/ol/source/VectorTile.js b/src/ol/source/VectorTile.js index e2ec40c495..c4ac79a917 100644 --- a/src/ol/source/VectorTile.js +++ b/src/ol/source/VectorTile.js @@ -144,12 +144,6 @@ class VectorTile extends UrlTile { */ this.sourceTileCache = new TileCache(this.tileCache.highWaterMark); - /** - * @private - * @type {Object>} - */ - this.sourceTilesByTileKey_ = {}; - /** * @private * @type {boolean} @@ -229,7 +223,6 @@ class VectorTile extends UrlTile { clear() { this.tileCache.clear(); this.sourceTileCache.clear(); - this.sourceTilesByTileKey_ = {}; } /** @@ -254,7 +247,7 @@ class VectorTile extends UrlTile { const sourceZ = sourceTileGrid.getZForResolution(resolution, 1); const minZoom = sourceTileGrid.getMinZoom(); - const previousSourceTiles = this.sourceTilesByTileKey_[tile.getKey()]; + const previousSourceTiles = tile.sourceTiles; let sourceTiles, covered, loadedZ; if (previousSourceTiles && previousSourceTiles.length > 0 && previousSourceTiles[0].tileCoord[0] === sourceZ) { sourceTiles = previousSourceTiles; @@ -330,29 +323,13 @@ class VectorTile extends UrlTile { if (tile.getState() < TileState.LOADED) { tile.setState(TileState.LOADED); } else if (!previousSourceTiles || !equals(sourceTiles, previousSourceTiles)) { - this.removeSourceTiles(tile); - this.addSourceTiles(tile, sourceTiles); + tile.sourceTiles = sourceTiles; } } this.sourceTileCache.expireCache({}); return sourceTiles; } - /** - * @param {VectorRenderTile} tile Tile. - * @param {Array} sourceTiles Source tiles. - */ - addSourceTiles(tile, sourceTiles) { - this.sourceTilesByTileKey_[tile.getKey()] = sourceTiles; - } - - /** - * @param {VectorRenderTile} tile Tile. - */ - removeSourceTiles(tile) { - delete this.sourceTilesByTileKey_[tile.getKey()]; - } - /** * @inheritDoc */ diff --git a/test/spec/ol/renderer/canvas/vectortilelayer.test.js b/test/spec/ol/renderer/canvas/vectortilelayer.test.js index aaeb91d0ca..27abdf7871 100644 --- a/test/spec/ol/renderer/canvas/vectortilelayer.test.js +++ b/test/spec/ol/renderer/canvas/vectortilelayer.test.js @@ -316,10 +316,10 @@ describe('ol.renderer.canvas.VectorTileLayer', function() { tileGrid: createXYZ() }); source.sourceTileCache.set('0/0/0.mvt', sourceTile); - source.sourceTilesByTileKey_[sourceTile.getKey()] = [sourceTile]; executorGroup = {}; source.getTile = function() { const tile = VectorTileSource.prototype.getTile.apply(source, arguments); + tile.sourceTiles = [sourceTile]; tile.executorGroups[getUid(layer)] = [executorGroup]; return tile; }; From 5a8df1d4e298b901b0b7b6ad5d4ca0a2c864582c Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Sun, 5 Jan 2020 22:30:54 +0100 Subject: [PATCH 09/10] We no longer need to increase the cache size --- src/ol/renderer/canvas/TileLayer.js | 22 ------------------- .../spec/ol/renderer/canvas/tilelayer.test.js | 9 -------- 2 files changed, 31 deletions(-) diff --git a/src/ol/renderer/canvas/TileLayer.js b/src/ol/renderer/canvas/TileLayer.js index 518a77a610..51fb12939f 100644 --- a/src/ol/renderer/canvas/TileLayer.js +++ b/src/ol/renderer/canvas/TileLayer.js @@ -362,7 +362,6 @@ class CanvasTileLayerRenderer extends CanvasLayerRenderer { this.manageTilePyramid(frameState, tileSource, tileGrid, pixelRatio, projection, extent, z, tileLayer.getPreload()); - this.updateCacheSize_(frameState, tileSource); this.scheduleExpireCache(frameState, tileSource); this.postRender(context, frameState); @@ -474,27 +473,6 @@ class CanvasTileLayerRenderer extends CanvasLayerRenderer { usedTiles[tileSourceKey][tile.getKey()] = true; } - /** - * Check if the cache is big enough, and increase its size if necessary. - * @param {import("../../PluggableMap.js").FrameState} frameState Frame state. - * @param {import("../../source/Tile.js").default} tileSource Tile source. - * @private - */ - updateCacheSize_(frameState, tileSource) { - const tileSourceKey = getUid(tileSource); - let size = 0; - if (tileSourceKey in frameState.usedTiles) { - size += Object.keys(frameState.usedTiles[tileSourceKey]).length; - } - if (tileSourceKey in frameState.wantedTiles) { - size += Object.keys(frameState.wantedTiles[tileSourceKey]).length; - } - const tileCache = tileSource.tileCache; - if (tileCache.highWaterMark < size) { - tileCache.highWaterMark = size; - } - } - /** * Manage tile pyramid. * This function performs a number of functions related to the tiles at the diff --git a/test/spec/ol/renderer/canvas/tilelayer.test.js b/test/spec/ol/renderer/canvas/tilelayer.test.js index d326d9d7d0..bf5a6c784e 100644 --- a/test/spec/ol/renderer/canvas/tilelayer.test.js +++ b/test/spec/ol/renderer/canvas/tilelayer.test.js @@ -78,15 +78,6 @@ describe('ol.renderer.canvas.TileLayer', function() { disposeMap(map); }); - it('increases the cache size if necessary', function(done) { - const tileCache = layer.getSource().tileCache; - expect(tileCache.highWaterMark).to.be(1); - map.once('rendercomplete', function() { - expect(tileCache.highWaterMark).to.be(2); - done(); - }); - }); - it('respects the source\'s zDirection setting', function(done) { layer.getSource().zDirection = 1; map.getView().setZoom(5.8); // would lead to z6 tile request with the default zDirection From 9f4dbd3c350edb976ef6907e945af3299e2dcb2c Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Wed, 8 Jan 2020 10:42:34 +0100 Subject: [PATCH 10/10] Reuse existing canvases from vector render tiles --- src/ol/Tile.js | 6 ++++++ src/ol/TileCache.js | 3 ++- src/ol/VectorRenderTile.js | 16 +++++++++++++++- src/ol/dom.js | 6 ++++-- 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/ol/Tile.js b/src/ol/Tile.js index e0a393854c..f49c97e616 100644 --- a/src/ol/Tile.js +++ b/src/ol/Tile.js @@ -144,6 +144,12 @@ class Tile extends EventTarget { this.dispatchEvent(EventType.CHANGE); } + /** + * Called by the tile cache when the tile is removed from the cache due to expiry + */ + release() { + } + /** * @return {string} Key. */ diff --git a/src/ol/TileCache.js b/src/ol/TileCache.js index d1527388f4..37f6113df7 100644 --- a/src/ol/TileCache.js +++ b/src/ol/TileCache.js @@ -15,7 +15,7 @@ class TileCache extends LRUCache { if (tile.getKey() in usedTiles) { break; } else { - this.pop(); + this.pop().release(); } } } @@ -33,6 +33,7 @@ class TileCache extends LRUCache { this.forEach(function(tile) { if (tile.tileCoord[0] !== z) { this.remove(getKey(tile.tileCoord)); + tile.release(); } }.bind(this)); } diff --git a/src/ol/VectorRenderTile.js b/src/ol/VectorRenderTile.js index 1b05c5659b..372fc8a3a0 100644 --- a/src/ol/VectorRenderTile.js +++ b/src/ol/VectorRenderTile.js @@ -18,6 +18,10 @@ import {createCanvasContext2D} from './dom.js'; * @property {number} renderedTileZ */ +/** + * @type {Array} + */ +const canvasPool = []; class VectorRenderTile extends Tile { @@ -107,7 +111,7 @@ class VectorRenderTile extends Tile { getContext(layer) { const key = getUid(layer); if (!(key in this.context_)) { - this.context_[key] = createCanvasContext2D(); + this.context_[key] = createCanvasContext2D(1, 1, canvasPool); } return this.context_[key]; } @@ -156,6 +160,16 @@ class VectorRenderTile extends Tile { load() { this.getSourceTiles(); } + + /** + * @inheritDoc + */ + release() { + for (const key in this.context_) { + canvasPool.push(this.context_[key].canvas); + } + super.release(); + } } diff --git a/src/ol/dom.js b/src/ol/dom.js index 900fa6d559..ad10c14671 100644 --- a/src/ol/dom.js +++ b/src/ol/dom.js @@ -7,10 +7,12 @@ * Create an html canvas element and returns its 2d context. * @param {number=} opt_width Canvas width. * @param {number=} opt_height Canvas height. + * @param {Array=} opt_canvasPool Canvas pool to take existing canvas from. * @return {CanvasRenderingContext2D} The context. */ -export function createCanvasContext2D(opt_width, opt_height) { - const canvas = document.createElement('canvas'); +export function createCanvasContext2D(opt_width, opt_height, opt_canvasPool) { + const canvas = opt_canvasPool && opt_canvasPool.length ? + opt_canvasPool.shift() : document.createElement('canvas'); if (opt_width) { canvas.width = opt_width; }