[Mesa-dev] [PATCH 2/8] draw: stop crashing with extra shader outputs
Roland Scheidegger
sroland at vmware.com
Fri Aug 2 07:56:22 PDT 2013
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 <zackr at 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
More information about the mesa-dev
mailing list