From 4d03c0bfaa29a9851a746e380fe5bdaffdf3d1c9 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Thu, 12 Dec 2013 17:03:40 +0100 Subject: [PATCH 1/4] Show an issue with ol.structs.RBush Note that the same test passes in the original implementation. --- test/spec/ol/structs/rbush.test.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/spec/ol/structs/rbush.test.js b/test/spec/ol/structs/rbush.test.js index 250aa374b5..757a6b287f 100644 --- a/test/spec/ol/structs/rbush.test.js +++ b/test/spec/ol/structs/rbush.test.js @@ -32,13 +32,18 @@ describe('ol.structs.RBush', function() { var objs; beforeEach(function() { - objs = [{}, {}, {}, {}, {}, {}]; + objs = [{}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {}]; rBush.insert([0, 0, 1, 1], objs[0]); rBush.insert([1, 1, 4, 4], objs[1]); rBush.insert([2, 2, 3, 3], objs[2]); rBush.insert([-5, -5, -4, -4], objs[3]); rBush.insert([-4, -4, -1, -1], objs[4]); rBush.insert([-3, -3, -2, -2], objs[5]); + rBush.insert([-3, -3, -2, -2], objs[6]); + rBush.insert([-3, -3, -2, -2], objs[7]); + rBush.insert([-3, -3, -2, -2], objs[8]); + rBush.insert([-3, -3, -2, -2], objs[9]); + rBush.insert([-3, -3, -2, -2], objs[10]); }); describe('#getAllInExtent', function() { From d3cc822f98f7bf8e959f347de92bdb851ddd9e25 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Fri, 13 Dec 2013 17:30:28 +0100 Subject: [PATCH 2/4] Do not ascend when node has more siblings --- src/ol/structs/rbush.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/ol/structs/rbush.js b/src/ol/structs/rbush.js index e0bdc81819..fe8c8e4153 100644 --- a/src/ol/structs/rbush.js +++ b/src/ol/structs/rbush.js @@ -609,8 +609,9 @@ ol.structs.RBush.prototype.remove_ = function(extent, value) { var path = [node]; /** @type {Array.} */ var indexes = [0]; - var child, children, i, ii; + var childrenDone, child, children, i, ii; while (path.length > 0) { + childrenDone = false; goog.asserts.assert(node.height > 0); if (node.height == 1) { children = node.children; @@ -622,8 +623,7 @@ ol.structs.RBush.prototype.remove_ = function(extent, value) { return; } } - node = path.pop(); - index = indexes.pop(); + childrenDone = true; } else if (index < node.children.length) { child = node.children[index]; if (ol.extent.containsExtent(child.extent, extent)) { @@ -635,8 +635,16 @@ ol.structs.RBush.prototype.remove_ = function(extent, value) { ++index; } } else { - node = path.pop(); - index = indexes.pop(); + childrenDone = true; + } + if (childrenDone === true) { + var lastPathIndex = path.length - 1; + node = path[lastPathIndex]; + index = ++indexes[lastPathIndex]; + if (index > node.children.length) { + path.pop(); + indexes.pop(); + } } } }; From 67d2cddb846ef660a8fef58bfbd3b8526788464b Mon Sep 17 00:00:00 2001 From: ahocevar Date: Fri, 13 Dec 2013 18:27:33 +0100 Subject: [PATCH 3/4] Verify that removing random extent nodes also works --- test/spec/ol/structs/rbush.test.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/spec/ol/structs/rbush.test.js b/test/spec/ol/structs/rbush.test.js index 757a6b287f..044811e6eb 100644 --- a/test/spec/ol/structs/rbush.test.js +++ b/test/spec/ol/structs/rbush.test.js @@ -299,6 +299,20 @@ describe('ol.structs.RBush', function() { }); + describe('#remove', function() { + + it('can remove all 2000 objects', function() { + var objs = rBush.getAll(); + var i, value; + for (i = objs.length - 1; i >= 0; --i) { + value = objs[i]; + rBush.remove(value); + } + expect(rBush.isEmpty()).to.be(true); + }); + + }); + }); }); From 8bfa0f7ae94f1e4e3ad59cae4682c5e6c4b39bc7 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Mon, 16 Dec 2013 13:53:55 +0100 Subject: [PATCH 4/4] Truthy check is enough --- src/ol/structs/rbush.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol/structs/rbush.js b/src/ol/structs/rbush.js index fe8c8e4153..2072aaf96b 100644 --- a/src/ol/structs/rbush.js +++ b/src/ol/structs/rbush.js @@ -637,7 +637,7 @@ ol.structs.RBush.prototype.remove_ = function(extent, value) { } else { childrenDone = true; } - if (childrenDone === true) { + if (childrenDone) { var lastPathIndex = path.length - 1; node = path[lastPathIndex]; index = ++indexes[lastPathIndex];