[Mesa-stable] [Mesa-dev] [PATCH] draw: fix line stippling

[Jose Fonseca]

On 13/03/16 20:40, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
>
> The logic was comparing actual ints, not true/false values.
> This meant that it was emitting always multiple line segments instead of just
> one even if the stipple test had the same result, which looks inefficient, and
> the segments also overlapped thus breaking line aa as well.
> (In practice, with the no-op default line stipple pattern, for a 10-pixel
> long line from 0-9 it was emitting 10 segments, with the individual segments
> ranging from 0-1, 0-2, 0-3 and so on.)
>
> This fixes https://bugs.freedesktop.org/show_bug.cgi?id=94193
>
> CC: <mesa-stable at lists.freedesktop.org>
> ---
>   src/gallium/auxiliary/draw/draw_pipe_stipple.c | 30 +++++++++++++-------------
>   1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/src/gallium/auxiliary/draw/draw_pipe_stipple.c b/src/gallium/auxiliary/draw/draw_pipe_stipple.c
> index dcf05aa..04854ad 100644
> --- a/src/gallium/auxiliary/draw/draw_pipe_stipple.c
> +++ b/src/gallium/auxiliary/draw/draw_pipe_stipple.c
> @@ -108,11 +108,11 @@ emit_segment(struct draw_stage *stage, struct prim_header *header,
>   }
>
>
> -static inline unsigned
> +static inline boolean
>   stipple_test(int counter, ushort pattern, int factor)
>   {
>      int b = (counter / factor) & 0xf;
> -   return (1 << b) & pattern;
> +   return !!((1 << b) & pattern);
>   }
>
>
> @@ -126,7 +126,7 @@ stipple_line(struct draw_stage *stage, struct prim_header *header)
>      const float *pos0 = v0->data[pos];
>      const float *pos1 = v1->data[pos];
>      float start = 0;
> -   int state = 0;
> +   boolean state = 0;
>
>      float x0 = pos0[0];
>      float x1 = pos1[0];
> @@ -143,29 +143,29 @@ stipple_line(struct draw_stage *stage, struct prim_header *header)
>         stipple->counter = 0;
>
>
> -   /* XXX ToDo: intead of iterating pixel-by-pixel, use a look-up table.
> +   /* XXX ToDo: instead of iterating pixel-by-pixel, use a look-up table.
>       */
>      for (i = 0; i < length; i++) {
> -      int result = stipple_test( (int) stipple->counter+i,
> -                                 (ushort) stipple->pattern, stipple->factor );
> +      boolean result = stipple_test((int)stipple->counter + i,
> +                                    (ushort)stipple->pattern, stipple->factor);
>         if (result != state) {
>            /* changing from "off" to "on" or vice versa */
> -	 if (state) {
> -	    if (start != i) {
> +         if (state) {
> +            if (start != i) {
>                  /* finishing an "on" segment */
> -	       emit_segment( stage, header, start / length, i / length );
> +               emit_segment(stage, header, start / length, i / length);
>               }
> -	 }
> -	 else {
> +         }
> +         else {
>               /* starting an "on" segment */
> -	    start = (float) i;
> -	 }
> -	 state = result;	
> +            start = (float)i;
> +         }
> +         state = result;
>         }
>      }
>
>      if (state && start < length)
> -      emit_segment( stage, header, start / length, 1.0 );
> +      emit_segment(stage, header, start / length, 1.0);
>
>      stipple->counter += length;
>   }
>

I think we're avoiding "boolean" over the standard C99 "bool".

Otherwise looks good.

Reviewed-by: Jose Fonseca <jfonseca at vmware.com>