commit 55dc5c8325b684754e96af26670edf667fa8ee9d
author John Stiles <johnstiles@google.com> Thu Aug 19 18:15:49 2021 -0400
committer SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> Fri Aug 20 14:04:52 2021 +0000
tree 5623d776a1224fa2d53ac9bc779bce37aa9e0819
parent 2fadbede910d8d306a41dab942272984c19a811c

Fix assertion in constant-folder when using float4(mat2) casts.

The no-op-arithmetic simplifier was written before we allowed casting a
mat2x2 to a float4, and did not expect a matrix inside a vector ctor.
The expression `float4(myMat2) * float4(anything)` would assert when we
tried to determine if `myMat2` was a constant zero or constant one.

The code has been rewritten to use getConstantSubexpression and now
allows matrices inside.

Change-Id: Id625141256bf89d816c57d2d21f16b0ec252c158
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440858
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>

resources/sksl/folding/MatrixFoldingES2.sksl

diff --git a/resources/sksl/folding/MatrixFoldingES2.sksl b/resources/sksl/folding/MatrixFoldingES2.sksl
index 544a0bb..1061ed6 100644
--- a/resources/sksl/folding/MatrixFoldingES2.sksl
+++ b/resources/sksl/folding/MatrixFoldingES2.sksl
@@ -1,3 +1,4 @@
+uniform float2x2 testMatrix2x2;
 uniform half4 colorRed, colorGreen;
 uniform half unknownInput;
 
@@ -32,6 +33,11 @@
     ok = ok &&  (float2x2(float4(1.0, 0.0, 0.0, 1.0)) == float2x2(1.0));
     ok = ok &&  (float2x2(1.0, 0.0, float2(0.0, 1.0)) == float2x2(1.0));
     ok = ok &&  (float2x2(float2(1.0, 0.0), 0.0, 1.0) == float2x2(1.0));
+
+    ok = ok &&  (float4(testMatrix2x2) * float4(1)) == float4(1, 2, 3, 4);
+    ok = ok &&  (float4(testMatrix2x2) * float4(1)) == float4(testMatrix2x2);
+    ok = ok &&  (float4(testMatrix2x2) * float4(0)) == float4(0);
+
     return ok;
 }
 

src/sksl/SkSLConstantFolder.cpp

diff --git a/src/sksl/SkSLConstantFolder.cpp b/src/sksl/SkSLConstantFolder.cpp
index abef8d5..f9824f5 100644
--- a/src/sksl/SkSLConstantFolder.cpp
+++ b/src/sksl/SkSLConstantFolder.cpp
@@ -160,30 +160,25 @@
 }
 
 static bool contains_constant_zero(const Expression& expr) {
-    if (expr.isAnyConstructor()) {
-        for (const auto& arg : expr.asAnyConstructor().argumentSpan()) {
-            if (contains_constant_zero(*arg)) {
-                return true;
-            }
+    int numSlots = expr.type().slotCount();
+    for (int index = 0; index < numSlots; ++index) {
+        const Expression* subexpr = expr.getConstantSubexpression(index);
+        if (subexpr && is_constant_scalar_value(*subexpr, 0.0f)) {
+            return true;
         }
-        return false;
     }
-    return is_constant_scalar_value(expr, 0.0);
+    return false;
 }
 
 static bool is_constant_value(const Expression& expr, float value) {
-    // This check only supports scalars and vectors (and in particular, not matrices).
-    SkASSERT(expr.type().isScalar() || expr.type().isVector());
-
-    if (expr.isAnyConstructor()) {
-        for (const auto& arg : expr.asAnyConstructor().argumentSpan()) {
-            if (!is_constant_value(*arg, value)) {
-                return false;
-            }
+    int numSlots = expr.type().slotCount();
+    for (int index = 0; index < numSlots; ++index) {
+        const Expression* subexpr = expr.getConstantSubexpression(index);
+        if (!subexpr || !is_constant_scalar_value(*subexpr, value)) {
+            return false;
         }
-        return true;
     }
-    return is_constant_scalar_value(expr, value);
+    return true;
 }
 
 bool ConstantFolder::ErrorOnDivideByZero(const Context& context, int offset, Operator op,

tests/sksl/folding/MatrixFoldingES2.glsl

diff --git a/tests/sksl/folding/MatrixFoldingES2.glsl b/tests/sksl/folding/MatrixFoldingES2.glsl
index 7a6a73f..d428cfe 100644
--- a/tests/sksl/folding/MatrixFoldingES2.glsl
+++ b/tests/sksl/folding/MatrixFoldingES2.glsl
@@ -1,5 +1,6 @@
 
 out vec4 sk_FragColor;
+uniform mat2 testMatrix2x2;
 uniform vec4 colorRed;
 uniform vec4 colorGreen;
 uniform float unknownInput;
@@ -7,5 +8,6 @@
     bool _0_ok = true;
     _0_ok = _0_ok && mat3(unknownInput) == mat3(mat2(1.0));
     _0_ok = _0_ok && mat3(9.0, 0.0, 0.0, 0.0, 9.0, 0.0, 0.0, 0.0, unknownInput) == mat3(mat2(9.0));
+    _0_ok = _0_ok && vec4(testMatrix2x2) == vec4(1.0, 2.0, 3.0, 4.0);
     return _0_ok ? colorGreen : colorRed;
 }