On 01/06/2012 08:26 AM, Dave Airlie wrote:
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 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.

Signed-off-by: Dave Airlie<airl...@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

Reply via email to