Good catch Brian. Change looks good to me AFAICT. We do need more this sort of assertions in the draw module. I never fail to get surprised at how intricate and sensitive draw module is -- mostly due the fact that it reaches out to the upstream gallium interface from within the driver. I do hope one day the state trackers just use geometry shaders for all those things, so that the draw module is a straighforward VS/GS processor, and does not need to create its own fragment sahders. (It is unfortunate that with util/u_blitter we replicate the bad example of reaching out to gallium interface from inside the driver.)
Jose ----- Original Message ----- > We were calling draw_total_vs_outputs() too early. The call to > draw_pt_emit_prepare() could result in the vertex size changing. > So call draw_total_vs_outputs() after draw_pt_emit_prepare(). > > This fix would seem to be needed for the non-LLVM code as well, > but it's not obvious. Instead, I added an assertion there to > try to catch this problem if it were to occur there. > > Bugzilla: > https://urldefense.proofpoint.com/v1/url?u=https://bugs.freedesktop.org/show_bug.cgi?id%3D72926&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=NMr9uy2iTjWVixC0wOcYCWEIYhfo80qKwRgdodpoDzA%3D%0A&m=91zldQ8lO5OhCBRVy4vcYnfNUSKL3Bo1lcxmHhpMqWQ%3D%0A&s=2dc25b5d2d9cdcc89cb242f8bdd09f59761026dd200cea7eb9dd3278928e3603 > Cc: 10.0 <mesa-sta...@lists.freedesktop.org> > --- > .../auxiliary/draw/draw_pt_fetch_shade_pipeline.c | 7 +++++-- > .../draw/draw_pt_fetch_shade_pipeline_llvm.c | 22 > ++++++++++++-------- > 2 files changed, 18 insertions(+), 11 deletions(-) > > diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c > b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c > index 8fcc170..2c5c4cd 100644 > --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c > +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c > @@ -72,8 +72,8 @@ static void fetch_pipeline_prepare( struct > draw_pt_middle_end *middle, > > const unsigned gs_out_prim = (gs ? gs->output_primitive : > u_assembled_prim(prim)); > - unsigned nr = MAX2( vs->info.num_inputs, > - draw_total_vs_outputs(draw) ); > + unsigned nr_vs_outputs = draw_total_vs_outputs(draw); > + unsigned nr = MAX2(vs->info.num_inputs, nr_vs_outputs); > > if (gs) { > nr = MAX2(nr, gs->info.num_outputs + 1); > @@ -129,6 +129,9 @@ static void fetch_pipeline_prepare( struct > draw_pt_middle_end *middle, > /* No need to prepare the shader. > */ > vs->prepare(vs, draw); > + > + /* Make sure that the vertex size didn't change at any point above */ > + assert(nr_vs_outputs == draw_total_vs_outputs(draw)); > } > > > 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 9f17241..60ec528 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 > @@ -141,19 +141,11 @@ llvm_middle_end_prepare( struct draw_pt_middle_end > *middle, > struct draw_geometry_shader *gs = draw->gs.geometry_shader; > const unsigned out_prim = gs ? gs->output_primitive : > u_assembled_prim(in_prim); > - const unsigned nr = MAX2(vs->info.num_inputs, > - draw_total_vs_outputs(draw)); > + unsigned nr; > > fpme->input_prim = in_prim; > fpme->opt = opt; > > - /* Always leave room for the vertex header whether we need it or > - * not. It's hard to get rid of it in particular because of the > - * viewport code in draw_pt_post_vs.c. > - */ > - fpme->vertex_size = sizeof(struct vertex_header) + nr * 4 * > sizeof(float); > - > - > draw_pt_post_vs_prepare( fpme->post_vs, > draw->clip_xy, > draw->clip_z, > @@ -177,6 +169,18 @@ llvm_middle_end_prepare( struct draw_pt_middle_end > *middle, > *max_vertices = 4096; > } > > + /* Get the number of float[4] attributes per vertex. > + * Note: this must be done after draw_pt_emit_prepare() since that > + * can effect the vertex size. > + */ > + nr = MAX2(vs->info.num_inputs, draw_total_vs_outputs(draw)); > + > + /* Always leave room for the vertex header whether we need it or > + * not. It's hard to get rid of it in particular because of the > + * viewport code in draw_pt_post_vs.c. > + */ > + fpme->vertex_size = sizeof(struct vertex_header) + nr * 4 * > sizeof(float); > + > /* return even number */ > *max_vertices = *max_vertices & ~1; > > -- > 1.7.10.4 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=NMr9uy2iTjWVixC0wOcYCWEIYhfo80qKwRgdodpoDzA%3D%0A&m=91zldQ8lO5OhCBRVy4vcYnfNUSKL3Bo1lcxmHhpMqWQ%3D%0A&s=beda6ecc8e4e1c8c7778d049ee75198ed00145f1ec140f1a3233a2dc5a93c732 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
