Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replot when webgl buffer dims don't match canvas dims #2939

Merged
merged 4 commits into from
Aug 22, 2018
Merged

Replot when webgl buffer dims don't match canvas dims #2939

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e3d947e
replot when webgl buffer dims don't match canvas dims
etpinard Aug 22, 2018
85a646f
avoid potential infinite loop when replotting due to wrong gl dims
etpinard Aug 22, 2018
6fcec6e
replot with more granular approach
etpinard Aug 22, 2018
3879a41
use scope variable instead of fullLayout private key
etpinard Aug 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/plot_api/plot_api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. This may not be the best idea actually. If the canvas and gl buffer dimensions still don't match after the replot, we could have an infinite loop.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how that would happen, but if you're worried about it we could do something to limit to 1 redraw like

if(!gd._reglRedrawing) {
    gd._reglRedrawing = true;
    return exports.newPlot(...).then(function() { gd._reglRedrawing = false; });
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's essentially when I went for in 85a646f

}
}
}

return Plots.previousPromises(gd);
Expand Down
34 changes: 34 additions & 0 deletions test/jasmine/tests/splom_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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() {
Expand Down