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: Create WebGL2 context by default #19620

Merged
merged 4 commits into from
Jun 11, 2020
Merged

WebGLRenderer: Create WebGL2 context by default #19620

merged 4 commits into from
Jun 11, 2020

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Jun 11, 2020

Fixes #19040

@mrdoob mrdoob added this to the r118 milestone Jun 11, 2020
@mrdoob mrdoob merged commit 041ca38 into dev Jun 11, 2020
@mrdoob mrdoob deleted the webgl2 branch June 11, 2020 00:33
@fernandojsg
Copy link
Collaborator

weee 👍

@mrdoob
Copy link
Owner Author

mrdoob commented Jun 11, 2020

Hmm... I'm seeing regressions...

https://raw.githack.com/mrdoob/three.js/dev/examples/webgl_loader_gltf.html

Before:

Screen Shot 2020-06-10 at 6 44 20 PM

After:

Screen Shot 2020-06-10 at 6 45 03 PM

@mrdoob
Copy link
Owner Author

mrdoob commented Jun 11, 2020

@elalish Seems to be the RoughnessMipmapper 🤔

You can test this on model-viewer by passing WebGL2 context to the renderer:

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

@mrdoob
Copy link
Owner Author

mrdoob commented Jun 11, 2020

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 11, 2020

So if the user needs a WebGL 1 context, it's necessary to create it on application level and pass it into the renderer, right?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 11, 2020

webgl_mirror_nodes is broken with shader compilation errors.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 11, 2020

Should be fixed via #19625.

@Mugen87 Mugen87 mentioned this pull request Jun 11, 2020
@mrdoob
Copy link
Owner Author

mrdoob commented Jun 11, 2020

So if the user needs a WebGL 1 context, it's necessary to create it on application level and pass it into the renderer, right?

What would a use case be to not want WebGL 2?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 11, 2020

For example a project does not have the budget to update its shader code (see #19627) but still needs an important bug fix in a newer three.js version.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 11, 2020

Seems to be the RoughnessMipmapper 🤔

webgl_loader_gltf reports a WebGL warning now:

[.WebGL-0x7fd1ef043800]RENDER WARNING: texture bound to texture unit 0 is not renderable. It might be non-power-of-2 or have incompatible texture filtering (maybe)?

@DefinitelyMaybe

This comment has been minimized.

@Mugen87 Mugen87 mentioned this pull request Jun 12, 2020
@mrdoob
Copy link
Owner Author

mrdoob commented Jun 12, 2020

For example a project does not have the budget to update its shader code (see #19627) but still needs an important bug fix in a newer three.js version.

I think that's fair.

Here are 2 possible ways:

new THREE.WebGLRenderer( { forceWebGL1: true } );

Or make a new class that extends WebGLRenderer.

new THREE.WebGL1Renderer();

And the internal code would just do if ( scope.isWebGL1Renderer === true )...

I think I prefer the later?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 12, 2020

I'm fine with both. Pick the one you like the best 😇 .

In any event, it's good if we provide users an easy fallback just for the case they struggle with the migration.

@Mugen87 Mugen87 mentioned this pull request Jun 13, 2020
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.

WebGLRenderer: WebGL fallback from WebGL2 does not honor context attributes
4 participants