-
-
Notifications
You must be signed in to change notification settings - Fork 35.2k
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 WebGL2.0 initial support with GLSL 1 to 3 runtime conversion #13717
Changes from 18 commits
d1d38c9
63839ee
b17356f
24e0ec4
b3af65a
b1f0db4
a04bea6
5392545
2b0dac6
cf3ff01
32cb672
2456569
6065f4f
f02117f
c1ff8fc
7a4dc58
fd7ced8
949623b
89c1738
42831e8
51dad3f
e11ed2b
ddc4893
4ec384f
b53cec2
5219c0f
3f0dfd9
062d7db
8ca782f
325d8e9
3cdf35b
dc11ded
bdbc68b
46ee5b6
ab65cb7
acee27d
f696296
d177df5
158fc6c
fda6fac
a18d1a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,7 +65,8 @@ 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', | ||
_webgl2 = parameters.webgl2 !== undefined ? parameters.webgl2 : false; | ||
|
||
var currentRenderList = null; | ||
var currentRenderState = null; | ||
|
@@ -190,11 +191,15 @@ function WebGLRenderer( parameters ) { | |
_canvas.addEventListener( 'webglcontextlost', onContextLost, false ); | ||
_canvas.addEventListener( 'webglcontextrestored', onContextRestore, false ); | ||
|
||
_gl = _context || _canvas.getContext( 'webgl', contextAttributes ) || _canvas.getContext( 'experimental-webgl', contextAttributes ); | ||
var webglVersion = _webgl2 && typeof WebGL2RenderingContext !== 'undefined' ? 'webgl2' : 'webgl'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think we should try to create WebGLContext if we fail to create WebGL2Context? (I don't) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can avoid a fallback if the user explicitly requests WebGL2 with a parameter. Compatibility can be verified on app level. Maybe we can enhance Detector in order to support the user with this task. Something like: if ( Detector.webgl2 ) // use WebGL 2 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added. Thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @Mugen87 I think that if we explicitly request WebGL2 context and it's not available, it shouldn't fallback to webgl1. Probably someone requesting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added two options. We may want autoWebgl2 as the default once Three.js + WebGL 2.0 becomes stable (See #13701 (comment)) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would just leave one option. Something like:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Combining the two options into a single one is nice. But I don't really like mix type, boolean or strings. So how about
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @takahirox yeah! I like that approach better 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||
|
||
_gl = _context || _canvas.getContext( webglVersion, contextAttributes ); | ||
|
||
if ( _gl === null && webglVersion === 'webgl' ) _gl = _canvas.getContext( 'experimental-webgl', contextAttributes ); | ||
|
||
if ( _gl === null ) { | ||
|
||
if ( _canvas.getContext( 'webgl' ) !== null ) { | ||
if ( _canvas.getContext( webglVersion ) !== null ) { | ||
|
||
throw new Error( 'Error creating WebGL context with your selected attributes.' ); | ||
|
||
|
@@ -2427,7 +2432,7 @@ function WebGLRenderer( parameters ) { | |
|
||
}; | ||
|
||
this.setRenderTarget = function ( renderTarget ) { | ||
this.setRenderTarget = function ( renderTarget, optType ) { | ||
|
||
_currentRenderTarget = renderTarget; | ||
|
||
|
@@ -2469,7 +2474,7 @@ function WebGLRenderer( parameters ) { | |
|
||
if ( _currentFramebuffer !== framebuffer ) { | ||
|
||
_gl.bindFramebuffer( _gl.FRAMEBUFFER, framebuffer ); | ||
_gl.bindFramebuffer( optType || _gl.FRAMEBUFFER, framebuffer ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking to move this The purpose of this PR is to enable existing core to run on WebGL 2.0 rather than to add WebGL2.0 features. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep I agree with that approach There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted. This change should be discussed well in another PR. |
||
_currentFramebuffer = framebuffer; | ||
|
||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ | |
|
||
function WebGLBufferRenderer( gl, extensions, info ) { | ||
|
||
var isWebGL2 = typeof WebGL2RenderingContext !== 'undefined' && gl instanceof WebGL2RenderingContext; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There're duplicated this webgl2 context check line across modules. Do you think it'd be worth to have
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you determine that when the context is instantiated and save it as a property? gl.isWebGL2 = boolean; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can if we don't mind polluting the context object. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated, Thanks! |
||
|
||
var mode; | ||
|
||
function setMode( value ) { | ||
|
@@ -37,11 +39,11 @@ function WebGLBufferRenderer( gl, extensions, info ) { | |
|
||
count = position.data.count; | ||
|
||
extension.drawArraysInstancedANGLE( mode, 0, count, geometry.maxInstancedCount ); | ||
extension[ isWebGL2 ? 'drawArraysInstanced' : 'drawArraysInstancedANGLE' ]( mode, 0, count, geometry.maxInstancedCount ); | ||
|
||
} else { | ||
|
||
extension.drawArraysInstancedANGLE( mode, start, count, geometry.maxInstancedCount ); | ||
extension[ isWebGL2 ? 'drawArraysInstanced' : 'drawArraysInstancedANGLE' ]( mode, start, count, geometry.maxInstancedCount ); | ||
|
||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,8 @@ function WebGLExtensions( gl ) { | |
|
||
var extensions = {}; | ||
|
||
var isWebGL2 = ( typeof WebGL2RenderingContext !== 'undefined' && gl instanceof WebGL2RenderingContext ); | ||
|
||
return { | ||
|
||
get: function ( name ) { | ||
|
@@ -21,7 +23,17 @@ function WebGLExtensions( gl ) { | |
switch ( name ) { | ||
|
||
case 'WEBGL_depth_texture': | ||
extension = gl.getExtension( 'WEBGL_depth_texture' ) || gl.getExtension( 'MOZ_WEBGL_depth_texture' ) || gl.getExtension( 'WEBKIT_WEBGL_depth_texture' ); | ||
|
||
if ( isWebGL2 ) { | ||
|
||
extension = gl; | ||
|
||
} else { | ||
|
||
extension = gl.getExtension( 'WEBGL_depth_texture' ) || gl.getExtension( 'MOZ_WEBGL_depth_texture' ) || gl.getExtension( 'WEBKIT_WEBGL_depth_texture' ); | ||
|
||
} | ||
|
||
break; | ||
|
||
case 'EXT_texture_filter_anisotropic': | ||
|
@@ -41,7 +53,26 @@ function WebGLExtensions( gl ) { | |
break; | ||
|
||
default: | ||
extension = gl.getExtension( name ); | ||
|
||
if ( isWebGL2 && | ||
[ 'ANGLE_instanced_arrays', | ||
'OES_texture_float', | ||
'OES_texture_half_float', | ||
'OES_texture_half_float_linear', | ||
'OES_element_index_uint', | ||
'OES_standard_derivatives', | ||
'EXT_frag_depth', | ||
'EXT_shader_texture_lod', | ||
'EXT_blend_minmax', | ||
'WEBGL_draw_buffers' ].indexOf( name ) >= 0 ) { | ||
|
||
extension = gl; | ||
|
||
} else { | ||
|
||
extension = gl.getExtension( name ); | ||
|
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Returning For example, Three.js devs would think as if WebGL 2.0 also has ANGLE_instanced_arrays extension although it's built-in feature in WebGL 2.0 with this code.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made branch, removing the workaround from dev...takahirox:WebGLRendererWebGL2.0_2 The code will be
Change will be a bit bigger, but less misleading. Personally I prefer this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that returning There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In any case I don't see it as a blocking issue, we could merge it and later decide the best way to fix this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with brushing up later. Personally I prefer starting with the second one because I like less misleading and probably we couldn't remove all There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
|
||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This property is for users who wanna write GLSL 3 shader code with
ShaderMaterial
. If this property is settrue
, we don't addto final shader code in
WebGLProgram
.Unless doing that, compile fails because of multiple
out
declaration (without specifying layout).I'm thinking better solution....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option is we let users write #version in user shader code for GLSL 3 like
and we determine GLSL 3 if the user shader code begins with "#version 300 es".