1178956 - Specify precision in OGLShaderProgram.cpp to fix compositor on iOS

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[(not currently active) Ted Mielczarek]

snorp suggested this fix. I'm not sure if it's the best fix, but it works:
http://hg.mozilla.org/users/tmielczarek_mozilla.com/gecko-ios/rev/eb42b3b1ea39

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: MozReview Request: bug 1178956 - Specify precision in OGLShaderProgram.cpp to fix compositor on iOS. r?kip

bug 1178956 -  Specify precision in OGLShaderProgram.cpp to fix compositor on iOS. r?snorp

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8627855 [details]
MozReview Request: bug 1178956 - Specify precision in OGLShaderProgram.cpp to fix compositor on iOS. r?kip

I don't really know if this is correct or not. Do we want it to be mediump all the time or only GLES? kip should review this.

Comment 3

[:kip (Kearwood Gilbert)]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #2)
> Comment on attachment 8627855 [details]
> MozReview Request: bug 1178956 -  Specify precision in OGLShaderProgram.cpp
> to fix compositor on iOS. r?snorp
> 
> I don't really know if this is correct or not. Do we want it to be mediump
> all the time or only GLES? kip should review this.

For these DEAA coefficient uniforms, mediump should be the minimum.  I expect to see little visual benefit by increasing to highp, as the values are bounded within screen space.

If mediump is not sufficient, artifacts would be visible as incorrectly positioned polygon edges or as noise in the anti-aliased edges of layers that have been rotated or projected.  If you see such symptoms, I'd recommend bumping the DEAA coefficients to highp.

For Apple A7 GPU powered and newer ios devices, there will be no difference between mediump and lowp.  The A7 and newer are scalar GPU's, while the earlier GPU's are truly vector based.

A6 and earlier GPUs have true vector based FPU's.  They represent the lowp values as a 10-bit fixed point format that can be packed together for SIMD operations.  The cost of lowp on these earlier GPUs is the same as mediump unless the shader explicitly packs them in vector unit types.  If supporting A6 and earlier devices, it is recommended to pack color RGBA components into a lowp vec4.  Doing so is essential to achieving good performance.

A7 and later GPUs have scalar FPU's.  They don't differentiate between lowp and mediump, representing both of them as 16-bit floating point.  There is no benefit to lowp if you are only supporting A7 based devices or newer.  If using lowp, you must also test on A6 and earlier devices.

When running on a desktop GPU, these precision qualifiers have no functional effect.  Newer OpenGL versions will ignore them transparently; however, earlier OpenGL versions will fail to compile shaders that include them.  I suspect that Angle is stripping these off on desktop.

TL;DR - mediump should be safe unless you are doing something special.  mediump has no effect on Desktop.

Comment 4

[:kip (Kearwood Gilbert)]

Reviewing the OGLShaderProgram.cpp file, I see that we are in fact explicitly omitting the precision qualifiers here, likely to prevent shader compilation errors on desktop:

  fs << "#ifdef GL_ES" << endl;
  fs << "precision mediump float;" << endl;
  fs << "#define COLOR_PRECISION lowp" << endl;
  fs << "#else" << endl;
  fs << "#define COLOR_PRECISION" << endl;
  fs << "#endif" << endl;

I'd recommend that we use a glsl #define similar to COLOR_PRECISION to omit the mediump qualifier when running on desktop.

Comment 5

[:kip (Kearwood Gilbert)]

Comment on attachment 8627855 [details]
MozReview Request: bug 1178956 - Specify precision in OGLShaderProgram.cpp to fix compositor on iOS. r?kip

https://reviewboard.mozilla.org/r/12293/#review10797

mediump is the right precision qualifier for the uSSEdges uniform; however, the precision qualifiers should not be included on desktop OpenGL.  Please use a GLSL #define to specify the qualifier only on GL_ES, following the same pattern as COLOR_PRECISION in OGLShaderProgram.cpp  (perhaps named such as DEAA_EDGE_PRECISION).  r=me with that

Comment 6

[(not currently active) Ted Mielczarek]

Thanks for the detailed info!

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 8627855 [details]
MozReview Request: bug 1178956 - Specify precision in OGLShaderProgram.cpp to fix compositor on iOS. r?kip

bug 1178956 - Specify precision in OGLShaderProgram.cpp to fix compositor on iOS. r?kip

Comment 8

[:kip (Kearwood Gilbert)]

Comment on attachment 8627855 [details]
MozReview Request: bug 1178956 - Specify precision in OGLShaderProgram.cpp to fix compositor on iOS. r?kip

https://reviewboard.mozilla.org/r/12293/#review17991

Looks great

Comment 9

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/integration/mozilla-inbound/rev/374bc18695f3c25d356044d01d2d61afe59858a2
bug 1178956 - Specify precision in OGLShaderProgram.cpp to fix compositor on iOS. r=kip

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/374bc18695f3