[Mesa-dev] [PATCH] softpipe: don't clamp or do logical operations on floating-point buffers.

[Brian Paul]

On 11/06/2011 05:31 AM, Morgan Armand wrote:
> ---
>   src/gallium/drivers/softpipe/sp_quad_blend.c |   78 ++++++++++++++++++-------
>   1 files changed, 56 insertions(+), 22 deletions(-)
>
> diff --git a/src/gallium/drivers/softpipe/sp_quad_blend.c b/src/gallium/drivers/softpipe/sp_quad_blend.c
> index 598df26..4813ada 100644
> --- a/src/gallium/drivers/softpipe/sp_quad_blend.c
> +++ b/src/gallium/drivers/softpipe/sp_quad_blend.c
> @@ -702,19 +702,19 @@ blend_quad(struct quad_stage *qs,
>       */
>      switch (softpipe->blend->rt[blend_index].rgb_func) {
>      case PIPE_BLEND_ADD:
> -      VEC4_ADD_SAT(quadColor[0], source[0], blend_dest[0]); /* R */
> -      VEC4_ADD_SAT(quadColor[1], source[1], blend_dest[1]); /* G */
> -      VEC4_ADD_SAT(quadColor[2], source[2], blend_dest[2]); /* B */
> +      VEC4_ADD(quadColor[0], source[0], blend_dest[0]); /* R */
> +      VEC4_ADD(quadColor[1], source[1], blend_dest[1]); /* G */
> +      VEC4_ADD(quadColor[2], source[2], blend_dest[2]); /* B */
>         break;
>      case PIPE_BLEND_SUBTRACT:
> -      VEC4_SUB_SAT(quadColor[0], source[0], blend_dest[0]); /* R */
> -      VEC4_SUB_SAT(quadColor[1], source[1], blend_dest[1]); /* G */
> -      VEC4_SUB_SAT(quadColor[2], source[2], blend_dest[2]); /* B */
> +      VEC4_SUB(quadColor[0], source[0], blend_dest[0]); /* R */
> +      VEC4_SUB(quadColor[1], source[1], blend_dest[1]); /* G */
> +      VEC4_SUB(quadColor[2], source[2], blend_dest[2]); /* B */
>         break;
>      case PIPE_BLEND_REVERSE_SUBTRACT:
> -      VEC4_SUB_SAT(quadColor[0], blend_dest[0], source[0]); /* R */
> -      VEC4_SUB_SAT(quadColor[1], blend_dest[1], source[1]); /* G */
> -      VEC4_SUB_SAT(quadColor[2], blend_dest[2], source[2]); /* B */
> +      VEC4_SUB(quadColor[0], blend_dest[0], source[0]); /* R */
> +      VEC4_SUB(quadColor[1], blend_dest[1], source[1]); /* G */
> +      VEC4_SUB(quadColor[2], blend_dest[2], source[2]); /* B */
>         break;
>      case PIPE_BLEND_MIN:
>         VEC4_MIN(quadColor[0], source[0], blend_dest[0]); /* R */
> @@ -735,13 +735,13 @@ blend_quad(struct quad_stage *qs,
>       */
>      switch (softpipe->blend->rt[blend_index].alpha_func) {
>      case PIPE_BLEND_ADD:
> -      VEC4_ADD_SAT(quadColor[3], source[3], blend_dest[3]); /* A */
> +      VEC4_ADD(quadColor[3], source[3], blend_dest[3]); /* A */
>         break;
>      case PIPE_BLEND_SUBTRACT:
> -      VEC4_SUB_SAT(quadColor[3], source[3], blend_dest[3]); /* A */
> +      VEC4_SUB(quadColor[3], source[3], blend_dest[3]); /* A */
>         break;
>      case PIPE_BLEND_REVERSE_SUBTRACT:
> -      VEC4_SUB_SAT(quadColor[3], blend_dest[3], source[3]); /* A */
> +      VEC4_SUB(quadColor[3], blend_dest[3], source[3]); /* A */
>         break;
>      case PIPE_BLEND_MIN:
>         VEC4_MIN(quadColor[3], source[3], blend_dest[3]); /* A */
> @@ -856,6 +856,10 @@ blend_fallback(struct quad_stage *qs,
>
>      for (cbuf = 0; cbuf<  softpipe->framebuffer.nr_cbufs; cbuf++)
>      {
> +      const enum pipe_format format =
> +         softpipe->framebuffer.cbufs[cbuf]->format;
> +      const struct util_format_description *desc =
> +         util_format_description(format);

I'd suggest doing the format query in choose_blend_quad() and saving 
the results to an array as we do for the clamp[] values.  The 
util_format_description() call involves a big switch statement so it's 
good to avoid calling it frequently.

Otherwise this looks OK to me.

-Brian