Add forEach method to rtree, use it in feature cache

This saves having to create feature lookup objects and iterate through lookup properties multiple times.

src/ol/renderer/canvas/canvasvectorlayerrenderer.js

@@ -1,6 +1,7 @@
 goog.provide('ol.renderer.canvas.VectorLayer');
 
 goog.require('goog.asserts');
+goog.require('goog.async.nextTick');
 goog.require('goog.dom');
 goog.require('goog.dom.TagName');
 goog.require('goog.events');
@@ -247,12 +248,13 @@ ol.renderer.canvas.VectorLayer.prototype.getFeaturesForPixel =
     function(pixel, success, opt_error) {
   // TODO What do we want to pass to the error callback?
   var map = this.getMap();
-  var result = [];
+  var features = [];
 
   var source = this.getVectorLayer().getSource();
   var location = map.getCoordinateFromPixel(pixel);
   var tileCoord = this.tileGrid_.getTileCoordForCoordAndZ(location, 0);
   var key = tileCoord.toString();
+
   if (this.tileCache_.containsKey(key)) {
     var cachedTile = this.tileCache_.get(key);
     var symbolSizes = cachedTile[1];
@@ -262,24 +264,15 @@ ol.renderer.canvas.VectorLayer.prototype.getFeaturesForPixel =
     var halfMaxHeight = maxSymbolSize[1] / 2;
     var locationMin = [location[0] - halfMaxWidth, location[1] - halfMaxHeight];
     var locationMax = [location[0] + halfMaxWidth, location[1] + halfMaxHeight];
-    var locationBbox = ol.extent.boundingExtent([locationMin, locationMax]);
+    var extent = ol.extent.boundingExtent([locationMin, locationMax]);
-    var candidates = source.getFeaturesObjectForExtent(locationBbox,
+    var projection = map.getView().getView2D().getProjection();
-        map.getView().getView2D().getProjection());
-    if (goog.isNull(candidates)) {
-      // data is not loaded
-      if (goog.isDef(opt_error)) {
-        goog.global.setTimeout(function() { opt_error(); }, 0);
-      }
-      return;
-    }
 
-    var candidate, geom, type, symbolBounds, symbolSize, symbolOffset,
+    source.forEachFeatureInExtent(extent, projection, function(candidate) {
+      var geom, type, symbolBounds, symbolSize, symbolOffset,
           halfWidth, halfHeight, uid, coordinates, j;
-    for (var id in candidates) {
+
-      candidate = candidates[id];
+      if (candidate.getRenderIntent() == ol.FeatureRenderIntent.HIDDEN) {
-      if (candidate.getRenderIntent() ==
+        return;
-          ol.FeatureRenderIntent.HIDDEN) {
-        continue;
       }
       geom = candidate.getGeometry();
       type = geom.getType();
@@ -304,27 +297,30 @@ ol.renderer.canvas.VectorLayer.prototype.getFeaturesForPixel =
         }
         for (j = coordinates.length - 1; j >= 0; --j) {
           if (ol.extent.containsCoordinate(symbolBounds, coordinates[j])) {
-            result.push(candidate);
+            features.push(candidate);
             break;
           }
         }
       } else if (goog.isFunction(geom.containsCoordinate)) {
         // For polygons, check if the pixel location is inside the polygon
         if (geom.containsCoordinate(location)) {
-          result.push(candidate);
+          features.push(candidate);
         }
       } else if (goog.isFunction(geom.distanceFromCoordinate)) {
         // For lines, check if the distance to the pixel location is
         // within the rendered line width
         if (2 * geom.distanceFromCoordinate(location) <=
             symbolSizes[goog.getUid(candidate)][0]) {
-          result.push(candidate);
+          features.push(candidate);
-        }
         }
       }
+
+    });
   }
   var layer = this.getLayer();
-  goog.global.setTimeout(function() { success(result, layer); }, 0);
+  goog.async.nextTick(function() {
+    success(features, layer);
+  });
 };
 
 
@@ -482,10 +478,9 @@ ol.renderer.canvas.VectorLayer.prototype.renderFrame =
   // TODO make gutter configurable?
   var tileGutter = 15 * tileResolution;
   var tile, tileCoord, key, x, y, i, type;
-  var deferred = false;
   var dirty = false;
   var tileExtent, featuresObject, tileHasFeatures;
-  fetchTileData:
+
   for (x = tileRange.minX; x <= tileRange.maxX; ++x) {
     for (y = tileRange.minY; y <= tileRange.maxY; ++y) {
       tileCoord = new ol.TileCoord(0, x, y);
@@ -499,15 +494,11 @@ ol.renderer.canvas.VectorLayer.prototype.renderFrame =
         tileExtent[1] -= tileGutter;
         tileExtent[3] += tileGutter;
         tileHasFeatures = false;
-        featuresObject = source.getFeaturesObjectForExtent(
+        source.forEachFeatureInExtent(
-            tileExtent, projection);
+            tileExtent, projection, function(feature) {
-        if (goog.isNull(featuresObject)) {
+              featuresToRender[goog.getUid(feature)] = feature;
-          deferred = true;
+              tileHasFeatures = true;
-          break fetchTileData;
+            });
-        }
-        tileHasFeatures = tileHasFeatures ||
-            !goog.object.isEmpty(featuresObject);
-        goog.object.extend(featuresToRender, featuresObject);
         if (tileHasFeatures) {
           tilesOnSketchCanvas[key] = tileCoord;
         }
@@ -525,6 +516,7 @@ ol.renderer.canvas.VectorLayer.prototype.renderFrame =
   var groups = style.groupFeaturesBySymbolizerLiteral(
       featuresToRender, tileResolution);
   var numGroups = groups.length;
+  var deferred = false;
   var group;
   for (var j = 0; j < numGroups; ++j) {
     group = groups[j];

src/ol/source/vectorsource.js

@@ -224,13 +224,16 @@ ol.source.Vector.prototype.getFeatures = function(opt_filter) {
  *
  * @param {ol.Extent} extent Bounding extent.
  * @param {ol.proj.Projection} projection Target projection.
- * @return {Object.<string, ol.Feature>} Features lookup object.
+ * @param {function(this: T, ol.Feature)} callback Callback called with each
+ *     feature.
+ * @param {T=} opt_thisArg The object to be used as the value of 'this' for
+ *     the callback.
+ * @template T
  */
-ol.source.Vector.prototype.getFeaturesObjectForExtent = function(extent,
+ol.source.Vector.prototype.forEachFeatureInExtent = function(extent,
-    projection) {
+    projection, callback, opt_thisArg) {
-  // TODO: create forEachFeatureInExtent method instead
   // TODO: transform if requested project is different than loaded projection
-  return this.featureCache_.getFeaturesObjectForExtent(extent);
+  this.featureCache_.forEach(extent, callback, opt_thisArg);
 };
 
 
@@ -394,13 +397,19 @@ ol.source.FeatureCache.prototype.getFeaturesObject = function() {
 
 
 /**
- * Get all features whose bounding box intersects the provided extent.
+ * Operate on each feature whose bounding box intersects the provided extent.
  *
  * @param {ol.Extent} extent Bounding extent.
- * @return {Object.<string, ol.Feature>} Features.
+ * @param {function(this: T, ol.Feature)} callback Callback called with each
+ *     feature.
+ * @param {T=} opt_thisArg The object to be used as the value of 'this' for
+ *     the callback.
+ * @template T
  */
-ol.source.FeatureCache.prototype.getFeaturesObjectForExtent = function(extent) {
+ol.source.FeatureCache.prototype.forEach =
-  return this.rTree_.searchReturningObject(extent);
+    function(extent, callback, opt_thisArg) {
+  this.rTree_.forEach(
+      extent, /** @type {function(Object)} */ (callback), opt_thisArg);
 };
 
 

src/ol/structs/rtree.js

@@ -499,8 +499,7 @@ ol.structs.RTree.prototype.removeSubtree_ = function(rect, obj, root) {
             ol.structs.RTree.recalculateExtent_(tree);
             workingObject.target = undefined;
             if (tree.nodes.length < this.minWidth_) { // Underflow
-              workingObject.nodes = /** @type {Array} */
+              workingObject.nodes = this.searchSubtree_(tree, true, [], tree);
-                  (this.searchSubtree_(tree, true, [], tree));
             }
             break;
           } else if (goog.isDef(lTree.nodes)) {
@@ -528,15 +527,13 @@ ol.structs.RTree.prototype.removeSubtree_ = function(rect, obj, root) {
       workingObject.nodes.length = 0;
       if (hitStack.length === 0 && tree.nodes.length <= 1) {
         // Underflow..on root!
-        workingObject.nodes = /** @type {Array} */
+        this.searchSubtree_(tree, true, workingObject.nodes, tree);
-            (this.searchSubtree_(tree, true, workingObject.nodes, tree));
         tree.nodes.length = 0;
         hitStack.push(tree);
         countStack.push(1);
       } else if (hitStack.length > 0 && tree.nodes.length < this.minWidth_) {
         // Underflow..AGAIN!
-        workingObject.nodes = /** @type {Array} */
+        this.searchSubtree_(tree, true, workingObject.nodes, tree);
-            (this.searchSubtree_(tree, true, workingObject.nodes, tree));
         tree.nodes.length = 0;
       } else {
         workingObject.nodes = undefined; // Just start resizing
@@ -562,24 +559,24 @@ ol.structs.RTree.prototype.removeSubtree_ = function(rect, obj, root) {
  */
 ol.structs.RTree.prototype.search = function(extent, opt_type) {
   var rect = /** @type {ol.structs.RTreeNode} */ ({extent: extent});
-  return /** @type {Array} */ (
+  return this.searchSubtree_(rect, false, [], this.rootTree_, opt_type);
-      this.searchSubtree_(rect, false, [], this.rootTree_, opt_type));
 };
 
 
 /**
- * Non-recursive search function
+ * Search in the given extent and call the callback with each result.
  *
- * @param {ol.Extent} extent Extent.
+ * @param {ol.Extent} extent Extent to search.
- * @param {string|number=} opt_type Optional type of the objects we want to
+ * @param {function(this: T, Object)} callback Callback called with each result.
- *     find.
+ * @param {T=} opt_thisArg The object to be used as the value of 'this' for
- * @return {Object} Result. Keys are UIDs of the values.
+ *     the callback.
  * @this {ol.structs.RTree}
+ * @template T
  */
-ol.structs.RTree.prototype.searchReturningObject = function(extent, opt_type) {
+ol.structs.RTree.prototype.forEach = function(extent, callback, opt_thisArg) {
   var rect = /** @type {ol.structs.RTreeNode} */ ({extent: extent});
-  return /** @type {Object} */ (
+  this.searchSubtree_(
-      this.searchSubtree_(rect, false, [], this.rootTree_, opt_type, true));
+      rect, false, [], this.rootTree_, undefined, callback, opt_thisArg);
 };
 
 
@@ -588,17 +585,19 @@ ol.structs.RTree.prototype.searchReturningObject = function(extent, opt_type) {
  *
  * @param {ol.structs.RTreeNode} rect Rectangle.
  * @param {boolean} returnNode Do we return nodes?
- * @param {Array|Object} result Result.
+ * @param {Array} result Result.
  * @param {ol.structs.RTreeNode} root Root.
  * @param {string|number=} opt_type Optional type to search for.
- * @param {boolean=} opt_resultAsObject If set, result will be an object keyed
+ * @param {function(this: T, Object)=} opt_callback Callback called with each
- *     by UID.
+ *     result.
+ * @param {T=} opt_thisArg The object to be used as the value of 'this' for
+ *     the callback.
  * @private
- * @return {Array|Object} Result.
+ * @template T
+ * @return {Array} Result.
  */
 ol.structs.RTree.prototype.searchSubtree_ = function(
-    rect, returnNode, result, root, opt_type, opt_resultAsObject) {
+    rect, returnNode, result, root, opt_type, opt_callback, opt_thisArg) {
-  var resultObject = {};
   var hitStack = []; // Contains the elements that overlap
 
   if (!ol.extent.intersects(rect.extent, root.extent)) {
@@ -621,8 +620,8 @@ ol.structs.RTree.prototype.searchSubtree_ = function(
             // walk all the way in to the leaf to know that we don't need it
             if (!goog.isDef(opt_type) || lTree.type == opt_type) {
               var obj = lTree.leaf;
-              if (goog.isDef(opt_resultAsObject)) {
+              if (goog.isDef(opt_callback)) {
-                resultObject[goog.getUid(obj).toString()] = obj;
+                opt_callback.call(opt_thisArg, obj);
               } else {
                 result.push(obj);
               }
@@ -635,9 +634,5 @@ ol.structs.RTree.prototype.searchSubtree_ = function(
     }
   } while (hitStack.length > 0);
 
-  if (goog.isDef(opt_resultAsObject)) {
-    return resultObject;
-  } else {
   return result;
-  }
 };

test/spec/ol/structs/rtree.test.js

@@ -109,11 +109,38 @@ describe('ol.structs.RTree', function() {
       expect(result.length).to.be(3);
     });
 
-    it('can return objects instead of arrays', function() {
+  });
-      var obj = {foo: 'bar'};
+
-      rTree.insert([5, 5, 5, 5], obj);
+  describe('#forEach()', function() {
-      var result = rTree.searchReturningObject([4, 4, 6, 6]);
+    var tree;
-      expect(result[goog.getUid(obj)]).to.equal(obj);
+    beforeEach(function() {
+      tree = new ol.structs.RTree();
+    });
+
+    it('calls a callback for each result in the search extent', function() {
+      var one = {};
+      tree.insert([4.5, 4.5, 5, 5], one);
+
+      var two = {};
+      tree.insert([5, 5, 5.5, 5.5], two);
+
+      var callback = sinon.spy();
+      tree.forEach([4, 4, 6, 6], callback);
+      expect(callback.callCount).to.be(2);
+      expect(callback.calledWith(one)).to.be(true);
+      expect(callback.calledWith(two)).to.be(true);
+    });
+
+    it('accepts a this argument', function() {
+      var obj = {};
+      tree.insert([5, 5, 5, 5], obj);
+
+      var callback = sinon.spy();
+      var thisArg = {};
+      tree.forEach([4, 4, 6, 6], callback, thisArg);
+      expect(callback.callCount).to.be(1);
+      expect(callback.calledWith(obj)).to.be(true);
+      expect(callback.calledOn(thisArg)).to.be(true);
     });
 
   });