Merge pull request #2311 from plotly/unify-selection

Refactor scattergl selection

Author: dy

package.json

@@ -57,6 +57,7 @@
     "3d-view": "^2.0.0",
     "@plotly/d3-sankey": "^0.5.0",
     "alpha-shape": "^1.0.0",
+    "array-range": "^1.0.1",
     "bubleify": "^1.0.0",
     "canvas-fit": "^1.5.0",
     "color-normalize": "^1.0.3",
@@ -97,7 +98,7 @@
     "regl": "^1.3.1",
     "regl-error2d": "^2.0.3",
     "regl-line2d": "^2.1.2",
-    "regl-scatter2d": "^2.1.12",
+    "regl-scatter2d": "^2.1.13",
     "right-now": "^1.0.0",
     "robust-orientation": "^1.1.3",
     "sane-topojson": "^2.0.0",

src/traces/scattergl/attributes.js

@@ -8,8 +8,9 @@
 
 'use strict';
 
+var plotAttrs = require('../../plots/attributes');
 var scatterAttrs = require('../scatter/attributes');
-var colorAttributes = require('../../components/colorscale/color_attributes');
+var colorAttrs = require('../../components/colorscale/color_attributes');
 
 var DASHES = require('../../constants/gl2d_dashes');
 var extendFlat = require('../../lib/extend').extendFlat;
@@ -56,7 +57,7 @@ var attrs = module.exports = overrideAll({
             description: 'Sets the style of the lines.'
         }
     },
-    marker: extendFlat({}, colorAttributes('marker'), {
+    marker: extendFlat({}, colorAttrs('marker'), {
         symbol: scatterMarkerAttrs.symbol,
         size: scatterMarkerAttrs.size,
         sizeref: scatterMarkerAttrs.sizeref,
@@ -65,7 +66,7 @@ var attrs = module.exports = overrideAll({
         opacity: scatterMarkerAttrs.opacity,
         showscale: scatterMarkerAttrs.showscale,
         colorbar: scatterMarkerAttrs.colorbar,
-        line: extendFlat({}, colorAttributes('marker.line'), {
+        line: extendFlat({}, colorAttrs('marker.line'), {
             width: scatterMarkerLineAttrs.width
         })
     }),
@@ -82,6 +83,10 @@ var attrs = module.exports = overrideAll({
         marker: scatterAttrs.unselected.marker
     },
 
+    opacity: extendFlat({}, plotAttrs.opacity, {
+        editType: 'calc'
+    }),
+
     error_y: scatterAttrs.error_y,
     error_x: scatterAttrs.error_x
 }, 'calc', 'nested');

src/traces/scattergl/index.js

@@ -28,6 +28,7 @@ var createError = require('regl-error2d');
 var rgba = require('color-normalize');
 var svgSdf = require('svg-path-sdf');
 var createRegl = require('regl');
+var arrayRange = require('array-range');
 var fillHoverText = require('../scatter/fill_hover_text');
 var isNumeric = require('fast-isnumeric');
 
@@ -122,7 +123,6 @@ function calc(container, trace) {
         }
     }
 
-
     calcColorscales(trace);
 
     var options = sceneOptions(container, subplot, trace, positions);
@@ -396,7 +396,9 @@ function sceneOptions(container, subplot, trace, positions) {
     function makeSelectedOptions(selected, markerOpts) {
         var options = {};
 
-        if(selected.marker.symbol) {
+        if(!selected) return options;
+
+        if(selected.marker && selected.marker.symbol) {
             options = makeMarkerOptions(extend({}, markerOpts, selected.marker));
         }
 
@@ -599,12 +601,15 @@ function sceneUpdate(container, subplot) {
                     scene.error2d.draw(i);
                     scene.error2d.draw(i + scene.count);
                 }
-                if(scene.scatter2d && !scene.selectBatch) {
-                    scene.scatter2d.draw(i);
+                if(scene.scatter2d) {
+                    // traces in no-selection mode
+                    if(!scene.selectBatch || !scene.selectBatch[i]) {
+                        scene.scatter2d.draw(i);
+                    }
                 }
             }
 
-            // persistent selection draw
+            // draw traces in selection mode
             if(scene.select2d && scene.selectBatch) {
                 scene.select2d.draw(scene.selectBatch);
                 scene.scatter2d.draw(scene.unselectBatch);
@@ -740,8 +745,8 @@ function plot(container, subplot, cdata) {
     if(!cdata.length) return;
 
     var layout = container._fullLayout;
-    var stash = cdata[0][0].t;
-    var scene = stash.scene;
+    var scene = cdata[0][0].t.scene;
+    var dragmode = layout.dragmode;
 
     // we may have more subplots than initialized data due to Axes.getSubplots method
     if(!scene) return;
@@ -782,6 +787,7 @@ function plot(container, subplot, cdata) {
             scene.fill2d = createLine(regl);
         }
 
+        // update main marker options
         if(scene.line2d) {
             scene.line2d.update(scene.lineOptions);
         }
@@ -790,13 +796,7 @@ function plot(container, subplot, cdata) {
             scene.error2d.update(errorBatch);
         }
         if(scene.scatter2d) {
-            if(!scene.selectBatch) {
-                scene.scatter2d.update(scene.markerOptions);
-            }
-            else {
-                scene.scatter2d.update(scene.unselectedOptions);
-                scene.select2d.update(scene.selectedOptions);
-            }
+            scene.scatter2d.update(scene.markerOptions);
         }
         // fill requires linked traces, so we generate it's positions here
         if(scene.fill2d) {
@@ -888,21 +888,15 @@ function plot(container, subplot, cdata) {
         }
     }
 
-    // make sure selection layer is initialized if we require selection
-    var dragmode = layout.dragmode;
-
-    if(dragmode === 'lasso' || dragmode === 'select') {
-        if(scene.select2d && scene.selectBatch) {
-            scene.scatter2d.update(scene.unselectedOptions);
-        }
-    }
+    var selectMode = dragmode === 'lasso' || dragmode === 'select';
 
     // provide viewport and range
     var vpRange = cdata.map(function(cdscatter) {
         if(!cdscatter || !cdscatter[0] || !cdscatter[0].trace) return;
         var cd = cdscatter[0];
         var trace = cd.trace;
         var stash = cd.t;
+        var id = stash.index;
         var x = stash.rawx,
             y = stash.rawy;
 
@@ -924,30 +918,16 @@ function plot(container, subplot, cdata) {
             (height - vpSize.t) - (1 - yaxis.domain[1]) * vpSize.h
         ];
 
-        if(trace.selectedpoints || dragmode === 'lasso' || dragmode === 'select') {
-            // create select2d
-            if(!scene.select2d && scene.scatter2d) {
-                var selectRegl = layout._glcanvas.data()[1].regl;
+        if(trace.selectedpoints || selectMode) {
+            if(!selectMode) selectMode = true;
 
-                // create scatter instance by cloning scatter2d
-                scene.select2d = createScatter(selectRegl, {clone: scene.scatter2d});
-                scene.select2d.update(scene.selectedOptions);
+            if(!scene.selectBatch) scene.selectBatch = [];
+            if(!scene.unselectBatch) scene.unselectBatch = [];
 
-                // create selection style once we have something selected
-                if(trace.selectedpoints && !scene.selectBatch) {
-                    scene.selectBatch = Array(scene.count);
-                    scene.unselectBatch = Array(scene.count);
-                    scene.scatter2d.update(scene.unselectedOptions);
-                }
-            }
-            else {
-                // update selection positions, since they may have changed by panning or alike
-                scene.select2d.update(scene.selectedOptions);
-            }
+            // regenerate scene batch, if traces number changed during selection
+            if(trace.selectedpoints) {
+                scene.selectBatch[id] = trace.selectedpoints;
 
-            // form unselected batch
-            if(trace.selectedpoints && !scene.unselectBatch[stash.index]) {
-                scene.selectBatch[stash.index] = trace.selectedpoints;
                 var selPts = trace.selectedpoints;
                 var selDict = {};
                 for(i = 0; i < selPts.length; i++) {
@@ -957,7 +937,7 @@ function plot(container, subplot, cdata) {
                 for(i = 0; i < stash.count; i++) {
                     if(!selDict[i]) unselPts.push(i);
                 }
-                scene.unselectBatch[stash.index] = unselPts;
+                scene.unselectBatch[id] = unselPts;
             }
 
             // precalculate px coords since we are not going to pan during select
@@ -979,6 +959,21 @@ function plot(container, subplot, cdata) {
         } : null;
     });
 
+    if(selectMode) {
+        // create select2d
+        if(!scene.select2d) {
+            // create scatter instance by cloning scatter2d
+            scene.select2d = createScatter(layout._glcanvas.data()[1].regl, {clone: scene.scatter2d});
+        }
+
+        // update only traces with selection
+        scene.scatter2d.update(scene.unselectedOptions.map(function(opts, i) {
+            return scene.selectBatch[i] ? opts : null;
+        }));
+        scene.select2d.update(scene.markerOptions);
+        scene.select2d.update(scene.selectedOptions);
+    }
+
     // uploat viewport/range data to GPU
     if(scene.fill2d) {
         scene.fill2d.update(vpRange);
@@ -1193,18 +1188,25 @@ function selectPoints(searchInfo, polygon) {
         }
     }
     else {
-        unels = Array(stash.count);
-        for(i = 0; i < stash.count; i++) {
-            unels[i] = i;
-        }
+        unels = arrayRange(stash.count);
     }
 
-    // create selection style once we have something selected
+    // make sure selectBatch is created
     if(!scene.selectBatch) {
-        scene.selectBatch = Array(scene.count);
-        scene.unselectBatch = Array(scene.count);
+        scene.selectBatch = [];
+        scene.unselectBatch = [];
+    }
+
+    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
         scene.scatter2d.update(scene.unselectedOptions);
     }
+
     scene.selectBatch[stash.index] = els;
     scene.unselectBatch[stash.index] = unels;
 

test/jasmine/tests/gl2d_plot_interact_test.js

@@ -4,6 +4,7 @@ var Plotly = require('@lib/index');
 var Plots = require('@src/plots/plots');
 var Lib = require('@src/lib');
 var Drawing = require('@src/components/drawing');
+var ScatterGl = require('@src/traces/scattergl');
 
 var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
@@ -14,6 +15,7 @@ var selectButton = require('../assets/modebar_button');
 var delay = require('../assets/delay');
 var readPixel = require('../assets/read_pixel');
 
+
 function countCanvases() {
     return d3.selectAll('canvas').size();
 }
@@ -406,7 +408,6 @@ describe('Test gl2d plots', function() {
         .then(done);
     });
 
-
     it('@noCI should display selection of big number of miscellaneous points', function(done) {
         var colorList = [
             '#006385', '#F06E75', '#90ed7d', '#f7a35c', '#8085e9',
@@ -645,4 +646,75 @@ describe('Test gl2d plots', function() {
         .catch(fail)
         .then(done);
     });
+
+    it('should restyle opacity', function(done) {
+        // #2299
+        spyOn(ScatterGl, 'calc');
+
+        var dat = [{
+            'x': [1, 2, 3],
+            'y': [1, 2, 3],
+            'type': 'scattergl',
+            'mode': 'markers'
+        }];
+
+        Plotly.plot(gd, dat, {width: 500, height: 500})
+        .then(function() {
+            expect(ScatterGl.calc).toHaveBeenCalledTimes(1);
+
+            return Plotly.restyle(gd, {'opacity': 0.1});
+        })
+        .then(function() {
+            expect(ScatterGl.calc).toHaveBeenCalledTimes(2);
+        })
+        .catch(fail)
+        .then(done);
+    });
+
+    it('should update selected points', function(done) {
+        // #2298
+        var dat = [{
+            'x': [1],
+            'y': [1],
+            'type': 'scattergl',
+            'mode': 'markers',
+            'selectedpoints': [0]
+        }];
+
+        Plotly.plot(gd, dat, {
+            width: 500,
+            height: 500,
+            dragmode: 'select'
+        })
+        .then(function() {
+            var scene = gd._fullLayout._plots.xy._scene;
+
+            expect(scene.count).toBe(1);
+            expect(scene.selectBatch).toEqual([[0]]);
+            expect(scene.unselectBatch).toEqual([[]]);
+            spyOn(scene.scatter2d, 'draw');
+
+            var trace = {
+                x: [2],
+                y: [1],
+                type: 'scattergl',
+                mode: 'markers',
+                marker: {color: 'red'}
+            };
+
+            return Plotly.addTraces(gd, trace);
+        })
+        .then(function() {
+            var scene = gd._fullLayout._plots.xy._scene;
+
+            expect(scene.count).toBe(2);
+            expect(scene.selectBatch).toBeDefined();
+            expect(scene.unselectBatch).toBeDefined();
+            expect(scene.markerOptions.length).toBe(2);
+            expect(scene.markerOptions[1].color).toEqual(new Uint8Array([255, 0, 0, 255]));
+            expect(scene.scatter2d.draw).toHaveBeenCalled();
+        })
+        .catch(fail)
+        .then(done);
+    });
 });