[Mesa-dev] [PATCH 2/2] llvmpipe: Simplify vertex and geometry shaders.

Zack Rusin zackr at vmware.com
Mon Mar 24 12:49:03 PDT 2014


The series looks great to me.

----- Original Message -----
> From: José Fonseca <jfonseca at vmware.com>
> 
> Eliminate lp_vertex_shader, as it added nothing over draw_vertex_shader.
> 
> Simplify lp_geometry_shader, as most of the incoming state is unneeded.
> (We could also just use draw_geometry_shader if we were willing to peek
> inside the structure.)
> ---
>  src/gallium/drivers/llvmpipe/lp_context.h     |  4 +--
>  src/gallium/drivers/llvmpipe/lp_draw_arrays.c |  8 ++---
>  src/gallium/drivers/llvmpipe/lp_state.h       | 13 ++------
>  src/gallium/drivers/llvmpipe/lp_state_gs.c    | 32 +++++++------------
>  src/gallium/drivers/llvmpipe/lp_state_vs.c    | 46
>  +++++++--------------------
>  5 files changed, 33 insertions(+), 70 deletions(-)
> 
> diff --git a/src/gallium/drivers/llvmpipe/lp_context.h
> b/src/gallium/drivers/llvmpipe/lp_context.h
> index 05cdfe5..ee8033c 100644
> --- a/src/gallium/drivers/llvmpipe/lp_context.h
> +++ b/src/gallium/drivers/llvmpipe/lp_context.h
> @@ -46,8 +46,8 @@
>  struct llvmpipe_vbuf_render;
>  struct draw_context;
>  struct draw_stage;
> +struct draw_vertex_shader;
>  struct lp_fragment_shader;
> -struct lp_vertex_shader;
>  struct lp_blend_state;
>  struct lp_setup_context;
>  struct lp_setup_variant;
> @@ -63,7 +63,7 @@ struct llvmpipe_context {
>     const struct pipe_depth_stencil_alpha_state *depth_stencil;
>     const struct pipe_rasterizer_state *rasterizer;
>     struct lp_fragment_shader *fs;
> -   const struct lp_vertex_shader *vs;
> +   struct draw_vertex_shader *vs;
>     const struct lp_geometry_shader *gs;
>     const struct lp_velems_state *velems;
>     const struct lp_so_state *so;
> diff --git a/src/gallium/drivers/llvmpipe/lp_draw_arrays.c
> b/src/gallium/drivers/llvmpipe/lp_draw_arrays.c
> index 3df0a5c..99e6d19 100644
> --- a/src/gallium/drivers/llvmpipe/lp_draw_arrays.c
> +++ b/src/gallium/drivers/llvmpipe/lp_draw_arrays.c
> @@ -112,11 +112,11 @@ llvmpipe_draw_vbo(struct pipe_context *pipe, const
> struct pipe_draw_info *info)
>     llvmpipe_prepare_geometry_sampling(lp,
>                                        lp->num_sampler_views[PIPE_SHADER_GEOMETRY],
>                                        lp->sampler_views[PIPE_SHADER_GEOMETRY]);
> -   if (lp->gs && !lp->gs->shader.tokens) {
> +   if (lp->gs && lp->gs->no_tokens) {
>        /* we have an empty geometry shader with stream output, so
>           attach the stream output info to the current vertex shader */
>        if (lp->vs) {
> -         draw_vs_attach_so(lp->vs->draw_data,
> &lp->gs->shader.stream_output);
> +         draw_vs_attach_so(lp->vs, &lp->gs->stream_output);
>        }
>     }
>     draw_collect_pipeline_statistics(draw,
> @@ -136,11 +136,11 @@ llvmpipe_draw_vbo(struct pipe_context *pipe, const
> struct pipe_draw_info *info)
>     }
>     draw_set_mapped_so_targets(draw, 0, NULL);
>  
> -   if (lp->gs && !lp->gs->shader.tokens) {
> +   if (lp->gs && lp->gs->no_tokens) {
>        /* we have attached stream output to the vs for rendering,
>           now lets reset it */
>        if (lp->vs) {
> -         draw_vs_reset_so(lp->vs->draw_data);
> +         draw_vs_reset_so(lp->vs);
>        }
>     }
>     
> diff --git a/src/gallium/drivers/llvmpipe/lp_state.h
> b/src/gallium/drivers/llvmpipe/lp_state.h
> index 8635cf1..2da6caa 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state.h
> +++ b/src/gallium/drivers/llvmpipe/lp_state.h
> @@ -65,17 +65,10 @@ struct llvmpipe_context;
>  
>  
>  
> -/** Subclass of pipe_shader_state */
> -struct lp_vertex_shader
> -{
> -   struct pipe_shader_state shader;
> -   struct draw_vertex_shader *draw_data;
> -};
> -
> -/** Subclass of pipe_shader_state */
>  struct lp_geometry_shader {
> -   struct pipe_shader_state shader;
> -   struct draw_geometry_shader *draw_data;
> +   boolean no_tokens;
> +   struct pipe_stream_output_info stream_output;
> +   struct draw_geometry_shader *dgs;
>  };
>  
>  /** Vertex element state */
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_gs.c
> b/src/gallium/drivers/llvmpipe/lp_state_gs.c
> index 74cf992..c94afed 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_gs.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_gs.c
> @@ -48,7 +48,7 @@ llvmpipe_create_gs_state(struct pipe_context *pipe,
>  
>     state = CALLOC_STRUCT(lp_geometry_shader);
>     if (state == NULL )
> -      goto fail;
> +      goto no_state;
>  
>     /* debug */
>     if (LP_DEBUG & DEBUG_TGSI) {
> @@ -57,26 +57,19 @@ llvmpipe_create_gs_state(struct pipe_context *pipe,
>     }
>  
>     /* copy stream output info */
> -   state->shader = *templ;
> -   if (templ->tokens) {
> -      /* copy shader tokens, the ones passed in will go away. */
> -      state->shader.tokens = tgsi_dup_tokens(templ->tokens);
> -      if (state->shader.tokens == NULL)
> -         goto fail;
> -
> -      state->draw_data = draw_create_geometry_shader(llvmpipe->draw, templ);
> -      if (state->draw_data == NULL)
> -         goto fail;
> +   state->no_tokens = !templ->tokens;
> +   memcpy(&state->stream_output, &templ->stream_output, sizeof
> state->stream_output);
> +
> +   state->dgs = draw_create_geometry_shader(llvmpipe->draw, templ);
> +   if (state->dgs == NULL) {
> +      goto no_dgs;
>     }
>  
>     return state;
>  
> -fail:
> -   if (state) {
> -      FREE( (void *)state->shader.tokens );
> -      FREE( state->draw_data );
> -      FREE( state );
> -   }
> +no_dgs:
> +   FREE( state );
> +no_state:
>     return NULL;
>  }
>  
> @@ -89,7 +82,7 @@ llvmpipe_bind_gs_state(struct pipe_context *pipe, void *gs)
>     llvmpipe->gs = (struct lp_geometry_shader *)gs;
>  
>     draw_bind_geometry_shader(llvmpipe->draw,
> -                             (llvmpipe->gs ? llvmpipe->gs->draw_data :
> NULL));
> +                             (llvmpipe->gs ? llvmpipe->gs->dgs : NULL));
>  
>     llvmpipe->dirty |= LP_NEW_GS;
>  }
> @@ -107,8 +100,7 @@ llvmpipe_delete_gs_state(struct pipe_context *pipe, void
> *gs)
>        return;
>     }
>  
> -   draw_delete_geometry_shader(llvmpipe->draw, state->draw_data);
> -   FREE( (void *)state->shader.tokens );
> +   draw_delete_geometry_shader(llvmpipe->draw, state->dgs);
>     FREE(state);
>  }
>  
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_vs.c
> b/src/gallium/drivers/llvmpipe/lp_state_vs.c
> index 7efb81b..826ee5b 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_vs.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_vs.c
> @@ -43,36 +43,19 @@ llvmpipe_create_vs_state(struct pipe_context *pipe,
>                           const struct pipe_shader_state *templ)
>  {
>     struct llvmpipe_context *llvmpipe = llvmpipe_context(pipe);
> -   struct lp_vertex_shader *state;
> +   struct draw_vertex_shader *vs;
>  
> -   state = CALLOC_STRUCT(lp_vertex_shader);
> -   if (state == NULL )
> -      goto fail;
> -
> -   /* copy shader tokens, the ones passed in will go away.
> -    */
> -   state->shader.tokens = tgsi_dup_tokens(templ->tokens);
> -   if (state->shader.tokens == NULL)
> -      goto fail;
> -
> -   state->draw_data = draw_create_vertex_shader(llvmpipe->draw, templ);
> -   if (state->draw_data == NULL)
> -      goto fail;
> +   vs = draw_create_vertex_shader(llvmpipe->draw, templ);
> +   if (vs == NULL) {
> +      return NULL;
> +   }
>  
>     if (LP_DEBUG & DEBUG_TGSI) {
> -      debug_printf("llvmpipe: Create vertex shader %p:\n", (void *) state);
> +      debug_printf("llvmpipe: Create vertex shader %p:\n", (void *) vs);
>        tgsi_dump(templ->tokens, 0);
>     }
>  
> -   return state;
> -
> -fail:
> -   if (state) {
> -      FREE( (void *)state->shader.tokens );
> -      FREE( state->draw_data );
> -      FREE( state );
> -   }
> -   return NULL;
> +   return vs;
>  }
>  
>  
> @@ -80,13 +63,12 @@ static void
>  llvmpipe_bind_vs_state(struct pipe_context *pipe, void *_vs)
>  {
>     struct llvmpipe_context *llvmpipe = llvmpipe_context(pipe);
> -   const struct lp_vertex_shader *vs = (const struct lp_vertex_shader *)_vs;
> +   struct draw_vertex_shader *vs = (struct draw_vertex_shader *)_vs;
>  
>     if (llvmpipe->vs == vs)
>        return;
>  
> -   draw_bind_vertex_shader(llvmpipe->draw,
> -                           vs ? vs->draw_data : NULL);
> +   draw_bind_vertex_shader(llvmpipe->draw, vs);
>  
>     llvmpipe->vs = vs;
>  
> @@ -95,16 +77,12 @@ llvmpipe_bind_vs_state(struct pipe_context *pipe, void
> *_vs)
>  
>  
>  static void
> -llvmpipe_delete_vs_state(struct pipe_context *pipe, void *vs)
> +llvmpipe_delete_vs_state(struct pipe_context *pipe, void *_vs)
>  {
>     struct llvmpipe_context *llvmpipe = llvmpipe_context(pipe);
> +   struct draw_vertex_shader *vs = (struct draw_vertex_shader *)_vs;
>  
> -   struct lp_vertex_shader *state =
> -      (struct lp_vertex_shader *)vs;
> -
> -   draw_delete_vertex_shader(llvmpipe->draw, state->draw_data);
> -   FREE( (void *)state->shader.tokens );
> -   FREE( state );
> +   draw_delete_vertex_shader(llvmpipe->draw, vs);
>  }
>  
>  
> --
> 1.8.3.2
> 


More information about the mesa-dev mailing list