Scatterpolargl fixes

src/plots/polar/index.js

@@ -53,6 +53,9 @@ function plot(gd) {
 
 function clean(newFullData, newFullLayout, oldFullData, oldFullLayout) {
     var oldIds = oldFullLayout._subplots[name] || [];
+    var hadGl = (oldFullLayout._has && oldFullLayout._has('gl'));
+    var hasGl = (newFullLayout._has && newFullLayout._has('gl'));
+    var mustCleanScene = hadGl && !hasGl;
 
     for(var i = 0; i < oldIds.length; i++) {
         var id = oldIds[i];
@@ -66,6 +69,11 @@ function clean(newFullData, newFullLayout, oldFullData, oldFullLayout) {
                 oldSubplot.clipPaths[k].remove();
             }
         }
+
+        if(mustCleanScene && oldSubplot._scene) {
+            oldSubplot._scene.destroy();
+            oldSubplot._scene = null;
+        }
     }
 }
 

src/traces/scattergl/index.js

@@ -42,8 +42,8 @@ function calc(gd, trace) {
     var xa = AxisIDs.getFromId(gd, trace.xaxis);
     var ya = AxisIDs.getFromId(gd, trace.yaxis);
     var subplot = fullLayout._plots[trace.xaxis + trace.yaxis];
-    var count = trace._length;
-    var count2 = count * 2;
+    var len = trace._length;
+    var len2 = len * 2;
     var stash = {};
     var i, xx, yy;
 
@@ -52,21 +52,21 @@ function calc(gd, trace) {
 
     // we need hi-precision for scatter2d,
     // regl-scatter2d uses NaNs for bad/missing values
-    var positions = new Array(count2);
-    for(i = 0; i < count; i++) {
+    var positions = new Array(len2);
+    for(i = 0; i < len; i++) {
         xx = x[i];
         yy = y[i];
         positions[i * 2] = xx === BADNUM ? NaN : xx;
         positions[i * 2 + 1] = yy === BADNUM ? NaN : yy;
     }
 
     if(xa.type === 'log') {
-        for(i = 0; i < count2; i += 2) {
+        for(i = 0; i < len2; i += 2) {
             positions[i] = xa.c2l(positions[i]);
         }
     }
     if(ya.type === 'log') {
-        for(i = 1; i < count2; i += 2) {
+        for(i = 1; i < len2; i += 2) {
             positions[i] = ya.c2l(positions[i]);
         }
     }
@@ -77,8 +77,8 @@ function calc(gd, trace) {
         // FIXME: delegate this to webworker
         stash.tree = cluster(positions);
     } else {
-        var ids = stash.ids = new Array(count);
-        for(i = 0; i < count; i++) {
+        var ids = stash.ids = new Array(len);
+        for(i = 0; i < len; i++) {
             ids[i] = i;
         }
     }
@@ -92,12 +92,9 @@ function calc(gd, trace) {
     // For graphs with very large number of points and array marker.size,
     // use average marker size instead to speed things up.
     setFirstScatter(fullLayout, trace);
-    var ppad;
-    if(count < TOO_MANY_POINTS) {
-        ppad = calcMarkerSize(trace, count);
-    } else if(opts.marker) {
-        ppad = 2 * (opts.marker.sizeAvg || Math.max(opts.marker.size, 3));
-    }
+    var ppad = len < TOO_MANY_POINTS ?
+        calcMarkerSize(trace, len) :
+        2 * (opts.marker.sizeAvg || Math.max(opts.marker.size, 3));
     calcAxisExpansion(gd, trace, xa, ya, x, y, ppad);
     if(opts.errorX) expandForErrorBars(trace, xa, opts.errorX);
     if(opts.errorY) expandForErrorBars(trace, ya, opts.errorY);
@@ -111,7 +108,7 @@ function calc(gd, trace) {
 
     // FIXME: organize it in a more appropriate manner, probably in sceneOptions
     // put point-cluster instance for optimized regl calc
-    if(opts.marker && count >= TOO_MANY_POINTS) {
+    if(opts.marker && len >= TOO_MANY_POINTS) {
         opts.marker.cluster = stash.tree;
     }
 
@@ -129,13 +126,10 @@ function calc(gd, trace) {
 
     // stash scene ref
     stash._scene = scene;
-    stash.index = scene.count;
+    stash.index = scene.count++;
     stash.x = x;
     stash.y = y;
     stash.positions = positions;
-    stash.count = count;
-
-    scene.count++;
 
     return [{x: false, y: false, t: stash, trace: trace}];
 }
@@ -554,6 +548,7 @@ function plot(gd, subplot, cdata) {
         var trace = cd0.trace;
         var stash = cd0.t;
         var index = stash.index;
+        var len = trace._length;
         var x = stash.x;
         var y = stash.y;
 
@@ -574,7 +569,7 @@ function plot(gd, subplot, cdata) {
                     selDict[selPts[j]] = 1;
                 }
                 var unselPts = [];
-                for(j = 0; j < stash.count; j++) {
+                for(j = 0; j < len; j++) {
                     if(!selDict[j]) unselPts.push(j);
                 }
                 scene.unselectBatch[index] = unselPts;
@@ -585,9 +580,9 @@ function plot(gd, subplot, cdata) {
             // - spin that in a webworker
             // - compute selection from polygons in data coordinates
             //   (maybe just for linear axes)
-            var xpx = stash.xpx = new Array(stash.count);
-            var ypx = stash.ypx = new Array(stash.count);
-            for(j = 0; j < stash.count; j++) {
+            var xpx = stash.xpx = new Array(len);
+            var ypx = stash.ypx = new Array(len);
+            for(j = 0; j < len; j++) {
                 xpx[j] = xaxis.c2p(x[j]);
                 ypx[j] = yaxis.c2p(y[j]);
             }
@@ -849,6 +844,7 @@ function selectPoints(searchInfo, selectionTester) {
     var selection = [];
     var trace = cd[0].trace;
     var stash = cd[0].t;
+    var len = trace._length;
     var x = stash.x;
     var y = stash.y;
     var scene = stash._scene;
@@ -868,7 +864,7 @@ function selectPoints(searchInfo, selectionTester) {
     var i;
     if(selectionTester !== false && !selectionTester.degenerate) {
         els = [], unels = [];
-        for(i = 0; i < stash.count; i++) {
+        for(i = 0; i < len; i++) {
             if(selectionTester.contains([stash.xpx[i], stash.ypx[i]], false, i, searchInfo)) {
                 els.push(i);
                 selection.push({
@@ -882,7 +878,7 @@ function selectPoints(searchInfo, selectionTester) {
             }
         }
     } else {
-        unels = arrayRange(stash.count);
+        unels = arrayRange(len);
     }
 
     // make sure selectBatch is created
@@ -916,6 +912,7 @@ function selectPoints(searchInfo, selectionTester) {
 
 function styleTextSelection(cd) {
     var cd0 = cd[0];
+    var trace = cd0.trace;
     var stash = cd0.t;
     var scene = stash._scene;
     var index = stash.index;
@@ -932,8 +929,7 @@ function styleTextSelection(cd) {
         var utc = unselOpts.color;
         var base = baseOpts.color;
         var hasArrayBase = Array.isArray(base);
-        opts.color = new Array(stash.count);
-
+        opts.color = new Array(trace._length);
 
         for(i = 0; i < els.length; i++) {
             j = els[i];

src/traces/scatterpolargl/index.js

@@ -13,45 +13,52 @@ var isNumeric = require('fast-isnumeric');
 
 var ScatterGl = require('../scattergl');
 var calcColorscales = require('../scatter/colorscale_calc');
+var calcMarkerSize = require('../scatter/calc').calcMarkerSize;
 var Axes = require('../../plots/cartesian/axes');
 var makeHoverPointText = require('../scatterpolar/hover').makeHoverPointText;
 var subTypes = require('../scatter/subtypes');
 
 var TOO_MANY_POINTS = require('../scattergl/constants').TOO_MANY_POINTS;
 
-function calc(container, trace) {
-    var layout = container._fullLayout;
+function calc(gd, trace) {
+    var fullLayout = gd._fullLayout;
     var subplotId = trace.subplot;
-    var radialAxis = layout[subplotId].radialaxis;
-    var angularAxis = layout[subplotId].angularaxis;
+    var radialAxis = fullLayout[subplotId].radialaxis;
+    var angularAxis = fullLayout[subplotId].angularaxis;
     var rArray = radialAxis.makeCalcdata(trace, 'r');
     var thetaArray = angularAxis.makeCalcdata(trace, 'theta');
+    var len = trace._length;
     var stash = {};
 
-    if(trace._length < rArray.length) rArray = rArray.slice(0, trace._length);
-    if(trace._length < thetaArray.length) thetaArray = thetaArray.slice(0, trace._length);
-
-    calcColorscales(trace);
+    if(len < rArray.length) rArray = rArray.slice(0, len);
+    if(len < thetaArray.length) thetaArray = thetaArray.slice(0, len);
 
     stash.r = rArray;
     stash.theta = thetaArray;
 
-    trace._extremes.x = Axes.findExtremes(radialAxis, rArray, {tozero: true});
+    // We could add TOO_MANY_POINTS logic like in Scattergl.calc,
+    // if users asks for scaterpolargl charts with > 1e5 pts
+    var ppad = calcMarkerSize(trace, len);
+    trace._extremes.x = Axes.findExtremes(radialAxis, rArray, {ppad: ppad});
+
+    calcColorscales(trace);
 
     return [{x: false, y: false, t: stash, trace: trace}];
 }
 
-function plot(container, subplot, cdata) {
+function plot(gd, subplot, cdata) {
+    if(!cdata.length) return;
+
     var radialAxis = subplot.radialAxis;
     var angularAxis = subplot.angularAxis;
-
-    var scene = ScatterGl.sceneUpdate(container, subplot);
+    var scene = ScatterGl.sceneUpdate(gd, subplot);
 
     cdata.forEach(function(cdscatter, traceIndex) {
         if(!cdscatter || !cdscatter[0] || !cdscatter[0].trace) return;
         var cd = cdscatter[0];
         var trace = cd.trace;
         var stash = cd.t;
+        var len = trace._length;
         var rArray = stash.r;
         var thetaArray = stash.theta;
         var i;
@@ -67,12 +74,11 @@ function plot(container, subplot, cdata) {
             }
         }
 
-        var count = rArray.length;
-        var positions = new Array(count * 2);
-        var x = Array(count);
-        var y = Array(count);
+        var positions = new Array(len * 2);
+        var x = Array(len);
+        var y = Array(len);
 
-        for(i = 0; i < count; i++) {
+        for(i = 0; i < len; i++) {
             var r = subRArray[i];
             var xx, yy;
 
@@ -132,10 +138,9 @@ function plot(container, subplot, cdata) {
         stash.r = rArray;
         stash.theta = thetaArray;
         stash.positions = positions;
-        stash.count = count;
     });
 
-    return ScatterGl.plot(container, subplot, cdata);
+    return ScatterGl.plot(gd, subplot, cdata);
 }
 
 function hoverPoints(pointData, xval, yval, hovermode) {

test/jasmine/tests/scatterpolargl_test.js

@@ -1,10 +1,12 @@
 var Plotly = require('@lib');
 var Lib = require('@src/lib');
 
+var d3 = require('d3');
 var createGraphDiv = require('../assets/create_graph_div');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
 var failTest = require('../assets/fail_test');
 var mouseEvent = require('../assets/mouse_event');
+var readPixel = require('../assets/read_pixel');
 
 var customAssertions = require('../assets/custom_assertions');
 var assertHoverLabelContent = customAssertions.assertHoverLabelContent;
@@ -102,3 +104,69 @@ describe('Test scatterpolargl hover:', function() {
         });
     });
 });
+
+describe('Test scatterpolargl interactions:', function() {
+    var gd;
+
+    afterEach(function() {
+        Plotly.purge(gd);
+        destroyGraphDiv();
+    });
+
+    function countCanvases() {
+        return d3.selectAll('canvas').size();
+    }
+
+    function totalPixels() {
+        return readPixel(gd.querySelector('.gl-canvas-context'), 0, 0, 400, 400)
+            .reduce(function(acc, v) { return acc + v; }, 0);
+    }
+
+    it('@gl should be able to toggle from svg to gl', function(done) {
+        gd = createGraphDiv();
+
+        var scene;
+
+        Plotly.plot(gd, [{
+            type: 'scatterpolar',
+            r: [1, 2, 1],
+        }], {
+            width: 400,
+            height: 400
+        })
+        .then(function() {
+            expect(countCanvases()).toBe(0);
+            expect(d3.selectAll('.scatterlayer > .trace').size()).toBe(1);
+
+            return Plotly.restyle(gd, 'type', 'scatterpolargl');
+        })
+        .then(function() {
+            expect(countCanvases()).toBe(3);
+            expect(totalPixels()).not.toBe(0);
+            expect(d3.selectAll('.scatterlayer > .trace').size()).toBe(0);
+
+            scene = gd._fullLayout.polar._subplot._scene;
+            spyOn(scene, 'destroy').and.callThrough();
+
+            return Plotly.restyle(gd, 'type', 'scatterpolar');
+        })
+        .then(function() {
+            expect(countCanvases()).toBe(0);
+            expect(scene.destroy).toHaveBeenCalledTimes(1);
+            expect(gd._fullLayout.polar._subplot._scene).toBe(null);
+            expect(d3.selectAll('.scatterlayer > .trace').size()).toBe(1);
+
+            return Plotly.restyle(gd, 'type', 'scatterpolargl');
+        })
+        .then(function() {
+            expect(countCanvases()).toBe(3);
+            // this here was failing before
+            // https://github.com/plotly/plotly.js/issues/3094
+            // got fixed
+            expect(totalPixels()).not.toBe(0);
+            expect(d3.selectAll('.scatterlayer > .trace').size()).toBe(0);
+        })
+        .catch(failTest)
+        .then(done);
+    });
+});