ValhallaTeam / angleproject

Automatically exported from code.google.com/p/angleproject
Other
0 stars 0 forks source link

GL_FRAGMENT_PRECISION_HIGH not defined #413

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
N4, N7, and N10 all support highp precision in their fragment shaders but 
GL_FRAGMENT_PRECISION_HIGH is not defined.

According to the GLSL ES spec, GL_FRAGMENT_PRECISION_HIGH should be defined to 
1 in both the vertex and fragment shader if highp is supported.  If it is not 
supported, it should be undefined.

This bug is likely due to ANGLE preprocessor evaluation before handing the 
shader off to the driver.

We should be able to define GL_FRAGMENT_PRECISION_HIGH based on the result of 
glGetShaderPrecisionFormat:
http://www.khronos.org/opengles/sdk/docs/man/xhtml/glGetShaderPrecisionFormat.xm
l

Original issue reported on code.google.com by briander...@chromium.org on 22 Feb 2013 at 12:20

GoogleCodeExporter commented 9 years ago

Original comment by vangelis@chromium.org on 22 Feb 2013 at 7:12

GoogleCodeExporter commented 9 years ago
I can add support for GL_FRAGMENT_PRECISION_HIGH into the shader preprocessor. 
But the preprocessor clients (GL and DX backends) would need to enable it if 
the hardware actually supports it. It will be disabled by default.

Assigning this to myself for the first part. I will need help from Gregg and 
Nicolas to complete the second part.

Original comment by alokp@chromium.org on 22 Feb 2013 at 6:03

GoogleCodeExporter commented 9 years ago
Any updates? Support for this would simplify a patch of mine.

Original comment by briander...@chromium.org on 2 Mar 2013 at 1:23

GoogleCodeExporter commented 9 years ago
According to MSDN all Shader Model 2+ hardware supports FP24 (s16e7) pixel 
shaders 
(http://msdn.microsoft.com/en-us/library/windows/desktop/bb147365(v=vs.85).aspx)
. Since this is the minimum hardware supported by ANGLE, setting 
GL_FRAGMENT_PRECISION_HIGH should be unconditional.

I'll try to add this early next week.

Original comment by nicolas....@gmail.com on 2 Mar 2013 at 5:51

GoogleCodeExporter commented 9 years ago
Setting GL_FRAGMENT_PRECISION_HIGH unconditionally on desktop is a sane thing 
to do, however it'll also need to be defined properly for mobile devices that 
use ANGLE's preprocessor.

Original comment by briander...@chromium.org on 2 Mar 2013 at 6:46

GoogleCodeExporter commented 9 years ago

Original comment by kbr@chromium.org on 4 Mar 2013 at 7:01

GoogleCodeExporter commented 9 years ago
preprocessor support is almost done: https://codereview.appspot.com/7402051/

Original comment by alokp@chromium.org on 4 Mar 2013 at 7:11

GoogleCodeExporter commented 9 years ago
I don't think you need preprocessor changes at all. I've been able to add it to 
ShBuiltInResources as OES_fragment_precision_high, and then name the string 
"GL_FRAGMENT_PRECISION_HIGH" in InitExtensionBehavior(). The only side-effect 
is that you can use it in an #extension line, but that just gets ignored.

Original comment by nicolas....@gmail.com on 4 Mar 2013 at 7:29

GoogleCodeExporter commented 9 years ago
Here's my Git patch for that approach.

Original comment by nicolas....@gmail.com on 4 Mar 2013 at 7:30

Attachments:

GoogleCodeExporter commented 9 years ago
Nicolas: fragment-precision-high is not an extension, so adding it that way may 
be confusing, although it would always do the right thing.

My patch at https://codereview.appspot.com/7402051/ does essentially that. It 
also does some error checking. Could you please take a look?

Original comment by alokp@chromium.org on 4 Mar 2013 at 7:44

GoogleCodeExporter commented 9 years ago
Preprocessor support was added in r1984.
GL_FRAGMENT_PRECISION_HIGH was unconditionally enabled in ANGLE in r1985.

Next step is to enable it in chromium command-buffer for desktop GL and 
chrome-os EGL.

Original comment by alokp@chromium.org on 4 Mar 2013 at 8:34

GoogleCodeExporter commented 9 years ago
Those are not my findings. 
Upgrade to 4.3 last night on N4 and N7 have caused OpenGL shader problems.

Nexus4: GL_FRAGMENT_PRECISION_HIGH defined, ES3.0 context, 
glGetShaderPrecisionFormat for GL_HIGH_FLOAT returns non-zero values.

Nexus7: undefined, ES2.0, zeros from glGetShaderPrecisionFormat.

This would appear to indicate that N7 does NOT support high precision floats in 
fragment shaders, and the preprocessor define is correct in both cases.

Original comment by alancald...@googlemail.com on 28 Jul 2013 at 10:32

GoogleCodeExporter commented 9 years ago
Surely glGetShaderPrecisionFormat isn't driven by the preprocessor #define?

Original comment by alancald...@googlemail.com on 28 Jul 2013 at 11:55

GoogleCodeExporter commented 9 years ago
Re #12: Please take a look at 
https://code.google.com/p/chromium/issues/detail?id=234227 for additional 
background. Your results are correct and to be expected. Tegra 3 supports 
13-bits of precision in the fragment shader, which is more precision than 
required for mediump, but not quite enough precision to be considered highp.

Re #13: ANGLE's glGetShaderPrecisionFormat and GL_FRAGMENT_PRECISION_HIGH 
define are both driven by the results of the driver's 
glGetShaderPrecisionFormat. If the driver's glGetShaderPrecisionFormat doesn't 
return values that meet the minimum requirements of the spec, ANGLE will not 
claim support.

Alok, are there any more action items for this issue? I think it can be closed.

Original comment by briander...@chromium.org on 29 Jul 2013 at 5:07

GoogleCodeExporter commented 9 years ago

Original comment by c...@chromium.org on 7 Dec 2013 at 4:05

GoogleCodeExporter commented 9 years ago

Original comment by geofflang@chromium.org on 10 Dec 2013 at 3:53

GoogleCodeExporter commented 9 years ago
Implemented in cd0a9a888bcae98d577308e6eb2602f0f424e2ea "Define 
GL_FRAGMENT_PRECISION_HIGH".

Original comment by c...@chromium.org on 21 May 2014 at 7:14