Specify precision in OGLShaderProgram.cpp to fix compositor on iOS

RESOLVED FIXED in Firefox 44

Status

()

Core
Graphics: Layers
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: ted, Assigned: ted)

Tracking

(Blocks: 1 bug)

unspecified
mozilla44
Points:
---

Firefox Tracking Flags

(firefox44 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
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
(Assignee)

Comment 1

3 years ago
Created 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?snorp
Attachment #8627855 - Flags: review?(snorp)
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.
Attachment #8627855 - Flags: review?(snorp) → review?(kgilbert)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) 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.
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.
Attachment #8627855 - Flags: review?(kgilbert)
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
(Assignee)

Comment 6

3 years ago
Thanks for the detailed info!
(Assignee)

Comment 7

2 years ago
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
Attachment #8627855 - Attachment description: MozReview Request: bug 1178956 - Specify precision in OGLShaderProgram.cpp to fix compositor on iOS. r?snorp → MozReview Request: bug 1178956 - Specify precision in OGLShaderProgram.cpp to fix compositor on iOS. r?kip
Attachment #8627855 - Flags: review?(kgilbert)
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
Attachment #8627855 - Flags: review?(kgilbert) → review+
(Assignee)

Comment 9

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/374bc18695f3c25d356044d01d2d61afe59858a2
bug 1178956 - Specify precision in OGLShaderProgram.cpp to fix compositor on iOS. r=kip
https://hg.mozilla.org/mozilla-central/rev/374bc18695f3
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.