[Mesa-dev] [PATCH] st/mesa: fix default interpolation for colors. (v2)

[Brian Paul]

On 01/06/2012 06:57 AM, Dave Airlie wrote:
> From: Dave Airlie<airlied at 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<airlied at 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