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

Duplicate uniforms shared between vertex and fragment programs on mediump-only devices #817

Open
kring opened this issue May 30, 2013 · 11 comments
Assignees

Comments

@kring
Copy link
Member

kring commented May 30, 2013

The GLSL ES Spec (http://www.khronos.org/registry/gles/specs/2.0/GLSL_ES_Specification_1.0.17.pdf) says:

Do precision qualifiers for uniforms need to match?
Option 1: Yes.
Uniforms are defined to behave as if they are using the same storage in the vertex and fragment processors and may be implemented this way.
If uniforms are used in both the vertex and fragment shaders, developers should be warned if the precisions are different. Conversion of precision should never be implicit.
Option 2: No.
Uniforms may be used by both shaders but the same precision may not be available in both so there is a justification for allowing them to be different.
Using the same uniform in the vertex and fragment shaders will always require the precision to be specified in the vertex shader (since the default precision is highp). This is an unnecessary burden on developers.
RESOLUTION: Yes, precision qualifiers for uniforms must match.

So, on a device that only supports mediump precision in the fragment shader, it is not legal to declare a single uniform as highp in the vertex shader (the default) and mediump in the fragment shader. One way to resolve this is to declare it mediump everywhere, but this could cause artifacts in some situations. A safer solution is to duplicate the uniform, so the vertex shader gets a highp version and the fragment shader gets a mediump version. The Cesium renderer could do this automatically with a little bit of work.

We saw this problem in Chrome on a Samsung Galaxy Note 10.1, which has a Mali-400MP GPU. On that device, the shader failed to link as a result of the mismatched precision of the czm_viewport uniform (and probably others).

@pjcozzi
Copy link
Contributor

pjcozzi commented May 30, 2013

The suggested solution here is OK with me.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 12, 2013

Also mentioned on the forum.

@pjcozzi pjcozzi mentioned this issue Aug 13, 2013
11 tasks
@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 27, 2013

Note to self: this is only for uniforms, not varyings.

The only shared globals in ES are uniforms. The precisions of varyings do not need to match.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 6, 2014

Potential future debugging tool: WEBGL_debug_shader_precision

@pjcozzi
Copy link
Contributor

pjcozzi commented Aug 26, 2015

WEBGL_debug_shader_precision was rejected as an extension.

@pjcozzi
Copy link
Contributor

pjcozzi commented Aug 26, 2015

@lilleyse I looked more at this. I think the right general-purpose approach is check if highp is supported in fragment shaders in the ShaderProgram constructor function. If it is, we are good.

If not, extract the uniforms from vertexShaderText and fragmentShaderText. If there are any duplicate uniforms, rename the uniform in the fragment shader (for example). Add a new private member to ShaderProgram that is the list of new uniforms (kinda like _automaticUniforms) and their original uniform name. Extract them in partitionUniforms. Then, finally set their value in _setUniforms. _setUniforms is a hotspot so do not do anything slow in there (or any slower than it already is).

For unit testing, use a custom vertex and fragment shader like this and we'll mock high precision to not be supported.

@lilleyse
Copy link
Contributor

What is the best way to mock high precision not being supported?

@pjcozzi
Copy link
Contributor

pjcozzi commented Aug 30, 2015

To test for highp, see: https://github.com/AnalyticalGraphicsInc/webglreport/blob/master/webglreport.js#L120

For mocking it, you could add a function to Context, and use spyOn(...).and.returnValue(..); (see here). However, it might just be easier to add the value to ContextLimits, and then explicitly set it in the test and then restore it at the end of the test.

@lilleyse
Copy link
Contributor

@kring Do you have a mediump only device that we can use to test this? The changes are on this branch: https://github.com/lilleyse/cesium/tree/uniform-mediump.

@kring
Copy link
Member Author

kring commented Aug 31, 2015

No I don't, sorry. The device mentioned in the original description belonged to AGI. It's entirely possible a software update since then has made the problem go away, though.

@lilleyse
Copy link
Contributor

lilleyse commented Aug 5, 2020

Technically this is still a bug but almost all devices support GL_FRAGMENT_PRECISION_HIGH nowadays.

If you change the code below to force the #else path CesiumJS will crash with

RuntimeError: Program failed to link. Link log: Uniforms with the same name but different type/precision: czm_viewerPositionWC

if (isFragmentShader) {
result +=
"\
#ifdef GL_FRAGMENT_PRECISION_HIGH\n\
precision highp float;\n\
#else\n\
precision mediump float;\n\
#endif\n\n";
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants