kripken / BananaBread

BananaBread is a C++ 3D game engine that runs on the web using JavaScript+WebGL+HTML
1.38k stars 347 forks source link

Precision mismatch #42

Closed svenpanne closed 9 years ago

svenpanne commented 10 years ago

A few months ago, some WebGL conformance fixes were made for Chrome: https://code.google.com/p/chromium/issues/detail?id=249018 These fixes include a check that the precision of uniforms match when linking GLSL programs, which is necessary for e.g. https://www.khronos.org/registry/webgl/conformance-suites/1.0.2/conformance/glsl/misc/shader-with-global-variable-precision-mismatch.html

I am not sure if it is a bug in BananaBread itself or the demo on https://developer.mozilla.org/demos/detail/bananabread, but with recent Chrome versions containing this new check, this leads to some shaders not linking, which again results in strange colors and some output of the form

[STDOUT] Uniforms with the same name but different type/precisio

in the console. FireFox is not WebGL conformant in this respect (see Khronos link above) and happily links the program nevertheless, so the demo works with it.

In detail the problem is as follows: In the vertex shader (which uses "precision highp float" per default, see GLSL spec) we have:

uniform vec4 waterfadeparams;

and in the fragment shader (which has no default for float) we have:

precision mediump float; uniform vec4 waterfadeparams;

Chrome correctly detects the precision mismatch and refuses to link the program. There are several other uniforms in the demo with similar problems: depthfxparams, envscale, glowcolor, lightdir, lightscale, parallaxscale, specscale, ...

kripken commented 10 years ago

Do you know if this is an issue on desktop GL as well, or just with WebGL? I wonder if this is an issue in Sauerbraten or our glue code for GL.

svenpanne commented 10 years ago

The precision specification is by definition a no-op in the desktop GLSL, it was only introduced for compatibility with the WebGL/OpenGL ES GLSL. Therefore things work if you simply pass the GLSL through to your desktop driver, but the WebGL spec nevertheless requires the linking step to always fail (FireFox obviously omits this check and fails the corresponding conformance tests on the desktop). This makes sense, because on actual mobile hardware + drivers, lowp/mediump/highp actually makes a difference.

Note that the crypt demo http://crypt-webgl.unigine.com/ has a similar bug: In the vertex shader you have a default "precision highp int" while in the fragment shader the default is "precision mediump int". So simply declaring an "int" uniform without any further annotations/defaults in both kinds of shaders will lead to a linking error. Alas, I don't have a clue how/where to report that bug... .-(

kripken commented 10 years ago

cc @juj and @vvuk for the GL stuff - not sure if we want to work around this in emscripten, or expect projects to fix it themselves - and @wolfviking0 for the unigine stuff.

juj commented 10 years ago

Are all these uniforms such that both vertex and fragment shaders share them? I think it's generally uncommon for both vs and fs to reuse this many uniforms. Perhaps the shader code redundantly specifies uniforms for both shaders, even though it doesn't use them?

If the uniforms are used in both shaders, then I think the general solution would be to explicitly flag those individual mediump fragment shader uniforms that vertex shaders refer to as mediump as well, i.e. have

uniform mediump vec4 waterfadeparams;

in the vertex shader. Does that help getting around the issue?

svenpanne commented 10 years ago

From what I've seen, the inconsistencies were caused by precision mismatches between the vertex and fragment shaders only, not inter-vertex nor inter-fragment problems, but I am not 100% sure. And yes, consistently declaring the precision will solve the problem for WebGL/OpenGL ES. Other aspects of the uniforms matched, I tested this by removing only the precision check in Chrome.

I am not 100% sure which versions of Chrome contain the stricter check, but current Canary builds will definitely have them, so looking into their console might help in testing.

kripken commented 10 years ago

Ok, I see the issue on chrome 33, and I pushed fc015116c71b77cef3e34a3f72eda460474216c8 in emscripten so that we emit precision in both vertex and fragment shaders, this fixes the issue in a local build.

@modeswitch , do you remember how we push a new build to the demo site?

modeswitch commented 10 years ago

@kripken yeah, I can rebuild. Is this urgent, or can I wait until I'm back in the office?

kripken commented 10 years ago

Not that urgent, whenever you get around to it, thanks.

On Fri, Jan 3, 2014 at 9:22 AM, Alan K notifications@github.com wrote:

@kripken https://github.com/kripken yeah, I can rebuild. Is this urgent, or can I wait until I'm back in the office?

— Reply to this email directly or view it on GitHubhttps://github.com/kripken/BananaBread/issues/42#issuecomment-31537740 .

svenpanne commented 10 years ago

I just checked https://developer.mozilla.org/de/demos/detail/bananabread with a freshly built Chrome from sources (Version 35.0.1901.0 aura (258235)), and the problem is still there: In the console one can see the warning about the precision mismatch, and the colors in the demo are still funky due to the missing shaders. It would really be nice if the demo site gets an update...

kripken commented 10 years ago

Agreed, and gdc is now over, so no excuses ;) @modeswitch , let me know if you need anything from me for this.

kripken commented 10 years ago

@modeswitch , any chance you can get around to this? Alternatively I can push a build but I think only you can set up the networking stuff...

modeswitch commented 10 years ago

Yeah, I'll make time to give bananabread some love.

slacka commented 9 years ago

@svenpanne Is issue #53 a duplicate of this bug? Also the precision mismatch issue is discussed at https://bugzilla.mozilla.org/show_bug.cgi?id=1094047

svenpanne commented 9 years ago

Yep, that's a duplicate. It's nice to see that FireFox starts to follow the WebGL spec in a stricter way, too. :smiley:

kripken commented 9 years ago

Updated the build, should be fixed.