D3D: fix uniform block alignment error

src/compiler/translator/ResourcesHLSL.cpp

@@ -915,7 +915,8 @@ TString ResourcesHLSL::uniformBlockMembersString(const TInterfaceBlock &interfac
 
     Std140PaddingHelper padHelper = mStructureHLSL->getPaddingHelper();
 
-    for (unsigned int typeIndex = 0; typeIndex < interfaceBlock.fields().size(); typeIndex++)
+    const unsigned int fieldCount = static_cast<unsigned int>(interfaceBlock.fields().size());
+    for (unsigned int typeIndex = 0; typeIndex < fieldCount; typeIndex++)
     {
         const TField &field    = *interfaceBlock.fields()[typeIndex];
         const TType &fieldType = *field.type();
@@ -935,7 +936,8 @@ TString ResourcesHLSL::uniformBlockMembersString(const TInterfaceBlock &interfac
         {
             const bool useHLSLRowMajorPacking =
                 (fieldType.getLayoutQualifier().matrixPacking == EmpColumnMajor);
-            hlsl += padHelper.postPaddingString(fieldType, useHLSLRowMajorPacking, false);
+            hlsl += padHelper.postPaddingString(fieldType, useHLSLRowMajorPacking,
+                                                typeIndex == fieldCount - 1, false);
         }
     }
 

src/compiler/translator/StructureHLSL.cpp

@@ -62,7 +62,7 @@ TString Define(const TStructure &structure,
             if (padHelper)
             {
                 string += padHelper->postPaddingString(fieldType, useHLSLRowMajorPacking,
-                                                       memberSize == 0 && forcePadding);
+                                                       memberSize == 0, forcePadding);
             }
         }
     }
@@ -92,6 +92,12 @@ TString WriteParameterList(const std::vector<TType> &parameters)
     return parameterList;
 }
 
