[Mesa-dev] [PATCH 2/2] llvmpipe: Simplify vertex and geometry shaders.
Jose Fonseca
jfonseca at vmware.com
Wed Mar 26 08:54:40 PDT 2014
I see the crashes you're referring to.
I don't quite understand why though: concerning the geometry shader, other than cosmetic changes, in theory I should just have replaced a null/non-null `tokens` pointer with a boolean `no_tokens`, though obviously I missed something.
I should also had broken this in two separate changes: vs portion, and gs portion.
Jose
----- Original Message -----
> 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