Merge pull request #2579 from plotly/uid-trace-removal

Even better cartesian trace updates and removal

Author: etpinard

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;
     });
 }
 

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
@@ -495,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();
         }
     }
 

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: {

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,73 @@ 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 < 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
+                });
+            }
         }
     }
 
-    for(i = 0; i < plotMethodsToCall.length; i++) {
-        plotMethod = plotMethodsToCall[i];
+    layerData.sort(function(a, b) { return a.i - b.i; });
 
-        // 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];
+    var layers = plotinfo.plot.selectAll('g.mlayer')
+        .data(layerData, function(d) { return d.className; });
 
-        plotMethod(gd, plotinfo, cdModule, transitionOpts, makeOnCompleteCallback);
-    }
+    layers.enter().append('g')
+        .attr('class', function(d) { return d.className; })
+        .classed('mlayer', true);
+
+    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 +477,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

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;
+};

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();
         }
     }
 

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')

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')

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')

src/traces/carpet/plot.js

@@ -17,20 +17,26 @@ 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) {
-    if(!cdcarpet.length) {
-        plotinfo.plot.select('.carpetlayer')
-            .selectAll('g.trace')
-            .remove();
-    }
+module.exports = function plot(gd, plotinfo, cdcarpet, carpetLayer) {
+    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];
+
+        if(!uidLookup[oldUid]) {
+            d3.select(this).remove();
+        }
+    });
 
     for(var i = 0; i < cdcarpet.length; i++) {
-        plotOne(gd, plotinfo, cdcarpet[i]);
+        plotOne(gd, plotinfo, cdcarpet[i], carpetLayer);
     }
 };
 
-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 +45,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');