Re: [Mesa-dev] [PATCH 2/2] glsl: set user defined varyings to smooth by default

2016-02-16 Thread Ian Romanick
On 02/15/2016 07:12 AM, Iago Toral wrote:
> On Mon, 2016-02-15 at 18:38 +1100, Timothy Arceri wrote:
>> This is usually handled by the backends in order to handle the
>> various interactions with the gl_*Color built-ins.
>>
>> The problem is this means linking will fail if one side on the
>> interface adds the smooth qualifier to the varying and the other
>> side just uses the default even though they match.
>>
>> This fixes various deqp tests and should have no impact on
>> built-ins as they generate GLSL IR directly.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92743
>> ---
>>  src/compiler/glsl/ast_to_hir.cpp | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
>> b/src/compiler/glsl/ast_to_hir.cpp
>> index b639378..47d52ee 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -2750,6 +2750,11 @@ interpret_interpolation_qualifier(const struct 
>> ast_type_qualifier *qual,
>>"vertex shader inputs or fragment shader outputs",
>>interpolation_string(interpolation));
>>}
>> +   } else if ((mode == ir_var_shader_in &&
>> +   state->stage != MESA_SHADER_VERTEX) ||
>> +  (mode == ir_var_shader_out &&
>> +   state->stage != MESA_SHADER_FRAGMENT)) {
>> +  interpolation = INTERP_QUALIFIER_SMOOTH;
>> }
> 
> The GLES spec explicitly says that in the absence of an interp qualifier
> smooth is used, but I can't find the same statement in the desktop GLSL
> spec. Should we make this ES specific?

Desktop OpenGL has an API control (via glShadeModel) that OpenGL ES 2.0+
does not have.  If a compatibility profile vertex shader outputs
built-in varyings (or maybe just color... I'd have to look) to
fixed-function, the interpolation mode is determined by the shading
model (GL_FLAT or GL_SMOOTH).

> Iago
> 
> ___
> 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 2/2] glsl: set user defined varyings to smooth by default

2016-02-15 Thread Timothy Arceri
On Tue, 2016-02-16 at 09:04 +1100, Timothy Arceri wrote:
> On Mon, 2016-02-15 at 16:12 +0100, Iago Toral wrote:
> > On Mon, 2016-02-15 at 18:38 +1100, Timothy Arceri wrote:
> > > This is usually handled by the backends in order to handle the
> > > various interactions with the gl_*Color built-ins.
> > > 
> > > The problem is this means linking will fail if one side on the
> > > interface adds the smooth qualifier to the varying and the other
> > > side just uses the default even though they match.
> > > 
> > > This fixes various deqp tests and should have no impact on
> > > built-ins as they generate GLSL IR directly.
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92743
> > > ---
> > >  src/compiler/glsl/ast_to_hir.cpp | 5 +
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/src/compiler/glsl/ast_to_hir.cpp
> > > b/src/compiler/glsl/ast_to_hir.cpp
> > > index b639378..47d52ee 100644
> > > --- a/src/compiler/glsl/ast_to_hir.cpp
> > > +++ b/src/compiler/glsl/ast_to_hir.cpp
> > > @@ -2750,6 +2750,11 @@ interpret_interpolation_qualifier(const
> > > struct ast_type_qualifier *qual,
> > >    "vertex shader inputs or fragment
> > > shader
> > > outputs",
> > >    interpolation_string(interpolation));
> > >    }
> > > +   } else if ((mode == ir_var_shader_in &&
> > > +   state->stage != MESA_SHADER_VERTEX) ||
> > > +  (mode == ir_var_shader_out &&
> > > +   state->stage != MESA_SHADER_FRAGMENT)) {
> > > +  interpolation = INTERP_QUALIFIER_SMOOTH;
> > > }
> > 
> > The GLES spec explicitly says that in the absence of an interp
> > qualifier
> > smooth is used, but I can't find the same statement in the desktop
> > GLSL
> > spec. Should we make this ES specific?
> 
> I couldn't find it in the spec either thats why I didn't send this
> out
> last year when I wrote it. However the OpenGL wiki says thats what it
> is by default, and thats what our implementation does after
> validation
> 
> I'll write a piglit test to see what AMD and Nvidia do on the desktop
> just to be sure.

They are inconsitent even not failing to link when they should so I
just sent a patch to change this for ES.

> 
> Thanks for taking a look.
> 
> > 
> > Iago
> > 
> > ___
> > 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 2/2] glsl: set user defined varyings to smooth by default

2016-02-15 Thread Timothy Arceri
On Mon, 2016-02-15 at 16:12 +0100, Iago Toral wrote:
> On Mon, 2016-02-15 at 18:38 +1100, Timothy Arceri wrote:
> > This is usually handled by the backends in order to handle the
> > various interactions with the gl_*Color built-ins.
> > 
> > The problem is this means linking will fail if one side on the
> > interface adds the smooth qualifier to the varying and the other
> > side just uses the default even though they match.
> > 
> > This fixes various deqp tests and should have no impact on
> > built-ins as they generate GLSL IR directly.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92743
> > ---
> >  src/compiler/glsl/ast_to_hir.cpp | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/src/compiler/glsl/ast_to_hir.cpp
> > b/src/compiler/glsl/ast_to_hir.cpp
> > index b639378..47d52ee 100644
> > --- a/src/compiler/glsl/ast_to_hir.cpp
> > +++ b/src/compiler/glsl/ast_to_hir.cpp
> > @@ -2750,6 +2750,11 @@ interpret_interpolation_qualifier(const
> > struct ast_type_qualifier *qual,
> >    "vertex shader inputs or fragment shader
> > outputs",
> >    interpolation_string(interpolation));
> >    }
> > +   } else if ((mode == ir_var_shader_in &&
> > +   state->stage != MESA_SHADER_VERTEX) ||
> > +  (mode == ir_var_shader_out &&
> > +   state->stage != MESA_SHADER_FRAGMENT)) {
> > +  interpolation = INTERP_QUALIFIER_SMOOTH;
> > }
> 
> The GLES spec explicitly says that in the absence of an interp
> qualifier
> smooth is used, but I can't find the same statement in the desktop
> GLSL
> spec. Should we make this ES specific?

I couldn't find it in the spec either thats why I didn't send this out
last year when I wrote it. However the OpenGL wiki says thats what it
is by default, and thats what our implementation does after validation

I'll write a piglit test to see what AMD and Nvidia do on the desktop
just to be sure.

Thanks for taking a look.

> 
> Iago
> 
> ___
> 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 2/2] glsl: set user defined varyings to smooth by default

2016-02-15 Thread Iago Toral
On Mon, 2016-02-15 at 18:38 +1100, Timothy Arceri wrote:
> This is usually handled by the backends in order to handle the
> various interactions with the gl_*Color built-ins.
> 
> The problem is this means linking will fail if one side on the
> interface adds the smooth qualifier to the varying and the other
> side just uses the default even though they match.
> 
> This fixes various deqp tests and should have no impact on
> built-ins as they generate GLSL IR directly.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92743
> ---
>  src/compiler/glsl/ast_to_hir.cpp | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
> b/src/compiler/glsl/ast_to_hir.cpp
> index b639378..47d52ee 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -2750,6 +2750,11 @@ interpret_interpolation_qualifier(const struct 
> ast_type_qualifier *qual,
>"vertex shader inputs or fragment shader outputs",
>interpolation_string(interpolation));
>}
> +   } else if ((mode == ir_var_shader_in &&
> +   state->stage != MESA_SHADER_VERTEX) ||
> +  (mode == ir_var_shader_out &&
> +   state->stage != MESA_SHADER_FRAGMENT)) {
> +  interpolation = INTERP_QUALIFIER_SMOOTH;
> }

The GLES spec explicitly says that in the absence of an interp qualifier
smooth is used, but I can't find the same statement in the desktop GLSL
spec. Should we make this ES specific?

Iago

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


[Mesa-dev] [PATCH 2/2] glsl: set user defined varyings to smooth by default

2016-02-14 Thread Timothy Arceri
This is usually handled by the backends in order to handle the
various interactions with the gl_*Color built-ins.

The problem is this means linking will fail if one side on the
interface adds the smooth qualifier to the varying and the other
side just uses the default even though they match.

This fixes various deqp tests and should have no impact on
built-ins as they generate GLSL IR directly.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92743
---
 src/compiler/glsl/ast_to_hir.cpp | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index b639378..47d52ee 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -2750,6 +2750,11 @@ interpret_interpolation_qualifier(const struct 
ast_type_qualifier *qual,
   "vertex shader inputs or fragment shader outputs",
   interpolation_string(interpolation));
   }
+   } else if ((mode == ir_var_shader_in &&
+   state->stage != MESA_SHADER_VERTEX) ||
+  (mode == ir_var_shader_out &&
+   state->stage != MESA_SHADER_FRAGMENT)) {
+  interpolation = INTERP_QUALIFIER_SMOOTH;
}
 
return interpolation;
-- 
2.5.0

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