[Mesa-dev] [PATCH v2] swr: fix transform feedback logic

[Ilia Mirkin]

On Thu, Jul 27, 2017 at 2:45 PM, Kyriazis, George
<george.kyriazis at intel.com> wrote:
>
> On Jul 27, 2017, at 1:25 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>
> On Wed, Jul 26, 2017 at 6:27 PM, George Kyriazis
> <george.kyriazis at intel.com> wrote:
>
> The shader that is used to copy vertex data out of the vs/gs shaders to
> the user-specified buffer (streamout or SO shader) was not using the
> correct offsets.
>
> Adjust the offsets that are used just for the SO shader:
> - Make sure that position is handled in the same special way
>  as in the vs/gs shaders
> - Use the correct offset to be passed in the core
> - consolidate register slot mapping logic into one function, since it's
>  been calculated in 2 different places (one for calcuating the slot mask,
>  and one for the register offsets themselves
>
> Also make room for all attibutes in the backend vertex area.
>
> Fixes:
> - all vtk GL2PS tests
> - 18 piglit tests (16 ext_transform_feedback tests,
>  arb-quads-follow-provoking-vertex and primitive-type gl_points
>
> v2:
>
> - take care of more SGV slots in slot mapping logic
> - trim feState.vsVertexSize
> - fix GS interface and incorporate GS while calculating vsVertexSize
>
> Note that vsVertexSize is used in the core as the one parameter that
> controls vertex size between all stages, so it has to be adjusted
> appropriately
> for the whole vs/gs/fs pipeline.
>
> fixes:
> - fixes total of 20 piglit tests
>
> CC: 17.2 <mesa-stable at lists.freedesktop.org>
> ---
> src/gallium/drivers/swr/swr_draw.cpp   | 38
> +++++++++++++++++++++++++++++-----
> src/gallium/drivers/swr/swr_shader.cpp | 32 +++++++++++++++++++++++++++-
> src/gallium/drivers/swr/swr_shader.h   |  3 +++
> src/gallium/drivers/swr/swr_state.cpp  |  5 +++--
> 4 files changed, 70 insertions(+), 8 deletions(-)
>
> diff --git a/src/gallium/drivers/swr/swr_draw.cpp
> b/src/gallium/drivers/swr/swr_draw.cpp
> index 62ad3f7..38a711e 100644
> --- a/src/gallium/drivers/swr/swr_draw.cpp
> +++ b/src/gallium/drivers/swr/swr_draw.cpp
> @@ -81,8 +81,11 @@ swr_draw_vbo(struct pipe_context *pipe, const struct
> pipe_draw_info *info)
>                offsets[output_buffer] = so->output[i].dst_offset;
>             }
>
> +            unsigned attrib_slot = so->output[i].register_index;
> +            attrib_slot = swr_so_adjust_attrib(attrib_slot, ctx->vs);
> +
>             state.stream.decl[num].bufferIndex = output_buffer;
> -            state.stream.decl[num].attribSlot =
> so->output[i].register_index - 1;
> +            state.stream.decl[num].attribSlot = attrib_slot;
>
>
>
> I'm confused... this still doesn't take GS into account. There does
> not need to be any relation between VS outputs and GS outputs, and the
> SO is all in reference to GS outputs (if a GS is there).
>
> Yes, it doesn’t take GS into account in swr_draw_vbo() at this point.  There
> are some other issues with GS that will prevent it from working.  Of course
> we want to fix it, just not in this patch.  This patch fixes the majority of
> the issues that we are concerned with relating to SO, which is what is
> important for 17.2.

Ah OK - I just thought based on the change description that this was
meant to work for GS. If it's a "todo" for later, that's fine too,
although it might be nice to indicate that in a comment.