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/maprenderer.js

@@ -42,32 +42,27 @@ ol.renderer.Map = function(container, map) {
 
   /**
    * @private
-   * @type {Object.<number, ol.renderer.Layer>}
+   * @type {Object.<string, ol.renderer.Layer>}
    */
   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.<number>}
+   * @type {Array.<?number>}
    */
   this.layersListenerKeys_ = null;
 
   /**
    * @private
-   * @type {Object.<number, ?number>}
+   * @type {Object.<string, ?number>}
    */
   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));
+  }
 };