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.

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

WebGLRenderer: Fix for runtime error that happens when null is provided as context #23969

Merged
merged 1 commit into from
Apr 30, 2022

Conversation

soadzoor
Copy link
Contributor

@soadzoor soadzoor commented Apr 29, 2022

Description

If someone passes a context to the WebGLRenderer, but the context is null (because the user's device doesn't support webgl, or webgl2, or whatever context they tried to create), we have a runtime error, saying TypeError: _context is null.

The problem was introduced with this commit: fb9d575

To make things a little bit more confusing, the official documentation says that the default value for context is null: https://threejs.org/docs/index.html?q=webg#api/en/renderers/WebGLRenderer

Which is, of course, technically incorrect. It's not null, but undefined by default. The root of the problems is that some of the checks inside the WebGLRenderer's "constructor" is checking whether the context is undefined. If it's not, it assumes it has a valid value, although it can be easily null. Because canvas.getContext either returns a context, or null.

For a long time this was the official recommended way for creating WebGLRenderer with WebGL 2.0 context:

var context = canvas.getContext( 'webgl2' );
var renderer = new THREE.WebGLRenderer( { canvas: canvas, context: context } );

I know this can be now considered "legacy code", but still, if webgl2 is not available on a device, and someone still uses the code like this, it will give the user an error instead of trying to fall back to webgl 1.

This PR replaces the strict comparisons context !== undefined with context != undefined, and provides a technically more accurate default value in the documentation.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 29, 2022

To make things a little bit more confusing, the official documentation says that the default value for context is null

I think the docs are right. The line _context = parameters.context !== undefined ? parameters.context : null, is in some sense a replacement for the default parameter syntax.

The better change is replacing the following line:

if ( parameters.context !== undefined ) {

with

if ( _context !== null ) {

BTW: We do not use != in general. Better to make the check more explicit.

@soadzoor soadzoor force-pushed the webgl-2-fallback branch 2 times, most recently from 64d880e to b154c2d Compare April 29, 2022 14:47
@soadzoor
Copy link
Contributor Author

@Mugen87 Alright, I'm fine with that, I modified the PR according to it

@soadzoor soadzoor changed the title Fix for runtime error that happens when null is provided as context WebGLRenderer: Fix for runtime error that happens when null is provided as context Apr 29, 2022
@WestLangley
Copy link
Collaborator

BTW: We do not use != in general.

Agreed. It would be a good idea to grep for those and fix them. 😇

Also, ==.

@Mugen87 Mugen87 added this to the r140 milestone Apr 29, 2022
@mrdoob mrdoob merged commit 253d441 into mrdoob:dev Apr 30, 2022
@mrdoob
Copy link
Owner

mrdoob commented Apr 30, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants