From 96c6cd0ab06070d22b5eedd008cd37272a1f7ef5 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Wed, 27 Nov 2013 14:21:56 +0100 Subject: [PATCH 1/8] Documenting limitations of forEach and forEachInExtent Because the tree's structure changes when nodes are inserted, updated or deleted, no modifications are allowed in a forEach loop. --- src/ol/structs/rbush.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ol/structs/rbush.js b/src/ol/structs/rbush.js index 88c21b6c73..dc6d3dfb97 100644 --- a/src/ol/structs/rbush.js +++ b/src/ol/structs/rbush.js @@ -379,6 +379,8 @@ ol.structs.RBush.prototype.condense_ = function(path) { /** + * Calls a callback function with each node in the tree. Inside the callback, + * no tree modifications (insert, update, remove) can be made. * @param {function(this: S, T): *} callback Callback. * @param {S=} opt_obj Scope. * @return {*} Callback return value. @@ -420,6 +422,8 @@ ol.structs.RBush.prototype.forEach_ = function(node, callback, opt_obj) { /** + * Calls a callback function with each node in the provided extent. Inside the + * callback, no tree modifications (insert, update, remove) can be made. * @param {ol.Extent} extent Extent. * @param {function(this: S, T): *} callback Callback. * @param {S=} opt_obj Scope. From 137f797bee3fe6292d9a404ca71e475cb7f67f31 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Wed, 27 Nov 2013 14:48:06 +0100 Subject: [PATCH 2/8] Use ol.structs.RBush in ol.interaction.Modify This is one more step to bring the master and vector-api branches closer together. I am well aware that the current Modify interaction will probably be rewritten as part of the vector-api effort, but with changes like this we will eventually be able to remove ol.structs.RTree from master. --- src/ol/interaction/modifyinteraction.js | 134 +++++++++++++----------- 1 file changed, 70 insertions(+), 64 deletions(-) diff --git a/src/ol/interaction/modifyinteraction.js b/src/ol/interaction/modifyinteraction.js index 2966f635e5..6854a49856 100644 --- a/src/ol/interaction/modifyinteraction.js +++ b/src/ol/interaction/modifyinteraction.js @@ -4,7 +4,6 @@ goog.require('goog.array'); goog.require('goog.asserts'); goog.require('goog.events'); goog.require('goog.functions'); -goog.require('goog.object'); goog.require('ol.CollectionEventType'); goog.require('ol.Feature'); goog.require('ol.MapBrowserEvent.EventType'); @@ -22,7 +21,7 @@ goog.require('ol.layer.Vector'); goog.require('ol.layer.VectorEventType'); goog.require('ol.layer.VectorLayerRenderIntent'); goog.require('ol.source.Vector'); -goog.require('ol.structs.RTree'); +goog.require('ol.structs.RBush'); /** @@ -81,11 +80,11 @@ ol.interaction.Modify = function(opt_options) { this.modifiable_ = false; /** - * Segment RTree for each layer - * @type {Object.<*, ol.structs.RTree>} + * Segment RBush for each layer + * @type {Object.<*, ol.structs.RBush>} * @private */ - this.rTree_ = null; + this.rBush_ = null; /** * @type {number} @@ -122,8 +121,8 @@ ol.interaction.Modify.prototype.setMap = function(map) { } if (!goog.isNull(map)) { - if (goog.isNull(this.rTree_)) { - this.rTree_ = new ol.structs.RTree(); + if (goog.isNull(this.rBush_)) { + this.rBush_ = new ol.structs.RBush(); } if (goog.isNull(this.sketchLayer_)) { var sketchLayer = new ol.layer.Vector({ @@ -142,7 +141,7 @@ ol.interaction.Modify.prototype.setMap = function(map) { false, this); } else { // removing from a map, clean up - this.rTree_ = null; + this.rBush_ = null; this.sketchLayer_ = null; } @@ -230,16 +229,19 @@ ol.interaction.Modify.prototype.addIndex_ = function(features, layer) { * @private */ ol.interaction.Modify.prototype.removeIndex_ = function(features) { - var rTree = this.rTree_; - for (var i = 0, ii = features.length; i < ii; ++i) { - var feature = features[i]; - var segmentDataMatches = rTree.search(feature.getGeometry().getBounds(), - goog.getUid(feature)); - for (var j = segmentDataMatches.length - 1; j >= 0; --j) { - var segmentDataMatch = segmentDataMatches[j]; - rTree.remove(ol.extent.boundingExtent(segmentDataMatch.segment), - segmentDataMatch); - } + var rBush = this.rBush_; + var i, feature, nodesToRemove; + for (i = features.length - 1; i >= 0; --i) { + feature = features[i]; + nodesToRemove = []; + rBush.forEachInExtent(feature.getGeometry().getBounds(), function(node) { + if (feature === node.feature) { + nodesToRemove.push(node); + } + }); + } + for (i = nodesToRemove.length - 1; i >= 0; --i) { + rBush.remove(nodesToRemove[i]); } }; @@ -273,8 +275,7 @@ ol.interaction.Modify.prototype.handleIntentChange_ = function(evt) { */ ol.interaction.Modify.prototype.addSegments_ = function(feature, geometry, layer) { - var uid = goog.getUid(feature); - var rTree = this.rTree_; + var rBush = this.rBush_; var segment, segmentData, coordinates; if (geometry instanceof ol.geom.Point) { coordinates = geometry.getCoordinates(); @@ -284,7 +285,7 @@ ol.interaction.Modify.prototype.addSegments_ = segment: [coordinates, coordinates], style: layer.getStyle() }); - rTree.insert(geometry.getBounds(), segmentData, uid); + rBush.insert(geometry.getBounds(), segmentData); } else if (geometry instanceof ol.geom.LineString || geometry instanceof ol.geom.LinearRing) { coordinates = geometry.getCoordinates(); @@ -297,7 +298,7 @@ ol.interaction.Modify.prototype.addSegments_ = style: layer.getStyle(), segment: segment }); - rTree.insert(ol.extent.boundingExtent(segment), segmentData, uid); + rBush.insert(ol.extent.boundingExtent(segment), segmentData); } } else if (geometry instanceof ol.geom.Polygon) { var rings = geometry.getRings(); @@ -345,13 +346,12 @@ ol.interaction.Modify.prototype.handleDragStart = function(evt) { var insertVertices = []; var vertex = vertexFeature.getGeometry().getCoordinates(); var vertexExtent = ol.extent.boundingExtent([vertex]); - var segmentDataMatches = this.rTree_.search(vertexExtent); var distinctFeatures = {}; - for (var i = 0, ii = segmentDataMatches.length; i < ii; ++i) { - var segmentDataMatch = segmentDataMatches[i]; - var segment = segmentDataMatch.segment; - if (!(goog.getUid(segmentDataMatch.feature) in distinctFeatures)) { - var feature = segmentDataMatch.feature; + var dragSegments = this.dragSegments_; + this.rBush_.forEachInExtent(vertexExtent, function(node) { + var segment = node.segment; + if (!(goog.getUid(node.feature) in distinctFeatures)) { + var feature = node.feature; distinctFeatures[goog.getUid(feature)] = true; var original = new ol.Feature(feature.getAttributes()); original.setGeometry(feature.getGeometry().clone()); @@ -362,16 +362,16 @@ ol.interaction.Modify.prototype.handleDragStart = function(evt) { } if (renderIntent == ol.layer.VectorLayerRenderIntent.TEMPORARY) { if (ol.coordinate.equals(segment[0], vertex)) { - this.dragSegments_.push([segmentDataMatch, 0]); + dragSegments.push([node, 0]); } else if (ol.coordinate.equals(segment[1], vertex)) { - this.dragSegments_.push([segmentDataMatch, 1]); + dragSegments.push([node, 1]); } } else if ( ol.coordinate.squaredDistanceToSegment(vertex, segment) === 0) { - insertVertices.push([segmentDataMatch, vertex]); + insertVertices.push([node, vertex]); } - } - for (i = insertVertices.length - 1; i >= 0; --i) { + }); + for (var i = insertVertices.length - 1; i >= 0; --i) { this.insertVertex_.apply(this, insertVertices[i]); } } @@ -387,12 +387,9 @@ ol.interaction.Modify.prototype.handleDrag = function(evt) { for (var i = 0, ii = this.dragSegments_.length; i < ii; ++i) { var dragSegment = this.dragSegments_[i]; var segmentData = dragSegment[0]; - var feature = segmentData.feature; var geometry = segmentData.geometry; var coordinates = geometry.getCoordinates(); - var segment = segmentData.segment; - var oldBounds = ol.extent.boundingExtent(segment); if (geometry instanceof ol.geom.Point) { coordinates = vertex; segment[0] = segment[1] = vertex; @@ -402,10 +399,20 @@ ol.interaction.Modify.prototype.handleDrag = function(evt) { segment[index] = vertex; } geometry.setCoordinates(coordinates); - var newBounds = ol.extent.boundingExtent(segment); this.createOrUpdateVertexFeature_(segmentData.style, vertex); - this.rTree_.remove(oldBounds, segmentData); - this.rTree_.insert(newBounds, segmentData, goog.getUid(feature)); + } +}; + + +/** + * @inheritDoc + */ +ol.interaction.Modify.prototype.handleDragEnd = function(evt) { + var segmentData; + for (var i = this.dragSegments_.length - 1; i >= 0; --i) { + segmentData = this.dragSegments_[i][0]; + this.rBush_.update(ol.extent.boundingExtent(segmentData.segment), + segmentData); } }; @@ -446,13 +453,13 @@ ol.interaction.Modify.prototype.handleMouseMove_ = function(evt) { this.modifiable_ = false; var vertexFeature = this.vertexFeature_; - var rTree = this.rTree_; - var segmentDataMatches = rTree.search(box); + var rBush = this.rBush_; + var nodes = rBush.allInExtent(box); var renderIntent = ol.layer.VectorLayerRenderIntent.HIDDEN; - if (segmentDataMatches.length > 0) { - segmentDataMatches.sort(sortByDistance); - var segmentDataMatch = segmentDataMatches[0]; - var segment = segmentDataMatch.segment; // the closest segment + if (nodes.length > 0) { + nodes.sort(sortByDistance); + var node = nodes[0]; + var segment = node.segment; // the closest segment var vertex = (ol.coordinate.closestOnSegment(pixelCoordinate, segment)); var vertexPixel = map.getPixelFromCoordinate(vertex); if (Math.sqrt(ol.coordinate.squaredDistance(pixel, vertexPixel)) <= @@ -467,8 +474,7 @@ ol.interaction.Modify.prototype.handleMouseMove_ = function(evt) { vertex = squaredDist1 > squaredDist2 ? segment[1] : segment[0]; renderIntent = ol.layer.VectorLayerRenderIntent.TEMPORARY; } - vertexFeature = this.createOrUpdateVertexFeature_(segmentDataMatch.style, - vertex); + vertexFeature = this.createOrUpdateVertexFeature_(node.style, vertex); this.modifiable_ = true; } } @@ -494,18 +500,14 @@ ol.interaction.Modify.prototype.insertVertex_ = var coordinates = geometry.getCoordinates(); coordinates.splice(index + 1, 0, vertex); geometry.setCoordinates(coordinates); - var rTree = this.rTree_; + var rBush = this.rBush_; goog.asserts.assert(goog.isDef(segment)); - rTree.remove(ol.extent.boundingExtent(segment), segmentData); - var uid = goog.getUid(feature); - var segmentDataMatches = this.rTree_.search(geometry.getBounds(), uid); - for (var i = 0, ii = segmentDataMatches.length; i < ii; ++i) { - var segmentDataMatch = segmentDataMatches[i]; - if (segmentDataMatch.geometry === geometry && - segmentDataMatch.index > index) { - ++segmentDataMatch.index; + rBush.remove(segmentData); + this.rBush_.forEachInExtent(geometry.getBounds(), function(node) { + if (node.geometry === geometry && node.index > index) { + ++node.index; } - } + }); var newSegmentData = /** @type {ol.interaction.SegmentDataType} */ ({ style: segmentData.style, segment: [segment[0], vertex], @@ -513,13 +515,17 @@ ol.interaction.Modify.prototype.insertVertex_ = geometry: geometry, index: index }); - rTree.insert(ol.extent.boundingExtent(newSegmentData.segment), newSegmentData, - uid); + rBush.insert(ol.extent.boundingExtent(newSegmentData.segment), + newSegmentData); this.dragSegments_.push([newSegmentData, 1]); - newSegmentData = goog.object.clone(newSegmentData); - newSegmentData.segment = [vertex, segment[1]]; - newSegmentData.index += 1; - rTree.insert(ol.extent.boundingExtent(newSegmentData.segment), newSegmentData, - uid); + newSegmentData = /** @type {ol.interaction.SegmentDataType} */ ({ + style: segmentData.style, + segment: [vertex, segment[1]], + feature: feature, + geometry: geometry, + index: index + 1 + }); + rBush.insert(ol.extent.boundingExtent(newSegmentData.segment), + newSegmentData); this.dragSegments_.push([newSegmentData, 0]); }; From 665781ee03af9dfd32a85f5b2dea27cebfa3f8bd Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Wed, 27 Nov 2013 14:44:05 +0100 Subject: [PATCH 3/8] Throw an exception if an ol.structs.RBush is modified while reading --- src/ol/structs/rbush.js | 41 +++++++++++++++++++++++- test/spec/ol/structs/rbush.js | 60 +++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/ol/structs/rbush.js b/src/ol/structs/rbush.js index dc6d3dfb97..36d86329b7 100644 --- a/src/ol/structs/rbush.js +++ b/src/ol/structs/rbush.js @@ -189,6 +189,12 @@ ol.structs.RBush = function(opt_maxEntries) { */ this.valueExtent_ = {}; + /** + * @private + * @type {number} + */ + this.readers_ = 0; + }; @@ -387,7 +393,12 @@ ol.structs.RBush.prototype.condense_ = function(path) { * @template S */ ol.structs.RBush.prototype.forEach = function(callback, opt_obj) { - return this.forEach_(this.root_, callback, opt_obj); + ++this.readers_; + try { + return this.forEach_(this.root_, callback, opt_obj); + } finally { + --this.readers_; + } }; @@ -432,6 +443,25 @@ ol.structs.RBush.prototype.forEach_ = function(node, callback, opt_obj) { */ ol.structs.RBush.prototype.forEachInExtent = function(extent, callback, opt_obj) { + ++this.readers_; + try { + return this.forEachInExtent_(extent, callback, opt_obj); + } finally { + --this.readers_; + } +}; + + +/** + * @param {ol.Extent} extent Extent. + * @param {function(this: S, T): *} callback Callback. + * @param {S=} opt_obj Scope. + * @private + * @return {*} Callback return value. + * @template S + */ +ol.structs.RBush.prototype.forEachInExtent_ = + function(extent, callback, opt_obj) { /** @type {Array.>} */ var toVisit = [this.root_]; var result; @@ -496,6 +526,9 @@ ol.structs.RBush.prototype.getKey_ = function(value) { * @param {T} value Value. */ ol.structs.RBush.prototype.insert = function(extent, value) { + if (this.readers_) { + throw Error('cannot insert value while reading'); + } var key = this.getKey_(value); goog.asserts.assert(!this.valueExtent_.hasOwnProperty(key)); this.insert_(extent, value, this.root_.height - 1); @@ -535,6 +568,9 @@ ol.structs.RBush.prototype.insert_ = function(extent, value, level) { * @param {T} value Value. */ ol.structs.RBush.prototype.remove = function(value) { + if (this.readers_) { + throw Error('cannot remove value while reading'); + } var key = this.getKey_(value); goog.asserts.assert(this.valueExtent_.hasOwnProperty(key)); var extent = this.valueExtent_[key]; @@ -629,6 +665,9 @@ ol.structs.RBush.prototype.splitRoot_ = function(node1, node2) { * @param {T} value Value. */ ol.structs.RBush.prototype.update = function(extent, value) { + if (this.readers_) { + throw Error('cannot update value while reading'); + } var key = this.getKey_(value); var currentExtent = this.valueExtent_[key]; goog.asserts.assert(goog.isDef(currentExtent)); diff --git a/test/spec/ol/structs/rbush.js b/test/spec/ol/structs/rbush.js index 7549662134..473126ac09 100644 --- a/test/spec/ol/structs/rbush.js +++ b/test/spec/ol/structs/rbush.js @@ -55,6 +55,27 @@ describe('ol.structs.RBush', function() { }); + describe('#insert', function() { + + it('throws an exception if called while iterating over all values', + function() { + expect(function() { + rBush.forEach(function(value) { + rBush.insert([0, 0, 1, 1], {}); + }); + }).to.throwException(); + }); + + it('throws an exception if called while iterating over an extent', + function() { + expect(function() { + rBush.forEachInExtent([-10, -10, 10, 10], function(value) { + rBush.insert([0, 0, 1, 1], {}); + }); + }).to.throwException(); + }); + }); + describe('#remove', function() { it('can remove each object', function() { @@ -66,6 +87,45 @@ describe('ol.structs.RBush', function() { } }); + it('throws an exception if called while iterating over all values', + function() { + expect(function() { + rBush.forEach(function(value) { + rBush.remove(value); + }); + }).to.throwException(); + }); + + it('throws an exception if called while iterating over an extent', + function() { + expect(function() { + rBush.forEachInExtent([-10, -10, 10, 10], function(value) { + rBush.remove(value); + }); + }).to.throwException(); + }); + + }); + + describe('#update', function() { + + it('throws an exception if called while iterating over all values', + function() { + expect(function() { + rBush.forEach(function(value) { + rBush.update([0, 0, 1, 1], objs[1]); + }); + }).to.throwException(); + }); + + it('throws an exception if called while iterating over an extent', + function() { + expect(function() { + rBush.forEachInExtent([-10, -10, 10, 10], function(value) { + rBush.update([0, 0, 1, 1], objs[1]); + }); + }).to.throwException(); + }); }); }); From 978041b68c89e63f73a907d8333b4d6e3e083e48 Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Wed, 27 Nov 2013 14:54:14 +0100 Subject: [PATCH 4/8] Only activate ol.structs.RBush conflict checks when goog.DEBUG is true --- src/ol/structs/rbush.js | 42 +++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/ol/structs/rbush.js b/src/ol/structs/rbush.js index 36d86329b7..63bba6bfdd 100644 --- a/src/ol/structs/rbush.js +++ b/src/ol/structs/rbush.js @@ -189,11 +189,13 @@ ol.structs.RBush = function(opt_maxEntries) { */ this.valueExtent_ = {}; - /** - * @private - * @type {number} - */ - this.readers_ = 0; + if (goog.DEBUG) { + /** + * @private + * @type {number} + */ + this.readers_ = 0; + } }; @@ -393,11 +395,15 @@ ol.structs.RBush.prototype.condense_ = function(path) { * @template S */ ol.structs.RBush.prototype.forEach = function(callback, opt_obj) { - ++this.readers_; - try { + if (goog.DEBUG) { + ++this.readers_; + try { + return this.forEach_(this.root_, callback, opt_obj); + } finally { + --this.readers_; + } + } else { return this.forEach_(this.root_, callback, opt_obj); - } finally { - --this.readers_; } }; @@ -443,11 +449,15 @@ ol.structs.RBush.prototype.forEach_ = function(node, callback, opt_obj) { */ ol.structs.RBush.prototype.forEachInExtent = function(extent, callback, opt_obj) { - ++this.readers_; - try { + if (goog.DEBUG) { + ++this.readers_; + try { + return this.forEachInExtent_(extent, callback, opt_obj); + } finally { + --this.readers_; + } + } else { return this.forEachInExtent_(extent, callback, opt_obj); - } finally { - --this.readers_; } }; @@ -526,7 +536,7 @@ ol.structs.RBush.prototype.getKey_ = function(value) { * @param {T} value Value. */ ol.structs.RBush.prototype.insert = function(extent, value) { - if (this.readers_) { + if (goog.DEBUG && this.readers_) { throw Error('cannot insert value while reading'); } var key = this.getKey_(value); @@ -568,7 +578,7 @@ ol.structs.RBush.prototype.insert_ = function(extent, value, level) { * @param {T} value Value. */ ol.structs.RBush.prototype.remove = function(value) { - if (this.readers_) { + if (goog.DEBUG && this.readers_) { throw Error('cannot remove value while reading'); } var key = this.getKey_(value); @@ -665,7 +675,7 @@ ol.structs.RBush.prototype.splitRoot_ = function(node1, node2) { * @param {T} value Value. */ ol.structs.RBush.prototype.update = function(extent, value) { - if (this.readers_) { + if (goog.DEBUG && this.readers_) { throw Error('cannot update value while reading'); } var key = this.getKey_(value); From 828456d18e86130a2ef74e62c79c7bda43a8a521 Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Wed, 27 Nov 2013 14:59:24 +0100 Subject: [PATCH 5/8] Rename ol.structs.RBush#all to getAll --- src/ol/structs/rbush.js | 32 ++++++++++++++++---------------- test/spec/ol/structs/rbush.js | 16 ++++++++-------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/ol/structs/rbush.js b/src/ol/structs/rbush.js index 63bba6bfdd..2ec4a192da 100644 --- a/src/ol/structs/rbush.js +++ b/src/ol/structs/rbush.js @@ -200,22 +200,6 @@ ol.structs.RBush = function(opt_maxEntries) { }; -/** - * @return {Array.} All. - */ -ol.structs.RBush.prototype.all = function() { - var values = []; - this.forEach( - /** - * @param {T} value Value. - */ - function(value) { - values.push(value); - }); - return values; -}; - - /** * @param {ol.structs.RBushNode.} node Node. * @param {function(ol.structs.RBushNode., ol.structs.RBushNode.): number} @@ -520,6 +504,22 @@ ol.structs.RBush.prototype.forEachNode = function(callback, opt_obj) { }; +/** + * @return {Array.} All. + */ +ol.structs.RBush.prototype.getAll = function() { + var values = []; + this.forEach( + /** + * @param {T} value Value. + */ + function(value) { + values.push(value); + }); + return values; +}; + + /** * @param {T} value Value. * @private diff --git a/test/spec/ol/structs/rbush.js b/test/spec/ol/structs/rbush.js index 473126ac09..579d4e1c54 100644 --- a/test/spec/ol/structs/rbush.js +++ b/test/spec/ol/structs/rbush.js @@ -10,10 +10,10 @@ describe('ol.structs.RBush', function() { describe('when empty', function() { - describe('#all', function() { + describe('#getAll', function() { it('returns the expected number of objects', function() { - expect(rBush.all()).to.be.empty(); + expect(rBush.getAll()).to.be.empty(); }); }); @@ -81,9 +81,9 @@ describe('ol.structs.RBush', function() { it('can remove each object', function() { var i, ii; for (i = 0, ii = objs.length; i < ii; ++i) { - expect(rBush.all()).to.contain(objs[i]); + expect(rBush.getAll()).to.contain(objs[i]); rBush.remove(objs[i]); - expect(rBush.all()).not.to.contain(objs[i]); + expect(rBush.getAll()).not.to.contain(objs[i]); } }); @@ -164,7 +164,7 @@ describe('ol.structs.RBush', function() { rBush.remove(objs[i]); expect(rBush.allInExtent(extents[i])).to.be.empty(); } - expect(rBush.all()).to.be.empty(); + expect(rBush.getAll()).to.be.empty(); }); it('can remove objects in random order', function() { @@ -182,7 +182,7 @@ describe('ol.structs.RBush', function() { rBush.remove(objs[index]); expect(rBush.allInExtent(extents[index])).to.be.empty(); } - expect(rBush.all()).to.be.empty(); + expect(rBush.getAll()).to.be.empty(); }); }); @@ -201,10 +201,10 @@ describe('ol.structs.RBush', function() { } }); - describe('#all', function() { + describe('#getAll', function() { it('returns the expected number of objects', function() { - expect(rBush.all().length).to.be(1000); + expect(rBush.getAll().length).to.be(1000); }); }); From f847b372619076698ae15cecf99b6cfa0febe27c Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Wed, 27 Nov 2013 14:59:46 +0100 Subject: [PATCH 6/8] Rename ol.structs.RBush#allInExtent to getAllInExtent --- src/ol/structs/rbush.js | 34 +++++++++++++++++----------------- test/spec/ol/structs/rbush.js | 30 +++++++++++++++--------------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/ol/structs/rbush.js b/src/ol/structs/rbush.js index 2ec4a192da..8dd8af4960 100644 --- a/src/ol/structs/rbush.js +++ b/src/ol/structs/rbush.js @@ -229,23 +229,6 @@ ol.structs.RBush.prototype.allDistMargin_ = function(node, compare) { }; -/** - * @param {ol.Extent} extent Extent. - * @return {Array.} All in extent. - */ -ol.structs.RBush.prototype.allInExtent = function(extent) { - var values = []; - this.forEachInExtent(extent, - /** - * @param {T} value Value. - */ - function(value) { - values.push(value); - }); - return values; -}; - - /** * FIXME empty description for jsdoc */ @@ -520,6 +503,23 @@ ol.structs.RBush.prototype.getAll = function() { }; +/** + * @param {ol.Extent} extent Extent. + * @return {Array.} All in extent. + */ +ol.structs.RBush.prototype.getAllInExtent = function(extent) { + var values = []; + this.forEachInExtent(extent, + /** + * @param {T} value Value. + */ + function(value) { + values.push(value); + }); + return values; +}; + + /** * @param {T} value Value. * @private diff --git a/test/spec/ol/structs/rbush.js b/test/spec/ol/structs/rbush.js index 579d4e1c54..cdf6b7167d 100644 --- a/test/spec/ol/structs/rbush.js +++ b/test/spec/ol/structs/rbush.js @@ -33,15 +33,15 @@ describe('ol.structs.RBush', function() { rBush.insert([-3, -3, -2, -2], objs[5]); }); - describe('#allInExtent', function() { + describe('#getAllInExtent', function() { it('returns the expected objects', function() { var result; - result = rBush.allInExtent([2, 2, 3, 3]); + result = rBush.getAllInExtent([2, 2, 3, 3]); expect(result).to.contain(objs[1]); expect(result).to.contain(objs[2]); expect(result.length).to.be(2); - result = rBush.allInExtent([-1, -1, 2, 2]); + result = rBush.getAllInExtent([-1, -1, 2, 2]); expect(result).to.contain(objs[0]); expect(result).to.contain(objs[1]); expect(result).to.contain(objs[2]); @@ -50,7 +50,7 @@ describe('ol.structs.RBush', function() { }); it('returns an empty array when given a disjoint extent', function() { - expect(rBush.allInExtent([5, 5, 6, 6]).length).to.be(0); + expect(rBush.getAllInExtent([5, 5, 6, 6]).length).to.be(0); }); }); @@ -144,12 +144,12 @@ describe('ol.structs.RBush', function() { } }); - describe('#allInExtent', function() { + describe('#getAllInExtent', function() { it('returns the expected objects', function() { var i, ii; for (i = 0, ii = objs.length; i < ii; ++i) { - expect(rBush.allInExtent(extents[i])).to.eql([objs[i]]); + expect(rBush.getAllInExtent(extents[i])).to.eql([objs[i]]); } }); @@ -160,9 +160,9 @@ describe('ol.structs.RBush', function() { it('can remove each object in turn', function() { var i, ii; for (i = 0, ii = objs.length; i < ii; ++i) { - expect(rBush.allInExtent(extents[i])).to.eql([objs[i]]); + expect(rBush.getAllInExtent(extents[i])).to.eql([objs[i]]); rBush.remove(objs[i]); - expect(rBush.allInExtent(extents[i])).to.be.empty(); + expect(rBush.getAllInExtent(extents[i])).to.be.empty(); } expect(rBush.getAll()).to.be.empty(); }); @@ -178,9 +178,9 @@ describe('ol.structs.RBush', function() { } for (i = 0, ii = objs.length; i < ii; ++i) { var index = indexes[i]; - expect(rBush.allInExtent(extents[index])).to.eql([objs[index]]); + expect(rBush.getAllInExtent(extents[index])).to.eql([objs[index]]); rBush.remove(objs[index]); - expect(rBush.allInExtent(extents[index])).to.be.empty(); + expect(rBush.getAllInExtent(extents[index])).to.be.empty(); } expect(rBush.getAll()).to.be.empty(); }); @@ -209,10 +209,10 @@ describe('ol.structs.RBush', function() { }); - describe('#allInExtent', function() { + describe('#getAllInExtent', function() { it('returns the expected number of objects', function() { - expect(rBush.allInExtent([0, 0, 10600, 10600]).length).to.be(1000); + expect(rBush.getAllInExtent([0, 0, 10600, 10600]).length).to.be(1000); }); it('can perform 1000 in-extent searches', function() { @@ -223,7 +223,7 @@ describe('ol.structs.RBush', function() { var max = [min[0] + Math.random() * 500, min[1] + Math.random() * 500]; var extent = [min[0], min[1], max[0], max[1]]; - n += rBush.allInExtent(extent).length; + n += rBush.getAllInExtent(extent).length; } expect(n).not.to.be(0); }); @@ -237,7 +237,7 @@ describe('ol.structs.RBush', function() { var max = [min[0] + Math.random() * 500, min[1] + Math.random() * 500]; var extent = [min[0], min[1], max[0], max[1]]; - n += rBush.allInExtent(extent).length; + n += rBush.getAllInExtent(extent).length; } expect(n).to.be(0); }); @@ -255,7 +255,7 @@ describe('ol.structs.RBush', function() { var extent = [min[0], min[1], max[0], max[1]]; rBush.insert(extent, {id: i}); } - expect(rBush.allInExtent([0, 0, 10600, 10600]).length).to.be(2000); + expect(rBush.getAllInExtent([0, 0, 10600, 10600]).length).to.be(2000); }); }); From 5d3a5ae68efce926994b309e710503abe9f28d4f Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Wed, 27 Nov 2013 15:07:36 +0100 Subject: [PATCH 7/8] Fix infinite loop bug in ol.structs.RBush --- src/ol/structs/rbush.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ol/structs/rbush.js b/src/ol/structs/rbush.js index 8dd8af4960..549c3be3da 100644 --- a/src/ol/structs/rbush.js +++ b/src/ol/structs/rbush.js @@ -614,7 +614,8 @@ ol.structs.RBush.prototype.remove_ = function(extent, value) { return; } } - ++index; + node = path.pop(); + index = indexes.pop(); } else if (index < node.children.length) { child = node.children[index]; if (ol.extent.containsExtent(child.extent, extent)) { From c00d748384715af634ca5ec7ec137d5399afab62 Mon Sep 17 00:00:00 2001 From: Tom Payne Date: Wed, 27 Nov 2013 15:11:42 +0100 Subject: [PATCH 8/8] Update ol.interaction.Modify to use ol.structs.RTree#getAllInExtent --- src/ol/interaction/modifyinteraction.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol/interaction/modifyinteraction.js b/src/ol/interaction/modifyinteraction.js index 6854a49856..e473be2f9c 100644 --- a/src/ol/interaction/modifyinteraction.js +++ b/src/ol/interaction/modifyinteraction.js @@ -454,7 +454,7 @@ ol.interaction.Modify.prototype.handleMouseMove_ = function(evt) { this.modifiable_ = false; var vertexFeature = this.vertexFeature_; var rBush = this.rBush_; - var nodes = rBush.allInExtent(box); + var nodes = rBush.getAllInExtent(box); var renderIntent = ol.layer.VectorLayerRenderIntent.HIDDEN; if (nodes.length > 0) { nodes.sort(sortByDistance);