From 1bdafbf016b26a2c34f58292b92d713291f7ec95 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Thu, 28 May 2020 12:17:20 -0400 Subject: [PATCH] Reland "Improve matrix construction abilities in Metal codegen." This is a reland of daa573eb91cbaea22d86c442653ccb66dc815a22 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 Bug: skia:10280 Change-Id: If5591392bb96e1cfb643d4e3c19a0ee4affec58d Reviewed-on: https://skia-review.googlesource.com/c/skia/+/292689 Commit-Queue: John Stiles Commit-Queue: Brian Osman Auto-Submit: John Stiles Reviewed-by: Brian Osman --- src/sksl/SkSLMetalCodeGenerator.cpp | 279 ++++++++++++++++++---------- src/sksl/SkSLMetalCodeGenerator.h | 6 +- tests/SkSLMetalTest.cpp | 142 +++++++++----- 3 files changed, 282 insertions(+), 145 deletions(-) diff --git a/src/sksl/SkSLMetalCodeGenerator.cpp b/src/sksl/SkSLMetalCodeGenerator.cpp index a8a407b279..fc40d3a2bd 100644 --- a/src/sksl/SkSLMetalCodeGenerator.cpp +++ b/src/sksl/SkSLMetalCodeGenerator.cpp @@ -379,82 +379,94 @@ void MetalCodeGenerator::writeSpecialIntrinsic(const FunctionCall & c, SpecialIn } } -// 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; +// 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; int columns = matrix.columns(); int rows = matrix.rows(); - 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(", "); + 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 (i == j) { - fExtraFunctions.writeText("x"); - } else { - fExtraFunctions.writeText("0"); + 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; } } - fExtraFunctions.writeText(")"); + + ++argPosition; + if (argPosition >= argType.columns() * argType.rows()) { + ++argIndex; + argPosition = 0; + } } - 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); + } + + if (argPosition != 0 || argIndex != args.size()) { + SkDEBUGFAIL("incorrect number of arguments for matrix constructor"); name = ""; } - fHelpers[key] = name; + + fExtraFunctions.printf("));\n}\n"); return name; } @@ -468,43 +480,116 @@ bool MetalCodeGenerator::canCoerce(const Type& t1, const Type& t2) { return t1.isFloat() && t2.isFloat(); } -void MetalCodeGenerator::writeConstructor(const Constructor& c, Precedence parentPrecedence) { - if (c.fArguments.size() == 1 && this->canCoerce(c.fType, c.fArguments[0]->fType)) { - this->writeExpression(*c.fArguments[0], parentPrecedence); - return; +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; } - 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); + + // 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 = ""; - int scalarCount = 0; - for (const auto& arg : c.fArguments) { + for (const std::unique_ptr& expr : 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; - } + this->writeExpression(*expr, kSequence_Precedence); } this->write(")"); + 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("("); + } + scalarCount += arg->fType.columns(); + } + this->writeExpression(*arg, kSequence_Precedence); + if (scalarCount && scalarCount == c.fType.rows()) { + this->write(")"); + scalarCount = 0; + } + } + this->write(")"); } void MetalCodeGenerator::writeFragCoord() { diff --git a/src/sksl/SkSLMetalCodeGenerator.h b/src/sksl/SkSLMetalCodeGenerator.h index 8a8c0a9759..65216dc3aa 100644 --- a/src/sksl/SkSLMetalCodeGenerator.h +++ b/src/sksl/SkSLMetalCodeGenerator.h @@ -11,6 +11,7 @@ #include #include #include +#include #include "src/sksl/SkSLCodeGenerator.h" #include "src/sksl/SkSLMemoryLayout.h" @@ -189,7 +190,8 @@ protected: void writeInverseHack(const Expression& mat); - String getMatrixConstructHelper(const Type& matrix, const Type& arg); + bool matrixConstructHelperIsNeeded(const Constructor& c); + String getMatrixConstructHelper(const Constructor& c); void writeMatrixTimesEqualHelper(const Type& left, const Type& right, const Type& result); @@ -278,7 +280,7 @@ protected: std::unordered_map fRequirements; bool fSetupFragPositionGlobal = false; bool fSetupFragPositionLocal = false; - std::unordered_map fHelpers; + std::unordered_set fHelpers; int fUniformBuffer = -1; String fRTHeightName; diff --git a/tests/SkSLMetalTest.cpp b/tests/SkSLMetalTest.cpp index 13cecb9ff9..a8640bad7f 100644 --- a/tests/SkSLMetalTest.cpp +++ b/tests/SkSLMetalTest.cpp @@ -60,54 +60,104 @@ 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, - "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]));" - "}", + 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__", *SkSL::ShaderCapsFactory::Default(), - "#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"); +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__"); } DEF_TEST(SkSLMetalConstantSwizzle, r) {