From 6866f063753ad3ff7fba3c7e0f8cfd73c98de986 Mon Sep 17 00:00:00 2001 From: Simon Seyock Date: Tue, 24 Sep 2019 16:55:33 +0200 Subject: [PATCH 1/2] Remove simplified geometry cache --- src/ol/geom/Geometry.js | 6 ----- src/ol/geom/GeometryCollection.js | 45 +++++++++++++------------------ src/ol/geom/SimpleGeometry.js | 37 +++++++++++-------------- 3 files changed, 34 insertions(+), 54 deletions(-) diff --git a/src/ol/geom/Geometry.js b/src/ol/geom/Geometry.js index df5d7e2285..4a6d5956cc 100644 --- a/src/ol/geom/Geometry.js +++ b/src/ol/geom/Geometry.js @@ -45,12 +45,6 @@ class Geometry extends BaseObject { */ this.extentRevision_ = -1; - /** - * @protected - * @type {Object} - */ - this.simplifiedGeometryCache = {}; - /** * @protected * @type {number} diff --git a/src/ol/geom/GeometryCollection.js b/src/ol/geom/GeometryCollection.js index eec00aa9aa..e1b63d76dd 100644 --- a/src/ol/geom/GeometryCollection.js +++ b/src/ol/geom/GeometryCollection.js @@ -6,7 +6,6 @@ import EventType from '../events/EventType.js'; import {createOrUpdateEmpty, closestSquaredDistanceXY, extend, getCenter} from '../extent.js'; import Geometry from './Geometry.js'; import GeometryType from './GeometryType.js'; -import {clear} from '../obj.js'; /** * @classdesc @@ -131,8 +130,7 @@ class GeometryCollection extends Geometry { * @inheritDoc */ getSimplifiedGeometry(squaredTolerance) { - if (this.simplifiedGeometryRevision != this.getRevision()) { - clear(this.simplifiedGeometryCache); + if (this.simplifiedGeometryRevision !== this.getRevision()) { this.simplifiedGeometryMaxMinSquaredTolerance = 0; this.simplifiedGeometryRevision = this.getRevision(); } @@ -141,30 +139,25 @@ class GeometryCollection extends Geometry { squaredTolerance < this.simplifiedGeometryMaxMinSquaredTolerance)) { return this; } - const key = squaredTolerance.toString(); - if (this.simplifiedGeometryCache.hasOwnProperty(key)) { - return this.simplifiedGeometryCache[key]; + + const simplifiedGeometries = []; + const geometries = this.geometries_; + let simplified = false; + for (let i = 0, ii = geometries.length; i < ii; ++i) { + const geometry = geometries[i]; + const simplifiedGeometry = geometry.getSimplifiedGeometry(squaredTolerance); + simplifiedGeometries.push(simplifiedGeometry); + if (simplifiedGeometry !== geometry) { + simplified = true; + } + } + if (simplified) { + const simplifiedGeometryCollection = new GeometryCollection(null); + simplifiedGeometryCollection.setGeometriesArray(simplifiedGeometries); + return simplifiedGeometryCollection; } else { - const simplifiedGeometries = []; - const geometries = this.geometries_; - let simplified = false; - for (let i = 0, ii = geometries.length; i < ii; ++i) { - const geometry = geometries[i]; - const simplifiedGeometry = geometry.getSimplifiedGeometry(squaredTolerance); - simplifiedGeometries.push(simplifiedGeometry); - if (simplifiedGeometry !== geometry) { - simplified = true; - } - } - if (simplified) { - const simplifiedGeometryCollection = new GeometryCollection(null); - simplifiedGeometryCollection.setGeometriesArray(simplifiedGeometries); - this.simplifiedGeometryCache[key] = simplifiedGeometryCollection; - return simplifiedGeometryCollection; - } else { - this.simplifiedGeometryMaxMinSquaredTolerance = squaredTolerance; - return this; - } + this.simplifiedGeometryMaxMinSquaredTolerance = squaredTolerance; + return this; } } diff --git a/src/ol/geom/SimpleGeometry.js b/src/ol/geom/SimpleGeometry.js index ebc847f3d3..6f7de81427 100644 --- a/src/ol/geom/SimpleGeometry.js +++ b/src/ol/geom/SimpleGeometry.js @@ -6,7 +6,6 @@ import {createOrUpdateFromFlatCoordinates, getCenter} from '../extent.js'; import Geometry from './Geometry.js'; import GeometryLayout from './GeometryLayout.js'; import {rotate, scale, translate, transform2D} from './flat/transform.js'; -import {clear} from '../obj.js'; /** * @classdesc @@ -95,8 +94,7 @@ class SimpleGeometry extends Geometry { * @inheritDoc */ getSimplifiedGeometry(squaredTolerance) { - if (this.simplifiedGeometryRevision != this.getRevision()) { - clear(this.simplifiedGeometryCache); + if (this.simplifiedGeometryRevision !== this.getRevision()) { this.simplifiedGeometryMaxMinSquaredTolerance = 0; this.simplifiedGeometryRevision = this.getRevision(); } @@ -107,26 +105,21 @@ class SimpleGeometry extends Geometry { squaredTolerance <= this.simplifiedGeometryMaxMinSquaredTolerance)) { return this; } - const key = squaredTolerance.toString(); - if (this.simplifiedGeometryCache.hasOwnProperty(key)) { - return this.simplifiedGeometryCache[key]; + + const simplifiedGeometry = + this.getSimplifiedGeometryInternal(squaredTolerance); + const simplifiedFlatCoordinates = simplifiedGeometry.getFlatCoordinates(); + if (simplifiedFlatCoordinates.length < this.flatCoordinates.length) { + return simplifiedGeometry; } else { - const simplifiedGeometry = - this.getSimplifiedGeometryInternal(squaredTolerance); - const simplifiedFlatCoordinates = simplifiedGeometry.getFlatCoordinates(); - if (simplifiedFlatCoordinates.length < this.flatCoordinates.length) { - this.simplifiedGeometryCache[key] = simplifiedGeometry; - return simplifiedGeometry; - } else { - // Simplification did not actually remove any coordinates. We now know - // that any calls to getSimplifiedGeometry with a squaredTolerance less - // than or equal to the current squaredTolerance will also not have any - // effect. This allows us to short circuit simplification (saving CPU - // cycles) and prevents the cache of simplified geometries from filling - // up with useless identical copies of this geometry (saving memory). - this.simplifiedGeometryMaxMinSquaredTolerance = squaredTolerance; - return this; - } + // Simplification did not actually remove any coordinates. We now know + // that any calls to getSimplifiedGeometry with a squaredTolerance less + // than or equal to the current squaredTolerance will also not have any + // effect. This allows us to short circuit simplification (saving CPU + // cycles) and prevents the cache of simplified geometries from filling + // up with useless identical copies of this geometry (saving memory). + this.simplifiedGeometryMaxMinSquaredTolerance = squaredTolerance; + return this; } } From 04738f494f177b5943d37d8c518e13682abc1a6e Mon Sep 17 00:00:00 2001 From: Simon Seyock Date: Tue, 24 Sep 2019 17:47:32 +0200 Subject: [PATCH 2/2] Removed tests for simplified geometry cache --- test/spec/ol/geom/linestring.test.js | 13 ------------- test/spec/ol/geom/polygon.test.js | 9 --------- 2 files changed, 22 deletions(-) diff --git a/test/spec/ol/geom/linestring.test.js b/test/spec/ol/geom/linestring.test.js index a4c7892bca..8c7b73d897 100644 --- a/test/spec/ol/geom/linestring.test.js +++ b/test/spec/ol/geom/linestring.test.js @@ -336,19 +336,6 @@ describe('ol.geom.LineString', function() { [[0, 0], [3, 3], [5, 1], [7, 5]]); }); - it('caches by resolution', function() { - const simplifiedGeometry1 = lineString.getSimplifiedGeometry(1); - const simplifiedGeometry2 = lineString.getSimplifiedGeometry(1); - expect(simplifiedGeometry1).to.be(simplifiedGeometry2); - }); - - it('invalidates the cache when the geometry changes', function() { - const simplifiedGeometry1 = lineString.getSimplifiedGeometry(1); - lineString.setCoordinates(lineString.getCoordinates()); - const simplifiedGeometry2 = lineString.getSimplifiedGeometry(1); - expect(simplifiedGeometry1).not.to.be(simplifiedGeometry2); - }); - it('remembers the minimum squared tolerance', function() { sinon.spy(lineString, 'getSimplifiedGeometryInternal'); const simplifiedGeometry1 = lineString.getSimplifiedGeometry(0.05); diff --git a/test/spec/ol/geom/polygon.test.js b/test/spec/ol/geom/polygon.test.js index 2fee739827..ebcb0839c3 100644 --- a/test/spec/ol/geom/polygon.test.js +++ b/test/spec/ol/geom/polygon.test.js @@ -496,15 +496,6 @@ describe('ol/geom/Polygon', function() { [[[3, 0], [0, 3], [0, 6], [6, 6], [3, 3]]]); }); - it('caches multiple simplified geometries', function() { - const simplifiedGeometry1 = polygon.getSimplifiedGeometry(4); - const simplifiedGeometry2 = polygon.getSimplifiedGeometry(9); - const simplifiedGeometry3 = polygon.getSimplifiedGeometry(4); - const simplifiedGeometry4 = polygon.getSimplifiedGeometry(9); - expect(simplifiedGeometry1).to.be(simplifiedGeometry3); - expect(simplifiedGeometry2).to.be(simplifiedGeometry4); - }); - }); });