From fce8aad45abc6e57347455afe926ebdbe3218d3b Mon Sep 17 00:00:00 2001 From: archmoj Date: Sun, 21 Oct 2018 14:46:28 -0400 Subject: [PATCH 1/6] check destroy functions before execution --- src/traces/scattergl/index.js | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js index c3068932a5b..b2f16dd7d0d 100644 --- a/src/traces/scattergl/index.js +++ b/src/traces/scattergl/index.js @@ -295,13 +295,28 @@ function sceneUpdate(gd, subplot) { // remove scene resources scene.destroy = function destroy() { - if(scene.fill2d) scene.fill2d.destroy(); - if(scene.scatter2d) scene.scatter2d.destroy(); - if(scene.error2d) scene.error2d.destroy(); - if(scene.line2d) scene.line2d.destroy(); - if(scene.select2d) scene.select2d.destroy(); + if((scene.fill2d) && + (scene.fill2d.destroy)) + scene.fill2d.destroy(); + if((scene.scatter2d) && + (scene.scatter2d.destroy)) + scene.scatter2d.destroy(); + if((scene.error2d) && + (scene.error2d.destroy)) + scene.error2d.destroy(); + if((scene.line2d) && + (scene.line2d.destroy)) + scene.line2d.destroy(); + if((scene.select2d) && + (scene.select2d.destroy)) + scene.select2d.destroy(); if(scene.glText) { - scene.glText.forEach(function(text) { text.destroy(); }); + scene.glText.forEach( + function(text) { + if (text.destroy) + text.destroy(); + } + ); } scene.lineOptions = null; From 709294c608d281c0191d49b5cc053fc7c93b5082 Mon Sep 17 00:00:00 2001 From: archmoj Date: Sun, 21 Oct 2018 15:12:29 -0400 Subject: [PATCH 2/6] fix for issue 2999 --- src/traces/scattergl/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js index b2f16dd7d0d..bd7bd2c9b1a 100644 --- a/src/traces/scattergl/index.js +++ b/src/traces/scattergl/index.js @@ -313,7 +313,7 @@ function sceneUpdate(gd, subplot) { if(scene.glText) { scene.glText.forEach( function(text) { - if (text.destroy) + if(text.destroy) text.destroy(); } ); From dd51b95c1c719dcf0d44fe25fb9bddba65d3ecd6 Mon Sep 17 00:00:00 2001 From: archmoj Date: Sun, 21 Oct 2018 15:20:36 -0400 Subject: [PATCH 3/6] updated condistions --- src/traces/scattergl/index.js | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js index bd7bd2c9b1a..4f1c607fcda 100644 --- a/src/traces/scattergl/index.js +++ b/src/traces/scattergl/index.js @@ -296,25 +296,24 @@ function sceneUpdate(gd, subplot) { // remove scene resources scene.destroy = function destroy() { if((scene.fill2d) && - (scene.fill2d.destroy)) - scene.fill2d.destroy(); + (scene.fill2d.destroy)) { + scene.fill2d.destroy();} if((scene.scatter2d) && - (scene.scatter2d.destroy)) - scene.scatter2d.destroy(); + (scene.scatter2d.destroy)) { + scene.scatter2d.destroy();} if((scene.error2d) && - (scene.error2d.destroy)) - scene.error2d.destroy(); + (scene.error2d.destroy)) { + scene.error2d.destroy();} if((scene.line2d) && - (scene.line2d.destroy)) - scene.line2d.destroy(); + (scene.line2d.destroy)) { + scene.line2d.destroy();} if((scene.select2d) && - (scene.select2d.destroy)) - scene.select2d.destroy(); + (scene.select2d.destroy)) { + scene.select2d.destroy();} if(scene.glText) { scene.glText.forEach( function(text) { - if(text.destroy) - text.destroy(); + if(text.destroy) text.destroy(); } ); } From 55ede356fb3918ae3ecd0069dfe84c5c6e66da8c Mon Sep 17 00:00:00 2001 From: archmoj Date: Tue, 23 Oct 2018 16:18:50 -0400 Subject: [PATCH 4/6] modified if statements --- src/traces/scattergl/index.js | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/src/traces/scattergl/index.js b/src/traces/scattergl/index.js index 4f1c607fcda..a0826e655bf 100644 --- a/src/traces/scattergl/index.js +++ b/src/traces/scattergl/index.js @@ -295,27 +295,15 @@ function sceneUpdate(gd, subplot) { // remove scene resources scene.destroy = function destroy() { - if((scene.fill2d) && - (scene.fill2d.destroy)) { - scene.fill2d.destroy();} - if((scene.scatter2d) && - (scene.scatter2d.destroy)) { - scene.scatter2d.destroy();} - if((scene.error2d) && - (scene.error2d.destroy)) { - scene.error2d.destroy();} - if((scene.line2d) && - (scene.line2d.destroy)) { - scene.line2d.destroy();} - if((scene.select2d) && - (scene.select2d.destroy)) { - scene.select2d.destroy();} + if(scene.fill2d && scene.fill2d.destroy) scene.fill2d.destroy(); + if(scene.scatter2d && scene.scatter2d.destroy) scene.scatter2d.destroy(); + if(scene.error2d && scene.error2d.destroy) scene.error2d.destroy(); + if(scene.line2d && scene.line2d.destroy) scene.line2d.destroy(); + if(scene.select2d && scene.select2d.destroy) scene.select2d.destroy(); if(scene.glText) { - scene.glText.forEach( - function(text) { - if(text.destroy) text.destroy(); - } - ); + scene.glText.forEach(function(text) { + if(text.destroy) text.destroy(); + }); } scene.lineOptions = null; From 1c8702a7c76e9de04bb73207bcf7467fea99c26d Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 24 Oct 2018 11:36:40 -0400 Subject: [PATCH 5/6] improved test to be integer --- src/plot_api/plot_api.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index d3224a9dd12..dfc66dd9868 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -261,8 +261,8 @@ exports.plot = function(gd, data, layout, config) { if(regl) { // Unfortunately, this can happen when relayouting to large // width/height on some browsers. - if(fullLayout.width !== regl._gl.drawingBufferWidth || - fullLayout.height !== regl._gl.drawingBufferHeight + if(Math.floor(fullLayout.width) !== regl._gl.drawingBufferWidth || + Math.floor(fullLayout.height) !== regl._gl.drawingBufferHeight ) { var msg = 'WebGL context buffer and canvas dimensions do not match due to browser/WebGL bug.'; if(drawFrameworkCalls) { From 81e2dce2589afecbb914ccd0610bde7a5ec2e101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 24 Oct 2018 15:58:25 -0400 Subject: [PATCH 6/6] add test for non-integer gd width/height --- test/jasmine/tests/gl2d_plot_interact_test.js | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/test/jasmine/tests/gl2d_plot_interact_test.js b/test/jasmine/tests/gl2d_plot_interact_test.js index aa01c2a671a..e80d9606352 100644 --- a/test/jasmine/tests/gl2d_plot_interact_test.js +++ b/test/jasmine/tests/gl2d_plot_interact_test.js @@ -261,6 +261,57 @@ describe('Test gl plot side effects', function() { .catch(failTest) .then(done); }); + + it('@gl should not clear context when dimensions are not integers', function(done) { + spyOn(Plots, 'cleanPlot').and.callThrough(); + spyOn(Lib, 'log').and.callThrough(); + + var w = 500.5; + var h = 400.5; + var w0 = Math.floor(w); + var h0 = Math.floor(h); + + function assertDims(msg) { + var fullLayout = gd._fullLayout; + expect(fullLayout.width).toBe(w, msg); + expect(fullLayout.height).toBe(h, msg); + + var canvas = fullLayout._glcanvas; + expect(canvas.node().width).toBe(w0, msg); + expect(canvas.node().height).toBe(h0, msg); + + var gl = canvas.data()[0].regl._gl; + expect(gl.drawingBufferWidth).toBe(w0, msg); + expect(gl.drawingBufferHeight).toBe(h0, msg); + } + + Plotly.plot(gd, [{ + type: 'scattergl', + mode: 'lines', + y: [1, 2, 1] + }], { + width: w, + height: h + }) + .then(function() { + assertDims('base state'); + + // once from supplyDefaults + expect(Plots.cleanPlot).toHaveBeenCalledTimes(1); + expect(Lib.log).toHaveBeenCalledTimes(0); + + return Plotly.restyle(gd, 'mode', 'markers'); + }) + .then(function() { + assertDims('after restyle'); + + // one more supplyDefaults + expect(Plots.cleanPlot).toHaveBeenCalledTimes(2); + expect(Lib.log).toHaveBeenCalledTimes(0); + }) + .catch(failTest) + .then(done); + }); }); describe('Test gl2d plots', function() {