[Mesa-dev] [PATCH 2/2] draw: additional fix for the no-position case with llvm

[Brian Paul]

On 03/01/2013 04:52 PM, sroland at vmware.com wrote:
> From: Roland Scheidegger<sroland at vmware.com>
>
> Similar fix to what is done for the non-llvm case, we could otherwise still
> hit the stages (near certainly with gs) which crash. It is probably a much
> better idea to skip trying to draw at that point anyway.
> ---
>   .../draw/draw_pt_fetch_shade_pipeline_llvm.c       |   42 +++++++++++---------
>   1 file changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
> index b0c18ed..a8ddad6 100644
> --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
> @@ -300,31 +300,37 @@ llvm_pipeline_generic( struct draw_pt_middle_end *middle,
>         FREE(vert_info->verts);
>         vert_info =&gs_vert_info;
>         prim_info =&gs_prim_info;
> -
> -      clipped = draw_pt_post_vs_run( fpme->post_vs, vert_info );
> -
>      }
>
>      /* stream output needs to be done before clipping */
>      draw_pt_so_emit( fpme->so_emit,
> -		    vert_info,
> +                    vert_info,
>                       prim_info );
>
> -   if (clipped) {
> -      opt |= PT_PIPELINE;
> -   }
> -
> -   /* Do we need to run the pipeline? Now will come here if clipped
> +   /*
> +    * if there's no position, need to stop now, or the latter stages
> +    * will try to access non-existent position output.
>       */
> -   if (opt&  PT_PIPELINE) {
> -      pipeline( fpme,
> -                vert_info,
> -                prim_info );
> -   }
> -   else {
> -      emit( fpme->emit,
> -            vert_info,
> -            prim_info );
> +   if (draw_current_shader_position_output(draw) != -1) {
> +      if ((opt&  PT_SHADE)&&  gshader) {
> +         clipped = draw_pt_post_vs_run( fpme->post_vs, vert_info );
> +      }
> +      if (clipped) {
> +         opt |= PT_PIPELINE;
> +      }
> +
> +      /* Do we need to run the pipeline? Now will come here if clipped
> +       */
> +      if (opt&  PT_PIPELINE) {
> +         pipeline( fpme,
> +                   vert_info,
> +                   prim_info );
> +      }
> +      else {
> +         emit( fpme->emit,
> +               vert_info,
> +               prim_info );

Maybe it's just me, but I'd prefer to see these simple function calls 
have all their args on one line.


> +      }
>      }
>      FREE(vert_info->verts);
>   }

The series looks OK to me.

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