Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a bug with polygon stroke styles. #700

Merged
merged 1 commit into from
Jun 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 34 additions & 8 deletions src/polygonFeature.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeling that this comment can be made more readable. Also when we refer to get, are we talking about Map.prototype.get()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our style function can either be called like style('strokeColor'), which returns the value set for strokeColor, or as style.get('strokeColor'), which always returns a function for strokeColor. Here, if we know that the value is not a function, then we can pass it along to the stroke, whereas if it is a function, we have to get the appropriate value for the polygon by calling that function with the polygon's values.

* 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.
Expand Down Expand Up @@ -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;
}
Expand Down
21 changes: 19 additions & 2 deletions tests/cases/polygonFeature.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}],
Expand All @@ -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;
}
};
Expand Down Expand Up @@ -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');
Expand All @@ -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';
Expand Down