[Mesa-dev] [PATCH 2/2] llvmpipe: Simplify vertex and geometry shaders.
Zack Rusin
zackr at vmware.com
Wed Mar 26 08:19:44 PDT 2014
Actually Jose I think we'll need to revert this. That's because draw always assumed that if geometry shader is present it means that the geometry shader is present, but that is not true anymore. That's because d3d10 creates a null geometry shader to pass around the stream output. Before the patch the draw geometry shader was only created if tokens weren't null, with your change it always is and it's causing lots and lots of crashes because various parts are trying to execute a null geometry shader. Although I agree it'd be nice if we could handle it, I don't see a trivial way of fixing it.
z
----- Original Message -----
> 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