Row major matrix packing fix
Row major matrix packing wasn't properly detected when the layout
qualifier was on the block member rather than on the block itself
or when a nested structure had a matrix packing qualifier.
Fixes all failing tests in:
deqp/functional/gles3/uniformbuffers*
No regressions in:
dEQP-GLES3.functional.ubo*
Change-Id: I1549a70c4286a8a84b695bc876d71d9cf636b306
Reviewed-on: https://swiftshader-review.googlesource.com/16588
Tested-by: Alexis Hétu <sugoi@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/OpenGL/compiler/OutputASM.cpp b/src/OpenGL/compiler/OutputASM.cpp
index 76a177a..4ef41be 100644
--- a/src/OpenGL/compiler/OutputASM.cpp
+++ b/src/OpenGL/compiler/OutputASM.cpp
@@ -285,8 +285,8 @@
{
}
- BlockLayoutEncoder::BlockLayoutEncoder(bool rowMajor)
- : mCurrentOffset(0), isRowMajor(rowMajor)
+ BlockLayoutEncoder::BlockLayoutEncoder()
+ : mCurrentOffset(0)
{
}
@@ -295,20 +295,15 @@
int arrayStride;
int matrixStride;
- bool isVariableRowMajor = isRowMajor;
- TLayoutMatrixPacking matrixPacking = type.getLayoutQualifier().matrixPacking;
- if(matrixPacking != EmpUnspecified)
- {
- isVariableRowMajor = (matrixPacking == EmpRowMajor);
- }
- getBlockLayoutInfo(type, type.getArraySize(), isVariableRowMajor, &arrayStride, &matrixStride);
+ bool isRowMajor = type.getLayoutQualifier().matrixPacking == EmpRowMajor;
+ getBlockLayoutInfo(type, type.getArraySize(), isRowMajor, &arrayStride, &matrixStride);
const BlockMemberInfo memberInfo(static_cast<int>(mCurrentOffset * BytesPerComponent),
static_cast<int>(arrayStride * BytesPerComponent),
static_cast<int>(matrixStride * BytesPerComponent),
- (matrixStride > 0) && isVariableRowMajor);
+ (matrixStride > 0) && isRowMajor);
- advanceOffset(type, type.getArraySize(), isVariableRowMajor, arrayStride, matrixStride);
+ advanceOffset(type, type.getArraySize(), isRowMajor, arrayStride, matrixStride);
return memberInfo;
}
@@ -330,7 +325,7 @@
mCurrentOffset = sw::align(mCurrentOffset, ComponentsPerRegister);
}
- Std140BlockEncoder::Std140BlockEncoder(bool rowMajor) : BlockLayoutEncoder(rowMajor)
+ Std140BlockEncoder::Std140BlockEncoder() : BlockLayoutEncoder()
{
}
@@ -2376,7 +2371,7 @@
arg = &unpackedUniform;
index = 0;
}
- else if((srcBlock->matrixPacking() == EmpRowMajor) && memberType.isMatrix())
+ else if((memberType.getLayoutQualifier().matrixPacking == EmpRowMajor) && memberType.isMatrix())
{
int numCols = memberType.getNominalSize();
int numRows = memberType.getSecondarySize();
@@ -3591,7 +3586,7 @@
block->blockStorage(), isRowMajor, registerIndex, blockId));
blockDefinitions.push_back(BlockDefinitionIndexMap());
- Std140BlockEncoder currentBlockEncoder(isRowMajor);
+ Std140BlockEncoder currentBlockEncoder;
currentBlockEncoder.enterAggregateType();
for(const auto &field : fields)
{
diff --git a/src/OpenGL/compiler/OutputASM.h b/src/OpenGL/compiler/OutputASM.h
index f6fd184..a1d16f8 100644
--- a/src/OpenGL/compiler/OutputASM.h
+++ b/src/OpenGL/compiler/OutputASM.h
@@ -99,7 +99,7 @@
class BlockLayoutEncoder
{
public:
- BlockLayoutEncoder(bool rowMajor);
+ BlockLayoutEncoder();
virtual ~BlockLayoutEncoder() {}
BlockMemberInfo encodeType(const TType &type);
@@ -117,7 +117,6 @@
protected:
size_t mCurrentOffset;
- bool isRowMajor;
void nextRegister();
@@ -130,7 +129,7 @@
class Std140BlockEncoder : public BlockLayoutEncoder
{
public:
- Std140BlockEncoder(bool rowMajor);
+ Std140BlockEncoder();
void enterAggregateType() override;
void exitAggregateType() override;
diff --git a/src/OpenGL/compiler/ParseHelper.cpp b/src/OpenGL/compiler/ParseHelper.cpp
index a7d7d01..4f6ffff 100644
--- a/src/OpenGL/compiler/ParseHelper.cpp
+++ b/src/OpenGL/compiler/ParseHelper.cpp
@@ -781,9 +781,8 @@
return true;
if (type.getBasicType() == EbtStruct || type.isInterfaceBlock()) {
- const TFieldList& fields = type.getStruct()->fields();
- for(unsigned int i = 0; i < fields.size(); ++i) {
- if (containsSampler(*fields[i]->type()))
+ for(const auto &field : type.getStruct()->fields()) {
+ if (containsSampler(*(field->type())))
return true;
}
}
@@ -2296,11 +2295,11 @@
size_t instanceSize = 0;
TIntermConstantUnion *tempConstantNode = node->getAsConstantUnion();
- for(size_t index = 0; index < fields.size(); ++index) {
- if (fields[index]->name() == identifier) {
+ for(const auto &field : fields) {
+ if (field->name() == identifier) {
break;
} else {
- instanceSize += fields[index]->type()->getObjectSize();
+ instanceSize += field->type()->getObjectSize();
}
}
@@ -2356,8 +2355,7 @@
}
// check for sampler types and apply layout qualifiers
- for(size_t memberIndex = 0; memberIndex < fieldList->size(); ++memberIndex) {
- TField* field = (*fieldList)[memberIndex];
+ for(const auto &field : *fieldList) {
TType* fieldType = field->type();
if(IsSampler(fieldType->getBasicType())) {
error(field->line(), "unsupported type", fieldType->getBasicString(), "sampler types are not allowed in interface blocks");
@@ -2399,6 +2397,9 @@
}
fieldType->setLayoutQualifier(fieldLayoutQualifier);
+
+ // Recursively propagate the matrix packing setting down to all block/structure members
+ fieldType->setMatrixPackingIfUnspecified(fieldLayoutQualifier.matrixPacking);
}
// add array index
@@ -2418,9 +2419,8 @@
if(!instanceName)
{
// define symbols for the members of the interface block
- for(size_t memberIndex = 0; memberIndex < fieldList->size(); ++memberIndex)
+ for(const auto &field : *fieldList)
{
- TField* field = (*fieldList)[memberIndex];
TType* fieldType = field->type();
// set parent pointer of the field variable
@@ -2925,12 +2925,12 @@
recover();
}
- for(unsigned int i = 0; i < fieldList->size(); ++i)
+ for(const auto &field : *fieldList)
{
//
// Careful not to replace already known aspects of type, like array-ness
//
- TType *type = (*fieldList)[i]->type();
+ TType *type = field->type();
type->setBasicType(typeSpecifier.type);
type->setNominalSize(typeSpecifier.primarySize);
type->setSecondarySize(typeSpecifier.secondarySize);
@@ -2951,7 +2951,7 @@
type->setStruct(typeSpecifier.userDef->getStruct());
}
- if(structNestingErrorCheck(typeSpecifier.line, *(*fieldList)[i]))
+ if(structNestingErrorCheck(typeSpecifier.line, *field))
{
recover();
}
@@ -2986,17 +2986,16 @@
}
// ensure we do not specify any storage qualifiers on the struct members
- for(unsigned int typeListIndex = 0; typeListIndex < fieldList->size(); typeListIndex++)
+ for(const auto &field : *fieldList)
{
- const TField &field = *(*fieldList)[typeListIndex];
- const TQualifier qualifier = field.type()->getQualifier();
+ const TQualifier qualifier = field->type()->getQualifier();
switch(qualifier)
{
case EvqGlobal:
case EvqTemporary:
break;
default:
- error(field.line(), "invalid qualifier on struct member", getQualifierString(qualifier));
+ error(field->line(), "invalid qualifier on struct member", getQualifierString(qualifier));
recover();
break;
}
diff --git a/src/OpenGL/compiler/SymbolTable.cpp b/src/OpenGL/compiler/SymbolTable.cpp
index 5d7ad33..0269efc 100644
--- a/src/OpenGL/compiler/SymbolTable.cpp
+++ b/src/OpenGL/compiler/SymbolTable.cpp
@@ -113,9 +113,9 @@
bool TStructure::containsArrays() const
{
- for(size_t i = 0; i < mFields->size(); ++i)
+ for(const auto& field : *mFields)
{
- const TType *fieldType = (*mFields)[i]->type();
+ const TType *fieldType = field->type();
if(fieldType->isArray() || fieldType->isStructureContainingArrays())
return true;
}
@@ -124,9 +124,9 @@
bool TStructure::containsType(TBasicType type) const
{
- for(size_t i = 0; i < mFields->size(); ++i)
+ for(const auto& field : *mFields)
{
- const TType *fieldType = (*mFields)[i]->type();
+ const TType *fieldType = field->type();
if(fieldType->getBasicType() == type || fieldType->isStructureContainingType(type))
return true;
}
@@ -135,23 +135,31 @@
bool TStructure::containsSamplers() const
{
- for(size_t i = 0; i < mFields->size(); ++i)
+ for(const auto& field : *mFields)
{
- const TType *fieldType = (*mFields)[i]->type();
+ const TType *fieldType = field->type();
if(IsSampler(fieldType->getBasicType()) || fieldType->isStructureContainingSamplers())
return true;
}
return false;
}
+void TStructure::setMatrixPackingIfUnspecified(TLayoutMatrixPacking matrixPacking)
+{
+ for(auto& field : *mFields)
+ {
+ field->type()->setMatrixPackingIfUnspecified(matrixPacking);
+ }
+}
+
TString TFieldListCollection::buildMangledName() const
{
TString mangledName(mangledNamePrefix());
mangledName += *mName;
- for(size_t i = 0; i < mFields->size(); ++i)
+ for(const auto& field : *mFields)
{
mangledName += '-';
- mangledName += (*mFields)[i]->type()->getMangledName();
+ mangledName += field->type()->getMangledName();
}
return mangledName;
}
@@ -159,9 +167,9 @@
size_t TFieldListCollection::calculateObjectSize() const
{
size_t size = 0;
- for(size_t i = 0; i < mFields->size(); ++i)
+ for(const auto& field : *mFields)
{
- size_t fieldSize = (*mFields)[i]->type()->getObjectSize();
+ size_t fieldSize = field->type()->getObjectSize();
if(fieldSize > INT_MAX - size)
size = INT_MAX;
else
@@ -173,8 +181,8 @@
int TStructure::calculateDeepestNesting() const
{
int maxNesting = 0;
- for(size_t i = 0; i < mFields->size(); ++i)
- maxNesting = std::max(maxNesting, (*mFields)[i]->type()->getDeepestStructNesting());
+ for(const auto& field : *mFields)
+ maxNesting = std::max(maxNesting, field->type()->getDeepestStructNesting());
return 1 + maxNesting;
}
diff --git a/src/OpenGL/compiler/Types.h b/src/OpenGL/compiler/Types.h
index 960352f..4b11498 100644
--- a/src/OpenGL/compiler/Types.h
+++ b/src/OpenGL/compiler/Types.h
@@ -137,6 +137,8 @@
bool equals(const TStructure &other) const;
+ void setMatrixPackingIfUnspecified(TLayoutMatrixPacking matrixPacking);
+
void setUniqueId(int uniqueId)
{
mUniqueId = uniqueId;
@@ -282,6 +284,21 @@
TLayoutQualifier getLayoutQualifier() const { return layoutQualifier; }
void setLayoutQualifier(TLayoutQualifier lq) { layoutQualifier = lq; }
+ void setMatrixPackingIfUnspecified(TLayoutMatrixPacking matrixPacking)
+ {
+ if(isStruct())
+ {
+ // If the structure's matrix packing is specified, it overrules the block's matrix packing
+ structure->setMatrixPackingIfUnspecified((layoutQualifier.matrixPacking == EmpUnspecified) ?
+ matrixPacking : layoutQualifier.matrixPacking);
+ }
+ // If the member's matrix packing is specified, it overrules any higher level matrix packing
+ if(layoutQualifier.matrixPacking == EmpUnspecified)
+ {
+ layoutQualifier.matrixPacking = matrixPacking;
+ }
+ }
+
// One-dimensional size of single instance type
int getNominalSize() const { return primarySize; }
void setNominalSize(int s) { primarySize = s; }
