[Mesa-dev] [PATCH 1/2] softpipe/draw: fix vertex id in soft paths.
Roland Scheidegger
sroland at vmware.com
Wed Mar 27 23:16:25 UTC 2019
Am 27.03.19 um 20:47 schrieb Dave Airlie:
> From: Dave Airlie <airlied at redhat.com>
>
> This fixes the vertex id fetch in the non-llvm drawing paths.
>
> This vertex id in elt mode comes from the elts not just a linear
> value.
>
> Note we don't bad
add?
> basevertex in the elts case as it's already included
> in the elts by the looks of it (at least tests fail if I add it)
Yes, the vsplit code is responsible for this, so that the elts you'll
get in draw later actually represent the indices you have to fetch.
I'm not entirely sure this is still the best design today but I think
this evolved naturally from before base index actually existed in the
APIs. (More below.)
>
> Fixes piglit end-primitive tests and some others.
> ---
> .../auxiliary/draw/draw_pt_fetch_shade_pipeline.c | 13 +++++++++----
> src/gallium/auxiliary/draw/draw_vs.h | 3 ++-
> src/gallium/auxiliary/draw/draw_vs_exec.c | 7 ++++---
> src/gallium/auxiliary/draw/draw_vs_llvm.c | 3 ++-
> src/gallium/auxiliary/draw/draw_vs_variant.c | 4 ++--
> 5 files changed, 19 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 f76e022994a..3207932c601 100644
> --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c
> +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c
> @@ -205,6 +205,7 @@ static void
> draw_vertex_shader_run(struct draw_vertex_shader *vshader,
> const void *constants[PIPE_MAX_CONSTANT_BUFFERS],
> unsigned const_size[PIPE_MAX_CONSTANT_BUFFERS],
> + const struct draw_fetch_info *fetch_info,
> const struct draw_vertex_info *input_verts,
> struct draw_vertex_info *output_verts)
> {
> @@ -222,7 +223,8 @@ draw_vertex_shader_run(struct draw_vertex_shader *vshader,
> const_size,
> input_verts->count,
> input_verts->vertex_size,
> - input_verts->vertex_size);
> + input_verts->vertex_size,
> + fetch_info->elts);
> }
>
>
> @@ -267,18 +269,17 @@ fetch_pipeline_generic(struct draw_pt_middle_end *middle,
> */
> fetch( fpme->fetch, fetch_info, (char *)fetched_vert_info.verts );
>
> - /* Finished with fetch:
> - */
> - fetch_info = NULL;
> vert_info = &fetched_vert_info;
>
> /* Run the shader, note that this overwrites the data[] parts of
> * the pipeline verts.
> + * Need fetch info to get vertex id correct.
> */
> if (fpme->opt & PT_SHADE) {
> draw_vertex_shader_run(vshader,
> draw->pt.user.vs_constants,
> draw->pt.user.vs_constants_size,
> + fetch_info,
> vert_info,
> &vs_vert_info);
>
> @@ -286,6 +287,10 @@ fetch_pipeline_generic(struct draw_pt_middle_end *middle,
> vert_info = &vs_vert_info;
> }
>
> + /* Finished with fetch:
> + */
> + fetch_info = NULL;
> +
> if ((fpme->opt & PT_SHADE) && gshader) {
> draw_geometry_shader_run(gshader,
> draw->pt.user.gs_constants,
> diff --git a/src/gallium/auxiliary/draw/draw_vs.h b/src/gallium/auxiliary/draw/draw_vs.h
> index 6a900224c11..65520ea6e7a 100644
> --- a/src/gallium/auxiliary/draw/draw_vs.h
> +++ b/src/gallium/auxiliary/draw/draw_vs.h
> @@ -140,7 +140,8 @@ struct draw_vertex_shader {
> const unsigned const_size[PIPE_MAX_CONSTANT_BUFFERS],
> unsigned count,
> unsigned input_stride,
> - unsigned output_stride );
> + unsigned output_stride,
> + const unsigned *fetch_elts);
>
>
> void (*delete)( struct draw_vertex_shader * );
> diff --git a/src/gallium/auxiliary/draw/draw_vs_exec.c b/src/gallium/auxiliary/draw/draw_vs_exec.c
> index dbd7aa551eb..c1f8edaae69 100644
> --- a/src/gallium/auxiliary/draw/draw_vs_exec.c
> +++ b/src/gallium/auxiliary/draw/draw_vs_exec.c
> @@ -93,7 +93,8 @@ vs_exec_run_linear(struct draw_vertex_shader *shader,
> const unsigned const_size[PIPE_MAX_CONSTANT_BUFFERS],
> unsigned count,
> unsigned input_stride,
> - unsigned output_stride)
> + unsigned output_stride,
> + const unsigned *fetch_elts)
> {
> struct exec_vertex_shader *evs = exec_vertex_shader(shader);
> struct tgsi_exec_machine *machine = evs->machine;
> @@ -133,7 +134,7 @@ vs_exec_run_linear(struct draw_vertex_shader *shader,
> if (shader->info.uses_vertexid) {
> unsigned vid = machine->SysSemanticToIndex[TGSI_SEMANTIC_VERTEXID];
> assert(vid < ARRAY_SIZE(machine->SystemValue));
> - machine->SystemValue[vid].xyzw[0].i[j] = i + j + basevertex;
> + machine->SystemValue[vid].xyzw[0].i[j] = fetch_elts ? fetch_elts[i + j] : (i + j + basevertex);
> }
> if (shader->info.uses_basevertex) {
> unsigned vid = machine->SysSemanticToIndex[TGSI_SEMANTIC_BASEVERTEX];
> @@ -143,7 +144,7 @@ vs_exec_run_linear(struct draw_vertex_shader *shader,
> if (shader->info.uses_vertexid_nobase) {
> unsigned vid = machine->SysSemanticToIndex[TGSI_SEMANTIC_VERTEXID_NOBASE];
> assert(vid < ARRAY_SIZE(machine->SystemValue));
> - machine->SystemValue[vid].xyzw[0].i[j] = i + j;
> + machine->SystemValue[vid].xyzw[0].i[j] = fetch_elts ? fetch_elts[i + j] : (i + j);
So, I'm pretty sure you'd actually have to subtract basevertex here for
the elts case - fetch_elts still includes it already (this is also what
the llvm paths do:
"system_values.vertex_id_nobase = LLVMBuildSub(builder,
true_index_array, system_values.basevertex, "");).")
Might not hit that with gl state tracker though...
With that fixed,
For the series:
Reviewed-by: Roland Scheidegger <sroland at vmware.com>
Thanks for tackling this! We got a bit sloppy there when we implemented
this stuff in draw first, essentially only made sure it worked with llvm
paths...
Roland
> }
>
> for (slot = 0; slot < shader->info.num_inputs; slot++) {
> diff --git a/src/gallium/auxiliary/draw/draw_vs_llvm.c b/src/gallium/auxiliary/draw/draw_vs_llvm.c
> index c92e4317216..15486f8ffa8 100644
> --- a/src/gallium/auxiliary/draw/draw_vs_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_vs_llvm.c
> @@ -53,7 +53,8 @@ vs_llvm_run_linear( struct draw_vertex_shader *shader,
> const unsigned constants_size[PIPE_MAX_CONSTANT_BUFFERS],
> unsigned count,
> unsigned input_stride,
> - unsigned output_stride )
> + unsigned output_stride,
> + const unsigned *elts)
> {
> /* we should never get here since the entire pipeline is
> * generated in draw_pt_fetch_shade_pipeline_llvm.c */
> diff --git a/src/gallium/auxiliary/draw/draw_vs_variant.c b/src/gallium/auxiliary/draw/draw_vs_variant.c
> index af36a86674d..44cf29b8e47 100644
> --- a/src/gallium/auxiliary/draw/draw_vs_variant.c
> +++ b/src/gallium/auxiliary/draw/draw_vs_variant.c
> @@ -179,7 +179,7 @@ static void PIPE_CDECL vsvg_run_elts( struct draw_vs_variant *variant,
> vsvg->base.vs->draw->pt.user.vs_constants_size,
> count,
> temp_vertex_stride,
> - temp_vertex_stride);
> + temp_vertex_stride, NULL);
>
> /* FIXME: geometry shading? */
>
> @@ -247,7 +247,7 @@ static void PIPE_CDECL vsvg_run_linear( struct draw_vs_variant *variant,
> vsvg->base.vs->draw->pt.user.vs_constants_size,
> count,
> temp_vertex_stride,
> - temp_vertex_stride);
> + temp_vertex_stride, NULL);
>
> if (vsvg->base.key.clip) {
> /* not really handling clipping, just do the rhw so we can
>
More information about the mesa-dev
mailing list