Fixed INVALID_OPERATION error with WebGL when using linewidth on IE11 #9861

Merged
merged 3 commits into from Dec 15, 2016

Projects

None yet

3 participants

@Ludobaka
Contributor
Ludobaka commented Oct 14, 2016 edited

When using linewidth attribute of THREE.Material on IE11, console throws a INVALID_OPERATION error. It is due to IE11 using WebGL 0.93, which is a preliminary version of WebGL.

To fix this, I propose to add a flag in WebGLState checking that WebGL version is greater or equals to 1.0. Then the flag is used before calling gl.lineWidth().

This fix makes easy to manage ThreeJS apps when wanting a compatibility with IE11. Otherwise, you have to check yourself when creating a material with linewidth attribute and avoid to set this parameter when OpenGL Version < 1.0.

Note :

@mrdoob
Owner
mrdoob commented Oct 14, 2016

@kenrussell does this sound like the right approach to handle this?

@kenrussell

@mrdoob one comment on this, though the overall approach seems OK.

src/renderers/webgl/WebGLState.js
@@ -336,6 +336,8 @@ function WebGLState( gl, extensions, paramThreeToGL ) {
var maxTextures = gl.getParameter( gl.MAX_TEXTURE_IMAGE_UNITS );
+ var lineWidthAvailable = parseFloat(gl.getParameter(gl.VERSION).split(' ')[1]) >= 1.0;
@kenrussell
kenrussell Oct 19, 2016

I think you should make this code more robust to maintenance releases. Future WebGL releases might be numbered 2.0.1, for example.

@kenrussell

Nice. Looks good to me.

@Ludobaka
Contributor
Ludobaka commented Nov 17, 2016 edited

Ping for merging branch ?
Last comment has been answered and the fix has no impact on ThreeJS except for computers running with IE11. I need this one for r83 :)

Thx.

@kenrussell

This still seems reasonable to me for what it's worth.

Some sort of user-agent detection is needed to handle this case cleanly; this seems as good as any.

@mrdoob
Owner
mrdoob commented Nov 17, 2016

This fix makes easy to manage ThreeJS apps when wanting a compatibility with IE11

I know it's always better to "feature check" but, in this case... Wouldn't it be simpler to just check for IE11? Is there any other platform that has this issue?

@Ludobaka
Contributor
Ludobaka commented Nov 18, 2016 edited

I thought it was better to feature check with WebGL version because :

  • It's sometimes tricky to check browser version, and it also need more code to perform it
  • I've encountered this problem with IE11, but it may occur also on old version of other browsers where WebGL is used as a draft. I tried to find a list of browsers version with corresponding WebGL version, but without success for now.
  • It is easier to just have a look of WebGL specifications, and do the feature check depending on the version. As @kenrussell said, WebGL may have minor releases with version 2, so each may include new features we want to check.

If you want, I can make a browser check, but it seems a little bit less efficient and consistent.

@mrdoob
Owner
mrdoob commented Nov 19, 2016 edited

If you want, I can make a browser check, but it seems a little bit less efficient and consistent.

Yeah, I think I would prefer doing a browser check for now. At least until we find another case were that fails. That way we have a better understanding of where it fails.

@Ludobaka
Contributor

Modification is ready to push, but I prefer to discuss about it before. I followed the Microsoft Documentation using userAgent.

However, there is an important warning about the detection :
"As a result, there's no guarantee that the user-agent string accurately reflects the browser being used."

I've also seen that there is minor versions of WebGL 0.9 in the wild. So we have to be sure the problem may not come from somewhere else (it could with early versions of Edge for instance).

So this is my last argument for keeping the gl.VERSION detection, if we keep it as it, I will add a comment to explain the problem in WebGLState source to make it meaningful.

src/renderers/webgl/WebGLState.js
@@ -724,7 +728,7 @@ function WebGLState( gl, extensions, paramThreeToGL ) {
function setLineWidth( width ) {
- if ( width !== currentLineWidth ) {
+ if ( lineWidthAvailable && width !== currentLineWidth ) {
gl.lineWidth( width );
@mrdoob
mrdoob Dec 14, 2016 Owner

How about we move the check here?

if ( lineWidthAvailable ) gl.lineWidth( width );
@Ludobaka
Ludobaka Dec 14, 2016 Contributor

More meaningful, I agree ;)

@@ -336,6 +336,10 @@ function WebGLState( gl, extensions, paramThreeToGL ) {
var maxTextures = gl.getParameter( gl.MAX_TEXTURE_IMAGE_UNITS );
+ var glVersion = gl.getParameter(gl.VERSION).split(' ')[1];
@mrdoob
mrdoob Dec 14, 2016 Owner

Also, how about...

var glVersion = parseFloat( /^WebGL\ ([0-9])/.exec( gl.getParameter( gl.VERSION) )[ 1 ] );
var lineWidthAvailable = parseFloat( glVersion ) >= 1.0;
@Ludobaka
Ludobaka Dec 14, 2016 Contributor

As said before, WebGL future version might be tagged like "WebGL 2.0.3", I will just modify the regex a bit and that would work fine ;)

@Ludobaka
Ludobaka Dec 14, 2016 Contributor

I understand that you are simplifying the regex, do you prefer yours or a more detailed one like :

var glVersion = parseFloat( /^WebGL\ ([0-9].[0-9])(.[0-9])?/.exec( gl.getParameter( gl.VERSION) )[ 1 ] );
var lineWidthAvailable = parseFloat( glVersion ) >= 1.0;

By using this one, the regex is ready to be encapsulated somewhere else when the time will come to test WebGL versions (like when WebGL 2.0 will be used enough).

@mrdoob mrdoob merged commit 7931f84 into mrdoob:dev Dec 15, 2016
@mrdoob
Owner
mrdoob commented Dec 15, 2016

Thanks!

@Ludobaka Ludobaka deleted the Ludobaka:fix_line_width_ie11 branch Dec 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment