Merge pull request #2377 from plotly/regl-scatter-fix

Fix color clustering
plotly · Feb 22, 2018 · 1256c4f · 1256c4f
2 parents ce5bc97 + 0e746bf
commit 1256c4f
Show file tree

Hide file tree

Showing 10 changed files with 942 additions and 1,316 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -83,7 +83,7 @@
     "gl-select-box": "^1.0.2",
     "gl-spikes2d": "^1.0.1",
     "gl-surface3d": "^1.3.4",
-    "glslify": "^6.1.0",
+    "glslify": "^6.1.1",
     "has-hover": "^1.0.1",
     "has-passive-events": "^1.0.0",
     "kdgrass": "^1.0.1",
@@ -99,8 +99,8 @@
     "polybooljs": "^1.2.0",
     "regl": "^1.3.1",
     "regl-error2d": "^2.0.3",
-    "regl-line2d": "^2.1.4",
-    "regl-scatter2d": "^2.1.13",
+    "regl-line2d": "^2.1.5",
+    "regl-scatter2d": "^2.1.17",
     "right-now": "^1.0.0",
     "robust-orientation": "^1.1.3",
     "sane-topojson": "^2.0.0",
@@ -119,18 +119,18 @@
     "check-node-version": "^3.2.0",
     "deep-equal": "^1.0.1",
     "ecstatic": "^3.2.0",
-    "eslint": "^4.17.0",
+    "eslint": "^4.18.0",
     "falafel": "^2.0.0",
     "fs-extra": "^2.0.0",
     "fuse.js": "^3.2.0",
     "glob": "^7.0.0",
     "gzip-size": "^4.1.0",
     "image-size": "^0.6.2",
     "into-stream": "^3.1.0",
-    "jasmine-core": "^2.4.1",
+    "jasmine-core": "^2.99.1",
     "jsdom": "^11.6.2",
     "karma": "^2.0.0",
-    "karma-browserify": "^5.1.1",
+    "karma-browserify": "^5.2.0",
     "karma-chrome-launcher": "^2.0.0",
     "karma-coverage": "^1.0.0",
     "karma-firefox-launcher": "^1.0.1",

diff --git a/src/lib/index.js b/src/lib/index.js
@@ -876,3 +876,19 @@ lib.subplotSort = function(a, b) {
     }
     return numB - numA;
 };
+
+// repeatable pseudorandom generator
+var randSeed = 2000000000;
+
+lib.seedPseudoRandom = function() {
+    randSeed = 2000000000;
+};
+
+lib.pseudoRandom = function() {
+    var lastVal = randSeed;
+    randSeed = (69069 * randSeed + 1) % 4294967296;
+    // don't let consecutive vals be too close together
+    // gets away from really trying to be random, in favor of better local uniformity
+    if(Math.abs(randSeed - lastVal) < 429496729) return lib.pseudoRandom();
+    return randSeed / 4294967296;
+};
diff --git a/src/traces/box/plot.js b/src/traces/box/plot.js
@@ -13,22 +13,6 @@ var d3 = require('d3');
 var Lib = require('../../lib');
 var Drawing = require('../../components/drawing');
 
-// repeatable pseudorandom generator
-var randSeed = 2000000000;
-
-function seed() {
-    randSeed = 2000000000;
-}
-
-function rand() {
-    var lastVal = randSeed;
-    randSeed = (69069 * randSeed + 1) % 4294967296;
-    // don't let consecutive vals be too close together
-    // gets away from really trying to be random, in favor of better local uniformity
-    if(Math.abs(randSeed - lastVal) < 429496729) return rand();
-    return randSeed / 4294967296;
-}
-
 // constants for dynamic jitter (ie less jitter for sparser points)
 var JITTERCOUNT = 5; // points either side of this to include
 var JITTERSPREAD = 0.01; // fraction of IQR to count as "dense"
@@ -179,8 +163,8 @@ function plotPoints(sel, axes, trace, t) {
     // to support violin points
     var mode = trace.boxpoints || trace.points;
 
-    // repeatable pseudorandom number generator
-    seed();
+    // repeatable pseudo-random number generator
+    Lib.seedPseudoRandom();
 
     sel.selectAll('g.points')
         // since box plot points get an extra level of nesting, each
@@ -247,7 +231,7 @@ function plotPoints(sel, axes, trace, t) {
                 var v = pt.v;
 
                 var jitterOffset = trace.jitter ?
-                    (newJitter * jitterFactors[i] * (rand() - 0.5)) :
+                    (newJitter * jitterFactors[i] * (Lib.pseudoRandom() - 0.5)) :
                     0;
 
                 var posPx = d.pos + bPos + bdPos * (trace.pointpos + jitterOffset);

diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js
@@ -779,9 +779,9 @@ function plot(container, subplot, cdata) {
         }
         // fill requires linked traces, so we generate it's positions here
         if(scene.fill2d) {
-            scene.fillOptions.forEach(function(fillOptions, i) {
+            scene.fillOptions = scene.fillOptions.map(function(fillOptions, i) {
                 var cdscatter = cdata[i];
-                if(!fillOptions || !cdscatter || !cdscatter[0] || !cdscatter[0].trace) return;
+                if(!fillOptions || !cdscatter || !cdscatter[0] || !cdscatter[0].trace) return null;
                 var cd = cdscatter[0];
                 var trace = cd.trace;
                 var stash = cd.t;
@@ -861,6 +861,8 @@ function plot(container, subplot, cdata) {
 
                 fillOptions.opacity = trace.opacity;
                 fillOptions.positions = pos;
+
+                return fillOptions;
             });
 
             scene.fill2d.update(scene.fillOptions);
@@ -945,7 +947,7 @@ function plot(container, subplot, cdata) {
             scene.select2d = createScatter(layout._glcanvas.data()[1].regl, {clone: scene.scatter2d});
         }
 
-        if(scene.scatter2d) {
+        if(scene.scatter2d && scene.selectBatch && scene.selectBatch.length) {
             // update only traces with selection
             scene.scatter2d.update(scene.unselectedOptions.map(function(opts, i) {
                 return scene.selectBatch[i] ? opts : null;

diff --git a/test/image/baselines/gl2d_line_select.png b/test/image/baselines/gl2d_line_select.png
diff --git a/test/image/baselines/gl2d_scatter-color-clustering.png b/test/image/baselines/gl2d_scatter-color-clustering.png
diff --git a/test/image/mocks/gl2d_line_select.json b/test/image/mocks/gl2d_line_select.json
@@ -0,0 +1,14 @@
+{
+    "data": [{
+        "type": "scattergl",
+        "mode": "lines",
+        "x": [1, 2, 3],
+        "y": [1, 2, 1]
+    }],
+    "layout": {
+        "dragmode": "select",
+        "showlegend": false,
+        "width": 400,
+        "height": 400
+    }
+}
diff --git a/test/image/mocks/gl2d_scatter-color-clustering.json b/test/image/mocks/gl2d_scatter-color-clustering.json
diff --git a/test/jasmine/tests/gl2d_plot_interact_test.js b/test/jasmine/tests/gl2d_plot_interact_test.js
@@ -16,7 +16,6 @@ var selectButton = require('../assets/modebar_button');
 var delay = require('../assets/delay');
 var readPixel = require('../assets/read_pixel');
 
-
 function countCanvases() {
     return d3.selectAll('canvas').size();
 }
@@ -648,7 +647,6 @@ describe('@gl Test gl2d plots', function() {
         .then(done);
     });
 
-
     it('should not scroll document while panning', function(done) {
         var mock = {
             data: [
@@ -773,4 +771,66 @@ describe('@gl Test gl2d plots', function() {
         .catch(fail)
         .then(done);
     });
+
+    it('should remove fill2d', function(done) {
+        var mock = require('@mocks/gl2d_axes_labels2.json');
+
+        Plotly.plot(gd, mock.data, mock.layout)
+        .then(delay(1000))
+        .then(function() {
+            expect(readPixel(gd.querySelector('.gl-canvas-context'), 100, 80)[0]).not.toBe(0);
+
+            return Plotly.restyle(gd, {fill: 'none'});
+        })
+        .then(function() {
+            expect(readPixel(gd.querySelector('.gl-canvas-context'), 100, 80)[0]).toBe(0);
+        })
+        .catch(fail)
+        .then(done);
+    });
+
+    it('should be able to draw more than 4096 colors', function(done) {
+        var x = [];
+        var color = [];
+        var N = 1e5;
+        var w = 500;
+        var h = 500;
+
+        Lib.seedPseudoRandom();
+
+        for(var i = 0; i < N; i++) {
+            x.push(i);
+            color.push(Lib.pseudoRandom());
+        }
+
+        Plotly.newPlot(gd, [{
+            type: 'scattergl',
+            mode: 'markers',
+            x: x,
+            y: color,
+            marker: {
+                color: color,
+                colorscale: [
+                    [0, 'rgb(255, 0, 0)'],
+                    [0.5, 'rgb(0, 255, 0)'],
+                    [1.0, 'rgb(0, 0, 255)']
+                ]
+            }
+        }], {
+            width: w,
+            height: h,
+            margin: {l: 0, t: 0, b: 0, r: 0}
+        })
+        .then(function() {
+            var total = readPixel(gd.querySelector('.gl-canvas-context'), 0, 0, w, h)
+                .reduce(function(acc, v) { return acc + v; }, 0);
+
+            // the total value was 3777134 before PR
+            // https://github.com/plotly/plotly.js/pull/2377
+            // and 105545275 after.
+            expect(total).toBeGreaterThan(4e6);
+        })
+        .catch(fail)
+        .then(done);
+    });
 });