Merge pull request #1560 from twpayne/hide-oriented-rings

Hide oriented rings

src/ol/geom/flatgeom.js

@@ -546,6 +546,51 @@ ol.geom.flat.linearRingsGetInteriorPoint =
 };
 
 
+/**
+ * @param {Array.<number>} flatCoordinates Flat coordinates.
+ * @param {number} offset Offset.
+ * @param {Array.<number>} ends Ends.
+ * @param {number} stride Stride.
+ * @return {boolean} `true` if all rings are correctly oriented, `false`
+ *     otherwise.
+ */
+ol.geom.flat.linearRingsAreOriented =
+    function(flatCoordinates, offset, ends, stride) {
+  var i, ii;
+  for (i = 0, ii = ends.length; i < ii; ++i) {
+    var end = ends[i];
+    var isClockwise = ol.geom.flat.linearRingIsClockwise(
+        flatCoordinates, offset, end, stride);
+    if (i === 0 ? !isClockwise : isClockwise) {
+      return false;
+    }
+    offset = end;
+  }
+  return true;
+};
+
+
+/**
+ * @param {Array.<number>} flatCoordinates Flat coordinates.
+ * @param {number} offset Offset.
+ * @param {Array.<Array.<number>>} endss Endss.
+ * @param {number} stride Stride.
+ * @return {boolean} `true` if all rings are correctly oriented, `false`
+ *     otherwise.
+ */
+ol.geom.flat.linearRingssAreOriented =
+    function(flatCoordinates, offset, endss, stride) {
+  var i, ii;
+  for (i = 0, ii = endss.length; i < ii; ++i) {
+    if (!ol.geom.flat.linearRingsAreOriented(
+        flatCoordinates, offset, endss[i], stride)) {
+      return false;
+    }
+  }
+  return true;
+};
+
+
 /**
  * @param {Array.<number>} flatCoordinates Flat coordinates.
  * @param {number} offset Offset.

src/ol/geom/multipolygon.js

@@ -50,6 +50,18 @@ ol.geom.MultiPolygon = function(coordinates, opt_layout) {
    */
   this.maxDeltaRevision_ = -1;
 
+  /**
+   * @private
+   * @type {number}
+   */
+  this.orientedRevision_ = -1;
+
+  /**
+   * @private
+   * @type {Array.<number>}
+   */
+  this.orientedFlatCoordinates_ = null;
+
   this.setCoordinates(coordinates, opt_layout);
 
 };
@@ -82,7 +94,7 @@ ol.geom.MultiPolygon.prototype.closestPointXY =
     this.maxDeltaRevision_ = this.getRevision();
   }
   return ol.geom.closest.getssClosestPoint(
-      this.flatCoordinates, 0, this.endss_, this.stride,
+      this.getOrientedFlatCoordinates(), 0, this.endss_, this.stride,
       this.maxDelta_, true, x, y, closestPoint, minSquaredDistance);
 };
 
@@ -92,7 +104,7 @@ ol.geom.MultiPolygon.prototype.closestPointXY =
  */
 ol.geom.MultiPolygon.prototype.containsXY = function(x, y) {
   return ol.geom.flat.linearRingssContainsXY(
-      this.flatCoordinates, 0, this.endss_, this.stride, x, y);
+      this.getOrientedFlatCoordinates(), 0, this.endss_, this.stride, x, y);
 };
 
 
@@ -101,7 +113,7 @@ ol.geom.MultiPolygon.prototype.containsXY = function(x, y) {
  */
 ol.geom.MultiPolygon.prototype.getArea = function() {
   return ol.geom.flat.linearRingssArea(
-      this.flatCoordinates, 0, this.endss_, this.stride);
+      this.getOrientedFlatCoordinates(), 0, this.endss_, this.stride);
 };
 
 