+int GetElementPadding(int elementIndex, int alignment)
+{
+    const int paddingOffset = elementIndex % alignment;
+    return paddingOffset != 0 ? (alignment - paddingOffset) : 0;
+}
+
 }  // anonymous namespace
 
 Std140PaddingHelper::Std140PaddingHelper(const std::map<TString, int> &structElementIndexes,
@@ -123,9 +129,19 @@ int Std140PaddingHelper::prePadding(const TType &type, bool forcePadding)
 {
     if (type.getBasicType() == EbtStruct || type.isMatrix() || type.isArray())
     {
-        // no padding needed, HLSL will align the field to a new register
-        mElementIndex = 0;
-        return 0;
+        if (forcePadding)
+        {
+            // Add padding between the structure's members to follow the std140 rules manually.
+            const int forcePaddingCount = GetElementPadding(mElementIndex, 4);
+            mElementIndex               = 0;
+            return forcePaddingCount;
+        }
+        else
+        {
+            // no padding needed, HLSL will align the field to a new register
+            mElementIndex = 0;
+            return 0;
+        }
     }
 
     const GLenum glType     = GLVariableType(type);
@@ -133,18 +149,27 @@ int Std140PaddingHelper::prePadding(const TType &type, bool forcePadding)
 
     if (numComponents >= 4)
     {
-        // no padding needed, HLSL will align the field to a new register
-        mElementIndex = 0;
-        return 0;
+        if (forcePadding)
+        {
+            // Add padding between the structure's members to follow the std140 rules manually.
+            const int forcePaddingCount = GetElementPadding(mElementIndex, 4);
+            mElementIndex               = numComponents % 4;
+            return forcePaddingCount;
+        }
+        else
+        {
+            // no padding needed, HLSL will align the field to a new register
+            mElementIndex = 0;
+            return 0;
+        }
     }
 
     if (mElementIndex + numComponents > 4)
     {
         if (forcePadding)
         {
-            // If this structure will be used as HLSL StructuredBuffer member's type, we should add
-            // padding between the structure's members to follow the std140 rules manually.
-            const int forcePaddingCount = 4 - mElementIndex;
+            // Add padding between the structure's members to follow the std140 rules manually.
+            const int forcePaddingCount = GetElementPadding(mElementIndex, 4);
             mElementIndex               = numComponents;
             return forcePaddingCount;
         }
@@ -156,9 +181,8 @@ int Std140PaddingHelper::prePadding(const TType &type, bool forcePadding)
         }
     }
 
-    const int alignment     = numComponents == 3 ? 4 : numComponents;
-    const int paddingOffset = (mElementIndex % alignment);
-    const int paddingCount  = (paddingOffset != 0 ? (alignment - paddingOffset) : 0);
+    const int alignment    = numComponents == 3 ? 4 : numComponents;
+    const int paddingCount = GetElementPadding(mElementIndex, alignment);
 
     mElementIndex += paddingCount;
     mElementIndex += numComponents;
@@ -183,23 +207,33 @@ TString Std140PaddingHelper::prePaddingString(const TType &type, bool forcePaddi
 
 TString Std140PaddingHelper::postPaddingString(const TType &type,
                                                bool useHLSLRowMajorPacking,
+                                               bool isLastElement,
                                                bool forcePadding)
 {
     if (!type.isMatrix() && !type.isArray() && type.getBasicType() != EbtStruct)
     {
         if (forcePadding)
         {
-            // If this structure will be used as HLSL StructuredBuffer member's type, we
-            // should force to pad the end of the structure to follow the std140 rules.
-            TString forcePaddingStr;
-            const int paddingOffset = mElementIndex % 4;
-            const int paddingCount  = paddingOffset != 0 ? (4 - paddingOffset) : 0;
-            for (int paddingIndex = 0; paddingIndex < paddingCount; paddingIndex++)
+            const GLenum glType     = GLVariableType(type);
+            const int numComponents = gl::VariableComponentCount(glType);
+            if (isLastElement || (numComponents >= 4))
             {
-                forcePaddingStr += "    float pad_" + next() + ";\n";
+                // If this structure will be used as HLSL StructuredBuffer member's type, in
+                // order to follow the std140 rules, add padding at the end of the structure
+                // if necessary. Or if the current element straddles a vec4 boundary, add
+                // padding to round up the base offset of the next element to the base
+                // alignment of a vec4.
+                TString forcePaddingStr;
+                const int paddingCount = GetElementPadding(mElementIndex, 4);
+                for (int paddingIndex = 0; paddingIndex < paddingCount; paddingIndex++)
+                {
+                    forcePaddingStr += "    float pad_" + next() + ";\n";
+                }
+                mElementIndex = 0;
+                return forcePaddingStr;
             }
-            return forcePaddingStr;
         }
+
         return "";
     }
 

src/compiler/translator/StructureHLSL.h

@@ -34,7 +34,10 @@ class Std140PaddingHelper
     int elementIndex() const { return mElementIndex; }
     int prePadding(const TType &type, bool forcePadding);
     TString prePaddingString(const TType &type, bool forcePadding);
-    TString postPaddingString(const TType &type, bool useHLSLRowMajorPacking, bool forcePadding);
+    TString postPaddingString(const TType &type,
+                              bool useHLSLRowMajorPacking,
+                              bool isLastElement,
+                              bool forcePadding);
 
   private:
     TString next();

src/tests/gl_tests/UniformBufferTest.cpp

@@ -3003,6 +3003,134 @@ TEST_P(UniformBlockWithOneLargeArrayMemberTest, MemberTypeIsFloat)
     checkResults(GLColor::red, GLColor::black, GLColor::red, GLColor::red);
 }
 
+// Test uniform block whose member is structure type, which contains a float member and a mat4
+// member.
+TEST_P(UniformBlockWithOneLargeArrayMemberTest, MemberTypeIsMixStructFloatAndMat4)
+{
+    std::ostringstream stream;
+    generateArraySizeAndDivisorsDeclaration(stream, false, true, false);
+    const std::string &kFS =
+        "#version 300 es\n"
+        "precision highp float;\n" +
+        stream.str() +
+        "out vec4 my_FragColor;\n"
+        "struct S { float factor; mat4 color; };\n"
+        "layout(std140) uniform buffer { S s[arraySize]; };\n"
+        "void main()\n"
+        "{\n"
+        "    uvec2 coord = uvec2(floor(gl_FragCoord.xy));\n"
+        "    uint index = coord.x +  coord.y * 128u;\n"
+        "    uint index_x = index / divisor1;\n"
+        "    uint index_y = (index % divisor1) / divisor2;\n"
+        "    my_FragColor = s[index_x].factor * s[index_x].color[index_y];\n"
+        "}\n";
+
+    GLint blockSize;
+    ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS.c_str());
+    GLint uniformBufferIndex = glGetUniformBlockIndex(program, "buffer");
+    glGetActiveUniformBlockiv(program, uniformBufferIndex, GL_UNIFORM_BLOCK_DATA_SIZE, &blockSize);
+
+    glBindBuffer(GL_UNIFORM_BUFFER, mUniformBuffer);
+    glBufferData(GL_UNIFORM_BUFFER, blockSize, nullptr, GL_STATIC_DRAW);
+
+    glBindBufferBase(GL_UNIFORM_BUFFER, 0, mUniformBuffer);
+    glUniformBlockBinding(program, uniformBufferIndex, 0);
+
+    // The member s is an array of S structures, each element of s should be rounded up
+    // to the base alignment of a vec4 according to std140 storage layout rules.
+    const GLuint arraySize  = getArraySize();
+    const GLuint floatCount = arraySize * (kVectorPerMat * kFloatPerVector + kFloatPerVector);
+    std::vector<GLfloat> floatData(floatCount, 0.0f);
+    const size_t strideofFloatCount = kVectorPerMat * kFloatPerVector + kFloatPerVector;
+
+    setArrayValues(floatData, 0, arraySize, strideofFloatCount, 0, 1, 1, 2.0f, 0.0f, 0.0f, 0.0f, 4,
+                   4, 4, 0.0f, 0.0f, 0.5f, 0.5f);
+    glBufferSubData(GL_UNIFORM_BUFFER, 0,
+                    std::min(static_cast<size_t>(blockSize), floatCount * sizeof(GLfloat)),
+                    floatData.data());
+    drawQuad(program.get(), essl3_shaders::PositionAttrib(), 0.5f);
+    checkResults(GLColor::blue, GLColor::blue, GLColor::blue, GLColor::blue);
+
+    setArrayValues(floatData, 0, arraySize, strideofFloatCount, 0, 1, 1, 2.0f, 0.0f, 0.0f, 0.0f, 4,
+                   4, 4, 0.0f, 0.5f, 0.0f, 0.5f);
+    glBufferSubData(GL_UNIFORM_BUFFER, 0,
+                    std::min(static_cast<size_t>(blockSize), floatCount * sizeof(GLfloat)),
+                    floatData.data());
+    drawQuad(program.get(), essl3_shaders::PositionAttrib(), 0.5f);
+    checkResults(GLColor::green, GLColor::green, GLColor::green, GLColor::green);
+
+    setArrayValues(floatData, arraySize / 4, arraySize / 2, strideofFloatCount, 0, 1, 1, 2.0f, 0.0f,
+                   0.0f, 0.0f, 4, 4, 4, 0.5f, 0.0f, 0.0f, 0.5f);
+    glBufferSubData(GL_UNIFORM_BUFFER, 0,
+                    std::min(static_cast<size_t>(blockSize), floatCount * sizeof(GLfloat)),
+                    floatData.data());
+    drawQuad(program.get(), essl3_shaders::PositionAttrib(), 0.5f);
+    checkResults(GLColor::green, GLColor::red, GLColor::green, GLColor::green);
+}
+
+// Test uniform block whose member is structure type, which contains a float member and a vec4
+// member.
+TEST_P(UniformBlockWithOneLargeArrayMemberTest, MemberTypeIsMixStructFloatAndVec4)
+{
+    ANGLE_SKIP_TEST_IF(IsOSX());
+
+    std::ostringstream stream;
+    generateArraySizeAndDivisorsDeclaration(stream, false, false, false);
+    const std::string &kFS =
+        "#version 300 es\n"
+        "precision highp float;\n" +
+        stream.str() +
+        "out vec4 my_FragColor;\n"
+        "struct S { float color1; vec4 color2; };\n"
+        "layout(std140) uniform buffer { S s[arraySize]; };\n"
+        "void main()\n"
+        "{\n"
+        "    uvec2 coord = uvec2(floor(gl_FragCoord.xy));\n"
+        "    uint index = (coord.x +  coord.y * 128u) / divisor;\n"
+        "    my_FragColor = vec4(s[index].color1, s[index].color2.xyz);\n"
+        "}\n";
+
+    GLint blockSize;
+    ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS.c_str());
+    GLint uniformBufferIndex = glGetUniformBlockIndex(program, "buffer");
+    glGetActiveUniformBlockiv(program, uniformBufferIndex, GL_UNIFORM_BLOCK_DATA_SIZE, &blockSize);
+
+    glBindBuffer(GL_UNIFORM_BUFFER, mUniformBuffer);
+    glBufferData(GL_UNIFORM_BUFFER, blockSize, nullptr, GL_STATIC_DRAW);
+
+    glBindBufferBase(GL_UNIFORM_BUFFER, 0, mUniformBuffer);
+    glUniformBlockBinding(program, uniformBufferIndex, 0);
+
+    const GLuint arraySize  = getArraySize();
+    const GLuint floatCount = arraySize * 2 * kFloatPerVector;
+    std::vector<GLfloat> floatData(floatCount, 0.0f);
+    const size_t strideofFloatCount = 2 * kFloatPerVector;
+
+    setArrayValues(floatData, 0, arraySize, strideofFloatCount, 0, 1, 1, 1.0f, 0.0f, 0.0f, 0.0f, 4,
+                   1, 4, 1.0f, 1.0f, 1.0f, 0.0f);
+    glBufferSubData(GL_UNIFORM_BUFFER, 0,
+                    std::min(static_cast<size_t>(blockSize), floatCount * sizeof(GLfloat)),
+                    floatData.data());
+    drawQuad(program.get(), essl3_shaders::PositionAttrib(), 0.5f);
+    checkResults(GLColor::white, GLColor::white, GLColor::white, GLColor::white);
+
+    setArrayValues(floatData, 0, arraySize, strideofFloatCount, 0, 1, 1, 1.0f, 0.0f, 0.0f, 0.0f, 4,
+                   1, 4, 0.0f, 0.0f, 1.0f, 0.0f);
+    glBufferSubData(GL_UNIFORM_BUFFER, 0,
+                    std::min(static_cast<size_t>(blockSize), floatCount * sizeof(GLfloat)),
+                    floatData.data());
+    drawQuad(program.get(), essl3_shaders::PositionAttrib(), 0.5f);
+    checkResults(GLColor::red, GLColor::red, GLColor::red, GLColor::red);
+
+    setArrayValues(floatData, arraySize / 4, arraySize / 2, strideofFloatCount, 0, 1, 1, 0.0f, 0.0f,
+                   0.0f, 0.0f, 4, 1, 4, 0.0f, 1.0f, 1.0f, 0.0f);
+    glBufferSubData(GL_UNIFORM_BUFFER, 0,
+                    std::min(static_cast<size_t>(blockSize), floatCount * sizeof(GLfloat)),
+                    floatData.data());
+    drawQuad(program.get(), essl3_shaders::PositionAttrib(), 0.5f);
+    checkResults(GLColor::red, GLColor::blue, GLColor::red, GLColor::red);
+}
+
 // Test to transfer a uniform block large array member as an actual parameter to a function.
 TEST_P(UniformBlockWithOneLargeArrayMemberTest, MemberAsActualParameter)
 {