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

[Kyriazis, George]

> On Jul 27, 2017, at 1:49 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> 
> 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.

I’ll add a comment in the change description before checking in, to explain it a bit more.

Thanks!

George