From 1fe4a44759903aeb4a4e4cfc94c3e82961714e76 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Mon, 19 Mar 2018 20:05:40 +0100 Subject: [PATCH 1/2] Add locations to opaque types for OpenGL --- Test/baseResults/spv.debugInfo.1.1.frag.out | 1 + Test/baseResults/spv.debugInfo.frag.out | 1 + glslang/MachineIndependent/iomapper.cpp | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Test/baseResults/spv.debugInfo.1.1.frag.out b/Test/baseResults/spv.debugInfo.1.1.frag.out index b7303762..9bd14910 100644 --- a/Test/baseResults/spv.debugInfo.1.1.frag.out +++ b/Test/baseResults/spv.debugInfo.1.1.frag.out @@ -94,6 +94,7 @@ void main() MemberDecorate 54(ubuf) 0 Offset 0 Decorate 54(ubuf) Block Decorate 56 DescriptorSet 3 + Decorate 69(s2d) Location 0 Decorate 69(s2d) DescriptorSet 3 3: TypeVoid 4: TypeFunction 3 diff --git a/Test/baseResults/spv.debugInfo.frag.out b/Test/baseResults/spv.debugInfo.frag.out index de544151..0bb266bd 100644 --- a/Test/baseResults/spv.debugInfo.frag.out +++ b/Test/baseResults/spv.debugInfo.frag.out @@ -97,6 +97,7 @@ void main() Decorate 54(ubuf) Block Decorate 56 DescriptorSet 3 Decorate 56 Binding 0 + Decorate 69(s2d) Location 0 Decorate 69(s2d) DescriptorSet 3 Decorate 69(s2d) Binding 1 3: TypeVoid diff --git a/glslang/MachineIndependent/iomapper.cpp b/glslang/MachineIndependent/iomapper.cpp index a7deb9d5..619d9187 100644 --- a/glslang/MachineIndependent/iomapper.cpp +++ b/glslang/MachineIndependent/iomapper.cpp @@ -431,7 +431,8 @@ struct TDefaultIoResolverBase : public glslang::TIoMapResolver // no locations added if already present, a built-in variable, a block, or an opaque if (type.getQualifier().hasLocation() || type.isBuiltIn() || - type.getBasicType() == EbtBlock || type.containsOpaque()) + type.getBasicType() == EbtBlock || + (type.containsOpaque() && intermediate.getSpv().openGl == 0)) return -1; // no locations on blocks of built-in variables From 2d539049996678c2503449ae5ea313d019cd52e1 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Mon, 19 Mar 2018 23:28:19 +0100 Subject: [PATCH 2/2] Take into account the number of locations taken up by a uniform When assigning uniform locations it now takes into account the number of locations occupied by the type. For uniforms, all types except arrays and structs take up one location. For arrays the base location count is multiplied by the array dimensions and for structs it is the sum of the locations of each member. --- glslang/MachineIndependent/iomapper.cpp | 6 +++- glslang/MachineIndependent/linkValidate.cpp | 30 +++++++++++++++++++ .../MachineIndependent/localintermediate.h | 1 + 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/glslang/MachineIndependent/iomapper.cpp b/glslang/MachineIndependent/iomapper.cpp index 619d9187..68172538 100644 --- a/glslang/MachineIndependent/iomapper.cpp +++ b/glslang/MachineIndependent/iomapper.cpp @@ -443,7 +443,11 @@ struct TDefaultIoResolverBase : public glslang::TIoMapResolver return -1; } - return nextUniformLocation++; + int location = nextUniformLocation; + + nextUniformLocation += TIntermediate::computeTypeUniformLocationSize(type); + + return location; } bool validateInOut(EShLanguage /*stage*/, const char* /*name*/, const TType& /*type*/, bool /*is_live*/) override { diff --git a/glslang/MachineIndependent/linkValidate.cpp b/glslang/MachineIndependent/linkValidate.cpp index 9aba2792..aa9c5121 100644 --- a/glslang/MachineIndependent/linkValidate.cpp +++ b/glslang/MachineIndependent/linkValidate.cpp @@ -962,6 +962,36 @@ int TIntermediate::computeTypeLocationSize(const TType& type, EShLanguage stage) return 1; } +// Same as computeTypeLocationSize but for uniforms +int TIntermediate::computeTypeUniformLocationSize(const TType& type) +{ + // "Individual elements of a uniform array are assigned + // consecutive locations with the first element taking location + // location." + if (type.isArray()) { + // TODO: perf: this can be flattened by using getCumulativeArraySize(), and a deref that discards all arrayness + TType elementType(type, 0); + if (type.isImplicitlySizedArray()) { + // TODO: are there valid cases of having an implicitly-sized array with a location? If so, running this code too early. + return computeTypeUniformLocationSize(elementType); + } else + return type.getOuterArraySize() * computeTypeUniformLocationSize(elementType); + } + + // "Each subsequent inner-most member or element gets incremental + // locations for the entire structure or array." + if (type.isStruct()) { + int size = 0; + for (int member = 0; member < (int)type.getStruct()->size(); ++member) { + TType memberType(type, member); + size += computeTypeUniformLocationSize(memberType); + } + return size; + } + + return 1; +} + // Accumulate xfb buffer ranges and check for collisions as the accumulation is done. // // Returns < 0 if no collision, >= 0 if collision and the value returned is a colliding value. diff --git a/glslang/MachineIndependent/localintermediate.h b/glslang/MachineIndependent/localintermediate.h index d6ecbe34..d6f13ae4 100644 --- a/glslang/MachineIndependent/localintermediate.h +++ b/glslang/MachineIndependent/localintermediate.h @@ -590,6 +590,7 @@ public: int addUsedOffsets(int binding, int offset, int numOffsets); bool addUsedConstantId(int id); static int computeTypeLocationSize(const TType&, EShLanguage); + static int computeTypeUniformLocationSize(const TType&); bool setXfbBufferStride(int buffer, unsigned stride) {