Merge pull request #3810 from plotly/scattergl-selection-fixes

scattergl selection fixes

Author: etpinard

package.json

@@ -58,7 +58,6 @@
   "dependencies": {
     "@plotly/d3-sankey": "0.7.2",
     "alpha-shape": "^1.0.0",
-    "array-range": "^1.0.1",
     "canvas-fit": "^1.5.0",
     "color-normalize": "^1.3.0",
     "convex-hull": "^1.0.3",

src/traces/scattergl/index.js

@@ -12,7 +12,6 @@ var createScatter = require('regl-scatter2d');
 var createLine = require('regl-line2d');
 var createError = require('regl-error2d');
 var cluster = require('point-cluster');
-var arrayRange = require('array-range');
 var Text = require('gl-text');
 
 var Registry = require('../../registry');
@@ -116,7 +115,6 @@ function calc(gd, trace) {
         opts.marker.snap = stash.tree || TOO_MANY_POINTS;
     }
 
-    // save scene opts batch
     scene.lineOptions.push(opts.line);
     scene.errorXOptions.push(opts.errorX);
     scene.errorYOptions.push(opts.errorY);
@@ -127,8 +125,9 @@ function calc(gd, trace) {
     scene.textOptions.push(opts.text);
     scene.textSelectedOptions.push(opts.textSel);
     scene.textUnselectedOptions.push(opts.textUnsel);
+    scene.selectBatch.push([]);
+    scene.unselectBatch.push([]);
 
-    // stash scene ref
     stash._scene = scene;
     stash.index = scene.count;
     stash.x = x;
@@ -146,7 +145,6 @@ function expandForErrorBars(trace, ax, opts) {
     extremes.max = extremes.max.concat(errExt.max);
 }
 
-// create scene options
 function sceneOptions(gd, subplot, trace, positions, x, y) {
     var opts = convert.style(gd, trace);
 
@@ -193,13 +191,12 @@ function sceneOptions(gd, subplot, trace, positions, x, y) {
     return opts;
 }
 
-
 // make sure scene exists on subplot, return it
 function sceneUpdate(gd, subplot) {
     var scene = subplot._scene;
 
     var resetOpts = {
-        // number of traces in subplot, since scene:subplot → 1:1
+        // number of traces in subplot, since scene:subplot -> 1:1
         count: 0,
         // whether scene requires init hook in plot call (dirty plot call)
         dirty: true,
@@ -213,19 +210,20 @@ function sceneUpdate(gd, subplot) {
         errorYOptions: [],
         textOptions: [],
         textSelectedOptions: [],
-        textUnselectedOptions: []
+        textUnselectedOptions: [],
+        // selection batches
+        selectBatch: [],
+        unselectBatch: []
     };
 
+    // regl- component stubs, initialized in dirty plot call
     var initOpts = {
-        selectBatch: null,
-        unselectBatch: null,
-        // regl- component stubs, initialized in dirty plot call
         fill2d: false,
         scatter2d: false,
         error2d: false,
         line2d: false,
         glText: false,
-        select2d: null
+        select2d: false
     };
 
     if(!subplot._scene) {
@@ -276,17 +274,22 @@ function sceneUpdate(gd, subplot) {
                     if(scene.errorXOptions[i]) error2d.draw(i);
                     if(scene.errorYOptions[i]) error2d.draw(i + count);
                 }
-                if(scatter2d && scene.markerOptions[i] && (!selectBatch || !selectBatch[i])) {
-                    scatter2d.draw(i);
+                if(scatter2d && scene.markerOptions[i]) {
+                    if(unselectBatch[i].length) {
+                        var arg = Lib.repeat([], scene.count);
+                        arg[i] = unselectBatch[i];
+                        scatter2d.draw(arg);
+                    } else if(!selectBatch[i].length) {
+                        scatter2d.draw(i);
+                    }
                 }
                 if(glText[i] && scene.textOptions[i]) {
                     glText[i].render();
                 }
             }
 
-            if(scatter2d && select2d && selectBatch) {
+            if(select2d) {
                 select2d.draw(selectBatch);
-                scatter2d.draw(unselectBatch);
             }
 
             scene.dirty = false;
@@ -325,7 +328,7 @@ function sceneUpdate(gd, subplot) {
         };
     }
 
-    // In case if we have scene from the last calc - reset data
+    // in case if we have scene from the last calc - reset data
     if(!scene.dirty) {
         Lib.extendFlat(scene, resetOpts);
     }
@@ -363,6 +366,7 @@ function plot(gd, subplot, cdata) {
         return;
     }
 
+    var count = scene.count;
     var regl = fullLayout._glcanvas.data()[0].regl;
 
     // that is needed for fills
@@ -383,28 +387,28 @@ function plot(gd, subplot, cdata) {
             scene.fill2d = createLine(regl);
         }
         if(scene.glText === true) {
-            scene.glText = new Array(scene.count);
-            for(i = 0; i < scene.count; i++) {
+            scene.glText = new Array(count);
+            for(i = 0; i < count; i++) {
                 scene.glText[i] = new Text(regl);
             }
         }
 
         // update main marker options
         if(scene.glText) {
-            if(scene.count > scene.glText.length) {
+            if(count > scene.glText.length) {
                 // add gl text marker
-                var textsToAdd = scene.count - scene.glText.length;
+                var textsToAdd = count - scene.glText.length;
                 for(i = 0; i < textsToAdd; i++) {
                     scene.glText.push(new Text(regl));
                 }
-            } else if(scene.count < scene.glText.length) {
+            } else if(count < scene.glText.length) {
                 // remove gl text marker
-                var textsToRemove = scene.glText.length - scene.count;
-                var removedTexts = scene.glText.splice(scene.count, textsToRemove);
+                var textsToRemove = scene.glText.length - count;
+                var removedTexts = scene.glText.splice(count, textsToRemove);
                 removedTexts.forEach(function(text) { text.destroy(); });
             }
 
-            for(i = 0; i < scene.count; i++) {
+            for(i = 0; i < count; i++) {
                 scene.glText[i].update(scene.textOptions[i]);
             }
         }
@@ -437,7 +441,7 @@ function plot(gd, subplot, cdata) {
         }
 
         // fill requires linked traces, so we generate it's positions here
-        scene.fillOrder = Lib.repeat(null, scene.count);
+        scene.fillOrder = Lib.repeat(null, count);
         if(scene.fill2d) {
             scene.fillOptions = scene.fillOptions.map(function(fillOptions, i) {
                 var cdscatter = cdata[i];
@@ -556,13 +560,11 @@ function plot(gd, subplot, cdata) {
     }
 
     // form batch arrays, and check for selected points
-    scene.selectBatch = null;
-    scene.unselectBatch = null;
     var dragmode = fullLayout.dragmode;
     var selectMode = dragmode === 'lasso' || dragmode === 'select';
     var clickSelectEnabled = fullLayout.clickmode.indexOf('select') > -1;
 
-    for(i = 0; i < cdata.length; i++) {
+    for(i = 0; i < count; i++) {
         var cd0 = cdata[i][0];
         var trace = cd0.trace;
         var stash = cd0.t;
@@ -574,11 +576,6 @@ function plot(gd, subplot, cdata) {
         if(trace.selectedpoints || selectMode || clickSelectEnabled) {
             if(!selectMode) selectMode = true;
 
-            if(!scene.selectBatch) {
-                scene.selectBatch = [];
-                scene.unselectBatch = [];
-            }
-
             // regenerate scene batch, if traces number changed during selection
             if(trace.selectedpoints) {
                 var selPts = scene.selectBatch[index] = Lib.selIndices2selPoints(trace);
@@ -610,21 +607,24 @@ function plot(gd, subplot, cdata) {
         }
     }
 
-
     if(selectMode) {
-        // create select2d
+        // create scatter instance by cloning scatter2d
         if(!scene.select2d) {
-            // create scatter instance by cloning scatter2d
             scene.select2d = createScatter(fullLayout._glcanvas.data()[1].regl);
         }
 
-        if(scene.scatter2d && scene.selectBatch && scene.selectBatch.length) {
-            // update only traces with selection
-            scene.scatter2d.update(scene.markerUnselectedOptions.map(function(opts, i) {
-                return scene.selectBatch[i] ? opts : null;
-            }));
+        // use unselected styles on 'context' canvas
+        if(scene.scatter2d) {
+            var unselOpts = new Array(count);
+            for(i = 0; i < count; i++) {
+                unselOpts[i] = scene.selectBatch[i].length || scene.unselectBatch[i].length ?
+                    scene.markerUnselectedOptions[i] :
+                    {};
+            }
+            scene.scatter2d.update(unselOpts);
         }
 
+        // use selected style on 'focus' canvas
         if(scene.select2d) {
             scene.select2d.update(scene.markerOptions);
             scene.select2d.update(scene.markerSelectedOptions);
@@ -639,9 +639,9 @@ function plot(gd, subplot, cdata) {
             });
         }
     } else {
+        // reset 'context' scatter2d opts to base opts,
+        // thus unsetting markerUnselectedOptions from selection
         if(scene.scatter2d) {
-            // reset scatter2d opts to base opts,
-            // thus unsetting markerUnselectedOptions from selection
             scene.scatter2d.update(scene.markerOptions);
         }
     }
@@ -680,7 +680,6 @@ function plot(gd, subplot, cdata) {
     }
 }
 
-
 function hoverPoints(pointData, xval, yval, hovermode) {
     var cd = pointData.cd;
     var stash = cd[0].t;
@@ -758,7 +757,6 @@ function hoverPoints(pointData, xval, yval, hovermode) {
     return [pointData];
 }
 
-
 function calcHover(pointData, x, y, trace) {
     var xa = pointData.xa;
     var ya = pointData.ya;
@@ -861,7 +859,6 @@ function calcHover(pointData, x, y, trace) {
     return pointData;
 }
 
-
 function selectPoints(searchInfo, selectionTester) {
     var cd = searchInfo.cd;
     var selection = [];
@@ -871,23 +868,23 @@ function selectPoints(searchInfo, selectionTester) {
     var x = stash.x;
     var y = stash.y;
     var scene = stash._scene;
+    var index = stash.index;
 
     if(!scene) return selection;
 
     var hasText = subTypes.hasText(trace);
     var hasMarkers = subTypes.hasMarkers(trace);
     var hasOnlyLines = !hasMarkers && !hasText;
+
     if(trace.visible !== true || hasOnlyLines) return selection;
 
+    var els = [];
+    var unels = [];
+
     // degenerate polygon does not enable selection
     // filter out points by visible scatter ones
-    var els = null;
-    var unels = null;
-    // FIXME: clearing selection does not work here
-    var i;
     if(selectionTester !== false && !selectionTester.degenerate) {
-        els = [], unels = [];
-        for(i = 0; i < len; i++) {
+        for(var i = 0; i < len; i++) {
             if(selectionTester.contains([stash.xpx[i], stash.ypx[i]], false, i, searchInfo)) {
                 els.push(i);
                 selection.push({
@@ -899,32 +896,27 @@ function selectPoints(searchInfo, selectionTester) {
                 unels.push(i);
             }
         }
-    } else {
-        unels = arrayRange(len);
     }
 
-    // make sure selectBatch is created
-    if(!scene.selectBatch) {
-        scene.selectBatch = [];
-        scene.unselectBatch = [];
-    }
+    if(hasMarkers) {
+        var scatter2d = scene.scatter2d;
 
-    if(!scene.selectBatch[stash.index]) {
-        // enter every trace select mode
-        for(i = 0; i < scene.count; i++) {
-            scene.selectBatch[i] = [];
-            scene.unselectBatch[i] = [];
-        }
-        // we should turn scatter2d into unselected once we have any points selected
-        if(hasMarkers) {
-            scene.scatter2d.update(scene.markerUnselectedOptions);
+        if(!els.length && !unels.length) {
+            // reset to base styles when clearing
+            var baseOpts = new Array(scene.count);
+            baseOpts[index] = scene.markerOptions[index];
+            scatter2d.update.apply(scatter2d, baseOpts);
+        } else if(!scene.selectBatch[index].length && !scene.unselectBatch[index].length) {
+            // set unselected styles on 'context' canvas (if not done already)
+            var unselOpts = new Array(scene.count);
+            unselOpts[index] = scene.markerUnselectedOptions[index];
+            scatter2d.update.apply(scatter2d, unselOpts);
         }
     }
 
-    scene.selectBatch[stash.index] = els;
-    scene.unselectBatch[stash.index] = unels;
+    scene.selectBatch[index] = els;
+    scene.unselectBatch[index] = unels;
 
-    // update text options
     if(hasText) {
         styleTextSelection(cd);
     }
@@ -946,7 +938,7 @@ function styleTextSelection(cd) {
     var opts = Lib.extendFlat({}, baseOpts);
     var i, j;
 
-    if(els && unels) {
+    if(els.length || unels.length) {
         var stc = selOpts.color;
         var utc = unselOpts.color;
         var base = baseOpts.color;

src/traces/scatterpolargl/index.js

@@ -155,6 +155,8 @@ function plot(gd, subplot, cdata) {
         scene.textOptions.push(opts.text);
         scene.textSelectedOptions.push(opts.textSel);
         scene.textUnselectedOptions.push(opts.textUnsel);
+        scene.selectBatch.push([]);
+        scene.unselectBatch.push([]);
 
         stash.x = x;
         stash.y = y;