subject:"\[Mesa\-dev\] \[PATCH\] glsl\: do not set the 'smooth' qualifier by default on ES shaders"

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

2017-09-27 Thread Dieter Nützel


Tested-by: Dieter Nützel 

Dieter

Am 26.09.2017 17:11, schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

It leads to surprising states with integer inputs and outputs on
vertex processing stages (e.g. geometry stages). Instead, rely on the
driver to choose smooth interpolation by default.

We still allow varyings to match when one stage declares it as smooth
and the other declares it without interpolation qualifiers.
--
This should cover all the relevant I/O checks, tested with dEQP.
---
 src/compiler/glsl/ast_to_hir.cpp| 11 ---
 src/compiler/glsl/link_varyings.cpp | 17 -
 src/mesa/main/shader_query.cpp  |  8 +++-
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp 
b/src/compiler/glsl/ast_to_hir.cpp

index c46454956d7..1999e68158c 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3119,31 +3119,20 @@ interpret_interpolation_qualifier(const struct
ast_type_qualifier *qual,
   struct _mesa_glsl_parse_state 
*state,

   YYLTYPE *loc)
 {
glsl_interp_mode interpolation;
if (qual->flags.q.flat)
   interpolation = INTERP_MODE_FLAT;
else if (qual->flags.q.noperspective)
   interpolation = INTERP_MODE_NOPERSPECTIVE;
else if (qual->flags.q.smooth)
   interpolation = INTERP_MODE_SMOOTH;
-   else if (state->es_shader &&
-((mode == ir_var_shader_in &&
-  state->stage != MESA_SHADER_VERTEX) ||
- (mode == ir_var_shader_out &&
-  state->stage != MESA_SHADER_FRAGMENT)))
-  /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
-   *
-   *"When no interpolation qualifier is present, smooth 
interpolation

-   *is used."
-   */
-  interpolation = INTERP_MODE_SMOOTH;
else
   interpolation = INTERP_MODE_NONE;

validate_interpolation_qualifier(state, loc,
 interpolation,
 qual, var_type, mode);

return interpolation;
 }

diff --git a/src/compiler/glsl/link_varyings.cpp
b/src/compiler/glsl/link_varyings.cpp
index 528506fd0eb..7c90de393ad 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -318,22 +318,37 @@ cross_validate_types_and_qualifiers(struct
gl_shader_program *prog,
}

/* GLSL >= 4.40 removes text requiring interpolation qualifiers
 * to match cross stage, they must only match within the same 
stage.

 *
 * From page 84 (page 90 of the PDF) of the GLSL 4.40 spec:
 *
 * "It is a link-time error if, within the same stage, the 
interpolation

 * qualifiers of variables of the same name do not match.
 *
+* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
+*
+*"When no interpolation qualifier is present, smooth 
interpolation

+*is used."
+*
+* So we match variables where one is smooth and the other has no 
explicit

+* qualifier.
 */
