Re: [Mesa-dev] [PATCH 05/10] glsl: Don't allow gl_FragData[i], with i0 in GLES shaders

2014-12-02 Thread Tapani Pälli 



On 12/01/2014 05:43 PM, Matteo Bruni wrote:

2014-12-01 14:04 GMT+01:00 Eduardo Lima Mitev el...@igalia.com:

The OpenGL ES Shading Language specification describes the
values that may be output by a fragment shader. These are
gl_FragColor and gl_FragData[0]. Multiple render targets
are not supported in GLES.

Fixes dEQP test:
   * dEQP-GLES3.functional.shaders.fragdata.invalid_assign_to_1
---
  src/glsl/ast_array_index.cpp | 21 -
  1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
index ff0c757..b507d34 100644
--- a/src/glsl/ast_array_index.cpp
+++ b/src/glsl/ast_array_index.cpp
@@ -46,7 +46,9 @@ ast_array_specifier::print(void) const
   *
   * This function also checks whether the array is a built-in array whose
   * maximum size is too small to accommodate the given index, and if so uses
- * loc and state to report the error.
+ * loc and state to report the error. It also checks that the built-in array
+ * gl_FragData is not accessed with indexes greater than zero in OpenGL ES,
+ * where multiple render targets are not allowed.
   */
  static void
  update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE *loc,
