[Mesa-dev] [PATCH 2/2] gallium/draw: fix two side handling

[Brian Paul]

On 12/19/2011 09:29 AM, Dave Airlie wrote:
> From: Dave Airlie<airlied at redhat.com>
>
> The previous fix linked the vertex/fragment shader handling in softpipe for
> the unspecified front color output case, but specified back color case.
>
> However in that case we were doing a copy from back to non-existant front,
> this code checks we have existant front/backs and only does the copy when
> they both exist.
>
> Signed-off-by: Dave Airlie<airlied at redhat.com>
> ---
>   src/gallium/auxiliary/draw/draw_pipe_twoside.c |   27 ++++++++----------------
>   1 files changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/src/gallium/auxiliary/draw/draw_pipe_twoside.c b/src/gallium/auxiliary/draw/draw_pipe_twoside.c
> index 9a3f3fe..b1a70a0 100644
> --- a/src/gallium/auxiliary/draw/draw_pipe_twoside.c
> +++ b/src/gallium/auxiliary/draw/draw_pipe_twoside.c
> @@ -38,8 +38,8 @@
>   struct twoside_stage {
>      struct draw_stage stage;
>      float sign;         /**<  +1 or -1 */
> -   uint attrib_front0, attrib_back0;
> -   uint attrib_front1, attrib_back1;
> +   int attrib_front0, attrib_back0;
> +   int attrib_front1, attrib_back1;
>   };
>
>
> @@ -48,9 +48,6 @@ static INLINE struct twoside_stage *twoside_stage( struct draw_stage *stage )
>      return (struct twoside_stage *)stage;
>   }
>
> -
> -
> -
>   /**
>    * Copy back color(s) to front color(s).
>    */
> @@ -60,12 +57,12 @@ copy_bfc( struct twoside_stage *twoside,
>             unsigned idx )
>   {
>      struct vertex_header *tmp = dup_vert(&twoside->stage, v, idx );
> -
> -   if (twoside->attrib_back0) {
> +
> +   if (twoside->attrib_back0>= 0&&  twoside->attrib_front0>= 0) {
>         COPY_4FV(tmp->data[twoside->attrib_front0],
>                  tmp->data[twoside->attrib_back0]);
>      }
> -   if (twoside->attrib_back1) {
> +   if (twoside->attrib_back1>= 0&&  twoside->attrib_front1>= 0) {
>         COPY_4FV(tmp->data[twoside->attrib_front1],
>                  tmp->data[twoside->attrib_back1]);
>      }
> @@ -109,10 +106,10 @@ static void twoside_first_tri( struct draw_stage *stage,
>      const struct draw_vertex_shader *vs = stage->draw->vs.vertex_shader;
>      uint i;
>
> -   twoside->attrib_front0 = 0;
> -   twoside->attrib_front1 = 0;
> -   twoside->attrib_back0 = 0;
> -   twoside->attrib_back1 = 0;
> +   twoside->attrib_front0 = -1;
> +   twoside->attrib_front1 = -1;
> +   twoside->attrib_back0 = -1;
> +   twoside->attrib_back1 = -1;
>
>      /* Find which vertex shader outputs are front/back colors */
>      for (i = 0; i<  vs->info.num_outputs; i++) {
> @@ -130,12 +127,6 @@ static void twoside_first_tri( struct draw_stage *stage,
>         }
>      }
>
> -   if (!twoside->attrib_back0)
> -      twoside->attrib_front0 = 0;
> -
> -   if (!twoside->attrib_back1)
> -      twoside->attrib_front1 = 0;
> -
>      /*
>       * We'll multiply the primitive's determinant by this sign to determine
>       * if the triangle is back-facing (negative).

This looks like a good fix in general.

Reviewed-by: Brian Paul <brianp at vmware.com>