Support WebGL2 and GLSL 3.00 in the scheme suggested by Mr Doob #13701

yoshikiohshima · 2018-03-27T17:31:09Z

Another attempt to support WebGL2RenderingContext and GLSL 3.00. This is follow up of #13692.

WebGLRenderer.js parameter may specify to call getContext() with argument 'webgl2'. Without the argument, the behavior is identical as before and compatible with existing code.
When webgl2 is specified, there is an option to use GLSL 3.00 by setting needsGLSL300 for material
objects.
Multiple Render Target can be specified from the client code.

Implementation:
The share code in ShaderChunk and ShaderLib is written with "in/out" instead of "varying/attribute".
There is a function called convertToGLSL1() that takes such code snippets and replace in/out to varying/attribute (and does a few other things.)

extensions natively supported by WebGL2RenderingContext are handled.
(For our project, we have a GPGPU framework that uses GLSL3.00 features such as texelFetch and new buffer types such as R32F. This patch allows integrating the framework with Three.js. It's been working well for our purpose, and hopefully it is useful for others who are trying a similar stuff.)

fernandojsg · 2018-03-27T18:39:45Z

I would still prefer to go for the approach discussed on #12250. As currently the WebGL(1)Renderer is the stable one, and WebGL2Renderer will be unstable for a while, I think it's better to keep untouched everything we can on WebGLRenderer and move the conversion to the new one.

mrdoob · 2018-03-27T18:47:17Z

@fernandojsg I'm kind of curious to see how much we can simplify this PR.

yoshikiohshima · 2018-03-27T18:49:32Z

The scheme implemented here is in line with "Include some kind of postprocessing of the whole shader like https://github.com/Jam3/glsl-100-to-300 so this could help also converting users's WebGL1 shaders.", except that the post processing goes from 3.00 to 1.00. I wondered a bit why the suggestion was to go from 3.00 to 1.00 but good news there is that the migration path is smoother, as only shader code we took time to rewrite will have to deal with 3.00 and nothing else can stay at 1.00.

mrdoob · 2018-03-27T18:50:11Z

src/renderers/WebGLRenderer.js

@@ -65,7 +65,9 @@ function WebGLRenderer( parameters ) {
 		_antialias = parameters.antialias !== undefined ? parameters.antialias : false,
 		_premultipliedAlpha = parameters.premultipliedAlpha !== undefined ? parameters.premultipliedAlpha : true,
 		_preserveDrawingBuffer = parameters.preserveDrawingBuffer !== undefined ? parameters.preserveDrawingBuffer : false,
-		_powerPreference = parameters.powerPreference !== undefined ? parameters.powerPreference : 'default';
+		_powerPreference = parameters.powerPreference !== undefined ? parameters.powerPreference : 'default',
+		_webglversion = parameters.webgl2 !== undefined  ? 'webgl2' : 'webgl';


Instead of adding a new parameter, can we try doing this for now?

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

Then, inside the renderer we can do:

var isWebGL2 = _context instanceof WebGL2RenderingContext;

Passing a context created outside does work already. And you don't have to passing canvas even. a canonical check for isWebGL2 at one place presumably at WebGLRenderer would be good. (How do we proceed? Would you prefer a fresh pull request or it is okay for me to push a new commit to this branch and you pull them? Or this is close enough to be pulled, and we can fix issues over time?)

How do we proceed? Would you prefer a fresh pull request or it is okay for me to push a new commit to this branch and you pull them? Or this is close enough to be pulled, and we can fix issues over time?

Yeah, push changes to this branch.

I'm trying to see what's the minimum amount of changes needed in the current code to make this work.

And I now remember why those isWebGL2 tests are not as simple as (_context instanceof WebGL2RenderingContext): it did not work on browsers that don't support WebGL2. And also the renderer could potentially have a method called isWebGL2 or such, but the other classes such as WebGLExtensions.js, WebGLTextures.js, etc. only receives the actual context object and they still have to check it anyway. (And actually, it'd be very helpful if you agree to keep webgl2 parameter to the WebGLRenderer. It won't get in the way of others, right?)

I pushed a commit with the depth_frag error and just a cosmetic change to make the isWebGL2 check the same everywhere.

mrdoob · 2018-03-27T19:04:51Z

src/renderers/shaders/ShaderLib/depth_frag.glsl

@@ -32,11 +34,11 @@ void main() {

 	#if DEPTH_PACKING == 3200

-		gl_FragColor = vec4( vec3( 1.0 - gl_FragCoord.z ), opacity );


Is the 1.0 - removal intentional?

Wow. Definitely not. Thanks for spotting.

fernandojsg · 2018-03-27T19:08:01Z

@mrdoob so with the current approach on this PR we'll have webgl1 and webgl2 on the main renderer. Is that the desired approach instead of having two different renderers?

takahirox · 2018-03-28T09:43:21Z

@mrdoob

I'm kind of curious to see how much we can simplify this PR.

I think the change will be smaller if we convert from GLSL 1.0 to GLSL 3.0.

WIP repo dev...takahirox:WebGLRendererWebGL2.0

yoshikiohshima · 2018-03-28T16:24:17Z

Ahh, using #define. It just did not occurred to me to use that!

As you can see, the only changes to support WebGL2 features so far are setRenderTarget and (partial combinations of) some other types for the frame buffer. (Just those were what we needed for our project.) For that it was good not to have to use WebGL2Renderer just for that. As we keep adding those "little changes" to support more WebGL2 features, there may be a point where it might look complicated. But then, it is going to be not only WebGLRenderer.js but many other files such as WebGLTextures.

…tensions

takahirox · 2018-03-28T17:56:33Z

I opened PR of GLSL 1 to 3 conversion approach #13717
You can compare and comment.

Mugen87 · 2018-03-28T18:10:17Z

I agree with @bhouston. It would be great if three.js auto-detects WebGL2 and uses it, if possible. From my point of view, this should be the first milestone of the WebGL2Renderer project. So all existing features of three.js should work with WebGL1 and WebGL2. If this is done, we can think about WebGL2 exclusive stuff...

TBH, I'm not a fan of a separate WebGL2Renderer. It's just my personal opinion but i believe this approach will produce more maintenance effort than a consolidated solution. And i think it's more consistent and transparent for users to work with just a single WebGLRenderer that supports both versions.

fernandojsg · 2018-03-28T18:55:40Z

Ok so we are all on the same boat for WebGLRenderer to detect webgl2 and use its context and converting the shaders as needed (it will just depend on the direction :D)
@Mugen87 having a separate renderer I totally agree with you too that it will introduce more maintenance effort than having just one renderer. I'm just afraid of the current state of the WebGLRenderer as I believe it's not modular enough to do all the changes without messing it a lot. But following that idea one approach could be to just use one renderer and force us to modularize things as new features arrive that could modify a lot the current flow (eg: UBOs).

takahirox · 2018-03-28T19:12:07Z

About WebGL2Renderer, I came to think it'd be good to considering WebGL2Renderer option when we realize WebGLRenderer becomes messy for WebGL 2.0. We don't need to presuppose renderer separation yet.

I (maybe) almost have done initial WebGL2.0 support in #13717 but it doesn't look too messy yet. And I suppose even if we add all WebGL 2.0 features it won't be too messy. Personally I wanna keep considering both modulability and WebGL2.0 into WebGLRenderer in parallel.

And probably we might wanna think WebGL2Renderer option if we attempt to optimize WebGL 2.0 flow for the performance. So we should discuss it with the performance improvement number, it'd be good enough to start to think about it after we explore the WebGL 2.0 features performance benefit.

About auto WebGL 2.0 detection as the default, it'd be nice once Three.js + WebGL 2.0 is tested well and becomes stable. So doing now is a bit too early. Maybe r93 (or later) would be good?

yoshikiohshima · 2018-03-28T19:12:40Z

From what I have seen here, I have a sense that detecting webgl2 and use it when possible is not in mr doob’s mind. It’d be rather to use webgl2 when specified explicitly

bhouston · 2018-03-28T19:35:48Z

From what I have seen here, I have a sense that detecting webgl2 and use it when possible is not in mr doob’s mind. It’d be rather to use webgl2 when specified explicitly

How about it is part of the configuration option passed into WebGLRenderer.... e.g. new WebGLRenderer( { webgl2support: true } ); Or something like that. And then it becomes the default.

So for now it is default off but once our WebGL 2 support is fully baked and good, we make it the default. :)

yoshikiohshima · 2018-03-28T19:50:15Z

Passing in a flag to use WebGL2 is in this pull request, in case you missed it.

fernandojsg · 2018-03-28T19:51:12Z

Is there anything I'm missing on the advantages to convert from glsl3 to 1 instead of the opposite?
/cc @yoshikiohshima

yoshikiohshima · 2018-03-28T19:56:11Z

It depending how the future turns out, but automatic fallback means that the same application running in different environment may use different versions of context. It might be okay, or it might be desirable: but it could be pretty bad for developers. The difference is bigger than a few extensions.

takahirox · 2018-03-28T20:20:34Z

As I wrote in #13717 I still prefer GLSL 1 to 3 conversion because

Less changes because we don't need to change src/renderer/shaders
Keep Three.js + WebGL 1.0 stable

For us, WebGL 2.0 support is still unstable and experimental. So I don't think it's a good idea to affect Three.js + WebGL 1.0 stability by changing shader code (even if we convert to GLSL1 runtime).

yoshikiohshima · 2018-03-29T03:33:17Z

Just to get a closure, and to do what is supposed to happen, I merged @takahirox 's changes (which was a merge of his and mine) into this pull request. I believe the end result is identical but this one has a bit longer commit history. (And yes, it does work with our project out of the box: https://gist.github.com/yoshikiohshima/d25fb351cb226aa9aa1c6a62c553d43b So that is great for us)

takahirox · 2018-03-29T05:17:28Z

Well, I didn't want you to remove GLSL 3 to 1 conversion from this PR. We no longer be able to compare now between "3 to 1" and "1 to 3".

DavidPeicho · 2018-04-03T22:22:10Z

I worked on it too a few days ago: https://github.com/DavidPeicho/three.js
I added new types and formats as well as 3D textures support (I really needed this), and a sample used to check that 3D textures really work.
I didn't go as far as you for the GLSL migration, as I am only using RawShaderMaterial, but maybe it would be nice if I take a look to mix our code?

@mrdoob maybe you don't want to bloat the WebGLRenderer 1 with new features yet?

yoshikiohshima · 2018-04-04T17:49:56Z

@DavidPeicho I don't know how @mrdoob and people decide what to do, but as @takahirox has made more changes, his PR would carry more weight? If that one gets merged, I'd say that you would make a PR that is meant to go on top of it. In my mind, too many ideas in one PR make it harder to understand so mixing your changes and this one may not what I would suggest.

(And, as long as we get a better three.js in the end, I should not complain that the idea and code here was simply cannibalized without giving me github karma...)

DavidPeicho · 2018-04-04T19:10:46Z

Yes, obviously the proposed PR is a lot more robust than mine!
If this one is approved I will simply rebase my changes on top of it.
At first, I thought WebGLRenderer would never support WebGL2 that's why I planned to do it on my own. But apparently we can merge our force together 👍

mrdoob · 2018-05-22T22:01:49Z

Should we close this in favor of #13717?

yoshikiohshima · 2018-05-23T16:15:39Z

@mrdoob Last time I checked, our software did not work with #13717 out of the box but hopefully that is something we can fix on our end with #13717. I'll check it soon.

mrdoob · 2018-05-23T19:57:46Z

@yoshikiohshima okay! let us know.

mrdoob · 2018-07-12T15:29:00Z

Closing in favour of #13717.

webgl2 patch in convertToGLSL1() scheme

c497726

yoshikiohshima mentioned this pull request Mar 27, 2018

Support webgl2 and GLSL 3.00 #13692

Closed

mrdoob reviewed Mar 27, 2018

View reviewed changes

mrdoob added this to the r92 milestone Mar 27, 2018

mrdoob reviewed Mar 27, 2018

View reviewed changes

yoshikiohshima and others added 6 commits March 27, 2018 12:52

fix accidental edit

4706b5d

make all isWebGL2 test the same

4adf688

WebGLRenderer WebGL2.0 basic support

d1d38c9

Minor clean up

63839ee

Add internalFormat conversion function to WebGLTextures

b17356f

Update setupFrameBufferTexture()

24e0ec4

takahirox added 6 commits March 28, 2018 21:31

Skip GLSL conversion if material is RawShaderMaterial

b3af65a

Ignore EXT_frag_depth for WebGL 2.0

b1f0db4

Ignore EXT_shader_texture_lod for WebGL 2.0

a04bea6

Update instancing for WebGL 2.0

5392545

Clean up WebGLUtils

2b0dac6

Clean up WebGLProgram

cf3ff01

takahirox added 7 commits March 29, 2018 01:25

Add .isGLSL3 to ShaderMaterial

32cb672

Clean up WebGLTextures

2456569

Update EXT_blend_minmax handling for WebGL 2.0

6065f4f

Update WebGLUtils for WebGL 2.0 UNSIGNED_INT_24_8

f02117f

Update internalFormat for WebGL2.0

c1ff8fc

Add WebGL_draw_buffers as built-in extensionn for WebGL2.0 to WebGLEx…

7a4dc58

…tensions

Clean up WebGLProgram

fd7ced8

Add webgl2 option to WebGLRenderer

949623b

merge takahirox:WebGLRendererWebGL2.0

5aac4d0

mrdoob modified the milestones: r92, r93 Apr 18, 2018

mrdoob modified the milestones: r93, r94 May 23, 2018

mrdoob modified the milestones: r94, r95 Jun 27, 2018

mrdoob closed this Jul 12, 2018

mrdoob removed this from the r95 milestone Jul 12, 2018

takahirox added the WebGL2 label Sep 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support WebGL2 and GLSL 3.00 in the scheme suggested by Mr Doob #13701

Support WebGL2 and GLSL 3.00 in the scheme suggested by Mr Doob #13701

yoshikiohshima commented Mar 27, 2018

fernandojsg commented Mar 27, 2018

mrdoob commented Mar 27, 2018 •

edited

Loading

yoshikiohshima commented Mar 27, 2018

mrdoob Mar 27, 2018 •

edited

Loading

yoshikiohshima Mar 27, 2018

mrdoob Mar 27, 2018

yoshikiohshima Mar 27, 2018

mrdoob Mar 27, 2018

yoshikiohshima Mar 27, 2018

fernandojsg commented Mar 27, 2018

takahirox commented Mar 28, 2018

yoshikiohshima commented Mar 28, 2018

takahirox commented Mar 28, 2018 •

edited

Loading

Mugen87 commented Mar 28, 2018 •

edited

Loading

fernandojsg commented Mar 28, 2018

takahirox commented Mar 28, 2018 •

edited

Loading

yoshikiohshima commented Mar 28, 2018

bhouston commented Mar 28, 2018

yoshikiohshima commented Mar 28, 2018

fernandojsg commented Mar 28, 2018

yoshikiohshima commented Mar 28, 2018

takahirox commented Mar 28, 2018 •

edited

Loading

yoshikiohshima commented Mar 29, 2018

takahirox commented Mar 29, 2018 •

edited

Loading

DavidPeicho commented Apr 3, 2018 •

edited

Loading

yoshikiohshima commented Apr 4, 2018

DavidPeicho commented Apr 4, 2018

mrdoob commented May 22, 2018

yoshikiohshima commented May 23, 2018

mrdoob commented May 23, 2018

mrdoob commented Jul 12, 2018

		@@ -32,11 +34,11 @@ void main() {

		#if DEPTH_PACKING == 3200

		gl_FragColor = vec4( vec3( 1.0 - gl_FragCoord.z ), opacity );

Support WebGL2 and GLSL 3.00 in the scheme suggested by Mr Doob #13701

Support WebGL2 and GLSL 3.00 in the scheme suggested by Mr Doob #13701

Conversation

yoshikiohshima commented Mar 27, 2018

fernandojsg commented Mar 27, 2018

mrdoob commented Mar 27, 2018 • edited Loading

yoshikiohshima commented Mar 27, 2018

mrdoob Mar 27, 2018 • edited Loading

Choose a reason for hiding this comment

yoshikiohshima Mar 27, 2018

Choose a reason for hiding this comment

mrdoob Mar 27, 2018

Choose a reason for hiding this comment

yoshikiohshima Mar 27, 2018

Choose a reason for hiding this comment

mrdoob Mar 27, 2018

Choose a reason for hiding this comment

yoshikiohshima Mar 27, 2018

Choose a reason for hiding this comment

fernandojsg commented Mar 27, 2018

takahirox commented Mar 28, 2018

yoshikiohshima commented Mar 28, 2018

takahirox commented Mar 28, 2018 • edited Loading

Mugen87 commented Mar 28, 2018 • edited Loading

fernandojsg commented Mar 28, 2018

takahirox commented Mar 28, 2018 • edited Loading

yoshikiohshima commented Mar 28, 2018

bhouston commented Mar 28, 2018

yoshikiohshima commented Mar 28, 2018

fernandojsg commented Mar 28, 2018

yoshikiohshima commented Mar 28, 2018

takahirox commented Mar 28, 2018 • edited Loading

yoshikiohshima commented Mar 29, 2018

takahirox commented Mar 29, 2018 • edited Loading

DavidPeicho commented Apr 3, 2018 • edited Loading

yoshikiohshima commented Apr 4, 2018

DavidPeicho commented Apr 4, 2018

mrdoob commented May 22, 2018

yoshikiohshima commented May 23, 2018

mrdoob commented May 23, 2018

mrdoob commented Jul 12, 2018

mrdoob commented Mar 27, 2018 •

edited

Loading

mrdoob Mar 27, 2018 •

edited

Loading

takahirox commented Mar 28, 2018 •

edited

Loading

Mugen87 commented Mar 28, 2018 •

edited

Loading

takahirox commented Mar 28, 2018 •

edited

Loading

takahirox commented Mar 28, 2018 •

edited

Loading

takahirox commented Mar 29, 2018 •

edited

Loading

DavidPeicho commented Apr 3, 2018 •

edited

Loading