Re: [Mesa-dev] [PATCH] st/mesa: fix default interpolation for colors. (v2)
On Fri, Jan 6, 2012 at 1:57 PM, Dave Airlie airl...@gmail.com wrote: From: Dave Airlie airl...@redhat.com Brian mentioned that mesa-demos/reflect was broken on softpipe, by my previous commit. The problem was were blindly translating none to perspective, when color/pntc at least need it linear. v2: no regressions version. use shademodel to pick what none means. Ignore this one, since shademodel should be passed into the drivers separately. The v1 is correct, the regression it finds is a bug elsewhere I'm nearly sure. Dave. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: fix default interpolation for colors. (v2)
On 01/06/2012 06:57 AM, Dave Airlie wrote:
From: Dave Airlieairl...@redhat.com
Brian mentioned that mesa-demos/reflect was broken on softpipe,
by my previous commit. The problem was were blindly translating none
to perspective, when color/pntc at least need it linear.
v2: no regressions version.
use shademodel to pick what none means.
Signed-off-by: Dave Airlieairl...@redhat.com
---
src/mesa/state_tracker/st_program.c | 19 ---
1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/mesa/state_tracker/st_program.c
b/src/mesa/state_tracker/st_program.c
index 146a9f3..1f6094e 100644
--- a/src/mesa/state_tracker/st_program.c
+++ b/src/mesa/state_tracker/st_program.c
@@ -436,10 +436,14 @@ st_get_vp_variant(struct st_context *st,
static unsigned
-st_translate_interp(enum glsl_interp_qualifier glsl_qual)
+st_translate_interp(enum glsl_interp_qualifier glsl_qual, bool is_color,
GLenum shade_model)
{
switch (glsl_qual) {
case INTERP_QUALIFIER_NONE:
+ if (is_color)
+ if (shade_model == GL_FLAT)
+return TGSI_INTERPOLATE_LINEAR;
+ return TGSI_INTERPOLATE_PERSPECTIVE;
This doesn't look right. If shade_mode == GL_FLAT, shouldn't we
return TGSI_INTERPOLATE_CONSTANT?
case INTERP_QUALIFIER_SMOOTH:
return TGSI_INTERPOLATE_PERSPECTIVE;
case INTERP_QUALIFIER_FLAT:
@@ -542,12 +546,14 @@ st_translate_fragment_program(struct st_context *st,
case FRAG_ATTRIB_COL0:
input_semantic_name[slot] = TGSI_SEMANTIC_COLOR;
input_semantic_index[slot] = 0;
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ TRUE,
st-ctx-Light.ShadeModel);
break;
case FRAG_ATTRIB_COL1:
input_semantic_name[slot] = TGSI_SEMANTIC_COLOR;
input_semantic_index[slot] = 1;
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ TRUE,
st-ctx-Light.ShadeModel);
break;
case FRAG_ATTRIB_FOGC:
input_semantic_name[slot] = TGSI_SEMANTIC_FOG;
@@ -601,10 +607,9 @@ st_translate_fragment_program(struct st_context *st,
assert(attr= FRAG_ATTRIB_TEX0);
input_semantic_index[slot] = (attr - FRAG_ATTRIB_TEX0);
input_semantic_name[slot] = TGSI_SEMANTIC_GENERIC;
- if (attr == FRAG_ATTRIB_PNTC)
- interpMode[slot] = TGSI_INTERPOLATE_LINEAR;
- else
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ (attr ==
FRAG_ATTRIB_PNTC),
+
st-ctx-Light.ShadeModel);
The ShadeModel value should only apply to color attibutes so it
shouldn't appear here in the texcoords/generic/point-coord case.
I think the code should read:
if (attr == FRAG_ATTRIB_PNTC)
interpMode[slot] = TGSI_INTERPOLATE_LINEAR;
else
interpMode[slot] =
st_translate_interp((stfp-Base.InterpQualifier[attr], false, 0);
break;
}
}
-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: fix default interpolation for colors. (v2)
On Fri, Jan 6, 2012 at 3:13 PM, Brian Paul bri...@vmware.com wrote:
On 01/06/2012 06:57 AM, Dave Airlie wrote:
From: Dave Airlieairl...@redhat.com
Brian mentioned that mesa-demos/reflect was broken on softpipe,
by my previous commit. The problem was were blindly translating none
to perspective, when color/pntc at least need it linear.
v2: no regressions version.
use shademodel to pick what none means.
Signed-off-by: Dave Airlieairl...@redhat.com
---
src/mesa/state_tracker/st_program.c | 19 ---
1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/mesa/state_tracker/st_program.c
b/src/mesa/state_tracker/st_program.c
index 146a9f3..1f6094e 100644
--- a/src/mesa/state_tracker/st_program.c
+++ b/src/mesa/state_tracker/st_program.c
@@ -436,10 +436,14 @@ st_get_vp_variant(struct st_context *st,
static unsigned
-st_translate_interp(enum glsl_interp_qualifier glsl_qual)
+st_translate_interp(enum glsl_interp_qualifier glsl_qual, bool is_color,
GLenum shade_model)
{
switch (glsl_qual) {
case INTERP_QUALIFIER_NONE:
+ if (is_color)
+ if (shade_model == GL_FLAT)
+ return TGSI_INTERPOLATE_LINEAR;
+ return TGSI_INTERPOLATE_PERSPECTIVE;
This doesn't look right. If shade_mode == GL_FLAT, shouldn't we return
TGSI_INTERPOLATE_CONSTANT?
Yeah the code is very wrong, I was confused by the fact that softpipe
perspective interp is broken and some piglit results.
case INTERP_QUALIFIER_SMOOTH:
return TGSI_INTERPOLATE_PERSPECTIVE;
case INTERP_QUALIFIER_FLAT:
@@ -542,12 +546,14 @@ st_translate_fragment_program(struct st_context *st,
case FRAG_ATTRIB_COL0:
input_semantic_name[slot] = TGSI_SEMANTIC_COLOR;
input_semantic_index[slot] = 0;
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ TRUE,
st-ctx-Light.ShadeModel);
break;
case FRAG_ATTRIB_COL1:
input_semantic_name[slot] = TGSI_SEMANTIC_COLOR;
input_semantic_index[slot] = 1;
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ TRUE,
st-ctx-Light.ShadeModel);
break;
case FRAG_ATTRIB_FOGC:
input_semantic_name[slot] = TGSI_SEMANTIC_FOG;
@@ -601,10 +607,9 @@ st_translate_fragment_program(struct st_context *st,
assert(attr= FRAG_ATTRIB_TEX0);
input_semantic_index[slot] = (attr - FRAG_ATTRIB_TEX0);
input_semantic_name[slot] = TGSI_SEMANTIC_GENERIC;
- if (attr == FRAG_ATTRIB_PNTC)
- interpMode[slot] = TGSI_INTERPOLATE_LINEAR;
- else
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ (attr ==
FRAG_ATTRIB_PNTC),
+
st-ctx-Light.ShadeModel);
The ShadeModel value should only apply to color attibutes so it shouldn't
appear here in the texcoords/generic/point-coord case.
I think the code should read:
if (attr == FRAG_ATTRIB_PNTC)
interpMode[slot] = TGSI_INTERPOLATE_LINEAR;
else
interpMode[slot] =
st_translate_interp((stfp-Base.InterpQualifier[attr], false, 0);
Yeah I'll probably just commit v1 + that change.
then I'll try and figure why softpipe gives different answer for
perspective than everyone else.
Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: fix default interpolation for colors. (v2)
On 01/06/2012 08:26 AM, Dave Airlie wrote:
On Fri, Jan 6, 2012 at 3:13 PM, Brian Paulbri...@vmware.com wrote:
On 01/06/2012 06:57 AM, Dave Airlie wrote:
From: Dave Airlieairl...@redhat.com
Brian mentioned that mesa-demos/reflect was broken on softpipe,
by my previous commit. The problem was were blindly translating none
to perspective, when color/pntc at least need it linear.
v2: no regressions version.
use shademodel to pick what none means.
Signed-off-by: Dave Airlieairl...@redhat.com
---
src/mesa/state_tracker/st_program.c | 19 ---
1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/mesa/state_tracker/st_program.c
b/src/mesa/state_tracker/st_program.c
index 146a9f3..1f6094e 100644
--- a/src/mesa/state_tracker/st_program.c
+++ b/src/mesa/state_tracker/st_program.c
@@ -436,10 +436,14 @@ st_get_vp_variant(struct st_context *st,
static unsigned
-st_translate_interp(enum glsl_interp_qualifier glsl_qual)
+st_translate_interp(enum glsl_interp_qualifier glsl_qual, bool is_color,
GLenum shade_model)
{
switch (glsl_qual) {
case INTERP_QUALIFIER_NONE:
+ if (is_color)
+ if (shade_model == GL_FLAT)
+return TGSI_INTERPOLATE_LINEAR;
+ return TGSI_INTERPOLATE_PERSPECTIVE;
This doesn't look right. If shade_mode == GL_FLAT, shouldn't we return
TGSI_INTERPOLATE_CONSTANT?
Yeah the code is very wrong, I was confused by the fact that softpipe
perspective interp is broken and some piglit results.
case INTERP_QUALIFIER_SMOOTH:
return TGSI_INTERPOLATE_PERSPECTIVE;
case INTERP_QUALIFIER_FLAT:
@@ -542,12 +546,14 @@ st_translate_fragment_program(struct st_context *st,
case FRAG_ATTRIB_COL0:
input_semantic_name[slot] = TGSI_SEMANTIC_COLOR;
input_semantic_index[slot] = 0;
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ TRUE,
st-ctx-Light.ShadeModel);
break;
case FRAG_ATTRIB_COL1:
input_semantic_name[slot] = TGSI_SEMANTIC_COLOR;
input_semantic_index[slot] = 1;
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ TRUE,
st-ctx-Light.ShadeModel);
break;
case FRAG_ATTRIB_FOGC:
input_semantic_name[slot] = TGSI_SEMANTIC_FOG;
@@ -601,10 +607,9 @@ st_translate_fragment_program(struct st_context *st,
assert(attr= FRAG_ATTRIB_TEX0);
input_semantic_index[slot] = (attr - FRAG_ATTRIB_TEX0);
input_semantic_name[slot] = TGSI_SEMANTIC_GENERIC;
- if (attr == FRAG_ATTRIB_PNTC)
- interpMode[slot] = TGSI_INTERPOLATE_LINEAR;
- else
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ (attr ==
FRAG_ATTRIB_PNTC),
+
st-ctx-Light.ShadeModel);
The ShadeModel value should only apply to color attibutes so it shouldn't
appear here in the texcoords/generic/point-coord case.
I think the code should read:
if (attr == FRAG_ATTRIB_PNTC)
interpMode[slot] = TGSI_INTERPOLATE_LINEAR;
else
interpMode[slot] =
st_translate_interp((stfp-Base.InterpQualifier[attr], false, 0);
Yeah I'll probably just commit v1 + that change.
then I'll try and figure why softpipe gives different answer for
perspective than everyone else.
Dave.
Looking forward, I think we'll eventually want to remove the
pipe_rasterizer_state::flatshade field and always use the fragment
shader interpolation qualifiers.
This would mean that if a shader was used both with
glShadeModel(GL_FLAT) and GL_SMOOTH we'd wind up with two variants of
the shader, but that should be rare.
-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: fix default interpolation for colors. (v2)
On 01/06/2012 04:35 PM, Brian Paul wrote:
On 01/06/2012 08:26 AM, Dave Airlie wrote:
On Fri, Jan 6, 2012 at 3:13 PM, Brian Paulbri...@vmware.com wrote:
On 01/06/2012 06:57 AM, Dave Airlie wrote:
From: Dave Airlieairl...@redhat.com
Brian mentioned that mesa-demos/reflect was broken on softpipe,
by my previous commit. The problem was were blindly translating none
to perspective, when color/pntc at least need it linear.
v2: no regressions version.
use shademodel to pick what none means.
Signed-off-by: Dave Airlieairl...@redhat.com
---
src/mesa/state_tracker/st_program.c | 19 ---
1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/mesa/state_tracker/st_program.c
b/src/mesa/state_tracker/st_program.c
index 146a9f3..1f6094e 100644
--- a/src/mesa/state_tracker/st_program.c
+++ b/src/mesa/state_tracker/st_program.c
@@ -436,10 +436,14 @@ st_get_vp_variant(struct st_context *st,
static unsigned
-st_translate_interp(enum glsl_interp_qualifier glsl_qual)
+st_translate_interp(enum glsl_interp_qualifier glsl_qual, bool
is_color,
GLenum shade_model)
{
switch (glsl_qual) {
case INTERP_QUALIFIER_NONE:
+ if (is_color)
+ if (shade_model == GL_FLAT)
+return TGSI_INTERPOLATE_LINEAR;
+ return TGSI_INTERPOLATE_PERSPECTIVE;
This doesn't look right. If shade_mode == GL_FLAT, shouldn't we return
TGSI_INTERPOLATE_CONSTANT?
Yeah the code is very wrong, I was confused by the fact that softpipe
perspective interp is broken and some piglit results.
case INTERP_QUALIFIER_SMOOTH:
return TGSI_INTERPOLATE_PERSPECTIVE;
case INTERP_QUALIFIER_FLAT:
@@ -542,12 +546,14 @@ st_translate_fragment_program(struct
st_context *st,
case FRAG_ATTRIB_COL0:
input_semantic_name[slot] = TGSI_SEMANTIC_COLOR;
input_semantic_index[slot] = 0;
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ TRUE,
st-ctx-Light.ShadeModel);
break;
case FRAG_ATTRIB_COL1:
input_semantic_name[slot] = TGSI_SEMANTIC_COLOR;
input_semantic_index[slot] = 1;
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ TRUE,
st-ctx-Light.ShadeModel);
break;
case FRAG_ATTRIB_FOGC:
input_semantic_name[slot] = TGSI_SEMANTIC_FOG;
@@ -601,10 +607,9 @@ st_translate_fragment_program(struct st_context
*st,
assert(attr= FRAG_ATTRIB_TEX0);
input_semantic_index[slot] = (attr -
FRAG_ATTRIB_TEX0);
input_semantic_name[slot] = TGSI_SEMANTIC_GENERIC;
- if (attr == FRAG_ATTRIB_PNTC)
- interpMode[slot] = TGSI_INTERPOLATE_LINEAR;
- else
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ (attr ==
FRAG_ATTRIB_PNTC),
+
st-ctx-Light.ShadeModel);
The ShadeModel value should only apply to color attibutes so it
shouldn't
appear here in the texcoords/generic/point-coord case.
I think the code should read:
if (attr == FRAG_ATTRIB_PNTC)
interpMode[slot] = TGSI_INTERPOLATE_LINEAR;
else
interpMode[slot] =
st_translate_interp((stfp-Base.InterpQualifier[attr], false, 0);
Yeah I'll probably just commit v1 + that change.
then I'll try and figure why softpipe gives different answer for
perspective than everyone else.
Dave.
Looking forward, I think we'll eventually want to remove the
pipe_rasterizer_state::flatshade field and always use the fragment
shader interpolation qualifiers.
This would mean that if a shader was used both with
glShadeModel(GL_FLAT) and GL_SMOOTH we'd wind up with two variants of
the shader, but that should be rare.
But all the radeon and NV GPUs have the shade model switch built-in,
they don't need 2 shader variants ...
-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/mesa: fix default interpolation for colors. (v2)
On Fri, Jan 6, 2012 at 4:50 PM, Christoph Bumiller
e0425...@student.tuwien.ac.at wrote:
On 01/06/2012 04:35 PM, Brian Paul wrote:
On 01/06/2012 08:26 AM, Dave Airlie wrote:
On Fri, Jan 6, 2012 at 3:13 PM, Brian Paulbri...@vmware.com wrote:
On 01/06/2012 06:57 AM, Dave Airlie wrote:
From: Dave Airlieairl...@redhat.com
Brian mentioned that mesa-demos/reflect was broken on softpipe,
by my previous commit. The problem was were blindly translating none
to perspective, when color/pntc at least need it linear.
v2: no regressions version.
use shademodel to pick what none means.
Signed-off-by: Dave Airlieairl...@redhat.com
---
src/mesa/state_tracker/st_program.c | 19 ---
1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/mesa/state_tracker/st_program.c
b/src/mesa/state_tracker/st_program.c
index 146a9f3..1f6094e 100644
--- a/src/mesa/state_tracker/st_program.c
+++ b/src/mesa/state_tracker/st_program.c
@@ -436,10 +436,14 @@ st_get_vp_variant(struct st_context *st,
static unsigned
-st_translate_interp(enum glsl_interp_qualifier glsl_qual)
+st_translate_interp(enum glsl_interp_qualifier glsl_qual, bool
is_color,
GLenum shade_model)
{
switch (glsl_qual) {
case INTERP_QUALIFIER_NONE:
+ if (is_color)
+ if (shade_model == GL_FLAT)
+ return TGSI_INTERPOLATE_LINEAR;
+ return TGSI_INTERPOLATE_PERSPECTIVE;
This doesn't look right. If shade_mode == GL_FLAT, shouldn't we return
TGSI_INTERPOLATE_CONSTANT?
Yeah the code is very wrong, I was confused by the fact that softpipe
perspective interp is broken and some piglit results.
case INTERP_QUALIFIER_SMOOTH:
return TGSI_INTERPOLATE_PERSPECTIVE;
case INTERP_QUALIFIER_FLAT:
@@ -542,12 +546,14 @@ st_translate_fragment_program(struct
st_context *st,
case FRAG_ATTRIB_COL0:
input_semantic_name[slot] = TGSI_SEMANTIC_COLOR;
input_semantic_index[slot] = 0;
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ TRUE,
st-ctx-Light.ShadeModel);
break;
case FRAG_ATTRIB_COL1:
input_semantic_name[slot] = TGSI_SEMANTIC_COLOR;
input_semantic_index[slot] = 1;
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ TRUE,
st-ctx-Light.ShadeModel);
break;
case FRAG_ATTRIB_FOGC:
input_semantic_name[slot] = TGSI_SEMANTIC_FOG;
@@ -601,10 +607,9 @@ st_translate_fragment_program(struct st_context
*st,
assert(attr= FRAG_ATTRIB_TEX0);
input_semantic_index[slot] = (attr -
FRAG_ATTRIB_TEX0);
input_semantic_name[slot] = TGSI_SEMANTIC_GENERIC;
- if (attr == FRAG_ATTRIB_PNTC)
- interpMode[slot] = TGSI_INTERPOLATE_LINEAR;
- else
- interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr]);
+ interpMode[slot] =
st_translate_interp(stfp-Base.InterpQualifier[attr],
+ (attr ==
FRAG_ATTRIB_PNTC),
+
st-ctx-Light.ShadeModel);
The ShadeModel value should only apply to color attibutes so it
shouldn't
appear here in the texcoords/generic/point-coord case.
I think the code should read:
if (attr == FRAG_ATTRIB_PNTC)
interpMode[slot] = TGSI_INTERPOLATE_LINEAR;
else
interpMode[slot] =
st_translate_interp((stfp-Base.InterpQualifier[attr], false, 0);
Yeah I'll probably just commit v1 + that change.
then I'll try and figure why softpipe gives different answer for
perspective than everyone else.
Dave.
Looking forward, I think we'll eventually want to remove the
pipe_rasterizer_state::flatshade field and always use the fragment
shader interpolation qualifiers.
This would mean that if a shader was used both with
glShadeModel(GL_FLAT) and GL_SMOOTH we'd wind up with two variants of
the shader, but that should be rare.
But all the radeon and NV GPUs have the shade model switch built-in,
they don't need 2 shader variants ...
I agree with this point. r300g ignores the TGSI interpolate modes,
because such state doesn't exist in hardware. It's a GL3 feature. All
I can do is to follow pipe_rasterizer_state::flatshade.
Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
