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 <chr...@ijw.co.nz> On Tue, Jun 24, 2014 at 8:10 PM, Iago Toral <ito...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev