Skip to content

Make all gl traces show "no webgl message" and fail gracefully #2697

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.

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

Merged
merged 6 commits into from
Jun 6, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
32 changes: 23 additions & 9 deletions src/lib/prepare_regl.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

'use strict';

var showNoWebGlMsg = require('./show_no_webgl_msg');

// Note that this module should be ONLY required into
// files corresponding to regl trace modules
// so that bundles with non-regl only don't include
Expand All @@ -21,23 +23,35 @@ var createRegl = require('regl');
*
* @param {DOM node or object} gd : graph div object
* @param {array} extensions : list of extension to pass to createRegl
*
* @return {boolean} true if all createRegl calls succeeded, false otherwise
*/
module.exports = function prepareRegl(gd, extensions) {
var fullLayout = gd._fullLayout;
var success = true;

fullLayout._glcanvas.each(function(d) {
if(d.regl) return;
// only parcoords needs pick layer
if(d.pick && !fullLayout._has('parcoords')) return;

d.regl = createRegl({
canvas: this,
attributes: {
antialias: !d.pick,
preserveDrawingBuffer: true
},
pixelRatio: gd._context.plotGlPixelRatio || global.devicePixelRatio,
extensions: extensions || []
});
try {
d.regl = createRegl({
canvas: this,
attributes: {
antialias: !d.pick,
preserveDrawingBuffer: true
},
pixelRatio: gd._context.plotGlPixelRatio || global.devicePixelRatio,
extensions: extensions || []
});
} catch(e) {
success = false;
}
});

if(!success) {
showNoWebGlMsg({container: fullLayout._glcontainer.node()});
}
return success;
};
9 changes: 7 additions & 2 deletions src/lib/show_no_webgl_msg.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var noop = function() {};
* Expects 'scene' to have property 'container'
*
*/
module.exports = function showWebGlMsg(scene) {
module.exports = function showNoWebGlMsg(scene) {
for(var prop in scene) {
if(typeof scene[prop] === 'function') scene[prop] = noop;
}
Expand All @@ -31,10 +31,15 @@ module.exports = function showWebGlMsg(scene) {
};

var div = document.createElement('div');
div.textContent = 'Webgl is not supported by your browser - visit https://get.webgl.org for more info';
div.className = 'no-webgl';
div.textContent = 'WebGL is not supported by your browser - visit https://get.webgl.org for more info';
div.style.cursor = 'pointer';
div.style.fontSize = '24px';
div.style.color = Color.defaults[0];
div.style.position = 'absolute';
div.style.left = '20px';
div.style.top = '50%';
div.style.width = div.style.height = '100%';

scene.container.appendChild(div);
scene.container.style.background = '#FFFFFF';
Expand Down
7 changes: 6 additions & 1 deletion src/plots/gl2d/scene2d.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ function Scene2D(options, fullLayout) {
this.updateRefs(fullLayout);

this.makeFramework();
if(this.stopped) return;

// update options
this.glplotOptions = createOptions(this);
Expand Down Expand Up @@ -121,7 +122,11 @@ proto.makeFramework = function() {
premultipliedAlpha: true
});

if(!gl) showNoWebGlMsg(this);
if(!gl) {
showNoWebGlMsg(this);
this.stopped = true;
return;
}

this.canvas = liveCanvas;
this.gl = gl;
Expand Down
2 changes: 1 addition & 1 deletion src/plots/gl3d/scene.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ function initializeGLPlot(scene, fullLayout, canvas, gl) {
* The destroy method - which will remove the container from the DOM
* is overridden with a function that removes the container only.
*/
showNoWebGlMsg(scene);
return showNoWebGlMsg(scene);
}

var relayoutCallback = function(scene) {
Expand Down
1 change: 1 addition & 0 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,7 @@ plots.cleanPlot = function(newFullData, newFullLayout, oldFullData, oldFullLayou
if(hadGl && !hasGl) {
if(oldFullLayout._glcontainer !== undefined) {
oldFullLayout._glcontainer.selectAll('.gl-canvas').remove();
oldFullLayout._glcontainer.selectAll('.no-webgl').remove();
oldFullLayout._glcanvas = null;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/traces/parcoords/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ module.exports = function plot(gd, cdparcoords) {
var root = fullLayout._paperdiv;
var container = fullLayout._glcontainer;

prepareRegl(gd);
var success = prepareRegl(gd);
if(!success) return;

var gdDimensions = {};
var gdDimensionsOriginalOrder = {};
Expand Down
10 changes: 9 additions & 1 deletion src/traces/scattergl/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,15 @@ function plot(gd, subplot, cdata) {
var width = fullLayout.width;
var height = fullLayout.height;

prepareRegl(gd, ['ANGLE_instanced_arrays', 'OES_element_index_uint']);
var success = prepareRegl(gd, ['ANGLE_instanced_arrays', 'OES_element_index_uint']);
if(!success) {
scene.error2d = false;
scene.line2d = false;
scene.scatter2d = false;
scene.fill2d = false;
Copy link
Collaborator

@alexcjohnson alexcjohnson Jun 5, 2018

Choose a reason for hiding this comment

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

what happens if we add some other component to support a new feature, would we also have to add it here? Will the tests below fail and force that, or would we need to add a new test for the new feature? Just wondering if there's some way we can preempt these concerns like

if(!success) {
  for(var key in scene) {
    if(something about key or scene[key]) scene[key] = 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.

all DRY'ed up in eb594d1

return;
}

var regl = fullLayout._glcanvas.data()[0].regl;

// that is needed for fills
Expand Down
8 changes: 6 additions & 2 deletions src/traces/splom/base_plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ function plot(gd) {
var _module = Registry.getModule(SPLOM);
var splomCalcData = getModuleCalcData(gd.calcdata, _module)[0];

prepareRegl(gd, ['ANGLE_instanced_arrays', 'OES_element_index_uint']);
var success = prepareRegl(gd, ['ANGLE_instanced_arrays', 'OES_element_index_uint']);
if(!success) return;

if(fullLayout._hasOnlyLargeSploms) {
drawGrid(gd);
Expand Down Expand Up @@ -209,7 +210,10 @@ function clean(newFullData, newFullLayout, oldFullData, oldFullLayout, oldCalcda
var trace = cd0.trace;
var scene = cd0.t._scene;

if(trace.type === 'splom' && scene && scene.matrix) {
if(
trace.type === 'splom' &&
scene && scene.matrix && scene.matrix.destroy
) {
scene.matrix.destroy();
cd0.t._scene = null;
}
Expand Down
105 changes: 105 additions & 0 deletions test/jasmine/bundle_tests/no_webgl_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
var Plotly = require('@lib');

var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var failTest = require('../assets/fail_test');

describe('Plotly w/o WebGL support:', function() {
var gd;

beforeEach(function() {
gd = createGraphDiv();
});

afterEach(function() {
Plotly.purge(gd);
destroyGraphDiv();
});

function checkNoWebGLMsg(visible) {
var msg = gd.querySelector('div.no-webgl');
if(visible) {
expect(msg.innerHTML.substr(0, 22)).toBe('WebGL is not supported');
} else {
expect(msg).toBe(null);
}
}

it('gl3d subplots', function(done) {
Plotly.react(gd, require('@mocks/gl3d_autocolorscale.json'))
.then(function() {
checkNoWebGLMsg(true);
return Plotly.react(gd, require('@mocks/10.json'));
})
.then(function() {
checkNoWebGLMsg(false);
})
.catch(failTest)
.then(done);
});

it('gl2d subplots', function(done) {
Plotly.react(gd, require('@mocks/gl2d_pointcloud-basic.json'))
.then(function() {
checkNoWebGLMsg(true);
return Plotly.react(gd, require('@mocks/10.json'));
})
.then(function() {
checkNoWebGLMsg(false);
})
.catch(failTest)
.then(done);
});

it('scattergl subplots', function(done) {
Plotly.react(gd, require('@mocks/gl2d_12.json'))
.then(function() {
checkNoWebGLMsg(true);
return Plotly.react(gd, require('@mocks/10.json'));
})
.then(function() {
checkNoWebGLMsg(false);
})
.catch(failTest)
.then(done);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this gives:

peek 2018-06-05 17-26

Copy link
Contributor Author

Choose a reason for hiding this comment

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

new stylez

peek 2018-06-06 15-10

});

it('scatterpolargl subplots', function(done) {
Plotly.react(gd, require('@mocks/glpolar_scatter.json'))
.then(function() {
checkNoWebGLMsg(true);
return Plotly.react(gd, require('@mocks/10.json'));
})
.then(function() {
checkNoWebGLMsg(false);
})
.catch(failTest)
.then(done);
});

it('splom subplots', function(done) {
Plotly.react(gd, require('@mocks/splom_0.json'))
.then(function() {
checkNoWebGLMsg(true);
return Plotly.react(gd, require('@mocks/10.json'));
})
.then(function() {
checkNoWebGLMsg(false);
})
.catch(failTest)
.then(done);
});

it('parcoords subplots', function(done) {
Plotly.react(gd, require('@mocks/gl2d_parcoords_2.json'))
.then(function() {
checkNoWebGLMsg(true);
return Plotly.react(gd, require('@mocks/10.json'));
})
.then(function() {
checkNoWebGLMsg(false);
})
.catch(failTest)
.then(done);
});
});
3 changes: 2 additions & 1 deletion test/jasmine/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ func.defaultConfig = {
flags: [
'--touch-events',
'--window-size=' + argv.width + ',' + argv.height,
isCI ? '--ignore-gpu-blacklist' : ''
isCI ? '--ignore-gpu-blacklist' : '',
(isBundleTest && basename(testFileGlob) === 'no_webgl') ? '--disable-webgl' : ''
]
},
_Firefox: {
Expand Down