On Wednesday, June 18, 2014 11:16:47 AM Ian Romanick wrote: > On 06/18/2014 02:51 AM, Iago Toral Quiroga wrote: > > From: Samuel Iglesias Gonsalvez <sigles...@igalia.com> > > > > This implements parsing requirements for multi-stream support in > > geometry shaders as defined in ARB_gpu_shader5. > > > > Signed-off-by: Samuel Iglesias Gonsalvez <sigles...@igalia.com> > > A few minor nits below. With those fixed, this patch is > > Reviewed-by: Ian Romanick <ian.d.roman...@intel.com> > > > --- > > src/glsl/ast.h | 5 +++++ > > src/glsl/ast_to_hir.cpp | 17 +++++++++++++++ > > src/glsl/ast_type.cpp | 39 +++++++++++++++++++++++++++++++++- > > src/glsl/glsl_parser.yy | 49 +++++++++++++++++++++++++++++++++++++++++++ > > src/glsl/glsl_parser_extras.h | 18 ++++++++++++++++ > > src/glsl/glsl_types.h | 5 +++++ > > src/glsl/ir.h | 5 +++++ > > 7 files changed, 137 insertions(+), 1 deletion(-) > > > > diff --git a/src/glsl/ast.h b/src/glsl/ast.h > > index 56e7bd8..c8a3394 100644 > > --- a/src/glsl/ast.h > > +++ b/src/glsl/ast.h > > @@ -509,6 +509,8 @@ struct ast_type_qualifier { > > /** \name Layout qualifiers for GL_ARB_gpu_shader5 */ > > /** \{ */ > > unsigned invocations:1; > > + unsigned stream:1; /* Has stream value assigned */ > > + unsigned explicit_stream:1; /* stream value assigned explicitly by shader code */ > > End-of-line comments should begin with /**< for Doxygen. > > > /** \} */ > > } > > /** \brief Set of flags, accessed by name. */ > > @@ -542,6 +544,9 @@ struct ast_type_qualifier { > > /** Maximum output vertices in GLSL 1.50 geometry shaders. */ > > int max_vertices; > > > > + /** Stream in GLSL 1.50 geometry shaders. */ > > + unsigned stream; > > + > > /** Input or output primitive type in GLSL 1.50 geometry shaders */ > > GLenum prim_type; > > > > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp > > index 132a955..c1bc0f9 100644 > > --- a/src/glsl/ast_to_hir.cpp > > +++ b/src/glsl/ast_to_hir.cpp > > @@ -2461,6 +2461,11 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual, > > if (qual->flags.q.sample) > > var->data.sample = 1; > > > > + if (state->stage == MESA_SHADER_GEOMETRY && > > + qual->flags.q.out && qual->flags.q.stream) { > > + var->data.stream = qual->stream; > > + } > > + > > if (qual->flags.q.attribute && state->stage != MESA_SHADER_VERTEX) { > > var->type = glsl_type::error_type; > > _mesa_glsl_error(loc, state, > > @@ -5092,6 +5097,8 @@ ast_process_structure_or_interface_block(exec_list *instructions, > > interpret_interpolation_qualifier(qual, var_mode, state, &loc); > > fields[i].centroid = qual->flags.q.centroid ? 1 : 0; > > fields[i].sample = qual->flags.q.sample ? 1 : 0; > > Add a blank link here. > > > + /* Only save explicitly defined streams in block's field */ > > And put the */ on it's own line.
Ian, I think you are one of the few people that do that. Mesa overwhelmingly starts and ends comments on the same line, where they fit on one line: (attempt at finding /* on one line and */ on the next line): $ git grep -A1 '/\*' | grep '\*/' | grep -v '/\*.*\*/' | wc -l 2354 (attempt at finding one line /* ... */ comments): $ git grep '/\*.*\*/' | wc -l 35025 So I would leave it as is. But it's not a big deal... --Ken
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev