Am 02.08.2013 08:28, schrieb Zack Rusin: > Draw sometimes injects extra shader outputs (aa points, lines or > front face), unfortunately most of the pipeline and llvm code > didn't handle them at all. It only worked if number of inputs > happened to be bigger or equal to the number of shader outputs > plus the extra injected outputs. In particular when running > the pipeline which depends on the vertex_id in the vertex_header > things were completely broken. The patch adjust the code to > correctly use the total number of shader outputs (the standard > ones plus the injected ones) to make it all stop crashing and > work. > > Signed-off-by: Zack Rusin <za...@vmware.com> > --- > src/gallium/auxiliary/draw/draw_context.c | 43 > ++++++++++++++++++++ > src/gallium/auxiliary/draw/draw_context.h | 5 +++ > src/gallium/auxiliary/draw/draw_gs.c | 2 +- > src/gallium/auxiliary/draw/draw_llvm.c | 3 ++ > src/gallium/auxiliary/draw/draw_llvm.h | 4 +- > src/gallium/auxiliary/draw/draw_pipe.h | 3 +- > .../auxiliary/draw/draw_pt_fetch_shade_pipeline.c | 6 +-- > .../draw/draw_pt_fetch_shade_pipeline_llvm.c | 8 +--- > src/gallium/auxiliary/draw/draw_vs_variant.c | 2 +- > 9 files changed, 61 insertions(+), 15 deletions(-) > > diff --git a/src/gallium/auxiliary/draw/draw_context.c > b/src/gallium/auxiliary/draw/draw_context.c > index 2e95b5c..8bf3596 100644 > --- a/src/gallium/auxiliary/draw/draw_context.c > +++ b/src/gallium/auxiliary/draw/draw_context.c > @@ -622,6 +622,49 @@ draw_num_shader_outputs(const struct draw_context *draw) > > > /** > + * Return total number of the vertex shader outputs. This function > + * also counts any extra vertex output attributes that may > + * be filled in by some draw stages (such as AA point, AA line, > + * front face). > + */ > +uint > +draw_total_vs_shader_outputs(const struct draw_context *draw) > +{ > + const struct tgsi_shader_info *info = &draw->vs.vertex_shader->info; > + uint count; > + > + count = info->num_outputs; > + count += draw->extra_shader_outputs.num; > + > + return count; > +} > + > +/** > + * Return total number of the geometry shader outputs. This function > + * also counts any extra geometry output attributes that may > + * be filled in by some draw stages (such as AA point, AA line, front > + * face). > + */ > +uint > +draw_total_gs_shader_outputs(const struct draw_context *draw) > +{ > + > + const struct tgsi_shader_info *info; > + uint count; > + > + if (!draw->gs.geometry_shader) > + return 0; > + > + info = &draw->gs.geometry_shader->info; > + > + count = info->num_outputs; > + count += draw->extra_shader_outputs.num; > + > + return count; > +} > + > + > +/** > * Provide TGSI sampler objects for vertex/geometry shaders that use > * texture fetches. This state only needs to be set once per context. > * This might only be used by software drivers for the time being. > diff --git a/src/gallium/auxiliary/draw/draw_context.h > b/src/gallium/auxiliary/draw/draw_context.h > index 0815047..e9aa24d 100644 > --- a/src/gallium/auxiliary/draw/draw_context.h > +++ b/src/gallium/auxiliary/draw/draw_context.h > @@ -139,6 +139,11 @@ draw_will_inject_frontface(const struct draw_context > *draw); > uint > draw_num_shader_outputs(const struct draw_context *draw); > > +uint > +draw_total_vs_shader_outputs(const struct draw_context *draw); > + > +uint > +draw_total_gs_shader_outputs(const struct draw_context *draw); The "shader" in the the name is kinda redundant with "vs" and "gs" in there, so it could be shortened a bit. Doesn't really matter though.
> > void > draw_texture_sampler(struct draw_context *draw, > diff --git a/src/gallium/auxiliary/draw/draw_gs.c > b/src/gallium/auxiliary/draw/draw_gs.c > index cd63e2b..32fd91f 100644 > --- a/src/gallium/auxiliary/draw/draw_gs.c > +++ b/src/gallium/auxiliary/draw/draw_gs.c > @@ -534,7 +534,7 @@ int draw_geometry_shader_run(struct draw_geometry_shader > *shader, > { > const float (*input)[4] = (const float (*)[4])input_verts->verts->data; > unsigned input_stride = input_verts->vertex_size; > - unsigned num_outputs = shader->info.num_outputs; > + unsigned num_outputs = draw_total_gs_shader_outputs(shader->draw); > unsigned vertex_size = sizeof(struct vertex_header) + num_outputs * 4 * > sizeof(float); > unsigned num_input_verts = input_prim->linear ? > input_verts->count : > diff --git a/src/gallium/auxiliary/draw/draw_llvm.c > b/src/gallium/auxiliary/draw/draw_llvm.c > index c195a2b..8ecb3e7 100644 > --- a/src/gallium/auxiliary/draw/draw_llvm.c > +++ b/src/gallium/auxiliary/draw/draw_llvm.c > @@ -1827,6 +1827,7 @@ draw_llvm_make_variant_key(struct draw_llvm *llvm, char > *store) > key->need_edgeflags = (llvm->draw->vs.edgeflag_output ? TRUE : FALSE); > key->ucp_enable = llvm->draw->rasterizer->clip_plane_enable; > key->has_gs = llvm->draw->gs.geometry_shader != NULL; > + key->num_outputs = draw_total_vs_shader_outputs(llvm->draw); > key->pad1 = 0; > > /* All variants of this shader will have the same value for > @@ -2264,6 +2265,8 @@ draw_gs_llvm_make_variant_key(struct draw_llvm *llvm, > char *store) > > key = (struct draw_gs_llvm_variant_key *)store; > > + key->num_outputs = draw_total_gs_shader_outputs(llvm->draw); > + > /* All variants of this shader will have the same value for > * nr_samplers. Not yet trying to compact away holes in the > * sampler array. > diff --git a/src/gallium/auxiliary/draw/draw_llvm.h > b/src/gallium/auxiliary/draw/draw_llvm.h > index 0675e3b..1d238a2 100644 > --- a/src/gallium/auxiliary/draw/draw_llvm.h > +++ b/src/gallium/auxiliary/draw/draw_llvm.h > @@ -301,12 +301,13 @@ struct draw_llvm_variant_key > unsigned bypass_viewport:1; > unsigned need_edgeflags:1; > unsigned has_gs:1; > + unsigned num_outputs:8; > /* > * it is important there are no holes in this struct > * (and all padding gets zeroed). > */ > unsigned ucp_enable:PIPE_MAX_CLIP_PLANES; > - unsigned pad1:32-PIPE_MAX_CLIP_PLANES; > + unsigned pad1:24-PIPE_MAX_CLIP_PLANES; > > /* Variable number of vertex elements: > */ > @@ -321,6 +322,7 @@ struct draw_gs_llvm_variant_key > { > unsigned nr_samplers:8; > unsigned nr_sampler_views:8; > + unsigned num_outputs:8; Hmm just realizing the same key for vs says it is important there aren't any holes whereas this one does not. > struct draw_sampler_static_state samplers[1]; > }; > diff --git a/src/gallium/auxiliary/draw/draw_pipe.h > b/src/gallium/auxiliary/draw/draw_pipe.h > index 2e48b56..70c286f 100644 > --- a/src/gallium/auxiliary/draw/draw_pipe.h > +++ b/src/gallium/auxiliary/draw/draw_pipe.h > @@ -35,6 +35,7 @@ > > #include "pipe/p_compiler.h" > #include "draw_private.h" /* for sizeof(vertex_header) */ > +#include "draw_context.h" > > > /** > @@ -120,7 +121,7 @@ dup_vert( struct draw_stage *stage, > { > struct vertex_header *tmp = stage->tmp[idx]; > const uint vsize = sizeof(struct vertex_header) > - + stage->draw->vs.num_vs_outputs * 4 * sizeof(float); > + + draw_num_shader_outputs(stage->draw) * 4 * sizeof(float); > memcpy(tmp, vert, vsize); > tmp->vertex_id = UNDEFINED_VERTEX_ID; > return tmp; > 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 84f86ae..7f23d0b 100644 > --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c > +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c > @@ -72,12 +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)); > - > - /* Add one to num_outputs because the pipeline occasionally tags on > - * an additional texcoord, eg for AA lines. > - */ > unsigned nr = MAX2( vs->info.num_inputs, > - vs->info.num_outputs + 1 ); > + draw_total_vs_shader_outputs(draw) ); > > if (gs) { > nr = MAX2(nr, gs->info.num_outputs + 1); > 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 17b948a..f6ef5fd 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,12 +141,8 @@ 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); > - > - /* Add one to num_outputs because the pipeline occasionally tags on > - * an additional texcoord, eg for AA lines. > - */ > - const unsigned nr = MAX2( vs->info.num_inputs, > - vs->info.num_outputs + 1 ); > + const unsigned nr = MAX2(vs->info.num_inputs, > + draw_total_vs_shader_outputs(draw)); > > fpme->input_prim = in_prim; > fpme->opt = opt; > diff --git a/src/gallium/auxiliary/draw/draw_vs_variant.c > b/src/gallium/auxiliary/draw/draw_vs_variant.c > index 37500c7..2156241 100644 > --- a/src/gallium/auxiliary/draw/draw_vs_variant.c > +++ b/src/gallium/auxiliary/draw/draw_vs_variant.c > @@ -315,7 +315,7 @@ draw_vs_create_variant_generic( struct draw_vertex_shader > *vs, > vsvg->draw = vs->draw; > > vsvg->temp_vertex_stride = MAX2(key->nr_inputs, > - vsvg->base.vs->info.num_outputs) * 4 * > sizeof(float); > + draw_total_vs_shader_outputs(vs->draw)) * > 4 * sizeof(float); > > /* Build free-standing fetch and emit functions: > */ > Makes sense. Roland _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev