Bug 143993 - [Texmap] highp precision should be used conditionally for fragment shaders on OpenGL ES
Summary: [Texmap] highp precision should be used conditionally for fragment shaders on...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jinyoung Hur
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-21 06:41 PDT by Jinyoung Hur
Modified: 2015-09-03 19:08 PDT (History)
10 users (show)

See Also:


Attachments
Patch (2.35 KB, patch)
2015-04-21 07:21 PDT, Jinyoung Hur
no flags Details | Formatted Diff | Diff
Patch (4.11 KB, patch)
2015-04-23 01:22 PDT, Jinyoung Hur
no flags Details | Formatted Diff | Diff
Patch (9.69 KB, patch)
2015-08-28 08:45 PDT, Jinyoung Hur
no flags Details | Formatted Diff | Diff
Patch (10.21 KB, patch)
2015-08-28 09:36 PDT, Jinyoung Hur
no flags Details | Formatted Diff | Diff
Patch (2.77 KB, patch)
2015-08-28 10:31 PDT, Jinyoung Hur
no flags Details | Formatted Diff | Diff
Patch (2.65 KB, patch)
2015-08-31 09:15 PDT, Jinyoung Hur
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jinyoung Hur 2015-04-21 06:41:01 PDT
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.
Comment 1 Jinyoung Hur 2015-04-21 07:21:37 PDT
Created attachment 251228 [details]
Patch
Comment 2 Gyuyoung Kim 2015-04-22 18:39:16 PDT
CC'ing Brent, Zan
Comment 3 Gyuyoung Kim 2015-04-22 18:44:59 PDT
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
Comment 4 Jinyoung Hur 2015-04-23 01:22:29 PDT
Created attachment 251417 [details]
Patch
Comment 5 Jinyoung Hur 2015-04-23 01:25:53 PDT
Thank you for reviewing.
I've uploaded a new patch which includes a modified test.
Comment 6 Jinyoung Hur 2015-08-28 08:45:21 PDT
Created attachment 260154 [details]
Patch
Comment 7 Jinyoung Hur 2015-08-28 09:36:19 PDT
Created attachment 260159 [details]
Patch
Comment 8 Jinyoung Hur 2015-08-28 10:31:54 PDT
Created attachment 260161 [details]
Patch
Comment 9 Jinyoung Hur 2015-08-28 10:37:44 PDT
(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.
Comment 10 Jinyoung Hur 2015-08-28 10:40:59 PDT
(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.
Comment 11 BJ Burg 2015-08-31 07:59:38 PDT
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.
Comment 12 Jinyoung Hur 2015-08-31 09:15:24 PDT
Created attachment 260287 [details]
Patch
Comment 13 Jinyoung Hur 2015-08-31 09:17:15 PDT
(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.
Comment 14 Martin Robinson 2015-09-03 17:11:35 PDT
Is there a way to detect this at runtime or via the headers?
Comment 15 Jinyoung Hur 2015-09-03 18:05:56 PDT
(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
Comment 16 Martin Robinson 2015-09-03 18:20:42 PDT
Comment on attachment 260287 [details]
Patch

Oh, thanks. I understand now. This seems totally reasonable.
Comment 17 WebKit Commit Bot 2015-09-03 19:08:31 PDT
Comment on attachment 260287 [details]
Patch

Clearing flags on attachment: 260287

Committed r189331: <http://trac.webkit.org/changeset/189331>
Comment 18 WebKit Commit Bot 2015-09-03 19:08:40 PDT
All reviewed patches have been landed.  Closing bug.