Skip to content

Constraint contours with rounded-off edgepath bug fix #4102

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Aug 28, 2019
Merged

Constraint contours with rounded-off edgepath bug fix #4102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
672a1c2
fix `carpet` attr description
etpinard Jul 31, 2019
703381f
some linting
etpinard Jul 31, 2019
f6b8636
add `noHover` categories to carpet and contourcarpet traces
etpinard Jul 31, 2019
5e80682
fixup comments
etpinard Jul 31, 2019
006ddc7
resolve #1311 - replace join(',') calls
etpinard Aug 2, 2019
40c354a
move joinAllPaths and map_pathinfo to contourcarpet/plot.js
etpinard Aug 2, 2019
3a04fd3
add mock testing constraints that equal boundary min/max
etpinard Aug 1, 2019
9542351
add mocks showing contour(carpet) constraint rounded-off edgepath bug
etpinard Aug 1, 2019
0fe878f
merge initFullPath logic into closeBoundaries routine
etpinard Aug 1, 2019
6ba77c6
fix contour(carpet) rounded off edgpath constraints contour bug
etpinard Aug 2, 2019
26e66fe
make contourcarpet w/ levels contours work w/ closeBoundaries
etpinard Aug 2, 2019
6f9a59d
return early in closeBoundaries when constraint level have edgepath(s)
etpinard Aug 2, 2019
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
2 changes: 1 addition & 1 deletion src/traces/carpet/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ module.exports = {
editType: 'calc',
description: [
'An identifier for this carpet, so that `scattercarpet` and',
'`scattercontour` traces can specify a carpet plot on which',
'`contourcarpet` traces can specify a carpet plot on which',
'they lie'
].join(' ')
},
Expand Down
2 changes: 1 addition & 1 deletion src/traces/carpet/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module.exports = {
moduleType: 'trace',
name: 'carpet',
basePlotModule: require('../../plots/cartesian'),
categories: ['cartesian', 'svg', 'carpet', 'carpetAxis', 'notLegendIsolatable', 'noMultiCategory'],
categories: ['cartesian', 'svg', 'carpet', 'carpetAxis', 'notLegendIsolatable', 'noMultiCategory', 'noHover'],
meta: {
description: [
'The data describing carpet axis layout is set in `y` and (optionally)',
Expand Down
107 changes: 65 additions & 42 deletions src/traces/contour/close_boundaries.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,57 +8,80 @@

'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 contours = trace.contours;
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 = 0; i < pathinfo.length; i++) {
var pi = pathinfo[i];
pi.prefixBoundary = !pi.edgepaths.length &&
(edgeVal2 > pi.level || pi.starts.length && edgeVal2 === pi.level);
}
break;
case 'constraint':
// after convertToConstraints, pathinfo has length=0
pi0.prefixBoundary = false;

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]);
}
// joinAllPaths does enough already when edgepaths are present
if(pi0.edgepaths.length) return;

pi0.prefixBoundary = false;
var na = pi0.x.length;
var nb = pi0.y.length;
var boundaryMax = -Infinity;
var boundaryMin = Infinity;

switch(operation) {
case '>':
if(contours.value > boundaryMax) {
pi0.prefixBoundary = true;
}
break;
case '<':
if(contours.value < boundaryMin) {
pi0.prefixBoundary = true;
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.apply(null, contours.value);
v2 = Math.max.apply(null, contours.value);
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.apply(null, contours.value);
v2 = Math.max.apply(null, contours.value);
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.starts.length && 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.starts.length && v2 === boundaryMin)) {
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;
}
Expand Down
30 changes: 21 additions & 9 deletions src/traces/contour/convert_to_constraints.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ var Lib = require('../../lib');
// need weird range loops and flipped contours instead of the usual format. This function
// does some weird manipulation of the extracted pathinfo data such that it magically
// draws contours correctly *as* constraints.
//
// ** I do not know which "weird range loops" the comment above is referring to.
module.exports = function(pathinfo, operation) {
var i, pi0, pi1;

Expand All @@ -29,18 +31,20 @@ module.exports = function(pathinfo, operation) {
Lib.warn('Contour data invalid for the specified inequality operation.');
}

// In this case there should be exactly two contour levels in pathinfo. We
// simply concatenate the info into one pathinfo and flip all of the data
// in one. This will draw the contour as closed.
// In this case there should be exactly one contour levels in pathinfo.
// We flip all of the data. This will draw the contour as closed.
pi0 = pathinfo[0];

for(i = 0; i < pi0.edgepaths.length; i++) {
pi0.edgepaths[i] = op0(pi0.edgepaths[i]);
}

for(i = 0; i < pi0.paths.length; i++) {
pi0.paths[i] = op0(pi0.paths[i]);
}
for(i = 0; i < pi0.starts.length; i++) {
pi0.starts[i] = op0(pi0.starts[i]);
}

return pathinfo;
case '][':
var tmp = op0;
Expand All @@ -54,33 +58,41 @@ module.exports = function(pathinfo, operation) {
Lib.warn('Contour data invalid for the specified inequality range operation.');
}

// In this case there should be exactly two contour levels in pathinfo. We
// simply concatenate the info into one pathinfo and flip all of the data
// in one. This will draw the contour as closed.
// In this case there should be exactly two contour levels in pathinfo.
// - We concatenate the info into one pathinfo.
// - We must also flip all of the data in the `[]` case.
// This will draw the contours as closed.
pi0 = copyPathinfo(pathinfo[0]);
pi1 = copyPathinfo(pathinfo[1]);

for(i = 0; i < pi0.edgepaths.length; i++) {
pi0.edgepaths[i] = op0(pi0.edgepaths[i]);
}

for(i = 0; i < pi0.paths.length; i++) {
pi0.paths[i] = op0(pi0.paths[i]);
}
for(i = 0; i < pi0.starts.length; i++) {
pi0.starts[i] = op0(pi0.starts[i]);
}

while(pi1.edgepaths.length) {
pi0.edgepaths.push(op1(pi1.edgepaths.shift()));
}
while(pi1.paths.length) {
pi0.paths.push(op1(pi1.paths.shift()));
}
while(pi1.starts.length) {
pi0.starts.push(op1(pi1.starts.shift()));
}

return [pi0];
}
};

