Skip to content

Better cartesian trace updates and removal #2574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Apr 30, 2018
Merged

Better cartesian trace updates and removal #2574

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
6d88255
make bar, box & violin plot method use d3 enter/exit/update pattern
etpinard Apr 19, 2018
31e5b12
lint and minor perf improvements in Cartesian.clean
etpinard Apr 19, 2018
156f127
move up linkSubplots before cleanPlot
etpinard Apr 19, 2018
13a15d1
improve cartesian trace delete pattern
etpinard Apr 19, 2018
b4ea2ce
use plotMethods stash in range slider range plots
etpinard Apr 19, 2018
072f028
fixup trace deletion for carpet traces
etpinard Apr 19, 2018
e5c860a
simplify arg1 -> plotMethod logic
etpinard Apr 20, 2018
400bfe6
ping imagetest after having restarting nw.js
etpinard Apr 24, 2018
284c206
introduce new strategy for cartesian trace module layer updates
etpinard Apr 24, 2018
da63ed4
make heatmap/contour(carpet)/carpet plot method remove its own trace
etpinard Apr 24, 2018
a1c76bc
sub imagelayer->heatmaplayer, maplayer->contourlayer in tests
etpinard Apr 24, 2018
fa38ca5
update `cliponaxis: false`
etpinard Apr 24, 2018
48ee0d8
:lock: contour with heatmap coloring layer order
etpinard Apr 24, 2018
362da1f
:lock: scattercarpet + scatter coexistence
etpinard Apr 24, 2018
3bef9a9
add getUidsFromCalcData helper
etpinard Apr 30, 2018
d47276f
make visible trace module -> layer data a 1D loop w/ + indexOf + sort
etpinard Apr 30, 2018
83422f7
skip gl tags in flaky-tagged jasmine test cmd
etpinard Apr 30, 2018
8c9a7bf
Revert "skip gl tags in flaky-tagged jasmine test cmd"
etpinard Apr 30, 2018
f771310
Merge pull request #2579 from plotly/uid-trace-removal
etpinard Apr 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/components/rangeslider/draw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
});
}

Expand Down
136 changes: 46 additions & 90 deletions src/plots/cartesian/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -182,115 +179,77 @@ 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 <g subplot> 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) {
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();
}
}
}
Expand All @@ -308,7 +267,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);
Expand Down Expand Up @@ -516,11 +475,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++) {
Expand Down
32 changes: 24 additions & 8 deletions src/plots/get_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does a falsy arg1 happen?

Also, I recall seeing somewhere that js can optimize typeof arg1 === 'string' internally so it never has to generate the string values, whereas with var typeOfArg1 = typeof arg1; typeOfArg1 === 'string' it can't do that so it's actually better to write out typeof arg1 === '...' in each clause. I can't find it now though...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tips!

Simplified in -> e5c860a

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);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/plots/plots.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Loading