From e3d947efc60295b426332f0b971635ebb7ca7507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Aug 2018 12:37:03 -0400 Subject: [PATCH 1/4] replot when webgl buffer dims don't match canvas dims - this can happen when relayouting to large width/height on some browsers (e.g. Chrome 68) --- src/plot_api/plot_api.js | 18 +++++++++++++++++ test/jasmine/tests/splom_test.js | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index d81255aa7f2..660885f4051 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -242,6 +242,24 @@ exports.plot = function(gd, data, layout, config) { fullLayout._glcanvas .attr('width', fullLayout.width) .attr('height', fullLayout.height); + + + var regl = fullLayout._glcanvas.data()[0].regl; + if(regl) { + if( + fullLayout.width !== regl._gl.drawingBufferWidth || + fullLayout.height !== regl._gl.drawingBufferHeight + ) { + // Unfortunately, this can happen when relayouting to large + // width/height on some browsers. + Lib.log([ + 'WebGL context buffer and canvas dimensions do not match,', + 'due to browser/WebGL bug.', + 'Clearing graph and plotting again.' + ].join(' ')); + exports.newPlot(gd, gd.data, gd.layout); + } + } } return Plots.previousPromises(gd); diff --git a/test/jasmine/tests/splom_test.js b/test/jasmine/tests/splom_test.js index 6f74bd9b922..62eafa82b4a 100644 --- a/test/jasmine/tests/splom_test.js +++ b/test/jasmine/tests/splom_test.js @@ -1,6 +1,7 @@ var Plotly = require('@lib'); var Lib = require('@src/lib'); var Plots = require('@src/plots/plots'); +var plotApi = require('@src/plot_api/plot_api'); var SUBPLOT_PATTERN = require('@src/plots/cartesian/constants').SUBPLOT_PATTERN; var d3 = require('d3'); @@ -682,6 +683,39 @@ describe('Test splom interactions:', function() { .catch(failTest) .then(done); }); + + it('@gl should clear graph and replot when canvas and WebGL context dimensions do not match', function(done) { + var fig = Lib.extendDeep({}, require('@mocks/splom_iris.json')); + + function assertDims(msg, w, h) { + var canvas = gd._fullLayout._glcanvas; + expect(canvas.node().width).toBe(w, msg); + expect(canvas.node().height).toBe(h, msg); + + var gl = canvas.data()[0].regl._gl; + expect(gl.drawingBufferWidth).toBe(w, msg); + expect(gl.drawingBufferHeight).toBe(h, msg); + } + + spyOn(plotApi, 'newPlot').and.callThrough(); + spyOn(Lib, 'log'); + + Plotly.plot(gd, fig).then(function() { + expect(plotApi.newPlot).toHaveBeenCalledTimes(0); + expect(Lib.log).toHaveBeenCalledTimes(0); + assertDims('base', 600, 500); + + return Plotly.relayout(gd, {width: 4810, height: 3656}); + }) + .then(function() { + expect(plotApi.newPlot).toHaveBeenCalledTimes(1); + expect(Lib.log) + .toHaveBeenCalledWith('WebGL context buffer and canvas dimensions do not match, due to browser/WebGL bug. Clearing graph and plotting again.'); + assertDims('base', 4810, 3656); + }) + .catch(failTest) + .then(done); + }); }); describe('Test splom hover:', function() { From 85a646f374ab26b191a2c0eb9a1af470b0b72386 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Aug 2018 14:37:55 -0400 Subject: [PATCH 2/4] avoid potential infinite loop when replotting due to wrong gl dims --- src/plot_api/plot_api.js | 23 ++++++++++++----------- test/jasmine/tests/splom_test.js | 10 ++++++---- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 660885f4051..a30fc0208b7 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -243,21 +243,22 @@ exports.plot = function(gd, data, layout, config) { .attr('width', fullLayout.width) .attr('height', fullLayout.height); - var regl = fullLayout._glcanvas.data()[0].regl; if(regl) { - if( - fullLayout.width !== regl._gl.drawingBufferWidth || + // Unfortunately, this can happen when relayouting to large + // width/height on some browsers. + if(fullLayout.width !== regl._gl.drawingBufferWidth || fullLayout.height !== regl._gl.drawingBufferHeight ) { - // Unfortunately, this can happen when relayouting to large - // width/height on some browsers. - Lib.log([ - 'WebGL context buffer and canvas dimensions do not match,', - 'due to browser/WebGL bug.', - 'Clearing graph and plotting again.' - ].join(' ')); - exports.newPlot(gd, gd.data, gd.layout); + var msg = 'WebGL context buffer and canvas dimensions do not match due to browser/WebGL bug.'; + if(fullLayout._redrawFromWrongGlDimensions) { + Lib.error(msg); + } else { + Lib.log(msg + ' Clearing graph and plotting again.'); + fullLayout._redrawFromWrongGlDimensions = 1; + Plots.cleanPlot([], {}, gd._fullData, fullLayout, gd.calcdata); + exports.plot(gd, gd.data, gd.layout); + } } } } diff --git a/test/jasmine/tests/splom_test.js b/test/jasmine/tests/splom_test.js index 62eafa82b4a..c791a9447e3 100644 --- a/test/jasmine/tests/splom_test.js +++ b/test/jasmine/tests/splom_test.js @@ -697,20 +697,22 @@ describe('Test splom interactions:', function() { expect(gl.drawingBufferHeight).toBe(h, msg); } - spyOn(plotApi, 'newPlot').and.callThrough(); + spyOn(plotApi, 'plot').and.callThrough(); spyOn(Lib, 'log'); Plotly.plot(gd, fig).then(function() { - expect(plotApi.newPlot).toHaveBeenCalledTimes(0); + expect(plotApi.plot).toHaveBeenCalledTimes(0); expect(Lib.log).toHaveBeenCalledTimes(0); + expect(gd._fullLayout._redrawFromWrongGlDimensions).toBeUndefined(); assertDims('base', 600, 500); return Plotly.relayout(gd, {width: 4810, height: 3656}); }) .then(function() { - expect(plotApi.newPlot).toHaveBeenCalledTimes(1); + expect(plotApi.plot).toHaveBeenCalledTimes(1); expect(Lib.log) - .toHaveBeenCalledWith('WebGL context buffer and canvas dimensions do not match, due to browser/WebGL bug. Clearing graph and plotting again.'); + .toHaveBeenCalledWith('WebGL context buffer and canvas dimensions do not match due to browser/WebGL bug. Clearing graph and plotting again.'); + expect(gd._fullLayout._redrawFromWrongGlDimensions).toBe(1); assertDims('base', 4810, 3656); }) .catch(failTest) From 6fcec6eeea723cb5cc40fe01813760d078510fb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Aug 2018 15:35:03 -0400 Subject: [PATCH 3/4] replot with more granular approach --- src/plot_api/plot_api.js | 7 +++++-- test/jasmine/tests/splom_test.js | 31 +++++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index a30fc0208b7..ee6edd4f9c4 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -255,9 +255,12 @@ exports.plot = function(gd, data, layout, config) { Lib.error(msg); } else { Lib.log(msg + ' Clearing graph and plotting again.'); - fullLayout._redrawFromWrongGlDimensions = 1; Plots.cleanPlot([], {}, gd._fullData, fullLayout, gd.calcdata); - exports.plot(gd, gd.data, gd.layout); + Plots.supplyDefaults(gd); + fullLayout = gd._fullLayout; + Plots.doCalcdata(gd); + fullLayout._redrawFromWrongGlDimensions = 1; + return drawFramework(); } } } diff --git a/test/jasmine/tests/splom_test.js b/test/jasmine/tests/splom_test.js index c791a9447e3..f99c0362eb8 100644 --- a/test/jasmine/tests/splom_test.js +++ b/test/jasmine/tests/splom_test.js @@ -1,7 +1,6 @@ var Plotly = require('@lib'); var Lib = require('@src/lib'); var Plots = require('@src/plots/plots'); -var plotApi = require('@src/plot_api/plot_api'); var SUBPLOT_PATTERN = require('@src/plots/cartesian/constants').SUBPLOT_PATTERN; var d3 = require('d3'); @@ -697,23 +696,43 @@ describe('Test splom interactions:', function() { expect(gl.drawingBufferHeight).toBe(h, msg); } - spyOn(plotApi, 'plot').and.callThrough(); + var methods = ['cleanPlot', 'supplyDefaults', 'doCalcdata']; + + methods.forEach(function(m) { spyOn(Plots, m).and.callThrough(); }); + + function assetsFnCall(msg, exp) { + methods.forEach(function(m) { + expect(Plots[m]).toHaveBeenCalledTimes(exp[m], msg); + Plots[m].calls.reset(); + }); + } + spyOn(Lib, 'log'); Plotly.plot(gd, fig).then(function() { - expect(plotApi.plot).toHaveBeenCalledTimes(0); + assetsFnCall('base', { + cleanPlot: 1, // called once from inside Plots.supplyDefaults + supplyDefaults: 1, + doCalcdata: 1 + }); + assertDims('base', 600, 500); expect(Lib.log).toHaveBeenCalledTimes(0); expect(gd._fullLayout._redrawFromWrongGlDimensions).toBeUndefined(); - assertDims('base', 600, 500); + + spyOn(gd._fullData[0]._module, 'plot').and.callThrough(); return Plotly.relayout(gd, {width: 4810, height: 3656}); }) .then(function() { - expect(plotApi.plot).toHaveBeenCalledTimes(1); + assetsFnCall('after', { + cleanPlot: 4, // 3 three from supplyDefaults, once in drawFramework + supplyDefaults: 3, // 1 from relayout, 1 from automargin, 1 in drawFramework + doCalcdata: 1 // once in drawFramework + }); + assertDims('after', 4810, 3656); expect(Lib.log) .toHaveBeenCalledWith('WebGL context buffer and canvas dimensions do not match due to browser/WebGL bug. Clearing graph and plotting again.'); expect(gd._fullLayout._redrawFromWrongGlDimensions).toBe(1); - assertDims('base', 4810, 3656); }) .catch(failTest) .then(done); From 3879a41d94423b5fb73a7547213fc6bf4bdc1b0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 22 Aug 2018 16:15:31 -0400 Subject: [PATCH 4/4] use scope variable instead of fullLayout private key ... to avoid infinite drawFramework loops --- src/plot_api/plot_api.js | 5 +++-- test/jasmine/tests/splom_test.js | 2 -- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index ee6edd4f9c4..9804f2d50ff 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -199,6 +199,7 @@ exports.plot = function(gd, data, layout, config) { // draw framework first so that margin-pushing // components can position themselves correctly + var drawFrameworkCalls = 0; function drawFramework() { var basePlotModules = fullLayout._basePlotModules; @@ -251,7 +252,7 @@ exports.plot = function(gd, data, layout, config) { fullLayout.height !== regl._gl.drawingBufferHeight ) { var msg = 'WebGL context buffer and canvas dimensions do not match due to browser/WebGL bug.'; - if(fullLayout._redrawFromWrongGlDimensions) { + if(drawFrameworkCalls) { Lib.error(msg); } else { Lib.log(msg + ' Clearing graph and plotting again.'); @@ -259,7 +260,7 @@ exports.plot = function(gd, data, layout, config) { Plots.supplyDefaults(gd); fullLayout = gd._fullLayout; Plots.doCalcdata(gd); - fullLayout._redrawFromWrongGlDimensions = 1; + drawFrameworkCalls++; return drawFramework(); } } diff --git a/test/jasmine/tests/splom_test.js b/test/jasmine/tests/splom_test.js index f99c0362eb8..37781d39c78 100644 --- a/test/jasmine/tests/splom_test.js +++ b/test/jasmine/tests/splom_test.js @@ -717,7 +717,6 @@ describe('Test splom interactions:', function() { }); assertDims('base', 600, 500); expect(Lib.log).toHaveBeenCalledTimes(0); - expect(gd._fullLayout._redrawFromWrongGlDimensions).toBeUndefined(); spyOn(gd._fullData[0]._module, 'plot').and.callThrough(); @@ -732,7 +731,6 @@ describe('Test splom interactions:', function() { assertDims('after', 4810, 3656); expect(Lib.log) .toHaveBeenCalledWith('WebGL context buffer and canvas dimensions do not match due to browser/WebGL bug. Clearing graph and plotting again.'); - expect(gd._fullLayout._redrawFromWrongGlDimensions).toBe(1); }) .catch(failTest) .then(done);