From 626a319222fe958c95138b0dcf1596569a46b73f Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Fri, 27 Sep 2013 15:43:54 +0200 Subject: [PATCH 1/7] Accessor for polygon rings --- src/ol/geom/polygon.js | 27 +++++++---- src/ol/parser/gpxparser.js | 7 +-- src/ol/parser/wktparser.js | 5 ++- .../renderer/canvas/canvasvectorrenderer.js | 2 +- test/spec/ol/geom/polygon.test.js | 18 ++++---- test/spec/ol/parser/geojson.test.js | 9 ++-- test/spec/ol/parser/wkt.test.js | 45 ++++++++++--------- 7 files changed, 65 insertions(+), 48 deletions(-) diff --git a/src/ol/geom/polygon.js b/src/ol/geom/polygon.js index b1ec7d68a2..fd0131fb59 100644 --- a/src/ol/geom/polygon.js +++ b/src/ol/geom/polygon.js @@ -35,8 +35,9 @@ ol.geom.Polygon = function(coordinates) { /** * @type {Array.} + * @private */ - this.rings = new Array(numRings); + this.rings_ = new Array(numRings); var ringCoords; for (var i = 0; i < numRings; ++i) { ringCoords = coordinates[i]; @@ -51,7 +52,7 @@ ol.geom.Polygon = function(coordinates) { ringCoords.reverse(); } } - this.rings[i] = new ol.geom.LinearRing(ringCoords); + this.rings_[i] = new ol.geom.LinearRing(ringCoords); } }; @@ -62,7 +63,7 @@ goog.inherits(ol.geom.Polygon, ol.geom.Geometry); * @inheritDoc */ ol.geom.Polygon.prototype.getBounds = function() { - return this.rings[0].getBounds(); + return this.rings_[0].getBounds(); }; @@ -70,10 +71,10 @@ ol.geom.Polygon.prototype.getBounds = function() { * @return {Array.} Coordinates array. */ ol.geom.Polygon.prototype.getCoordinates = function() { - var count = this.rings.length; + var count = this.rings_.length; var coordinates = new Array(count); for (var i = 0; i < count; ++i) { - coordinates[i] = this.rings[i].getCoordinates(); + coordinates[i] = this.rings_[i].getCoordinates(); } return coordinates; }; @@ -87,6 +88,16 @@ ol.geom.Polygon.prototype.getType = function() { }; +/** + * Get polygon rings. + * @return {Array.} Array of rings. The first ring is the + * exterior and any additional rings are interior. + */ +ol.geom.Polygon.prototype.getRings = function() { + return this.rings_; +}; + + /** * Check whether a given coordinate is inside this polygon. Note that this is a * fast and simple check - points on an edge or vertex of the polygon or one of @@ -96,7 +107,7 @@ ol.geom.Polygon.prototype.getType = function() { * @return {boolean} Whether the coordinate is inside the polygon. */ ol.geom.Polygon.prototype.containsCoordinate = function(coordinate) { - var rings = this.rings; + var rings = this.rings_; /** @type {boolean} */ var containsCoordinate; for (var i = 0, ii = rings.length; i < ii; ++i) { @@ -122,7 +133,7 @@ ol.geom.Polygon.prototype.getInteriorPoint = function() { if (goog.isNull(this.labelPoint_)) { var center = ol.extent.getCenter(this.getBounds()), resultY = center[1], - vertices = this.rings[0].getCoordinates(), + vertices = this.rings_[0].getCoordinates(), intersections = [], maxLength = 0, i, vertex1, vertex2, x, segmentLength, resultX; @@ -163,7 +174,7 @@ ol.geom.Polygon.prototype.getInteriorPoint = function() { * @inheritDoc */ ol.geom.Polygon.prototype.transform = function(transform) { - var rings = this.rings; + var rings = this.rings_; for (var i = 0, ii = rings.length; i < ii; ++i) { rings[i].transform(transform); } diff --git a/src/ol/parser/gpxparser.js b/src/ol/parser/gpxparser.js index 9648360d87..18cc711e05 100644 --- a/src/ol/parser/gpxparser.js +++ b/src/ol/parser/gpxparser.js @@ -155,7 +155,7 @@ ol.parser.GPX = function(opt_options) { var desc = attributes['description'] || this.defaultDesc; this.writeNode('desc', desc, undefined, node); var geom = feature.getGeometry(); - var i, ii; + var i, ii, rings; if (geom instanceof ol.geom.LineString) { this.writeNode('trkseg', feature.getGeometry(), undefined, node); } else if (geom instanceof ol.geom.MultiLineString) { @@ -163,8 +163,9 @@ ol.parser.GPX = function(opt_options) { this.writeNode('trkseg', geom.components[i], undefined, node); } } else if (geom instanceof ol.geom.Polygon) { - for (i = 0, ii = geom.rings.length; i < ii; ++i) { - this.writeNode('trkseg', geom.rings[i], undefined, node); + rings = geom.getRings(); + for (i = 0, ii = rings.length; i < ii; ++i) { + this.writeNode('trkseg', rings[i], undefined, node); } } return node; diff --git a/src/ol/parser/wktparser.js b/src/ol/parser/wktparser.js index 7ce05cd233..c41e2d82c3 100644 --- a/src/ol/parser/wktparser.js +++ b/src/ol/parser/wktparser.js @@ -229,9 +229,10 @@ ol.parser.WKT.prototype.encodeMultiLineString_ = function(geom) { */ ol.parser.WKT.prototype.encodePolygon_ = function(geom) { var array = []; - for (var i = 0, ii = geom.rings.length; i < ii; ++i) { + var rings = geom.getRings(); + for (var i = 0, ii = rings.length; i < ii; ++i) { array.push('(' + this.encodeLineString_.apply(this, - [geom.rings[i]]) + ')'); + [rings[i]]) + ')'); } return array.join(','); }; diff --git a/src/ol/renderer/canvas/canvasvectorrenderer.js b/src/ol/renderer/canvas/canvasvectorrenderer.js index 25c746ec70..3be9e21e9a 100644 --- a/src/ol/renderer/canvas/canvasvectorrenderer.js +++ b/src/ol/renderer/canvas/canvasvectorrenderer.js @@ -362,7 +362,7 @@ ol.renderer.canvas.Vector.prototype.renderPolygonFeatures_ = } for (j = 0, jj = components.length; j < jj; ++j) { poly = components[j]; - rings = poly.rings; + rings = poly.getRings(); numRings = rings.length; if (numRings > 0) { // TODO: scenario 4 diff --git a/test/spec/ol/geom/polygon.test.js b/test/spec/ol/geom/polygon.test.js index b895d27482..c5fd875431 100644 --- a/test/spec/ol/geom/polygon.test.js +++ b/test/spec/ol/geom/polygon.test.js @@ -16,15 +16,15 @@ describe('ol.geom.Polygon', function() { }); - describe('#rings', function() { + describe('#getRings()', function() { - it('is an array of LinearRing', function() { + it('returns an array of LinearRing', function() { var poly = new ol.geom.Polygon([outer, inner1, inner2]); - - expect(poly.rings.length).to.be(3); - expect(poly.rings[0]).to.be.a(ol.geom.LinearRing); - expect(poly.rings[1]).to.be.a(ol.geom.LinearRing); - expect(poly.rings[2]).to.be.a(ol.geom.LinearRing); + var rings = poly.getRings(); + expect(rings.length).to.be(3); + expect(rings[0]).to.be.a(ol.geom.LinearRing); + expect(rings[1]).to.be.a(ol.geom.LinearRing); + expect(rings[2]).to.be.a(ol.geom.LinearRing); }); var isClockwise = ol.geom.LinearRing.isClockwise; @@ -34,7 +34,7 @@ describe('ol.geom.Polygon', function() { expect(isClockwise(outer)).to.be(false); var poly = new ol.geom.Polygon([outer]); - var ring = poly.rings[0]; + var ring = poly.getRings()[0]; expect(isClockwise(ring.getCoordinates())).to.be(true); }); @@ -44,7 +44,7 @@ describe('ol.geom.Polygon', function() { expect(isClockwise(inner)).to.be(true); var poly = new ol.geom.Polygon([outer, inner]); - var ring = poly.rings[1]; + var ring = poly.getRings()[1]; expect(isClockwise(ring.getCoordinates())).to.be(false); }); diff --git a/test/spec/ol/parser/geojson.test.js b/test/spec/ol/parser/geojson.test.js index c8c7923c32..281e21eea7 100644 --- a/test/spec/ol/parser/geojson.test.js +++ b/test/spec/ol/parser/geojson.test.js @@ -150,10 +150,11 @@ describe('ol.parser.GeoJSON', function() { var obj = parser.read(str); expect(obj).to.be.a(ol.geom.Polygon); - expect(obj.rings.length).to.be(3); - expect(obj.rings[0]).to.be.a(ol.geom.LinearRing); - expect(obj.rings[1]).to.be.a(ol.geom.LinearRing); - expect(obj.rings[2]).to.be.a(ol.geom.LinearRing); + var rings = obj.getRings(); + expect(rings.length).to.be(3); + expect(rings[0]).to.be.a(ol.geom.LinearRing); + expect(rings[1]).to.be.a(ol.geom.LinearRing); + expect(rings[2]).to.be.a(ol.geom.LinearRing); }); it('parses geometry collection', function() { diff --git a/test/spec/ol/parser/wkt.test.js b/test/spec/ol/parser/wkt.test.js index c49d7c9cb3..697e3ffa58 100644 --- a/test/spec/ol/parser/wkt.test.js +++ b/test/spec/ol/parser/wkt.test.js @@ -75,9 +75,10 @@ describe('ol.parser.WKT', function() { var wkt = 'POLYGON((30 10,10 20,20 40,40 40,30 10))'; var geom = parser.read(wkt); expect(geom.getType()).to.eql(ol.geom.GeometryType.POLYGON); - expect(geom.rings.length).to.eql(1); - expect(geom.rings[0].getType()).to.eql(ol.geom.GeometryType.LINEARRING); - expect(geom.rings[0].getCoordinates()).to.eql( + var rings = geom.getRings(); + expect(rings.length).to.eql(1); + expect(rings[0].getType()).to.eql(ol.geom.GeometryType.LINEARRING); + expect(rings[0].getCoordinates()).to.eql( [[30, 10], [10, 20], [20, 40], [40, 40], [30, 10]]); expect(parser.write(geom)).to.eql(wkt); @@ -85,12 +86,13 @@ describe('ol.parser.WKT', function() { wkt = 'POLYGON((35 10,10 20,15 40,45 45,35 10),(20 30,30 20,35 35,20 30))'; geom = parser.read(wkt); expect(geom.getType()).to.eql(ol.geom.GeometryType.POLYGON); - expect(geom.rings.length).to.eql(2); - expect(geom.rings[0].getType()).to.eql(ol.geom.GeometryType.LINEARRING); - expect(geom.rings[1].getType()).to.eql(ol.geom.GeometryType.LINEARRING); - expect(geom.rings[0].getCoordinates()).to.eql( + var rings = geom.getRings(); + expect(rings.length).to.eql(2); + expect(rings[0].getType()).to.eql(ol.geom.GeometryType.LINEARRING); + expect(rings[1].getType()).to.eql(ol.geom.GeometryType.LINEARRING); + expect(rings[0].getCoordinates()).to.eql( [[35, 10], [10, 20], [15, 40], [45, 45], [35, 10]]); - expect(geom.rings[1].getCoordinates()).to.eql( + expect(rings[1].getCoordinates()).to.eql( [[20, 30], [30, 20], [35, 35], [20, 30]]); expect(parser.write(geom)).to.eql(wkt); @@ -98,9 +100,10 @@ describe('ol.parser.WKT', function() { wkt = 'POLYGON ( (30 10, 10 20, 20 40, 40 40, 30 10) )'; geom = parser.read(wkt); expect(geom.getType()).to.eql(ol.geom.GeometryType.POLYGON); - expect(geom.rings.length).to.eql(1); - expect(geom.rings[0].getType()).to.eql(ol.geom.GeometryType.LINEARRING); - expect(geom.rings[0].getCoordinates()).to.eql( + var rings = geom.getRings(); + expect(rings.length).to.eql(1); + expect(rings[0].getType()).to.eql(ol.geom.GeometryType.LINEARRING); + expect(rings[0].getCoordinates()).to.eql( [[30, 10], [10, 20], [20, 40], [40, 40], [30, 10]]); }); @@ -113,13 +116,13 @@ describe('ol.parser.WKT', function() { expect(geom.components.length).to.eql(2); expect(geom.components[0].getType()).to.eql(ol.geom.GeometryType.POLYGON); expect(geom.components[1].getType()).to.eql(ol.geom.GeometryType.POLYGON); - expect(geom.components[0].rings.length).to.eql(1); - expect(geom.components[1].rings.length).to.eql(2); - expect(geom.components[0].rings[0].getCoordinates()).to.eql( + expect(geom.components[0].getRings().length).to.eql(1); + expect(geom.components[1].getRings().length).to.eql(2); + expect(geom.components[0].getRings()[0].getCoordinates()).to.eql( [[40, 40], [45, 30], [20, 45], [40, 40]]); - expect(geom.components[1].rings[0].getCoordinates()).to.eql( + expect(geom.components[1].getRings()[0].getCoordinates()).to.eql( [[20, 35], [45, 20], [30, 5], [10, 10], [10, 30], [20, 35]]); - expect(geom.components[1].rings[1].getCoordinates()).to.eql( + expect(geom.components[1].getRings()[1].getCoordinates()).to.eql( [[30, 20], [20, 25], [20, 15], [30, 20]]); expect(parser.write(geom)).to.eql(wkt); @@ -132,13 +135,13 @@ describe('ol.parser.WKT', function() { expect(geom.components.length).to.eql(2); expect(geom.components[0].getType()).to.eql(ol.geom.GeometryType.POLYGON); expect(geom.components[1].getType()).to.eql(ol.geom.GeometryType.POLYGON); - expect(geom.components[0].rings.length).to.eql(1); - expect(geom.components[1].rings.length).to.eql(2); - expect(geom.components[0].rings[0].getCoordinates()).to.eql( + expect(geom.components[0].getRings().length).to.eql(1); + expect(geom.components[1].getRings().length).to.eql(2); + expect(geom.components[0].getRings()[0].getCoordinates()).to.eql( [[40, 40], [45, 30], [20, 45], [40, 40]]); - expect(geom.components[1].rings[0].getCoordinates()).to.eql( + expect(geom.components[1].getRings()[0].getCoordinates()).to.eql( [[20, 35], [45, 20], [30, 5], [10, 10], [10, 30], [20, 35]]); - expect(geom.components[1].rings[1].getCoordinates()).to.eql( + expect(geom.components[1].getRings()[1].getCoordinates()).to.eql( [[30, 20], [20, 25], [20, 15], [30, 20]]); }); From 9b47c15bd80133bd3d9b228ab2c92af8abf90aa0 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Fri, 27 Sep 2013 19:10:51 +0200 Subject: [PATCH 2/7] Make geometries event targets Previously, the tests were using eql to make assertions about matching geometries. This is inappropriate for structures with circular references (as with goog.events.EventTarget); --- src/ol/geom/geometry.js | 26 +++++++++++++++++++- test/spec/ol/geom/geometry.test.js | 37 +++++++++++++++++++++++++++++ test/spec/ol/parser/geojson.test.js | 31 +++++++++++++++++------- 3 files changed, 85 insertions(+), 9 deletions(-) create mode 100644 test/spec/ol/geom/geometry.test.js diff --git a/src/ol/geom/geometry.js b/src/ol/geom/geometry.js index 189f347421..9578c7cd5b 100644 --- a/src/ol/geom/geometry.js +++ b/src/ol/geom/geometry.js @@ -1,6 +1,9 @@ goog.provide('ol.geom.Geometry'); +goog.provide('ol.geom.GeometryEvent'); goog.provide('ol.geom.GeometryType'); +goog.require('goog.events.Event'); +goog.require('goog.events.EventTarget'); goog.require('ol.Extent'); goog.require('ol.TransformFunction'); @@ -8,8 +11,12 @@ goog.require('ol.TransformFunction'); /** * @constructor + * @extends {goog.events.EventTarget} */ -ol.geom.Geometry = function() {}; +ol.geom.Geometry = function() { + goog.base(this); +}; +goog.inherits(ol.geom.Geometry, goog.events.EventTarget); /** @@ -48,6 +55,23 @@ ol.geom.Geometry.prototype.getType = goog.abstractMethod; ol.geom.Geometry.prototype.transform = goog.abstractMethod; + +/** + * Constructor for geometry events. + * @constructor + * @extends {goog.events.Event} + * @param {string} type Event type. + * @param {ol.geom.Geometry} target The target geometry. + * @param {ol.Extent} oldExtent The previous geometry extent. + */ +ol.geom.GeometryEvent = function(type, target, oldExtent) { + goog.base(this, type, target); + + this.oldExtent = oldExtent; +}; +goog.inherits(ol.geom.GeometryEvent, goog.events.Event); + + /** * Geometry types. * diff --git a/test/spec/ol/geom/geometry.test.js b/test/spec/ol/geom/geometry.test.js new file mode 100644 index 0000000000..71da801eee --- /dev/null +++ b/test/spec/ol/geom/geometry.test.js @@ -0,0 +1,37 @@ +goog.provide('ol.test.geom.Geometry'); + +describe('ol.geom.Geometry', function() { + + describe('constructor', function() { + it('creates a new geometry', function() { + var geom = new ol.geom.Geometry(); + expect(geom).to.be.a(ol.geom.Geometry); + expect(geom).to.be.a(goog.events.EventTarget); + }); + }); + +}); + +describe('ol.geom.GeometryEvent', function() { + + describe('constructor', function() { + + it('creates a new event', function() { + var point = new ol.geom.Point([1, 2]); + var bounds = point.getBounds(); + var evt = new ol.geom.GeometryEvent('change', point, bounds); + expect(evt).to.be.a(ol.geom.GeometryEvent); + expect(evt).to.be.a(goog.events.Event); + expect(evt.target).to.be(point); + expect(evt.oldExtent).to.be(bounds); + }); + + }); + +}); + +goog.require('goog.events.Event'); +goog.require('goog.events.EventTarget'); +goog.require('ol.geom.Geometry'); +goog.require('ol.geom.GeometryEvent'); +goog.require('ol.geom.Point'); diff --git a/test/spec/ol/parser/geojson.test.js b/test/spec/ol/parser/geojson.test.js index 281e21eea7..ade32cbbbd 100644 --- a/test/spec/ol/parser/geojson.test.js +++ b/test/spec/ol/parser/geojson.test.js @@ -73,13 +73,15 @@ describe('ol.parser.GeoJSON', function() { it('encodes point', function() { var point = new ol.geom.Point([10, 20]); var geojson = parser.write(point); - expect(point).to.eql(parser.read(geojson)); + expect(point.getCoordinates()).to.eql( + parser.read(geojson).getCoordinates()); }); it('encodes linestring', function() { var linestring = new ol.geom.LineString([[10, 20], [30, 40]]); var geojson = parser.write(linestring); - expect(linestring).to.eql(parser.read(geojson)); + expect(linestring.getCoordinates()).to.eql( + parser.read(geojson).getCoordinates()); }); it('encodes polygon', function() { @@ -88,7 +90,8 @@ describe('ol.parser.GeoJSON', function() { inner2 = [[8, 8], [9, 8], [9, 9], [8, 9], [8, 8]]; var polygon = new ol.geom.Polygon([outer, inner1, inner2]); var geojson = parser.write(polygon); - expect(polygon).to.eql(parser.read(geojson)); + expect(polygon.getCoordinates()).to.eql( + parser.read(geojson).getCoordinates()); }); it('encodes geometry collection', function() { @@ -97,9 +100,12 @@ describe('ol.parser.GeoJSON', function() { new ol.geom.LineString([[30, 40], [50, 60]]) ]); var geojson = parser.write(collection); - // surprised to see read return an Array of geometries instead of - // a true ol.geom.GeometryCollection, so compare collection.components - expect(collection.components).to.eql(parser.read(geojson)); + var got = parser.read(geojson); + expect(collection.components.length).to.equal(got.length); + for (var i = 0, ii = collection.components.length; i < ii; ++i) { + expect(collection.components[i].getCoordinates()).to.eql( + got[i].getCoordinates()); + } }); it('encodes feature collection', function() { @@ -107,9 +113,18 @@ describe('ol.parser.GeoJSON', function() { array = parser.read(str); var geojson = parser.write(array); var result = parser.read(geojson); + expect(array.length).to.equal(result.length); + var got, exp, gotAttr, expAttr; for (var i = 0, ii = array.length; i < ii; ++i) { - expect(array[i].getGeometry()).to.eql(result[i].getGeometry()); - expect(array[i].getAttributes()).to.eql(result[i].getAttributes()); + got = array[i]; + exp = result[i]; + expect(got.getGeometry().getCoordinates()).to.eql( + exp.getGeometry().getCoordinates()); + gotAttr = got.getAttributes(); + delete gotAttr.geometry; + expAttr = exp.getAttributes(); + delete expAttr.geometry; + expect(gotAttr).to.eql(expAttr); } }); From e78690c2d2c19c9f1edb21621a45793d5e5084e3 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Fri, 27 Sep 2013 19:14:56 +0200 Subject: [PATCH 3/7] Add setCoordinates for point and dispatch change event --- src/ol/geom/point.js | 17 ++++++++++++++++- test/spec/ol/geom/point.test.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/ol/geom/point.js b/src/ol/geom/point.js index 2b349c84c2..ffa269d902 100644 --- a/src/ol/geom/point.js +++ b/src/ol/geom/point.js @@ -1,8 +1,10 @@ goog.provide('ol.geom.Point'); goog.require('goog.asserts'); +goog.require('goog.events.EventType'); goog.require('ol.Coordinate'); goog.require('ol.geom.Geometry'); +goog.require('ol.geom.GeometryEvent'); goog.require('ol.geom.GeometryType'); @@ -71,11 +73,24 @@ ol.geom.Point.prototype.getType = function() { }; +/** + * Update the point coordinates. + * @param {ol.Coordinate} coordinates Coordinates array. + */ +ol.geom.Point.prototype.setCoordinates = function(coordinates) { + var oldBounds = this.bounds_; + this.bounds_ = null; + this.coordinates_ = coordinates; + this.dispatchEvent(new ol.geom.GeometryEvent(goog.events.EventType.CHANGE, + this, oldBounds)); +}; + + /** * @inheritDoc */ ol.geom.Point.prototype.transform = function(transform) { var coordinates = this.getCoordinates(); transform(coordinates, coordinates, coordinates.length); - this.bounds_ = null; + this.setCoordinates(coordinates); // for change event }; diff --git a/test/spec/ol/geom/point.test.js b/test/spec/ol/geom/point.test.js index 48b5a7f1e6..dda5bc9014 100644 --- a/test/spec/ol/geom/point.test.js +++ b/test/spec/ol/geom/point.test.js @@ -34,6 +34,35 @@ describe('ol.geom.Point', function() { }); + describe('#setCoordinates()', function() { + + it('updates the coordinates', function() { + var point = new ol.geom.Point([10, 20]); + point.setCoordinates([30, 40]); + expect(point.getCoordinates()).to.eql([30, 40]); + }); + + it('invalidates bounds', function() { + var point = new ol.geom.Point([10, 20]); + point.setCoordinates([30, 40]); + expect(point.getBounds()).to.eql([30, 40, 30, 40]); + }); + + it('triggers a change event', function(done) { + var point = new ol.geom.Point([10, 20]); + expect(point.getBounds()).to.eql([10, 20, 10, 20]); + goog.events.listen(point, 'change', function(evt) { + expect(evt.target).to.equal(point); + expect(evt.oldExtent).to.eql([10, 20, 10, 20]); + expect(evt.target.getBounds()).to.eql([30, 40, 30, 40]); + expect(evt.target.getCoordinates()).to.eql([30, 40]); + done(); + }); + point.setCoordinates([30, 40]); + }); + + }); + describe('#transform()', function() { var forward = ol.proj.getTransform('EPSG:4326', 'EPSG:3857'); @@ -57,6 +86,7 @@ describe('ol.geom.Point', function() { }); +goog.require('goog.events'); goog.require('ol.geom.Geometry'); goog.require('ol.geom.Point'); goog.require('ol.proj'); From 30b2e3930b06246ab2026d4a72e8417f8e29b5ba Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Fri, 27 Sep 2013 19:23:29 +0200 Subject: [PATCH 4/7] Add setCoordinates for linestring and dispatch change event --- src/ol/geom/linestring.js | 17 +++++++++++++++- test/spec/ol/geom/linestring.test.js | 30 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/ol/geom/linestring.js b/src/ol/geom/linestring.js index b992d5c490..bbb598bfd8 100644 --- a/src/ol/geom/linestring.js +++ b/src/ol/geom/linestring.js @@ -1,10 +1,12 @@ goog.provide('ol.geom.LineString'); goog.require('goog.asserts'); +goog.require('goog.events.EventType'); goog.require('ol.CoordinateArray'); goog.require('ol.extent'); goog.require('ol.geom'); goog.require('ol.geom.Geometry'); +goog.require('ol.geom.GeometryEvent'); goog.require('ol.geom.GeometryType'); @@ -108,6 +110,19 @@ ol.geom.LineString.prototype.distanceFromCoordinate = function(coordinate) { }; +/** + * Update the linestring coordinates. + * @param {ol.CoordinateArray} coordinates Coordinates array. + */ +ol.geom.LineString.prototype.setCoordinates = function(coordinates) { + var oldBounds = this.bounds_; + this.bounds_ = null; + this.coordinates_ = coordinates; + this.dispatchEvent(new ol.geom.GeometryEvent(goog.events.EventType.CHANGE, + this, oldBounds)); +}; + + /** * @inheritDoc */ @@ -118,5 +133,5 @@ ol.geom.LineString.prototype.transform = function(transform) { coord = coordinates[i]; transform(coord, coord, coord.length); } - this.bounds_ = null; + this.setCoordinates(coordinates); // for change event }; diff --git a/test/spec/ol/geom/linestring.test.js b/test/spec/ol/geom/linestring.test.js index 5cf4f65885..627adcbd36 100644 --- a/test/spec/ol/geom/linestring.test.js +++ b/test/spec/ol/geom/linestring.test.js @@ -34,6 +34,35 @@ describe('ol.geom.LineString', function() { }); + describe('#setCoordinates()', function() { + + it('updates the coordinates', function() { + var line = new ol.geom.LineString([[10, 20], [30, 40]]); + line.setCoordinates([[30, 40], [50, 60]]); + expect(line.getCoordinates()).to.eql([[30, 40], [50, 60]]); + }); + + it('invalidates bounds', function() { + var line = new ol.geom.LineString([[10, 20], [30, 40]]); + line.setCoordinates([[30, 40], [50, 60]]); + expect(line.getBounds()).to.eql([30, 40, 50, 60]); + }); + + it('triggers a change event', function(done) { + var line = new ol.geom.LineString([[10, 20], [30, 40]]); + expect(line.getBounds()).to.eql([10, 20, 30, 40]); + goog.events.listen(line, 'change', function(evt) { + expect(evt.target).to.equal(line); + expect(evt.oldExtent).to.eql([10, 20, 30, 40]); + expect(evt.target.getBounds()).to.eql([30, 40, 50, 60]); + expect(evt.target.getCoordinates()).to.eql([[30, 40], [50, 60]]); + done(); + }); + line.setCoordinates([[30, 40], [50, 60]]); + }); + + }); + describe('#transform()', function() { var forward = ol.proj.getTransform('EPSG:4326', 'EPSG:3857'); @@ -67,6 +96,7 @@ describe('ol.geom.LineString', function() { }); +goog.require('goog.events'); goog.require('ol.geom.Geometry'); goog.require('ol.geom.LineString'); goog.require('ol.proj'); From b8216193687490bdc825d6798b1f392c6228b14e Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Fri, 27 Sep 2013 21:58:20 +0200 Subject: [PATCH 5/7] Change event for polygons --- src/ol/geom/polygon.js | 28 ++++++++++++++++++-- test/spec/ol/geom/polygon.test.js | 43 +++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/ol/geom/polygon.js b/src/ol/geom/polygon.js index fd0131fb59..6016a8d08a 100644 --- a/src/ol/geom/polygon.js +++ b/src/ol/geom/polygon.js @@ -1,9 +1,12 @@ goog.provide('ol.geom.Polygon'); goog.require('goog.asserts'); +goog.require('goog.events'); +goog.require('goog.events.EventType'); goog.require('ol.CoordinateArray'); goog.require('ol.extent'); goog.require('ol.geom.Geometry'); +goog.require('ol.geom.GeometryEvent'); goog.require('ol.geom.GeometryType'); goog.require('ol.geom.LinearRing'); @@ -38,7 +41,7 @@ ol.geom.Polygon = function(coordinates) { * @private */ this.rings_ = new Array(numRings); - var ringCoords; + var ringCoords, ring; for (var i = 0; i < numRings; ++i) { ringCoords = coordinates[i]; if (i === 0) { @@ -52,7 +55,10 @@ ol.geom.Polygon = function(coordinates) { ringCoords.reverse(); } } - this.rings_[i] = new ol.geom.LinearRing(ringCoords); + ring = new ol.geom.LinearRing(ringCoords); + goog.events.listen(ring, goog.events.EventType.CHANGE, + this.handleRingChange_, false, this); + this.rings_[i] = ring; } }; @@ -98,6 +104,24 @@ ol.geom.Polygon.prototype.getRings = function() { }; +/** + * Listener for ring change events. + * @param {ol.geom.GeometryEvent} evt Geometry event. + * @private + */ +ol.geom.Polygon.prototype.handleRingChange_ = function(evt) { + var ring = evt.target; + var oldExtent = null; + if (ring === this.rings_[0]) { + oldExtent = evt.oldExtent; + } else { + oldExtent = this.getBounds(); + } + this.dispatchEvent(new ol.geom.GeometryEvent(goog.events.EventType.CHANGE, + this, oldExtent)); +}; + + /** * Check whether a given coordinate is inside this polygon. Note that this is a * fast and simple check - points on an edge or vertex of the polygon or one of diff --git a/test/spec/ol/geom/polygon.test.js b/test/spec/ol/geom/polygon.test.js index c5fd875431..d80ed15598 100644 --- a/test/spec/ol/geom/polygon.test.js +++ b/test/spec/ol/geom/polygon.test.js @@ -132,8 +132,51 @@ describe('ol.geom.Polygon', function() { }); + describe('change event', function() { + + var outer, inner; + beforeEach(function() { + outer = [[0, 0], [10, 0], [10, 10], [0, 10], [0, 0]]; + inner = [[2, 2], [2, 8], [8, 8], [8, 2], [2, 2]]; + }); + + it('is fired when outer ring is modified', function(done) { + var poly = new ol.geom.Polygon([outer, inner]); + var rings = poly.getRings(); + var bounds = poly.getBounds(); + goog.events.listen(poly, 'change', function(evt) { + expect(evt.target).to.be(poly); + expect(evt.oldExtent).to.eql(bounds); + expect(evt.target.getBounds()).to.eql([0, 0, 11, 10]); + done(); + }); + + var outerCoords = rings[0].getCoordinates(); + outerCoords[1][0] = 11; + rings[0].setCoordinates(outerCoords); + }); + + it('is fired when inner ring is modified', function(done) { + var poly = new ol.geom.Polygon([outer, inner]); + var rings = poly.getRings(); + var bounds = poly.getBounds(); + goog.events.listen(poly, 'change', function(evt) { + expect(evt.target).to.be(poly); + expect(evt.oldExtent).to.eql(bounds); + expect(evt.target.getBounds()).to.eql([0, 0, 10, 10]); + done(); + }); + + var innerCoords = rings[1].getCoordinates(); + innerCoords[1][0] = 3; + rings[1].setCoordinates(innerCoords); + }); + + }); + }); +goog.require('goog.events'); goog.require('ol.geom.Geometry'); goog.require('ol.geom.LinearRing'); goog.require('ol.geom.Polygon'); From adf99d592ab8bef51f5d6a7de71f0b9004201f28 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Thu, 3 Oct 2013 14:21:15 -0600 Subject: [PATCH 6/7] Listen for geometry events and fire feature events --- src/ol/feature.js | 63 ++++++++++++++++++++++++++++++++++-- test/spec/ol/feature.test.js | 45 ++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 2 deletions(-) diff --git a/src/ol/feature.js b/src/ol/feature.js index 97e7300aca..424a04ddf6 100644 --- a/src/ol/feature.js +++ b/src/ol/feature.js @@ -1,7 +1,13 @@ goog.provide('ol.Feature'); +goog.provide('ol.FeatureEvent'); +goog.provide('ol.FeatureEventType'); +goog.require('goog.events'); +goog.require('goog.events.Event'); +goog.require('goog.events.EventType'); goog.require('ol.Object'); goog.require('ol.geom.Geometry'); +goog.require('ol.geom.GeometryEvent'); goog.require('ol.layer.VectorLayerRenderIntent'); @@ -99,16 +105,44 @@ ol.Feature.prototype.getSymbolizers = function() { }; +/** + * Listener for geometry change events. + * @param {ol.geom.GeometryEvent} evt Geometry event. + * @private + */ +ol.Feature.prototype.handleGeometryChange_ = function(evt) { + this.dispatchEvent(new ol.FeatureEvent( + ol.FeatureEventType.CHANGE, this, evt.oldExtent)); +}; + + /** * @inheritDoc * @param {string} key Key. * @param {*} value Value. */ ol.Feature.prototype.set = function(key, value) { - if (!goog.isDef(this.geometryName_) && (value instanceof ol.geom.Geometry)) { - this.geometryName_ = key; + var geometry = this.getGeometry(); + var oldExtent = null; + if (goog.isDefAndNotNull(geometry)) { + oldExtent = geometry.getBounds(); + if (key === this.geometryName_) { + goog.events.unlisten(geometry, goog.events.EventType.CHANGE, + this.handleGeometryChange_, false, this); + } + } + if (value instanceof ol.geom.Geometry) { + if (!goog.isDef(this.geometryName_)) { + this.geometryName_ = key; + } + if (key === this.geometryName_) { + goog.events.listen(value, goog.events.EventType.CHANGE, + this.handleGeometryChange_, false, this); + } } goog.base(this, 'set', key, value); + this.dispatchEvent(new ol.FeatureEvent( + ol.FeatureEventType.CHANGE, this, oldExtent)); }; @@ -150,3 +184,28 @@ ol.Feature.prototype.setSymbolizers = function(symbolizers) { * @type {string} */ ol.Feature.DEFAULT_GEOMETRY = 'geometry'; + + +/** + * @enum {string} + */ +ol.FeatureEventType = { + CHANGE: 'featurechange' +}; + + + +/** + * Constructor for feature events. + * @constructor + * @extends {goog.events.Event} + * @param {string} type Event type. + * @param {ol.Feature} target The target feature. + * @param {ol.Extent} oldExtent The previous geometry extent. + */ +ol.FeatureEvent = function(type, target, oldExtent) { + goog.base(this, type, target); + + this.oldExtent = oldExtent; +}; +goog.inherits(ol.FeatureEvent, goog.events.Event); diff --git a/test/spec/ol/feature.test.js b/test/spec/ol/feature.test.js index 65626c53a5..1f31c8cc78 100644 --- a/test/spec/ol/feature.test.js +++ b/test/spec/ol/feature.test.js @@ -155,6 +155,28 @@ describe('ol.Feature', function() { }); + it('triggers a featurechange event', function(done) { + var feature = new ol.Feature(); + goog.events.listen(feature, 'featurechange', function(evt) { + expect(evt.target).to.be(feature); + expect(evt.oldExtent).to.be(null); + done(); + }); + feature.set('foo', 'bar'); + }); + + it('triggers a featurechange event with oldExtent', function(done) { + var feature = new ol.Feature({ + geom: new ol.geom.Point([15, 30]) + }); + goog.events.listen(feature, 'featurechange', function(evt) { + expect(evt.target).to.be(feature); + expect(evt.oldExtent).to.eql([15, 30, 15, 30]); + done(); + }); + feature.setGeometry(new ol.geom.Point([1, 2])); + }); + }); describe('#setGeometry()', function() { @@ -197,11 +219,34 @@ describe('ol.Feature', function() { expect(feature.getGeometry()).to.be(point); }); + it('triggers a featurechange event', function(done) { + var feature = new ol.Feature(); + goog.events.listen(feature, 'featurechange', function(evt) { + expect(evt.target).to.be(feature); + done(); + }); + feature.setGeometry('foo', point); + }); + + it('triggers a featurechange event with old extent', function(done) { + var first = new ol.geom.Point([10, 20]); + var feature = new ol.Feature({geom: first}); + var second = new ol.geom.Point([20, 30]); + goog.events.listen(feature, 'featurechange', function(evt) { + expect(evt.target).to.be(feature); + expect(evt.target.getGeometry()).to.be(second); + expect(evt.oldExtent).to.eql(first.getBounds()); + done(); + }); + feature.setGeometry(second); + }); + }); }); +goog.require('goog.events'); goog.require('goog.object'); goog.require('ol.Feature'); goog.require('ol.geom.Point'); From 67fab12fefc7aaeeb4c52b22e3cdea146ebd30ed Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Thu, 3 Oct 2013 15:16:34 -0600 Subject: [PATCH 7/7] Listen for feature events and fire layer events --- src/ol/layer/vectorlayer.js | 37 +++++++++++++++++++++++--- test/spec/ol/layer/vectorlayer.test.js | 31 +++++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/ol/layer/vectorlayer.js b/src/ol/layer/vectorlayer.js index 362e34de12..d6dea95c45 100644 --- a/src/ol/layer/vectorlayer.js +++ b/src/ol/layer/vectorlayer.js @@ -3,9 +3,11 @@ goog.provide('ol.layer.VectorEventType'); goog.require('goog.array'); goog.require('goog.asserts'); +goog.require('goog.events'); goog.require('goog.events.Event'); goog.require('goog.object'); goog.require('ol.Feature'); +goog.require('ol.FeatureEventType'); goog.require('ol.extent'); goog.require('ol.layer.Layer'); goog.require('ol.proj'); @@ -114,16 +116,18 @@ ol.layer.FeatureCache.prototype.getFeatureWithUid = function(uid) { /** * Remove a feature from the cache. * @param {ol.Feature} feature Feature. + * @param {ol.Extent=} opt_extent Optional extent (used when the current feature + * extent is different than the one in the index). */ -ol.layer.FeatureCache.prototype.remove = function(feature) { +ol.layer.FeatureCache.prototype.remove = function(feature, opt_extent) { var id = goog.getUid(feature).toString(), geometry = feature.getGeometry(); delete this.idLookup_[id]; - // index by bounding box if (!goog.isNull(geometry)) { - this.rTree_.remove(geometry.getBounds(), feature); + var extent = goog.isDef(opt_extent) ? opt_extent : geometry.getBounds(); + this.rTree_.remove(extent, feature); } }; @@ -181,19 +185,44 @@ ol.layer.Vector.prototype.addFeatures = function(features) { if (!goog.isNull(geometry)) { ol.extent.extend(extent, geometry.getBounds()); } + goog.events.listen(feature, ol.FeatureEventType.CHANGE, + this.handleFeatureChange_, false, this); } this.dispatchEvent(new ol.layer.VectorEvent(ol.layer.VectorEventType.ADD, features, [extent])); }; +/** + * Listener for feature change events. + * @param {ol.FeatureEvent} evt The feature change event. + * @private + */ +ol.layer.Vector.prototype.handleFeatureChange_ = function(evt) { + goog.asserts.assertInstanceof(evt.target, ol.Feature); + var feature = /** @type {ol.Feature} */ (evt.target); + var extents = []; + if (!goog.isNull(evt.oldExtent)) { + extents.push(evt.oldExtent); + } + var geometry = feature.getGeometry(); + if (!goog.isNull(geometry)) { + this.featureCache_.remove(feature, evt.oldExtent); + this.featureCache_.add(feature); + extents.push(geometry.getBounds()); + } + this.dispatchEvent(new ol.layer.VectorEvent(ol.layer.VectorEventType.CHANGE, + [feature], extents)); +}; + + /** * Remove all features from the layer. */ ol.layer.Vector.prototype.clear = function() { this.featureCache_.clear(); this.dispatchEvent( - new ol.layer.VectorEvent(ol.layer.VectorEventType.CHANGE, [], [])); + new ol.layer.VectorEvent(ol.layer.VectorEventType.REMOVE, [], [])); }; diff --git a/test/spec/ol/layer/vectorlayer.test.js b/test/spec/ol/layer/vectorlayer.test.js index 84435a6d72..ea6977736e 100644 --- a/test/spec/ol/layer/vectorlayer.test.js +++ b/test/spec/ol/layer/vectorlayer.test.js @@ -147,6 +147,37 @@ describe('ol.layer.Vector', function() { }); + describe('ol.layer.VectorEvent', function() { + + var layer, features; + + beforeEach(function() { + features = [ + new ol.Feature({ + g: new ol.geom.Point([16.0, 48.0]) + }), + new ol.Feature({ + g: new ol.geom.LineString([[17.0, 49.0], [17.1, 49.1]]) + }) + ]; + layer = new ol.layer.Vector({ + source: new ol.source.Vector({}) + }); + layer.addFeatures(features); + }); + + it('dispatches events on feature change', function(done) { + layer.on('featurechange', function(evt) { + expect(evt.features[0]).to.be(features[0]); + expect(evt.extents[0]).to.eql(features[0].getGeometry().getBounds()); + done(); + }); + features[0].set('foo', 'bar'); + + }); + + }); + }); goog.require('goog.dispose');