Fix scalarizing vec and mat constructor args

src/compiler/translator/Compiler.cpp

@@ -516,20 +516,24 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
                                     &symbolTable, shaderVersion);
     }
 
+    int simplifyScalarized = (compileOptions & SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS)
+                                 ? IntermNodePatternMatcher::kScalarizedVecOrMatConstructor
+                                 : 0;
+
     // Split multi declarations and remove calls to array length().
     // Note that SimplifyLoopConditions needs to be run before any other AST transformations
     // that may need to generate new statements from loop conditions or loop expressions.
-    SimplifyLoopConditions(
-        root,
-        IntermNodePatternMatcher::kMultiDeclaration | IntermNodePatternMatcher::kArrayLengthMethod,
-        &getSymbolTable(), getShaderVersion());
+    SimplifyLoopConditions(root,
+                           IntermNodePatternMatcher::kMultiDeclaration |
+                               IntermNodePatternMatcher::kArrayLengthMethod | simplifyScalarized,
+                           &getSymbolTable(), getShaderVersion());
 
     // Note that separate declarations need to be run before other AST transformations that
     // generate new statements from expressions.
     SeparateDeclarations(root);
 
-    SplitSequenceOperator(root, IntermNodePatternMatcher::kArrayLengthMethod, &getSymbolTable(),
-                          getShaderVersion());
+    SplitSequenceOperator(root, IntermNodePatternMatcher::kArrayLengthMethod | simplifyScalarized,
+                          &getSymbolTable(), getShaderVersion());
 
     RemoveArrayLengthMethod(root);
 

src/compiler/translator/IntermNodePatternMatcher.cpp

@@ -15,6 +15,33 @@
 namespace sh
 {
 
+namespace
+{
+
+bool ContainsMatrixNode(const TIntermSequence &sequence)
+{
+    for (size_t ii = 0; ii < sequence.size(); ++ii)
+    {
+        TIntermTyped *node = sequence[ii]->getAsTyped();
+        if (node && node->isMatrix())
+            return true;
+    }
+    return false;
+}
+
+bool ContainsVectorNode(const TIntermSequence &sequence)
+{
+    for (size_t ii = 0; ii < sequence.size(); ++ii)
+    {
+        TIntermTyped *node = sequence[ii]->getAsTyped();
+        if (node && node->isVector())
+            return true;
+    }
+    return false;
+}
+
+}  // anonymous namespace
+
 IntermNodePatternMatcher::IntermNodePatternMatcher(const unsigned int mask) : mMask(mask)
 {
 }
@@ -105,6 +132,20 @@ bool IntermNodePatternMatcher::match(TIntermAggregate *node, TIntermNode *parent
             }
         }
     }
+    if ((mMask & kScalarizedVecOrMatConstructor) != 0)
+    {
+        if (node->getOp() == EOpConstruct)
+        {
+            if (node->getType().isVector() && ContainsMatrixNode(*(node->getSequence())))
+            {
+                return true;
+            }
+            else if (node->getType().isMatrix() && ContainsVectorNode(*(node->getSequence())))
+            {
+                return true;
+            }
+        }
+    }
     return false;
 }
 

src/compiler/translator/IntermNodePatternMatcher.h

@@ -48,7 +48,11 @@ class IntermNodePatternMatcher
         kNamelessStructDeclaration = 0x0001 << 5,
 
         // Matches array length() method.
-        kArrayLengthMethod = 0x0001 << 6
+        kArrayLengthMethod = 0x0001 << 6,
+
+        // Matches a vector or matrix constructor whose arguments are scalarized by the
+        // SH_SCALARIZE_VEC_OR_MAT_CONSTRUCTOR_ARGUMENTS workaround.
+        kScalarizedVecOrMatConstructor = 0x0001 << 7
     };
     IntermNodePatternMatcher(const unsigned int mask);
 

src/compiler/translator/ScalarizeVecAndMatConstructorArgs.cpp

@@ -15,6 +15,7 @@
 
 #include "angle_gl.h"
 #include "common/angleutils.h"
+#include "compiler/translator/IntermNodePatternMatcher.h"
 #include "compiler/translator/IntermNode_util.h"
 #include "compiler/translator/IntermTraverse.h"
 
