Fix handling matrix qualifiers on block members
Individual block member row_major/column_major layout qualifiers may
override the qualifiers set on the block. During parsing, this was
already being handled correctly, so that the qualifier is resolved for
each block member and recorded for each TField / InterfaceBlockField.
Now we always write the qualifiers on a per-member granularity to the
output GLSL shaders, so that the native driver gets the correct
per-member qualifiers. This replaces earlier behavior where the matrix
qualifiers were only written per-block.
Also only use qualifiers from individual members in block layout.
Since the block-level qualifier information is no longer used after
parsing, it is no longer kept in the AST. A dummy value is still set
to the InterfaceBlock structs exposed through the ShaderVars
interface, since that has existing usage in Chromium that needs to be
removed before the field can be removed.
Some AMD OpenGL drivers don't seem to handle matrix layout qualifiers
correctly, so most of the added tests need to be skipped for AMD GL.
On NVIDIA and Intel the tests pass.
BUG=angleproject:2271
TEST=angle_unittests, angle_end2end_tests,
dEQP-GLES31.functional.program_interface_query.uniform.matrix*
Change-Id: I1baa7a633bc2da548743c2190cb72db491b5227a
Reviewed-on: https://chromium-review.googlesource.com/800174
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/CollectVariables.cpp b/src/compiler/translator/CollectVariables.cpp
index bd8cbc9..b35b6ec 100644
--- a/src/compiler/translator/CollectVariables.cpp
+++ b/src/compiler/translator/CollectVariables.cpp
@@ -666,7 +666,8 @@
if (interfaceBlock->blockType == BlockType::BLOCK_UNIFORM ||
interfaceBlock->blockType == BlockType::BLOCK_BUFFER)
{
- interfaceBlock->isRowMajorLayout = (blockType->matrixPacking() == EmpRowMajor);
+ // TODO(oetuaho): Remove setting isRowMajorLayout.
+ interfaceBlock->isRowMajorLayout = false;
interfaceBlock->binding = blockType->blockBinding();
interfaceBlock->layout = GetBlockLayoutType(blockType->blockStorage());
}
diff --git a/src/compiler/translator/OutputGLSLBase.cpp b/src/compiler/translator/OutputGLSLBase.cpp
index 6e0f25f..9f0fb68 100644
--- a/src/compiler/translator/OutputGLSLBase.cpp
+++ b/src/compiler/translator/OutputGLSLBase.cpp
@@ -1221,29 +1221,10 @@
break;
}
- out << ", ";
-
if (interfaceBlock->blockBinding() > 0)
{
- out << "binding = " << interfaceBlock->blockBinding();
out << ", ";
- }
-
- switch (interfaceBlock->matrixPacking())
- {
- case EmpUnspecified:
- case EmpColumnMajor:
- // Default matrix packing is column major.
- out << "column_major";
- break;
-
- case EmpRowMajor:
- out << "row_major";
- break;
-
- default:
- UNREACHABLE();
- break;
+ out << "binding = " << interfaceBlock->blockBinding();
}
out << ") ";
@@ -1255,9 +1236,30 @@
out << hashName(TName(interfaceBlock->name())) << "{\n";
const TFieldList &fields = interfaceBlock->fields();
- for (size_t i = 0; i < fields.size(); ++i)
+ for (const TField *field : fields)
{
- const TField *field = fields[i];
+ if (field->type()->isMatrix() || field->type()->isStructureContainingMatrices())
+ {
+ out << "layout(";
+ switch (field->type()->getLayoutQualifier().matrixPacking)
+ {
+ case EmpUnspecified:
+ case EmpColumnMajor:
+ // Default matrix packing is column major.
+ out << "column_major";
+ break;
+
+ case EmpRowMajor:
+ out << "row_major";
+ break;
+
+ default:
+ UNREACHABLE();
+ break;
+ }
+ out << ") ";
+ }
+
if (writeVariablePrecision(field->type()->getPrecision()))
out << " ";
out << getTypeName(*field->type()) << " " << hashName(TName(field->name()));
diff --git a/src/compiler/translator/Types.cpp b/src/compiler/translator/Types.cpp
index f7c76b6..2c0a649 100644
--- a/src/compiler/translator/Types.cpp
+++ b/src/compiler/translator/Types.cpp
@@ -437,6 +437,11 @@
return mStructure ? mStructure->containsArrays() : false;
}
+bool TType::isStructureContainingMatrices() const
+{
+ return mStructure ? mStructure->containsMatrices() : false;
+}
+
bool TType::isStructureContainingType(TBasicType t) const
{
return mStructure ? mStructure->containsType(t) : false;
@@ -930,6 +935,17 @@
return false;
}
+bool TFieldListCollection::containsMatrices() const
+{
+ for (const auto *field : *mFields)
+ {
+ const TType *fieldType = field->type();
+ if (fieldType->isMatrix() || fieldType->isStructureContainingMatrices())
+ return true;
+ }
+ return false;
+}
+
bool TFieldListCollection::containsType(TBasicType type) const
{
for (const auto *field : *mFields)
@@ -1032,7 +1048,6 @@
mName(name),
mInstanceName(instanceName),
mBlockStorage(layoutQualifier.blockStorage),
- mMatrixPacking(layoutQualifier.matrixPacking),
mBinding(layoutQualifier.binding)
{
}
diff --git a/src/compiler/translator/Types.h b/src/compiler/translator/Types.h
index a5d6fae..380c374 100644
--- a/src/compiler/translator/Types.h
+++ b/src/compiler/translator/Types.h
@@ -55,6 +55,7 @@
const TFieldList &fields() const { return *mFields; }
bool containsArrays() const;
+ bool containsMatrices() const;
bool containsType(TBasicType t) const;
bool containsSamplers() const;
@@ -92,15 +93,15 @@
const TString &instanceName() const { return *mInstanceName; }
bool hasInstanceName() const { return mInstanceName != nullptr; }
TLayoutBlockStorage blockStorage() const { return mBlockStorage; }
- TLayoutMatrixPacking matrixPacking() const { return mMatrixPacking; }
int blockBinding() const { return mBinding; }
private:
const TString *mName;
const TString *mInstanceName; // for interface block instance names
TLayoutBlockStorage mBlockStorage;
- TLayoutMatrixPacking mMatrixPacking;
int mBinding;
+
+ // Note that we only record matrix packing on a per-field granularity.
};
//
@@ -281,6 +282,7 @@
bool isNamelessStruct() const;
bool isStructureContainingArrays() const;
+ bool isStructureContainingMatrices() const;
bool isStructureContainingType(TBasicType t) const;
bool isStructureContainingSamplers() const;
diff --git a/src/compiler/translator/blocklayout.cpp b/src/compiler/translator/blocklayout.cpp
index fd8c450..30f1240 100644
--- a/src/compiler/translator/blocklayout.cpp
+++ b/src/compiler/translator/blocklayout.cpp
@@ -28,6 +28,13 @@
}
template <typename VarT>
+void GetUniformBlockInfo(const std::vector<VarT> &fields,
+ const std::string &prefix,
+ sh::BlockLayoutEncoder *encoder,
+ bool inRowMajorLayout,
+ BlockLayoutMap *blockInfoOut);
+
+template <typename VarT>
void GetUniformBlockStructMemberInfo(const std::vector<VarT> &fields,
const std::string &fieldName,
sh::BlockLayoutEncoder *encoder,
@@ -71,7 +78,7 @@
unsigned int arrayNestingIndex,
const std::string &arrayName,
sh::BlockLayoutEncoder *encoder,
- bool inRowMajorLayout,
+ bool isRowMajorMatrix,
BlockLayoutMap *blockInfoOut)
{
const unsigned int currentArraySize = field.getNestedArraySize(arrayNestingIndex);
@@ -81,14 +88,61 @@
if (arrayNestingIndex + 2u < field.arraySizes.size())
{
GetUniformBlockArrayOfArraysMemberInfo(field, arrayNestingIndex + 1u, elementName,
- encoder, inRowMajorLayout, blockInfoOut);
+ encoder, isRowMajorMatrix, blockInfoOut);
}
else
{
std::vector<unsigned int> innermostArraySize(
1u, field.getNestedArraySize(arrayNestingIndex + 1u));
(*blockInfoOut)[elementName] =
- encoder->encodeType(field.type, innermostArraySize, inRowMajorLayout);
+ encoder->encodeType(field.type, innermostArraySize, isRowMajorMatrix);
+ }
+ }
+}
+
+template <typename VarT>
+void GetUniformBlockInfo(const std::vector<VarT> &fields,
+ const std::string &prefix,
+ sh::BlockLayoutEncoder *encoder,
+ bool inRowMajorLayout,
+ BlockLayoutMap *blockInfoOut)
+{
+ for (const VarT &field : fields)
+ {
+ // Skip samplers. On Vulkan we use this for the default uniform block, so samplers may be
+ // included.
+ if (gl::IsSamplerType(field.type))
+ {
+ continue;
+ }
+
+ const std::string &fieldName = (prefix.empty() ? field.name : prefix + "." + field.name);
+
+ bool rowMajorLayout = (inRowMajorLayout || IsRowMajorLayout(field));
+
+ if (field.isStruct())
+ {
+ if (field.isArray())
+ {
+ GetUniformBlockStructArrayMemberInfo(field, 0u, fieldName, encoder, rowMajorLayout,
+ blockInfoOut);
+ }
+ else
+ {
+ GetUniformBlockStructMemberInfo(field.fields, fieldName, encoder, rowMajorLayout,
+ blockInfoOut);
+ }
+ }
+ else if (field.isArrayOfArrays())
+ {
+ GetUniformBlockArrayOfArraysMemberInfo(field, 0u, fieldName, encoder,
+ rowMajorLayout && gl::IsMatrixType(field.type),
+ blockInfoOut);
+ }
+ else
+ {
+ (*blockInfoOut)[fieldName] = encoder->encodeType(
+ field.type, field.arraySizes, rowMajorLayout && gl::IsMatrixType(field.type));
}
}
}
@@ -212,70 +266,25 @@
}
}
-template <typename VarT>
-void GetUniformBlockInfo(const std::vector<VarT> &fields,
+void GetUniformBlockInfo(const std::vector<InterfaceBlockField> &fields,
const std::string &prefix,
sh::BlockLayoutEncoder *encoder,
- bool inRowMajorLayout,
BlockLayoutMap *blockInfoOut)
{
- for (const VarT &field : fields)
- {
- // Skip samplers. On Vulkan we use this for the default uniform block, so samplers may be
- // included.
- if (gl::IsSamplerType(field.type))
- {
- continue;
- }
-
- const std::string &fieldName = (prefix.empty() ? field.name : prefix + "." + field.name);
-
- if (field.isStruct())
- {
- bool rowMajorLayout = (inRowMajorLayout || IsRowMajorLayout(field));
-
- if (field.isArray())
- {
- GetUniformBlockStructArrayMemberInfo(field, 0u, fieldName, encoder, rowMajorLayout,
- blockInfoOut);
- }
- else
- {
- GetUniformBlockStructMemberInfo(field.fields, fieldName, encoder, rowMajorLayout,
- blockInfoOut);
- }
- }
- else if (field.isArrayOfArrays())
- {
- bool isRowMajorMatrix = (gl::IsMatrixType(field.type) && inRowMajorLayout);
- GetUniformBlockArrayOfArraysMemberInfo(field, 0u, fieldName, encoder, isRowMajorMatrix,
- blockInfoOut);
- }
- else
- {
- bool isRowMajorMatrix = (gl::IsMatrixType(field.type) && inRowMajorLayout);
- (*blockInfoOut)[fieldName] =
- encoder->encodeType(field.type, field.arraySizes, isRowMajorMatrix);
- }
- }
+ // Matrix packing is always recorded in individual fields, so they'll set the row major layout
+ // flag to true if needed.
+ GetUniformBlockInfo(fields, prefix, encoder, false, blockInfoOut);
}
-template void GetUniformBlockInfo(const std::vector<InterfaceBlockField> &,
- const std::string &,
- sh::BlockLayoutEncoder *,
- bool,
- BlockLayoutMap *);
+void GetUniformBlockInfo(const std::vector<Uniform> &uniforms,
+ const std::string &prefix,
+ sh::BlockLayoutEncoder *encoder,
+ BlockLayoutMap *blockInfoOut)
+{
+ // Matrix packing is always recorded in individual fields, so they'll set the row major layout
+ // flag to true if needed.
+ GetUniformBlockInfo(uniforms, prefix, encoder, false, blockInfoOut);
+}
-template void GetUniformBlockInfo(const std::vector<Uniform> &,
- const std::string &,
- sh::BlockLayoutEncoder *,
- bool,
- BlockLayoutMap *);
-
-template void GetUniformBlockInfo(const std::vector<ShaderVariable> &,
- const std::string &,
- sh::BlockLayoutEncoder *,
- bool,
- BlockLayoutMap *);
} // namespace sh
diff --git a/src/compiler/translator/blocklayout.h b/src/compiler/translator/blocklayout.h
index 2b7acf4..4be1835 100644
--- a/src/compiler/translator/blocklayout.h
+++ b/src/compiler/translator/blocklayout.h
@@ -131,13 +131,16 @@
using BlockLayoutMap = std::map<std::string, BlockMemberInfo>;
-// Only valid to call with ShaderVariable, InterfaceBlockField and Uniform.
-template <typename VarT>
-void GetUniformBlockInfo(const std::vector<VarT> &fields,
+void GetUniformBlockInfo(const std::vector<InterfaceBlockField> &fields,
const std::string &prefix,
sh::BlockLayoutEncoder *encoder,
- bool inRowMajorLayout,
- BlockLayoutMap *blockLayoutMap);
+ BlockLayoutMap *blockInfoOut);
+
+// Used for laying out the default uniform block on the Vulkan backend.
+void GetUniformBlockInfo(const std::vector<Uniform> &uniforms,
+ const std::string &prefix,
+ sh::BlockLayoutEncoder *encoder,
+ BlockLayoutMap *blockInfoOut);
} // namespace sh