From 6d88255bc76ce8b0a7b6c1a90ec00fb5886d6227 Mon Sep 17 00:00:00 2001 From: etienne Date: Thu, 19 Apr 2018 17:54:04 -0400 Subject: [PATCH 01/18] make bar, box & violin plot method use d3 enter/exit/update pattern - so that we no longer have the clear all their on every Cartesian.plot call !!! - Note that other svg traces (e.g. heatmap & contours) use trace.uid to append/update/clear their SVG layers weren't the focus of this commit. --- src/traces/bar/plot.js | 235 +++++++++++++++++++------------------- src/traces/box/plot.js | 213 ++++++++++++++++++---------------- src/traces/violin/plot.js | 167 ++++++++++++++------------- 3 files changed, 324 insertions(+), 291 deletions(-) diff --git a/src/traces/bar/plot.js b/src/traces/bar/plot.js index 28f81a985d3..0acc8eecd8e 100644 --- a/src/traces/bar/plot.js +++ b/src/traces/bar/plot.js @@ -37,136 +37,139 @@ module.exports = function plot(gd, plotinfo, cdbar) { var bartraces = plotinfo.plot.select('.barlayer') .selectAll('g.trace.bars') - .data(cdbar); + .data(cdbar, function(d) { return d[0].trace.uid; }); bartraces.enter().append('g') - .attr('class', 'trace bars'); + .attr('class', 'trace bars') + .append('g') + .attr('class', 'points'); - if(!plotinfo.isRangePlot) { - bartraces.each(function(d) { - d[0].node3 = d3.select(this); - }); - } + bartraces.exit().remove(); - bartraces.append('g') - .attr('class', 'points') - .each(function(d) { - var sel = d3.select(this); - var t = d[0].t; - var trace = d[0].trace; - var poffset = t.poffset; - var poffsetIsArray = Array.isArray(poffset); - - sel.selectAll('g.point') - .data(Lib.identity) - .enter().append('g').classed('point', true) - .each(function(di, i) { - // now display the bar - // clipped xf/yf (2nd arg true): non-positive - // log values go off-screen by plotwidth - // so you see them continue if you drag the plot - var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset), - p1 = p0 + di.w, - s0 = di.b, - s1 = s0 + di.s; - - var x0, x1, y0, y1; - if(trace.orientation === 'h') { - y0 = ya.c2p(p0, true); - y1 = ya.c2p(p1, true); - x0 = xa.c2p(s0, true); - x1 = xa.c2p(s1, true); - - // for selections - di.ct = [x1, (y0 + y1) / 2]; - } - else { - x0 = xa.c2p(p0, true); - x1 = xa.c2p(p1, true); - y0 = ya.c2p(s0, true); - y1 = ya.c2p(s1, true); - - // for selections - di.ct = [(x0 + x1) / 2, y1]; - } - - if(!isNumeric(x0) || !isNumeric(x1) || - !isNumeric(y0) || !isNumeric(y1) || - x0 === x1 || y0 === y1) { - d3.select(this).remove(); - return; - } - - var lw = (di.mlw + 1 || trace.marker.line.width + 1 || - (di.trace ? di.trace.marker.line.width : 0) + 1) - 1, - offset = d3.round((lw / 2) % 1, 2); - - function roundWithLine(v) { - // if there are explicit gaps, don't round, - // it can make the gaps look crappy - return (fullLayout.bargap === 0 && fullLayout.bargroupgap === 0) ? - d3.round(Math.round(v) - offset, 2) : v; - } - - function expandToVisible(v, vc) { - // if it's not in danger of disappearing entirely, - // round more precisely - return Math.abs(v - vc) >= 2 ? roundWithLine(v) : - // but if it's very thin, expand it so it's - // necessarily visible, even if it might overlap - // its neighbor - (v > vc ? Math.ceil(v) : Math.floor(v)); - } - - if(!gd._context.staticPlot) { - // if bars are not fully opaque or they have a line - // around them, round to integer pixels, mainly for - // safari so we prevent overlaps from its expansive - // pixelation. if the bars ARE fully opaque and have - // no line, expand to a full pixel to make sure we - // can see them - var op = Color.opacity(di.mc || trace.marker.color), - fixpx = (op < 1 || lw > 0.01) ? - roundWithLine : expandToVisible; - x0 = fixpx(x0, x1); - x1 = fixpx(x1, x0); - y0 = fixpx(y0, y1); - y1 = fixpx(y1, y0); - } - - // append bar path and text - var bar = d3.select(this); - - bar.append('path') - .style('vector-effect', 'non-scaling-stroke') - .attr('d', - 'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z') - .call(Drawing.setClipUrl, plotinfo.layerClipId); - - appendBarText(gd, bar, d, i, x0, x1, y0, y1); - - if(plotinfo.layerClipId) { - Drawing.hideOutsideRangePoint(d[i], bar.select('text'), xa, ya, trace.xcalendar, trace.ycalendar); - } - }); - }); - - // error bars are on the top - Registry.getComponentMethod('errorbars', 'plot')(bartraces, plotinfo); + bartraces.order(); - // lastly, clip points groups of `cliponaxis !== false` traces - // on `plotinfo._hasClipOnAxisFalse === true` subplots bartraces.each(function(d) { + var cd0 = d[0]; + var t = cd0.t; + var trace = cd0.trace; + var sel = d3.select(this); + + if(!plotinfo.isRangePlot) cd0.node3 = sel; + + var poffset = t.poffset; + var poffsetIsArray = Array.isArray(poffset); + + var bars = sel.select('g.points').selectAll('g.point').data(Lib.identity); + + bars.enter().append('g') + .classed('point', true); + + bars.exit().remove(); + + bars.each(function(di, i) { + var bar = d3.select(this); + + // now display the bar + // clipped xf/yf (2nd arg true): non-positive + // log values go off-screen by plotwidth + // so you see them continue if you drag the plot + var p0 = di.p + ((poffsetIsArray) ? poffset[i] : poffset), + p1 = p0 + di.w, + s0 = di.b, + s1 = s0 + di.s; + + var x0, x1, y0, y1; + if(trace.orientation === 'h') { + y0 = ya.c2p(p0, true); + y1 = ya.c2p(p1, true); + x0 = xa.c2p(s0, true); + x1 = xa.c2p(s1, true); + + // for selections + di.ct = [x1, (y0 + y1) / 2]; + } + else { + x0 = xa.c2p(p0, true); + x1 = xa.c2p(p1, true); + y0 = ya.c2p(s0, true); + y1 = ya.c2p(s1, true); + + // for selections + di.ct = [(x0 + x1) / 2, y1]; + } + + if(!isNumeric(x0) || !isNumeric(x1) || + !isNumeric(y0) || !isNumeric(y1) || + x0 === x1 || y0 === y1) { + bar.remove(); + return; + } + + var lw = (di.mlw + 1 || trace.marker.line.width + 1 || + (di.trace ? di.trace.marker.line.width : 0) + 1) - 1, + offset = d3.round((lw / 2) % 1, 2); + + function roundWithLine(v) { + // if there are explicit gaps, don't round, + // it can make the gaps look crappy + return (fullLayout.bargap === 0 && fullLayout.bargroupgap === 0) ? + d3.round(Math.round(v) - offset, 2) : v; + } + + function expandToVisible(v, vc) { + // if it's not in danger of disappearing entirely, + // round more precisely + return Math.abs(v - vc) >= 2 ? roundWithLine(v) : + // but if it's very thin, expand it so it's + // necessarily visible, even if it might overlap + // its neighbor + (v > vc ? Math.ceil(v) : Math.floor(v)); + } + + if(!gd._context.staticPlot) { + // if bars are not fully opaque or they have a line + // around them, round to integer pixels, mainly for + // safari so we prevent overlaps from its expansive + // pixelation. if the bars ARE fully opaque and have + // no line, expand to a full pixel to make sure we + // can see them + var op = Color.opacity(di.mc || trace.marker.color), + fixpx = (op < 1 || lw > 0.01) ? + roundWithLine : expandToVisible; + x0 = fixpx(x0, x1); + x1 = fixpx(x1, x0); + y0 = fixpx(y0, y1); + y1 = fixpx(y1, y0); + } + + Lib.ensureSingle(bar, 'path') + .style('vector-effect', 'non-scaling-stroke') + .attr('d', + 'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z') + .call(Drawing.setClipUrl, plotinfo.layerClipId); + + appendBarText(gd, bar, d, i, x0, x1, y0, y1); + + if(plotinfo.layerClipId) { + Drawing.hideOutsideRangePoint(d[i], bar.select('text'), xa, ya, trace.xcalendar, trace.ycalendar); + } + }); + + // lastly, clip points groups of `cliponaxis !== false` traces + // on `plotinfo._hasClipOnAxisFalse === true` subplots var hasClipOnAxisFalse = d[0].trace.cliponaxis === false; - Drawing.setClipUrl(d3.select(this), hasClipOnAxisFalse ? null : plotinfo.layerClipId); + Drawing.setClipUrl(sel, hasClipOnAxisFalse ? null : plotinfo.layerClipId); }); + + // error bars are on the top + Registry.getComponentMethod('errorbars', 'plot')(bartraces, plotinfo); }; function appendBarText(gd, bar, calcTrace, i, x0, x1, y0, y1) { var textPosition; function appendTextNode(bar, text, textFont) { - var textSelection = bar.append('text') + var textSelection = Lib.ensureSingle(bar, 'text') .text(text) .attr({ 'class': 'bartext bartext-' + textPosition, diff --git a/src/traces/box/plot.js b/src/traces/box/plot.js index 042c7363d10..7ca8763876d 100644 --- a/src/traces/box/plot.js +++ b/src/traces/box/plot.js @@ -24,10 +24,15 @@ function plot(gd, plotinfo, cdbox) { var boxtraces = plotinfo.plot.select('.boxlayer') .selectAll('g.trace.boxes') - .data(cdbox) - .enter().append('g') + .data(cdbox, function(d) { return d[0].trace.uid; }); + + boxtraces.enter().append('g') .attr('class', 'trace boxes'); + boxtraces.exit().remove(); + + boxtraces.order(); + boxtraces.each(function(d) { var cd0 = d[0]; var t = cd0.t; @@ -105,67 +110,70 @@ function plotBoxAndWhiskers(sel, axes, trace, t) { bdPos1 = t.bdPos; } - sel.selectAll('path.box') - .data(Lib.identity) - .enter().append('path') + var paths = sel.selectAll('path.box').data(Lib.identity); + + paths.enter().append('path') .style('vector-effect', 'non-scaling-stroke') - .attr('class', 'box') - .each(function(d) { - var pos = d.pos; - var posc = posAxis.c2p(pos + bPos, true) + bPosPxOffset; - var pos0 = posAxis.c2p(pos + bPos - bdPos0, true) + bPosPxOffset; - var pos1 = posAxis.c2p(pos + bPos + bdPos1, true) + bPosPxOffset; - var posw0 = posAxis.c2p(pos + bPos - wdPos, true) + bPosPxOffset; - var posw1 = posAxis.c2p(pos + bPos + wdPos, true) + bPosPxOffset; - var posm0 = posAxis.c2p(pos + bPos - bdPos0 * nw, true) + bPosPxOffset; - var posm1 = posAxis.c2p(pos + bPos + bdPos1 * nw, true) + bPosPxOffset; - var q1 = valAxis.c2p(d.q1, true); - var q3 = valAxis.c2p(d.q3, true); - // make sure median isn't identical to either of the - // quartiles, so we can see it - var m = Lib.constrain( - valAxis.c2p(d.med, true), - Math.min(q1, q3) + 1, Math.max(q1, q3) - 1 - ); + .attr('class', 'box'); + + paths.exit().remove(); + + paths.each(function(d) { + var pos = d.pos; + var posc = posAxis.c2p(pos + bPos, true) + bPosPxOffset; + var pos0 = posAxis.c2p(pos + bPos - bdPos0, true) + bPosPxOffset; + var pos1 = posAxis.c2p(pos + bPos + bdPos1, true) + bPosPxOffset; + var posw0 = posAxis.c2p(pos + bPos - wdPos, true) + bPosPxOffset; + var posw1 = posAxis.c2p(pos + bPos + wdPos, true) + bPosPxOffset; + var posm0 = posAxis.c2p(pos + bPos - bdPos0 * nw, true) + bPosPxOffset; + var posm1 = posAxis.c2p(pos + bPos + bdPos1 * nw, true) + bPosPxOffset; + var q1 = valAxis.c2p(d.q1, true); + var q3 = valAxis.c2p(d.q3, true); + // make sure median isn't identical to either of the + // quartiles, so we can see it + var m = Lib.constrain( + valAxis.c2p(d.med, true), + Math.min(q1, q3) + 1, Math.max(q1, q3) - 1 + ); + + // for compatibility with box, violin, and candlestick + // perhaps we should put this into cd0.t instead so it's more explicit, + // but what we have now is: + // - box always has d.lf, but boxpoints can be anything + // - violin has d.lf and should always use it (boxpoints is undefined) + // - candlestick has only min/max + var useExtremes = (d.lf === undefined) || (trace.boxpoints === false); + var lf = valAxis.c2p(useExtremes ? d.min : d.lf, true); + var uf = valAxis.c2p(useExtremes ? d.max : d.uf, true); + var ln = valAxis.c2p(d.ln, true); + var un = valAxis.c2p(d.un, true); - // for compatibility with box, violin, and candlestick - // perhaps we should put this into cd0.t instead so it's more explicit, - // but what we have now is: - // - box always has d.lf, but boxpoints can be anything - // - violin has d.lf and should always use it (boxpoints is undefined) - // - candlestick has only min/max - var useExtremes = (d.lf === undefined) || (trace.boxpoints === false); - var lf = valAxis.c2p(useExtremes ? d.min : d.lf, true); - var uf = valAxis.c2p(useExtremes ? d.max : d.uf, true); - var ln = valAxis.c2p(d.ln, true); - var un = valAxis.c2p(d.un, true); - - if(trace.orientation === 'h') { - d3.select(this).attr('d', - 'M' + m + ',' + posm0 + 'V' + posm1 + // median line - 'M' + q1 + ',' + pos0 + 'V' + pos1 + // left edge - (notched ? 'H' + ln + 'L' + m + ',' + posm1 + 'L' + un + ',' + pos1 : '') + // top notched edge - 'H' + q3 + // end of the top edge - 'V' + pos0 + // right edge - (notched ? 'H' + un + 'L' + m + ',' + posm0 + 'L' + ln + ',' + pos0 : '') + // bottom notched edge - 'Z' + // end of the box - 'M' + q1 + ',' + posc + 'H' + lf + 'M' + q3 + ',' + posc + 'H' + uf + // whiskers - ((whiskerWidth === 0) ? '' : // whisker caps - 'M' + lf + ',' + posw0 + 'V' + posw1 + 'M' + uf + ',' + posw0 + 'V' + posw1)); - } else { - d3.select(this).attr('d', - 'M' + posm0 + ',' + m + 'H' + posm1 + // median line - 'M' + pos0 + ',' + q1 + 'H' + pos1 + // top of the box - (notched ? 'V' + ln + 'L' + posm1 + ',' + m + 'L' + pos1 + ',' + un : '') + // notched right edge - 'V' + q3 + // end of the right edge - 'H' + pos0 + // bottom of the box - (notched ? 'V' + un + 'L' + posm0 + ',' + m + 'L' + pos0 + ',' + ln : '') + // notched left edge - 'Z' + // end of the box - 'M' + posc + ',' + q1 + 'V' + lf + 'M' + posc + ',' + q3 + 'V' + uf + // whiskers - ((whiskerWidth === 0) ? '' : // whisker caps - 'M' + posw0 + ',' + lf + 'H' + posw1 + 'M' + posw0 + ',' + uf + 'H' + posw1)); - } - }); + if(trace.orientation === 'h') { + d3.select(this).attr('d', + 'M' + m + ',' + posm0 + 'V' + posm1 + // median line + 'M' + q1 + ',' + pos0 + 'V' + pos1 + // left edge + (notched ? 'H' + ln + 'L' + m + ',' + posm1 + 'L' + un + ',' + pos1 : '') + // top notched edge + 'H' + q3 + // end of the top edge + 'V' + pos0 + // right edge + (notched ? 'H' + un + 'L' + m + ',' + posm0 + 'L' + ln + ',' + pos0 : '') + // bottom notched edge + 'Z' + // end of the box + 'M' + q1 + ',' + posc + 'H' + lf + 'M' + q3 + ',' + posc + 'H' + uf + // whiskers + ((whiskerWidth === 0) ? '' : // whisker caps + 'M' + lf + ',' + posw0 + 'V' + posw1 + 'M' + uf + ',' + posw0 + 'V' + posw1)); + } else { + d3.select(this).attr('d', + 'M' + posm0 + ',' + m + 'H' + posm1 + // median line + 'M' + pos0 + ',' + q1 + 'H' + pos1 + // top of the box + (notched ? 'V' + ln + 'L' + posm1 + ',' + m + 'L' + pos1 + ',' + un : '') + // notched right edge + 'V' + q3 + // end of the right edge + 'H' + pos0 + // bottom of the box + (notched ? 'V' + un + 'L' + posm0 + ',' + m + 'L' + pos0 + ',' + ln : '') + // notched left edge + 'Z' + // end of the box + 'M' + posc + ',' + q1 + 'V' + lf + 'M' + posc + ',' + q3 + 'V' + uf + // whiskers + ((whiskerWidth === 0) ? '' : // whisker caps + 'M' + posw0 + ',' + lf + 'H' + posw1 + 'M' + posw0 + ',' + uf + 'H' + posw1)); + } + }); } function plotPoints(sel, axes, trace, t) { @@ -180,7 +188,7 @@ function plotPoints(sel, axes, trace, t) { // repeatable pseudo-random number generator Lib.seedPseudoRandom(); - sel.selectAll('g.points') + var gPoints = sel.selectAll('g.points') // since box plot points get an extra level of nesting, each // box needs the trace styling info .data(function(d) { @@ -189,10 +197,14 @@ function plotPoints(sel, axes, trace, t) { v.trace = trace; }); return d; - }) - .enter().append('g') - .attr('class', 'points') - .selectAll('path') + }); + + gPoints.enter().append('g') + .attr('class', 'points'); + + gPoints.exit().remove(); + + var paths = gPoints.selectAll('path') .data(function(d) { var i; @@ -265,10 +277,14 @@ function plotPoints(sel, axes, trace, t) { } return pts; - }) - .enter().append('path') - .classed('point', true) - .call(Drawing.translatePoints, xa, ya); + }); + + paths.enter().append('path') + .classed('point', true); + + paths.exit().remove(); + + paths.call(Drawing.translatePoints, xa, ya); } function plotBoxMean(sel, axes, trace, t) { @@ -288,38 +304,41 @@ function plotBoxMean(sel, axes, trace, t) { bdPos1 = t.bdPos; } - sel.selectAll('path.mean') - .data(Lib.identity) - .enter().append('path') + var paths = sel.selectAll('path.mean').data(Lib.identity); + + paths.enter().append('path') .attr('class', 'mean') .style({ fill: 'none', 'vector-effect': 'non-scaling-stroke' - }) - .each(function(d) { - var posc = posAxis.c2p(d.pos + bPos, true) + bPosPxOffset; - var pos0 = posAxis.c2p(d.pos + bPos - bdPos0, true) + bPosPxOffset; - var pos1 = posAxis.c2p(d.pos + bPos + bdPos1, true) + bPosPxOffset; - var m = valAxis.c2p(d.mean, true); - var sl = valAxis.c2p(d.mean - d.sd, true); - var sh = valAxis.c2p(d.mean + d.sd, true); - - if(trace.orientation === 'h') { - d3.select(this).attr('d', - 'M' + m + ',' + pos0 + 'V' + pos1 + - (trace.boxmean === 'sd' ? - 'm0,0L' + sl + ',' + posc + 'L' + m + ',' + pos0 + 'L' + sh + ',' + posc + 'Z' : - '') - ); - } else { - d3.select(this).attr('d', - 'M' + pos0 + ',' + m + 'H' + pos1 + - (trace.boxmean === 'sd' ? - 'm0,0L' + posc + ',' + sl + 'L' + pos0 + ',' + m + 'L' + posc + ',' + sh + 'Z' : - '') - ); - } }); + + paths.exit().remove(); + + paths.each(function(d) { + var posc = posAxis.c2p(d.pos + bPos, true) + bPosPxOffset; + var pos0 = posAxis.c2p(d.pos + bPos - bdPos0, true) + bPosPxOffset; + var pos1 = posAxis.c2p(d.pos + bPos + bdPos1, true) + bPosPxOffset; + var m = valAxis.c2p(d.mean, true); + var sl = valAxis.c2p(d.mean - d.sd, true); + var sh = valAxis.c2p(d.mean + d.sd, true); + + if(trace.orientation === 'h') { + d3.select(this).attr('d', + 'M' + m + ',' + pos0 + 'V' + pos1 + + (trace.boxmean === 'sd' ? + 'm0,0L' + sl + ',' + posc + 'L' + m + ',' + pos0 + 'L' + sh + ',' + posc + 'Z' : + '') + ); + } else { + d3.select(this).attr('d', + 'M' + pos0 + ',' + m + 'H' + pos1 + + (trace.boxmean === 'sd' ? + 'm0,0L' + posc + ',' + sl + 'L' + pos0 + ',' + m + 'L' + posc + ',' + sh + 'Z' : + '') + ); + } + }); } module.exports = { diff --git a/src/traces/violin/plot.js b/src/traces/violin/plot.js index 4b89227cad6..7bdeee3d659 100644 --- a/src/traces/violin/plot.js +++ b/src/traces/violin/plot.js @@ -34,10 +34,15 @@ module.exports = function plot(gd, plotinfo, cd) { var traces = plotinfo.plot.select('.violinlayer') .selectAll('g.trace.violins') - .data(cd) - .enter().append('g') + .data(cd, function(d) { return d[0].trace.uid; }); + + traces.enter().append('g') .attr('class', 'trace violins'); + traces.exit().remove(); + + traces.order(); + traces.each(function(d) { var cd0 = d[0]; var t = cd0.t; @@ -69,78 +74,81 @@ module.exports = function plot(gd, plotinfo, cd) { var hasMeanLine = trace.meanline && trace.meanline.visible; var groupStats = fullLayout._violinScaleGroupStats[trace.scalegroup]; - sel.selectAll('path.violin') - .data(Lib.identity) - .enter().append('path') + var violins = sel.selectAll('path.violin').data(Lib.identity); + + violins.enter().append('path') .style('vector-effect', 'non-scaling-stroke') - .attr('class', 'violin') - .each(function(d) { - var pathSel = d3.select(this); - var density = d.density; - var len = density.length; - var posCenter = d.pos + bPos; - var posCenterPx = posAxis.c2p(posCenter); - var scale; - - switch(trace.scalemode) { - case 'width': - scale = groupStats.maxWidth / bdPos; - break; - case 'count': - scale = (groupStats.maxWidth / bdPos) * (groupStats.maxCount / d.pts.length); - break; - } + .attr('class', 'violin'); + + violins.exit().remove(); + + violins.each(function(d) { + var pathSel = d3.select(this); + var density = d.density; + var len = density.length; + var posCenter = d.pos + bPos; + var posCenterPx = posAxis.c2p(posCenter); + var scale; + + switch(trace.scalemode) { + case 'width': + scale = groupStats.maxWidth / bdPos; + break; + case 'count': + scale = (groupStats.maxWidth / bdPos) * (groupStats.maxCount / d.pts.length); + break; + } - var pathPos, pathNeg, path; - var i, k, pts, pt; + var pathPos, pathNeg, path; + var i, k, pts, pt; - if(hasPositiveSide) { - pts = new Array(len); - for(i = 0; i < len; i++) { - pt = pts[i] = {}; - pt[t.posLetter] = posCenter + (density[i].v / scale); - pt[t.valLetter] = density[i].t; - } - pathPos = makePath(pts); + if(hasPositiveSide) { + pts = new Array(len); + for(i = 0; i < len; i++) { + pt = pts[i] = {}; + pt[t.posLetter] = posCenter + (density[i].v / scale); + pt[t.valLetter] = density[i].t; } + pathPos = makePath(pts); + } - if(hasNegativeSide) { - pts = new Array(len); - for(k = 0, i = len - 1; k < len; k++, i--) { - pt = pts[k] = {}; - pt[t.posLetter] = posCenter - (density[i].v / scale); - pt[t.valLetter] = density[i].t; - } - pathNeg = makePath(pts); + if(hasNegativeSide) { + pts = new Array(len); + for(k = 0, i = len - 1; k < len; k++, i--) { + pt = pts[k] = {}; + pt[t.posLetter] = posCenter - (density[i].v / scale); + pt[t.valLetter] = density[i].t; } + pathNeg = makePath(pts); + } + + if(hasBothSides) { + path = pathPos + 'L' + pathNeg.substr(1) + 'Z'; + } + else { + var startPt = [posCenterPx, valAxis.c2p(density[0].t)]; + var endPt = [posCenterPx, valAxis.c2p(density[len - 1].t)]; - if(hasBothSides) { - path = pathPos + 'L' + pathNeg.substr(1) + 'Z'; + if(trace.orientation === 'h') { + startPt.reverse(); + endPt.reverse(); } - else { - var startPt = [posCenterPx, valAxis.c2p(density[0].t)]; - var endPt = [posCenterPx, valAxis.c2p(density[len - 1].t)]; - - if(trace.orientation === 'h') { - startPt.reverse(); - endPt.reverse(); - } - - if(hasPositiveSide) { - path = 'M' + startPt + 'L' + pathPos.substr(1) + 'L' + endPt; - } else { - path = 'M' + endPt + 'L' + pathNeg.substr(1) + 'L' + startPt; - } + + if(hasPositiveSide) { + path = 'M' + startPt + 'L' + pathPos.substr(1) + 'L' + endPt; + } else { + path = 'M' + endPt + 'L' + pathNeg.substr(1) + 'L' + startPt; } - pathSel.attr('d', path); - - // save a few things used in getPositionOnKdePath, getKdeValue - // on hover and for meanline draw block below - d.posCenterPx = posCenterPx; - d.posDensityScale = scale * bdPos; - d.path = pathSel.node(); - d.pathLength = d.path.getTotalLength() / (hasBothSides ? 2 : 1); - }); + } + pathSel.attr('d', path); + + // save a few things used in getPositionOnKdePath, getKdeValue + // on hover and for meanline draw block below + d.posCenterPx = posCenterPx; + d.posDensityScale = scale * bdPos; + d.path = pathSel.node(); + d.pathLength = d.path.getTotalLength() / (hasBothSides ? 2 : 1); + }); if(hasBox) { var boxWidth = trace.box.width; @@ -179,24 +187,27 @@ module.exports = function plot(gd, plotinfo, cd) { } else { if(hasMeanLine) { - sel.selectAll('path.mean') - .data(Lib.identity) - .enter().append('path') + var meanPaths = sel.selectAll('path.mean').data(Lib.identity); + + meanPaths.enter().append('path') .attr('class', 'mean') .style({ fill: 'none', 'vector-effect': 'non-scaling-stroke' - }) - .each(function(d) { - var v = valAxis.c2p(d.mean, true); - var p = helpers.getPositionOnKdePath(d, trace, v); - - d3.select(this).attr('d', - trace.orientation === 'h' ? - 'M' + v + ',' + p[0] + 'V' + p[1] : - 'M' + p[0] + ',' + v + 'H' + p[1] - ); }); + + meanPaths.exit().remove(); + + meanPaths.each(function(d) { + var v = valAxis.c2p(d.mean, true); + var p = helpers.getPositionOnKdePath(d, trace, v); + + d3.select(this).attr('d', + trace.orientation === 'h' ? + 'M' + v + ',' + p[0] + 'V' + p[1] : + 'M' + p[0] + ',' + v + 'H' + p[1] + ); + }); } } From 31e5b12e90a2526c4da48f994a85ac9fee2d3332 Mon Sep 17 00:00:00 2001 From: etienne Date: Thu, 19 Apr 2018 17:58:04 -0400 Subject: [PATCH 02/18] lint and minor perf improvements in Cartesian.clean - use fullLayout[axName] instead of indexOf into axis list - for-in loop instead Object.keys + for-loop --- src/plots/cartesian/index.js | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index 8ffa10cdc7b..2388523f1e9 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -129,23 +129,20 @@ exports.plot = function(gd, traces, transitionOpts, makeOnCompleteCallback) { // traces are removed if(!Array.isArray(traces)) { traces = []; - - for(i = 0; i < calcdata.length; i++) { - traces.push(i); - } + for(i = 0; i < calcdata.length; i++) traces.push(i); } for(i = 0; i < subplots.length; i++) { - var subplot = subplots[i], - subplotInfo = fullLayout._plots[subplot]; + var subplot = subplots[i]; + var subplotInfo = fullLayout._plots[subplot]; // Get all calcdata for this subplot: var cdSubplot = []; var pcd; for(var j = 0; j < calcdata.length; j++) { - var cd = calcdata[j], - trace = cd[0].trace; + var cd = calcdata[j]; + var trace = cd[0].trace; // Skip trace if whitelist provided and it's not whitelisted: // if (Array.isArray(traces) && traces.indexOf(i) === -1) continue; @@ -285,12 +282,11 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) // delete any titles we don't need anymore // check if axis list has changed, and if so clear old titles if(oldSubplotList.xaxis && oldSubplotList.yaxis) { - var oldAxIDs = oldSubplotList.xaxis.concat(oldSubplotList.yaxis); - var newAxIDs = newSubplotList.xaxis.concat(newSubplotList.yaxis); - + var oldAxIDs = axisIds.listIds({_fullLayout: oldFullLayout}); for(i = 0; i < oldAxIDs.length; i++) { - if(newAxIDs.indexOf(oldAxIDs[i]) === -1) { - oldFullLayout._infolayer.selectAll('.g-' + oldAxIDs[i] + 'title').remove(); + var oldAxId = oldAxIDs[i]; + if(!newFullLayout[axisIds.id2name(oldAxId)]) { + oldFullLayout._infolayer.selectAll('.g-' + oldAxId + 'title').remove(); } } } @@ -516,11 +512,8 @@ function purgeSubplotLayers(layers, fullLayout) { // must remove overlaid subplot trace layers 'manually' - var subplots = fullLayout._plots; - var subplotIds = Object.keys(subplots); - - for(var i = 0; i < subplotIds.length; i++) { - var subplotInfo = subplots[subplotIds[i]]; + for(var k in fullLayout._plots) { + var subplotInfo = fullLayout._plots[k]; var overlays = subplotInfo.overlays || []; for(var j = 0; j < overlays.length; j++) { From 156f127275a6ffc636a3ea70e16f569cf509858e Mon Sep 17 00:00:00 2001 From: etienne Date: Thu, 19 Apr 2018 17:59:15 -0400 Subject: [PATCH 03/18] move up linkSubplots before cleanPlot - so that we can lookup subplots in newFullLayout._plots instead of indexOf into (something looong) subplots lists. --- src/plots/cartesian/index.js | 2 +- src/plots/plots.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index 2388523f1e9..c9e5024e346 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -304,7 +304,7 @@ exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) else if(oldSubplotList.cartesian) { for(i = 0; i < oldSubplotList.cartesian.length; i++) { var oldSubplotId = oldSubplotList.cartesian[i]; - if(newSubplotList.cartesian.indexOf(oldSubplotId) === -1) { + if(!newPlots[oldSubplotId]) { var selector = '.' + oldSubplotId + ',.' + oldSubplotId + '-x,.' + oldSubplotId + '-y'; oldFullLayout._cartesianlayer.selectAll(selector).remove(); removeSubplotExtras(oldSubplotId, oldFullLayout); diff --git a/src/plots/plots.js b/src/plots/plots.js index 05aa915e073..072a1323949 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -438,12 +438,12 @@ plots.supplyDefaults = function(gd) { newFullLayout._hasTernary = newFullLayout._has('ternary'); newFullLayout._hasPie = newFullLayout._has('pie'); - // clean subplots and other artifacts from previous plot calls - plots.cleanPlot(newFullData, newFullLayout, oldFullData, oldFullLayout, oldCalcdata); - // relink / initialize subplot axis objects plots.linkSubplots(newFullData, newFullLayout, oldFullData, oldFullLayout); + // clean subplots and other artifacts from previous plot calls + plots.cleanPlot(newFullData, newFullLayout, oldFullData, oldFullLayout, oldCalcdata); + // relink functions and _ attributes to promote consistency between plots relinkPrivateKeys(newFullLayout, oldFullLayout); From 13a15d1a10bdcfb688fd6d8a2ed557762fbf39f1 Mon Sep 17 00:00:00 2001 From: etienne Date: Thu, 19 Apr 2018 18:03:19 -0400 Subject: [PATCH 04/18] improve cartesian trace delete pattern ... when corresponding module is gone from fullLayout._modules To do so, call plot methods of visible modules AND of recently deleted ones, by stashing list of module on plot after Cartesian.plot in each plotinfo object. Make sure to list unique 'plot method' NOT unique trace module as sometimes multiple module use same 'plot method'. --- src/plots/cartesian/index.js | 105 ++++++++++++----------------------- src/plots/get_data.js | 32 ++++++++--- 2 files changed, 58 insertions(+), 79 deletions(-) diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index c9e5024e346..c4de914f6ad 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -179,106 +179,69 @@ exports.plot = function(gd, traces, transitionOpts, makeOnCompleteCallback) { }; function plotOne(gd, plotinfo, cdSubplot, transitionOpts, makeOnCompleteCallback) { - var fullLayout = gd._fullLayout, - modules = fullLayout._modules; - - // remove old traces, then redraw everything - // - // TODO: scatterlayer is manually excluded from this since it knows how - // to update instead of fully removing and redrawing every time. The - // remaining plot traces should also be able to do this. Once implemented, - // we won't need this - which should sometimes be a big speedup. - if(plotinfo.plot) { - plotinfo.plot.selectAll('g:not(.scatterlayer):not(.ohlclayer)').selectAll('g.trace').remove(); + var fullLayout = gd._fullLayout; + var modules = fullLayout._modules; + // list of plot methods to call (this list includes plot methods of 'gone' modules) + var plotMethodsToCall = plotinfo.plotMethods || []; + // list of plot methods of visible module on this subplot + var plotMethods = []; + var i, plotMethod; + + for(i = 0; i < modules.length; i++) { + var _module = modules[i]; + + if(_module.basePlotModule.name === 'cartesian') { + plotMethod = _module.plot; + Lib.pushUnique(plotMethodsToCall, plotMethod); + Lib.pushUnique(plotMethods, plotMethod); + } } - // plot all traces for each module at once - for(var j = 0; j < modules.length; j++) { - var _module = modules[j]; - - // skip over non-cartesian trace modules - if(!_module.plot || _module.basePlotModule.name !== 'cartesian') continue; + for(i = 0; i < plotMethodsToCall.length; i++) { + plotMethod = plotMethodsToCall[i]; // plot all traces of this type on this subplot at once - var cdModuleAndOthers = getModuleCalcData(cdSubplot, _module); + var cdModuleAndOthers = getModuleCalcData(cdSubplot, plotMethod); var cdModule = cdModuleAndOthers[0]; // don't need to search the found traces again - in fact we need to NOT // so that if two modules share the same plotter we don't double-plot cdSubplot = cdModuleAndOthers[1]; - _module.plot(gd, plotinfo, cdModule, transitionOpts, makeOnCompleteCallback); + plotMethod(gd, plotinfo, cdModule, transitionOpts, makeOnCompleteCallback); } + + // save list of plot methods on subplot for later, + // so that they can be called to clear traces of 'gone' modules + plotinfo.plotMethods = plotMethods; } exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) { - var oldModules = oldFullLayout._modules || []; - var newModules = newFullLayout._modules || []; var oldPlots = oldFullLayout._plots || {}; - - var hadScatter, hasScatter; - var hadOHLC, hasOHLC; - var hadGl, hasGl; - var i, k, subplotInfo, moduleName; + var newPlots = newFullLayout._plots || {}; + var oldSubplotList = oldFullLayout._subplots || {}; + var plotinfo; + var i, k; // when going from a large splom graph to something else, // we need to clear so that the new cartesian subplot // can have the correct layer ordering if(oldFullLayout._hasOnlyLargeSploms && !newFullLayout._hasOnlyLargeSploms) { for(k in oldPlots) { - subplotInfo = oldPlots[k]; - if(subplotInfo.plotgroup) subplotInfo.plotgroup.remove(); + plotinfo = oldPlots[k]; + if(plotinfo.plotgroup) plotinfo.plotgroup.remove(); } } - for(i = 0; i < oldModules.length; i++) { - moduleName = oldModules[i].name; - if(moduleName === 'scatter') hadScatter = true; - else if(moduleName === 'scattergl') hadGl = true; - else if(moduleName === 'ohlc') hadOHLC = true; - } - - for(i = 0; i < newModules.length; i++) { - moduleName = newModules[i].name; - if(moduleName === 'scatter') hasScatter = true; - else if(moduleName === 'scattergl') hasGl = true; - else if(moduleName === 'ohlc') hasOHLC = true; - } - - var layersToEmpty = []; - if(hadScatter && !hasScatter) layersToEmpty.push('g.scatterlayer'); - if(hadOHLC && !hasOHLC) layersToEmpty.push('g.ohlclayer'); - - if(layersToEmpty.length) { - for(var layeri = 0; layeri < layersToEmpty.length; layeri++) { - for(k in oldPlots) { - subplotInfo = oldPlots[k]; - if(subplotInfo.plot) { - subplotInfo.plot.select(layersToEmpty[layeri]) - .selectAll('g.trace') - .remove(); - } - } - - oldFullLayout._infolayer.selectAll('g.rangeslider-container') - .select(layersToEmpty[layeri]) - .selectAll('g.trace') - .remove(); - } - } + var hadGl = (oldFullLayout._has && oldFullLayout._has('gl')); + var hasGl = (newFullLayout._has && newFullLayout._has('gl')); if(hadGl && !hasGl) { for(k in oldPlots) { - subplotInfo = oldPlots[k]; - - if(subplotInfo._scene) { - subplotInfo._scene.destroy(); - } + plotinfo = oldPlots[k]; + if(plotinfo._scene) plotinfo._scene.destroy(); } } - var oldSubplotList = oldFullLayout._subplots || {}; - var newSubplotList = newFullLayout._subplots || {xaxis: [], yaxis: []}; - // delete any titles we don't need anymore // check if axis list has changed, and if so clear old titles if(oldSubplotList.xaxis && oldSubplotList.yaxis) { diff --git a/src/plots/get_data.js b/src/plots/get_data.js index e06ca4608c3..1e35a3767b4 100644 --- a/src/plots/get_data.js +++ b/src/plots/get_data.js @@ -45,26 +45,42 @@ exports.getSubplotCalcData = function(calcData, type, subplotId) { * the calcdata we *will* plot with this module, and the ones we *won't* * * @param {array} calcdata: as in gd.calcdata - * @param {object|string} typeOrModule: the plotting module, or its name + * @param {object|string|fn} arg1: + * the plotting module, or its name, or its plot method * * @return {array[array]} [foundCalcdata, remainingCalcdata] */ -exports.getModuleCalcData = function(calcdata, typeOrModule) { +exports.getModuleCalcData = function(calcdata, arg1) { var moduleCalcData = []; var remainingCalcData = []; - var _module = typeof typeOrModule === 'string' ? Registry.getModule(typeOrModule) : typeOrModule; - if(!_module) return [moduleCalcData, calcdata]; + + var plotMethod; + if(arg1) { + var typeOfArg1 = typeof arg1; + if(typeOfArg1 === 'string') { + plotMethod = Registry.getModule(arg1).plot; + } else if(typeOfArg1 === 'function') { + plotMethod = arg1; + } else { + plotMethod = arg1.plot; + } + } + if(!plotMethod) { + return [moduleCalcData, calcdata]; + } for(var i = 0; i < calcdata.length; i++) { var cd = calcdata[i]; var trace = cd[0].trace; if(trace.visible !== true) continue; - // we use this to find data to plot - so if there's a .plot - if(trace._module.plot === _module.plot) { + // group calcdata trace not by 'module' (as the name of this function + // would suggest), but by 'module plot method' so that if some traces + // share the same module plot method (e.g. bar and histogram), we + // only call it one! + if(trace._module.plot === plotMethod) { moduleCalcData.push(cd); - } - else { + } else { remainingCalcData.push(cd); } } From b4ea2ceeb81954db10c77c81f8ce3296d6ddef8e Mon Sep 17 00:00:00 2001 From: etienne Date: Thu, 19 Apr 2018 18:03:42 -0400 Subject: [PATCH 05/18] use plotMethods stash in range slider range plots --- src/components/rangeslider/draw.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/components/rangeslider/draw.js b/src/components/rangeslider/draw.js index ea673113a2f..d90766439df 100644 --- a/src/components/rangeslider/draw.js +++ b/src/components/rangeslider/draw.js @@ -448,15 +448,16 @@ function drawRangePlot(rangeSlider, gd, axisOpts, opts) { Plots.supplyDefaults(mockFigure); - var xa = mockFigure._fullLayout.xaxis, - ya = mockFigure._fullLayout[oppAxisName]; + var xa = mockFigure._fullLayout.xaxis; + var ya = mockFigure._fullLayout[oppAxisName]; var plotinfo = { id: id, plotgroup: plotgroup, xaxis: xa, yaxis: ya, - isRangePlot: true + isRangePlot: true, + plotMethods: opts._plotMethods }; if(isMainPlot) mainplotinfo = plotinfo; @@ -466,6 +467,10 @@ function drawRangePlot(rangeSlider, gd, axisOpts, opts) { } Cartesian.rangePlot(gd, plotinfo, filterRangePlotCalcData(calcData, id)); + + // stash list of plot methods on range-plot for later, + // so that they can be called to clear traces of 'gone' modules + opts._plotMethods = plotinfo.plotMethods; }); } From 072f02805350184ed520705a0d3bb007b2c010d3 Mon Sep 17 00:00:00 2001 From: etienne Date: Thu, 19 Apr 2018 18:05:44 -0400 Subject: [PATCH 06/18] fixup trace deletion for carpet traces - which is a weird case. It relies on trace.uid (like heatmaps/contour), but has group (like scatter/bar/...) which previously got deleted in that big selectAll('g.trace').remove() in Cartesian.plot. Now make _module.plot take care of trace deletion! --- src/traces/carpet/plot.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/traces/carpet/plot.js b/src/traces/carpet/plot.js index ce728562e93..c59366c9c0f 100644 --- a/src/traces/carpet/plot.js +++ b/src/traces/carpet/plot.js @@ -19,6 +19,12 @@ var Lib = require('../../lib'); var alignmentConstants = require('../../constants/alignment'); module.exports = function plot(gd, plotinfo, cdcarpet) { + if(!cdcarpet.length) { + plotinfo.plot.select('.carpetlayer') + .selectAll('g.trace') + .remove(); + } + for(var i = 0; i < cdcarpet.length; i++) { plotOne(gd, plotinfo, cdcarpet[i]); } @@ -33,7 +39,7 @@ function plotOne(gd, plotinfo, cd) { bax = trace.baxis, fullLayout = gd._fullLayout; - var gridLayer = plotinfo.plot.selectAll('.carpetlayer'); + var gridLayer = plotinfo.plot.select('.carpetlayer'); var clipLayer = fullLayout._clips; var axisLayer = Lib.ensureSingle(gridLayer, 'g', 'carpet' + trace.uid).classed('trace', true); From e5c860a10488e0ddfdb8eba445909a90b643f3c2 Mon Sep 17 00:00:00 2001 From: etienne Date: Fri, 20 Apr 2018 11:04:00 -0400 Subject: [PATCH 07/18] simplify arg1 -> plotMethod logic --- src/plots/get_data.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/plots/get_data.js b/src/plots/get_data.js index 1e35a3767b4..93b83f6c4d9 100644 --- a/src/plots/get_data.js +++ b/src/plots/get_data.js @@ -55,15 +55,12 @@ exports.getModuleCalcData = function(calcdata, arg1) { var remainingCalcData = []; var plotMethod; - if(arg1) { - var typeOfArg1 = typeof arg1; - if(typeOfArg1 === 'string') { - plotMethod = Registry.getModule(arg1).plot; - } else if(typeOfArg1 === 'function') { - plotMethod = arg1; - } else { - plotMethod = arg1.plot; - } + if(typeof arg1 === 'string') { + plotMethod = Registry.getModule(arg1).plot; + } else if(typeof arg1 === 'function') { + plotMethod = arg1; + } else { + plotMethod = arg1.plot; } if(!plotMethod) { return [moduleCalcData, calcdata]; From 400bfe6bd449772e6a8d92d310fa02b1bfa6e74b Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 24 Apr 2018 17:17:50 -0400 Subject: [PATCH 08/18] ping imagetest after having restarting nw.js - should fix some intermittent hangups when running the image tests locally --- tasks/baseline.js | 4 +--- tasks/test_export.js | 4 +--- tasks/test_image.js | 4 +--- tasks/util/container_commands.js | 16 +++++++++++----- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tasks/baseline.js b/tasks/baseline.js index 707ce8a74be..2e6f666a883 100644 --- a/tasks/baseline.js +++ b/tasks/baseline.js @@ -14,6 +14,4 @@ var cmd = containerCommands.getRunCmd( ); console.log(msg); -common.execCmd(containerCommands.ping, function() { - common.execCmd(cmd); -}); +common.execCmd(cmd); diff --git a/tasks/test_export.js b/tasks/test_export.js index d747d818b6e..de08f97e7e7 100644 --- a/tasks/test_export.js +++ b/tasks/test_export.js @@ -14,6 +14,4 @@ var cmd = containerCommands.getRunCmd( ); console.log(msg); -common.execCmd(containerCommands.ping, function() { - common.execCmd(cmd); -}); +common.execCmd(cmd); diff --git a/tasks/test_image.js b/tasks/test_image.js index eed7ccd9148..8156cabaead 100644 --- a/tasks/test_image.js +++ b/tasks/test_image.js @@ -14,6 +14,4 @@ var cmd = containerCommands.getRunCmd( ); console.log(msg); -common.execCmd(containerCommands.ping, function() { - common.execCmd(cmd); -}); +common.execCmd(cmd); diff --git a/tasks/util/container_commands.js b/tasks/util/container_commands.js index fa011c5eaf2..8a6200ccf77 100644 --- a/tasks/util/container_commands.js +++ b/tasks/util/container_commands.js @@ -21,7 +21,8 @@ containerCommands.setup = [ containerCommands.cpIndex, containerCommands.injectEnv, containerCommands.restart, - 'sleep 1', + containerCommands.ping, + 'sleep 5' ].join(' && '); containerCommands.dockerRun = [ @@ -34,12 +35,17 @@ containerCommands.dockerRun = [ containerCommands.getRunCmd = function(isCI, commands) { var _commands = Array.isArray(commands) ? commands.slice() : [commands]; + var cmd; - if(isCI) return getRunCI(_commands); + if(isCI) { + _commands = [containerCommands.ping].concat(_commands); + cmd = getRunCI(_commands); + } else { + _commands = [containerCommands.setup].concat(_commands); + cmd = getRunLocal(_commands); + } - // add setup commands locally - _commands = [containerCommands.setup].concat(_commands); - return getRunLocal(_commands); + return cmd; }; function getRunLocal(commands) { From 284c206d89b8dfcdea6af2cb525c42872051bca5 Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 24 Apr 2018 17:29:06 -0400 Subject: [PATCH 09/18] introduce new strategy for cartesian trace module layer updates - make trace module layers 'data-driven', one layer per unique plot module. Use _module.layerName to denote reused plot methods. - pass layer to trace _module.plot (as 3rd arg) to not mutate node data and allow a given _module.plot method to trace in multiple layers - no need for previous `_plotMethods` stash attempt - rename 'imagelayer' -> 'heatmaplayer', 'maplayer' -> 'contourlayer' for consistency for other trace modules --- src/components/rangeslider/draw.js | 7 +- src/plot_api/subroutines.js | 11 +-- src/plots/cartesian/constants.js | 7 +- src/plots/cartesian/index.js | 96 +++++++++++++++++--------- src/traces/bar/plot.js | 11 ++- src/traces/box/plot.js | 5 +- src/traces/candlestick/index.js | 1 + src/traces/carpet/plot.js | 5 +- src/traces/contour/plot.js | 45 +++++------- src/traces/contourcarpet/plot.js | 4 +- src/traces/heatmap/plot.js | 5 +- src/traces/histogram/index.js | 1 + src/traces/histogram2d/index.js | 1 + src/traces/histogram2dcontour/index.js | 1 + src/traces/ohlc/plot.js | 4 +- src/traces/scatter/plot.js | 20 +++--- src/traces/scattercarpet/plot.js | 9 ++- src/traces/scatterpolar/plot.js | 4 +- src/traces/scatterternary/plot.js | 4 +- src/traces/violin/plot.js | 5 +- 20 files changed, 128 insertions(+), 118 deletions(-) diff --git a/src/components/rangeslider/draw.js b/src/components/rangeslider/draw.js index d90766439df..079667bd7e2 100644 --- a/src/components/rangeslider/draw.js +++ b/src/components/rangeslider/draw.js @@ -456,8 +456,7 @@ function drawRangePlot(rangeSlider, gd, axisOpts, opts) { plotgroup: plotgroup, xaxis: xa, yaxis: ya, - isRangePlot: true, - plotMethods: opts._plotMethods + isRangePlot: true }; if(isMainPlot) mainplotinfo = plotinfo; @@ -467,10 +466,6 @@ function drawRangePlot(rangeSlider, gd, axisOpts, opts) { } Cartesian.rangePlot(gd, plotinfo, filterRangePlotCalcData(calcData, id)); - - // stash list of plot methods on range-plot for later, - // so that they can be called to clear traces of 'gone' modules - opts._plotMethods = plotinfo.plotMethods; }); } diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index dfd76a1021f..494dea1fb75 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -21,7 +21,6 @@ var Titles = require('../components/titles'); var ModeBar = require('../components/modebar'); var Axes = require('../plots/cartesian/axes'); -var cartesianConstants = require('../plots/cartesian/constants'); var alignmentConstants = require('../constants/alignment'); var axisConstraints = require('../plots/cartesian/constraints'); var enforceAxisConstraints = axisConstraints.enforce; @@ -199,15 +198,9 @@ exports.lsInner = function(gd) { Drawing.setClipUrl(plotinfo.plot, plotClipId); - for(i = 0; i < cartesianConstants.traceLayerClasses.length; i++) { - var layer = cartesianConstants.traceLayerClasses[i]; - if(layer !== 'scatterlayer' && layer !== 'barlayer') { - plotinfo.plot.selectAll('g.' + layer).call(Drawing.setClipUrl, layerClipId); - } - } - // stash layer clipId value (null or same as clipId) - // to DRY up Drawing.setClipUrl calls downstream + // to DRY up Drawing.setClipUrl calls on trace-module and trace layers + // downstream plotinfo.layerClipId = layerClipId; // figure out extra axis line and tick positions as needed diff --git a/src/plots/cartesian/constants.js b/src/plots/cartesian/constants.js index 59ac6f1b87f..6a2cd12ce72 100644 --- a/src/plots/cartesian/constants.js +++ b/src/plots/cartesian/constants.js @@ -61,15 +61,16 @@ module.exports = { DFLTRANGEY: [-1, 4], // Layers to keep trace types in the right order + // N.B. each 'unique' plot method must have its own layer traceLayerClasses: [ - 'imagelayer', - 'maplayer', + 'heatmaplayer', + 'contourcarpetlayer', 'contourlayer', 'barlayer', 'carpetlayer', 'violinlayer', 'boxlayer', 'ohlclayer', - 'scatterlayer' + 'scattercarpetlayer', 'scatterlayer' ], layerValue2layerClass: { diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index c4de914f6ad..59b507c86ca 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -10,10 +10,13 @@ 'use strict'; var d3 = require('d3'); + +var Registry = require('../../registry'); var Lib = require('../../lib'); var Plots = require('../plots'); -var getModuleCalcData = require('../get_data').getModuleCalcData; +var Drawing = require('../../components/drawing'); +var getModuleCalcData = require('../get_data').getModuleCalcData; var axisIds = require('./axis_ids'); var constants = require('./constants'); var xmlnsNamespaces = require('../../constants/xmlns_namespaces'); @@ -179,40 +182,75 @@ exports.plot = function(gd, traces, transitionOpts, makeOnCompleteCallback) { }; function plotOne(gd, plotinfo, cdSubplot, transitionOpts, makeOnCompleteCallback) { + var traceLayerClasses = constants.traceLayerClasses; var fullLayout = gd._fullLayout; var modules = fullLayout._modules; - // list of plot methods to call (this list includes plot methods of 'gone' modules) - var plotMethodsToCall = plotinfo.plotMethods || []; - // list of plot methods of visible module on this subplot - var plotMethods = []; - var i, plotMethod; - - for(i = 0; i < modules.length; i++) { - var _module = modules[i]; - - if(_module.basePlotModule.name === 'cartesian') { - plotMethod = _module.plot; - Lib.pushUnique(plotMethodsToCall, plotMethod); - Lib.pushUnique(plotMethods, plotMethod); + var _module, cdModuleAndOthers, cdModule; + + var layerData = []; + + for(var i = 0; i < traceLayerClasses.length; i++) { + var className = traceLayerClasses[i]; + + for(var j = 0; j < modules.length; j++) { + _module = modules[j]; + + if(_module.basePlotModule.name === 'cartesian' && + (_module.layerName || _module.name + 'layer') === className + ) { + var plotMethod = _module.plot; + + // plot all traces of this type on this subplot at once + cdModuleAndOthers = getModuleCalcData(cdSubplot, plotMethod); + cdModule = cdModuleAndOthers[0]; + // don't need to search the found traces again - in fact we need to NOT + // so that if two modules share the same plotter we don't double-plot + cdSubplot = cdModuleAndOthers[1]; + + if(cdModule.length) { + layerData.push({ + className: className, + plotMethod: plotMethod, + cdModule: cdModule + }); + } + break; + } } } - for(i = 0; i < plotMethodsToCall.length; i++) { - plotMethod = plotMethodsToCall[i]; + var layers = plotinfo.plot.selectAll('g.mlayer') + .data(layerData, function(d) { return d.className; }); - // plot all traces of this type on this subplot at once - var cdModuleAndOthers = getModuleCalcData(cdSubplot, plotMethod); - var cdModule = cdModuleAndOthers[0]; - // don't need to search the found traces again - in fact we need to NOT - // so that if two modules share the same plotter we don't double-plot - cdSubplot = cdModuleAndOthers[1]; + layers.enter().append('g') + .attr('class', function(d) { return d.className; }) + .classed('mlayer', true); - plotMethod(gd, plotinfo, cdModule, transitionOpts, makeOnCompleteCallback); - } + layers.exit().remove(); + + layers.order(); + + layers.each(function(d) { + var sel = d3.select(this); + var className = d.className; - // save list of plot methods on subplot for later, - // so that they can be called to clear traces of 'gone' modules - plotinfo.plotMethods = plotMethods; + d.plotMethod( + gd, plotinfo, d.cdModule, sel, + transitionOpts, makeOnCompleteCallback + ); + + // layers that allow `cliponaxis: false` + if(className !== 'scatterlayer' && className !== 'barlayer') { + Drawing.setClipUrl(sel, plotinfo.layerClipId); + } + }); + + // call Scattergl.plot separately + if(fullLayout._has('scattergl')) { + _module = Registry.getModule('scattergl'); + cdModule = getModuleCalcData(cdSubplot, _module)[0]; + _module.plot(gd, plotinfo, cdModule); + } } exports.clean = function(newFullData, newFullLayout, oldFullData, oldFullLayout) { @@ -441,10 +479,6 @@ function makeSubplotLayer(gd, plotinfo) { ensureSingleAndAddDatum(plotinfo.gridlayer, 'g', plotinfo.xaxis._id); ensureSingleAndAddDatum(plotinfo.gridlayer, 'g', plotinfo.yaxis._id); plotinfo.gridlayer.selectAll('g').sort(axisIds.idSort); - - for(var i = 0; i < constants.traceLayerClasses.length; i++) { - ensureSingle(plotinfo.plot, 'g', constants.traceLayerClasses[i]); - } } plotinfo.xlines diff --git a/src/traces/bar/plot.js b/src/traces/bar/plot.js index 0acc8eecd8e..31de45c3b29 100644 --- a/src/traces/bar/plot.js +++ b/src/traces/bar/plot.js @@ -30,13 +30,12 @@ var attributes = require('./attributes'), // padding in pixels around text var TEXTPAD = 3; -module.exports = function plot(gd, plotinfo, cdbar) { - var xa = plotinfo.xaxis, - ya = plotinfo.yaxis, - fullLayout = gd._fullLayout; +module.exports = function plot(gd, plotinfo, cdbar, barLayer) { + var xa = plotinfo.xaxis; + var ya = plotinfo.yaxis; + var fullLayout = gd._fullLayout; - var bartraces = plotinfo.plot.select('.barlayer') - .selectAll('g.trace.bars') + var bartraces = barLayer.selectAll('g.trace.bars') .data(cdbar, function(d) { return d[0].trace.uid; }); bartraces.enter().append('g') diff --git a/src/traces/box/plot.js b/src/traces/box/plot.js index 7ca8763876d..ade346c6f93 100644 --- a/src/traces/box/plot.js +++ b/src/traces/box/plot.js @@ -17,13 +17,12 @@ var Drawing = require('../../components/drawing'); var JITTERCOUNT = 5; // points either side of this to include var JITTERSPREAD = 0.01; // fraction of IQR to count as "dense" -function plot(gd, plotinfo, cdbox) { +function plot(gd, plotinfo, cdbox, boxLayer) { var fullLayout = gd._fullLayout; var xa = plotinfo.xaxis; var ya = plotinfo.yaxis; - var boxtraces = plotinfo.plot.select('.boxlayer') - .selectAll('g.trace.boxes') + var boxtraces = boxLayer.selectAll('g.trace.boxes') .data(cdbox, function(d) { return d[0].trace.uid; }); boxtraces.enter().append('g') diff --git a/src/traces/candlestick/index.js b/src/traces/candlestick/index.js index 4a6372585b7..12911def4df 100644 --- a/src/traces/candlestick/index.js +++ b/src/traces/candlestick/index.js @@ -36,6 +36,7 @@ module.exports = { supplyDefaults: require('./defaults'), calc: require('./calc'), plot: require('../box/plot').plot, + layerName: 'boxlayer', style: require('../box/style'), hoverPoints: require('../ohlc/hover'), selectPoints: require('../ohlc/select') diff --git a/src/traces/carpet/plot.js b/src/traces/carpet/plot.js index c59366c9c0f..0e270726da1 100644 --- a/src/traces/carpet/plot.js +++ b/src/traces/carpet/plot.js @@ -30,7 +30,7 @@ module.exports = function plot(gd, plotinfo, cdcarpet) { } }; -function plotOne(gd, plotinfo, cd) { +function plotOne(gd, plotinfo, cd, carpetLayer) { var t = cd[0]; var trace = cd[0].trace, xa = plotinfo.xaxis, @@ -39,10 +39,9 @@ function plotOne(gd, plotinfo, cd) { bax = trace.baxis, fullLayout = gd._fullLayout; - var gridLayer = plotinfo.plot.select('.carpetlayer'); var clipLayer = fullLayout._clips; - var axisLayer = Lib.ensureSingle(gridLayer, 'g', 'carpet' + trace.uid).classed('trace', true); + var axisLayer = Lib.ensureSingle(carpetLayer, 'g', 'carpet' + trace.uid).classed('trace', true); var minorLayer = Lib.ensureSingle(axisLayer, 'g', 'minorlayer'); var majorLayer = Lib.ensureSingle(axisLayer, 'g', 'majorlayer'); var boundaryLayer = Lib.ensureSingle(axisLayer, 'g', 'boundarylayer'); diff --git a/src/traces/contour/plot.js b/src/traces/contour/plot.js index 325a54c83c7..66f5e708216 100644 --- a/src/traces/contour/plot.js +++ b/src/traces/contour/plot.js @@ -33,25 +33,20 @@ exports.plot = function plot(gd, plotinfo, cdcontours) { } }; -function plotOne(gd, plotinfo, cd) { - var trace = cd[0].trace, - x = cd[0].x, - y = cd[0].y, - contours = trace.contours, - uid = trace.uid, - xa = plotinfo.xaxis, - ya = plotinfo.yaxis, - fullLayout = gd._fullLayout, - id = 'contour' + uid, - pathinfo = emptyPathinfo(contours, plotinfo, cd[0]); - - if(trace.visible !== true) { - fullLayout._paper.selectAll('.' + id + ',.hm' + uid).remove(); - fullLayout._infolayer.selectAll('.cb' + uid).remove(); - return; - } +function plotOne(gd, plotinfo, cd, contourLayer) { + var trace = cd[0].trace; + var x = cd[0].x; + var y = cd[0].y; + var contours = trace.contours; + var id = 'contour' + trace.uid; + var xa = plotinfo.xaxis; + var ya = plotinfo.yaxis; + var fullLayout = gd._fullLayout; + var pathinfo = emptyPathinfo(contours, plotinfo, cd[0]); // use a heatmap to fill - draw it behind the lines + var heatmapColoringLayer = Lib.ensureSingle(contourLayer, 'g', 'heatmapcoloring'); + var cdheatmaps = []; if(contours.coloring === 'heatmap') { if(trace.zauto && (trace.autocontour === false)) { trace._input.zmin = trace.zmin = @@ -59,15 +54,9 @@ function plotOne(gd, plotinfo, cd) { trace._input.zmax = trace.zmax = trace.zmin + pathinfo.length * contours.size; } - - heatmapPlot(gd, plotinfo, [cd]); - } - // in case this used to be a heatmap (or have heatmap fill) - else { - fullLayout._paper.selectAll('.hm' + uid).remove(); - fullLayout._infolayer.selectAll('g.rangeslider-container') - .selectAll('.hm' + uid).remove(); + cdheatmaps = [cd]; } + heatmapPlot(gd, plotinfo, cdheatmaps, heatmapColoringLayer); makeCrossings(pathinfo); findAllPaths(pathinfo); @@ -90,15 +79,15 @@ function plotOne(gd, plotinfo, cd) { } // draw everything - var plotGroup = exports.makeContourGroup(plotinfo, cd, id); + var plotGroup = exports.makeContourGroup(contourLayer, cd, id); makeBackground(plotGroup, perimeter, contours); makeFills(plotGroup, fillPathinfo, perimeter, contours); makeLinesAndLabels(plotGroup, pathinfo, gd, cd[0], contours, perimeter); clipGaps(plotGroup, plotinfo, fullLayout._clips, cd[0], perimeter); } -exports.makeContourGroup = function(plotinfo, cd, id) { - var plotgroup = plotinfo.plot.select('.maplayer') +exports.makeContourGroup = function(layer, cd, id) { + var plotgroup = layer .selectAll('g.contour.' + id) .data(cd); diff --git a/src/traces/contourcarpet/plot.js b/src/traces/contourcarpet/plot.js index 8ebfd7057b4..e55b2ce15fa 100644 --- a/src/traces/contourcarpet/plot.js +++ b/src/traces/contourcarpet/plot.js @@ -32,7 +32,7 @@ module.exports = function plot(gd, plotinfo, cdcontours) { } }; -function plotOne(gd, plotinfo, cd) { +function plotOne(gd, plotinfo, cd, contourcarpetLayer) { var trace = cd[0].trace; var carpet = trace._carpetTrace = lookupCarpet(gd, trace); @@ -96,7 +96,7 @@ function plotOne(gd, plotinfo, cd) { mapPathinfo(pathinfo, ab2p); // draw everything - var plotGroup = contourPlot.makeContourGroup(plotinfo, cd, id); + var plotGroup = contourPlot.makeContourGroup(contourcarpetLayer, cd, id); // Compute the boundary path var seg, xp, yp, i; diff --git a/src/traces/heatmap/plot.js b/src/traces/heatmap/plot.js index 00212e4bd5e..8b7d30ab4d0 100644 --- a/src/traces/heatmap/plot.js +++ b/src/traces/heatmap/plot.js @@ -25,7 +25,7 @@ module.exports = function(gd, plotinfo, cdheatmaps) { } }; -function plotOne(gd, plotinfo, cd) { +function plotOne(gd, plotinfo, cd, heatmapLayer) { var cd0 = cd[0]; var trace = cd0.trace; var uid = trace.uid; @@ -137,8 +137,7 @@ function plotOne(gd, plotinfo, cd) { // if image is entirely off-screen, don't even draw it var isOffScreen = (imageWidth <= 0 || imageHeight <= 0); - var plotgroup = plotinfo.plot.select('.imagelayer') - .selectAll('g.hm.' + id) + var plotgroup = heatmapLayer.selectAll('g.hm.' + id) .data(isOffScreen ? [] : [0]); plotgroup.enter().append('g') diff --git a/src/traces/histogram/index.js b/src/traces/histogram/index.js index c0949a06447..563689b2a4b 100644 --- a/src/traces/histogram/index.js +++ b/src/traces/histogram/index.js @@ -32,6 +32,7 @@ Histogram.supplyLayoutDefaults = require('../bar/layout_defaults'); Histogram.calc = require('./calc'); Histogram.setPositions = require('../bar/set_positions'); Histogram.plot = require('../bar/plot'); +Histogram.layerName = 'barlayer'; Histogram.style = require('../bar/style'); Histogram.colorbar = require('../scatter/colorbar'); Histogram.hoverPoints = require('./hover'); diff --git a/src/traces/histogram2d/index.js b/src/traces/histogram2d/index.js index c8e9695b8dc..af2549a9c18 100644 --- a/src/traces/histogram2d/index.js +++ b/src/traces/histogram2d/index.js @@ -15,6 +15,7 @@ Histogram2D.attributes = require('./attributes'); Histogram2D.supplyDefaults = require('./defaults'); Histogram2D.calc = require('../heatmap/calc'); Histogram2D.plot = require('../heatmap/plot'); +Histogram2D.layerName = 'heatmaplayer'; Histogram2D.colorbar = require('../heatmap/colorbar'); Histogram2D.style = require('../heatmap/style'); Histogram2D.hoverPoints = require('./hover'); diff --git a/src/traces/histogram2dcontour/index.js b/src/traces/histogram2dcontour/index.js index 7953ff48354..ccd98cc8bd7 100644 --- a/src/traces/histogram2dcontour/index.js +++ b/src/traces/histogram2dcontour/index.js @@ -15,6 +15,7 @@ Histogram2dContour.attributes = require('./attributes'); Histogram2dContour.supplyDefaults = require('./defaults'); Histogram2dContour.calc = require('../contour/calc'); Histogram2dContour.plot = require('../contour/plot').plot; +Histogram2dContour.layerName = 'contourlayer'; Histogram2dContour.style = require('../contour/style'); Histogram2dContour.colorbar = require('../contour/colorbar'); Histogram2dContour.hoverPoints = require('../contour/hover'); diff --git a/src/traces/ohlc/plot.js b/src/traces/ohlc/plot.js index 6a54b48a347..f915612c957 100644 --- a/src/traces/ohlc/plot.js +++ b/src/traces/ohlc/plot.js @@ -12,12 +12,10 @@ var d3 = require('d3'); var Lib = require('../../lib'); -module.exports = function plot(gd, plotinfo, cdOHLC) { +module.exports = function plot(gd, plotinfo, cdOHLC, ohlcLayer) { var xa = plotinfo.xaxis; var ya = plotinfo.yaxis; - var ohlcLayer = plotinfo.plot.select('g.ohlclayer'); - var traces = ohlcLayer.selectAll('g.trace') .data(cdOHLC, function(d) { return d[0].trace.uid; }); diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js index c9e92ee72e0..d75719bfa98 100644 --- a/src/traces/scatter/plot.js +++ b/src/traces/scatter/plot.js @@ -20,17 +20,15 @@ var linePoints = require('./line_points'); var linkTraces = require('./link_traces'); var polygonTester = require('../../lib/polygon').tester; -module.exports = function plot(gd, plotinfo, cdscatter, transitionOpts, makeOnCompleteCallback) { +module.exports = function plot(gd, plotinfo, cdscatter, scatterLayer, transitionOpts, makeOnCompleteCallback) { var i, uids, join, onComplete; - var scatterlayer = plotinfo.plot.select('g.scatterlayer'); - // If transition config is provided, then it is only a partial replot and traces not // updated are removed. var isFullReplot = !transitionOpts; var hasTransition = !!transitionOpts && transitionOpts.duration > 0; - join = scatterlayer.selectAll('g.trace') + join = scatterLayer.selectAll('g.trace') .data(cdscatter, function(d) { return d[0].trace.uid; }); // Append new traces: @@ -45,7 +43,7 @@ module.exports = function plot(gd, plotinfo, cdscatter, transitionOpts, makeOnCo // the z-order of fill layers is correct. linkTraces(gd, plotinfo, cdscatter); - createFills(gd, scatterlayer, plotinfo); + createFills(gd, scatterLayer, plotinfo); // Sort the traces, once created, so that the ordering is preserved even when traces // are shown and hidden. This is needed since we're not just wiping everything out @@ -54,7 +52,7 @@ module.exports = function plot(gd, plotinfo, cdscatter, transitionOpts, makeOnCo uids[cdscatter[i][0].trace.uid] = i; } - scatterlayer.selectAll('g.trace').sort(function(a, b) { + scatterLayer.selectAll('g.trace').sort(function(a, b) { var idx1 = uids[a[0].trace.uid]; var idx2 = uids[b[0].trace.uid]; return idx1 > idx2 ? 1 : -1; @@ -81,12 +79,12 @@ module.exports = function plot(gd, plotinfo, cdscatter, transitionOpts, makeOnCo transition.each(function() { // Must run the selection again since otherwise enters/updates get grouped together // and these get executed out of order. Except we need them in order! - scatterlayer.selectAll('g.trace').each(function(d, i) { + scatterLayer.selectAll('g.trace').each(function(d, i) { plotOne(gd, i, plotinfo, d, cdscatter, this, transitionOpts); }); }); } else { - scatterlayer.selectAll('g.trace').each(function(d, i) { + scatterLayer.selectAll('g.trace').each(function(d, i) { plotOne(gd, i, plotinfo, d, cdscatter, this, transitionOpts); }); } @@ -96,13 +94,13 @@ module.exports = function plot(gd, plotinfo, cdscatter, transitionOpts, makeOnCo } // remove paths that didn't get used - scatterlayer.selectAll('path:not([d])').remove(); + scatterLayer.selectAll('path:not([d])').remove(); }; -function createFills(gd, scatterlayer, plotinfo) { +function createFills(gd, scatterLayer, plotinfo) { var trace; - scatterlayer.selectAll('g.trace').each(function(d) { + scatterLayer.selectAll('g.trace').each(function(d) { var tr = d3.select(this); // Loop only over the traces being redrawn: diff --git a/src/traces/scattercarpet/plot.js b/src/traces/scattercarpet/plot.js index 6edb7a718c1..1b82fa59d95 100644 --- a/src/traces/scattercarpet/plot.js +++ b/src/traces/scattercarpet/plot.js @@ -13,26 +13,25 @@ var scatterPlot = require('../scatter/plot'); var Axes = require('../../plots/cartesian/axes'); var Drawing = require('../../components/drawing'); -module.exports = function plot(gd, plotinfoproxy, data) { +module.exports = function plot(gd, plotinfoproxy, data, layer) { var i, trace, node; var carpet = data[0][0].carpet; - // mimic cartesian plotinfo var plotinfo = { xaxis: Axes.getFromId(gd, carpet.xaxis || 'x'), yaxis: Axes.getFromId(gd, carpet.yaxis || 'y'), - plot: plotinfoproxy.plot + plot: plotinfoproxy.plot, }; - scatterPlot(gd, plotinfo, data); + scatterPlot(gd, plotinfo, data, layer); for(i = 0; i < data.length; i++) { trace = data[i][0].trace; // Note: .select is adequate but seems to mutate the node data, // which is at least a bit suprising and causes problems elsewhere - node = plotinfo.plot.selectAll('g.trace' + trace.uid + ' .js-line'); + node = layer.selectAll('g.trace' + trace.uid + ' .js-line'); // Note: it would be more efficient if this didn't need to be applied // separately to all scattercarpet traces, but that would require diff --git a/src/traces/scatterpolar/plot.js b/src/traces/scatterpolar/plot.js index 3e5cb62a30b..5ad3a019bef 100644 --- a/src/traces/scatterpolar/plot.js +++ b/src/traces/scatterpolar/plot.js @@ -60,5 +60,7 @@ module.exports = function plot(gd, subplot, moduleCalcData) { } } - scatterPlot(gd, plotinfo, moduleCalcData); + var scatterLayer = subplot.layers.frontplot.select('g.scatterlayer'); + + scatterPlot(gd, plotinfo, moduleCalcData, scatterLayer); }; diff --git a/src/traces/scatterternary/plot.js b/src/traces/scatterternary/plot.js index 26b323d306e..08ef636933a 100644 --- a/src/traces/scatterternary/plot.js +++ b/src/traces/scatterternary/plot.js @@ -25,5 +25,7 @@ module.exports = function plot(gd, ternary, moduleCalcData) { layerClipId: ternary._hasClipOnAxisFalse ? ternary.clipIdRelative : null }; - scatterPlot(gd, plotinfo, moduleCalcData); + var scatterLayer = ternary.layers.frontplot.select('g.scatterlayer'); + + scatterPlot(gd, plotinfo, moduleCalcData, scatterLayer); }; diff --git a/src/traces/violin/plot.js b/src/traces/violin/plot.js index 7bdeee3d659..99d70430fdc 100644 --- a/src/traces/violin/plot.js +++ b/src/traces/violin/plot.js @@ -15,7 +15,7 @@ var boxPlot = require('../box/plot'); var linePoints = require('../scatter/line_points'); var helpers = require('./helpers'); -module.exports = function plot(gd, plotinfo, cd) { +module.exports = function plot(gd, plotinfo, cd, violinLayer) { var fullLayout = gd._fullLayout; var xa = plotinfo.xaxis; var ya = plotinfo.yaxis; @@ -32,8 +32,7 @@ module.exports = function plot(gd, plotinfo, cd) { return Drawing.smoothopen(segments[0], 1); } - var traces = plotinfo.plot.select('.violinlayer') - .selectAll('g.trace.violins') + var traces = violinLayer.selectAll('g.trace.violins') .data(cd, function(d) { return d[0].trace.uid; }); traces.enter().append('g') From da63ed4fa9030caba7d7ecdbebc24e8728753d39 Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 24 Apr 2018 17:31:42 -0400 Subject: [PATCH 10/18] make heatmap/contour(carpet)/carpet plot method remove its own trace ... instead of relying on slow selectAll() during subroutines.drawData and Plots.cleanPlot. This can be a major perf improvement on graph with many DOM nodes (e.g. from many traces and/or many subplots) --- src/plot_api/subroutines.js | 27 +++------------------------ src/plots/plots.js | 25 ++++--------------------- src/traces/carpet/plot.js | 21 +++++++++++++-------- src/traces/contour/plot.js | 15 ++++++++++++--- src/traces/contourcarpet/plot.js | 21 ++++++++++++--------- src/traces/heatmap/plot.js | 31 ++++++++++++++----------------- 6 files changed, 58 insertions(+), 82 deletions(-) diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index 494dea1fb75..019072e20cf 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -488,34 +488,13 @@ exports.doCamera = function(gd) { exports.drawData = function(gd) { var fullLayout = gd._fullLayout; var calcdata = gd.calcdata; - var rangesliderContainers = fullLayout._infolayer.selectAll('g.rangeslider-container'); var i; - // in case of traces that were heatmaps or contour maps - // previously, remove them and their colorbars explicitly + // remove old colorbars explicitly for(i = 0; i < calcdata.length; i++) { var trace = calcdata[i][0].trace; - var isVisible = (trace.visible === true); - var uid = trace.uid; - - if(!isVisible || !Registry.traceIs(trace, '2dMap')) { - var query = ( - '.hm' + uid + - ',.contour' + uid + - ',#clip' + uid - ); - - fullLayout._paper - .selectAll(query) - .remove(); - - rangesliderContainers - .selectAll(query) - .remove(); - } - - if(!isVisible || !trace._module.colorbar) { - fullLayout._infolayer.selectAll('.cb' + uid).remove(); + if(trace.visible !== true || !trace._module.colorbar) { + fullLayout._infolayer.select('.cb' + trace.uid).remove(); } } diff --git a/src/plots/plots.js b/src/plots/plots.js index 072a1323949..1af954ec8b7 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -672,8 +672,6 @@ plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayou } } - var hasPaper = !!oldFullLayout._paper; - var hasInfoLayer = !!oldFullLayout._infolayer; var hadGl = oldFullLayout._has && oldFullLayout._has('gl'); var hasGl = newFullLayout._has && newFullLayout._has('gl'); @@ -684,6 +682,8 @@ plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayou } } + var hasInfoLayer = !!oldFullLayout._infolayer; + oldLoop: for(i = 0; i < oldFullData.length; i++) { var oldTrace = oldFullData[i], @@ -695,26 +695,9 @@ plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayou if(oldUid === newTrace.uid) continue oldLoop; } - var query = ( - '.hm' + oldUid + - ',.contour' + oldUid + - ',.carpet' + oldUid + - ',#clip' + oldUid + - ',.trace' + oldUid - ); - - // clean old heatmap, contour traces and clip paths - // that rely on uid identifiers - if(hasPaper) { - oldFullLayout._paper.selectAll(query).remove(); - } - - // clean old colorbars and range slider plot + // clean old colorbars if(hasInfoLayer) { - oldFullLayout._infolayer.selectAll('.cb' + oldUid).remove(); - - oldFullLayout._infolayer.selectAll('g.rangeslider-container') - .selectAll(query).remove(); + oldFullLayout._infolayer.select('.cb' + oldUid).remove(); } } diff --git a/src/traces/carpet/plot.js b/src/traces/carpet/plot.js index 0e270726da1..5e9d5434bb5 100644 --- a/src/traces/carpet/plot.js +++ b/src/traces/carpet/plot.js @@ -18,15 +18,20 @@ var svgTextUtils = require('../../lib/svg_text_utils'); var Lib = require('../../lib'); var alignmentConstants = require('../../constants/alignment'); -module.exports = function plot(gd, plotinfo, cdcarpet) { - if(!cdcarpet.length) { - plotinfo.plot.select('.carpetlayer') - .selectAll('g.trace') - .remove(); - } +module.exports = function plot(gd, plotinfo, cdcarpet, carpetLayer) { + var i; + + carpetLayer.selectAll('g.trace').each(function() { + var classString = d3.select(this).attr('class'); + var oldUid = classString.split('carpet')[1].split(/\s/)[0]; + for(i = 0; i < cdcarpet.length; i++) { + if(oldUid === cdcarpet[i][0].trace.uid) return; + } + d3.select(this).remove(); + }); - for(var i = 0; i < cdcarpet.length; i++) { - plotOne(gd, plotinfo, cdcarpet[i]); + for(i = 0; i < cdcarpet.length; i++) { + plotOne(gd, plotinfo, cdcarpet[i], carpetLayer); } }; diff --git a/src/traces/contour/plot.js b/src/traces/contour/plot.js index 66f5e708216..f5534b7fcdf 100644 --- a/src/traces/contour/plot.js +++ b/src/traces/contour/plot.js @@ -27,9 +27,18 @@ var constants = require('./constants'); var costConstants = constants.LABELOPTIMIZER; -exports.plot = function plot(gd, plotinfo, cdcontours) { - for(var i = 0; i < cdcontours.length; i++) { - plotOne(gd, plotinfo, cdcontours[i]); +exports.plot = function plot(gd, plotinfo, cdcontours, contourLayer) { + var i; + + contourLayer.selectAll('g.contour').each(function(d) { + for(i = 0; i < cdcontours.length; i++) { + if(d.trace.uid === cdcontours[i][0].trace.uid) return; + } + d3.select(this).remove(); + }); + + for(i = 0; i < cdcontours.length; i++) { + plotOne(gd, plotinfo, cdcontours[i], contourLayer); } }; diff --git a/src/traces/contourcarpet/plot.js b/src/traces/contourcarpet/plot.js index e55b2ce15fa..ab773a8fc69 100644 --- a/src/traces/contourcarpet/plot.js +++ b/src/traces/contourcarpet/plot.js @@ -26,9 +26,18 @@ var lookupCarpet = require('../carpet/lookup_carpetid'); var closeBoundaries = require('../contour/close_boundaries'); -module.exports = function plot(gd, plotinfo, cdcontours) { - for(var i = 0; i < cdcontours.length; i++) { - plotOne(gd, plotinfo, cdcontours[i]); +module.exports = function plot(gd, plotinfo, cdcontours, contourcarpetLayer) { + var i; + + contourcarpetLayer.selectAll('g.contour').each(function(d) { + for(i = 0; i < cdcontours.length; i++) { + if(d.trace.uid === cdcontours[i][0].trace.uid) return; + } + d3.select(this).remove(); + }); + + for(i = 0; i < cdcontours.length; i++) { + plotOne(gd, plotinfo, cdcontours[i], contourcarpetLayer); } }; @@ -46,7 +55,6 @@ function plotOne(gd, plotinfo, cd, contourcarpetLayer) { var uid = trace.uid; var xa = plotinfo.xaxis; var ya = plotinfo.yaxis; - var fullLayout = gd._fullLayout; var id = 'contour' + uid; var pathinfo = emptyPathinfo(contours, plotinfo, cd[0]); var isConstraint = contours.type === 'constraint'; @@ -59,11 +67,6 @@ function plotOne(gd, plotinfo, cd, contourcarpetLayer) { return [xa.c2p(pt[0]), ya.c2p(pt[1])]; } - if(trace.visible !== true) { - fullLayout._infolayer.selectAll('.cb' + uid).remove(); - return; - } - // Define the perimeter in a/b coordinates: var perimeter = [ [a[0], b[b.length - 1]], diff --git a/src/traces/heatmap/plot.js b/src/traces/heatmap/plot.js index 8b7d30ab4d0..9b125db9eff 100644 --- a/src/traces/heatmap/plot.js +++ b/src/traces/heatmap/plot.js @@ -9,6 +9,7 @@ 'use strict'; +var d3 = require('d3'); var tinycolor = require('tinycolor2'); var Registry = require('../../registry'); @@ -18,32 +19,28 @@ var xmlnsNamespaces = require('../../constants/xmlns_namespaces'); var maxRowLength = require('./max_row_length'); +module.exports = function(gd, plotinfo, cdheatmaps, heatmapLayer) { + var i; -module.exports = function(gd, plotinfo, cdheatmaps) { - for(var i = 0; i < cdheatmaps.length; i++) { - plotOne(gd, plotinfo, cdheatmaps[i]); + heatmapLayer.selectAll('.hm > image').each(function(d) { + var oldTrace = d.trace || {}; + for(i = 0; i < cdheatmaps.length; i++) { + if(oldTrace.uid === cdheatmaps[i][0].trace.uid) return; + } + d3.select(this.parentNode).remove(); + }); + + for(i = 0; i < cdheatmaps.length; i++) { + plotOne(gd, plotinfo, cdheatmaps[i], heatmapLayer); } }; function plotOne(gd, plotinfo, cd, heatmapLayer) { var cd0 = cd[0]; var trace = cd0.trace; - var uid = trace.uid; var xa = plotinfo.xaxis; var ya = plotinfo.yaxis; - var fullLayout = gd._fullLayout; - var id = 'hm' + uid; - - // in case this used to be a contour map - fullLayout._paper.selectAll('.contour' + uid).remove(); - fullLayout._infolayer.selectAll('g.rangeslider-container') - .selectAll('.contour' + uid).remove(); - - if(trace.visible !== true) { - fullLayout._paper.selectAll('.' + id).remove(); - fullLayout._infolayer.selectAll('.cb' + uid).remove(); - return; - } + var id = 'hm' + trace.uid; var z = cd0.z; var x = cd0.x; From a1c76bcf36c14ba2658ba9634a47d0e252cea044 Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 24 Apr 2018 17:33:33 -0400 Subject: [PATCH 11/18] sub imagelayer->heatmaplayer, maplayer->contourlayer in tests --- test/jasmine/tests/range_slider_test.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/test/jasmine/tests/range_slider_test.js b/test/jasmine/tests/range_slider_test.js index abbad0f08f1..4c725d5526d 100644 --- a/test/jasmine/tests/range_slider_test.js +++ b/test/jasmine/tests/range_slider_test.js @@ -422,7 +422,6 @@ describe('Rangeslider visibility property', function() { }); it('should clear traces in range plot when needed', function(done) { - function count(query) { return d3.select(getRangeSlider()).selectAll(query).size(); } @@ -468,12 +467,12 @@ describe('Rangeslider visibility property', function() { }]); }) .then(function() { - expect(count('g.imagelayer > g.hm')).toEqual(1); + expect(count('g.heatmaplayer > g.hm')).toEqual(1); return Plotly.restyle(gd, 'visible', false); }) .then(function() { - expect(count('g.imagelayer > g.hm')).toEqual(0); + expect(count('g.heatmaplayer > g.hm')).toEqual(0); return Plotly.restyle(gd, { visible: true, @@ -481,25 +480,25 @@ describe('Rangeslider visibility property', function() { }); }) .then(function() { - expect(count('g.maplayer > g.contour')).toEqual(1); + expect(count('g.contourlayer > g.contour')).toEqual(1); return Plotly.restyle(gd, 'type', 'heatmap'); }) .then(function() { - expect(count('g.imagelayer > g.hm')).toEqual(1); - expect(count('g.maplayer > g.contour')).toEqual(0); + expect(count('g.heatmaplayer > g.hm')).toEqual(1); + expect(count('g.contourlayer > g.contour')).toEqual(0); return Plotly.restyle(gd, 'type', 'contour'); }) .then(function() { - expect(count('g.imagelayer > g.hm')).toEqual(0); - expect(count('g.maplayer > g.contour')).toEqual(1); + expect(count('g.heatmaplayer > g.hm')).toEqual(0); + expect(count('g.contourlayer > g.contour')).toEqual(1); return Plotly.deleteTraces(gd, [0]); }) .then(function() { - expect(count('g.imagelayer > g.hm')).toEqual(0); - expect(count('g.maplayer > g.contour')).toEqual(0); + expect(count('g.heatmaplayer > g.hm')).toEqual(0); + expect(count('g.contourlayer > g.contour')).toEqual(0); }) .catch(failTest) .then(done); From fa38ca5c7b677315f55d4c460f62908db7a8b074 Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 24 Apr 2018 17:34:19 -0400 Subject: [PATCH 12/18] update `cliponaxis: false` ... to take into account take now not all trace module layers are added to each subplot. --- test/jasmine/tests/bar_test.js | 24 +++++++++++++++++++----- test/jasmine/tests/scatter_test.js | 24 +++++++++++++++++++----- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/test/jasmine/tests/bar_test.js b/test/jasmine/tests/bar_test.js index 29cfd175833..00e3d4f4712 100644 --- a/test/jasmine/tests/bar_test.js +++ b/test/jasmine/tests/bar_test.js @@ -1549,16 +1549,30 @@ describe('bar hover', function() { var fig = Lib.extendDeep({}, require('@mocks/bar_cliponaxis-false.json')); gd = createGraphDiv(); - // only show one trace + // only show one bar trace fig.data = [fig.data[0]]; + // add a non-bar trace to make sure its module layer gets clipped + fig.data.push({ + type: 'contour', + z: [[0, 0.5, 1], [0.5, 1, 3]] + }); + + function _assertClip(sel, exp, size, msg) { + if(exp === null) { + expect(sel.size()).toBe(0, msg + 'selection should not exist'); + } else { + assertClip(sel, exp, size, msg); + } + } + function _assert(layerClips, barDisplays, barTextDisplays, barClips) { var subplotLayer = d3.select('.plot'); var barLayer = subplotLayer.select('.barlayer'); - assertClip(subplotLayer, layerClips[0], 1, 'subplot layer'); - assertClip(subplotLayer.select('.maplayer'), layerClips[1], 1, 'some other trace layer'); - assertClip(barLayer, layerClips[2], 1, 'bar layer'); + _assertClip(subplotLayer, layerClips[0], 1, 'subplot layer'); + _assertClip(subplotLayer.select('.contourlayer'), layerClips[1], 1, 'some other trace layer'); + _assertClip(barLayer, layerClips[2], 1, 'bar layer'); assertNodeDisplay( barLayer.selectAll('.point'), @@ -1589,7 +1603,7 @@ describe('bar hover', function() { }) .then(function() { _assert( - [true, false, false], + [true, null, null], [], [], [false, 0] diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index ec295c7d9d9..cf574adedf1 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/test/jasmine/tests/scatter_test.js @@ -1231,13 +1231,27 @@ describe('Test scatter *clipnaxis*:', function() { // add lines fig.data[0].mode = 'markers+lines+text'; + // add a non-scatter trace to make sure its module layer gets clipped + fig.data.push({ + type: 'contour', + z: [[0, 0.5, 1], [0.5, 1, 3]] + }); + + function _assertClip(sel, exp, size, msg) { + if(exp === null) { + expect(sel.size()).toBe(0, msg + 'selection should not exist'); + } else { + assertClip(sel, exp, size, msg); + } + } + function _assert(layerClips, nodeDisplays, errorBarClips, lineClips) { var subplotLayer = d3.select('.plot'); var scatterLayer = subplotLayer.select('.scatterlayer'); - assertClip(subplotLayer, layerClips[0], 1, 'subplot layer'); - assertClip(subplotLayer.select('.maplayer'), layerClips[1], 1, 'some other trace layer'); - assertClip(scatterLayer, layerClips[2], 1, 'scatter layer'); + _assertClip(subplotLayer, layerClips[0], 1, 'subplot layer'); + _assertClip(subplotLayer.select('.contourlayer'), layerClips[1], 1, 'some other trace layer'); + _assertClip(scatterLayer, layerClips[2], 1, 'scatter layer'); assertNodeDisplay( scatterLayer.selectAll('.point'), @@ -1274,7 +1288,7 @@ describe('Test scatter *clipnaxis*:', function() { }) .then(function() { _assert( - [true, false, false], + [true, null, null], [], [false, 0], [false, 0] @@ -1292,7 +1306,7 @@ describe('Test scatter *clipnaxis*:', function() { }) .then(function() { _assert( - [true, false, false], + [true, null, null], [], [false, 0], [false, 0] From 48ee0d87cbcc872cb816ace72336c35711b6a7a0 Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 24 Apr 2018 17:36:31 -0400 Subject: [PATCH 13/18] :lock: contour with heatmap coloring layer order - previously, Contour.plot called Heatmap.call for traces with 'contour.coloring': 'heatmap' which plotted a heatmap in always below the contours. Now as does not always exist, we plot the heatmap fill inside in a ensureSingle layer. --- test/jasmine/tests/contour_test.js | 51 ++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/test/jasmine/tests/contour_test.js b/test/jasmine/tests/contour_test.js index dea87bd6869..98b50bd77be 100644 --- a/test/jasmine/tests/contour_test.js +++ b/test/jasmine/tests/contour_test.js @@ -407,4 +407,55 @@ describe('contour plotting and editing', function() { .catch(fail) .then(done); }); + + it('should always draw heatmap coloring layer below contour lines', function(done) { + var cnt = 0; + + function _assert(exp) { + var msg = ' index in (call #' + cnt + ')'; + var contourLayer = gd.querySelector('.xy > .plot > .contourlayer'); + var hmIndex = -1; + var contoursIndex = -1; + + for(var i in contourLayer.children) { + var child = contourLayer.children[i]; + if(child.querySelector) { + if(child.querySelector('.hm')) hmIndex = +i; + else if(child.querySelector('.contourlevel')) contoursIndex = +i; + } + } + + expect(hmIndex).toBe(exp.hmIndex, 'heatmap' + msg); + expect(contoursIndex).toBe(exp.contoursIndex, 'contours' + msg); + cnt++; + } + + Plotly.newPlot(gd, [{ + type: 'contour', + z: [[1, 2, 3], [1, 3, 0]], + contours: {coloring: 'heatmap'} + }]) + .then(function() { + _assert({ + hmIndex: 0, + contoursIndex: 1 + }); + return Plotly.restyle(gd, 'contours.coloring', 'lines'); + }) + .then(function() { + _assert({ + hmIndex: -1, + contoursIndex: 1 + }); + return Plotly.restyle(gd, 'contours.coloring', 'heatmap'); + }) + .then(function() { + _assert({ + hmIndex: 0, + contoursIndex: 1 + }); + }) + .catch(fail) + .then(done); + }); }); From 362da1fe1bb78cb28f4e23fc2cba57cd1756d95b Mon Sep 17 00:00:00 2001 From: etienne Date: Tue, 24 Apr 2018 17:36:47 -0400 Subject: [PATCH 14/18] :lock: scattercarpet + scatter coexistence --- test/jasmine/tests/carpet_test.js | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/jasmine/tests/carpet_test.js b/test/jasmine/tests/carpet_test.js index 1bae596789d..db602897759 100644 --- a/test/jasmine/tests/carpet_test.js +++ b/test/jasmine/tests/carpet_test.js @@ -511,6 +511,32 @@ describe('Test carpet interactions:', function() { .catch(fail) .then(done); }); + + it('scattercarpet should be able to coexist with scatter traces', function(done) { + var mock = Lib.extendDeep({}, require('@mocks/scattercarpet.json')); + + function _assert(exp) { + expect(d3.selectAll('.point').size()) + .toBe(exp, 'number of scatter pts on graph'); + } + + Plotly.newPlot(gd, mock).then(function() { + _assert(12); + + return Plotly.addTraces(gd, { + y: [1, 2, 1] + }); + }) + .then(function() { + _assert(15); + return Plotly.deleteTraces(gd, [0]); + }) + .then(function() { + _assert(3); + }) + .catch(fail) + .then(done); + }); }); describe('scattercarpet array attributes', function() { From 3bef9a99c2439e74d582c028a2c1e02c70739ffd Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 30 Apr 2018 11:03:23 -0400 Subject: [PATCH 15/18] add getUidsFromCalcData helper --- src/plots/get_data.js | 17 +++++++++++++++++ src/traces/carpet/plot.js | 11 ++++++----- src/traces/contour/plot.js | 11 +++++------ src/traces/contourcarpet/plot.js | 11 +++++------ src/traces/heatmap/plot.js | 11 ++++++----- 5 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/plots/get_data.js b/src/plots/get_data.js index 93b83f6c4d9..8d6bceb5b20 100644 --- a/src/plots/get_data.js +++ b/src/plots/get_data.js @@ -123,3 +123,20 @@ exports.getSubplotData = function getSubplotData(data, type, subplotId) { return subplotData; }; + +/** + * Get a lookup object of trace uids corresponding in a given calcdata array. + * + * @param {array} calcdata: as in gd.calcdata (or a subset) + * @return {object} lookup object of uids (`uid: 1`) + */ +exports.getUidsFromCalcData = function(calcdata) { + var out = {}; + + for(var i = 0; i < calcdata.length; i++) { + var trace = calcdata[i][0].trace; + out[trace.uid] = 1; + } + + return out; +}; diff --git a/src/traces/carpet/plot.js b/src/traces/carpet/plot.js index 5e9d5434bb5..c1a04ba4087 100644 --- a/src/traces/carpet/plot.js +++ b/src/traces/carpet/plot.js @@ -17,20 +17,21 @@ var orientText = require('./orient_text'); var svgTextUtils = require('../../lib/svg_text_utils'); var Lib = require('../../lib'); var alignmentConstants = require('../../constants/alignment'); +var getUidsFromCalcData = require('../../plots/get_data').getUidsFromCalcData; module.exports = function plot(gd, plotinfo, cdcarpet, carpetLayer) { - var i; + var uidLookup = getUidsFromCalcData(cdcarpet); carpetLayer.selectAll('g.trace').each(function() { var classString = d3.select(this).attr('class'); var oldUid = classString.split('carpet')[1].split(/\s/)[0]; - for(i = 0; i < cdcarpet.length; i++) { - if(oldUid === cdcarpet[i][0].trace.uid) return; + + if(!uidLookup[oldUid]) { + d3.select(this).remove(); } - d3.select(this).remove(); }); - for(i = 0; i < cdcarpet.length; i++) { + for(var i = 0; i < cdcarpet.length; i++) { plotOne(gd, plotinfo, cdcarpet[i], carpetLayer); } }; diff --git a/src/traces/contour/plot.js b/src/traces/contour/plot.js index f5534b7fcdf..cc391889854 100644 --- a/src/traces/contour/plot.js +++ b/src/traces/contour/plot.js @@ -16,6 +16,7 @@ var Drawing = require('../../components/drawing'); var svgTextUtils = require('../../lib/svg_text_utils'); var Axes = require('../../plots/cartesian/axes'); var setConvert = require('../../plots/cartesian/set_convert'); +var getUidsFromCalcData = require('../../plots/get_data').getUidsFromCalcData; var heatmapPlot = require('../heatmap/plot'); var makeCrossings = require('./make_crossings'); @@ -26,18 +27,16 @@ var closeBoundaries = require('./close_boundaries'); var constants = require('./constants'); var costConstants = constants.LABELOPTIMIZER; - exports.plot = function plot(gd, plotinfo, cdcontours, contourLayer) { - var i; + var uidLookup = getUidsFromCalcData(cdcontours); contourLayer.selectAll('g.contour').each(function(d) { - for(i = 0; i < cdcontours.length; i++) { - if(d.trace.uid === cdcontours[i][0].trace.uid) return; + if(!uidLookup[d.trace.uid]) { + d3.select(this).remove(); } - d3.select(this).remove(); }); - for(i = 0; i < cdcontours.length; i++) { + for(var i = 0; i < cdcontours.length; i++) { plotOne(gd, plotinfo, cdcontours[i], contourLayer); } }; diff --git a/src/traces/contourcarpet/plot.js b/src/traces/contourcarpet/plot.js index ab773a8fc69..16689efd848 100644 --- a/src/traces/contourcarpet/plot.js +++ b/src/traces/contourcarpet/plot.js @@ -13,6 +13,7 @@ var map1dArray = require('../carpet/map_1d_array'); var makepath = require('../carpet/makepath'); var Drawing = require('../../components/drawing'); var Lib = require('../../lib'); +var getUidsFromCalcData = require('../../plots/get_data').getUidsFromCalcData; var makeCrossings = require('../contour/make_crossings'); var findAllPaths = require('../contour/find_all_paths'); @@ -25,18 +26,16 @@ var mapPathinfo = require('./map_pathinfo'); var lookupCarpet = require('../carpet/lookup_carpetid'); var closeBoundaries = require('../contour/close_boundaries'); - module.exports = function plot(gd, plotinfo, cdcontours, contourcarpetLayer) { - var i; + var uidLookup = getUidsFromCalcData(cdcontours); contourcarpetLayer.selectAll('g.contour').each(function(d) { - for(i = 0; i < cdcontours.length; i++) { - if(d.trace.uid === cdcontours[i][0].trace.uid) return; + if(!uidLookup[d.trace.uid]) { + d3.select(this).remove(); } - d3.select(this).remove(); }); - for(i = 0; i < cdcontours.length; i++) { + for(var i = 0; i < cdcontours.length; i++) { plotOne(gd, plotinfo, cdcontours[i], contourcarpetLayer); } }; diff --git a/src/traces/heatmap/plot.js b/src/traces/heatmap/plot.js index 9b125db9eff..3d5f73828f4 100644 --- a/src/traces/heatmap/plot.js +++ b/src/traces/heatmap/plot.js @@ -16,21 +16,22 @@ var Registry = require('../../registry'); var Lib = require('../../lib'); var Colorscale = require('../../components/colorscale'); var xmlnsNamespaces = require('../../constants/xmlns_namespaces'); +var getUidsFromCalcData = require('../../plots/get_data').getUidsFromCalcData; var maxRowLength = require('./max_row_length'); module.exports = function(gd, plotinfo, cdheatmaps, heatmapLayer) { - var i; + var uidLookup = getUidsFromCalcData(cdheatmaps); heatmapLayer.selectAll('.hm > image').each(function(d) { var oldTrace = d.trace || {}; - for(i = 0; i < cdheatmaps.length; i++) { - if(oldTrace.uid === cdheatmaps[i][0].trace.uid) return; + + if(!uidLookup[oldTrace.uid]) { + d3.select(this.parentNode).remove(); } - d3.select(this.parentNode).remove(); }); - for(i = 0; i < cdheatmaps.length; i++) { + for(var i = 0; i < cdheatmaps.length; i++) { plotOne(gd, plotinfo, cdheatmaps[i], heatmapLayer); } }; From d47276f969f2cbcfbd5f3383247e23c71ff960de Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 30 Apr 2018 11:06:26 -0400 Subject: [PATCH 16/18] make visible trace module -> layer data a 1D loop w/ + indexOf + sort - I don't see the need to optimize here using e.g a lookup object. traceLayerClasses.length ~ 10, and fullLayout._modules.length < 20, most often 1 or 2. --- src/plots/cartesian/index.js | 50 +++++++++++++++++------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/src/plots/cartesian/index.js b/src/plots/cartesian/index.js index 59b507c86ca..7bf7e43ba57 100644 --- a/src/plots/cartesian/index.js +++ b/src/plots/cartesian/index.js @@ -189,36 +189,34 @@ function plotOne(gd, plotinfo, cdSubplot, transitionOpts, makeOnCompleteCallback var layerData = []; - for(var i = 0; i < traceLayerClasses.length; i++) { - var className = traceLayerClasses[i]; - - for(var j = 0; j < modules.length; j++) { - _module = modules[j]; - - if(_module.basePlotModule.name === 'cartesian' && - (_module.layerName || _module.name + 'layer') === className - ) { - var plotMethod = _module.plot; - - // plot all traces of this type on this subplot at once - cdModuleAndOthers = getModuleCalcData(cdSubplot, plotMethod); - cdModule = cdModuleAndOthers[0]; - // don't need to search the found traces again - in fact we need to NOT - // so that if two modules share the same plotter we don't double-plot - cdSubplot = cdModuleAndOthers[1]; - - if(cdModule.length) { - layerData.push({ - className: className, - plotMethod: plotMethod, - cdModule: cdModule - }); - } - break; + for(var i = 0; i < modules.length; i++) { + _module = modules[i]; + var name = _module.name; + + if(Registry.modules[name].categories.svg) { + var className = (_module.layerName || name + 'layer'); + var plotMethod = _module.plot; + + // plot all traces of this type on this subplot at once + cdModuleAndOthers = getModuleCalcData(cdSubplot, plotMethod); + cdModule = cdModuleAndOthers[0]; + // don't need to search the found traces again - in fact we need to NOT + // so that if two modules share the same plotter we don't double-plot + cdSubplot = cdModuleAndOthers[1]; + + if(cdModule.length) { + layerData.push({ + i: traceLayerClasses.indexOf(className), + className: className, + plotMethod: plotMethod, + cdModule: cdModule + }); } } } + layerData.sort(function(a, b) { return a.i - b.i; }); + var layers = plotinfo.plot.selectAll('g.mlayer') .data(layerData, function(d) { return d.className; }); From 83422f7ce39abfa571e43abb8c3e628216e91c9b Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 30 Apr 2018 11:20:25 -0400 Subject: [PATCH 17/18] skip gl tags in flaky-tagged jasmine test cmd --- .circleci/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/test.sh b/.circleci/test.sh index ea9b64140e4..656d8705148 100755 --- a/.circleci/test.sh +++ b/.circleci/test.sh @@ -34,7 +34,7 @@ case $1 in jasmine2) retry npm run test-jasmine -- --tags=gl --skip-tags=noCI,flaky - retry npm run test-jasmine -- --tags=flaky --skip-tags=noCI + retry npm run test-jasmine -- --tags=flaky --skip-tags=noCI,gl npm run test-bundle || EXIT_STATE=$? exit $EXIT_STATE ;; From 8c9a7bf660f3e02fc7ba14ffc9cfe5b953c6a3f3 Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 30 Apr 2018 11:43:58 -0400 Subject: [PATCH 18/18] Revert "skip gl tags in flaky-tagged jasmine test cmd" - so that we cover tests that are tagged @gl and @flaky This reverts commit 83422f7ce39abfa571e43abb8c3e628216e91c9b. --- .circleci/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/test.sh b/.circleci/test.sh index 656d8705148..ea9b64140e4 100755 --- a/.circleci/test.sh +++ b/.circleci/test.sh @@ -34,7 +34,7 @@ case $1 in jasmine2) retry npm run test-jasmine -- --tags=gl --skip-tags=noCI,flaky - retry npm run test-jasmine -- --tags=flaky --skip-tags=noCI,gl + retry npm run test-jasmine -- --tags=flaky --skip-tags=noCI npm run test-bundle || EXIT_STATE=$? exit $EXIT_STATE ;;