@@ -54,6 +56,23 @@ update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE *loc,
  {
 if (ir_dereference_variable *deref_var = ir-as_dereference_variable()) {
ir_variable *var = deref_var-var;
+
+  /* Page 89 in the section 3.8 (Fragment Shaders) of the the
+   * OpenGL ES 2.0.25 spec says:
+   * The OpenGL ES Shading Language specification describes the
+   * values that may be output by a fragment shader. These are
+   * gl_FragColor and gl_FragData[0].
+   *  ...
+   * gl_FragData is supported for compatibility with the desktop
+   * OpenGL Shading Language, but only a single fragment color
+   * output is allowed in the OpenGL ES Shading Language.
+   */
+  if (state-es_shader  idx  0 
+  strcmp(var-name, gl_FragData) == 0) {
+ _mesa_glsl_error(loc, state,
+  multiple render targets are not supported);
+  }
+
if (idx  (int)var-data.max_array_access) {
   var-data.max_array_access = idx;

--
2.1.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


AFAICS this restriction is lifted in ES 3.0+ (e.g. see section 3.9.2
in the OpenGL ES 3.0 spec).


Yep, it seems this check should be against earlier versions and only 
when not having extensions that allow MRT enabled (NV_draw_buffers).


You can still use gl_FragData[] when using OpenGL ES 3 and GLSL ES 1.00, 
however when using GLSL ES 3.00 gl_FragData[] gets deprecated and you 
should use output layout qualifier to bind a output variable and target 
draw buffer together.





___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev



// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 05/10] glsl: Don't allow gl_FragData[i], with i0 in GLES shaders

2014-12-02 Thread Eduardo Lima Mitev 
On 12/02/2014 01:31 PM, Tapani Pälli wrote:

 AFAICS this restriction is lifted in ES 3.0+ (e.g. see section 3.9.2
 in the OpenGL ES 3.0 spec).
 
 Yep, it seems this check should be against earlier versions and only
 when not having extensions that allow MRT enabled (NV_draw_buffers).
 

Hi,

Yes, we will update the patch to consider the extensions too. Good catch.

 You can still use gl_FragData[] when using OpenGL ES 3 and GLSL ES 1.00,
 however when using GLSL ES 3.00 gl_FragData[] gets deprecated and you
 should use output layout qualifier to bind a output variable and target
 draw buffer together.
 

I suppose this is already working like that. This patch does not modify
that behavior.

Thanks for the feedback,

Eduardo
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 05/10] glsl: Don't allow gl_FragData[i], with i0 in GLES shaders

2014-12-02 Thread Ian Romanick 
On 12/01/2014 05:04 AM, Eduardo Lima Mitev wrote:
 The OpenGL ES Shading Language specification describes the
 values that may be output by a fragment shader. These are
 gl_FragColor and gl_FragData[0]. Multiple render targets
 are not supported in GLES.
 
 Fixes dEQP test:
   * dEQP-GLES3.functional.shaders.fragdata.invalid_assign_to_1

I think the test needs to be updated for interactions with
GL_NV_draw_buffers.  Right now every Mesa driver enables
GL_NV_draw_buffers by default, so I don't think this check is necessary.

 ---
  src/glsl/ast_array_index.cpp | 21 -
  1 file changed, 20 insertions(+), 1 deletion(-)
 
 diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
 index ff0c757..b507d34 100644
 --- a/src/glsl/ast_array_index.cpp
 +++ b/src/glsl/ast_array_index.cpp
 @@ -46,7 +46,9 @@ ast_array_specifier::print(void) const
   *
   * This function also checks whether the array is a built-in array whose
   * maximum size is too small to accommodate the given index, and if so uses
 - * loc and state to report the error.
 + * loc and state to report the error. It also checks that the built-in array
 + * gl_FragData is not accessed with indexes greater than zero in OpenGL ES,
 + * where multiple render targets are not allowed.
   */
  static void
  update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE *loc,
 @@ -54,6 +56,23 @@ update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE 
 *loc,
  {
 if (ir_dereference_variable *deref_var = ir-as_dereference_variable()) {
ir_variable *var = deref_var-var;
 +
 +  /* Page 89 in the section 3.8 (Fragment Shaders) of the the
 +   * OpenGL ES 2.0.25 spec says:
 +   * The OpenGL ES Shading Language specification describes the
 +   * values that may be output by a fragment shader. These are
 +   * gl_FragColor and gl_FragData[0].
 +   *  ...
 +   * gl_FragData is supported for compatibility with the desktop
 +   * OpenGL Shading Language, but only a single fragment color
 +   * output is allowed in the OpenGL ES Shading Language.
 +   */
 +  if (state-es_shader  idx  0 
 +  strcmp(var-name, gl_FragData) == 0) {
 + _mesa_glsl_error(loc, state,
 +  multiple render targets are not supported);
 +  }
 +
if (idx  (int)var-data.max_array_access) {
   var-data.max_array_access = idx;
  
 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 05/10] glsl: Don't allow gl_FragData[i], with i0 in GLES shaders

2014-12-01 Thread Eduardo Lima Mitev 
The OpenGL ES Shading Language specification describes the
values that may be output by a fragment shader. These are
gl_FragColor and gl_FragData[0]. Multiple render targets
are not supported in GLES.

Fixes dEQP test:
  * dEQP-GLES3.functional.shaders.fragdata.invalid_assign_to_1
---
 src/glsl/ast_array_index.cpp | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
index ff0c757..b507d34 100644
--- a/src/glsl/ast_array_index.cpp
+++ b/src/glsl/ast_array_index.cpp
@@ -46,7 +46,9 @@ ast_array_specifier::print(void) const
  *
  * This function also checks whether the array is a built-in array whose
  * maximum size is too small to accommodate the given index, and if so uses
- * loc and state to report the error.
+ * loc and state to report the error. It also checks that the built-in array
+ * gl_FragData is not accessed with indexes greater than zero in OpenGL ES,
+ * where multiple render targets are not allowed.
  */
 static void
 update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE *loc,
@@ -54,6 +56,23 @@ update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE *loc,
 {
if (ir_dereference_variable *deref_var = ir-as_dereference_variable()) {
   ir_variable *var = deref_var-var;
+
+  /* Page 89 in the section 3.8 (Fragment Shaders) of the the
+   * OpenGL ES 2.0.25 spec says:
+   * The OpenGL ES Shading Language specification describes the
+   * values that may be output by a fragment shader. These are
+   * gl_FragColor and gl_FragData[0].
+   *  ...
+   * gl_FragData is supported for compatibility with the desktop
+   * OpenGL Shading Language, but only a single fragment color
+   * output is allowed in the OpenGL ES Shading Language.
+   */
+  if (state-es_shader  idx  0 
+  strcmp(var-name, gl_FragData) == 0) {
+ _mesa_glsl_error(loc, state,
+  multiple render targets are not supported);
+  }
+
   if (idx  (int)var-data.max_array_access) {
  var-data.max_array_access = idx;
 
-- 
2.1.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 05/10] glsl: Don't allow gl_FragData[i], with i0 in GLES shaders

2014-12-01 Thread Matteo Bruni 
2014-12-01 14:04 GMT+01:00 Eduardo Lima Mitev el...@igalia.com:
 The OpenGL ES Shading Language specification describes the
 values that may be output by a fragment shader. These are
 gl_FragColor and gl_FragData[0]. Multiple render targets
 are not supported in GLES.

 Fixes dEQP test:
   * dEQP-GLES3.functional.shaders.fragdata.invalid_assign_to_1
 ---
  src/glsl/ast_array_index.cpp | 21 -
  1 file changed, 20 insertions(+), 1 deletion(-)

 diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
 index ff0c757..b507d34 100644
 --- a/src/glsl/ast_array_index.cpp
 +++ b/src/glsl/ast_array_index.cpp
 @@ -46,7 +46,9 @@ ast_array_specifier::print(void) const
   *
   * This function also checks whether the array is a built-in array whose
   * maximum size is too small to accommodate the given index, and if so uses
 - * loc and state to report the error.
 + * loc and state to report the error. It also checks that the built-in array
 + * gl_FragData is not accessed with indexes greater than zero in OpenGL ES,
 + * where multiple render targets are not allowed.
   */
  static void
  update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE *loc,
 @@ -54,6 +56,23 @@ update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE 
 *loc,
  {
 if (ir_dereference_variable *deref_var = ir-as_dereference_variable()) {
ir_variable *var = deref_var-var;
 +
 +  /* Page 89 in the section 3.8 (Fragment Shaders) of the the
 +   * OpenGL ES 2.0.25 spec says:
 +   * The OpenGL ES Shading Language specification describes the
 +   * values that may be output by a fragment shader. These are
 +   * gl_FragColor and gl_FragData[0].
 +   *  ...
 +   * gl_FragData is supported for compatibility with the desktop
 +   * OpenGL Shading Language, but only a single fragment color
 +   * output is allowed in the OpenGL ES Shading Language.
 +   */
 +  if (state-es_shader  idx  0 
 +  strcmp(var-name, gl_FragData) == 0) {
 + _mesa_glsl_error(loc, state,
 +  multiple render targets are not supported);
 +  }
 +
if (idx  (int)var-data.max_array_access) {
   var-data.max_array_access = idx;

 --
 2.1.3

 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev

AFAICS this restriction is lifted in ES 3.0+ (e.g. see section 3.9.2
in the OpenGL ES 3.0 spec).
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev