[Mesa-dev] [PATCH v2 00/23] Multi-stream support for geometry shaders
Chris Forbes
chrisf at ijw.co.nz
Tue Jun 24 02:58:27 PDT 2014
I sent comments on patch 18 - I think it can be made to work correctly
when not doing real SO.
It seems everyone allows EmitStreamVertex(0), so I think you should
follow suit. It would be nice to get the spec clarified to explicitly
allow this.
Patches 11, 12 and 21 are:
Reviewed-by: Chris Forbes <chrisf at ijw.co.nz>
On Tue, Jun 24, 2014 at 8:10 PM, 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
More information about the mesa-dev
mailing list