Fix GLSL struct with single matrix.

The parameters of the matrix constructor form an aggregate node that
was passed directly to the constructor of the structure, instead of
creating a one-element aggregate containing the matrix.

https://github.com/mc-imperial/shader-compiler-bugs/issues/74

Bug swiftshader:56

Change-Id: Iff9a2d8dc60d79a0dde28f2aad76407028486ec8
Reviewed-on: https://swiftshader-review.googlesource.com/19308
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>

diff --git a/src/OpenGL/compiler/Intermediate.cpp b/src/OpenGL/compiler/Intermediate.cpp
index fbcb16b..4016b11 100644 (file)
--- a/src/OpenGL/compiler/Intermediate.cpp
+++ b/src/OpenGL/compiler/Intermediate.cpp
@@ -529,8 +529,8 @@ TIntermTyped* TIntermediate::addUnaryMath(TOperator op, TIntermTyped* child, con
 //
 // This is the safe way to change the operator on an aggregate, as it
 // does lots of error checking and fixing.  Especially for establishing
-// a function call's operation on it's set of parameters.  Sequences
-// of instructions are also aggregates, but they just direnctly set
+// a function call's operation on its set of parameters.  Sequences
+// of instructions are also aggregates, but they just directly set
 // their operator to EOpSequence.
 //
 // Returns an aggregate node, which could be the one passed in if

diff --git a/src/OpenGL/compiler/ParseHelper.cpp b/src/OpenGL/compiler/ParseHelper.cpp
index cf6aa95..b795c06 100644 (file)
--- a/src/OpenGL/compiler/ParseHelper.cpp
+++ b/src/OpenGL/compiler/ParseHelper.cpp
@@ -3605,7 +3605,9 @@ TIntermTyped *TParseContext::addFunctionCallOrMethod(TFunction *fnCall, TIntermN
                                        //
                                        // Treat it like a built-in unary operator.
                                        //
-                                       callNode = createUnaryMath(op, paramNode->getAsTyped(), loc, &fnCandidate->getReturnType());
+                                       TIntermNode *operand = paramNode->getAsAggregate()->getSequence()[0];
+                                       callNode = createUnaryMath(op, operand->getAsTyped(), loc, &fnCandidate->getReturnType());
+
                                        if(callNode == nullptr)
                                        {
                                                std::stringstream extraInfoStream;

diff --git a/src/OpenGL/compiler/glslang.y b/src/OpenGL/compiler/glslang.y
index f970d3b..ab69684 100644 (file)
--- a/src/OpenGL/compiler/glslang.y
+++ b/src/OpenGL/compiler/glslang.y
@@ -351,7 +351,7 @@ function_call_header_with_parameters
         TParameter param = { 0, new TType($2->getType()) };
         $1->addParameter(param);
         $$.function = $1;
-        $$.nodePair.node1 = $2;
+        $$.nodePair.node1 = context->intermediate.makeAggregate($2, @2);
     }
     | function_call_header_with_parameters COMMA assignment_expression {
         TParameter param = { 0, new TType($3->getType()) };

diff --git a/src/OpenGL/compiler/glslang_tab.cpp b/src/OpenGL/compiler/glslang_tab.cpp
index 45d73fe..6d8f589 100644 (file)
--- a/src/OpenGL/compiler/glslang_tab.cpp
+++ b/src/OpenGL/compiler/glslang_tab.cpp
@@ -2574,7 +2574,7 @@ yyreduce:
         TParameter param = { 0, new TType((yyvsp[0].interm.intermTypedNode)->getType()) };
         (yyvsp[-1].interm.function)->addParameter(param);
         (yyval.interm).function = (yyvsp[-1].interm.function);
-        (yyval.interm).nodePair.node1 = (yyvsp[0].interm.intermTypedNode);
+        (yyval.interm).nodePair.node1 = context->intermediate.makeAggregate((yyvsp[0].interm.intermTypedNode), (yylsp[0]));
     }
 
     break;

diff --git a/tests/unittests/unittests.cpp b/tests/unittests/unittests.cpp
index 89a24a0..6bef144 100644 (file)
--- a/tests/unittests/unittests.cpp
+++ b/tests/unittests/unittests.cpp
@@ -12,6 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// OpenGL ES unit tests that provide coverage for functionality not tested by
+// the dEQP test suite. Also used as a smoke test.
+
 #include "gtest/gtest.h"
 #include "gmock/gmock.h"
 
@@ -226,12 +229,18 @@ protected:
                glShaderSource(ph.vertexShader, 1, vsSource, nullptr);
                glCompileShader(ph.vertexShader);
                EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+               GLint vsCompileStatus = 0;
+               glGetShaderiv(ph.vertexShader, GL_COMPILE_STATUS, &vsCompileStatus);
+               EXPECT_EQ(vsCompileStatus, GL_TRUE);
 
                ph.fragmentShader = glCreateShader(GL_FRAGMENT_SHADER);
                const char* fsSource[1] = { fs.c_str() };
                glShaderSource(ph.fragmentShader, 1, fsSource, nullptr);
                glCompileShader(ph.fragmentShader);
                EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+               GLint fsCompileStatus = 0;
+               glGetShaderiv(ph.fragmentShader, GL_COMPILE_STATUS, &fsCompileStatus);
+               EXPECT_EQ(fsCompileStatus, GL_TRUE);
 
                glAttachShader(ph.program, ph.vertexShader);
                glAttachShader(ph.program, ph.fragmentShader);
@@ -503,6 +512,36 @@ TEST_F(SwiftShaderTest, DynamicIndexing)
        Uninitialize();
 }
 
+// Tests construction of a structure containing a single matrix
+TEST_F(SwiftShaderTest, MatrixInStruct)
+{
+       Initialize(2, false);
+
+       const std::string fs =
+               "#version 100\n"
+               "precision mediump float;\n"
+               "struct S\n"
+               "{\n"
+               "       mat2 rotation;\n"
+               "};\n"
+               "void main(void)\n"
+               "{\n"
+               "       float angle = 1.0;\n"
+               "       S(mat2(1.0, angle, 1.0, 1.0));\n"
+               "}\n";
+
+       GLuint fragmentShader = glCreateShader(GL_FRAGMENT_SHADER);
+       const char *fsSource[1] = { fs.c_str() };
+       glShaderSource(fragmentShader, 1, fsSource, nullptr);
+       glCompileShader(fragmentShader);
+       EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+       GLint compileStatus = 0;
+       glGetShaderiv(fragmentShader, GL_COMPILE_STATUS, &compileStatus);
+       EXPECT_NE(compileStatus, 0);
+
+       Uninitialize();
+}
+
 // Test sampling from a sampler in a struct as a function argument
 TEST_F(SwiftShaderTest, SamplerArrayInStructArrayAsFunctionArg)
 {