Commit b91900a2 by Tim Van Patten Committed by Commit Bot

Vulkan: Fix khr-gles3.shaders.uniform_block.common.name_matching test failure

Fix various shader translator validation errors related to uniform buffers and naming. Bug: angleproject:3459 Test: angle_deqp_khr_gles3_tests Change-Id: Iaa66b61e91c8f38ec7cccb43d71be9ba3cd83da3 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1684302 Commit-Queue: Tim Van Patten <timvp@google.com> Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org>
parent 69e46a18
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "libANGLE/Program.h" #include "libANGLE/Program.h"
#include <algorithm> #include <algorithm>
#include <utility>
#include "common/bitset_utils.h" #include "common/bitset_utils.h"
#include "common/debug.h" #include "common/debug.h"
...@@ -433,7 +434,9 @@ const char *GetLinkMismatchErrorString(LinkMismatchError linkError) ...@@ -433,7 +434,9 @@ const char *GetLinkMismatchErrorString(LinkMismatchError linkError)
case LinkMismatchError::LOCATION_MISMATCH: case LinkMismatchError::LOCATION_MISMATCH:
return "Location layout qualifier"; return "Location layout qualifier";
case LinkMismatchError::OFFSET_MISMATCH: case LinkMismatchError::OFFSET_MISMATCH:
return "Offset layout qualilfier"; return "Offset layout qualifier";
case LinkMismatchError::INSTANCE_NAME_MISMATCH:
return "Instance name qualifier";
case LinkMismatchError::LAYOUT_QUALIFIER_MISMATCH: case LinkMismatchError::LAYOUT_QUALIFIER_MISMATCH:
return "Layout qualifier"; return "Layout qualifier";
...@@ -492,6 +495,10 @@ LinkMismatchError AreMatchingInterfaceBlocks(const sh::InterfaceBlock &interface ...@@ -492,6 +495,10 @@ LinkMismatchError AreMatchingInterfaceBlocks(const sh::InterfaceBlock &interface
{ {
return LinkMismatchError::LAYOUT_QUALIFIER_MISMATCH; return LinkMismatchError::LAYOUT_QUALIFIER_MISMATCH;
} }
if (interfaceBlock1.instanceName.empty() != interfaceBlock2.instanceName.empty())
{
return LinkMismatchError::INSTANCE_NAME_MISMATCH;
}
const unsigned int numBlockMembers = static_cast<unsigned int>(interfaceBlock1.fields.size()); const unsigned int numBlockMembers = static_cast<unsigned int>(interfaceBlock1.fields.size());
for (unsigned int blockMemberIndex = 0; blockMemberIndex < numBlockMembers; blockMemberIndex++) for (unsigned int blockMemberIndex = 0; blockMemberIndex < numBlockMembers; blockMemberIndex++)
{ {
...@@ -3666,29 +3673,109 @@ bool Program::linkValidateGlobalNames(InfoLog &infoLog) const ...@@ -3666,29 +3673,109 @@ bool Program::linkValidateGlobalNames(InfoLog &infoLog) const
{ {
const std::vector<sh::Attribute> &attributes = const std::vector<sh::Attribute> &attributes =
mState.mAttachedShaders[ShaderType::Vertex]->getActiveAttributes(); mState.mAttachedShaders[ShaderType::Vertex]->getActiveAttributes();
std::unordered_map<std::string, const sh::Uniform *> uniformMap;
using BlockAndFieldPair =
std::pair<const sh::InterfaceBlock *, const sh::InterfaceBlockField *>;
std::unordered_map<std::string, std::vector<BlockAndFieldPair>> uniformBlockFieldMap;
for (const auto &attrib : attributes) for (ShaderType shaderType : kAllGraphicsShaderTypes)
{ {
for (ShaderType shaderType : kAllGraphicsShaderTypes) Shader *shader = mState.mAttachedShaders[shaderType];
if (!shader)
{ {
Shader *shader = mState.mAttachedShaders[shaderType]; continue;
if (!shader) }
// Build a map of Uniforms
const std::vector<sh::Uniform> uniforms = shader->getUniforms();
for (const auto &uniform : uniforms)
{
uniformMap[uniform.name] = &uniform;
}
// Build a map of Uniform Blocks
// This will also detect any field name conflicts between Uniform Blocks without instance
// names
const std::vector<sh::InterfaceBlock> &uniformBlocks = shader->getUniformBlocks();
for (const auto &uniformBlock : uniformBlocks)
{
// Only uniform blocks without an instance name can create a conflict with their field
// names
if (!uniformBlock.instanceName.empty())
{ {
continue; continue;
} }
const std::vector<sh::Uniform> &uniforms = shader->getUniforms(); for (const auto &field : uniformBlock.fields)
for (const auto &uniform : uniforms)
{ {
if (uniform.name == attrib.name) if (!uniformBlockFieldMap.count(field.name))
{ {
infoLog << "Name conflicts between a uniform and an attribute: " << attrib.name; // First time we've seen this uniform block field name, so add the
return false; // (Uniform Block, Field) pair immediately since there can't be a conflict yet
BlockAndFieldPair blockAndFieldPair(&uniformBlock, &field);
std::vector<BlockAndFieldPair> newUniformBlockList;
newUniformBlockList.push_back(blockAndFieldPair);
uniformBlockFieldMap[field.name] = newUniformBlockList;
continue;
}
// We've seen this name before.
// We need to check each of the uniform blocks that contain a field with this name
// to see if there's a conflict or not.
std::vector<BlockAndFieldPair> prevBlockFieldPairs =
uniformBlockFieldMap[field.name];
for (const auto prevBlockFieldPair : prevBlockFieldPairs)
{
const sh::InterfaceBlock *prevUniformBlock = prevBlockFieldPair.first;
const sh::InterfaceBlockField *prevUniformBlockField =
prevBlockFieldPair.second;
if (uniformBlock.isSameInterfaceBlockAtLinkTime(*prevUniformBlock))
{
// The same uniform block should, by definition, contain the same field name
continue;
}
// The uniform blocks don't match, so check if the necessary field properties
// also match
if ((field.name == prevUniformBlockField->name) &&
(field.type == prevUniformBlockField->type) &&
(field.precision == prevUniformBlockField->precision))
{
infoLog << "Name conflicts between uniform block field names: "
<< field.name;
return false;
}
} }
// No conflict, so record this pair
BlockAndFieldPair blockAndFieldPair(&uniformBlock, &field);
uniformBlockFieldMap[field.name].push_back(blockAndFieldPair);
} }
} }
} }
// Validate no uniform names conflict with attribute names
for (const auto &attrib : attributes)
{
if (uniformMap.count(attrib.name))
{
infoLog << "Name conflicts between a uniform and an attribute: " << attrib.name;
return false;
}
}
// Validate no Uniform Block fields conflict with other Uniforms
for (const auto &uniformBlockField : uniformBlockFieldMap)
{
const std::string &fieldName = uniformBlockField.first;
if (uniformMap.count(fieldName))
{
infoLog << "Name conflicts between a uniform and a uniform block field: " << fieldName;
return false;
}
}
return true; return true;
} }
......
...@@ -75,6 +75,7 @@ enum class LinkMismatchError ...@@ -75,6 +75,7 @@ enum class LinkMismatchError
BINDING_MISMATCH, BINDING_MISMATCH,
LOCATION_MISMATCH, LOCATION_MISMATCH,
OFFSET_MISMATCH, OFFSET_MISMATCH,
INSTANCE_NAME_MISMATCH,
// Interface block specific // Interface block specific
LAYOUT_QUALIFIER_MISMATCH, LAYOUT_QUALIFIER_MISMATCH,
......
...@@ -52,6 +52,3 @@ ...@@ -52,6 +52,3 @@
// Require 3D textures. // Require 3D textures.
3188 VULKAN : KHR-GLES3.packed_pixels.varied_rectangle.* = SKIP 3188 VULKAN : KHR-GLES3.packed_pixels.varied_rectangle.* = SKIP
// Shader related failure. Not Vulkan-specific.
3459 : KHR-GLES3.shaders.uniform_block.common.name_matching = FAIL
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment