From 1877f92d46ba8d7dae7badf3a055847b857c4a82 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Tue, 19 Nov 2013 14:49:49 -0700 Subject: [PATCH] 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. --- .../canvas/canvasvectorlayerrenderer.js | 60 ++++++++----------- src/ol/source/vectorsource.js | 27 ++++++--- src/ol/structs/rtree.js | 53 ++++++++-------- test/spec/ol/structs/rtree.test.js | 37 ++++++++++-- 4 files changed, 100 insertions(+), 77 deletions(-) diff --git a/src/ol/renderer/canvas/canvasvectorlayerrenderer.js b/src/ol/renderer/canvas/canvasvectorlayerrenderer.js index 6ec6e528dc..3874e0c8a0 100644 --- a/src/ol/renderer/canvas/canvasvectorlayerrenderer.js +++ b/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 candidates = source.getFeaturesObjectForExtent(locationBbox, - 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 extent = ol.extent.boundingExtent([locationMin, locationMax]); + var projection = map.getView().getView2D().getProjection(); - var candidate, geom, type, symbolBounds, symbolSize, symbolOffset, - halfWidth, halfHeight, uid, coordinates, j; - for (var id in candidates) { - candidate = candidates[id]; - if (candidate.getRenderIntent() == - ol.FeatureRenderIntent.HIDDEN) { - continue; + source.forEachFeatureInExtent(extent, projection, function(candidate) { + var geom, type, symbolBounds, symbolSize, symbolOffset, + halfWidth, halfHeight, uid, coordinates, j; + + if (candidate.getRenderIntent() == ol.FeatureRenderIntent.HIDDEN) { + return; } 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( - tileExtent, projection); - if (goog.isNull(featuresObject)) { - deferred = true; - break fetchTileData; - } - tileHasFeatures = tileHasFeatures || - !goog.object.isEmpty(featuresObject); - goog.object.extend(featuresToRender, featuresObject); + source.forEachFeatureInExtent( + tileExtent, projection, function(feature) { + featuresToRender[goog.getUid(feature)] = feature; + tileHasFeatures = true; + }); 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]; diff --git a/src/ol/source/vectorsource.js b/src/ol/source/vectorsource.js index fb2a20f57a..ec0085724f 100644 --- a/src/ol/source/vectorsource.js +++ b/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.} 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, - projection) { - // TODO: create forEachFeatureInExtent method instead +ol.source.Vector.prototype.forEachFeatureInExtent = function(extent, + projection, callback, opt_thisArg) { // 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.} 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) { - return this.rTree_.searchReturningObject(extent); +ol.source.FeatureCache.prototype.forEach = + function(extent, callback, opt_thisArg) { + this.rTree_.forEach( + extent, /** @type {function(Object)} */ (callback), opt_thisArg); }; diff --git a/src/ol/structs/rtree.js b/src/ol/structs/rtree.js index 6057998bc6..fc314e79c3 100644 --- a/src/ol/structs/rtree.js +++ b/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} */ - (this.searchSubtree_(tree, true, [], tree)); + workingObject.nodes = 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} */ ( - this.searchSubtree_(rect, false, [], this.rootTree_, opt_type)); + return 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 {string|number=} opt_type Optional type of the objects we want to - * find. - * @return {Object} Result. Keys are UIDs of the values. + * @param {ol.Extent} extent Extent to search. + * @param {function(this: T, Object)} callback Callback called with each result. + * @param {T=} opt_thisArg The object to be used as the value of 'this' for + * 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_(rect, false, [], this.rootTree_, opt_type, true)); + this.searchSubtree_( + 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 - * by UID. + * @param {function(this: T, Object)=} opt_callback Callback called with each + * 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) { - var resultObject = {}; + rect, returnNode, result, root, opt_type, opt_callback, opt_thisArg) { 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)) { - resultObject[goog.getUid(obj).toString()] = obj; + if (goog.isDef(opt_callback)) { + 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; - } + return result; }; diff --git a/test/spec/ol/structs/rtree.test.js b/test/spec/ol/structs/rtree.test.js index 0a65afb978..08a15373d0 100644 --- a/test/spec/ol/structs/rtree.test.js +++ b/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); - var result = rTree.searchReturningObject([4, 4, 6, 6]); - expect(result[goog.getUid(obj)]).to.equal(obj); + }); + + describe('#forEach()', function() { + var tree; + 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); }); });