Remove workarounds to support legacy coord transforms

lhkbob · Skia Commit-Bot · commit e7e25ac904c6 · 2020-06-29T20:23:54.000Z
Bug: skia:10416 Change-Id: I2f1b87521174d18afc59f12832441010cb94ea3f Reviewed-on: https://skia-review.googlesource.com/c/skia/+/299294 Reviewed-by: Brian Salomon <bsalomon@google.com> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/gpu/GrPrimitiveProcessor.cpp b/src/gpu/GrPrimitiveProcessor.cpp
@@ -15,23 +15,17 @@
  * the transform code is applied.
  */
 enum SampleFlag {
-    kExplicitlySampled_Flag          = 0b0000001,  // GrFP::isSampledWithExplicitCoords()
+    kExplicitlySampled_Flag          = 0b00001,  // GrFP::isSampledWithExplicitCoords()
 
-    kLegacyCoordTransform_Flag       = 0b0000010, // !GrFP::coordTransform(i)::isNoOp()
+    kLegacyCoordTransform_Flag       = 0b00010, // !GrFP::coordTransform(i)::isNoOp()
 
-    kNone_SampleMatrix_Flag          = 0b0000100, // GrFP::sampleMatrix()::isNoOp()
-    kConstUniform_SampleMatrix_Flag  = 0b0001000, // GrFP::sampleMatrix()::isConstUniform()
-    kVariable_SampleMatrix_Flag      = 0b0001100, // GrFP::sampleMatrix()::isVariable()
-
-    // Legacy coord transforms specialize on identity, S+T, no-perspective, and general matrix types
-    // FIXME these (and kLegacyCoordTransform) can be removed once all FPs no longer use them
-    kLCT_ScaleTranslate_Matrix_Flag  = 0b0010000, // GrFP::coordTransform(i)::isScaleTranslate()
-    kLCT_NoPersp_Matrix_Flag         = 0b0100000, // !GrFP::coordTransform(i)::hasPerspective()
-    kLCT_General_Matrix_Flag         = 0b0110000, // any other matrix type
+    kNone_SampleMatrix_Flag          = 0b00100, // GrFP::sampleMatrix()::isNoOp()
+    kConstUniform_SampleMatrix_Flag  = 0b01000, // GrFP::sampleMatrix()::isConstUniform()
+    kVariable_SampleMatrix_Flag      = 0b01100, // GrFP::sampleMatrix()::isVariable()
 
     // Currently, sample(matrix) only specializes on no-perspective or general.
     // FIXME add new flags as more matrix types are supported.
-    kPersp_Matrix_Flag               = 0b1000000, // GrFP::sampleMatrix()::fHasPerspective
+    kPersp_Matrix_Flag               = 0b10000, // GrFP::sampleMatrix()::fHasPerspective
 };
 
 GrPrimitiveProcessor::GrPrimitiveProcessor(ClassID classID) : GrProcessor(classID) {}