@@ -24,28 +25,6 @@ namespace sh
 namespace
 {
 
-bool ContainsMatrixNode(const TIntermSequence &sequence)
-{
-    for (size_t ii = 0; ii < sequence.size(); ++ii)
-    {
-        TIntermTyped *node = sequence[ii]->getAsTyped();
-        if (node && node->isMatrix())
-            return true;
-    }
-    return false;
-}
-
-bool ContainsVectorNode(const TIntermSequence &sequence)
-{
-    for (size_t ii = 0; ii < sequence.size(); ++ii)
-    {
-        TIntermTyped *node = sequence[ii]->getAsTyped();
-        if (node && node->isVector())
-            return true;
-    }
-    return false;
-}
-
 TIntermBinary *ConstructVectorIndexBinaryNode(TIntermSymbol *symbolNode, int index)
 {
     return new TIntermBinary(EOpIndexDirect, symbolNode, CreateIndexNode(index));
@@ -66,7 +45,8 @@ class ScalarizeArgsTraverser : public TIntermTraverser
                            TSymbolTable *symbolTable)
         : TIntermTraverser(true, false, false, symbolTable),
           mShaderType(shaderType),
-          mFragmentPrecisionHigh(fragmentPrecisionHigh)
+          mFragmentPrecisionHigh(fragmentPrecisionHigh),
+          mNodesToScalarize(IntermNodePatternMatcher::kScalarizedVecOrMatConstructor)
     {
     }
 
@@ -92,16 +72,24 @@ class ScalarizeArgsTraverser : public TIntermTraverser
 
     sh::GLenum mShaderType;
     bool mFragmentPrecisionHigh;
+
+    IntermNodePatternMatcher mNodesToScalarize;
 };
 
 bool ScalarizeArgsTraverser::visitAggregate(Visit visit, TIntermAggregate *node)
 {
-    if (visit == PreVisit && node->getOp() == EOpConstruct)
+    ASSERT(visit == PreVisit);
+    if (mNodesToScalarize.match(node, getParentNode()))
     {
-        if (node->getType().isVector() && ContainsMatrixNode(*(node->getSequence())))
+        if (node->getType().isVector())
+        {
             scalarizeArgs(node, false, true);
-        else if (node->getType().isMatrix() && ContainsVectorNode(*(node->getSequence())))
+        }
+        else
+        {
+            ASSERT(node->getType().isMatrix());
             scalarizeArgs(node, true, false);
+        }
     }
     return true;
 }
