Avoid duplicate lineTo and correctly close rings

src/ol/render/canvas/canvasimmediate.js

@@ -364,12 +364,15 @@ ol.render.canvas.Immediate.prototype.moveToLineTo_ = function(flatCoordinates, o
       flatCoordinates, offset, end, stride, this.transform_,
       this.pixelCoordinates_);
   context.moveTo(pixelCoordinates[0], pixelCoordinates[1]);
-  var i;
+  var length = pixelCoordinates.length;
-  for (i = 2; i < pixelCoordinates.length; i += 2) {
+  if (close) {
+    length -= 2;
+  }
+  for (var i = 2; i < length; i += 2) {
     context.lineTo(pixelCoordinates[i], pixelCoordinates[i + 1]);
   }
   if (close) {
-    context.lineTo(pixelCoordinates[0], pixelCoordinates[1]);
+    context.closePath();
   }
   return end;
 };
@@ -384,12 +387,10 @@ ol.render.canvas.Immediate.prototype.moveToLineTo_ = function(flatCoordinates, o
  * @return {number} End.
  */
 ol.render.canvas.Immediate.prototype.drawRings_ = function(flatCoordinates, offset, ends, stride) {
-  var context = this.context_;
   var i, ii;
   for (i = 0, ii = ends.length; i < ii; ++i) {
     offset = this.moveToLineTo_(
         flatCoordinates, offset, ends[i], stride, true);
-    context.closePath(); // FIXME is this needed here?
   }
   return offset;
 };

test/spec/ol/render/canvasimmediate.test.js

@@ -148,32 +148,69 @@ describe('ol.render.canvas.Immediate', function() {
 
   });
 
-  describe('#drawMultiPolygon', function() {
+  describe('#drawMultiPolygon()', function() {
+
     it('creates the correct canvas instructions for 3D geometries', function() {
-      var log = {
+
-        lineTo: [],
+      var instructions = [];
-        moveTo: []
+
-      };
+      function serialize(index, instruction) {
-      // FIXME move the canvas/context mocks outside of here for reuse
+        if (!instruction) {
-      var context = {
+          return 'id: ' + index + ' NO INSTRUCTION';
-        setLineDash: function() {},
-        beginPath: function() {},
-        closePath: function() {},
-        stroke: function() {},
-        lineTo: function(x, y) {
-          log.lineTo.push([x, y]);
-        },
-        moveTo: function(x, y) {
-          log.moveTo.push([x, y]);
         }
+        var parts = [
+          'id: ' + index,
+          'type: ' + instruction.type
+        ];
+        if (instruction.args) {
+          parts.push('args: [' + instruction.args.map(function(arg) {
+            if (typeof arg === 'number') {
+              return arg.toFixed(9)
+            } else {
+              return arg;
+            }
+          }).join(', ') + ']');
+        }
+        return parts.join(', ');
+      }
+
+      var context = {
+        beginPath: function() {},
+        moveTo: function(x, y) {
+          instructions.push({
+            type: 'moveTo',
+            args: [x, y]
+          });
+        },
+        lineTo: function(x, y) {
+          instructions.push({
+            type: 'lineTo',
+            args: [x, y]
+          });
+        },
+        closePath: function() {
+          instructions.push({
+            type: 'closePath'
+          });
+        },
+        setLineDash: function() {},
+        stroke: function() {}
       };
-      var transform = [0.0004088332670837288, 0, 0, 0, 0,
+
-        -0.0004088332670837288, 0, 0, 0, 0, 1, 0, 4480.991370439071,
+      var transform = [
-        1529.5752568707105, 0, 1];
+        0.0004088332670837288, 0, 0, 0,
-      var extent = [-10960437.252092224, 2762924.0275091752,
+        0, -0.0004088332670837288, 0, 0,
-        -7572748.158493212, 3741317.9895594316];
+        0, 0, 1, 0,
-      var canvas = new ol.render.canvas.Immediate(context, 1, extent,
+        4480.991370439071, 1529.5752568707105, 0, 1
-          transform);
+      ];
+
+      var extent = [
+        -10960437.252092224, 2762924.0275091752,
+        -7572748.158493212, 3741317.9895594316
+      ];
+
+      var canvas = new ol.render.canvas.Immediate(context, 1, extent, transform);
+
       canvas.strokeState_ = {
         lineCap: 'round',
         lineDash: [],
@@ -182,60 +219,59 @@ describe('ol.render.canvas.Immediate', function() {
         miterLimit: 10,
         strokeStyle: '#00FFFF'
       };
-      var multiPolygonGeometry = new ol.geom.MultiPolygon([
+
-        [[[-80.736061, 28.788576000000006, 0],
+      var multiPolygonGeometry = new ol.geom.MultiPolygon([[[
-           [-80.763557, 28.821799999999996, 0],
+        // first polygon
-           [-80.817406, 28.895123999999996, 0],
+        [-80.736061, 28.788576000000006, 0], // moveTo()
-           [-80.891304, 29.013130000000004, 0],
+        [-80.763557, 28.821799999999996, 0], // lineTo()
-           [-80.916512, 29.071560000000005, 0],
+        [-80.817406, 28.895123999999996, 0], // lineTo()
-           [-80.899323, 29.061249000000004, 0],
+        [-80.891304, 29.013130000000004, 0], // lineTo()
-           [-80.862663, 28.991361999999995, 0],
+        [-80.916512, 29.071560000000005, 0], // lineTo()
-           [-80.736061, 28.788576000000006, 0]]], [[
+        [-80.899323, 29.061249000000004, 0], // lineTo()
-          [-82.102127, 26.585724, 0],
+        [-80.862663, 28.991361999999995, 0], // lineTo()
-          [-82.067139, 26.497208, 0],
+        [-80.736061, 28.788576000000006, 0] //  closePath()
-          [-82.097641, 26.493585999999993, 0],
+      ]],[[
-          [-82.135895, 26.642279000000002, 0],
+        // second polygon
-          [-82.183495, 26.683082999999996, 0],
+        [-82.102127, 26.585724, 0], //          moveTo()
-          [-82.128838, 26.693342, 0],
+        [-82.067139, 26.497208, 0], //          lineTo()
-          [-82.102127, 26.585724, 0]]]
+        [-82.097641, 26.493585999999993, 0], // lineTo()
-      ]).transform('EPSG:4326', 'EPSG:3857');
+        [-82.135895, 26.642279000000002, 0], // lineTo()
+        [-82.183495, 26.683082999999996, 0], // lineTo()
+        [-82.128838, 26.693342, 0], //          lineTo()
+        [-82.102127, 26.585724, 0] //           closePath()
+      ]]]).transform('EPSG:4326', 'EPSG:3857');
+
       canvas.drawMultiPolygon(multiPolygonGeometry, null);
-      expect(log.lineTo.length).to.be(15);
+
-      expect(log.lineTo[0][0]).to.roughlyEqual(805.3521540835154, 1e-9);
+      var expected = [
-      expect(log.lineTo[0][1]).to.roughlyEqual(158.76358389011807, 1e-9);
+        // first polygon
-      expect(log.lineTo[1][0]).to.roughlyEqual(802.9014262612932, 1e-9);
+        {type: 'moveTo', args: [806.6035275946265, 160.48916296287916]},
-      expect(log.lineTo[1][1]).to.roughlyEqual(154.95335187132082, 1e-9);
+        {type: 'lineTo', args: [805.3521540835154, 158.76358389011807]},
-      expect(log.lineTo[2][0]).to.roughlyEqual(799.5382461724039, 1e-9);
+        {type: 'lineTo', args: [802.9014262612932, 154.95335187132082]},
-      expect(log.lineTo[2][1]).to.roughlyEqual(148.815592819916, 1e-9);
+        {type: 'lineTo', args: [799.5382461724039, 148.815592819916]},
-      expect(log.lineTo[3][0]).to.roughlyEqual(798.3910020835165, 1e-9);
+        {type: 'lineTo', args: [798.3910020835165, 145.77392230456553]},
-      expect(log.lineTo[3][1]).to.roughlyEqual(145.77392230456553, 1e-9);
+        {type: 'lineTo', args: [799.1732925724045, 146.31080369865776]},
-      expect(log.lineTo[4][0]).to.roughlyEqual(799.1732925724045, 1e-9);
+        {type: 'lineTo', args: [800.8417299057378, 149.94832216046188]},
-      expect(log.lineTo[4][1]).to.roughlyEqual(146.31080369865776, 1e-9);
+        {type: 'closePath'},
-      expect(log.lineTo[5][0]).to.roughlyEqual(800.8417299057378, 1e-9);
+        // second polygon
-      expect(log.lineTo[5][1]).to.roughlyEqual(149.94832216046188, 1e-9);
+        {type: 'moveTo', args: [744.4323460835158, 273.7179168205373]},
-      expect(log.lineTo[6][0]).to.roughlyEqual(806.6035275946265, 1e-9);
+        {type: 'lineTo', args: [746.0246888390716, 278.22094795365365]},
-      expect(log.lineTo[6][1]).to.roughlyEqual(160.48916296287916, 1e-9);
+        {type: 'lineTo', args: [744.6365089279602, 278.40513424671826]},
-      expect(log.lineTo[7][0]).to.roughlyEqual(806.6035275946265, 1e-9);
+        {type: 'lineTo', args: [742.8955268835157, 270.83899948444764]},
-      expect(log.lineTo[7][1]).to.roughlyEqual(160.48916296287916, 1e-9);
+        {type: 'lineTo', args: [740.7291979946272, 268.76099731369345]},
-      expect(log.lineTo[8][0]).to.roughlyEqual(746.0246888390716, 1e-9);
+        {type: 'lineTo', args: [743.2166987946266, 268.23842607400616]},
-      expect(log.lineTo[8][1]).to.roughlyEqual(278.22094795365365, 1e-9);
+        {type: 'closePath'}
-      expect(log.lineTo[9][0]).to.roughlyEqual(744.6365089279602, 1e-9);
+      ];
-      expect(log.lineTo[9][1]).to.roughlyEqual(278.40513424671826, 1e-9);
+
-      expect(log.lineTo[10][0]).to.roughlyEqual(742.8955268835157, 1e-9);
+
-      expect(log.lineTo[10][1]).to.roughlyEqual(270.83899948444764, 1e-9);
+      for (var i = 0, ii = instructions.length; i < ii; ++i) {
-      expect(log.lineTo[11][0]).to.roughlyEqual(740.7291979946272, 1e-9);
+        var actualInstruction = serialize(i, instructions[i]);
-      expect(log.lineTo[11][1]).to.roughlyEqual(268.76099731369345, 1e-9);
+        var expectedInstruction = serialize(i, expected[i]);
-      expect(log.lineTo[12][0]).to.roughlyEqual(743.2166987946266, 1e-9);
+        expect(actualInstruction).to.equal(expectedInstruction);
-      expect(log.lineTo[12][1]).to.roughlyEqual(268.23842607400616, 1e-9);
+      }
-      expect(log.lineTo[13][0]).to.roughlyEqual(744.4323460835158, 1e-9);
+
-      expect(log.lineTo[13][1]).to.roughlyEqual(273.7179168205373, 1e-9);
+      expect(instructions.length).to.equal(expected.length);
-      expect(log.lineTo[14][0]).to.roughlyEqual(744.4323460835158, 1e-9);
+
-      expect(log.lineTo[14][1]).to.roughlyEqual(273.7179168205373, 1e-9);
-      expect(log.moveTo.length).to.be(2);
-      expect(log.moveTo[0][0]).to.roughlyEqual(806.6035275946265, 1e-9);
-      expect(log.moveTo[0][1]).to.roughlyEqual(160.48916296287916, 1e-9);
-      expect(log.moveTo[1][0]).to.roughlyEqual(744.4323460835158, 1e-9);
-      expect(log.moveTo[1][1]).to.roughlyEqual(273.7179168205373, 1e-9);
     });
   });
 });