@@ -130,13 +142,33 @@ ol.geom.MultiPolygon.prototype.getInteriorPoints = function() {
     var ys = ol.geom.flat.linearRingssMidYs(
         this.flatCoordinates, 0, this.endss_, this.stride);
     this.interiorPoints_ = ol.geom.flat.linearRingssGetInteriorPoints(
-        this.flatCoordinates, 0, this.endss_, this.stride, ys);
+        this.getOrientedFlatCoordinates(), 0, this.endss_, this.stride, ys);
     this.interiorPointsRevision_ = this.getRevision();
   }
   return this.interiorPoints_;
 };
 
 
+/**
+ * @return {Array.<number>} Oriented flat coordinates.
+ */
+ol.geom.MultiPolygon.prototype.getOrientedFlatCoordinates = function() {
+  if (this.orientedRevision_ != this.getRevision()) {
+    var flatCoordinates = this.flatCoordinates;
+    if (ol.geom.flat.linearRingssAreOriented(
+        flatCoordinates, 0, this.endss_, this.stride)) {
+      this.orientedFlatCoordinates_ = flatCoordinates;
+    } else {
+      this.orientedFlatCoordinates_ = flatCoordinates.slice();
+      this.orientedFlatCoordinates_.length = ol.geom.flat.orientLinearRingss(
+          this.orientedFlatCoordinates_, 0, this.endss_, this.stride);
+    }
+    this.orientedRevision_ = this.getRevision();
+  }
+  return this.orientedFlatCoordinates_;
+};
+
+
 /**
  * @inheritDoc
  */
@@ -159,12 +191,20 @@ ol.geom.MultiPolygon.prototype.getSimplifiedGeometryInternal =
  * @return {Array.<ol.geom.Polygon>} Polygons.
  */
 ol.geom.MultiPolygon.prototype.getPolygons = function() {
-  // FIXME we should construct the polygons directly from the flat coordinates
+  var layout = this.layout;
-  var coordinates = this.getCoordinates();
+  var flatCoordinates = this.flatCoordinates;
+  var endss = this.endss_;
   var polygons = [];
+  var offset = 0;
   var i, ii;
-  for (i = 0, ii = coordinates.length; i < ii; ++i) {
+  for (i = 0, ii = endss.length; i < ii; ++i) {
-    polygons.push(new ol.geom.Polygon(coordinates[i]));
+    var ends = endss[i];
+    var end = ends[ends.length - 1];
+    var polygon = new ol.geom.Polygon(null);
+    polygon.setFlatCoordinates(
+        layout, flatCoordinates.slice(offset, end), ends.slice());
+    polygons.push(polygon);
+    offset = end;
   }
   return polygons;
 };
@@ -196,8 +236,6 @@ ol.geom.MultiPolygon.prototype.setCoordinates =
     var lastEnds = endss[endss.length - 1];
     this.flatCoordinates.length = lastEnds.length === 0 ?
         0 : lastEnds[lastEnds.length - 1];
-    ol.geom.flat.orientLinearRingss(
-        this.flatCoordinates, 0, this.endss_, this.stride);
     this.dispatchChangeEvent();
   }
 };

src/ol/geom/polygon.js

@@ -50,6 +50,18 @@ ol.geom.Polygon = function(coordinates, opt_layout) {
    */
   this.maxDeltaRevision_ = -1;
 
+  /**
+   * @private
+   * @type {number}
+   */
+  this.orientedRevision_ = -1;
+
+  /**
+   * @private
+   * @type {Array.<number>}
+   */
+  this.orientedFlatCoordinates_ = null;
+
   this.setCoordinates(coordinates, opt_layout);
 
 };
@@ -92,7 +104,7 @@ ol.geom.Polygon.prototype.closestPointXY =
  */
 ol.geom.Polygon.prototype.containsXY = function(x, y) {
   return ol.geom.flat.linearRingsContainsXY(
-      this.flatCoordinates, 0, this.ends_, this.stride, x, y);
+      this.getOrientedFlatCoordinates(), 0, this.ends_, this.stride, x, y);
 };
 
 
@@ -101,7 +113,7 @@ ol.geom.Polygon.prototype.containsXY = function(x, y) {
  */
 ol.geom.Polygon.prototype.getArea = function() {
   return ol.geom.flat.linearRingsArea(
-      this.flatCoordinates, 0, this.ends_, this.stride);
+      this.getOrientedFlatCoordinates(), 0, this.ends_, this.stride);
 };
 
 
@@ -130,7 +142,7 @@ ol.geom.Polygon.prototype.getInteriorPoint = function() {
     var extent = this.getExtent();
     var y = (extent[1] + extent[3]) / 2;
     this.interiorPoint_ = ol.geom.flat.linearRingsGetInteriorPoint(
-        this.flatCoordinates, 0, this.ends_, this.stride, y);
+        this.getOrientedFlatCoordinates(), 0, this.ends_, this.stride, y);
     this.interiorPointRevision_ = this.getRevision();
   }
   return this.interiorPoint_;
@@ -141,16 +153,43 @@ ol.geom.Polygon.prototype.getInteriorPoint = function() {
  * @return {Array.<ol.geom.LinearRing>} Linear rings.
  */
 ol.geom.Polygon.prototype.getLinearRings = function() {
+  var layout = this.layout;
+  var flatCoordinates = this.flatCoordinates;
+  var ends = this.ends_;
   var linearRings = [];
-  var coordinates = this.getCoordinates();
+  var offset = 0;
   var i, ii;
-  for (i = 0, ii = coordinates.length; i < ii; ++i) {
+  for (i = 0, ii = ends.length; i < ii; ++i) {
-    linearRings.push(new ol.geom.LinearRing(coordinates[i]));
+    var end = ends[i];
+    var linearRing = new ol.geom.LinearRing(null);
+    linearRing.setFlatCoordinates(layout, flatCoordinates.slice(offset, end));
+    linearRings.push(linearRing);
+    offset = end;
   }
   return linearRings;
 };
 
 
+/**
+ * @return {Array.<number>} Oriented flat coordinates.
+ */
+ol.geom.Polygon.prototype.getOrientedFlatCoordinates = function() {
+  if (this.orientedRevision_ != this.getRevision()) {
+    var flatCoordinates = this.flatCoordinates;
+    if (ol.geom.flat.linearRingsAreOriented(
+        flatCoordinates, 0, this.ends_, this.stride)) {
+      this.orientedFlatCoordinates_ = flatCoordinates;
+    } else {
+      this.orientedFlatCoordinates_ = flatCoordinates.slice();
+      this.orientedFlatCoordinates_.length = ol.geom.flat.orientLinearRings(
+          this.orientedFlatCoordinates_, 0, this.ends_, this.stride);
+    }
+    this.orientedRevision_ = this.getRevision();
+  }
+  return this.orientedFlatCoordinates_;
+};
+
+
 /**
  * @inheritDoc
  */
@@ -192,8 +231,6 @@ ol.geom.Polygon.prototype.setCoordinates = function(coordinates, opt_layout) {
     var ends = ol.geom.flat.deflateCoordinatess(
         this.flatCoordinates, 0, coordinates, this.stride, this.ends_);
     this.flatCoordinates.length = ends.length === 0 ? 0 : ends[ends.length - 1];
-    ol.geom.flat.orientLinearRings(
-        this.flatCoordinates, 0, this.ends_, this.stride);
     this.dispatchChangeEvent();
   }
 };

src/ol/render/canvas/canvasreplay.js

@@ -1096,7 +1096,7 @@ ol.render.canvas.PolygonReplay.prototype.drawPolygonGeometry =
          state.miterLimit, state.lineDash]);
   }
   var ends = polygonGeometry.getEnds();