@@ -42,31 +36,16 @@ const GrPrimitiveProcessor::TextureSampler& GrPrimitiveProcessor::textureSampler
 }
 
 uint32_t GrPrimitiveProcessor::computeCoordTransformsKey(const GrFragmentProcessor& fp) const {
-    // This is highly coupled with the code in GrGLSLGeometryProcessor::emitTransforms().
-    // At this point, all effects either don't use legacy coord transforms, or only use 1.
-    SkASSERT(fp.numCoordTransforms() <= 1);
+    // This is highly coupled with the code in GrGLSLGeometryProcessor::collectTransforms().
+    // At this point, all effects do not use really coord transforms; they may implicitly report
+    // a noop coord transform but this does not impact the key.
+    SkASSERT(fp.numCoordTransforms() == 0 ||
+             (fp.numCoordTransforms() == 1 && fp.coordTransform(0).isNoOp()));
 
     uint32_t key = 0;
     if (fp.isSampledWithExplicitCoords()) {
         key |= kExplicitlySampled_Flag;
     }
-    if (fp.numCoordTransforms() > 0) {
-        const GrCoordTransform& coordTransform = fp.coordTransform(0);
-        if (!coordTransform.isNoOp()) {
-            // A true identity matrix shouldn't result in a coord transform; proxy normalization
-            // and flipping will eventually present as a scale+translate matrix.
-            SkASSERT(!coordTransform.matrix().isIdentity() || coordTransform.normalize() ||
-                     coordTransform.reverseY());
-            key |= kLegacyCoordTransform_Flag;
-            if (coordTransform.matrix().isScaleTranslate()) {
-                key |= kLCT_ScaleTranslate_Matrix_Flag;
-            } else if (!coordTransform.matrix().hasPerspective()) {
-                key |= kLCT_NoPersp_Matrix_Flag;
-            } else {
-                key |= kLCT_General_Matrix_Flag;
-            }
-        }
-    }
 
     switch(fp.sampleMatrix().fKind) {
         case SkSL::SampleMatrix::Kind::kNone:
diff --git a/src/gpu/GrPrimitiveProcessor.h b/src/gpu/GrPrimitiveProcessor.h
@@ -14,8 +14,6 @@
 #include "src/gpu/GrShaderVar.h"
 #include "src/gpu/GrSwizzle.h"
 
-class GrCoordTransform;
-
 /*
  * The GrPrimitiveProcessor represents some kind of geometric primitive.  This includes the shape
  * of the primitive and the inherent color of the primitive.  The GrPrimitiveProcessor is
diff --git a/src/gpu/gl/builders/GrGLProgramBuilder.cpp b/src/gpu/gl/builders/GrGLProgramBuilder.cpp
@@ -15,7 +15,6 @@
 #include "src/core/SkWriteBuffer.h"
 #include "src/gpu/GrAutoLocaleSetter.h"
 #include "src/gpu/GrContextPriv.h"
-#include "src/gpu/GrCoordTransform.h"
 #include "src/gpu/GrPersistentCacheUtils.h"
 #include "src/gpu/GrProgramDesc.h"
 #include "src/gpu/GrShaderCaps.h"
diff --git a/src/gpu/glsl/GrGLSLFragmentProcessor.cpp b/src/gpu/glsl/GrGLSLFragmentProcessor.cpp
@@ -5,7 +5,6 @@
  * found in the LICENSE file.
  */
 
-#include "src/gpu/GrCoordTransform.h"
 #include "src/gpu/GrFragmentProcessor.h"
 #include "src/gpu/GrProcessor.h"
 #include "src/gpu/glsl/GrGLSLFragmentProcessor.h"
diff --git a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
@@ -153,45 +153,13 @@ SkString GrGLSLFPFragmentBuilder::writeProcessorFunction(GrGLSLFragmentProcessor
     // transforms), the value that would have been passed to _coords is lifted to the vertex shader
     // and stored in a unique varying. In that case it uses that variable and does not have a
     // second actual argument for _coords.
-    // FIXME: Once GrCoordTransforms are gone, and we can more easily associated this varying with
-    // the sample call site, then invokeChild() can pass the varying in, instead of requiring this
-    // dynamic signature.
-    int paramCount;
+    // FIXME: An alternative would be to have all FP functions have a float2 argument, and the
+    // parent FP invokes it with the varying reference when it's been lifted to the vertex shader.
+    int paramCount = 2;
     GrShaderVar params[] = { GrShaderVar(args.fInputColor, kHalf4_GrSLType),
                              GrShaderVar(args.fSampleCoord, kFloat2_GrSLType) };
 
-    if (args.fFp.isSampledWithExplicitCoords()) {
-        // All invokeChild() that point to 'fp' will evaluate these expressions and pass the float2
-        // in, so we need the 2nd argument.
-        paramCount = 2;
-
-        // FIXME: This is only needed for the short term until FPs no longer put transformation
-        // data in a GrCoordTransform (and we can then mark the parameter as read-only)
-        if (args.fTransformedCoords.count() > 0) {
-            SkASSERT(args.fTransformedCoords.count() == 1);
-
-            const GrShaderVar& transform = args.fTransformedCoords[0].fTransform;
-            switch (transform.getType()) {
-                case kFloat4_GrSLType:
-                    // This is a scale+translate, so there's no perspective division needed
-                    this->codeAppendf("%s = %s * %s.xz + %s.yw;\n", args.fSampleCoord,
-                                                                    args.fSampleCoord,
-                                                                    transform.c_str(),
-                                                                    transform.c_str());
-                    break;
-                case kFloat3x3_GrSLType:
-                    this->codeAppend("{\n");
-                    this->codeAppendf("float3 _coords3 = (%s * %s.xy1);\n",
-                                      transform.c_str(), args.fSampleCoord);
-                    this->codeAppendf("%s = _coords3.xy / _coords3.z;\n", args.fSampleCoord);
-                    this->codeAppend("}\n");
-                    break;
-                default:
-                    SkASSERT(transform.getType() == kVoid_GrSLType);
-                    break;
-            }
-        }
-    } else {
+    if (!args.fFp.isSampledWithExplicitCoords()) {
         // Sampled with a const/uniform matrix and/or a legacy coord transform. The actual
         // transformation code is emitted in the vertex shader, so this only has to access it.
         // Add a float2 _coords variable that maps to the associated varying and replaces the
@@ -210,15 +178,15 @@ SkString GrGLSLFPFragmentBuilder::writeProcessorFunction(GrGLSLFragmentProcessor
                     // and since we won't actually have a function parameter for local coords, add
                     // it as a local variable.
                     this->codeAppendf("float2 %s = %s.xy / %s.z;\n", args.fSampleCoord,
-                                    varying.getName().c_str(), varying.getName().c_str());
+                                      varying.getName().c_str(), varying.getName().c_str());
                     break;
                 default:
                     SkDEBUGFAILF("Unexpected varying type for coord: %s %d\n",
                                  varying.getName().c_str(), (int) varying.getType());
                     break;
             }
         }
-    }
+    } // else the function keeps its two arguments
 
     this->codeAppendf("half4 %s;\n", args.fOutputColor);
     fp->emitCode(args);
diff --git a/src/gpu/glsl/GrGLSLGeometryProcessor.cpp b/src/gpu/glsl/GrGLSLGeometryProcessor.cpp
diff --git a/src/gpu/glsl/GrGLSLGeometryProcessor.h b/src/gpu/glsl/GrGLSLGeometryProcessor.h