@@ -134,55 +122,55 @@ void ScalarizeArgsTraverser::scalarizeArgs(TIntermAggregate *aggregate,
     ASSERT(!aggregate->isArray());
     int size                  = static_cast<int>(aggregate->getType().getObjectSize());
     TIntermSequence *sequence = aggregate->getSequence();
-    TIntermSequence original(*sequence);
+    TIntermSequence originalArgs(*sequence);
     sequence->clear();
-    for (size_t ii = 0; ii < original.size(); ++ii)
+    for (TIntermNode *originalArgNode : originalArgs)
     {
         ASSERT(size > 0);
-        TIntermTyped *node = original[ii]->getAsTyped();
-        ASSERT(node);
-        createTempVariable(node);
-        if (node->isScalar())
+        TIntermTyped *originalArg = originalArgNode->getAsTyped();
+        ASSERT(originalArg);
+        createTempVariable(originalArg);
+        if (originalArg->isScalar())
         {
-            sequence->push_back(createTempSymbol(node->getType()));
+            sequence->push_back(createTempSymbol(originalArg->getType()));
             size--;
         }
-        else if (node->isVector())
+        else if (originalArg->isVector())
         {
             if (scalarizeVector)
             {
-                int repeat = std::min(size, node->getNominalSize());
+                int repeat = std::min(size, originalArg->getNominalSize());
                 size -= repeat;
                 for (int index = 0; index < repeat; ++index)
                 {
-                    TIntermSymbol *symbolNode = createTempSymbol(node->getType());
+                    TIntermSymbol *symbolNode = createTempSymbol(originalArg->getType());
                     TIntermBinary *newNode    = ConstructVectorIndexBinaryNode(symbolNode, index);
                     sequence->push_back(newNode);
                 }
             }
             else
             {
-                TIntermSymbol *symbolNode = createTempSymbol(node->getType());
+                TIntermSymbol *symbolNode = createTempSymbol(originalArg->getType());
                 sequence->push_back(symbolNode);
-                size -= node->getNominalSize();
+                size -= originalArg->getNominalSize();
             }
         }
         else
         {
-            ASSERT(node->isMatrix());
+            ASSERT(originalArg->isMatrix());
             if (scalarizeMatrix)
             {
                 int colIndex = 0, rowIndex = 0;
-                int repeat = std::min(size, node->getCols() * node->getRows());
+                int repeat = std::min(size, originalArg->getCols() * originalArg->getRows());
                 size -= repeat;
                 while (repeat > 0)
                 {
-                    TIntermSymbol *symbolNode = createTempSymbol(node->getType());
+                    TIntermSymbol *symbolNode = createTempSymbol(originalArg->getType());
                     TIntermBinary *newNode =
                         ConstructMatrixIndexBinaryNode(symbolNode, colIndex, rowIndex);
                     sequence->push_back(newNode);
                     rowIndex++;
-                    if (rowIndex >= node->getRows())
+                    if (rowIndex >= originalArg->getRows())
                     {
                         rowIndex = 0;
                         colIndex++;
@@ -192,9 +180,9 @@ void ScalarizeArgsTraverser::scalarizeArgs(TIntermAggregate *aggregate,
             }
             else
             {
-                TIntermSymbol *symbolNode = createTempSymbol(node->getType());
+                TIntermSymbol *symbolNode = createTempSymbol(originalArg->getType());
                 sequence->push_back(symbolNode);
-                size -= node->getCols() * node->getRows();
+                size -= originalArg->getCols() * originalArg->getRows();
             }
         }
     }

src/tests/angle_unittests.gypi

@@ -82,6 +82,7 @@
             '<(angle_path)/src/tests/compiler_tests/RemoveUnreferencedVariables_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/RewriteDoWhile_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/SamplerMultisample_test.cpp',
+            '<(angle_path)/src/tests/compiler_tests/ScalarizeVecAndMatConstructorArgs_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/ShaderExtension_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/ShaderImage_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/ShaderValidation_test.cpp',

src/tests/compiler_tests/ScalarizeVecAndMatConstructorArgs_test.cpp

+//
+// Copyright (c) 2017 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// ScalarizeVecAndMatConstructorArgs_test.cpp:
+//   Tests for scalarizing vector and matrix constructor args.
+//
+
+#include "GLSLANG/ShaderLang.h"
+#include "angle_gl.h"
+#include "gtest/gtest.h"
+#include "tests/test_utils/compiler_test.h"
+
+using namespace sh;
+
+namespace
+{
+
+class ScalarizeVecAndMatConstructorArgsTest : public MatchOutputCodeTest
+{
+  public:
+    ScalarizeVecAndMatConstructorArgsTest()
+        : MatchOutputCodeTest(GL_FRAGMENT_SHADER,
+                              SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS,
+                              SH_ESSL_OUTPUT)
+    {
+    }
+};
+
+// Verifies scalarizing matrix inside a vector constructor.
+TEST_F(ScalarizeVecAndMatConstructorArgsTest, MatrixInVectorConstructor)
+{
+    const std::string shaderString =
+        R"(
+        precision mediump float;
+
+        uniform mat2 umat2;
+
+        void main()
+        {
+            gl_FragColor = vec4(umat2);
+        })";
+    compile(shaderString);
+
+    std::vector<const char *> expectedStrings = {
+        "main()", " = _uumat2", "gl_FragColor = vec4(", "[0][0],", "[0][1],", "[1][0],", "[1][1])"};
+
+    EXPECT_TRUE(foundInCodeInOrder(expectedStrings));
+}
+
+// Verifies scalarizing a vector insized a matrix constructor.
+TEST_F(ScalarizeVecAndMatConstructorArgsTest, VectorInMatrixConstructor)
+{
+    const std::string shaderString =
+        R"(
+        precision mediump float;
+
+        uniform vec2 uvec2;
+
+        void main()
+        {
+            mat2 m = mat2(uvec2, uvec2);
+            gl_FragColor = vec4(m * uvec2, m * uvec2);
+        })";
+    compile(shaderString);
+
+    std::vector<const char *> expectedStrings = {
+        "main()", " = _uuvec2", "mat2(", "[0],", "[1],", "[0],", "[1])", "gl_FragColor = vec4("};
+
+    EXPECT_TRUE(foundInCodeInOrder(expectedStrings));
+}
+
+// Verifies that scalarizing vector and matrix constructor args inside a sequence operator preserves
+// correct order of operations.
+TEST_F(ScalarizeVecAndMatConstructorArgsTest, SequenceOperator)
+{
+    const std::string shaderString =
+        R"(
+        precision mediump float;
+
+        uniform vec2 u;
+
+        void main()
+        {
+            vec2 v = u;
+            mat2 m = (v[0] += 1.0, mat2(v, v[1], -v[0]));
+            gl_FragColor = vec4(m[0], m[1]);
+        })";
+    compile(shaderString);
+
+    std::vector<const char *> expectedStrings = {"_uv[0] += 1.0", "-_uv[0]"};
+
+    EXPECT_TRUE(foundInCodeInOrder(expectedStrings));
+}
+
+// Verifies that scalarizing vector and matrix constructor args inside multiple declarations
+// preserves the correct order of operations.
+TEST_F(ScalarizeVecAndMatConstructorArgsTest, MultiDeclaration)
+{
+    const std::string shaderString =
+        R"(
+        precision mediump float;
+
+        uniform vec2 u;
+
+        void main()
+        {
+            vec2 v = vec2(u[0]),
+                 w = mat2(v, v) * u;
+            gl_FragColor = vec4(v, w);
+        })";
+    compile(shaderString);
+
+    std::vector<const char *> expectedStrings = {"vec2(_uu[0])", "mat2("};
+
+    EXPECT_TRUE(foundInCodeInOrder(expectedStrings));
+}
+
+}  // anonymous namespace

