Skip to content

Commit b89c2d9

Browse files
jasondaviesjasondavies
committed
Fix clipExtent bug.
If a polygon does not intersect with the extent, but has one or more rings inside the extent, then the extent should be checked to see whether it is inside the polygon: if so, an additional exterior ring is generated. Previously, this check was only made if there were no visible rings at all. Fixes an issue noticed in d3#1453.
1 parent 7bb322f commit b89c2d9

5 files changed

Lines changed: 74 additions & 28 deletions

File tree

‎d3.js‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3050,19 +3050,22 @@ d3 = function() {
30503050
},
30513051
polygonEnd: function() {
30523052
listener = listener_;
3053-
if ((segments = d3.merge(segments)).length) {
3054-
listener.polygonStart();
3055-
d3_geo_clipPolygon(segments, compare, inside, interpolate, listener);
3056-
listener.polygonEnd();
3057-
} else if (insidePolygon([ x0, y0 ])) {
3058-
listener.polygonStart(), listener.lineStart();
3053+
segments = d3.merge(segments);
3054+
var inside = clean && insidePolygon([ x0, y0 ]), visible = segments.length;
3055+
if (inside || visible) listener.polygonStart();
3056+
if (inside) {
3057+
listener.lineStart();
30593058
interpolate(null, null, 1, listener);
3060-
listener.lineEnd(), listener.polygonEnd();
3059+
listener.lineEnd();
3060+
}
3061+
if (visible) {
3062+
d3_geo_clipPolygon(segments, compare, pointInside, interpolate, listener);
30613063
}
3064+
if (inside || visible) listener.polygonEnd();
30623065
segments = polygon = ring = null;
30633066
}
30643067
};
3065-
function inside(point) {
3068+
function pointInside(point) {
30663069
var a = corner(point, -1), i = insidePolygon([ a === 0 || a === 3 ? x0 : x1, a > 1 ? y1 : y0 ]);
30673070
return i;
30683071
}
@@ -3094,17 +3097,17 @@ d3 = function() {
30943097
listener.point(to[0], to[1]);
30953098
}
30963099
}
3097-
function visible(x, y) {
3100+
function pointVisible(x, y) {
30983101
return x0 <= x && x <= x1 && y0 <= y && y <= y1;
30993102
}
31003103
function point(x, y) {
3101-
if (visible(x, y)) listener.point(x, y);
3104+
if (pointVisible(x, y)) listener.point(x, y);
31023105
}
3103-
var x__, y__, v__, x_, y_, v_, first;
3106+
var x__, y__, v__, x_, y_, v_, first, clean;
31043107
function lineStart() {
31053108
clip.point = linePoint;
31063109
if (polygon) polygon.push(ring = []);
3107-
first = true;
3110+
first = clean = true;
31083111
v_ = false;
31093112
x_ = y_ = NaN;
31103113
}
@@ -3120,7 +3123,7 @@ d3 = function() {
31203123
function linePoint(x, y) {
31213124
x = Math.max(-d3_geo_clipExtentMAX, Math.min(d3_geo_clipExtentMAX, x));
31223125
y = Math.max(-d3_geo_clipExtentMAX, Math.min(d3_geo_clipExtentMAX, y));
3123-
var v = visible(x, y);
3126+
var v = pointVisible(x, y);
31243127
if (polygon) ring.push([ x, y ]);
31253128
if (first) {
31263129
x__ = x, y__ = y, v__ = v;
@@ -3139,9 +3142,11 @@ d3 = function() {
31393142
}
31403143
listener.point(b[0], b[1]);
31413144
if (!v) listener.lineEnd();
3145+
clean = false;
31423146
} else if (v) {
31433147
listener.lineStart();
31443148
listener.point(x, y);
3149+
clean = false;
31453150
}
31463151
}
31473152
}

‎d3.min.js‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/geo/clip-extent.js‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,20 +46,24 @@ function d3_geo_clipExtent(x0, y0, x1, y1) {
4646
},
4747
polygonEnd: function() {
4848
listener = listener_;
49-
if ((segments = d3.merge(segments)).length) {
50-
listener.polygonStart();
51-
d3_geo_clipPolygon(segments, compare, inside, interpolate, listener);
52-
listener.polygonEnd();
53-
} else if (insidePolygon([x0, y0])) {
54-
listener.polygonStart(), listener.lineStart();
49+
segments = d3.merge(segments);
50+
var inside = clean && insidePolygon([x0, y0]),
51+
visible = segments.length;
52+
if (inside || visible) listener.polygonStart();
53+
if (inside) {
54+
listener.lineStart();
5555
interpolate(null, null, 1, listener);
56-
listener.lineEnd(), listener.polygonEnd();
56+
listener.lineEnd();
57+
}
58+
if (visible) {
59+
d3_geo_clipPolygon(segments, compare, pointInside, interpolate, listener);
5760
}
61+
if (inside || visible) listener.polygonEnd();
5862
segments = polygon = ring = null;
5963
}
6064
};
6165

62-
function inside(point) {
66+
function pointInside(point) {
6367
var a = corner(point, -1),
6468
i = insidePolygon([a === 0 || a === 3 ? x0 : x1, a > 1 ? y1 : y0]);
6569
return i;
@@ -101,22 +105,23 @@ function d3_geo_clipExtent(x0, y0, x1, y1) {
101105
}
102106
}
103107

104-
function visible(x, y) {
108+
function pointVisible(x, y) {
105109
return x0 <= x && x <= x1 && y0 <= y && y <= y1;
106110
}
107111

108112
function point(x, y) {
109-
if (visible(x, y)) listener.point(x, y);
113+
if (pointVisible(x, y)) listener.point(x, y);
110114
}
111115

112116
var x__, y__, v__, // first point
113117
x_, y_, v_, // previous point
114-
first;
118+
first,
119+
clean;
115120

116121
function lineStart() {
117122
clip.point = linePoint;
118123
if (polygon) polygon.push(ring = []);
119-
first = true;
124+
first = clean = true;
120125
v_ = false;
121126
x_ = y_ = NaN;
122127
}
@@ -137,7 +142,7 @@ function d3_geo_clipExtent(x0, y0, x1, y1) {
137142
function linePoint(x, y) {
138143
x = Math.max(-d3_geo_clipExtentMAX, Math.min(d3_geo_clipExtentMAX, x));
139144
y = Math.max(-d3_geo_clipExtentMAX, Math.min(d3_geo_clipExtentMAX, y));
140-
var v = visible(x, y);
145+
var v = pointVisible(x, y);
141146
if (polygon) ring.push([x, y]);
142147
if (first) {
143148
x__ = x, y__ = y, v__ = v;
@@ -158,9 +163,11 @@ function d3_geo_clipExtent(x0, y0, x1, y1) {
158163
}
159164
listener.point(b[0], b[1]);
160165
if (!v) listener.lineEnd();
166+
clean = false;
161167
} else if (v) {
162168
listener.lineStart();
163169
listener.point(x, y);
170+
clean = false;
164171
}
165172
}
166173
}

‎test/geo/benchmark.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ var fs = require("fs"),
22
d3 = require("../../");
33

44
var formatNumber = d3.format(",.02r"),
5-
projection = d3.geo.stereographic().clipAngle(150),
5+
projection = d3.geo.stereographic().clipAngle(150).clipExtent([[0, 0], [960, 500]]),
66
path = d3.geo.path().projection(projection),
77
graticule = d3.geo.graticule().step([1, 1]),
88
circle = d3.geo.circle().angle(30),

‎test/geo/clip-extent-test.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,40 @@ suite.addBatch({
109109
assert.isTrue(stream.valid);
110110
clip.extent([[0, 0], [960, 500]]);
111111
assert.isFalse(stream.valid);
112+
},
113+
"a polygon that encloses the extent, with a hole": function(d3) {
114+
var clip = d3.geo.clipExtent().extent([[1, 1], [9, 9]]),
115+
stream = clip.stream(testContext);
116+
stream.polygonStart();
117+
stream.lineStart();
118+
stream.point(0, 0);
119+
stream.point(10, 0);
120+
stream.point(10, 10);
121+
stream.point(0, 10);
122+
stream.lineEnd();
123+
stream.lineStart();
124+
stream.point(4, 4);
125+
stream.point(4, 6);
126+
stream.point(6, 6);
127+
stream.point(6, 4);
128+
stream.lineEnd();
129+
stream.polygonEnd();
130+
assert.deepEqual(testContext.buffer(), [
131+
{type: "polygonStart"},
132+
{type: "lineStart"},
133+
{type: "point", x: 1, y: 1},
134+
{type: "point", x: 9, y: 1},
135+
{type: "point", x: 9, y: 9},
136+
{type: "point", x: 1, y: 9},
137+
{type: "lineEnd"},
138+
{type: "lineStart"},
139+
{type: "point", x: 4, y: 4},
140+
{type: "point", x: 4, y: 6},
141+
{type: "point", x: 6, y: 6},
142+
{type: "point", x: 6, y: 4},
143+
{type: "lineEnd"},
144+
{type: "polygonEnd"}
145+
]);
112146
}
113147
}
114148
}

0 commit comments

Comments
 (0)