From 1fa15b1642d82a2977cb24cbeebf34dcbf397e75 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Thu, 28 May 2020 17:36:54 +0000 Subject: [PATCH] Revert "Improve matrix construction abilities in Metal codegen." Doesn't build with `skia_compile_processors = true` This reverts commit daa573eb91cbaea22d86c442653ccb66dc815a22. Reason for revert: Original change's description: > Improve matrix construction abilities in Metal codegen. > > GLSL (and thus SkSL) is flexible about the input parameters to a matrix > constructor. You can mix vectors and scalars freely, and it will > populate them into your matrix as if it was a flat list of scalars. > > Metal does not natively support this, and requires the proper number of > floatNs to be passed in. However, the Metal code generator will now emit > constructor helper functions that will fix this up automatically. > > Additionally, this CL simplifies the Metal codegen for single-scalar > matrix construction. This should create a matrix with the passed-in > scalar running along the matrix diagonal. The Metal codegen previously > emitted a helper function to do this work on our behalf. However, > that's not necessary; Metal already contains a single-argument matrix > constructor that will do this work automatically for us. > > Change-Id: I76901bfe167502797aa4cb98d0e8986d9ebc51e5 > Bug: skia:10280 > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/292477 > Auto-Submit: John Stiles > Commit-Queue: Brian Osman > Reviewed-by: Brian Osman > Reviewed-by: Ethan Nicholas TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com Change-Id: I18610167e980eb1437842930deb9cc7509364f70 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia:10280 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/292573 Reviewed-by: John Stiles Commit-Queue: John Stiles --- src/sksl/SkSLMetalCodeGenerator.cpp | 283 ++++++++++------------------ src/sksl/SkSLMetalCodeGenerator.h | 6 +- tests/SkSLMetalTest.cpp | 142 +++++--------- 3 files changed, 147 insertions(+), 284 deletions(-) diff --git a/src/sksl/SkSLMetalCodeGenerator.cpp b/src/sksl/SkSLMetalCodeGenerator.cpp index e6665ffcdf..2b222d8ba1 100644 --- a/src/sksl/SkSLMetalCodeGenerator.cpp +++ b/src/sksl/SkSLMetalCodeGenerator.cpp @@ -379,94 +379,82 @@ void MetalCodeGenerator::writeSpecialIntrinsic(const FunctionCall & c, SpecialIn } } -// Generates a constructor for 'matrix' which reorganizes the input arguments into the proper shape. -// Keeps track of previously generated constructors so that we won't generate more than one -// constructor for any given permutation of input argument types. Returns the name of the -// generated constructor method. -String MetalCodeGenerator::getMatrixConstructHelper(const Constructor& c) { - const Type& matrix = c.fType; +// If it hasn't already been written, writes a constructor for 'matrix' which takes a single value +// of type 'arg'. +String MetalCodeGenerator::getMatrixConstructHelper(const Type& matrix, const Type& arg) { + String key = matrix.name() + arg.name(); + auto found = fHelpers.find(key); + if (found != fHelpers.end()) { + return found->second; + } + String name; int columns = matrix.columns(); int rows = matrix.rows(); - const std::vector>& args = c.fArguments; - - // Create the helper-method name and use it as our lookup key. - String name; - name.appendf("float%dx%d_from", columns, rows); - for (const std::unique_ptr& expr : args) { - name.appendf("_%s", expr->fType.displayName().c_str()); - } - - // If a helper-method has already been synthesized, we don't need to synthesize it again. - auto [iter, newlyCreated] = fHelpers.insert(name); - if (!newlyCreated) { - return name; - } - - // Unlike GLSL, Metal requires that matrices are initialized with exactly R vectors of C - // components apiece. (In Metal 2.0, you can also supply R*C scalars, but you still cannot - // supply a mixture of scalars and vectors.) - fExtraFunctions.printf("float%dx%d %s(", columns, rows, name.c_str()); - - size_t argIndex = 0; - const char* argSeparator = ""; - for (const std::unique_ptr& expr : c.fArguments) { - fExtraFunctions.printf("%s%s x%zu", argSeparator, - expr->fType.displayName().c_str(), argIndex++); - argSeparator = ", "; - } - - fExtraFunctions.printf(") {\n return float%dx%d(", columns, rows); - - argIndex = 0; - int argPosition = 0; - - const char* columnSeparator = ""; - for (int c = 0; c < columns; ++c) { - fExtraFunctions.printf("%sfloat%d(", columnSeparator, rows); - columnSeparator = "), "; - - const char* rowSeparator = ""; - for (int r = 0; r < rows; ++r) { - fExtraFunctions.printf("%s", rowSeparator); - rowSeparator = ", "; - - const Type& argType = args[argIndex]->fType; - switch (argType.kind()) { - case Type::kScalar_Kind: { - fExtraFunctions.printf("x%zu", argIndex); - break; + if (arg.isNumber()) { + // creating a matrix from a single scalar value + name = "float" + to_string(columns) + "x" + to_string(rows) + "_from_float"; + fExtraFunctions.printf("float%dx%d %s(float x) {\n", + columns, rows, name.c_str()); + fExtraFunctions.printf(" return float%dx%d(", columns, rows); + for (int i = 0; i < columns; ++i) { + if (i > 0) { + fExtraFunctions.writeText(", "); + } + fExtraFunctions.printf("float%d(", rows); + for (int j = 0; j < rows; ++j) { + if (j > 0) { + fExtraFunctions.writeText(", "); } - case Type::kVector_Kind: { - fExtraFunctions.printf("x%zu[%d]", argIndex, argPosition); - break; - } - case Type::kMatrix_Kind: { - fExtraFunctions.printf("x%zu[%d][%d]", argIndex, - argPosition / argType.rows(), - argPosition % argType.rows()); - break; - } - default: { - SkDEBUGFAIL("incorrect type of argument for matrix constructor"); - fExtraFunctions.printf(""); - break; + if (i == j) { + fExtraFunctions.writeText("x"); + } else { + fExtraFunctions.writeText("0"); } } - - ++argPosition; - if (argPosition >= argType.columns() * argType.rows()) { - ++argIndex; - argPosition = 0; - } + fExtraFunctions.writeText(")"); } - } - - if (argPosition != 0 || argIndex != args.size()) { - SkDEBUGFAIL("incorrect number of arguments for matrix constructor"); + fExtraFunctions.writeText(");\n}\n"); + } else if (arg.kind() == Type::kMatrix_Kind) { + // creating a matrix from another matrix + int argColumns = arg.columns(); + int argRows = arg.rows(); + name = "float" + to_string(columns) + "x" + to_string(rows) + "_from_float" + + to_string(argColumns) + "x" + to_string(argRows); + fExtraFunctions.printf("float%dx%d %s(float%dx%d m) {\n", + columns, rows, name.c_str(), argColumns, argRows); + fExtraFunctions.printf(" return float%dx%d(", columns, rows); + for (int i = 0; i < columns; ++i) { + if (i > 0) { + fExtraFunctions.writeText(", "); + } + fExtraFunctions.printf("float%d(", rows); + for (int j = 0; j < rows; ++j) { + if (j > 0) { + fExtraFunctions.writeText(", "); + } + if (i < argColumns && j < argRows) { + fExtraFunctions.printf("m[%d][%d]", i, j); + } else { + fExtraFunctions.writeText("0"); + } + } + fExtraFunctions.writeText(")"); + } + fExtraFunctions.writeText(");\n}\n"); + } else if (matrix.rows() == 2 && matrix.columns() == 2 && arg == *fContext.fFloat4_Type) { + // float2x2(float4) doesn't work, need to split it into float2x2(float2, float2) + name = "float2x2_from_float4"; + fExtraFunctions.printf( + "float2x2 %s(float4 v) {\n" + " return float2x2(float2(v[0], v[1]), float2(v[2], v[3]));\n" + "}\n", + name.c_str() + ); + } else { + SkASSERT(false); name = ""; } - - fExtraFunctions.printf("));\n}\n"); + fHelpers[key] = name; return name; } @@ -480,116 +468,43 @@ bool MetalCodeGenerator::canCoerce(const Type& t1, const Type& t2) { return t1.isFloat() && t2.isFloat(); } -bool MetalCodeGenerator::matrixConstructHelperIsNeeded(const Constructor& c) { - // A matrix construct helper is only necessary if we are, in fact, constructing a matrix. - if (c.fType.kind() != Type::kMatrix_Kind) { - return false; - } - - // GLSL is fairly free-form about inputs to its matrix constructors, but Metal is not; it - // expects exactly R vectors of C components apiece. (Metal 2.0 also allows a list of R*C - // scalars.) Some cases are simple to translate and so we handle those inline--e.g. a list of - // scalars can be constructed trivially. In more complex cases, we generate a helper function - // that converts our inputs into a properly-shaped matrix. - // A matrix construct helper method is always used if any input argument is a matrix. - // Helper methods are also necessary when any argument would span multiple rows. For instance: - // - // float2 x = (1, 2); - // float3x2(x, 3, 4, 5, 6) = | 1 3 5 | = no helper needed; conversion can be done inline - // | 2 4 6 | - // - // float2 x = (2, 3); - // float3x2(1, x, 4, 5, 6) = | 1 3 5 | = x spans multiple rows; a helper method will be used - // | 2 4 6 | - // - // float4 x = (1, 2, 3, 4); - // float2x2(x) = | 1 3 | = x spans multiple rows; a helper method will be used - // | 2 4 | - // - - int position = 0; - for (const std::unique_ptr& expr : c.fArguments) { - // If an input argument is a matrix, we need a helper function. - if (expr->fType.kind() == Type::kMatrix_Kind) { - return true; - } - position += expr->fType.columns(); - if (position > c.fType.rows()) { - // An input argument would span multiple rows; a helper function is required. - return true; - } - if (position == c.fType.rows()) { - // We've advanced to the end of a row. Wrap to the start of the next row. - position = 0; - } - } - - return false; -} - void MetalCodeGenerator::writeConstructor(const Constructor& c, Precedence parentPrecedence) { - // Handle special cases for single-argument constructors. - if (c.fArguments.size() == 1) { - // If the type is coercible, emit it directly. - const Expression& arg = *c.fArguments.front(); - if (this->canCoerce(c.fType, arg.fType)) { - this->writeExpression(arg, parentPrecedence); - return; - } - - // Metal supports creating matrices with a scalar on the diagonal via the single-argument - // matrix constructor. - if (c.fType.kind() == Type::kMatrix_Kind && arg.fType.isNumber()) { - const Type& matrix = c.fType; - this->write("float"); - this->write(to_string(matrix.columns())); - this->write("x"); - this->write(to_string(matrix.rows())); - this->write("("); - this->writeExpression(arg, parentPrecedence); - this->write(")"); - return; - } - } - - // Emit and invoke a matrix-constructor helper method if one is necessary. - if (this->matrixConstructHelperIsNeeded(c)) { - this->write(this->getMatrixConstructHelper(c)); - this->write("("); - const char* separator = ""; - for (const std::unique_ptr& expr : c.fArguments) { - this->write(separator); - separator = ", "; - this->writeExpression(*expr, kSequence_Precedence); - } - this->write(")"); + if (c.fArguments.size() == 1 && this->canCoerce(c.fType, c.fArguments[0]->fType)) { + this->writeExpression(*c.fArguments[0], parentPrecedence); return; } - - // Explicitly invoke the constructor, passing in the necessary arguments. - this->writeType(c.fType); - this->write("("); - const char* separator = ""; - int scalarCount = 0; - for (const std::unique_ptr& arg : c.fArguments) { - this->write(separator); - separator = ", "; - if (Type::kMatrix_Kind == c.fType.kind() && arg->fType.columns() < c.fType.rows()) { - // Merge scalars and smaller vectors together. - if (!scalarCount) { - this->writeType(c.fType.componentType()); - this->write(to_string(c.fType.rows())); - this->write("("); + if (c.fType.kind() == Type::kMatrix_Kind && c.fArguments.size() == 1) { + const Expression& arg = *c.fArguments[0]; + String name = this->getMatrixConstructHelper(c.fType, arg.fType); + this->write(name); + this->write("("); + this->writeExpression(arg, kSequence_Precedence); + this->write(")"); + } else { + this->writeType(c.fType); + this->write("("); + const char* separator = ""; + int scalarCount = 0; + for (const auto& arg : c.fArguments) { + this->write(separator); + separator = ", "; + if (Type::kMatrix_Kind == c.fType.kind() && arg->fType.columns() != c.fType.rows()) { + // merge scalars and smaller vectors together + if (!scalarCount) { + this->writeType(c.fType.componentType()); + this->write(to_string(c.fType.rows())); + this->write("("); + } + scalarCount += arg->fType.columns(); + } + this->writeExpression(*arg, kSequence_Precedence); + if (scalarCount && scalarCount == c.fType.rows()) { + this->write(")"); + scalarCount = 0; } - scalarCount += arg->fType.columns(); - } - this->writeExpression(*arg, kSequence_Precedence); - if (scalarCount && scalarCount == c.fType.rows()) { - this->write(")"); - scalarCount = 0; } + this->write(")"); } - this->write(")"); } void MetalCodeGenerator::writeFragCoord() { diff --git a/src/sksl/SkSLMetalCodeGenerator.h b/src/sksl/SkSLMetalCodeGenerator.h index 65216dc3aa..8a8c0a9759 100644 --- a/src/sksl/SkSLMetalCodeGenerator.h +++ b/src/sksl/SkSLMetalCodeGenerator.h @@ -11,7 +11,6 @@ #include #include #include -#include #include "src/sksl/SkSLCodeGenerator.h" #include "src/sksl/SkSLMemoryLayout.h" @@ -190,8 +189,7 @@ protected: void writeInverseHack(const Expression& mat); - bool matrixConstructHelperIsNeeded(const Constructor& c); - String getMatrixConstructHelper(const Constructor& c); + String getMatrixConstructHelper(const Type& matrix, const Type& arg); void writeMatrixTimesEqualHelper(const Type& left, const Type& right, const Type& result); @@ -280,7 +278,7 @@ protected: std::unordered_map fRequirements; bool fSetupFragPositionGlobal = false; bool fSetupFragPositionLocal = false; - std::unordered_set fHelpers; + std::unordered_map fHelpers; int fUniformBuffer = -1; String fRTHeightName; diff --git a/tests/SkSLMetalTest.cpp b/tests/SkSLMetalTest.cpp index a8640bad7f..13cecb9ff9 100644 --- a/tests/SkSLMetalTest.cpp +++ b/tests/SkSLMetalTest.cpp @@ -60,104 +60,54 @@ DEF_TEST(SkSLMetalHelloWorld, r) { "}\n"); } -DEF_TEST(SkSLMetal2x2MatrixCopyFromFloat2x2, r) { - test(r, R"__SkSL__( -void main() { - float2x2 m1 = float2x2(float2(1, 2), float2(3, 4)); - float2x2 m2 = m1; - float2x2 m3 = float2x2(m1); - sk_FragColor = half4(half(m1[0][0] + m2[0][0] + m3[0][0])); -})__SkSL__", - *SkSL::ShaderCapsFactory::Default(), - R"__MSL__(#include -#include -using namespace metal; -struct Inputs { -}; -struct Outputs { - float4 sk_FragColor [[color(0)]]; -}; -fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) { - Outputs _outputStruct; - thread Outputs* _out = &_outputStruct; - _out->sk_FragColor = float4((float2x2(float2(1.0, 2.0), float2(3.0, 4.0))[0][0] + float2x2(float2(1.0, 2.0), float2(3.0, 4.0))[0][0]) + float2x2(float2(1.0, 2.0), float2(3.0, 4.0))[0][0]); - return *_out; -} -)__MSL__"); -} - -DEF_TEST(SkSLMetal2x2MatrixCopyFromConstantPropagatedFloat4, r) { - test(r, R"__SkSL__( -void main() { - float2x2 m1 = float2x2(float4(1, 2, 3, 4)); - float2x2 m2 = m1; - float2x2 m3 = float2x2(m1); - sk_FragColor = half4(half(m1[0][0] + m2[0][0] + m3[0][0])); -})__SkSL__", - *SkSL::ShaderCapsFactory::Default(), -R"__MSL__(#include -#include -using namespace metal; -struct Inputs { -}; -struct Outputs { - float4 sk_FragColor [[color(0)]]; -}; -float2x2 float2x2_from_float4(float4 x0) { - return float2x2(float2(x0[0], x0[1]), float2(x0[2], x0[3])); -} -fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) { - Outputs _outputStruct; - thread Outputs* _out = &_outputStruct; - _out->sk_FragColor = float4((float2x2_from_float4(float4(1.0, 2.0, 3.0, 4.0))[0][0] + float2x2_from_float4(float4(1.0, 2.0, 3.0, 4.0))[0][0]) + float2x2_from_float4(float4(1.0, 2.0, 3.0, 4.0))[0][0]); - return *_out; -} -)__MSL__"); -} - DEF_TEST(SkSLMetalMatrices, r) { - test(r, R"__SkSL__( -void main() { - float2x2 m1 = float2x2(float4(1, 2, 3, 4)); - float2x2 m2 = float2x2(float4(0)); - float2x2 m3 = float2x2(m1); - float2x2 m4 = float2x2(1); - float2x2 m5 = float2x2(m1[0][0]); - float2x2 m6 = float2x2(1, 2, 3, 4); - float2x2 m7 = float2x2(5, float3(6, 7, 8)); - float3x2 m8 = float3x2(float2(1, 2), 3, float3(4, 5, 6)); - float3x3 m9 = float3x3(1); - float4x4 m10 = float4x4(1); - float4x4 m11 = float4x4(2); - sk_FragColor = half4(half(m1[0][0] + m2[0][0] + m3[0][0] + m4[0][0] + m5[0][0] + - m6[0][0] + m7[0][0] + m8[0][0] + m9[0][0] + m10[0][0] + m11[0][0])); -})__SkSL__", + test(r, + "void main() {" + "float2x2 m1 = float2x2(float4(1, 2, 3, 4));" + "float2x2 m2 = float2x2(float4(0));" + "float2x2 m3 = float2x2(m1);" + "float2x2 m4 = float2x2(1);" + "float2x2 m5 = float2x2(m1[0][0]);" + "float2x2 m6 = float2x2(1, 2, 3, 4);" + "float2x2 m7 = float2x2(5, 6, 7, 8);" + "float3x3 m8 = float3x3(1);" + "float3x3 m9 = float3x3(2);" + "float4x4 m10 = float4x4(1);" + "float4x4 m11 = float4x4(2);" + "sk_FragColor = half4(half(m1[0][0] + m2[0][0] + m3[0][0] + m4[0][0] + m5[0][0] + " + "m6[0][0] + m7[0][0] + m8[0][0] + m9[0][0] + m10[0][0] + m11[0][0]));" + "}", *SkSL::ShaderCapsFactory::Default(), -R"__MSL__(#include -#include -using namespace metal; -struct Inputs { -}; -struct Outputs { - float4 sk_FragColor [[color(0)]]; -}; -float2x2 float2x2_from_float4(float4 x0) { - return float2x2(float2(x0[0], x0[1]), float2(x0[2], x0[3])); -} -float2x2 float2x2_from_float_float3(float x0, float3 x1) { - return float2x2(float2(x0, x1[0]), float2(x1[1], x1[2])); -} -float3x2 float3x2_from_float2_float_float3(float2 x0, float x1, float3 x2) { - return float3x2(float2(x0[0], x0[1]), float2(x1, x2[0]), float2(x2[1], x2[2])); -} -fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) { - Outputs _outputStruct; - thread Outputs* _out = &_outputStruct; - float2x2 m5 = float2x2(float2x2_from_float4(float4(1.0, 2.0, 3.0, 4.0))[0][0]); - _out->sk_FragColor = float4((((((((((float2x2_from_float4(float4(1.0, 2.0, 3.0, 4.0))[0][0] + float2x2_from_float4(float4(0.0))[0][0]) + float2x2_from_float4(float4(1.0, 2.0, 3.0, 4.0))[0][0]) + float2x2(1.0)[0][0]) + m5[0][0]) + float2x2(float2(1.0, 2.0), float2(3.0, 4.0))[0][0]) + float2x2_from_float_float3(5.0, float3(6.0, 7.0, 8.0))[0][0]) + float3x2_from_float2_float_float3(float2(1.0, 2.0), 3.0, float3(4.0, 5.0, 6.0))[0][0]) + float3x3(1.0)[0][0]) + float4x4(1.0)[0][0]) + float4x4(2.0)[0][0]); - return *_out; -} -)__MSL__"); + "#include \n" + "#include \n" + "using namespace metal;\n" + "struct Inputs {\n" + "};\n" + "struct Outputs {\n" + " float4 sk_FragColor [[color(0)]];\n" + "};\n" + "float2x2 float2x2_from_float(float x) {\n" + " return float2x2(float2(x, 0), float2(0, x));\n" + "}\n" + "float2x2 float2x2_from_float4(float4 v) {\n" + " return float2x2(float2(v[0], v[1]), float2(v[2], v[3]));\n" + "}\n" + "float2x2 float2x2_from_float(float x) {\n" + " return float2x2(float2(x, 0), float2(0, x));\n" + "}\n" + "float3x3 float3x3_from_float(float x) {\n" + " return float3x3(float3(x, 0, 0), float3(0, x, 0), float3(0, 0, x));\n" + "}\n" + "float4x4 float4x4_from_float(float x) {\n" + " return float4x4(float4(x, 0, 0, 0), float4(0, x, 0, 0), float4(0, 0, x, 0), float4(0, 0, 0, x));\n" + "}\n" + "fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {\n" + " Outputs _outputStruct;\n" + " thread Outputs* _out = &_outputStruct;\n" + " float2x2 m5 = float2x2_from_float(float2x2_from_float4(float4(1.0, 2.0, 3.0, 4.0))[0][0]);\n" + " _out->sk_FragColor = float4((((((((((float2x2_from_float4(float4(1.0, 2.0, 3.0, 4.0))[0][0] + float2x2_from_float4(float4(0.0))[0][0]) + float2x2_from_float4(float4(1.0, 2.0, 3.0, 4.0))[0][0]) + float2x2_from_float(1.0)[0][0]) + m5[0][0]) + float2x2(float2(1.0, 2.0), float2(3.0, 4.0))[0][0]) + float2x2(float2(5.0, 6.0), float2(7.0, 8.0))[0][0]) + float3x3_from_float(1.0)[0][0]) + float3x3_from_float(2.0)[0][0]) + float4x4_from_float(1.0)[0][0]) + float4x4_from_float(2.0)[0][0]);\n" + " return *_out;\n" + "}\n"); } DEF_TEST(SkSLMetalConstantSwizzle, r) {