diff --git a/src/compiler/translator/ShaderStorageBlockOutputHLSL.cpp b/src/compiler/translator/ShaderStorageBlockOutputHLSL.cpp index f13e8c65d329c..b4b4fe012676b 100644 --- a/src/compiler/translator/ShaderStorageBlockOutputHLSL.cpp +++ b/src/compiler/translator/ShaderStorageBlockOutputHLSL.cpp @@ -51,10 +51,13 @@ const TField *GetFieldMemberInShaderStorageBlock(const TInterfaceBlock *interfac void GetShaderStorageBlockFieldMemberInfo(const TFieldList &fields, sh::BlockLayoutEncoder *encoder, TLayoutBlockStorage storage, + bool rowMajor, + bool isSSBOFieldMember, BlockMemberInfoMap *blockInfoOut); size_t GetBlockFieldMemberInfoAndReturnBlockSize(const TFieldList &fields, TLayoutBlockStorage storage, + bool rowMajor, BlockMemberInfoMap *blockInfoOut) { sh::Std140BlockEncoder std140Encoder; @@ -71,28 +74,34 @@ size_t GetBlockFieldMemberInfoAndReturnBlockSize(const TFieldList &fields, structureEncoder = &hlslEncoder; } - GetShaderStorageBlockFieldMemberInfo(fields, structureEncoder, storage, blockInfoOut); + GetShaderStorageBlockFieldMemberInfo(fields, structureEncoder, storage, rowMajor, false, + blockInfoOut); structureEncoder->exitAggregateType(); return structureEncoder->getBlockSize(); } -// TODO(jiajia.qin@intel.com): use correct row major attribute for structure field member. -// http://anglebug.com/1951 void GetShaderStorageBlockFieldMemberInfo(const TFieldList &fields, sh::BlockLayoutEncoder *encoder, TLayoutBlockStorage storage, + bool rowMajor, + bool isSSBOFieldMember, BlockMemberInfoMap *blockInfoOut) { for (const TField *field : fields) { const TType &fieldType = *field->type(); + bool isRowMajorLayout = rowMajor; + if (isSSBOFieldMember) + { + isRowMajorLayout = (fieldType.getLayoutQualifier().matrixPacking == EmpRowMajor); + } if (fieldType.getStruct()) { encoder->enterAggregateType(); // This is to set structure member offset and array stride using a new encoder to ensure // that the first field member offset in structure is always zero. size_t structureStride = GetBlockFieldMemberInfoAndReturnBlockSize( - fieldType.getStruct()->fields(), storage, blockInfoOut); + fieldType.getStruct()->fields(), storage, isRowMajorLayout, blockInfoOut); const BlockMemberInfo memberInfo(static_cast(encoder->getBlockSize()), static_cast(structureStride), 0, false); (*blockInfoOut)[field] = memberInfo; @@ -120,8 +129,6 @@ void GetShaderStorageBlockFieldMemberInfo(const TFieldList &fields, { fieldArraySizes.assign(arraySizes->begin(), arraySizes->end()); } - const bool isRowMajorLayout = - (fieldType.getLayoutQualifier().matrixPacking == EmpRowMajor); const BlockMemberInfo &memberInfo = encoder->encodeType(GLVariableType(fieldType), fieldArraySizes, isRowMajorLayout && fieldType.isMatrix()); @@ -148,7 +155,7 @@ void GetShaderStorageBlockMembersInfo(const TInterfaceBlock *interfaceBlock, } GetShaderStorageBlockFieldMemberInfo(interfaceBlock->fields(), encoder, - interfaceBlock->blockStorage(), blockInfoOut); + interfaceBlock->blockStorage(), false, true, blockInfoOut); } bool IsInArrayOfArraysChain(TIntermTyped *node) @@ -171,6 +178,8 @@ ShaderStorageBlockOutputHLSL::ShaderStorageBlockOutputHLSL(OutputHLSL *outputHLS TSymbolTable *symbolTable, ResourcesHLSL *resourcesHLSL) : TIntermTraverser(true, true, true, symbolTable), + mMatrixStride(0), + mRowMajor(false), mIsLoadFunctionCall(false), mOutputHLSL(outputHLSL), mResourcesHLSL(resourcesHLSL) @@ -448,14 +457,25 @@ void ShaderStorageBlockOutputHLSL::writeEOpIndexDirectOrIndirectOutput(TInfoSink { if (node->getType().isVector() && type.isMatrix()) { - int matrixStride = BlockLayoutEncoder::ComponentsPerRegister * - BlockLayoutEncoder::BytesPerComponent; - out << " + " << str(matrixStride); + if (mRowMajor) + { + out << " + " << BlockLayoutEncoder::BytesPerComponent; + } + else + { + out << " + " << mMatrixStride; + } } else if (node->getType().isScalar() && !type.isArray()) { - int scalarStride = BlockLayoutEncoder::BytesPerComponent; - out << " + " << str(scalarStride); + if (mRowMajor) + { + out << " + " << mMatrixStride; + } + else + { + out << " + " << BlockLayoutEncoder::BytesPerComponent; + } } out << " * "; @@ -486,6 +506,8 @@ void ShaderStorageBlockOutputHLSL::writeDotOperatorOutput(TInfoSinkBase &out, co auto fieldInfoIter = mBlockMemberInfoMap.find(field); ASSERT(fieldInfoIter != mBlockMemberInfoMap.end()); const BlockMemberInfo &memberInfo = fieldInfoIter->second; + mMatrixStride = memberInfo.matrixStride; + mRowMajor = memberInfo.isRowMajorMatrix; out << memberInfo.offset; const TType &fieldType = *field->type(); diff --git a/src/compiler/translator/ShaderStorageBlockOutputHLSL.h b/src/compiler/translator/ShaderStorageBlockOutputHLSL.h index a9b7145ccdd33..305a0366367e7 100644 --- a/src/compiler/translator/ShaderStorageBlockOutputHLSL.h +++ b/src/compiler/translator/ShaderStorageBlockOutputHLSL.h @@ -68,6 +68,8 @@ class ShaderStorageBlockOutputHLSL : public TIntermTraverser // Common part in dot operations. void writeDotOperatorOutput(TInfoSinkBase &out, const TField *field); + int mMatrixStride; + bool mRowMajor; bool mIsLoadFunctionCall; OutputHLSL *mOutputHLSL; ShaderStorageBlockFunctionHLSL *mSSBOFunctionHLSL; diff --git a/src/tests/gl_tests/ShaderStorageBufferTest.cpp b/src/tests/gl_tests/ShaderStorageBufferTest.cpp index ccb68ff4b383b..a4a2347739017 100644 --- a/src/tests/gl_tests/ShaderStorageBufferTest.cpp +++ b/src/tests/gl_tests/ShaderStorageBufferTest.cpp @@ -27,6 +27,52 @@ class ShaderStorageBufferTest31 : public ANGLETest setConfigBlueBits(8); setConfigAlphaBits(8); } + + void runStd140RowMajorMatrixTest(const char *computeShaderSource) + { + ANGLE_GL_COMPUTE_PROGRAM(program, computeShaderSource); + + glUseProgram(program); + + constexpr unsigned int kColumns = 2; + constexpr unsigned int kRows = 3; + constexpr unsigned int kBytesPerComponent = sizeof(float); + constexpr unsigned int kMatrixStride = 16; + // kMatrixStride / kBytesPerComponent is used instead of kColumns is because std140 layout + // requires that base alignment and stride of arrays of scalars and vectors are rounded up a + // multiple of the base alignment of a vec4. + constexpr float kInputDada[kRows][kMatrixStride / kBytesPerComponent] = { + {0.1, 0.2, 0.0, 0.0}, {0.3, 0.4, 0.0, 0.0}, {0.5, 0.6, 0.0, 0.0}}; + // Create shader storage buffer + GLBuffer shaderStorageBuffer[2]; + glBindBuffer(GL_SHADER_STORAGE_BUFFER, shaderStorageBuffer[0]); + glBufferData(GL_SHADER_STORAGE_BUFFER, kRows * kMatrixStride, kInputDada, GL_STATIC_DRAW); + glBindBuffer(GL_SHADER_STORAGE_BUFFER, shaderStorageBuffer[1]); + glBufferData(GL_SHADER_STORAGE_BUFFER, kRows * kMatrixStride, nullptr, GL_STATIC_DRAW); + + // Bind shader storage buffer + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, shaderStorageBuffer[0]); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 1, shaderStorageBuffer[1]); + + glDispatchCompute(1, 1, 1); + glFinish(); + + // Read back shader storage buffer + constexpr float kExpectedValues[kRows][kColumns] = {{0.1, 0.2}, {0.3, 0.4}, {0.5, 0.6}}; + glBindBuffer(GL_SHADER_STORAGE_BUFFER, shaderStorageBuffer[1]); + const GLfloat *ptr = reinterpret_cast( + glMapBufferRange(GL_SHADER_STORAGE_BUFFER, 0, kRows * kMatrixStride, GL_MAP_READ_BIT)); + for (unsigned int idx = 0; idx < kRows; idx++) + { + for (unsigned int idy = 0; idy < kColumns; idy++) + { + EXPECT_EQ(kExpectedValues[idx][idy], + *(ptr + idx * (kMatrixStride / kBytesPerComponent) + idy)); + } + } + + EXPECT_GL_NO_ERROR(); + } }; // Matched block names within a shader interface must match in terms of having the same number of @@ -245,7 +291,73 @@ TEST_P(ShaderStorageBufferTest31, ShaderStorageBufferVector) EXPECT_GL_NO_ERROR(); } -// Test that access/write to matrix data in shader storage buffer. +// Test that access/write to scalar data in matrix in shader storage block with row major. +TEST_P(ShaderStorageBufferTest31, ScalarDataInMatrixInSSBOWithRowMajorQualifier) +{ + // TODO(jiajia.qin@intel.com): Figure out why it fails on Intel Linux platform. + // http://anglebug.com/1951 + ANGLE_SKIP_TEST_IF(IsIntel() && IsLinux()); + ANGLE_SKIP_TEST_IF(IsAndroid()); + + constexpr char kComputeShaderSource[] = + R"(#version 310 es +layout(local_size_x=1, local_size_y=1, local_size_z=1) in; +layout(std140, binding = 0) buffer blockIn { + layout(row_major) mat2x3 data; +} instanceIn; +layout(std140, binding = 1) buffer blockOut { + layout(row_major) mat2x3 data; +} instanceOut; +void main() +{ + instanceOut.data[0][0] = instanceIn.data[0][0]; + instanceOut.data[0][1] = instanceIn.data[0][1]; + instanceOut.data[0][2] = instanceIn.data[0][2]; + instanceOut.data[1][0] = instanceIn.data[1][0]; + instanceOut.data[1][1] = instanceIn.data[1][1]; + instanceOut.data[1][2] = instanceIn.data[1][2]; +} +)"; + + runStd140RowMajorMatrixTest(kComputeShaderSource); +} + +// Test that access/write to scalar data in structure matrix in shader storage block with row major. +TEST_P(ShaderStorageBufferTest31, ScalarDataInMatrixInStructureInSSBOWithRowMajorQualifier) +{ + // TODO(jiajia.qin@intel.com): Figure out why it fails on Intel Linux platform. + // http://anglebug.com/1951 + ANGLE_SKIP_TEST_IF(IsIntel() && IsLinux()); + ANGLE_SKIP_TEST_IF(IsAndroid()); + + constexpr char kComputeShaderSource[] = + R"(#version 310 es +layout(local_size_x=1, local_size_y=1, local_size_z=1) in; +struct S +{ + mat2x3 data; +}; +layout(std140, binding = 0) buffer blockIn { + layout(row_major) S s; +} instanceIn; +layout(std140, binding = 1) buffer blockOut { + layout(row_major) S s; +} instanceOut; +void main() +{ + instanceOut.s.data[0][0] = instanceIn.s.data[0][0]; + instanceOut.s.data[0][1] = instanceIn.s.data[0][1]; + instanceOut.s.data[0][2] = instanceIn.s.data[0][2]; + instanceOut.s.data[1][0] = instanceIn.s.data[1][0]; + instanceOut.s.data[1][1] = instanceIn.s.data[1][1]; + instanceOut.s.data[1][2] = instanceIn.s.data[1][2]; +} +)"; + + runStd140RowMajorMatrixTest(kComputeShaderSource); +} + +// Test that access/write to column major matrix data in shader storage buffer. TEST_P(ShaderStorageBufferTest31, ShaderStorageBufferMatrix) { constexpr char kComputeShaderSource[] =