src/tests/test_utils/compiler_test.cpp

@@ -155,6 +155,29 @@ size_t MatchOutputCodeTest::findInCode(ShShaderOutput output, const char *string
     return code->second.find(stringToFind);
 }
 
+bool MatchOutputCodeTest::foundInCodeInOrder(ShShaderOutput output,
+                                             std::vector<const char *> stringsToFind)
+{
+    const auto code = mOutputCode.find(output);
+    EXPECT_NE(mOutputCode.end(), code);
+    if (code == mOutputCode.end())
+    {
+        return false;
+    }
+
+    size_t currentPos = 0;
+    for (const char *stringToFind : stringsToFind)
+    {
+        auto position = code->second.find(stringToFind, currentPos);
+        if (position == std::string::npos)
+        {
+            return false;
+        }
+        currentPos = position + strlen(stringToFind);
+    }
+    return true;
+}
+
 bool MatchOutputCodeTest::foundInCode(ShShaderOutput output,
                                       const char *stringToFind,
                                       const int expectedOccurrences) const
@@ -204,6 +227,18 @@ bool MatchOutputCodeTest::foundInCode(const char *stringToFind, const int expect
     return true;
 }
 
+bool MatchOutputCodeTest::foundInCodeInOrder(std::vector<const char *> stringsToFind)
+{
+    for (auto &code : mOutputCode)
+    {
+        if (!foundInCodeInOrder(code.first, stringsToFind))
+        {
+            return false;
+        }
+    }
+    return true;
+}
+
 bool MatchOutputCodeTest::notFoundInCode(const char *stringToFind) const
 {
     for (auto &code : mOutputCode)

src/tests/test_utils/compiler_test.h

@@ -10,6 +10,7 @@
 #define TESTS_TEST_UTILS_COMPILER_TEST_H_
 
 #include <map>
+#include <vector>
 
 #include "gtest/gtest.h"
 
@@ -68,6 +69,9 @@ class MatchOutputCodeTest : public testing::Test
     // source. If no matches are found, then string::npos is returned.
     size_t findInCode(ShShaderOutput output, const char *stringToFind) const;
 
+    // Test that the strings are found in the specified output in the specified order.
+    bool foundInCodeInOrder(ShShaderOutput output, std::vector<const char *> stringsToFind);
+
     // Test that the string occurs for exactly expectedOccurrences times
     bool foundInCode(ShShaderOutput output,
                      const char *stringToFind,
@@ -79,6 +83,9 @@ class MatchOutputCodeTest : public testing::Test
     // Test that the string occurs for exactly expectedOccurrences times in all outputs
     bool foundInCode(const char *stringToFind, const int expectedOccurrences) const;
 
+    // Test that the strings are found in all outputs in the specified order.
+    bool foundInCodeInOrder(std::vector<const char *> stringsToFind);
+
     // Test that the string is found in none of the outputs
     bool notFoundInCode(const char *stringToFind) const;