-  var flatCoordinates = polygonGeometry.getFlatCoordinates();
+  var flatCoordinates = polygonGeometry.getOrientedFlatCoordinates();
   var stride = polygonGeometry.getStride();
   this.drawFlatCoordinatess_(flatCoordinates, 0, ends, stride);
   this.endGeometry(polygonGeometry, data);
@@ -1132,7 +1132,7 @@ ol.render.canvas.PolygonReplay.prototype.drawMultiPolygonGeometry =
          state.miterLimit, state.lineDash]);
   }
   var endss = multiPolygonGeometry.getEndss();
-  var flatCoordinates = multiPolygonGeometry.getFlatCoordinates();
+  var flatCoordinates = multiPolygonGeometry.getOrientedFlatCoordinates();
   var stride = multiPolygonGeometry.getStride();
   var offset = 0;
   var i, ii;

test/spec/ol/geom/polygon.test.js

@@ -73,19 +73,23 @@ describe('ol.geom.Polygon', function() {
       expect(polygon.getStride()).to.be(2);
     });
 
-    it('reverses the outer ring if necessary', function() {
+    it('has the expected rings', function() {
-      polygon = new ol.geom.Polygon([outerRing.reverse(), innerRing]);
+      var linearRings = polygon.getLinearRings();
-      expect(polygon.getFlatCoordinates()).to.eql(flatCoordinates);
+      expect(linearRings).to.be.an(Array);
+      expect(linearRings).to.have.length(2);
+      expect(linearRings[0]).to.be.an(ol.geom.LinearRing);
+      expect(linearRings[0].getCoordinates()).to.eql(outerRing);
+      expect(linearRings[1]).to.be.an(ol.geom.LinearRing);
+      expect(linearRings[1].getCoordinates()).to.eql(innerRing);
     });
 
-    it('reverses inner rings if necessary', function() {
+    it('does not reverse any rings', function() {
-      polygon = new ol.geom.Polygon([outerRing, innerRing.reverse()]);
+      outerRing.reverse();
-      expect(polygon.getFlatCoordinates()).to.eql(flatCoordinates);
+      innerRing.reverse();
-    });
+      polygon = new ol.geom.Polygon([outerRing, innerRing]);
-
+      var coordinates = polygon.getCoordinates();
-    it('reverses all rings if necessary', function() {
+      expect(coordinates[0]).to.eql(outerRing);
-      polygon = new ol.geom.Polygon([outerRing.reverse(), innerRing.reverse()]);
+      expect(coordinates[1]).to.eql(innerRing);
-      expect(polygon.getFlatCoordinates()).to.eql(flatCoordinates);
     });
 
     it('does not contain outside coordinates', function() {
@@ -100,6 +104,29 @@ describe('ol.geom.Polygon', function() {
       expect(polygon.containsCoordinate(insideInner)).to.be(false);
     });
 
+    describe('#getOrientedFlatCoordinates', function() {
+
+      it('reverses the outer ring if necessary', function() {
+        outerRing.reverse();
+        polygon = new ol.geom.Polygon([outerRing, innerRing]);
+        expect(polygon.getOrientedFlatCoordinates()).to.eql(flatCoordinates);
+      });
+
+      it('reverses inner rings if necessary', function() {
+        innerRing.reverse();
+        polygon = new ol.geom.Polygon([outerRing, innerRing]);
+        expect(polygon.getOrientedFlatCoordinates()).to.eql(flatCoordinates);
+      });
+
+      it('reverses all rings if necessary', function() {
+        outerRing.reverse();
+        innerRing.reverse();
+        polygon = new ol.geom.Polygon([outerRing, innerRing]);
+        expect(polygon.getOrientedFlatCoordinates()).to.eql(flatCoordinates);
+      });
+
+    });
+
   });
 
   describe('construct with 3D coordinates', function() {
@@ -136,21 +163,6 @@ describe('ol.geom.Polygon', function() {
       expect(polygon.getStride()).to.be(3);
     });
 
-    it('reverses the outer ring if necessary', function() {
-      polygon = new ol.geom.Polygon([outerRing.reverse(), innerRing]);
-      expect(polygon.getFlatCoordinates()).to.eql(flatCoordinates);
-    });
-
-    it('reverses inner rings if necessary', function() {
-      polygon = new ol.geom.Polygon([outerRing, innerRing.reverse()]);
-      expect(polygon.getFlatCoordinates()).to.eql(flatCoordinates);
-    });
-
-    it('reverses all rings if necessary', function() {
-      polygon = new ol.geom.Polygon([outerRing.reverse(), innerRing.reverse()]);
-      expect(polygon.getFlatCoordinates()).to.eql(flatCoordinates);
-    });
-
     it('does not contain outside coordinates', function() {
       expect(polygon.containsCoordinate(outsideOuter)).to.be(false);
     });
@@ -163,6 +175,29 @@ describe('ol.geom.Polygon', function() {
       expect(polygon.containsCoordinate(insideInner)).to.be(false);
     });
 
+    describe('#getOrientedFlatCoordinates', function() {
+
+      it('reverses the outer ring if necessary', function() {
+        outerRing.reverse();
+        polygon = new ol.geom.Polygon([outerRing, innerRing]);
+        expect(polygon.getOrientedFlatCoordinates()).to.eql(flatCoordinates);
+      });
+
+      it('reverses inner rings if necessary', function() {
+        innerRing.reverse();
+        polygon = new ol.geom.Polygon([outerRing, innerRing]);
+        expect(polygon.getOrientedFlatCoordinates()).to.eql(flatCoordinates);
+      });
+
+      it('reverses all rings if necessary', function() {
+        outerRing.reverse();
+        innerRing.reverse();
+        polygon = new ol.geom.Polygon([outerRing, innerRing]);
+        expect(polygon.getOrientedFlatCoordinates()).to.eql(flatCoordinates);
+      });
+
+    });
+
   });
 
   describe('construct with 3D coordinates and layout XYM', function() {
@@ -200,21 +235,6 @@ describe('ol.geom.Polygon', function() {
       expect(polygon.getStride()).to.be(3);
     });
 
-    it('reverses the outer ring if necessary', function() {
-      polygon = new ol.geom.Polygon([outerRing.reverse(), innerRing]);
-      expect(polygon.getFlatCoordinates()).to.eql(flatCoordinates);
-    });
-
-    it('reverses inner rings if necessary', function() {
-      polygon = new ol.geom.Polygon([outerRing, innerRing.reverse()]);
-      expect(polygon.getFlatCoordinates()).to.eql(flatCoordinates);
-    });
-
-    it('reverses all rings if necessary', function() {
-      polygon = new ol.geom.Polygon([outerRing.reverse(), innerRing.reverse()]);
-      expect(polygon.getFlatCoordinates()).to.eql(flatCoordinates);
-    });
-
     it('does not contain outside coordinates', function() {
       expect(polygon.containsCoordinate(outsideOuter)).to.be(false);
     });
@@ -227,6 +247,29 @@ describe('ol.geom.Polygon', function() {
       expect(polygon.containsCoordinate(insideInner)).to.be(false);
     });
 
+    describe('#getOrientedFlatCoordinates', function() {
+
+      it('reverses the outer ring if necessary', function() {
+        outerRing.reverse();
+        polygon = new ol.geom.Polygon([outerRing, innerRing]);
+        expect(polygon.getOrientedFlatCoordinates()).to.eql(flatCoordinates);
+      });
+
+      it('reverses inner rings if necessary', function() {
+        innerRing.reverse();
+        polygon = new ol.geom.Polygon([outerRing, innerRing]);
+        expect(polygon.getOrientedFlatCoordinates()).to.eql(flatCoordinates);
+      });
+
+      it('reverses all rings if necessary', function() {
+        outerRing.reverse();
+        innerRing.reverse();
+        polygon = new ol.geom.Polygon([outerRing, innerRing]);
+        expect(polygon.getOrientedFlatCoordinates()).to.eql(flatCoordinates);
+      });
+
+    });
+
   });
 
   describe('construct with 4D coordinates', function() {
@@ -271,24 +314,6 @@ describe('ol.geom.Polygon', function() {
       expect(polygon.getStride()).to.be(4);
     });
 
-    it('reverses the outer ring if necessary', function() {
-      polygon = new ol.geom.Polygon(
-          [outerRing.reverse(), innerRing1, innerRing2]);
-      expect(polygon.getFlatCoordinates()).to.eql(flatCoordinates);
-    });
-
-    it('reverses inner rings if necessary', function() {
-      polygon = new ol.geom.Polygon(
-          [outerRing, innerRing1.reverse(), innerRing2.reverse()]);
-      expect(polygon.getFlatCoordinates()).to.eql(flatCoordinates);
-    });
-
-    it('reverses all rings if necessary', function() {
-      polygon = new ol.geom.Polygon(
-          [outerRing.reverse(), innerRing1.reverse(), innerRing2.reverse()]);
-      expect(polygon.getFlatCoordinates()).to.eql(flatCoordinates);
-    });
-
     it('does not contain outside coordinates', function() {
       expect(polygon.containsCoordinate(outsideOuter)).to.be(false);
     });
@@ -302,6 +327,31 @@ describe('ol.geom.Polygon', function() {
       expect(polygon.containsCoordinate(insideInner2)).to.be(false);
     });
 
+    describe('#getOrientedFlatCoordinates', function() {
+
+      it('reverses the outer ring if necessary', function() {
+        outerRing.reverse();
+        polygon = new ol.geom.Polygon([outerRing, innerRing1, innerRing2]);
+        expect(polygon.getOrientedFlatCoordinates()).to.eql(flatCoordinates);
+      });
+
+      it('reverses inner rings if necessary', function() {
+        innerRing1.reverse();
+        innerRing2.reverse();
+        polygon = new ol.geom.Polygon([outerRing, innerRing1, innerRing2]);
+        expect(polygon.getOrientedFlatCoordinates()).to.eql(flatCoordinates);
+      });
+
+      it('reverses all rings if necessary', function() {
+        outerRing.reverse();
+        innerRing1.reverse();
+        innerRing2.reverse();
+        polygon = new ol.geom.Polygon([outerRing, innerRing1, innerRing2]);
+        expect(polygon.getOrientedFlatCoordinates()).to.eql(flatCoordinates);
+      });
+
+    });
+
   });
 
   describe('with a simple polygon', function() {
@@ -337,4 +387,5 @@ describe('ol.geom.Polygon', function() {
 
 
 goog.require('ol.extent');
+goog.require('ol.geom.LinearRing');
 goog.require('ol.geom.Polygon');

test/spec/ol/interaction/drawinteraction.test.js

@@ -314,9 +314,8 @@ describe('ol.interaction.Draw', function() {
       var geometry = features[0].getGeometry();
       expect(geometry).to.be.a(ol.geom.Polygon);
 
-      // note that order is forced clockwise (despite drawing counter-clockwise)
       expect(geometry.getCoordinates()).to.eql([
-        [[10, -20], [30, -10], [30, -20], [10, -20]]
+        [[10, -20], [30, -20], [30, -10], [10, -20]]
       ]);
     });
 
@@ -397,9 +396,8 @@ describe('ol.interaction.Draw', function() {
       var coordinates = geometry.getCoordinates();
       expect(coordinates).to.have.length(1);
 
-      // note that order is forced clockwise (despite drawing counter-clockwise)
       expect(coordinates[0]).to.eql([
-        [[10, -20], [30, -10], [30, -20], [10, -20]]
+        [[10, -20], [30, -20], [30, -10], [10, -20]]
       ]);
     });