diff --git a/include/GLSLANG/ShaderLang.h b/include/GLSLANG/ShaderLang.h index 2264f95b0..6eaee9013 100644 --- a/include/GLSLANG/ShaderLang.h +++ b/include/GLSLANG/ShaderLang.h @@ -26,7 +26,7 @@ // Version number for shader translation API. // It is incremented every time the API changes. -#define ANGLE_SH_VERSION 207 +#define ANGLE_SH_VERSION 208 enum ShShaderSpec { @@ -281,12 +281,6 @@ const ShCompileOptions SH_INIT_SHARED_VARIABLES = UINT64_C(1) << 41; // http://anglebug.com/3246 const ShCompileOptions SH_FORCE_ATOMIC_VALUE_RESOLUTION = UINT64_C(1) << 42; -// Change interface block layout qualifiers to std140 for any layout that is not explicitly set to -// std430. This is to comply with GL_KHR_vulkan_glsl where shared and packed are not allowed (and -// std140 could be used instead) and unspecified layouts can assume either std140 or std430 (and we -// choose std140 as std430 is not yet universally supported). -const ShCompileOptions SH_REDEFINE_INTERFACE_LAYOUT_QUALIFIERS_WITH_STD = UINT64_C(1) << 43; - // Defines alternate strategies for implementing array index clamping. enum ShArrayIndexClampingStrategy { diff --git a/src/compiler.gni b/src/compiler.gni index 5f8a2d1d6..9fced4d71 100644 --- a/src/compiler.gni +++ b/src/compiler.gni @@ -160,8 +160,6 @@ angle_translator_sources = [ "src/compiler/translator/tree_ops/RewriteDoWhile.h", "src/compiler/translator/tree_ops/RewriteExpressionsWithShaderStorageBlock.cpp", "src/compiler/translator/tree_ops/RewriteExpressionsWithShaderStorageBlock.h", - "src/compiler/translator/tree_ops/RedefineInterfaceBlockLayoutQualifiersWithStd.cpp", - "src/compiler/translator/tree_ops/RedefineInterfaceBlockLayoutQualifiersWithStd.h", "src/compiler/translator/tree_ops/RewriteStructSamplers.cpp", "src/compiler/translator/tree_ops/RewriteStructSamplers.h", "src/compiler/translator/tree_ops/RewriteRepeatedAssignToSwizzled.cpp", diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp index 5dfe9da4a..fb84238ea 100644 --- a/src/compiler/translator/Compiler.cpp +++ b/src/compiler/translator/Compiler.cpp @@ -33,7 +33,6 @@ #include "compiler/translator/tree_ops/InitializeVariables.h" #include "compiler/translator/tree_ops/PruneEmptyCases.h" #include "compiler/translator/tree_ops/PruneNoOps.h" -#include "compiler/translator/tree_ops/RedefineInterfaceBlockLayoutQualifiersWithStd.h" #include "compiler/translator/tree_ops/RegenerateStructNames.h" #include "compiler/translator/tree_ops/RemoveArrayLengthMethod.h" #include "compiler/translator/tree_ops/RemoveInvariantDeclaration.h" @@ -724,13 +723,6 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root, } } - if (compileOptions & SH_REDEFINE_INTERFACE_LAYOUT_QUALIFIERS_WITH_STD) - { - // Change the interface block layouts based on GL_KHR_vulkan_glsl. Only std140 and std430 - // are allowed in Vulkan GLSL. - RedefineInterfaceBlockLayoutQualifiersWithStd(root, &mSymbolTable); - } - if (shouldCollectVariables(compileOptions)) { ASSERT(!mVariablesCollected); diff --git a/src/compiler/translator/OutputVulkanGLSL.cpp b/src/compiler/translator/OutputVulkanGLSL.cpp index 2ec1a1ba4..af0adbe3e 100644 --- a/src/compiler/translator/OutputVulkanGLSL.cpp +++ b/src/compiler/translator/OutputVulkanGLSL.cpp @@ -71,12 +71,20 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable) { const TInterfaceBlock *interfaceBlock = type.getInterfaceBlock(); name = interfaceBlock->name(); + TLayoutBlockStorage storage = interfaceBlock->blockStorage(); // Make sure block storage format is specified. - if (interfaceBlock->blockStorage() != EbsUnspecified) + if (storage != EbsStd430) { - blockStorage = getBlockStorageString(interfaceBlock->blockStorage()); + // Change interface block layout qualifiers to std140 for any layout that is not + // explicitly set to std430. This is to comply with GL_KHR_vulkan_glsl where shared and + // packed are not allowed (and std140 could be used instead) and unspecified layouts can + // assume either std140 or std430 (and we choose std140 as std430 is not yet universally + // supported). + storage = EbsStd140; } + + blockStorage = getBlockStorageString(storage); } // Specify matrix packing if necessary. diff --git a/src/compiler/translator/tree_ops/RedefineInterfaceBlockLayoutQualifiersWithStd.cpp b/src/compiler/translator/tree_ops/RedefineInterfaceBlockLayoutQualifiersWithStd.cpp deleted file mode 100644 index a32281f07..000000000 --- a/src/compiler/translator/tree_ops/RedefineInterfaceBlockLayoutQualifiersWithStd.cpp +++ /dev/null @@ -1,107 +0,0 @@ -// -// Copyright 2019 The ANGLE Project Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. -// -// RedefineInterfaceBlockLayoutQualifiersWithStd: Ensure layout qualifiers are either std140 or -// std430 to comply with Vulkan GLSL. -// - -#include "compiler/translator/tree_ops/RedefineInterfaceBlockLayoutQualifiersWithStd.h" - -#include "compiler/translator/SymbolTable.h" -#include "compiler/translator/tree_util/IntermTraverse.h" - -namespace sh -{ -namespace -{ -// Helper to replace the type of a symbol -TIntermSymbol *RedefineLayoutQualifierOfSymbolNode(TIntermSymbol *symbolNode, - const TLayoutQualifier &newLayoutQualifier, - TSymbolTable *symbolTable) -{ - const TVariable &oldVariable = symbolNode->variable(); - - ASSERT(symbolNode->getType().isInterfaceBlock()); - - const TType &oldType = symbolNode->getType(); - const TInterfaceBlock *oldInterfaceBlock = oldType.getInterfaceBlock(); - - // Create a new type based on the old type, but the memory layout qualifier changed. - TType *newType = new TType(oldType); - newType->setLayoutQualifier(newLayoutQualifier); - - // Create a new interface block based on the old one, with the new memory layout qualifier as - // well. - TInterfaceBlock *newInterfaceBlock = - new TInterfaceBlock(symbolTable, oldInterfaceBlock->name(), &oldInterfaceBlock->fields(), - newLayoutQualifier, oldInterfaceBlock->symbolType()); - - newType->setInterfaceBlock(newInterfaceBlock); - - // Create a new variable with the modified type, to substitute the old variable. - TVariable *newVariable = - new TVariable(oldVariable.uniqueId(), oldVariable.name(), oldVariable.symbolType(), - oldVariable.extension(), newType); - - return new TIntermSymbol(newVariable); -} - -class Traverser : public TIntermTraverser -{ - public: - explicit Traverser(TSymbolTable *symbolTable) - : TIntermTraverser(true, false, false, symbolTable) - { - symbolTable->push(); - } - - ~Traverser() override { mSymbolTable->pop(); } - - bool visitDeclaration(Visit visit, TIntermDeclaration *node) override - { - ASSERT(visit == PreVisit); - - if (!mInGlobalScope) - { - return false; - } - - const TIntermSequence &sequence = *(node->getSequence()); - TIntermTyped *declarator = sequence.front()->getAsTyped(); - const TType &type = declarator->getType(); - - if (type.isInterfaceBlock()) - { - ASSERT(declarator->getAsSymbolNode()); - - TLayoutQualifier layoutQualifier = type.getLayoutQualifier(); - - // If the layout qualifier is not explicitly std140 or std430, change it to std140 for - // uniforms and std430 otherwise. See the comment in the header for more information. - if (layoutQualifier.blockStorage != EbsStd140 && - layoutQualifier.blockStorage != EbsStd430) - { - layoutQualifier.blockStorage = - type.getQualifier() == EvqUniform ? EbsStd140 : EbsStd430; - - TIntermSymbol *replacement = RedefineLayoutQualifierOfSymbolNode( - declarator->getAsSymbolNode(), layoutQualifier, mSymbolTable); - - queueReplacementWithParent(node, declarator, replacement, OriginalNode::IS_DROPPED); - } - } - - return false; - } -}; -} // anonymous namespace - -void RedefineInterfaceBlockLayoutQualifiersWithStd(TIntermBlock *root, TSymbolTable *symbolTable) -{ - Traverser redefineInterfaceBlockLayoutQualifiersWithStd(symbolTable); - root->traverse(&redefineInterfaceBlockLayoutQualifiersWithStd); - redefineInterfaceBlockLayoutQualifiersWithStd.updateTree(); -} -} // namespace sh diff --git a/src/compiler/translator/tree_ops/RedefineInterfaceBlockLayoutQualifiersWithStd.h b/src/compiler/translator/tree_ops/RedefineInterfaceBlockLayoutQualifiersWithStd.h deleted file mode 100644 index 87b0c397a..000000000 --- a/src/compiler/translator/tree_ops/RedefineInterfaceBlockLayoutQualifiersWithStd.h +++ /dev/null @@ -1,25 +0,0 @@ -// -// Copyright 2019 The ANGLE Project Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. -// -// RedefineInterfaceBlockLayoutQualifiersWithStd: Change the memory layout qualifier of interface -// blocks if not specifically requested to be std140 or std430, i.e. the memory layout qualifier is -// changed if it's unspecified, shared or packed. This makes the layout qualifiers conformant with -// Vulkan GLSL (GL_KHR_vulkan_glsl). -// -// - For uniform buffers, std140 is used. It would have been more efficient to default to std430, -// but that would require GL_EXT_scalar_block_layout. -// - For storage buffers, std430 is used. - -#ifndef COMPILER_TRANSLATOR_TREEOPS_REDEFINEINTERFACEBLOCKLAYOUTQUALIFIERSWITHSTD_H_ -#define COMPILER_TRANSLATOR_TREEOPS_REDEFINEINTERFACEBLOCKLAYOUTQUALIFIERSWITHSTD_H_ - -namespace sh -{ -class TIntermBlock; -class TSymbolTable; -void RedefineInterfaceBlockLayoutQualifiersWithStd(TIntermBlock *root, TSymbolTable *symbolTable); -} // namespace sh - -#endif // COMPILER_TRANSLATOR_TREEOPS_REDEFINEINTERFACEBLOCKLAYOUTQUALIFIERSWITHSTD_H_ diff --git a/src/libANGLE/ProgramLinkedResources.cpp b/src/libANGLE/ProgramLinkedResources.cpp index eaea97400..afc9d318e 100644 --- a/src/libANGLE/ProgramLinkedResources.cpp +++ b/src/libANGLE/ProgramLinkedResources.cpp @@ -564,7 +564,7 @@ void InterfaceBlockInfo::getShaderBlockInfo(const std::vector 0) @@ -577,7 +577,7 @@ void InterfaceBlockInfo::getShaderBlockInfo(const std::vector *blocksOut) - : mShaderBlocks({}), mBlocksOut(blocksOut) +InterfaceBlockLinker::InterfaceBlockLinker(std::vector *blocksOut, + std::vector *unusedInterfaceBlocksOut) + : mShaderBlocks({}), mBlocksOut(blocksOut), mUnusedInterfaceBlocksOut(unusedInterfaceBlocksOut) {} InterfaceBlockLinker::~InterfaceBlockLinker() {} @@ -1059,7 +1060,10 @@ void InterfaceBlockLinker::linkBlocks(const GetBlockSizeFunc &getBlockSize, for (const sh::InterfaceBlock &block : *mShaderBlocks[shaderType]) { if (!IsActiveInterfaceBlock(block)) + { + mUnusedInterfaceBlocksOut->push_back(block.name); continue; + } if (visitedList.count(block.name) == 0) { @@ -1069,7 +1073,10 @@ void InterfaceBlockLinker::linkBlocks(const GetBlockSizeFunc &getBlockSize, } if (!block.active) + { + mUnusedInterfaceBlocksOut->push_back(block.name); continue; + } for (InterfaceBlock &priorBlock : *mBlocksOut) { @@ -1151,8 +1158,9 @@ void InterfaceBlockLinker::defineInterfaceBlock(const GetBlockSizeFunc &getBlock // UniformBlockLinker implementation. UniformBlockLinker::UniformBlockLinker(std::vector *blocksOut, - std::vector *uniformsOut) - : InterfaceBlockLinker(blocksOut), mUniformsOut(uniformsOut) + std::vector *uniformsOut, + std::vector *unusedInterfaceBlocksOut) + : InterfaceBlockLinker(blocksOut, unusedInterfaceBlocksOut), mUniformsOut(uniformsOut) {} UniformBlockLinker::~UniformBlockLinker() {} @@ -1174,9 +1182,12 @@ sh::ShaderVariableVisitor *UniformBlockLinker::getVisitor( } // ShaderStorageBlockLinker implementation. -ShaderStorageBlockLinker::ShaderStorageBlockLinker(std::vector *blocksOut, - std::vector *bufferVariablesOut) - : InterfaceBlockLinker(blocksOut), mBufferVariablesOut(bufferVariablesOut) +ShaderStorageBlockLinker::ShaderStorageBlockLinker( + std::vector *blocksOut, + std::vector *bufferVariablesOut, + std::vector *unusedInterfaceBlocksOut) + : InterfaceBlockLinker(blocksOut, unusedInterfaceBlocksOut), + mBufferVariablesOut(bufferVariablesOut) {} ShaderStorageBlockLinker::~ShaderStorageBlockLinker() {} @@ -1224,8 +1235,8 @@ ProgramLinkedResources::ProgramLinkedResources( std::vector *bufferVariablesOut, std::vector *atomicCounterBuffersOut) : varyingPacking(maxVaryingVectors, packMode), - uniformBlockLinker(uniformBlocksOut, uniformsOut), - shaderStorageBlockLinker(shaderStorageBlocksOut, bufferVariablesOut), + uniformBlockLinker(uniformBlocksOut, uniformsOut, &unusedInterfaceBlocks), + shaderStorageBlockLinker(shaderStorageBlocksOut, bufferVariablesOut, &unusedInterfaceBlocks), atomicCounterBufferLinker(atomicCounterBuffersOut) {} diff --git a/src/libANGLE/ProgramLinkedResources.h b/src/libANGLE/ProgramLinkedResources.h index e91102b1c..05433acce 100644 --- a/src/libANGLE/ProgramLinkedResources.h +++ b/src/libANGLE/ProgramLinkedResources.h @@ -108,7 +108,8 @@ class InterfaceBlockLinker : angle::NonCopyable const GetBlockMemberInfoFunc &getMemberInfo) const; protected: - InterfaceBlockLinker(std::vector *blocksOut); + InterfaceBlockLinker(std::vector *blocksOut, + std::vector *unusedInterfaceBlocksOut); void defineInterfaceBlock(const GetBlockSizeFunc &getBlockSize, const GetBlockMemberInfoFunc &getMemberInfo, const sh::InterfaceBlock &interfaceBlock, @@ -119,6 +120,7 @@ class InterfaceBlockLinker : angle::NonCopyable ShaderMap *> mShaderBlocks; std::vector *mBlocksOut; + std::vector *mUnusedInterfaceBlocksOut; virtual sh::ShaderVariableVisitor *getVisitor(const GetBlockMemberInfoFunc &getMemberInfo, const std::string &namePrefix, @@ -131,7 +133,8 @@ class UniformBlockLinker final : public InterfaceBlockLinker { public: UniformBlockLinker(std::vector *blocksOut, - std::vector *uniformsOut); + std::vector *uniformsOut, + std::vector *unusedInterfaceBlocksOut); ~UniformBlockLinker() override; private: @@ -150,7 +153,8 @@ class ShaderStorageBlockLinker final : public InterfaceBlockLinker { public: ShaderStorageBlockLinker(std::vector *blocksOut, - std::vector *bufferVariablesOut); + std::vector *bufferVariablesOut, + std::vector *unusedInterfaceBlocksOut); ~ShaderStorageBlockLinker() override; private: @@ -208,6 +212,7 @@ struct ProgramLinkedResources ShaderStorageBlockLinker shaderStorageBlockLinker; AtomicCounterBufferLinker atomicCounterBufferLinker; std::vector unusedUniforms; + std::vector unusedInterfaceBlocks; }; class CustomBlockLayoutEncoderFactory : angle::NonCopyable diff --git a/src/libANGLE/renderer/vulkan/GlslangWrapper.cpp b/src/libANGLE/renderer/vulkan/GlslangWrapper.cpp index acec3eb5c..1c10323d9 100644 --- a/src/libANGLE/renderer/vulkan/GlslangWrapper.cpp +++ b/src/libANGLE/renderer/vulkan/GlslangWrapper.cpp @@ -107,12 +107,13 @@ class IntermediateShaderSource final : angle::NonCopyable // Find @@ QUALIFIER-name @@ and replace it with |specifier|. void insertQualifierSpecifier(const std::string &name, const std::string &specifier); - // Remove @@ LAYOUT-name(*) @@ and @@ QUALIFIER-name @@ altogether. - void eraseLayoutAndQualifierSpecifiers(const std::string &name); - // Replace @@ DEFAULT-UNIFORMS-SET-BINDING @@ with |specifier|. void insertDefaultUniformsSpecifier(std::string &&specifier); + // Remove @@ LAYOUT-name(*) @@ and @@ QUALIFIER-name @@ altogether, optionally replacing them + // with something to make sure the shader still compiles. + void eraseLayoutAndQualifierSpecifiers(const std::string &name, const std::string &replacement); + // Get the transformed shader source as one string. std::string getShaderSource(); @@ -251,16 +252,18 @@ void IntermediateShaderSource::insertQualifierSpecifier(const std::string &name, } } -void IntermediateShaderSource::eraseLayoutAndQualifierSpecifiers(const std::string &name) +void IntermediateShaderSource::eraseLayoutAndQualifierSpecifiers(const std::string &name, + const std::string &replacement) { for (Token &block : mTokens) { - if ((block.type == TokenType::Layout || block.type == TokenType::Qualifier) && - block.text == name) + if (block.type == TokenType::Text || block.text != name) { - block.type = TokenType::Text; - block.text = ""; + continue; } + + block.text = block.type == TokenType::Layout ? "" : replacement; + block.type = TokenType::Text; } } @@ -341,7 +344,7 @@ void GlslangWrapper::GetShaderSource(const gl::ProgramState &programState, continue; } - vertexSource.eraseLayoutAndQualifierSpecifiers(attribute.name); + vertexSource.eraseLayoutAndQualifierSpecifiers(attribute.name, ""); } // Parse output locations and replace them in the fragment shader. @@ -435,8 +438,8 @@ void GlslangWrapper::GetShaderSource(const gl::ProgramState &programState, // Remove all the markers for unused varyings. for (const std::string &varyingName : resources.varyingPacking.getInactiveVaryingNames()) { - vertexSource.eraseLayoutAndQualifierSpecifiers(varyingName); - fragmentSource.eraseLayoutAndQualifierSpecifiers(varyingName); + vertexSource.eraseLayoutAndQualifierSpecifiers(varyingName, ""); + fragmentSource.eraseLayoutAndQualifierSpecifiers(varyingName, ""); } // Assign uniform locations @@ -475,6 +478,13 @@ void GlslangWrapper::GetShaderSource(const gl::ProgramState &programState, ++uniformBlockBinding; } + // Remove all the markers for unused interface blocks, and replace them with |struct|. + for (const std::string &unusedInterfaceBlock : resources.unusedInterfaceBlocks) + { + vertexSource.eraseLayoutAndQualifierSpecifiers(unusedInterfaceBlock, "struct"); + fragmentSource.eraseLayoutAndQualifierSpecifiers(unusedInterfaceBlock, "struct"); + } + // Assign textures to a descriptor set and binding. uint32_t textureBinding = 0; const auto &uniforms = programState.getUniforms(); @@ -532,8 +542,8 @@ void GlslangWrapper::GetShaderSource(const gl::ProgramState &programState, } else { - vertexSource.eraseLayoutAndQualifierSpecifiers(unusedUniform.name); - fragmentSource.eraseLayoutAndQualifierSpecifiers(unusedUniform.name); + vertexSource.eraseLayoutAndQualifierSpecifiers(unusedUniform.name, ""); + fragmentSource.eraseLayoutAndQualifierSpecifiers(unusedUniform.name, ""); } } diff --git a/src/libANGLE/renderer/vulkan/ProgramVk.cpp b/src/libANGLE/renderer/vulkan/ProgramVk.cpp index 2bc505090..e5b6e2e34 100644 --- a/src/libANGLE/renderer/vulkan/ProgramVk.cpp +++ b/src/libANGLE/renderer/vulkan/ProgramVk.cpp @@ -165,6 +165,12 @@ uint32_t GetUniformBlockArraySize(const std::vector &uniform return arraySize; } + +class Std140BlockLayoutEncoderFactory : public gl::CustomBlockLayoutEncoderFactory +{ + public: + sh::BlockLayoutEncoder *makeEncoder() override { return new sh::Std140BlockEncoder(); } +}; } // anonymous namespace // ProgramVk::ShaderInfo implementation. @@ -366,7 +372,8 @@ angle::Result ProgramVk::linkImpl(const gl::Context *glContext, void ProgramVk::linkResources(const gl::ProgramLinkedResources &resources) { - gl::ProgramLinkedResourcesLinker linker(nullptr); + Std140BlockLayoutEncoderFactory std140EncoderFactory; + gl::ProgramLinkedResourcesLinker linker(&std140EncoderFactory); linker.linkResources(mState, resources); } diff --git a/src/libANGLE/renderer/vulkan/ShaderVk.cpp b/src/libANGLE/renderer/vulkan/ShaderVk.cpp index fbba75d39..bc4a25b76 100644 --- a/src/libANGLE/renderer/vulkan/ShaderVk.cpp +++ b/src/libANGLE/renderer/vulkan/ShaderVk.cpp @@ -25,8 +25,7 @@ std::shared_ptr ShaderVk::compile(const gl::Context *conte gl::ShCompilerInstance *compilerInstance, ShCompileOptions options) { - ShCompileOptions compileOptions = - SH_INITIALIZE_UNINITIALIZED_LOCALS | SH_REDEFINE_INTERFACE_LAYOUT_QUALIFIERS_WITH_STD; + ShCompileOptions compileOptions = SH_INITIALIZE_UNINITIALIZED_LOCALS; ContextVk *contextVk = vk::GetImpl(context); diff --git a/src/tests/deqp_support/deqp_gles3_test_expectations.txt b/src/tests/deqp_support/deqp_gles3_test_expectations.txt index c37409e64..f1121c192 100644 --- a/src/tests/deqp_support/deqp_gles3_test_expectations.txt +++ b/src/tests/deqp_support/deqp_gles3_test_expectations.txt @@ -617,6 +617,7 @@ 3219 VULKAN : dEQP-GLES3.functional.shaders.conversions.matrix* = SKIP 3219 VULKAN : dEQP-GLES3.functional.shaders.functions.datatypes.mat* = SKIP 3219 VULKAN : dEQP-GLES3.functional.shaders.matrix.* = SKIP +3443 VULKAN : dEQP-GLES3.functional.ubo.random.basic_arrays.10 = FAIL // New vertex attribute formats: 3193 VULKAN : dEQP-GLES3.functional.vertex_arrays.single_attribute.strides.int2_10_10_10.* = SKIP @@ -714,8 +715,6 @@ 2672 VULKAN : dEQP-GLES3.functional.draw.draw_arrays_instanced.line_loop.* = SKIP 2672 VULKAN : dEQP-GLES3.functional.draw.draw_elements_instanced.line_loop.* = SKIP 2672 VULKAN : dEQP-GLES3.functional.draw.random.* = SKIP -3199 VULKAN : dEQP-GLES3.functional.shaders.linkage.uniform.block.layout_qualifier_mismatch_1 = FAIL -3199 VULKAN : dEQP-GLES3.functional.shaders.linkage.uniform.block.layout_qualifier_mismatch_2 = FAIL // Failures on newer NVIDIA drivers (411.95) and passes on older drivers (388.16). 2976 VULKAN NVIDIA : dEQP-GLES3.functional.shaders.invariance.* = FAIL