On Sat, 2016-10-22 at 23:09 +0300, Andres Gomez wrote: > In the case of layout-qualifier-names that can appear multiple times > in different declarations of the same shader or, even, the same > program, but that have to consistently hold the same value we are > using the ast_layout_expression class which holds a list to store all > the appearances to be able to check for coherence later. > > Until now, we were also holding inside the ast_layout_expression > values of the same layout-qualifier-name that could appear inside a > single layout-qualifier or across multiple layout-qualifiers in a > single declaration. > > This was a problem since, inside a declaration, only the last > appearance should be taken into account. As we were not doing this, > the compilation or linking was failing due to different values of the > same layout-qualifier-name in a single declaration when such > layout-qualifier-name had as a constraint to hold the same value > across the same shader or program. > > Now, we only hold the last appearanace of a repeated > layout-qualifier-name inside a single declaration. > > These following 2 example will help to illustrate the problem: > > - " #version 150 > #extension GL_ARB_shading_language_420pack: enable > #extension GL_ARB_enhanced_layouts: enable > > layout(max_vertices=2, max_vertices=3) out; > layout(max_vertices=3) out;" > > - " #version 150 > #extension GL_ARB_shading_language_420pack: enable > #extension GL_ARB_enhanced_layouts: enable > > layout(max_vertices=2) layout(max_vertices=3) out; > layout(max_vertices=3) out;" > > Although different values for "max_vertices" should result in a > compilation error. The above code is valid because max_vertices=2 is > ignored. > > In addition, the series includes changes to allow multiple > layout-qualifiers in the same declaration if > ARB_shading_language_420pack *or ARB_enhanced_layouts* are supported.
As mentioned in the piglit test reviews I'm not so sure this is correct. > > This change is not 100% clear to me since the addition in > ARB_enhanced_layouts is for a duplicated layout-qualifier-name in a > single layout-qualifier but it seems to make sense since the change > builds up on the precondition that multiple layout-qualifiers are > already allowed in a single declaration. > > The main changes in this v2 series are: > Patch 2/9 is new and fixes a related problem in Interface Blocks > that was detected when developing piglit tests for the original > series. > Patches 3/9 and 4/9 are split from the patch 2/5 of the original > series. > Patch 5/9 is new and allows multiple layout-qualifiers in the same > declaration just if the ARB_enhanced_layouts spec is supported. > Patches 7/9 and 8/9 replace patch 4/5 from the original series. > > Fixes: > - GL44-CTS.shading_language_420pack.qualifier_override_layout > - GL44-CTS.enhanced_layouts.xfb_duplicated_stride > > Andres Gomez (9): > glsl: ignore all but the rightmost layout-qualifier-name > glsl: merge layouts into the default one as the last step in > interface > blocks > glsl: ignore all but the rightmost layout qualifier name from the > rightmost layout qualifier > glsl: simplified error checking for duplicated layout-qualifiers > glsl: allow multiple layout-qualifier in single declaration if > enhanced layouts > glsl: push layout-qualifier-name values from variable declarations > to > global > Revert "glsl: geom shader max_vertices layout must match." > Revert "glsl: allow layout qualifier overrides with > ARB_shading_language_420pack" > glsl: simplified ast_type_qualifier::merge_[in|out]_qualifier API > > src/compiler/glsl/ast.h | 25 +++-- > src/compiler/glsl/ast_type.cpp | 127 ++++++++++++++------- > --- > src/compiler/glsl/glsl_parser.yy | 165 +++++++++++++++---- > ------------ > src/compiler/glsl/glsl_parser_extras.cpp | 4 +- > 4 files changed, 173 insertions(+), 148 deletions(-) > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev