Great, thanks Chris. Ilia, I'll push the patches today.
Iago On Sat, 2014-06-28 at 08:46 +1200, Chris Forbes wrote: > 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 <imir...@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 <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 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