function copyPathinfo(pi) {
return Lib.extendFlat({}, pi, {
edgepaths: Lib.extendDeep([], pi.edgepaths),
paths: Lib.extendDeep([], pi.paths)
paths: Lib.extendDeep([], pi.paths),
starts: Lib.extendDeep([], pi.starts)
});
}
18 changes: 10 additions & 8 deletions src/traces/contour/find_all_paths.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@ function ptDist(pt1, pt2) {
}

function makePath(pi, loc, edgeflag, xtol, ytol) {
var startLocStr = loc.join(',');
var locStr = startLocStr;
var locStr = loc.join(',');
var mi = pi.crossings[locStr];
var marchStep = startStep(mi, edgeflag, loc);
var marchStep = getStartStep(mi, edgeflag, loc);
// start by going backward a half step and finding the crossing point
var pts = [getInterpPx(pi, loc, [-marchStep[0], -marchStep[1]])];
var startStepStr = marchStep.join(',');
var m = pi.z.length;
var n = pi.z[0].length;
var startLoc = loc.slice();
var startStep = marchStep.slice();
var cnt;

// now follow the path
Expand All @@ -83,14 +83,16 @@ function makePath(pi, loc, edgeflag, xtol, ytol) {
pts.push(getInterpPx(pi, loc, marchStep));
loc[0] += marchStep[0];
loc[1] += marchStep[1];
locStr = loc.join(',');

// don't include the same point multiple times
if(equalPts(pts[pts.length - 1], pts[pts.length - 2], xtol, ytol)) pts.pop();
locStr = loc.join(',');

var atEdge = (marchStep[0] && (loc[0] < 0 || loc[0] > n - 2)) ||
(marchStep[1] && (loc[1] < 0 || loc[1] > m - 2));
var closedLoop = (locStr === startLocStr) && (marchStep.join(',') === startStepStr);

var closedLoop = loc[0] === startLoc[0] && loc[1] === startLoc[1] &&
marchStep[0] === startStep[0] && marchStep[1] === startStep[1];

// have we completed a loop, or reached an edge?
if((closedLoop) || (edgeflag && atEdge)) break;
Expand Down Expand Up @@ -184,7 +186,7 @@ function makePath(pi, loc, edgeflag, xtol, ytol) {
} else {
if(!edgeflag) {
Lib.log('Unclosed interior contour?',
pi.level, startLocStr, pts.join('L'));
pi.level, startLoc.join(','), pts.join('L'));
}

// edge path - does it start where an existing edge path ends, or vice versa?
Expand Down Expand Up @@ -234,7 +236,7 @@ function makePath(pi, loc, edgeflag, xtol, ytol) {

// special function to get the marching step of the
// first point in the path (leading to loc)
function startStep(mi, edgeflag, loc) {
function getStartStep(mi, edgeflag, loc) {
var dx = 0;
var dy = 0;
if(mi > 20 && edgeflag) {
Expand Down
56 changes: 29 additions & 27 deletions src/traces/contour/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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;
Expand Down Expand Up @@ -612,17 +610,18 @@ exports.drawLabels = function(labelGroup, labelData, gd, lineClip, labelClipPath
};

function clipGaps(plotGroup, plotinfo, gd, cd0, perimeter) {
var trace = cd0.trace;
var clips = gd._fullLayout._clips;
var clipId = 'clip' + cd0.trace.uid;
var clipId = 'clip' + trace.uid;

var clipPath = clips.selectAll('#' + clipId)
.data(cd0.trace.connectgaps ? [] : [0]);
.data(trace.connectgaps ? [] : [0]);
clipPath.enter().append('clipPath')
.classed('contourclip', true)
.attr('id', clipId);
clipPath.exit().remove();

if(cd0.trace.connectgaps === false) {
if(trace.connectgaps === false) {
var clipPathInfo = {
// fraction of the way from missing to present point
// to draw the boundary.
Expand All @@ -644,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);
Expand Down
2 changes: 1 addition & 1 deletion src/traces/contourcarpet/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module.exports = {
moduleType: 'trace',
name: 'contourcarpet',
basePlotModule: require('../../plots/cartesian'),
categories: ['cartesian', 'svg', 'carpet', 'contour', 'symbols', 'showLegend', 'hasLines', 'carpetDependent'],
categories: ['cartesian', 'svg', 'carpet', 'contour', 'symbols', 'showLegend', 'hasLines', 'carpetDependent', 'noHover'],
meta: {
hrName: 'contour_carpet',
description: [
Expand Down
Loading