Bug 143993 – [Texmap] highp precision should be used conditionally for fragment shaders on OpenGL ES

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Jinyoung Hur]

OpenGL ES 2.0 does not require that an implementation
support high precision in the fragment shader. So, high precision
declaration should be guarded using a preprocessor macro.

[Jinyoung Hur]

Created attachment 251228 [details]
Patch

[Gyuyoung Kim]

CC'ing Brent, Zan

[Gyuyoung Kim]

Comment on attachment 251228 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=251228&action=review

> Source/WebCore/ChangeLog:7
> +

I think you need to describe what is problem, what is fixed here.

> Source/WebCore/ChangeLog:8
> +        I believe this patch cannot be tested because we need a hardware that is not supporting

I'm not expert in this area though, it looks some tests are testing *highp float* type.

e.g) http://trac.webkit.org/browser/trunk/LayoutTests/fast/canvas/webgl/texture-complete.html

[Jinyoung Hur]

Created attachment 251417 [details]
Patch

[Jinyoung Hur]

Thank you for reviewing.
I've uploaded a new patch which includes a modified test.

[Jinyoung Hur]

Created attachment 260154 [details]
Patch

[Jinyoung Hur]

Created attachment 260159 [details]
Patch

[Jinyoung Hur]

Created attachment 260161 [details]
Patch

[Jinyoung Hur]

(In reply to comment #3)
> Comment on attachment 251228 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=251228&action=review
> 
> > Source/WebCore/ChangeLog:7
> > +
> 
> I think you need to describe what is problem, what is fixed here.
> 
> > Source/WebCore/ChangeLog:8
> > +        I believe this patch cannot be tested because we need a hardware that is not supporting
> 
> I'm not expert in this area though, it looks some tests are testing *highp
> float* type.
> 
> e.g)
> http://trac.webkit.org/browser/trunk/LayoutTests/fast/canvas/webgl/texture-
> complete.html

I noticed that some webgl tests have the same highp precision bugs in itself. I'll file  another bug for this later.

[Jinyoung Hur]

(In reply to comment #5)
> Thank you for reviewing.
> I've uploaded a new patch which includes a modified test.

There is no modified test anymore.
webgl tests were ignored and will be filed as a bug later.

[Blaze Burg]

Comment on attachment 260161 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=260161&action=review

> Source/WebCore/ChangeLog:15
> +        Test: css1/basic/containment.html

If you did not add a new test, usually the way to say that is simply:

No new tests, covered by existing tests.

[Jinyoung Hur]

Created attachment 260287 [details]
Patch

[Jinyoung Hur]

(In reply to comment #11)
> Comment on attachment 260161 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=260161&action=review
> 
> > Source/WebCore/ChangeLog:15
> > +        Test: css1/basic/containment.html
> 
> If you did not add a new test, usually the way to say that is simply:
> 
> No new tests, covered by existing tests.

Thanks for reviewing!
I've modified as you suggested.

[Martin Robinson]

Is there a way to detect this at runtime or via the headers?

[Jinyoung Hur]

(In reply to comment #14)
> Is there a way to detect this at runtime or via the headers?

I believe this is already detecting the highp precision supports at runtime, I mean when shaders are being compiled.

Khronos suggests to use GL_FRAGMENT_PRECISION_HIGH macro like this.

#ifdef GL_FRAGMENT_PRECISION_HIGH
precision highp float;
#else
precision mediump float;
#endif

https://www.khronos.org/webgl/wiki/WebGL_and_OpenGL_Differences

[Martin Robinson]

Comment on attachment 260287 [details]
Patch

Oh, thanks. I understand now. This seems totally reasonable.

[WebKit Commit Bot]

Comment on attachment 260287 [details]
Patch

Clearing flags on attachment: 260287

Committed r189331: <http://trac.webkit.org/changeset/189331>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.