[Mesa-dev] [PATCH v2 00/23] Multi-stream support for geometry shaders

Chris Forbes chrisf at ijw.co.nz
Fri Jun 27 13:46:44 PDT 2014


Ilia,

That bikeshed is already done :)

I just send an r-b for v3 of patch 18, which is the last patch that lacked one.

I think this is all good to land now.

-- Chris

On Sat, Jun 28, 2014 at 5:11 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> Iago,
>
> Not sure where you are with these patches... I guess some updates have
> been requested to rename stream -> index in some places? Anyways, once
> the generic bits are all sorted out and reviewed, I'd appreciate it if
> they could be pushed out, even if you're still working out hw-related
> issues with i965 so that the gallium mesa/st integration work can be
> pushed out as well.
>
> Thanks,
>
>   -ilia
>
> On Tue, Jun 24, 2014 at 4:10 AM, Iago Toral <itoral at igalia.com> wrote:
>> Hi,
>>
>> this is a summary of the review process for the multi-stream support series.
>> Most of the patches got a reviewed-by by either Chris or Ian and if I am not
>> mistaken only 4 patches still need a reviewed-by so I hopefully we get these
>> 4 reviewed soon :). Here is the situation of each of these patches:
>>
>> Patch 11: glsl: Add support for EmitStreamVertex() and EndStreamPrimitive().
>> - Ian requested to split this in 3 patches which have already been sent to
>> the mailing list as a reply.
>>   - glsl: Modify ir_emit_vertex to have a stream.
>>   - glsl: Modify ir_end_primitive to have a stream.
>>
>>   - Add support for EmitStreamVertex() and EndStreamPrimitive().
>> - There was some discussion on whether ir_to_mesa.cpp and
>> st_glsl_to_tgsi.cpp had to be updated too. Based on what was discussed I
>> concluded that it was not necessary in the end.
>>
>> Patch 12: glsl: Validate vertex emission in geometry shaders.
>> - We need to decide if we allow EmitStreamVertex(0) with outputs other than
>> points. Since Nvidia allows this Ian asked if AMD allowed this too in order
>> to decide if a bug to the spec should be filed. I confirmed that AMD allows
>> this too. Maybe we want to test other systems before making a decision
>> anyway.
>> - I can also change the patch to disallow this behavior until we decide
>> otherwise if that is preferred.
>>
>> Patch 18: i965: Implement GL_PRIMITIVES_GENERATED with non-zero streams.
>> - Needs review
>>
>> Patch 21: docs: mark "Geometry shader multiple streams" as done for i965
>> - Needs review
>>
>> Iago
>>
>>
>> El 2014-06-18 11:51, Iago Toral Quiroga escribió:
>>
>>> This series addresses review comments of the previous version and adds a
>>> few
>>> things. Summary of the changes in version 2:
>>>
>>> Patch 1:
>>>   - Ian: multiple layout(location=) qualifiers
>>>     + This was not a problem in the end.
>>>   - Chris: s/explicitely/explicitly/
>>>   - Chris: Remove 'Id' suffix in variables.
>>>   - Chris: !this->flags.q.streamId for consistency
>>>   - Chris: checks should pass if state->is_version(400, 0)
>>>   - Chris: Check against the real limit and not MAX_VERTEX_STREAMS
>>>   + Plus fixed some other issues we found
>>>
>>> Patch 2: already reviewed by Chris, unchanged.
>>>
>>> Patch 3:
>>>   - Chris: Replace magic 4's with appropriate constant
>>>     + Also added necessary memset(so_decl, 0, sizeof(so_decl));
>>>
>>> Patch 4: already reviewed by Chris, unchanged.
>>>
>>> Patch 5: [DONE]
>>>   - Chris: Not right for separable programs.
>>>     + Original patch was okay after discussion in the mailing list, but
>>>       I modified it to make the code more clear.
>>>
>>> Patch 6: already reviewed by Chris, unchanged.
>>>
>>> Patch 7: already reviewed by Chris, unchanged.
>>>
>>> Patch 8: already reviewed by Chris, unchanged.
>>>
>>> Patch 9:
>>>   - Chris: Does not work with multiple geometry shaders
>>>     + Now we only store info in the program and not individual shaders and
>>> we
>>>       set this value after linking all the shaders together (see patch
>>> 12).
>>>
>>> Patch 10: already reviewed by Chris, unchanged.
>>>
>>> Patch 11:
>>>   - Chris: EmitStreamVertex() and EndStreamPrimitive() should be normal
>>> builtins
>>>     + Reimplemented this following Chris' recommendations in the mailing
>>> list.
>>>
>>> Patch 12: already reviewed by Chris, but now expanded:
>>>   + Includes original patch 12 reviewed-by Chris.
>>>   + Sets prog->Geom.UsesStreams (this was being done in patch 11 in the
>>> original
>>>     series).
>>>   + Link error if emitting to wrong streams (this was being done in patch
>>> 11 in
>>>     original series)
>>>
>>> Patch 13: already reviewed by Chris, unchanged.
>>>
>>> Patch 14: already reviewed by Chris, but modified to adapt to changes in
>>> the
>>> implementation of ir_emit_vertex and ir_end_primitive from patch 11.
>>>
>>> Patch 15: already reviewed by Chris, unchanged.
>>>
>>> Patch 16:
>>>   - Chris: Do no set Ctx->Const.MaxVertexStreams to MAX_VERTEX_STREAMS
>>> globally.
>>>     + Now this is done in the i965 driver only (see patch 20).
>>>
>>> Patch 17: already reviewed by Chris, unchanged.
>>>
>>> Patch 18: NEW
>>>   + Adds multi-stream querying for GL_PRIMITIVES_GENERATED
>>>
>>> Patch 19 (was patch 18 in original series)
>>>   - Chris: s/stream/index
>>>   + Plus added the same logic for GL_PRIMITIVES_GENERATED
>>>
>>> Patch 20: NEW
>>>   + Enables up to MAX_VERTEX_STREAMS in i965.
>>>
>>> Patch 21: NEW
>>>   + Mark multiple streams done for i965.
>>>
>>> Patch 22: NEW
>>> Patch 23: NEW
>>>   + Fixed incorrect initilization and cloning of UsesEndPrimitive flag.
>>>
>>>
>>> Iago Toral Quiroga (20):
>>>   mesa: add StreamId information to transform feedback outputs.
>>>   i965: Enable transform feedback for streams > 0
>>>   glsl: Assign GLSL StreamIds to transform feedback outputs.
>>>   glsl: Fail to link if inter-stage input/outputs are not assigned to
>>>     stream 0
>>>   glsl: Add methods to retrive a varying's name and streamId.
>>>   glsl: Two varyings can't write to the same buffer from different
>>>     streams.
>>>   glsl: Only geometry shader outputs can be associated with non-zero
>>>     streams.
>>>   glsl: Store info about geometry shaders that emit vertices to non-zero
>>>     streams.
>>>   i965/gs: Set number of control data bits for stream mode.
>>>   glsl: Add support for EmitStreamVertex() and EndStreamPrimitive().
>>>   glsl: Validate vertex emission in geometry shaders.
>>>   i965/gs: Set control data bits for vertices emitted in stream mode.
>>>   glsl: include streamId when reading/printing emit-vertex and
>>>     end-primitive IR.
>>>   mesa: Include stream information in indexed queries.
>>>   i965: Implement GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN with non-zero
>>>     streams.
>>>   i965: Implement GL_PRIMITIVES_GENERATED with non-zero streams.
>>>   mesa: Enable simultaneous queries on different streams.
>>>   i965: Enable vertex streams up to MAX_VERTEX_STREAMS.
>>>   mesa: Init Geom.UsesEndPrimitive in shader programs.
>>>   mesa: Copy Geom.UsesEndPrimitive when cloning a geometry program.
>>>
>>> Samuel Iglesias Gonsalvez (3):
>>>   glsl: Add parsing support for multi-stream output in geometry shaders.
>>>   glsl: include streamId when reading/printing ir_variable IR.
>>>   docs: mark "Geometry shader multiple streams" as done for i965
>>>
>>>  docs/GL3.txt                                      |   2 +-
>>>  src/glsl/ast.h                                    |   5 +
>>>  src/glsl/ast_to_hir.cpp                           |  17 +++
>>>  src/glsl/ast_type.cpp                             |  39 +++++-
>>>  src/glsl/builtin_functions.cpp                    |  52 +++++++-
>>>  src/glsl/glsl_parser.yy                           |  49 +++++++
>>>  src/glsl/glsl_parser_extras.h                     |  18 +++
>>>  src/glsl/glsl_types.h                             |   5 +
>>>  src/glsl/ir.h                                     |  39 ++++--
>>>  src/glsl/ir_hierarchical_visitor.cpp              |  50 +++++---
>>>  src/glsl/ir_hierarchical_visitor.h                |   6 +-
>>>  src/glsl/ir_hv_accept.cpp                         |  21 ++-
>>>  src/glsl/ir_print_visitor.cpp                     |  20 ++-
>>>  src/glsl/ir_reader.cpp                            |  28 +++-
>>>  src/glsl/ir_rvalue_visitor.cpp                    |  37 ++++++
>>>  src/glsl/ir_rvalue_visitor.h                      |   6 +
>>>  src/glsl/link_varyings.cpp                        |  41 +++++-
>>>  src/glsl/link_varyings.h                          |  17 +++
>>>  src/glsl/linker.cpp                               | 148
>>> ++++++++++++++++++++--
>>>  src/glsl/lower_output_reads.cpp                   |   4 +-
>>>  src/glsl/lower_packed_varyings.cpp                |   4 +-
>>>  src/glsl/opt_dead_code_local.cpp                  |   2 +-
>>>  src/mesa/drivers/dri/i965/brw_context.c           |   4 +
>>>  src/mesa/drivers/dri/i965/brw_vec4_gs.c           |   9 +-
>>>  src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp |  51 +++++++-
>>>  src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h   |   1 +
>>>  src/mesa/drivers/dri/i965/gen6_queryobj.c         |  21 +--
>>>  src/mesa/drivers/dri/i965/gen7_sol_state.c        |  67 ++++++----
>>>  src/mesa/main/mtypes.h                            |   8 +-
>>>  src/mesa/main/queryobj.c                          |  18 +--
>>>  src/mesa/main/shaderapi.c                         |   1 +
>>>  src/mesa/main/shaderobj.c                         |   2 +
>>>  src/mesa/program/program.c                        |   2 +
>>>  33 files changed, 681 insertions(+), 113 deletions(-)
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list