Merge pull request #2442 from plotly/scattergl-visible-fix

scattergl visible restyle fix

Author: etpinard

src/traces/scattergl/attributes.js

@@ -83,9 +83,7 @@ var attrs = module.exports = overrideAll({
         marker: scatterAttrs.unselected.marker
     },
 
-    opacity: extendFlat({}, plotAttrs.opacity, {
-        editType: 'calc'
-    }),
+    opacity: plotAttrs.opacity
 
 }, 'calc', 'nested');
 

src/traces/scattergl/index.js

@@ -141,13 +141,9 @@ function sceneOptions(gd, subplot, trace, positions) {
     var fullLayout = gd._fullLayout;
     var count = positions.length / 2;
     var markerOpts = trace.marker;
-    var xaxis = AxisIDs.getFromId(gd, trace.xaxis);
-    var yaxis = AxisIDs.getFromId(gd, trace.yaxis);
-    var ptrX = 0;
-    var ptrY = 0;
     var i;
 
-    var hasLines, hasErrorX, hasErrorY, hasError, hasMarkers, hasFill;
+    var hasLines, hasErrorX, hasErrorY, hasMarkers, hasFill;
 
     if(trace.visible !== true) {
         hasLines = false;
@@ -159,7 +155,6 @@ function sceneOptions(gd, subplot, trace, positions) {
         hasLines = subTypes.hasLines(trace) && positions.length > 1;
         hasErrorX = trace.error_x && trace.error_x.visible === true;
         hasErrorY = trace.error_y && trace.error_y.visible === true;
-        hasError = hasErrorX || hasErrorY;
         hasMarkers = subTypes.hasMarkers(trace);
         hasFill = !!trace.fill && trace.fill !== 'none';
     }
@@ -169,67 +164,16 @@ function sceneOptions(gd, subplot, trace, positions) {
     var selectedOptions, unselectedOptions;
     var linePositions;
 
-    // get error values
-    var errorVals = hasError ?
-        Registry.getComponentMethod('errorbars', 'calcFromTrace')(trace, fullLayout) :
-        null;
-
-    if(hasErrorX) {
-        errorXOptions = {};
-        errorXOptions.positions = positions;
-        var errorsX = new Float64Array(4 * count);
-
-        if(xaxis.type === 'log') {
-            for(i = 0; i < count; ++i) {
-                errorsX[ptrX++] = positions[i * 2] - xaxis.d2l(errorVals[i].xs) || 0;
-                errorsX[ptrX++] = xaxis.d2l(errorVals[i].xh) - positions[i * 2] || 0;
-                errorsX[ptrX++] = 0;
-                errorsX[ptrX++] = 0;
-            }
-        } else {
-            for(i = 0; i < count; ++i) {
-                errorsX[ptrX++] = positions[i * 2] - errorVals[i].xs || 0;
-                errorsX[ptrX++] = errorVals[i].xh - positions[i * 2] || 0;
-                errorsX[ptrX++] = 0;
-                errorsX[ptrX++] = 0;
-            }
-        }
+    if(hasErrorX || hasErrorY) {
+        var calcFromTrace = Registry.getComponentMethod('errorbars', 'calcFromTrace');
+        var errorVals = calcFromTrace(trace, fullLayout);
 
-        if(trace.error_x.copy_ystyle) {
-            trace.error_x = trace.error_y;
+        if(hasErrorX) {
+            errorXOptions = makeErrorOptions('x', trace.error_x, errorVals);
         }
-
-        errorXOptions.errors = errorsX;
-        errorXOptions.capSize = trace.error_x.width * 2;
-        errorXOptions.lineWidth = trace.error_x.thickness;
-        errorXOptions.color = trace.error_x.color;
-    }
-
-    if(hasErrorY) {
-        errorYOptions = {};
-        errorYOptions.positions = positions;
-        var errorsY = new Float64Array(4 * count);
-
-        if(yaxis.type === 'log') {
-            for(i = 0; i < count; ++i) {
-                errorsY[ptrY++] = 0;
-                errorsY[ptrY++] = 0;
-                errorsY[ptrY++] = positions[i * 2 + 1] - yaxis.d2l(errorVals[i].ys) || 0;
-                errorsY[ptrY++] = yaxis.d2l(errorVals[i].yh) - positions[i * 2 + 1] || 0;
-            }
-        } else {
-            for(i = 0; i < count; ++i) {
-                errorsY[ptrY++] = 0;
-                errorsY[ptrY++] = 0;
-                errorsY[ptrY++] = positions[i * 2 + 1] - errorVals[i].ys || 0;
-                errorsY[ptrY++] = errorVals[i].yh - positions[i * 2 + 1] || 0;
-            }
+        if(hasErrorY) {
+            errorYOptions = makeErrorOptions('y', trace.error_y, errorVals);
         }
-
-        errorYOptions.errors = errorsY;
-        errorYOptions.capSize = trace.error_y.width * 2;
-        errorYOptions.lineWidth = trace.error_y.thickness;
-        errorYOptions.color = trace.error_y.color;
     }
 
     if(hasLines) {
@@ -330,6 +274,33 @@ function sceneOptions(gd, subplot, trace, positions) {
         markerOptions.positions = positions;
     }
 
+    function makeErrorOptions(axLetter, errorOpts, vals) {
+        var options = {};
+        options.positions = positions;
+
+        var ax = AxisIDs.getFromId(gd, trace[axLetter + 'axis']);
+        var errors = options.errors = new Float64Array(4 * count);
+        var pOffset = {x: 0, y: 1}[axLetter];
+        var eOffset = {x: [0, 1, 2, 3], y: [2, 3, 0, 1]}[axLetter];
+
+        for(var i = 0, p = 0; i < count; i++, p += 4) {
+            errors[p + eOffset[0]] = positions[i * 2 + pOffset] - ax.d2l(vals[i][axLetter + 's']) || 0;
+            errors[p + eOffset[1]] = ax.d2l(vals[i][axLetter + 'h']) - positions[i * 2 + pOffset] || 0;
+            errors[p + eOffset[2]] = 0;
+            errors[p + eOffset[3]] = 0;
+        }
+
+        if(errorOpts.copy_ystyle) {
+            errorOpts = trace.error_y;
+        }
+
+        options.capSize = errorOpts.width * 2;
+        options.lineWidth = errorOpts.thickness;
+        options.color = errorOpts.color;
+
+        return options;
+    }
+
     function makeSelectedOptions(selected, markerOpts) {
         var options = {};
 
@@ -486,39 +457,42 @@ function sceneUpdate(gd, subplot) {
     var scene = subplot._scene;
     var fullLayout = gd._fullLayout;
 
+    var reset = {
+        // 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,
+        // last used options
+        lineOptions: [],
+        fillOptions: [],
+        markerOptions: [],
+        selectedOptions: [],
+        unselectedOptions: [],
+        errorXOptions: [],
+        errorYOptions: []
+    };
+
+    var first = {
+        selectBatch: null,
+        unselectBatch: null,
+        // regl- component stubs, initialized in dirty plot call
+        fill2d: false,
+        scatter2d: false,
+        error2d: false,
+        line2d: false,
+        select2d: null
+    };
+
     if(!subplot._scene) {
-        scene = subplot._scene = {
-            // 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,
-
-            // last used options
-            lineOptions: [],
-            fillOptions: [],
-            markerOptions: [],
-            selectedOptions: [],
-            unselectedOptions: [],
-            errorXOptions: [],
-            errorYOptions: [],
-            selectBatch: null,
-            unselectBatch: null,
-
-            // regl- component stubs, initialized in dirty plot call
-            fill2d: false,
-            scatter2d: false,
-            error2d: false,
-            line2d: false,
-            select2d: null
-        };
+        scene = subplot._scene = Lib.extendFlat({}, reset, first);
 
-        // apply new option to all regl components
+        // apply new option to all regl components (used on drag)
         scene.update = function update(opt) {
-            var opts = Array(scene.count);
+            var opts = new Array(scene.count);
             for(var i = 0; i < scene.count; i++) {
                 opts[i] = opt;
             }
+
             if(scene.fill2d) scene.fill2d.update(opts);
             if(scene.scatter2d) scene.scatter2d.update(opts);
             if(scene.line2d) scene.line2d.update(opts);
@@ -532,21 +506,24 @@ function sceneUpdate(gd, subplot) {
         scene.draw = function draw() {
             var i;
             for(i = 0; i < scene.count; i++) {
-                if(scene.fill2d) scene.fill2d.draw(i);
+                if(scene.fill2d && scene.fillOptions[i]) {
+                    // must do all fills first
+                    scene.fill2d.draw(i);
+                }
             }
             for(i = 0; i < scene.count; i++) {
-                if(scene.line2d) {
+                if(scene.line2d && scene.lineOptions[i]) {
                     scene.line2d.draw(i);
                 }
-                if(scene.error2d) {
+                if(scene.error2d && scene.errorXOptions[i]) {
                     scene.error2d.draw(i);
+                }
+                if(scene.error2d && scene.errorYOptions[i]) {
                     scene.error2d.draw(i + scene.count);
                 }
-                if(scene.scatter2d) {
+                if(scene.scatter2d && scene.markerOptions[i] && (!scene.selectBatch || !scene.selectBatch[i])) {
                     // traces in no-selection mode
-                    if(!scene.selectBatch || !scene.selectBatch[i]) {
-                        scene.scatter2d.draw(i);
-                    }
+                    scene.scatter2d.draw(i);
                 }
             }
 
@@ -639,15 +616,7 @@ function sceneUpdate(gd, subplot) {
 
     // In case if we have scene from the last calc - reset data
     if(!scene.dirty) {
-        scene.dirty = true;
-        scene.count = 0;
-        scene.lineOptions = [];
-        scene.fillOptions = [];
-        scene.markerOptions = [];
-        scene.selectedOptions = [];
-        scene.unselectedOptions = [];
-        scene.errorXOptions = [];
-        scene.errorYOptions = [];
+        Lib.extendFlat(scene, reset);
     }
 
     return scene;

test/jasmine/tests/gl2d_plot_interact_test.js

@@ -282,10 +282,16 @@ describe('@gl Test gl2d plots', function() {
             gd.on('plotly_relayout', relayoutCallback);
         })
         .then(function() {
+            var scene = gd._fullLayout._plots.xy._scene;
+            spyOn(scene, 'draw');
+
             // Drag scene along the X axis
             return mouseTo([200, 200], [220, 200]);
         })
         .then(function() {
+            var scene = gd._fullLayout._plots.xy._scene;
+            expect(scene.draw).toHaveBeenCalledTimes(3);
+
             expect(gd.layout.xaxis.autorange).toBe(false);
             expect(gd.layout.yaxis.autorange).toBe(false);
             expect(gd.layout.xaxis.range).toBeCloseToArray(newX, precision);
@@ -379,6 +385,48 @@ describe('@gl Test gl2d plots', function() {
         .then(done);
     });
 
+    it('should be able to toggle trace with different modes', function(done) {
+        Plotly.newPlot(gd, [{
+            // a trace with all regl2d objects
+            type: 'scattergl',
+            y: [1, 2, 1],
+            error_x: {value: 10},
+            error_y: {value: 10},
+            fill: 'tozeroy'
+        }, {
+            type: 'scattergl',
+            mode: 'markers',
+            y: [0, 1, -1]
+        }])
+        .then(function() {
+            var scene = gd._fullLayout._plots.xy._scene;
+            spyOn(scene.fill2d, 'draw');
+            spyOn(scene.line2d, 'draw');
+            spyOn(scene.error2d, 'draw');
+            spyOn(scene.scatter2d, 'draw');
+
+            return Plotly.restyle(gd, 'visible', 'legendonly', [0]);
+        })
+        .then(function() {
+            var scene = gd._fullLayout._plots.xy._scene;
+            expect(scene.fill2d.draw).toHaveBeenCalledTimes(0);
+            expect(scene.line2d.draw).toHaveBeenCalledTimes(0);
+            expect(scene.error2d.draw).toHaveBeenCalledTimes(0);
+            expect(scene.scatter2d.draw).toHaveBeenCalledTimes(1);
+
+            return Plotly.restyle(gd, 'visible', true, [0]);
+        })
+        .then(function() {
+            var scene = gd._fullLayout._plots.xy._scene;
+            expect(scene.fill2d.draw).toHaveBeenCalledTimes(1);
+            expect(scene.line2d.draw).toHaveBeenCalledTimes(1);
+            expect(scene.error2d.draw).toHaveBeenCalledTimes(2, 'twice for x AND y');
+            expect(scene.scatter2d.draw).toHaveBeenCalledTimes(3, 'both traces have markers');
+        })
+        .catch(fail)
+        .then(done);
+    });
+
     it('@noCI should display selection of big number of regular points', function(done) {
         // generate large number of points
         var x = [], y = [], n = 2e2, N = n * n;