Skip to content

Commit 0fe878f

Browse files
etpinardetpinard
committed
merge initFullPath logic into closeBoundaries routine
- this way the "prefixBoundary" logic for both `levels` and `constraint` contours is in the same place - note that `cheater_contour` mock fails if we call closeBoundaries on contourcarpet trace with `levels` contours
1 parent 9542351 commit 0fe878f

File tree

3 files changed

+98
-76
lines changed

3 files changed

+98
-76
lines changed

Diff for: ‎src/traces/contour/close_boundaries.js

+60-43
Original file line numberDiff line numberDiff line change
@@ -8,57 +8,74 @@
88

99
'use strict';
1010

11-
module.exports = function(pathinfo, operation, perimeter, trace) {
12-
// Abandon all hope, ye who enter here.
13-
var i, v1, v2;
11+
module.exports = function(pathinfo, contours) {
1412
var pi0 = pathinfo[0];
15-
var na = pi0.x.length;
16-
var nb = pi0.y.length;
1713
var z = pi0.z;
18-
var contoursValue = trace.contours.value;
14+
var i;
1915

20-
var boundaryMax = -Infinity;
21-
var boundaryMin = Infinity;
16+
switch(contours.type) {
17+
case 'levels':
18+
// Why (just) use z[0][0] and z[0][1]?
19+
//
20+
// N.B. using boundaryMin instead of edgeVal2 here makes the
21+
// `contour_scatter` mock fail
22+
var edgeVal2 = Math.min(z[0][0], z[0][1]);
2223

23-
for(i = 0; i < nb; i++) {
24-
boundaryMin = Math.min(boundaryMin, z[i][0]);
25-
boundaryMin = Math.min(boundaryMin, z[i][na - 1]);
26-
boundaryMax = Math.max(boundaryMax, z[i][0]);
27-
boundaryMax = Math.max(boundaryMax, z[i][na - 1]);
28-
}
29-
30-
for(i = 1; i < na - 1; i++) {
31-
boundaryMin = Math.min(boundaryMin, z[0][i]);
32-
boundaryMin = Math.min(boundaryMin, z[nb - 1][i]);
33-
boundaryMax = Math.max(boundaryMax, z[0][i]);
34-
boundaryMax = Math.max(boundaryMax, z[nb - 1][i]);
35-
}
36-
37-
pi0.prefixBoundary = false;
38-
39-
switch(operation) {
40-
case '>':
41-
if(contoursValue > boundaryMax) {
42-
pi0.prefixBoundary = true;
24+
for(i = 0; i < pathinfo.length; i++) {
25+
var pi = pathinfo[i];
26+
pi.prefixBoundary = !pi.edgepaths.length && edgeVal2 > pi.level;
4327
}
4428
break;
45-
case '<':
46-
if(contoursValue < boundaryMin) {
47-
pi0.prefixBoundary = true;
29+
case 'constraint':
30+
// after convertToConstraints, pathinfo has length=0
31+
pi0.prefixBoundary = false;
32+
33+
var na = pi0.x.length;
34+
var nb = pi0.y.length;
35+
var boundaryMax = -Infinity;
36+
var boundaryMin = Infinity;
37+
38+
for(i = 0; i < nb; i++) {
39+
boundaryMin = Math.min(boundaryMin, z[i][0]);
40+
boundaryMin = Math.min(boundaryMin, z[i][na - 1]);
41+
boundaryMax = Math.max(boundaryMax, z[i][0]);
42+
boundaryMax = Math.max(boundaryMax, z[i][na - 1]);
4843
}
49-
break;
50-
case '[]':
51-
v1 = Math.min(contoursValue[0], contoursValue[1]);
52-
v2 = Math.max(contoursValue[0], contoursValue[1]);
53-
if(v2 < boundaryMin || v1 > boundaryMax) {
54-
pi0.prefixBoundary = true;
44+
for(i = 1; i < na - 1; i++) {
45+
boundaryMin = Math.min(boundaryMin, z[0][i]);
46+
boundaryMin = Math.min(boundaryMin, z[nb - 1][i]);
47+
boundaryMax = Math.max(boundaryMax, z[0][i]);
48+
boundaryMax = Math.max(boundaryMax, z[nb - 1][i]);
5549
}
56-
break;
57-
case '][':
58-
v1 = Math.min(contoursValue[0], contoursValue[1]);
59-
v2 = Math.max(contoursValue[0], contoursValue[1]);
60-
if(v1 < boundaryMin && v2 > boundaryMax) {
61-
pi0.prefixBoundary = true;
50+
51+
var contoursValue = contours.value;
52+
var v1, v2;
53+
54+
switch(contours._operation) {
55+
case '>':
56+
if(contoursValue > boundaryMax) {
57+
pi0.prefixBoundary = true;
58+
}
59+
break;
60+
case '<':
61+
if(contoursValue < boundaryMin) {
62+
pi0.prefixBoundary = true;
63+
}
64+
break;
65+
case '[]':
66+
v1 = Math.min(contoursValue[0], contoursValue[1]);
67+
v2 = Math.max(contoursValue[0], contoursValue[1]);
68+
if(v2 < boundaryMin || v1 > boundaryMax) {
69+
pi0.prefixBoundary = true;
70+
}
71+
break;
72+
case '][':
73+
v1 = Math.min(contoursValue[0], contoursValue[1]);
74+
v2 = Math.max(contoursValue[0], contoursValue[1]);
75+
if(v1 < boundaryMin && v2 > boundaryMax) {
76+
pi0.prefixBoundary = true;
77+
}
78+
break;
6279
}
6380
break;
6481
}

Diff for: ‎src/traces/contour/plot.js

+25-24
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ exports.plot = function plot(gd, plotinfo, cdcontours, contourLayer) {
6363

6464
var fillPathinfo = pathinfo;
6565
if(contours.type === 'constraint') {
66+
// N.B. this also mutates pathinfo
6667
fillPathinfo = convertToConstraints(pathinfo, contours._operation);
67-
closeBoundaries(fillPathinfo, contours._operation, perimeter, trace);
6868
}
6969

7070
// draw everything
@@ -88,10 +88,17 @@ function makeBackground(plotgroup, perimeter, contours) {
8888
}
8989

9090
function makeFills(plotgroup, pathinfo, perimeter, contours) {
91+
var hasFills = contours.coloring === 'fill' || (contours.type === 'constraint' && contours._operation !== '=');
92+
var boundaryPath = 'M' + perimeter.join('L') + 'Z';
93+
94+
// fills prefixBoundary in pathinfo items
95+
if(hasFills) {
96+
closeBoundaries(pathinfo, contours);
97+
}
98+
9199
var fillgroup = Lib.ensureSingle(plotgroup, 'g', 'contourfill');
92100

93-
var fillitems = fillgroup.selectAll('path')
94-
.data(contours.coloring === 'fill' || (contours.type === 'constraint' && contours._operation !== '=') ? pathinfo : []);
101+
var fillitems = fillgroup.selectAll('path').data(hasFills ? pathinfo : []);
95102
fillitems.enter().append('path');
96103
fillitems.exit().remove();
97104
fillitems.each(function(pi) {
@@ -100,30 +107,21 @@ function makeFills(plotgroup, pathinfo, perimeter, contours) {
100107
// if the whole perimeter is above this level, start with a path
101108
// enclosing the whole thing. With all that, the parity should mean
102109
// that we always fill everything above the contour, nothing below
103-
var fullpath = joinAllPaths(pi, perimeter);
110+
var fullpath = (pi.prefixBoundary ? boundaryPath : '') +
111+
joinAllPaths(pi, perimeter);
104112

105-
if(!fullpath) d3.select(this).remove();
106-
else d3.select(this).attr('d', fullpath).style('stroke', 'none');
113+
if(!fullpath) {
114+
d3.select(this).remove();
115+
} else {
116+
d3.select(this)
117+
.attr('d', fullpath)
118+
.style('stroke', 'none');
119+
}
107120
});
108121
}
109122

110-
function initFullPath(pi, perimeter) {
111-
var prefixBoundary = pi.prefixBoundary;
112-
if(prefixBoundary === undefined) {
113-
var edgeVal2 = Math.min(pi.z[0][0], pi.z[0][1]);
114-
prefixBoundary = (!pi.edgepaths.length && edgeVal2 > pi.level);
115-
}
116-
117-
if(prefixBoundary) {
118-
// TODO: why does ^^ not work for constraints?
119-
// pi.prefixBoundary gets set by closeBoundaries
120-
return 'M' + perimeter.join('L') + 'Z';
121-
}
122-
return '';
123-
}
124-
125123
function joinAllPaths(pi, perimeter) {
126-
var fullpath = initFullPath(pi, perimeter);
124+
var fullpath = '';
127125
var i = 0;
128126
var startsleft = pi.edgepaths.map(function(v, i) { return i; });
129127
var newloop = true;
@@ -645,10 +643,13 @@ function clipGaps(plotGroup, plotinfo, gd, cd0, perimeter) {
645643

646644
makeCrossings([clipPathInfo]);
647645
findAllPaths([clipPathInfo]);
648-
var fullpath = joinAllPaths(clipPathInfo, perimeter);
646+
closeBoundaries([clipPathInfo], {type: 'levels'});
649647

650648
var path = Lib.ensureSingle(clipPath, 'path', '');
651-
path.attr('d', fullpath);
649+
path.attr('d',
650+
(clipPathInfo.prefixBoundary ? 'M' + perimeter.join('L') + 'Z' : '') +
651+
joinAllPaths(clipPathInfo, perimeter)
652+
);
652653
} else clipId = null;
653654

654655
Drawing.setClipUrl(plotGroup, clipId, gd);

Diff for: ‎src/traces/contourcarpet/plot.js

+13-9
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ module.exports = function plot(gd, plotinfo, cdcontours, contourcarpetLayer) {
7777
var fillPathinfo = pathinfo;
7878
if(contours.type === 'constraint') {
7979
fillPathinfo = convertToConstraints(pathinfo, operation);
80-
closeBoundaries(fillPathinfo, operation, perimeter, trace);
8180
}
8281

8382
// Map the paths in a/b coordinates to pixel coordinates:
@@ -328,10 +327,18 @@ function makeBackground(plotgroup, clipsegments, xaxis, yaxis, isConstraint, col
328327
}
329328

330329
function makeFills(trace, plotgroup, xa, ya, pathinfo, perimeter, ab2p, carpet, carpetcd, coloring, boundaryPath) {
331-
var fillgroup = Lib.ensureSingle(plotgroup, 'g', 'contourfill');
330+
var hasFills = coloring === 'fill';
331+
332+
// fills prefixBoundary in pathinfo items
333+
//
334+
// N.B. cheater_contour mock fails if we call closeBoundaries
335+
// on contourcarpet traces that with `levels` contours
336+
if(hasFills && trace.contours.type === 'constraint') {
337+
closeBoundaries(pathinfo, trace.contours);
338+
}
332339

333-
var fillitems = fillgroup.selectAll('path')
334-
.data(coloring === 'fill' ? pathinfo : []);
340+
var fillgroup = Lib.ensureSingle(plotgroup, 'g', 'contourfill');
341+
var fillitems = fillgroup.selectAll('path').data(hasFills ? pathinfo : []);
335342
fillitems.enter().append('path');
336343
fillitems.exit().remove();
337344
fillitems.each(function(pi) {
@@ -340,11 +347,8 @@ function makeFills(trace, plotgroup, xa, ya, pathinfo, perimeter, ab2p, carpet,
340347
// if the whole perimeter is above this level, start with a path
341348
// enclosing the whole thing. With all that, the parity should mean
342349
// that we always fill everything above the contour, nothing below
343-
var fullpath = joinAllPaths(trace, pi, perimeter, ab2p, carpet, carpetcd, xa, ya);
344-
345-
if(pi.prefixBoundary) {
346-
fullpath = boundaryPath + fullpath;
347-
}
350+
var fullpath = (pi.prefixBoundary ? boundaryPath : '') +
351+
joinAllPaths(trace, pi, perimeter, ab2p, carpet, carpetcd, xa, ya);
348352

349353
if(!fullpath) {
350354
d3.select(this).remove();

0 commit comments

Comments
 (0)