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

Emscripten should not set a precision on vertex shaders #8627

Closed
greggman opened this issue May 16, 2019 · 5 comments
Closed

Emscripten should not set a precision on vertex shaders #8627

greggman opened this issue May 16, 2019 · 5 comments
Labels

Comments

@greggman
Copy link
Contributor

greggman commented May 16, 2019

this line

source = ensurePrecision(source);

is adding

precision mediump float;

to any vertex shader that doesn't have a precision set. While fragment shaders need a precision specified vertex shaders do not in WebGL and they default to highp. Setting them to mediump is breaking content on mobile for no reason

the solution is to delete that line.

While we're on the topic there is similar code for fragment shaders. I'd argue that code should default to highp. Why ... because lots of lighting code breaks (using world positions for lights etc) when in mediump. I get that it's probably set to mediump because some old phones don't support highp in fragment shaders but I suspect that's a pretty low percentage in 2019?

In any case, the vertex shader side should have that check removed.

@greggman
Copy link
Contributor Author

@solidpixel
Copy link

solidpixel commented May 17, 2019

For OpenGL ES you must specify a default precision for float variables in shaders, and will get compilation errors without it unless all variables have an explicit precision applied. Edit although sounds like WebGL doesn't, so perhaps for WebGL this can just be removed for vertex shaders.

The problem is not so much the presence of a default, it's just that mediump is the wrong default precision for vertex shaders; fp16 just isn't precise enough for position transforms.

The default selected should be based on shader type - use a highp default for vertex shaders and a mediump default for fragment shaders.

@greggman
Copy link
Contributor Author

greggman commented May 17, 2019

For OpenGL ES you must specify a default precision for float variables in shaders

This NOT true. Only fragment shaders need a default. Vertex shaders do not!!!

From the spec section 4.5.3

The vertex language has the following predeclared globally scoped default precision statements:

   precision highp float;
   precision highp int;
   precision lowp sampler2D;
   precision lowp samplerCube;

The fragment language has the following predeclared globally scoped default precision statements:

   precision mediump int;
   precision lowp sampler2D;
   precision lowp samplerCube;

The fragment language has no default precision qualifier for floating point types. Hence for float, floating point vector and matrix variable declarations, either the declaration must include a precision qualifier or the default float precision must have been previously declared.

And there are webgl conformance tests that you do NOT need a declaration in the vertex shader but do in the fragment shader. The conformance tests that you don't need one in the vertex shader are implicit because the majority of the tests that use vertex shaders don't declare a precision since none is needed. I know because I wrote most of the tests but you can look at them yourself. example

@solidpixel
Copy link

This NOT true. Only fragment shaders need a default. Vertex shaders do not!!!

Heh, worked with it for years, and never noticed that one =) Thanks for the correction.

@stale
Copy link

stale bot commented May 16, 2020

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label May 16, 2020
@stale stale bot closed this as completed May 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants