Fix a bug with polygon stroke styles.

If a polygon has a hole, then stroke styles were misapplied to polygons. This was caused by there being more than one stroke per polygon and the stroke styles being called with polygon indices. Fixes #699.
OpenGeoscience · Jun 1, 2017 · 85b595d · 85b595d
1 parent 248adae
commit 85b595d
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 10 deletions.
diff --git a/src/polygonFeature.js b/src/polygonFeature.js
@@ -149,6 +149,32 @@ var polygonFeature = function (arg) {
     });
   }
 
+  ////////////////////////////////////////////////////////////////////////////
+  /**
+   * Get the style for the stroke of the polygon.  Since polygons can have
+   * holes, the number of stroke lines may not be the same as the number of
+   * polygons.  If the style for a stroke is a function, this calls the
+   * appropriate value for the polygon.  Any style set for a stroke line should
+   * be wrapped in this function.
+   *
+   * @param {(object|function)?} styleValue The polygon's style value used for
+   *    the stroke.  This should be m_this.style(<name of style>) and not
+   *    m_this.style.get(<name of style>), as the result is more efficient if
+   *    the style is not a function.
+   * @returns {object|function} A style that can be used for the stroke.
+   * @private
+   */
+  ////////////////////////////////////////////////////////////////////////////
+  function linePolyStyle(styleValue) {
+    if (util.isFunction(styleValue)) {
+      return function (d) {
+        return styleValue(d[0], d[1], d[2], d[3]);
+      };
+    } else {
+      return styleValue;
+    }
+  }
+
   ////////////////////////////////////////////////////////////////////////////
   /**
    * Get/set polygon accessor.
@@ -287,15 +313,15 @@ var polygonFeature = function (arg) {
     }
     var polyStyle = m_this.style();
     m_lineFeature.style({
-      antialiasing: polyStyle.antialiasing,
+      antialiasing: linePolyStyle(polyStyle.antialiasing),
       closed: true,
-      lineCap: polyStyle.lineCap,
-      lineJoin: polyStyle.lineJoin,
-      miterLimit: polyStyle.miterLimit,
-      strokeWidth: polyStyle.strokeWidth,
-      strokeStyle: polyStyle.strokeStyle,
-      strokeColor: polyStyle.strokeColor,
-      strokeOffset: polyStyle.strokeOffset,
+      lineCap: linePolyStyle(polyStyle.lineCap),
+      lineJoin: linePolyStyle(polyStyle.lineJoin),
+      miterLimit: linePolyStyle(polyStyle.miterLimit),
+      strokeWidth: linePolyStyle(polyStyle.strokeWidth),
+      strokeStyle: linePolyStyle(polyStyle.strokeStyle),
+      strokeColor: linePolyStyle(polyStyle.strokeColor),
+      strokeOffset: linePolyStyle(polyStyle.strokeOffset),
       strokeOpacity: function (d) {
         return m_this.style.get('stroke')(d[2], d[3]) ? m_this.style.get('strokeOpacity')(d[0], d[1], d[2], d[3]) : 0;
       }

diff --git a/tests/cases/polygonFeature.js b/tests/cases/polygonFeature.js
@@ -26,7 +26,8 @@ describe('geo.polygonFeature', function () {
       inner: [
         [{x: 55, y: 10}, {x: 60, y: 15}, {x: 65, y: 10}, {x: 60, y: 5}]
       ],
-      fillColor: '#FF8000'
+      fillColor: '#FF8000',
+      strokeColor: '#008'
     }, {
       outer:
         [{x: 50, y: 8}, {x: 70, y: 8}, {x: 70, y: 12}, {x: 50, y: 12}],
@@ -36,15 +37,23 @@ describe('geo.polygonFeature', function () {
       uniformPolygon: true
     }
   ];
+  var stylesVisited = [];
   var testStyle = {
     fillOpacity: function (d, idx, poly, polyidx) {
+      stylesVisited.push({style: 'fillOpacity', params: [d, idx, poly, polyidx]});
       return poly.fillOpacity !== undefined ? poly.fillOpacity : 1;
     },
     fillColor: function (d, idx, poly, polyidx) {
+      stylesVisited.push({style: 'fillColor', params: [d, idx, poly, polyidx]});
       return poly.fillColor !== undefined ? poly.fillColor : 'blue';
     },
     stroke: true,
+    strokeColor: function (d, idx, poly, polyidx) {
+      stylesVisited.push({style: 'strokeColor', params: [d, idx, poly, polyidx]});
+      return poly.strokeColor !== undefined ? poly.strokeColor : 'red';
+    },
     uniformPolygon: function (d) {
+      stylesVisited.push({style: 'uniformPolygon', params: [d]});
       return d.uniformPolygon !== undefined ? d.uniformPolygon : false;
     }
   };
@@ -197,7 +206,7 @@ describe('geo.polygonFeature', function () {
   describe('geo.gl.polygonFeature', function () {
     var map, layer, polygons, glCounts, buildTime;
     it('basic usage', function () {
-
+      stylesVisited.splice(0, stylesVisited.length);
       mockVGLRenderer();
       map = create_map();
       layer = map.createLayer('feature');
@@ -212,6 +221,14 @@ describe('geo.polygonFeature', function () {
     waitForIt('next render gl A', function () {
       return vgl.mockCounts().createProgram >= (glCounts.createProgram || 0) + 2;
     });
+    it('check that styles were used', function () {
+      $.each(stylesVisited, function (idx, val) {
+        if (val.style === 'strokeColor') {
+          expect(typeof val.params[3]).toBe('number');
+          expect(val.params[3]).toBeLessThan(4);
+        }
+      });
+    });
     it('update the style', function () {
       polygons.style('fillColor', function (d) {
         return 'red';