From 4e69b2bb8b67072366d562516e40e0c7191b245a Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Fri, 12 Apr 2013 17:50:33 +0200 Subject: [PATCH 1/4] Create and remove layer renderers in renderFrame Previously, the map renderer would listen for layers being added and removed from the layers collection, and would create and remove layer renderers in response to these events. With this change, layer renderers are only created or removed when renderFrame is called, which leads to somewhat simpler code. We still need to listen to changes to the layers collection, but now these only trigger a new render. This new approach also has an advantage when layers change order. Swapping the order of two layers involves removing one and re-inserting it elsewhere. With the old approach, this would cause the deletion and re-creation of the layer renderer. With this new approach, the layer renderer is preserved. --- src/ol/renderer/canvas/canvasmaprenderer.js | 2 + src/ol/renderer/dom/dommaprenderer.js | 1 + src/ol/renderer/maprenderer.js | 119 ++++++++------------ src/ol/renderer/webgl/webglmaprenderer.js | 2 + 4 files changed, 55 insertions(+), 69 deletions(-) diff --git a/src/ol/renderer/canvas/canvasmaprenderer.js b/src/ol/renderer/canvas/canvasmaprenderer.js index 69191c5bae..b5e6d80458 100644 --- a/src/ol/renderer/canvas/canvasmaprenderer.js +++ b/src/ol/renderer/canvas/canvasmaprenderer.js @@ -153,4 +153,6 @@ ol.renderer.canvas.Map.prototype.renderFrame = function(frameState) { this.renderedVisible_ = true; } + this.removeUnusedLayerRenderers(frameState); + }; diff --git a/src/ol/renderer/dom/dommaprenderer.js b/src/ol/renderer/dom/dommaprenderer.js index f821be3f07..4866a11763 100644 --- a/src/ol/renderer/dom/dommaprenderer.js +++ b/src/ol/renderer/dom/dommaprenderer.js @@ -93,5 +93,6 @@ ol.renderer.dom.Map.prototype.renderFrame = function(frameState) { } this.calculateMatrices2D(frameState); + this.removeUnusedLayerRenderers(frameState); }; diff --git a/src/ol/renderer/maprenderer.js b/src/ol/renderer/maprenderer.js index 870cb37fdb..70a36d840e 100644 --- a/src/ol/renderer/maprenderer.js +++ b/src/ol/renderer/maprenderer.js @@ -42,32 +42,27 @@ ol.renderer.Map = function(container, map) { /** * @private - * @type {Object.} + * @type {Object.} */ this.layerRenderers_ = {}; - // - // We listen to layer add/remove to add/remove layer renderers. - // - /** * @private * @type {?number} */ - this.mapLayersChangedListenerKey_ = - goog.events.listen( - map, ol.Object.getChangedEventType(ol.MapProperty.LAYERS), - this.handleLayersChanged_, false, this); + this.mapLayersChangedListenerKey_ = goog.events.listen( + map, ol.Object.getChangedEventType(ol.MapProperty.LAYERS), + this.handleLayersChanged_, false, this); /** * @private - * @type {Array.} + * @type {Array.} */ this.layersListenerKeys_ = null; /** * @private - * @type {Object.} + * @type {Object.} */ this.layerRendererChangeListenKeys_ = {}; @@ -75,17 +70,6 @@ ol.renderer.Map = function(container, map) { goog.inherits(ol.renderer.Map, goog.Disposable); -/** - * @param {ol.layer.Layer} layer Layer. - * @private - */ -ol.renderer.Map.prototype.addLayer_ = function(layer) { - var layerRenderer = this.createLayerRenderer(layer); - this.setLayerRenderer_(layer, layerRenderer); - this.getMap().render(); -}; - - /** * @param {ol.FrameState} frameState FrameState. * @protected @@ -136,9 +120,8 @@ ol.renderer.Map.prototype.disposeInternal = function() { goog.dispose(layerRenderer); }); goog.events.unlistenByKey(this.mapLayersChangedListenerKey_); - if (!goog.isNull(this.layersListenerKeys_)) { - goog.array.forEach(this.layersListenerKeys_, goog.events.unlistenByKey); - } + goog.object.forEach( + this.layerRendererChangeListenKeys_, goog.events.unlistenByKey); goog.base(this, 'disposeInternal'); }; @@ -155,10 +138,17 @@ ol.renderer.Map.prototype.getCanvas = goog.functions.NULL; * @return {ol.renderer.Layer} Layer renderer. */ ol.renderer.Map.prototype.getLayerRenderer = function(layer) { - var layerKey = goog.getUid(layer); - var layerRenderer = this.layerRenderers_[layerKey]; - goog.asserts.assert(goog.isDef(layerRenderer)); - return layerRenderer; + var layerKey = goog.getUid(layer).toString(); + if (layerKey in this.layerRenderers_) { + return this.layerRenderers_[layerKey]; + } else { + var layerRenderer = this.createLayerRenderer(layer); + this.layerRenderers_[layerKey] = layerRenderer; + this.layerRendererChangeListenKeys_[layerKey] = goog.events.listen( + layerRenderer, goog.events.EventType.CHANGE, + this.handleLayerRendererChange_, false, this); + return layerRenderer; + } }; @@ -193,24 +183,21 @@ ol.renderer.Map.prototype.handleLayerRendererChange_ = function(event) { * @private */ ol.renderer.Map.prototype.handleLayersAdd_ = function(collectionEvent) { - var layer = /** @type {ol.layer.Layer} */ (collectionEvent.elem); - this.addLayer_(layer); + this.getMap().render(); }; /** + * @param {goog.events.Event} event Event. * @private */ -ol.renderer.Map.prototype.handleLayersChanged_ = function() { - goog.disposeAll(goog.object.getValues(this.layerRenderers_)); - this.layerRenderers_ = {}; +ol.renderer.Map.prototype.handleLayersChanged_ = function(event) { if (!goog.isNull(this.layersListenerKeys_)) { goog.array.forEach(this.layersListenerKeys_, goog.events.unlistenByKey); this.layersListenerKeys_ = null; } var layers = this.getMap().getLayers(); if (goog.isDefAndNotNull(layers)) { - layers.forEach(this.addLayer_, this); this.layersListenerKeys_ = [ goog.events.listen(layers, ol.CollectionEventType.ADD, this.handleLayersAdd_, false, this), @@ -218,6 +205,7 @@ ol.renderer.Map.prototype.handleLayersChanged_ = function() { this.handleLayersRemove_, false, this) ]; } + this.getMap().render(); }; @@ -226,37 +214,23 @@ ol.renderer.Map.prototype.handleLayersChanged_ = function() { * @private */ ol.renderer.Map.prototype.handleLayersRemove_ = function(collectionEvent) { - var layer = /** @type {ol.layer.Layer} */ (collectionEvent.elem); - this.removeLayer_(layer); -}; - - -/** - * @param {ol.layer.Layer} layer Layer. - * @private - */ -ol.renderer.Map.prototype.removeLayer_ = function(layer) { - goog.dispose(this.removeLayerRenderer_(layer)); this.getMap().render(); }; /** - * @param {ol.layer.Layer} layer Layer. + * @param {string} layerKey Layer key. * @return {ol.renderer.Layer} Layer renderer. * @private */ -ol.renderer.Map.prototype.removeLayerRenderer_ = function(layer) { - var layerKey = goog.getUid(layer); - if (layerKey in this.layerRenderers_) { - var layerRenderer = this.layerRenderers_[layerKey]; - delete this.layerRenderers_[layerKey]; - goog.events.unlistenByKey(this.layerRendererChangeListenKeys_[layerKey]); - delete this.layerRendererChangeListenKeys_[layerKey]; - return layerRenderer; - } else { - return null; - } +ol.renderer.Map.prototype.removeLayerRendererByKey_ = function(layerKey) { + goog.asserts.assert(layerKey in this.layerRenderers_); + var layerRenderer = this.layerRenderers_[layerKey]; + delete this.layerRenderers_[layerKey]; + goog.asserts.assert(layerKey in this.layerRendererChangeListenKeys_); + goog.events.unlistenByKey(this.layerRendererChangeListenKeys_[layerKey]); + delete this.layerRendererChangeListenKeys_[layerKey]; + return layerRenderer; }; @@ -268,16 +242,23 @@ ol.renderer.Map.prototype.renderFrame = goog.nullFunction; /** - * @param {ol.layer.Layer} layer Layer. - * @param {ol.renderer.Layer} layerRenderer Layer renderer. - * @private + * @param {!ol.FrameState} frameState Frame state. + * @protected */ -ol.renderer.Map.prototype.setLayerRenderer_ = function(layer, layerRenderer) { - var layerKey = goog.getUid(layer); - goog.asserts.assert(!(layerKey in this.layerRenderers_)); - this.layerRenderers_[layerKey] = layerRenderer; - goog.asserts.assert(!(layerKey in this.layerRendererChangeListenKeys_)); - this.layerRendererChangeListenKeys_[layerKey] = goog.events.listen( - layerRenderer, goog.events.EventType.CHANGE, - this.handleLayerRendererChange_, false, this); +ol.renderer.Map.prototype.removeUnusedLayerRenderers = + function(frameState) { + var layerRenderersToRemove = {}; + var layerKey; + for (layerKey in this.layerRenderers_) { + layerRenderersToRemove[layerKey] = true; + } + var layersArray = frameState.layersArray; + var i; + for (i = 0; i < layersArray.length; ++i) { + layerKey = goog.getUid(layersArray[i]).toString(); + delete layerRenderersToRemove[layerKey]; + } + for (layerKey in layerRenderersToRemove) { + goog.dispose(this.removeLayerRendererByKey_(layerKey)); + } }; diff --git a/src/ol/renderer/webgl/webglmaprenderer.js b/src/ol/renderer/webgl/webglmaprenderer.js index 77b03c3d36..3aed6a4972 100644 --- a/src/ol/renderer/webgl/webglmaprenderer.js +++ b/src/ol/renderer/webgl/webglmaprenderer.js @@ -641,4 +641,6 @@ ol.renderer.webgl.Map.prototype.renderFrame = function(frameState) { frameState.animate = true; } + this.removeUnusedLayerRenderers(frameState); + }; From 07fe17924bcc2b72d31d4a70f517d62484397e94 Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Tue, 16 Apr 2013 18:16:08 +0200 Subject: [PATCH 2/4] Remove layer renderers in post render function --- src/ol/renderer/canvas/canvasmaprenderer.js | 2 +- src/ol/renderer/dom/dommaprenderer.js | 2 +- src/ol/renderer/maprenderer.js | 35 ++++++++++++++------- src/ol/renderer/webgl/webglmaprenderer.js | 2 +- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/ol/renderer/canvas/canvasmaprenderer.js b/src/ol/renderer/canvas/canvasmaprenderer.js index b5e6d80458..0de485157b 100644 --- a/src/ol/renderer/canvas/canvasmaprenderer.js +++ b/src/ol/renderer/canvas/canvasmaprenderer.js @@ -153,6 +153,6 @@ ol.renderer.canvas.Map.prototype.renderFrame = function(frameState) { this.renderedVisible_ = true; } - this.removeUnusedLayerRenderers(frameState); + this.scheduleRemoveUnusedLayerRenderers(frameState); }; diff --git a/src/ol/renderer/dom/dommaprenderer.js b/src/ol/renderer/dom/dommaprenderer.js index 4866a11763..14072ba424 100644 --- a/src/ol/renderer/dom/dommaprenderer.js +++ b/src/ol/renderer/dom/dommaprenderer.js @@ -93,6 +93,6 @@ ol.renderer.dom.Map.prototype.renderFrame = function(frameState) { } this.calculateMatrices2D(frameState); - this.removeUnusedLayerRenderers(frameState); + this.scheduleRemoveUnusedLayerRenderers(frameState); }; diff --git a/src/ol/renderer/maprenderer.js b/src/ol/renderer/maprenderer.js index 70a36d840e..9f9690870d 100644 --- a/src/ol/renderer/maprenderer.js +++ b/src/ol/renderer/maprenderer.js @@ -241,24 +241,35 @@ ol.renderer.Map.prototype.removeLayerRendererByKey_ = function(layerKey) { ol.renderer.Map.prototype.renderFrame = goog.nullFunction; +/** + * @param {ol.Map} map Map. + * @param {!ol.FrameState} frameState Frame state. + * @private + */ +ol.renderer.Map.prototype.removeUnusedLayerRenderers_ = + function(map, frameState) { + var layerStates = frameState.layerStates; + var layerKey; + for (layerKey in this.layerRenderers_) { + if (!(layerKey in layerStates)) { + goog.dispose(this.removeLayerRendererByKey_(layerKey)); + } + } +}; + + /** * @param {!ol.FrameState} frameState Frame state. * @protected */ -ol.renderer.Map.prototype.removeUnusedLayerRenderers = +ol.renderer.Map.prototype.scheduleRemoveUnusedLayerRenderers = function(frameState) { - var layerRenderersToRemove = {}; var layerKey; for (layerKey in this.layerRenderers_) { - layerRenderersToRemove[layerKey] = true; - } - var layersArray = frameState.layersArray; - var i; - for (i = 0; i < layersArray.length; ++i) { - layerKey = goog.getUid(layersArray[i]).toString(); - delete layerRenderersToRemove[layerKey]; - } - for (layerKey in layerRenderersToRemove) { - goog.dispose(this.removeLayerRendererByKey_(layerKey)); + if (!(layerKey in frameState.layerStates)) { + frameState.postRenderFunctions.push( + goog.bind(this.removeUnusedLayerRenderers_, this)); + return; + } } }; diff --git a/src/ol/renderer/webgl/webglmaprenderer.js b/src/ol/renderer/webgl/webglmaprenderer.js index 3aed6a4972..465a2ab8d2 100644 --- a/src/ol/renderer/webgl/webglmaprenderer.js +++ b/src/ol/renderer/webgl/webglmaprenderer.js @@ -641,6 +641,6 @@ ol.renderer.webgl.Map.prototype.renderFrame = function(frameState) { frameState.animate = true; } - this.removeUnusedLayerRenderers(frameState); + this.scheduleRemoveUnusedLayerRenderers(frameState); }; From bc10446b0e480b8ae6fb4df24a2f74dd751d8bfe Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Tue, 16 Apr 2013 18:31:59 +0200 Subject: [PATCH 3/4] Track changes to layers in the map --- src/ol/map.js | 51 +++++++++++++++++++++++++++++ src/ol/renderer/maprenderer.js | 59 ---------------------------------- 2 files changed, 51 insertions(+), 59 deletions(-) diff --git a/src/ol/map.js b/src/ol/map.js index 3203a2b5fb..4bcbb91c47 100644 --- a/src/ol/map.js +++ b/src/ol/map.js @@ -29,6 +29,8 @@ goog.require('goog.style'); goog.require('goog.vec.Mat4'); goog.require('ol.BrowserFeature'); goog.require('ol.Collection'); +goog.require('ol.CollectionEvent'); +goog.require('ol.CollectionEventType'); goog.require('ol.Color'); goog.require('ol.Extent'); goog.require('ol.FrameState'); @@ -295,6 +297,14 @@ ol.Map = function(options) { goog.bind(this.getTilePriority, this), goog.bind(this.handleTileChange_, this)); + /** + * @private + * @type {Array.} + */ + this.layersListenerKeys_ = null; + + goog.events.listen(this, ol.Object.getChangedEventType(ol.MapProperty.LAYERS), + this.handleLayersChanged_, false, this); goog.events.listen(this, ol.Object.getChangedEventType(ol.MapProperty.VIEW), this.handleViewChanged_, false, this); goog.events.listen(this, ol.Object.getChangedEventType(ol.MapProperty.SIZE), @@ -302,6 +312,7 @@ ol.Map = function(options) { goog.events.listen( this, ol.Object.getChangedEventType(ol.MapProperty.BACKGROUND_COLOR), this.handleBackgroundColorChanged_, false, this); + this.setValues(optionsInternal.values); // this gives the map an initial size @@ -549,6 +560,46 @@ ol.Map.prototype.handleBrowserEvent = function(browserEvent, opt_type) { }; +/** + * @param {ol.CollectionEvent} collectionEvent Collection event. + * @private + */ +ol.Map.prototype.handleLayersAdd_ = function(collectionEvent) { + this.render(); +}; + + +/** + * @param {goog.events.Event} event Event. + * @private + */ +ol.Map.prototype.handleLayersChanged_ = function(event) { + if (!goog.isNull(this.layersListenerKeys_)) { + goog.array.forEach(this.layersListenerKeys_, goog.events.unlistenByKey); + this.layersListenerKeys_ = null; + } + var layers = this.getLayers(); + if (goog.isDefAndNotNull(layers)) { + this.layersListenerKeys_ = [ + goog.events.listen(layers, ol.CollectionEventType.ADD, + this.handleLayersAdd_, false, this), + goog.events.listen(layers, ol.CollectionEventType.REMOVE, + this.handleLayersRemove_, false, this) + ]; + } + this.render(); +}; + + +/** + * @param {ol.CollectionEvent} collectionEvent Collection event. + * @private + */ +ol.Map.prototype.handleLayersRemove_ = function(collectionEvent) { + this.render(); +}; + + /** * @param {ol.MapBrowserEvent} mapBrowserEvent The event to handle. */ diff --git a/src/ol/renderer/maprenderer.js b/src/ol/renderer/maprenderer.js index 9f9690870d..15276cca0d 100644 --- a/src/ol/renderer/maprenderer.js +++ b/src/ol/renderer/maprenderer.js @@ -1,7 +1,6 @@ goog.provide('ol.renderer.Map'); goog.require('goog.Disposable'); -goog.require('goog.array'); goog.require('goog.asserts'); goog.require('goog.dispose'); goog.require('goog.events'); @@ -9,10 +8,7 @@ goog.require('goog.events.EventType'); goog.require('goog.functions'); goog.require('goog.object'); goog.require('goog.vec.Mat4'); -goog.require('ol.CollectionEvent'); -goog.require('ol.CollectionEventType'); goog.require('ol.FrameState'); -goog.require('ol.Object'); goog.require('ol.layer.Layer'); goog.require('ol.renderer.Layer'); @@ -46,20 +42,6 @@ ol.renderer.Map = function(container, map) { */ this.layerRenderers_ = {}; - /** - * @private - * @type {?number} - */ - this.mapLayersChangedListenerKey_ = goog.events.listen( - map, ol.Object.getChangedEventType(ol.MapProperty.LAYERS), - this.handleLayersChanged_, false, this); - - /** - * @private - * @type {Array.} - */ - this.layersListenerKeys_ = null; - /** * @private * @type {Object.} @@ -119,7 +101,6 @@ ol.renderer.Map.prototype.disposeInternal = function() { goog.object.forEach(this.layerRenderers_, function(layerRenderer) { goog.dispose(layerRenderer); }); - goog.events.unlistenByKey(this.mapLayersChangedListenerKey_); goog.object.forEach( this.layerRendererChangeListenKeys_, goog.events.unlistenByKey); goog.base(this, 'disposeInternal'); @@ -178,46 +159,6 @@ ol.renderer.Map.prototype.handleLayerRendererChange_ = function(event) { }; -/** - * @param {ol.CollectionEvent} collectionEvent Collection event. - * @private - */ -ol.renderer.Map.prototype.handleLayersAdd_ = function(collectionEvent) { - this.getMap().render(); -}; - - -/** - * @param {goog.events.Event} event Event. - * @private - */ -ol.renderer.Map.prototype.handleLayersChanged_ = function(event) { - if (!goog.isNull(this.layersListenerKeys_)) { - goog.array.forEach(this.layersListenerKeys_, goog.events.unlistenByKey); - this.layersListenerKeys_ = null; - } - var layers = this.getMap().getLayers(); - if (goog.isDefAndNotNull(layers)) { - this.layersListenerKeys_ = [ - goog.events.listen(layers, ol.CollectionEventType.ADD, - this.handleLayersAdd_, false, this), - goog.events.listen(layers, ol.CollectionEventType.REMOVE, - this.handleLayersRemove_, false, this) - ]; - } - this.getMap().render(); -}; - - -/** - * @param {ol.CollectionEvent} collectionEvent Collection event. - * @private - */ -ol.renderer.Map.prototype.handleLayersRemove_ = function(collectionEvent) { - this.getMap().render(); -}; - - /** * @param {string} layerKey Layer key. * @return {ol.renderer.Layer} Layer renderer. From a07e70ea3234147d1f2ac4d94ccffc780ffd54f3 Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Tue, 16 Apr 2013 23:40:48 +0200 Subject: [PATCH 4/4] Handle obscure edge case where deferred post render function receives a null frame state --- src/ol/renderer/maprenderer.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ol/renderer/maprenderer.js b/src/ol/renderer/maprenderer.js index 15276cca0d..adc4de270f 100644 --- a/src/ol/renderer/maprenderer.js +++ b/src/ol/renderer/maprenderer.js @@ -184,15 +184,14 @@ ol.renderer.Map.prototype.renderFrame = goog.nullFunction; /** * @param {ol.Map} map Map. - * @param {!ol.FrameState} frameState Frame state. + * @param {ol.FrameState} frameState Frame state. * @private */ ol.renderer.Map.prototype.removeUnusedLayerRenderers_ = function(map, frameState) { - var layerStates = frameState.layerStates; var layerKey; for (layerKey in this.layerRenderers_) { - if (!(layerKey in layerStates)) { + if (goog.isNull(frameState) || !(layerKey in frameState.layerStates)) { goog.dispose(this.removeLayerRendererByKey_(layerKey)); } }