diff --git a/src/traces/contour/close_boundaries.js b/src/traces/contour/close_boundaries.js index aa121020fce..ad525edf4a7 100644 --- a/src/traces/contour/close_boundaries.js +++ b/src/traces/contour/close_boundaries.js @@ -8,57 +8,74 @@ 'use strict'; -module.exports = function(pathinfo, operation, perimeter, trace) { - // Abandon all hope, ye who enter here. - var i, v1, v2; +module.exports = function(pathinfo, contours) { var pi0 = pathinfo[0]; - var na = pi0.x.length; - var nb = pi0.y.length; var z = pi0.z; - var contoursValue = trace.contours.value; + var i; - var boundaryMax = -Infinity; - var boundaryMin = Infinity; + switch(contours.type) { + case 'levels': + // Why (just) use z[0][0] and z[0][1]? + // + // N.B. using boundaryMin instead of edgeVal2 here makes the + // `contour_scatter` mock fail + var edgeVal2 = Math.min(z[0][0], z[0][1]); - for(i = 0; i < nb; i++) { - boundaryMin = Math.min(boundaryMin, z[i][0]); - boundaryMin = Math.min(boundaryMin, z[i][na - 1]); - boundaryMax = Math.max(boundaryMax, z[i][0]); - boundaryMax = Math.max(boundaryMax, z[i][na - 1]); - } - - for(i = 1; i < na - 1; i++) { - boundaryMin = Math.min(boundaryMin, z[0][i]); - boundaryMin = Math.min(boundaryMin, z[nb - 1][i]); - boundaryMax = Math.max(boundaryMax, z[0][i]); - boundaryMax = Math.max(boundaryMax, z[nb - 1][i]); - } - - pi0.prefixBoundary = false; - - switch(operation) { - case '>': - if(contoursValue > boundaryMax) { - pi0.prefixBoundary = true; + for(i = 0; i < pathinfo.length; i++) { + var pi = pathinfo[i]; + pi.prefixBoundary = !pi.edgepaths.length && edgeVal2 > pi.level; } break; - case '<': - if(contoursValue < boundaryMin) { - pi0.prefixBoundary = true; + case 'constraint': + // after convertToConstraints, pathinfo has length=0 + pi0.prefixBoundary = false; + + var na = pi0.x.length; + var nb = pi0.y.length; + var boundaryMax = -Infinity; + var boundaryMin = Infinity; + + for(i = 0; i < nb; i++) { + boundaryMin = Math.min(boundaryMin, z[i][0]); + boundaryMin = Math.min(boundaryMin, z[i][na - 1]); + boundaryMax = Math.max(boundaryMax, z[i][0]); + boundaryMax = Math.max(boundaryMax, z[i][na - 1]); } - break; - case '[]': - v1 = Math.min(contoursValue[0], contoursValue[1]); - v2 = Math.max(contoursValue[0], contoursValue[1]); - if(v2 < boundaryMin || v1 > boundaryMax) { - pi0.prefixBoundary = true; + for(i = 1; i < na - 1; i++) { + boundaryMin = Math.min(boundaryMin, z[0][i]); + boundaryMin = Math.min(boundaryMin, z[nb - 1][i]); + boundaryMax = Math.max(boundaryMax, z[0][i]); + boundaryMax = Math.max(boundaryMax, z[nb - 1][i]); } - break; - case '][': - v1 = Math.min(contoursValue[0], contoursValue[1]); - v2 = Math.max(contoursValue[0], contoursValue[1]); - if(v1 < boundaryMin && v2 > boundaryMax) { - pi0.prefixBoundary = true; + + var contoursValue = contours.value; + var v1, v2; + + switch(contours._operation) { + case '>': + if(contoursValue > boundaryMax) { + pi0.prefixBoundary = true; + } + break; + case '<': + if(contoursValue < boundaryMin) { + pi0.prefixBoundary = true; + } + break; + case '[]': + v1 = Math.min(contoursValue[0], contoursValue[1]); + v2 = Math.max(contoursValue[0], contoursValue[1]); + if(v2 < boundaryMin || v1 > boundaryMax) { + pi0.prefixBoundary = true; + } + break; + case '][': + v1 = Math.min(contoursValue[0], contoursValue[1]); + v2 = Math.max(contoursValue[0], contoursValue[1]); + if(v1 < boundaryMin && v2 > boundaryMax) { + pi0.prefixBoundary = true; + } + break; } break; } diff --git a/src/traces/contour/plot.js b/src/traces/contour/plot.js index 975f32720fa..1db57c67c0d 100644 --- a/src/traces/contour/plot.js +++ b/src/traces/contour/plot.js @@ -63,8 +63,8 @@ exports.plot = function plot(gd, plotinfo, cdcontours, contourLayer) { var fillPathinfo = pathinfo; if(contours.type === 'constraint') { + // N.B. this also mutates pathinfo fillPathinfo = convertToConstraints(pathinfo, contours._operation); - closeBoundaries(fillPathinfo, contours._operation, perimeter, trace); } // draw everything @@ -88,10 +88,17 @@ function makeBackground(plotgroup, perimeter, contours) { } function makeFills(plotgroup, pathinfo, perimeter, contours) { + var hasFills = contours.coloring === 'fill' || (contours.type === 'constraint' && contours._operation !== '='); + var boundaryPath = 'M' + perimeter.join('L') + 'Z'; + + // fills prefixBoundary in pathinfo items + if(hasFills) { + closeBoundaries(pathinfo, contours); + } + var fillgroup = Lib.ensureSingle(plotgroup, 'g', 'contourfill'); - var fillitems = fillgroup.selectAll('path') - .data(contours.coloring === 'fill' || (contours.type === 'constraint' && contours._operation !== '=') ? pathinfo : []); + var fillitems = fillgroup.selectAll('path').data(hasFills ? pathinfo : []); fillitems.enter().append('path'); fillitems.exit().remove(); fillitems.each(function(pi) { @@ -100,30 +107,21 @@ function makeFills(plotgroup, pathinfo, perimeter, contours) { // if the whole perimeter is above this level, start with a path // enclosing the whole thing. With all that, the parity should mean // that we always fill everything above the contour, nothing below - var fullpath = joinAllPaths(pi, perimeter); + var fullpath = (pi.prefixBoundary ? boundaryPath : '') + + joinAllPaths(pi, perimeter); - if(!fullpath) d3.select(this).remove(); - else d3.select(this).attr('d', fullpath).style('stroke', 'none'); + if(!fullpath) { + d3.select(this).remove(); + } else { + d3.select(this) + .attr('d', fullpath) + .style('stroke', 'none'); + } }); } -function initFullPath(pi, perimeter) { - var prefixBoundary = pi.prefixBoundary; - if(prefixBoundary === undefined) { - var edgeVal2 = Math.min(pi.z[0][0], pi.z[0][1]); - prefixBoundary = (!pi.edgepaths.length && edgeVal2 > pi.level); - } - - if(prefixBoundary) { - // TODO: why does ^^ not work for constraints? - // pi.prefixBoundary gets set by closeBoundaries - return 'M' + perimeter.join('L') + 'Z'; - } - return ''; -} - function joinAllPaths(pi, perimeter) { - var fullpath = initFullPath(pi, perimeter); + var fullpath = ''; var i = 0; var startsleft = pi.edgepaths.map(function(v, i) { return i; }); var newloop = true; @@ -645,10 +643,13 @@ function clipGaps(plotGroup, plotinfo, gd, cd0, perimeter) { makeCrossings([clipPathInfo]); findAllPaths([clipPathInfo]); - var fullpath = joinAllPaths(clipPathInfo, perimeter); + closeBoundaries([clipPathInfo], {type: 'levels'}); var path = Lib.ensureSingle(clipPath, 'path', ''); - path.attr('d', fullpath); + path.attr('d', + (clipPathInfo.prefixBoundary ? 'M' + perimeter.join('L') + 'Z' : '') + + joinAllPaths(clipPathInfo, perimeter) + ); } else clipId = null; Drawing.setClipUrl(plotGroup, clipId, gd); diff --git a/src/traces/contourcarpet/plot.js b/src/traces/contourcarpet/plot.js index 5e6e491e090..46d5ece2a6d 100644 --- a/src/traces/contourcarpet/plot.js +++ b/src/traces/contourcarpet/plot.js @@ -77,7 +77,6 @@ module.exports = function plot(gd, plotinfo, cdcontours, contourcarpetLayer) { var fillPathinfo = pathinfo; if(contours.type === 'constraint') { fillPathinfo = convertToConstraints(pathinfo, operation); - closeBoundaries(fillPathinfo, operation, perimeter, trace); } // Map the paths in a/b coordinates to pixel coordinates: @@ -328,10 +327,18 @@ function makeBackground(plotgroup, clipsegments, xaxis, yaxis, isConstraint, col } function makeFills(trace, plotgroup, xa, ya, pathinfo, perimeter, ab2p, carpet, carpetcd, coloring, boundaryPath) { - var fillgroup = Lib.ensureSingle(plotgroup, 'g', 'contourfill'); + var hasFills = coloring === 'fill'; + + // fills prefixBoundary in pathinfo items + // + // N.B. cheater_contour mock fails if we call closeBoundaries + // on contourcarpet traces that with `levels` contours + if(hasFills && trace.contours.type === 'constraint') { + closeBoundaries(pathinfo, trace.contours); + } - var fillitems = fillgroup.selectAll('path') - .data(coloring === 'fill' ? pathinfo : []); + var fillgroup = Lib.ensureSingle(plotgroup, 'g', 'contourfill'); + var fillitems = fillgroup.selectAll('path').data(hasFills ? pathinfo : []); fillitems.enter().append('path'); fillitems.exit().remove(); fillitems.each(function(pi) { @@ -340,11 +347,8 @@ function makeFills(trace, plotgroup, xa, ya, pathinfo, perimeter, ab2p, carpet, // if the whole perimeter is above this level, start with a path // enclosing the whole thing. With all that, the parity should mean // that we always fill everything above the contour, nothing below - var fullpath = joinAllPaths(trace, pi, perimeter, ab2p, carpet, carpetcd, xa, ya); - - if(pi.prefixBoundary) { - fullpath = boundaryPath + fullpath; - } + var fullpath = (pi.prefixBoundary ? boundaryPath : '') + + joinAllPaths(trace, pi, perimeter, ab2p, carpet, carpetcd, xa, ya); if(!fullpath) { d3.select(this).remove();