-   if (input->data.interpolation != output->data.interpolation &&
+   unsigned input_interpolation = input->data.interpolation;
+   unsigned output_interpolation = output->data.interpolation;
+   if (prog->IsES) {
+  if (input_interpolation == INTERP_MODE_NONE)
+ input_interpolation = INTERP_MODE_SMOOTH;
+  if (output_interpolation == INTERP_MODE_NONE)
+ output_interpolation = INTERP_MODE_SMOOTH;
+   }
+   if (input_interpolation != output_interpolation &&
prog->data->Version < 440) {
   linker_error(prog,
"%s shader output `%s' specifies %s "
"interpolation qualifier, "
"but %s shader input specifies %s "
"interpolation qualifier\n",
_mesa_shader_stage_to_string(producer_stage),
output->name,
interpolation_string(output->data.interpolation),
_mesa_shader_stage_to_string(consumer_stage),
diff --git a/src/mesa/main/shader_query.cpp 
b/src/mesa/main/shader_query.cpp

index 64e68b4a26d..6712bb45fb2 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -1627,21 +1627,27 @@ validate_io(struct gl_program *producer,
struct gl_program *consumer)
*Precision  |   mediump   |  Yes
*   |highp|
*---+-+--
*Variance   |  invariant  |   No
*---+-+--
*Memory | all |  N/A
*
* Note that location mismatches are detected by the loops above 
that
* find the producer variable that goes with the consumer 
variable.

*/
-  if (producer_var->interpolation != consumer_var->interpolation) 
{

+  unsigned producer_interpolation

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

2017-09-27 Thread Marek Olšák

Reviewed-by: Marek Olšák 

Marek

On Tue, Sep 26, 2017 at 5:11 PM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> It leads to surprising states with integer inputs and outputs on
> vertex processing stages (e.g. geometry stages). Instead, rely on the
> driver to choose smooth interpolation by default.
>
> We still allow varyings to match when one stage declares it as smooth
> and the other declares it without interpolation qualifiers.
> --
> This should cover all the relevant I/O checks, tested with dEQP.
> ---
>  src/compiler/glsl/ast_to_hir.cpp| 11 ---
>  src/compiler/glsl/link_varyings.cpp | 17 -
>  src/mesa/main/shader_query.cpp  |  8 +++-
>  3 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
> b/src/compiler/glsl/ast_to_hir.cpp
> index c46454956d7..1999e68158c 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -3119,31 +3119,20 @@ interpret_interpolation_qualifier(const struct 
> ast_type_qualifier *qual,
>struct _mesa_glsl_parse_state *state,
>YYLTYPE *loc)
>  {
> glsl_interp_mode interpolation;
> if (qual->flags.q.flat)
>interpolation = INTERP_MODE_FLAT;
> else if (qual->flags.q.noperspective)
>interpolation = INTERP_MODE_NOPERSPECTIVE;
> else if (qual->flags.q.smooth)
>interpolation = INTERP_MODE_SMOOTH;
> -   else if (state->es_shader &&
> -((mode == ir_var_shader_in &&
> -  state->stage != MESA_SHADER_VERTEX) ||
> - (mode == ir_var_shader_out &&
> -  state->stage != MESA_SHADER_FRAGMENT)))
> -  /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
> -   *
> -   *"When no interpolation qualifier is present, smooth interpolation
> -   *is used."
> -   */
> -  interpolation = INTERP_MODE_SMOOTH;
> else
>interpolation = INTERP_MODE_NONE;
>
> validate_interpolation_qualifier(state, loc,
>  interpolation,
>  qual, var_type, mode);
>
> return interpolation;
>  }
>
> diff --git a/src/compiler/glsl/link_varyings.cpp 
> b/src/compiler/glsl/link_varyings.cpp
> index 528506fd0eb..7c90de393ad 100644
> --- a/src/compiler/glsl/link_varyings.cpp
> +++ b/src/compiler/glsl/link_varyings.cpp
> @@ -318,22 +318,37 @@ cross_validate_types_and_qualifiers(struct 
> gl_shader_program *prog,
> }
>
> /* GLSL >= 4.40 removes text requiring interpolation qualifiers
>  * to match cross stage, they must only match within the same stage.
>  *
>  * From page 84 (page 90 of the PDF) of the GLSL 4.40 spec:
>  *
>  * "It is a link-time error if, within the same stage, the 
> interpolation
>  * qualifiers of variables of the same name do not match.
>  *
> +* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
> +*
> +*"When no interpolation qualifier is present, smooth interpolation
> +*is used."
> +*
> +* So we match variables where one is smooth and the other has no explicit
> +* qualifier.
>  */
> -   if (input->data.interpolation != output->data.interpolation &&
> +   unsigned input_interpolation = input->data.interpolation;
> +   unsigned output_interpolation = output->data.interpolation;
> +   if (prog->IsES) {
> +  if (input_interpolation == INTERP_MODE_NONE)
> + input_interpolation = INTERP_MODE_SMOOTH;
> +  if (output_interpolation == INTERP_MODE_NONE)
> + output_interpolation = INTERP_MODE_SMOOTH;
> +   }
> +   if (input_interpolation != output_interpolation &&
> prog->data->Version < 440) {
>linker_error(prog,
> "%s shader output `%s' specifies %s "
> "interpolation qualifier, "
> "but %s shader input specifies %s "
> "interpolation qualifier\n",
> _mesa_shader_stage_to_string(producer_stage),
> output->name,
> interpolation_string(output->data.interpolation),
> _mesa_shader_stage_to_string(consumer_stage),
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 64e68b4a26d..6712bb45fb2 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -1627,21 +1627,27 @@ validate_io(struct gl_program *producer, struct 
> gl_program *consumer)
> *Precision  |   mediump   |  Yes
> *   |highp|
> *---+-+--
> *Variance   |  invariant  |   No
> *---+-+--
> *Memory | all |  N/A
> *
> * Note that location mismatches

[Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

2017-09-26 Thread Nicolai Hähnle

From: Nicolai Hähnle 

It leads to surprising states with integer inputs and outputs on
vertex processing stages (e.g. geometry stages). Instead, rely on the
driver to choose smooth interpolation by default.

We still allow varyings to match when one stage declares it as smooth
and the other declares it without interpolation qualifiers.
--
This should cover all the relevant I/O checks, tested with dEQP.
---
 src/compiler/glsl/ast_to_hir.cpp| 11 ---
 src/compiler/glsl/link_varyings.cpp | 17 -
 src/mesa/main/shader_query.cpp  |  8 +++-
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index c46454956d7..1999e68158c 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3119,31 +3119,20 @@ interpret_interpolation_qualifier(const struct 
ast_type_qualifier *qual,
   struct _mesa_glsl_parse_state *state,
   YYLTYPE *loc)
 {
glsl_interp_mode interpolation;
if (qual->flags.q.flat)
   interpolation = INTERP_MODE_FLAT;
else if (qual->flags.q.noperspective)
   interpolation = INTERP_MODE_NOPERSPECTIVE;
else if (qual->flags.q.smooth)
   interpolation = INTERP_MODE_SMOOTH;
-   else if (state->es_shader &&
-((mode == ir_var_shader_in &&
-  state->stage != MESA_SHADER_VERTEX) ||
- (mode == ir_var_shader_out &&
-  state->stage != MESA_SHADER_FRAGMENT)))
-  /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
-   *
-   *"When no interpolation qualifier is present, smooth interpolation
-   *is used."
-   */
-  interpolation = INTERP_MODE_SMOOTH;
else
   interpolation = INTERP_MODE_NONE;
 
validate_interpolation_qualifier(state, loc,
 interpolation,
 qual, var_type, mode);
 
return interpolation;
 }
 
diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index 528506fd0eb..7c90de393ad 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -318,22 +318,37 @@ cross_validate_types_and_qualifiers(struct 
gl_shader_program *prog,
}
 
/* GLSL >= 4.40 removes text requiring interpolation qualifiers
 * to match cross stage, they must only match within the same stage.
 *
 * From page 84 (page 90 of the PDF) of the GLSL 4.40 spec:
 *
 * "It is a link-time error if, within the same stage, the interpolation
 * qualifiers of variables of the same name do not match.
 *
+* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
+*
+*"When no interpolation qualifier is present, smooth interpolation
+*is used."
+*
+* So we match variables where one is smooth and the other has no explicit
+* qualifier.
 */
-   if (input->data.interpolation != output->data.interpolation &&
+   unsigned input_interpolation = input->data.interpolation;
+   unsigned output_interpolation = output->data.interpolation;
+   if (prog->IsES) {
+  if (input_interpolation == INTERP_MODE_NONE)
+ input_interpolation = INTERP_MODE_SMOOTH;
+  if (output_interpolation == INTERP_MODE_NONE)
+ output_interpolation = INTERP_MODE_SMOOTH;
+   }
+   if (input_interpolation != output_interpolation &&
prog->data->Version < 440) {
   linker_error(prog,
"%s shader output `%s' specifies %s "
"interpolation qualifier, "
"but %s shader input specifies %s "
"interpolation qualifier\n",
_mesa_shader_stage_to_string(producer_stage),
output->name,
interpolation_string(output->data.interpolation),
_mesa_shader_stage_to_string(consumer_stage),
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 64e68b4a26d..6712bb45fb2 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -1627,21 +1627,27 @@ validate_io(struct gl_program *producer, struct 
gl_program *consumer)
*Precision  |   mediump   |  Yes
*   |highp|
*---+-+--
*Variance   |  invariant  |   No
*---+-+--
*Memory | all |  N/A
*
* Note that location mismatches are detected by the loops above that
* find the producer variable that goes with the consumer variable.
*/
-  if (producer_var->interpolation != consumer_var->interpolation) {
+  unsigned producer_interpolation = producer_var->interpolation;
+  unsigned consumer_interpolation = consumer_var->interpolation;
+  if

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

2017-09-13 Thread Timothy Arceri




On 14/09/17 01:22, Nicolai Hähnle wrote:

On 12.09.2017 03:33, Timothy Arceri wrote:



On 12/09/17 00:57, Nicolai Hähnle wrote:

On 11.09.2017 16:47, Ilia Mirkin wrote:
On Mon, Sep 11, 2017 at 10:44 AM, Nicolai Hähnle 
 wrote:

From: Nicolai Hähnle 

It breaks integer inputs and outputs on vertex processing stages
(e.g. geometry stages). Instead, rely on the driver to choose
smooth interpolation by default.
---

How about this instead? I haven't fully thought it through, but
that's where the INTERP_MODE_SMOOTH is coming from.

To be honest I'm not 100% sure about what INTERP_MODE_NONE can
mean, but I think it's definitely better than removing the (very
valid!) assertion in the packing code.


I couldn't understand what that assertion was asserting, esp in the
presence of struct in/out variables.


structs can't have per-member interpolation qualifiers. So a struct 
that contains integer values must have FLAT or NONE interpolation as 
a whole.


That clashes pretty hard with that ES rule, though ("When no 
interpolation qualifier is present, smooth interpolation is used.").
 From what I recall, NONE is just a placeholder so that the the driver 
knows to use the value set by glShadeModel(). ES doesn't have this and 
so we set this up front. I believe removing it will cause a bunch of 
tests to fail validation as they expect the default to be set and 
cross validated against stage interfaces.


So in a way, NONE just means "use the API default". The API default is 
set by glShadeModel() in compatibility profiles. In both ES and core 
profiles it's always "smooth" -- but only ES has this particular 
language about using "smooth" by default in its shading language spec.


Am I really the only one weirded out by seeing integer I/O that claims 
to use smooth interpolation?


I'd much rather not set smooth, as my patch proposes, and if there are 
tests that trigger linking errors, relax those to allow "smooth" and 
"none" to match.


They are CTS tests. Feel free to do it an alternative way but I don't 
think this patch alone is enough, the tests seem valid enough to me.






I'm also a bit confused as to what the assert() is trying to catch.


It's catching the weirdness of integer I/O that claims to use smooth 
interpolation. Admittedly, the comment is wrong and this is not related 
to varying packing, but I guarantee that people will be confused about 
this in the future.


Cheers,
Nicolai






On second thought, that assertion is actually irrelevant for varying 
packing itself, but it's still pretty disturbing that it's broken by 
this particular shader.


Cheers,
Nicolai






---
  src/compiler/glsl/ast_to_hir.cpp | 11 ---
  1 file changed, 11 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp 
b/src/compiler/glsl/ast_to_hir.cpp

index 98d2f94e129..cb42041642d 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3119,31 +3119,20 @@ interpret_interpolation_qualifier(const 
struct ast_type_qualifier *qual,
    struct _mesa_glsl_parse_state 
*state,

    YYLTYPE *loc)
  {
 glsl_interp_mode interpolation;
 if (qual->flags.q.flat)
    interpolation = INTERP_MODE_FLAT;
 else if (qual->flags.q.noperspective)
    interpolation = INTERP_MODE_NOPERSPECTIVE;
 else if (qual->flags.q.smooth)
    interpolation = INTERP_MODE_SMOOTH;
-   else if (state->es_shader &&
-    ((mode == ir_var_shader_in &&
-  state->stage != MESA_SHADER_VERTEX) ||
- (mode == ir_var_shader_out &&
-  state->stage != MESA_SHADER_FRAGMENT)))
-  /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
-   *
-   *    "When no interpolation qualifier is present, smooth 
interpolation

-   *    is used."
-   */
-  interpolation = INTERP_MODE_SMOOTH;
 else
    interpolation = INTERP_MODE_NONE;

 validate_interpolation_qualifier(state, loc,
  interpolation,
  qual, var_type, mode);

 return interpolation;
  }

--
2.11.0

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




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




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

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

2017-09-13 Thread Nicolai Hähnle


On 12.09.2017 03:33, Timothy Arceri wrote:



On 12/09/17 00:57, Nicolai Hähnle wrote:

On 11.09.2017 16:47, Ilia Mirkin wrote:
On Mon, Sep 11, 2017 at 10:44 AM, Nicolai Hähnle  
wrote:

From: Nicolai Hähnle 

It breaks integer inputs and outputs on vertex processing stages
(e.g. geometry stages). Instead, rely on the driver to choose
smooth interpolation by default.
---

How about this instead? I haven't fully thought it through, but
that's where the INTERP_MODE_SMOOTH is coming from.

To be honest I'm not 100% sure about what INTERP_MODE_NONE can
mean, but I think it's definitely better than removing the (very
valid!) assertion in the packing code.


I couldn't understand what that assertion was asserting, esp in the
presence of struct in/out variables.


structs can't have per-member interpolation qualifiers. So a struct 
that contains integer values must have FLAT or NONE interpolation as a 
whole.


That clashes pretty hard with that ES rule, though ("When no 
interpolation qualifier is present, smooth interpolation is used.").
 From what I recall, NONE is just a placeholder so that the the driver 
knows to use the value set by glShadeModel(). ES doesn't have this and 
so we set this up front. I believe removing it will cause a bunch of 
tests to fail validation as they expect the default to be set and cross 
validated against stage interfaces.


So in a way, NONE just means "use the API default". The API default is 
set by glShadeModel() in compatibility profiles. In both ES and core 
profiles it's always "smooth" -- but only ES has this particular 
language about using "smooth" by default in its shading language spec.


Am I really the only one weirded out by seeing integer I/O that claims 
to use smooth interpolation?


I'd much rather not set smooth, as my patch proposes, and if there are 
tests that trigger linking errors, relax those to allow "smooth" and 
"none" to match.




I'm also a bit confused as to what the assert() is trying to catch.


It's catching the weirdness of integer I/O that claims to use smooth 
interpolation. Admittedly, the comment is wrong and this is not related 
to varying packing, but I guarantee that people will be confused about 
this in the future.


Cheers,
Nicolai






On second thought, that assertion is actually irrelevant for varying 
packing itself, but it's still pretty disturbing that it's broken by 
this particular shader.


Cheers,
Nicolai






---
  src/compiler/glsl/ast_to_hir.cpp | 11 ---
  1 file changed, 11 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp 
b/src/compiler/glsl/ast_to_hir.cpp

index 98d2f94e129..cb42041642d 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3119,31 +3119,20 @@ interpret_interpolation_qualifier(const 
struct ast_type_qualifier *qual,
struct _mesa_glsl_parse_state 
*state,

YYLTYPE *loc)
  {
 glsl_interp_mode interpolation;
 if (qual->flags.q.flat)
interpolation = INTERP_MODE_FLAT;
 else if (qual->flags.q.noperspective)
interpolation = INTERP_MODE_NOPERSPECTIVE;
 else if (qual->flags.q.smooth)
interpolation = INTERP_MODE_SMOOTH;
-   else if (state->es_shader &&
-((mode == ir_var_shader_in &&
-  state->stage != MESA_SHADER_VERTEX) ||
- (mode == ir_var_shader_out &&
-  state->stage != MESA_SHADER_FRAGMENT)))
-  /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
-   *
-   *"When no interpolation qualifier is present, smooth 
interpolation

-   *is used."
-   */
-  interpolation = INTERP_MODE_SMOOTH;
 else
interpolation = INTERP_MODE_NONE;

 validate_interpolation_qualifier(state, loc,
  interpolation,
  qual, var_type, mode);

 return interpolation;
  }

--
2.11.0

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




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



--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

2017-09-11 Thread Timothy Arceri




On 12/09/17 00:57, Nicolai Hähnle wrote:

On 11.09.2017 16:47, Ilia Mirkin wrote:
On Mon, Sep 11, 2017 at 10:44 AM, Nicolai Hähnle  
wrote:

From: Nicolai Hähnle 

It breaks integer inputs and outputs on vertex processing stages
(e.g. geometry stages). Instead, rely on the driver to choose
smooth interpolation by default.
---

How about this instead? I haven't fully thought it through, but
that's where the INTERP_MODE_SMOOTH is coming from.

To be honest I'm not 100% sure about what INTERP_MODE_NONE can
mean, but I think it's definitely better than removing the (very
valid!) assertion in the packing code.


I couldn't understand what that assertion was asserting, esp in the
presence of struct in/out variables.


structs can't have per-member interpolation qualifiers. So a struct that 
contains integer values must have FLAT or NONE interpolation as a whole.


That clashes pretty hard with that ES rule, though ("When no 
interpolation qualifier is present, smooth interpolation is used.").
From what I recall, NONE is just a placeholder so that the the driver 
knows to use the value set by glShadeModel(). ES doesn't have this and 
so we set this up front. I believe removing it will cause a bunch of 
tests to fail validation as they expect the default to be set and cross 
validated against stage interfaces.


I'm also a bit confused as to what the assert() is trying to catch.



On second thought, that assertion is actually irrelevant for varying 
packing itself, but it's still pretty disturbing that it's broken by 
this particular shader.


Cheers,
Nicolai






---
  src/compiler/glsl/ast_to_hir.cpp | 11 ---
  1 file changed, 11 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp 
b/src/compiler/glsl/ast_to_hir.cpp

index 98d2f94e129..cb42041642d 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3119,31 +3119,20 @@ interpret_interpolation_qualifier(const 
struct ast_type_qualifier *qual,
    struct _mesa_glsl_parse_state 
*state,

    YYLTYPE *loc)
  {
 glsl_interp_mode interpolation;
 if (qual->flags.q.flat)
    interpolation = INTERP_MODE_FLAT;
 else if (qual->flags.q.noperspective)
    interpolation = INTERP_MODE_NOPERSPECTIVE;
 else if (qual->flags.q.smooth)
    interpolation = INTERP_MODE_SMOOTH;
-   else if (state->es_shader &&
-    ((mode == ir_var_shader_in &&
-  state->stage != MESA_SHADER_VERTEX) ||
- (mode == ir_var_shader_out &&
-  state->stage != MESA_SHADER_FRAGMENT)))
-  /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
-   *
-   *    "When no interpolation qualifier is present, smooth 
interpolation

-   *    is used."
-   */
-  interpolation = INTERP_MODE_SMOOTH;
 else
    interpolation = INTERP_MODE_NONE;

 validate_interpolation_qualifier(state, loc,
  interpolation,
  qual, var_type, mode);

 return interpolation;
  }

--
2.11.0

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




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

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

2017-09-11 Thread Ilia Mirkin

On Mon, Sep 11, 2017 at 10:57 AM, Nicolai Hähnle  wrote:
> On 11.09.2017 16:47, Ilia Mirkin wrote:
>>
>> On Mon, Sep 11, 2017 at 10:44 AM, Nicolai Hähnle 
>> wrote:
>>>
>>> From: Nicolai Hähnle 
>>>
>>> It breaks integer inputs and outputs on vertex processing stages
>>> (e.g. geometry stages). Instead, rely on the driver to choose
>>> smooth interpolation by default.
>>> ---
>>>
>>> How about this instead? I haven't fully thought it through, but
>>> that's where the INTERP_MODE_SMOOTH is coming from.
>>>
>>> To be honest I'm not 100% sure about what INTERP_MODE_NONE can
>>> mean, but I think it's definitely better than removing the (very
>>> valid!) assertion in the packing code.
>>
>>
>> I couldn't understand what that assertion was asserting, esp in the
>> presence of struct in/out variables.
>
>
> structs can't have per-member interpolation qualifiers. So a struct that
> contains integer values must have FLAT or NONE interpolation as a whole.
>
> That clashes pretty hard with that ES rule, though ("When no interpolation
> qualifier is present, smooth interpolation is used.").
>
> On second thought, that assertion is actually irrelevant for varying packing
> itself, but it's still pretty disturbing that it's broken by this particular
> shader.

I'm just saying... what exactly is that assert protecting against that
varying packing cares so much about... I guess it wants to avoid
packing an int with a smooth float into a single varying? But that's
handled at packing time when it decides which packed varying to stuff
it into.

As an aside, what about doubles or int64? Also I'm pretty sure you can
explicitly say "smooth" in non-frag shaders with little to no
consequence. But I haven't checked.

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

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

2017-09-11 Thread Nicolai Hähnle


On 11.09.2017 16:47, Ilia Mirkin wrote:

On Mon, Sep 11, 2017 at 10:44 AM, Nicolai Hähnle  wrote:

From: Nicolai Hähnle 

It breaks integer inputs and outputs on vertex processing stages
(e.g. geometry stages). Instead, rely on the driver to choose
smooth interpolation by default.
---

How about this instead? I haven't fully thought it through, but
that's where the INTERP_MODE_SMOOTH is coming from.

To be honest I'm not 100% sure about what INTERP_MODE_NONE can
mean, but I think it's definitely better than removing the (very
valid!) assertion in the packing code.


I couldn't understand what that assertion was asserting, esp in the
presence of struct in/out variables.


structs can't have per-member interpolation qualifiers. So a struct that 
contains integer values must have FLAT or NONE interpolation as a whole.


That clashes pretty hard with that ES rule, though ("When no 
interpolation qualifier is present, smooth interpolation is used.").


On second thought, that assertion is actually irrelevant for varying 
packing itself, but it's still pretty disturbing that it's broken by 
this particular shader.


Cheers,
Nicolai






---
  src/compiler/glsl/ast_to_hir.cpp | 11 ---
  1 file changed, 11 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 98d2f94e129..cb42041642d 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3119,31 +3119,20 @@ interpret_interpolation_qualifier(const struct 
ast_type_qualifier *qual,
struct _mesa_glsl_parse_state *state,
YYLTYPE *loc)
  {
 glsl_interp_mode interpolation;
 if (qual->flags.q.flat)
interpolation = INTERP_MODE_FLAT;
 else if (qual->flags.q.noperspective)
interpolation = INTERP_MODE_NOPERSPECTIVE;
 else if (qual->flags.q.smooth)
interpolation = INTERP_MODE_SMOOTH;
-   else if (state->es_shader &&
-((mode == ir_var_shader_in &&
-  state->stage != MESA_SHADER_VERTEX) ||
- (mode == ir_var_shader_out &&
-  state->stage != MESA_SHADER_FRAGMENT)))
-  /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
-   *
-   *"When no interpolation qualifier is present, smooth interpolation
-   *is used."
-   */
-  interpolation = INTERP_MODE_SMOOTH;
 else
interpolation = INTERP_MODE_NONE;

 validate_interpolation_qualifier(state, loc,
  interpolation,
  qual, var_type, mode);

 return interpolation;
  }

--
2.11.0

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



--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

2017-09-11 Thread Ilia Mirkin

On Mon, Sep 11, 2017 at 10:44 AM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> It breaks integer inputs and outputs on vertex processing stages
> (e.g. geometry stages). Instead, rely on the driver to choose
> smooth interpolation by default.
> ---
>
> How about this instead? I haven't fully thought it through, but
> that's where the INTERP_MODE_SMOOTH is coming from.
>
> To be honest I'm not 100% sure about what INTERP_MODE_NONE can
> mean, but I think it's definitely better than removing the (very
> valid!) assertion in the packing code.

I couldn't understand what that assertion was asserting, esp in the
presence of struct in/out variables.

>
> ---
>  src/compiler/glsl/ast_to_hir.cpp | 11 ---
>  1 file changed, 11 deletions(-)
>
> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
> b/src/compiler/glsl/ast_to_hir.cpp
> index 98d2f94e129..cb42041642d 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -3119,31 +3119,20 @@ interpret_interpolation_qualifier(const struct 
> ast_type_qualifier *qual,
>struct _mesa_glsl_parse_state *state,
>YYLTYPE *loc)
>  {
> glsl_interp_mode interpolation;
> if (qual->flags.q.flat)
>interpolation = INTERP_MODE_FLAT;
> else if (qual->flags.q.noperspective)
>interpolation = INTERP_MODE_NOPERSPECTIVE;
> else if (qual->flags.q.smooth)
>interpolation = INTERP_MODE_SMOOTH;
> -   else if (state->es_shader &&
> -((mode == ir_var_shader_in &&
> -  state->stage != MESA_SHADER_VERTEX) ||
> - (mode == ir_var_shader_out &&
> -  state->stage != MESA_SHADER_FRAGMENT)))
> -  /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
> -   *
> -   *"When no interpolation qualifier is present, smooth interpolation
> -   *is used."
> -   */
> -  interpolation = INTERP_MODE_SMOOTH;
> else
>interpolation = INTERP_MODE_NONE;
>
> validate_interpolation_qualifier(state, loc,
>  interpolation,
>  qual, var_type, mode);
>
> return interpolation;
>  }
>
> --
> 2.11.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

2017-09-11 Thread Nicolai Hähnle

From: Nicolai Hähnle 

It breaks integer inputs and outputs on vertex processing stages
(e.g. geometry stages). Instead, rely on the driver to choose
smooth interpolation by default.
---

How about this instead? I haven't fully thought it through, but
that's where the INTERP_MODE_SMOOTH is coming from.

To be honest I'm not 100% sure about what INTERP_MODE_NONE can
mean, but I think it's definitely better than removing the (very
valid!) assertion in the packing code.

---
 src/compiler/glsl/ast_to_hir.cpp | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 98d2f94e129..cb42041642d 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3119,31 +3119,20 @@ interpret_interpolation_qualifier(const struct 
ast_type_qualifier *qual,
   struct _mesa_glsl_parse_state *state,
   YYLTYPE *loc)
 {
glsl_interp_mode interpolation;
if (qual->flags.q.flat)
   interpolation = INTERP_MODE_FLAT;
else if (qual->flags.q.noperspective)
   interpolation = INTERP_MODE_NOPERSPECTIVE;
else if (qual->flags.q.smooth)
   interpolation = INTERP_MODE_SMOOTH;
-   else if (state->es_shader &&
-((mode == ir_var_shader_in &&
-  state->stage != MESA_SHADER_VERTEX) ||
- (mode == ir_var_shader_out &&
-  state->stage != MESA_SHADER_FRAGMENT)))
-  /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
-   *
-   *"When no interpolation qualifier is present, smooth interpolation
-   *is used."
-   */
-  interpolation = INTERP_MODE_SMOOTH;
else
   interpolation = INTERP_MODE_NONE;
 
validate_interpolation_qualifier(state, loc,
 interpolation,
 qual, var_type, mode);
 
return interpolation;
 }
 
-- 
2.11.0

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

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

[Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

Re: [Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

[Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

10 matches

Site Navigation

Mail list logo

Footer information