Merge pull request #2583 from plotly/scatter-select-perf

etpinard · web-flow · commit 2b1713d583f9 · 2018-05-01T09:31:17.000-04:00
SVG trace on selection perf
diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js
diff --git a/src/components/legend/style.js b/src/components/legend/style.js
@@ -110,8 +110,8 @@ module.exports = function style(s, gd) {
 
         // constrain text, markers, etc so they'll fit on the legend
         if(showMarkers || showText || showLines) {
-            var dEdit = {},
-                tEdit = {};
+            var dEdit = {};
+            var tEdit = {};
 
             if(showMarkers) {
                 dEdit.mc = boundVal('marker.color', pickFirst);
@@ -142,6 +142,9 @@ module.exports = function style(s, gd) {
 
             dMod = [Lib.minExtend(d0, dEdit)];
             tMod = Lib.minExtend(trace, tEdit);
+
+            // always show legend items in base state
+            tMod.selectedpoints = null;
         }
 
         var ptgroup = d3.select(this).select('g.legendpoints');
diff --git a/src/plots/cartesian/select.js b/src/plots/cartesian/select.js
@@ -404,18 +404,20 @@ function updateSelectedState(gd, searchTraces, eventData) {
         var len = items.length;
         var item0 = items[0];
         var trace0 = item0.cd[0].trace;
+        var _module = item0._module;
+        var styleSelection = _module.styleOnSelect || _module.style;
 
         if(Registry.traceIs(trace0, 'regl')) {
             // plot regl traces per module
             var cds = new Array(len);
             for(j = 0; j < len; j++) {
                 cds[j] = items[j].cd;
             }
-            item0._module.style(gd, cds);
+            styleSelection(gd, cds);
         } else {
             // plot svg trace per trace
             for(j = 0; j < len; j++) {
-                item0._module.style(gd, items[j].cd);
+                styleSelection(gd, items[j].cd);
             }
         }
     }
diff --git a/src/plots/plots.js b/src/plots/plots.js
@@ -819,7 +819,7 @@ plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLa
         }
         // Then look for a subplot with the counteraxis overlaying the anchor
         // If that fails just use the first subplot including this axis
-        if(!mainSubplotID || ids.indexOf(mainSubplotID) === -1) {
+        if(!mainSubplotID || !newSubplots[mainSubplotID]) {
             mainSubplotID = '';
             for(j = 0; j < ids.length; j++) {
                 id = ids[j];
diff --git a/src/traces/bar/index.js b/src/traces/bar/index.js
@@ -20,7 +20,8 @@ Bar.setPositions = require('./set_positions');
 Bar.colorbar = require('../scatter/colorbar');
 Bar.arraysToCalcdata = require('./arrays_to_calcdata');
 Bar.plot = require('./plot');
-Bar.style = require('./style');
+Bar.style = require('./style').style;
+Bar.styleOnSelect = require('./style').styleOnSelect;
 Bar.hoverPoints = require('./hover');
 Bar.selectPoints = require('./select');
 
diff --git a/src/traces/bar/style.js b/src/traces/bar/style.js
@@ -13,7 +13,7 @@ var d3 = require('d3');
 var Drawing = require('../../components/drawing');
 var Registry = require('../../registry');
 
-module.exports = function style(gd, cd) {
+function style(gd, cd) {
     var s = cd ? cd[0].node3 : d3.select(gd).selectAll('g.trace.bars');
     var barcount = s.size();
     var fullLayout = gd._fullLayout;
@@ -35,34 +35,52 @@ module.exports = function style(gd, cd) {
 
     s.selectAll('g.points').each(function(d) {
         var sel = d3.select(this);
-        var pts = sel.selectAll('path');
-        var txs = sel.selectAll('text');
         var trace = d[0].trace;
+        stylePoints(sel, trace, gd);
+    });
+
+    Registry.getComponentMethod('errorbars', 'style')(s);
+}
 
-        Drawing.pointStyle(pts, trace, gd);
-        Drawing.selectedPointStyle(pts, trace);
+function stylePoints(sel, trace, gd) {
+    var pts = sel.selectAll('path');
+    var txs = sel.selectAll('text');
 
-        txs.each(function(d) {
-            var tx = d3.select(this);
-            var textFont;
+    Drawing.pointStyle(pts, trace, gd);
 
-            if(tx.classed('bartext-inside')) {
-                textFont = trace.insidetextfont;
-            } else if(tx.classed('bartext-outside')) {
-                textFont = trace.outsidetextfont;
-            }
-            if(!textFont) textFont = trace.textfont;
+    txs.each(function(d) {
+        var tx = d3.select(this);
+        var textFont;
 
-            function cast(k) {
-                var cont = textFont[k];
-                return Array.isArray(cont) ? cont[d.i] : cont;
-            }
+        if(tx.classed('bartext-inside')) {
+            textFont = trace.insidetextfont;
+        } else if(tx.classed('bartext-outside')) {
+            textFont = trace.outsidetextfont;
+        }
+        if(!textFont) textFont = trace.textfont;
 
-            Drawing.font(tx, cast('family'), cast('size'), cast('color'));
-        });
+        function cast(k) {
+            var cont = textFont[k];
+            return Array.isArray(cont) ? cont[d.i] : cont;
+        }
 
-        Drawing.selectedTextStyle(txs, trace);
+        Drawing.font(tx, cast('family'), cast('size'), cast('color'));
     });
+}
 
-    Registry.getComponentMethod('errorbars', 'style')(s);
+function styleOnSelect(gd, cd) {
+    var s = cd[0].node3;
+    var trace = cd[0].trace;
+
+    if(trace.selectedpoints) {
+        Drawing.selectedPointStyle(s.selectAll('path'), trace);
+        Drawing.selectedTextStyle(s.selectAll('text'), trace);
+    } else {
+        stylePoints(s, trace, gd);
+    }
+}
+
+module.exports = {
+    style: style,
+    styleOnSelect: styleOnSelect
 };
diff --git a/src/traces/box/index.js b/src/traces/box/index.js
@@ -17,7 +17,8 @@ Box.supplyLayoutDefaults = require('./layout_defaults').supplyLayoutDefaults;
 Box.calc = require('./calc');
 Box.setPositions = require('./set_positions').setPositions;
 Box.plot = require('./plot').plot;
-Box.style = require('./style');
+Box.style = require('./style').style;
+Box.styleOnSelect = require('./style').styleOnSelect;
 Box.hoverPoints = require('./hover').hoverPoints;
 Box.selectPoints = require('./select');
 
diff --git a/src/traces/box/style.js b/src/traces/box/style.js
@@ -12,7 +12,7 @@ var d3 = require('d3');
 var Color = require('../../components/color');
 var Drawing = require('../../components/drawing');
 
-module.exports = function style(gd, cd) {
+function style(gd, cd) {
     var s = cd ? cd[0].node3 : d3.select(gd).selectAll('g.trace.boxes');
 
     s.style('opacity', function(d) { return d[0].trace.opacity; });
@@ -50,7 +50,23 @@ module.exports = function style(gd, cd) {
 
             var pts = el.selectAll('path.point');
             Drawing.pointStyle(pts, trace, gd);
-            Drawing.selectedPointStyle(pts, trace);
         }
     });
+}
+
+function styleOnSelect(gd, cd) {
+    var s = cd[0].node3;
+    var trace = cd[0].trace;
+    var pts = s.selectAll('path.point');
+
+    if(trace.selectedpoints) {
+        Drawing.selectedPointStyle(pts, trace);
+    } else {
+        Drawing.pointStyle(pts, trace, gd);
+    }
+}
+
+module.exports = {
+    style: style,
+    styleOnSelect: styleOnSelect
 };
diff --git a/src/traces/candlestick/index.js b/src/traces/candlestick/index.js
@@ -37,7 +37,7 @@ module.exports = {
     calc: require('./calc'),
     plot: require('../box/plot').plot,
     layerName: 'boxlayer',
-    style: require('../box/style'),
+    style: require('../box/style').style,
     hoverPoints: require('../ohlc/hover'),
     selectPoints: require('../ohlc/select')
 };
diff --git a/src/traces/choropleth/index.js b/src/traces/choropleth/index.js
@@ -16,7 +16,8 @@ Choropleth.supplyDefaults = require('./defaults');
 Choropleth.colorbar = require('../heatmap/colorbar');
 Choropleth.calc = require('./calc');
 Choropleth.plot = require('./plot');
-Choropleth.style = require('./style');
+Choropleth.style = require('./style').style;
+Choropleth.styleOnSelect = require('./style').styleOnSelect;
 Choropleth.hoverPoints = require('./hover');
 Choropleth.eventData = require('./event_data');
 Choropleth.selectPoints = require('./select');
diff --git a/src/traces/choropleth/plot.js b/src/traces/choropleth/plot.js
@@ -15,7 +15,7 @@ var polygon = require('../../lib/polygon');
 
 var getTopojsonFeatures = require('../../lib/topojson_utils').getTopojsonFeatures;
 var locationToFeature = require('../../lib/geo_location_utils').locationToFeature;
-var style = require('./style');
+var style = require('./style').style;
 
 module.exports = function plot(gd, geo, calcData) {
     for(var i = 0; i < calcData.length; i++) {
diff --git a/src/traces/choropleth/style.js b/src/traces/choropleth/style.js
@@ -13,9 +13,9 @@ var Color = require('../../components/color');
 var Drawing = require('../../components/drawing');
 var Colorscale = require('../../components/colorscale');
 
-module.exports = function style(gd, calcTrace) {
+function style(gd, calcTrace) {
     if(calcTrace) styleTrace(gd, calcTrace);
-};
+}
 
 function styleTrace(gd, calcTrace) {
     var trace = calcTrace[0].trace;
@@ -40,5 +40,21 @@ function styleTrace(gd, calcTrace) {
             .style('opacity', marker.opacity);
     });
 
-    Drawing.selectedPointStyle(locs, trace);
+    Drawing.selectedPointStyle(locs, trace, gd);
 }
+
+function styleOnSelect(gd, calcTrace) {
+    var s = calcTrace[0].node3;
+    var trace = calcTrace[0].trace;
+
+    if(trace.selectedpoints) {
+        Drawing.selectedPointStyle(s.selectAll('.choroplethlocation'), trace, gd);
+    } else {
+        styleTrace(gd, calcTrace);
+    }
+}
+
+module.exports = {
+    style: style,
+    styleOnSelect: styleOnSelect
+};
diff --git a/src/traces/histogram/index.js b/src/traces/histogram/index.js
@@ -33,7 +33,8 @@ Histogram.calc = require('./calc');
 Histogram.setPositions = require('../bar/set_positions');
 Histogram.plot = require('../bar/plot');
 Histogram.layerName = 'barlayer';
-Histogram.style = require('../bar/style');
+Histogram.style = require('../bar/style').style;
+Histogram.styleOnSelect = require('../bar/style').styleOnSelect;
 Histogram.colorbar = require('../scatter/colorbar');
 Histogram.hoverPoints = require('./hover');
 Histogram.selectPoints = require('../bar/select');
diff --git a/src/traces/scatter/index.js b/src/traces/scatter/index.js
@@ -27,6 +27,7 @@ Scatter.arraysToCalcdata = require('./arrays_to_calcdata');
 Scatter.plot = require('./plot');
 Scatter.colorbar = require('./colorbar');
 Scatter.style = require('./style').style;
+Scatter.styleOnSelect = require('./style').styleOnSelect;
 Scatter.hoverPoints = require('./hover');
 Scatter.selectPoints = require('./select');
 Scatter.animatable = true;
diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js
@@ -407,14 +407,14 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
     function makePoints(d) {
         var join, selection, hasNode;
 
-        var trace = d[0].trace,
-            s = d3.select(this),
-            showMarkers = subTypes.hasMarkers(trace),
-            showText = subTypes.hasText(trace);
+        var trace = d[0].trace;
+        var s = d3.select(this);
+        var showMarkers = subTypes.hasMarkers(trace);
+        var showText = subTypes.hasText(trace);
 
-        var keyFunc = getKeyFunc(trace),
-            markerFilter = hideFilter,
-            textFilter = hideFilter;
+        var keyFunc = getKeyFunc(trace);
+        var markerFilter = hideFilter;
+        var textFilter = hideFilter;
 
         if(showMarkers) {
             markerFilter = (trace.marker.maxdisplayed || trace._needsCull) ? visFilter : Lib.identity;
@@ -442,18 +442,20 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
                 .style('opacity', 1);
         }
 
-        var markerScale = showMarkers && Drawing.tryColorscale(trace.marker, '');
-        var lineScale = showMarkers && Drawing.tryColorscale(trace.marker, 'line');
-
         join.order();
 
+        var styleFns;
+        if(showMarkers) {
+            styleFns = Drawing.makePointStyleFns(trace);
+        }
+
         join.each(function(d) {
             var el = d3.select(this);
             var sel = transition(el);
             hasNode = Drawing.translatePoint(d, sel, xa, ya);
 
             if(hasNode) {
-                Drawing.singlePointStyle(d, sel, trace, markerScale, lineScale, gd);
+                Drawing.singlePointStyle(d, sel, trace, styleFns, gd);
 
                 if(plotinfo.layerClipId) {
                     Drawing.hideOutsideRangePoint(d, sel, xa, ya, trace.xcalendar, trace.ycalendar);
diff --git a/src/traces/scatter/style.js b/src/traces/scatter/style.js
@@ -36,16 +36,24 @@ function style(gd, cd) {
 }
 
 function stylePoints(sel, trace, gd) {
-    var pts = sel.selectAll('path.point');
-    var txs = sel.selectAll('text');
+    Drawing.pointStyle(sel.selectAll('path.point'), trace, gd);
+    Drawing.textPointStyle(sel.selectAll('text'), trace, gd);
+}
+
+function styleOnSelect(gd, cd) {
+    var s = cd[0].node3;
+    var trace = cd[0].trace;
 
-    Drawing.pointStyle(pts, trace, gd);
-    Drawing.textPointStyle(txs, trace, gd);
-    Drawing.selectedPointStyle(pts, trace);
-    Drawing.selectedTextStyle(txs, trace);
+    if(trace.selectedpoints) {
+        Drawing.selectedPointStyle(s.selectAll('path.point'), trace);
+        Drawing.selectedTextStyle(s.selectAll('text'), trace);
+    } else {
+        stylePoints(s, trace, gd);
+    }
 }
 
 module.exports = {
     style: style,
-    stylePoints: stylePoints
+    stylePoints: stylePoints,
+    styleOnSelect: styleOnSelect
 };
diff --git a/src/traces/scattercarpet/index.js b/src/traces/scattercarpet/index.js
@@ -16,6 +16,7 @@ ScatterCarpet.colorbar = require('../scatter/colorbar');
 ScatterCarpet.calc = require('./calc');
 ScatterCarpet.plot = require('./plot');
 ScatterCarpet.style = require('../scatter/style').style;
+ScatterCarpet.styleOnSelect = require('../scatter/style').styleOnSelect;
 ScatterCarpet.hoverPoints = require('./hover');
 ScatterCarpet.selectPoints = require('../scatter/select');
 ScatterCarpet.eventData = require('./event_data');
diff --git a/src/traces/scattergeo/index.js b/src/traces/scattergeo/index.js
@@ -17,6 +17,7 @@ ScatterGeo.colorbar = require('../scatter/colorbar');
 ScatterGeo.calc = require('./calc');
 ScatterGeo.plot = require('./plot');
 ScatterGeo.style = require('./style');
+ScatterGeo.styleOnSelect = require('../scatter/style').styleOnSelect;
 ScatterGeo.hoverPoints = require('./hover');
 ScatterGeo.eventData = require('./event_data');
 ScatterGeo.selectPoints = require('./select');
diff --git a/src/traces/scattermapbox/convert.js b/src/traces/scattermapbox/convert.js
diff --git a/src/traces/scatterternary/index.js b/src/traces/scatterternary/index.js
diff --git a/src/traces/violin/index.js b/src/traces/violin/index.js
diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js
diff --git a/test/jasmine/tests/select_test.js b/test/jasmine/tests/select_test.js

Original file line number	Diff line number	Diff line change
`@@ -404,18 +404,20 @@ function updateSelectedState(gd, searchTraces, eventData) {`
`404`	`404`	`var len = items.length;`
`405`	`405`	`var item0 = items[0];`
`406`	`406`	`var trace0 = item0.cd[0].trace;`
	`407`	`+ var _module = item0._module;`
	`408`	`+ var styleSelection = _module.styleOnSelect \|\| _module.style;`
`407`	`409`
`408`	`410`	`if(Registry.traceIs(trace0, 'regl')) {`
`409`	`411`	`// plot regl traces per module`
`410`	`412`	`var cds = new Array(len);`
`411`	`413`	`for(j = 0; j < len; j++) {`
`412`	`414`	`cds[j] = items[j].cd;`
`413`	`415`	`}`
`414`		`- item0._module.style(gd, cds);`
	`416`	`+ styleSelection(gd, cds);`
`415`	`417`	`} else {`
`416`	`418`	`// plot svg trace per trace`
`417`	`419`	`for(j = 0; j < len; j++) {`
`418`		`- item0._module.style(gd, items[j].cd);`
	`420`	`+ styleSelection(gd, items[j].cd);`
`419`	`421`	`}`
`420`	`422`	`}`
`421`	`423`	`}`
Original file line number	Diff line number	Diff line change
`@@ -819,7 +819,7 @@ plots.linkSubplots = function(newFullData, newFullLayout, oldFullData, oldFullLa`
`819`	`819`	`}`
`820`	`820`	`// Then look for a subplot with the counteraxis overlaying the anchor`
`821`	`821`	`// If that fails just use the first subplot including this axis`
`822`		`- if(!mainSubplotID \|\| ids.indexOf(mainSubplotID) === -1) {`
	`822`	`+ if(!mainSubplotID \|\| !newSubplots[mainSubplotID]) {`
`823`	`823`	`mainSubplotID = '';`
`824`	`824`	`for(j = 0; j < ids.length; j++) {`
`825`	`825`	`id = ids[j];`