Revert "Switch runtime SkSL to always sample at explicit coords"

This reverts commit b1e9971.

Reason for revert: crbug_905548 GM drawing incorrect
before: https://gold.skia.org/img/images/32964364a8ac12af5a68bf95c24fb90c.png
after : https://gold.skia.org/img/images/0918b58c93c770c20cf475c37a70b528.png

Original change's description:
> Switch runtime SkSL to always sample at explicit coords
> 
> Change-Id: I4647d0f4a098acf399e1add1d87ca0752d0fdf90
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/266381
> Reviewed-by: Brian Salomon <[email protected]>
> Commit-Queue: Brian Osman <[email protected]>

[email protected],[email protected],[email protected],[email protected]

Change-Id: I0fb892148a424d5928878a49e3ccd5bfb9485b23
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/266840
Reviewed-by: Brian Salomon <[email protected]>
Commit-Queue: Brian Salomon <[email protected]>

src/effects/imagefilters/SkArithmeticImageFilter.cpp

@@ -36,8 +36,8 @@ uniform float4 k;
 in bool enforcePMColor;
 in fragmentProcessor child;
 
-void main(float x, float y, inout half4 color) {
-    half4 dst = sample(child, float2(x, y));
+void main(inout half4 color) {
+    half4 dst = sample(child);
     color = saturate(half(k.x) * color * dst + half(k.y) * color + half(k.z) * dst + half(k.w));
     @if (enforcePMColor) {
         color.rgb = min(color.rgb, color.a);

src/gpu/effects/GrSkSLFP.cpp

@@ -22,11 +22,13 @@ class GrGLSLSkSLFP : public GrGLSLFragmentProcessor {
     GrGLSLSkSLFP(SkSL::PipelineStageArgs&& args) : fArgs(std::move(args)) {}
 
     SkSL::String expandFormatArgs(const SkSL::String& raw,
-                                  EmitArgs& args,
-                                  std::vector<SkSL::Compiler::FormatArg>::const_iterator& fmtArg,
-                                  const char* coordsName) {
+                                  const EmitArgs& args,
+                                  const std::vector<SkSL::Compiler::FormatArg> formatArgs,
+                                  const char* coordsName,
+                                  const std::vector<SkString>& childNames) {
         SkSL::String result;
         int substringStartIndex = 0;
+        int formatArgIndex = 0;
         for (size_t i = 0; i < raw.length(); ++i) {
             char c = raw[i];
             if (c == '%') {
@@ -36,7 +38,7 @@ class GrGLSLSkSLFP : public GrGLSLFragmentProcessor {
                 c = raw[i];
                 switch (c) {
                     case 's': {
-                        const SkSL::Compiler::FormatArg& arg = *fmtArg++;
+                        const SkSL::Compiler::FormatArg& arg = formatArgs[formatArgIndex++];
                         switch (arg.fKind) {
                             case SkSL::Compiler::FormatArg::Kind::kInput:
                                 result += args.fInputColor;
@@ -56,12 +58,9 @@ class GrGLSLSkSLFP : public GrGLSLFragmentProcessor {
                                 result += args.fUniformHandler->getUniformCStr(
                                                                        fUniformHandles[arg.fIndex]);
                                 break;
-                            case SkSL::Compiler::FormatArg::Kind::kChildProcessor: {
-                                SkSL::String coords = this->expandFormatArgs(arg.fCoords, args,
-                                                                             fmtArg, coordsName);
-                                result += this->invokeChild(arg.fIndex, args, coords).c_str();
+                            case SkSL::Compiler::FormatArg::Kind::kChildProcessor:
+                                result += childNames[arg.fIndex].c_str();
                                 break;
-                            }
                             case SkSL::Compiler::FormatArg::Kind::kFunctionName:
                                 SkASSERT((int) fFunctionNames.size() > arg.fIndex);
                                 result += fFunctionNames[arg.fIndex].c_str();
@@ -95,28 +94,23 @@ class GrGLSLSkSLFP : public GrGLSLFragmentProcessor {
         SkString coords = args.fTransformedCoords.count()
             ? fragBuilder->ensureCoords2D(args.fTransformedCoords[0].fVaryingPoint)
             : SkString("sk_FragCoord");
-        // We need to ensure that we call invokeChild on each child FP at least once.
-        // Any child FP that isn't sampled won't trigger a call otherwise, leading to asserts later.
+        std::vector<SkString> childNames;
         for (int i = 0; i < this->numChildProcessors(); ++i) {
-            (void)this->invokeChild(i, args, SkSL::String("_coords"));
+            childNames.push_back(this->invokeChild(i, args));
         }
         for (const auto& f : fArgs.fFunctions) {
             fFunctionNames.emplace_back();
-            auto fmtArgIter = f.fFormatArgs.cbegin();
-            SkSL::String body =
-                    this->expandFormatArgs(f.fBody.c_str(), args, fmtArgIter, coords.c_str());
-            SkASSERT(fmtArgIter == f.fFormatArgs.cend());
+            SkSL::String body = this->expandFormatArgs(f.fBody.c_str(), args, f.fFormatArgs,
+                                                       coords.c_str(), childNames);
             fragBuilder->emitFunction(f.fReturnType,
                                       f.fName.c_str(),
                                       f.fParameters.size(),
                                       f.fParameters.data(),
                                       body.c_str(),
                                       &fFunctionNames.back());
         }
-        auto fmtArgIter = fArgs.fFormatArgs.cbegin();
-        fragBuilder->codeAppend(this->expandFormatArgs(fArgs.fCode.c_str(), args, fmtArgIter,
-                                                       coords.c_str()).c_str());
-        SkASSERT(fmtArgIter == fArgs.fFormatArgs.cend());
+        fragBuilder->codeAppend(this->expandFormatArgs(fArgs.fCode.c_str(), args, fArgs.fFormatArgs,
+                                                       coords.c_str(), childNames).c_str());
     }
 
     void onSetData(const GrGLSLProgramDataManager& pdman,
@@ -211,7 +205,6 @@ const char* GrSkSLFP::name() const {
 }
 
 void GrSkSLFP::addChild(std::unique_ptr<GrFragmentProcessor> child) {
-    child->setSampledWithExplicitCoords(true);
     this->registerChildProcessor(std::move(child));
 }
 
@@ -256,7 +249,7 @@ bool GrSkSLFP::onIsEqual(const GrFragmentProcessor& other) const {
 std::unique_ptr<GrFragmentProcessor> GrSkSLFP::clone() const {
     std::unique_ptr<GrSkSLFP> result(new GrSkSLFP(*this));
     for (int i = 0; i < this->numChildProcessors(); ++i) {
-        result->addChild(this->childProcessor(i).clone());
+        result->registerChildProcessor(this->childProcessor(i).clone());
     }
     return std::unique_ptr<GrFragmentProcessor>(result.release());
 }

src/sksl/SkSLCompiler.h

@@ -92,8 +92,8 @@ class SK_API Compiler : public ErrorReporter {
                 , fIndex(index) {}
 
         Kind fKind;
+
         int fIndex;
-        String fCoords;
     };
 
 #if !defined(SKSL_STANDALONE) && SK_SUPPORT_GPU

src/sksl/SkSLPipelineStageCodeGenerator.cpp

@@ -56,10 +56,9 @@ void PipelineStageCodeGenerator::writeBinaryExpression(const BinaryExpression& b
 void PipelineStageCodeGenerator::writeFunctionCall(const FunctionCall& c) {
     if (c.fFunction.fBuiltin && c.fFunction.fName == "sample" &&
         c.fArguments[0]->fType.kind() != Type::Kind::kSampler_Kind) {
-        SkASSERT(c.fArguments.size() == 2);
+        SkASSERT(c.fArguments.size() == 1);
         SkASSERT("fragmentProcessor"  == c.fArguments[0]->fType.name() ||
                  "fragmentProcessor?" == c.fArguments[0]->fType.name());
-        SkASSERT("float2" == c.fArguments[1]->fType.name());
         SkASSERT(Expression::kVariableReference_Kind == c.fArguments[0]->fKind);
         int index = 0;
         bool found = false;
@@ -81,15 +80,8 @@ void PipelineStageCodeGenerator::writeFunctionCall(const FunctionCall& c) {
         }
         SkASSERT(found);
         this->write("%s");
-        size_t childCallIndex = fArgs->fFormatArgs.size();
         fArgs->fFormatArgs.push_back(
                 Compiler::FormatArg(Compiler::FormatArg::Kind::kChildProcessor, index));
-        OutputStream* oldOut = fOut;
-        StringStream buffer;
-        fOut = &buffer;
-        this->writeExpression(*c.fArguments[1], kSequence_Precedence);
-        fOut = oldOut;
-        fArgs->fFormatArgs[childCallIndex].fCoords = buffer.str();
         return;
     }
     if (c.fFunction.fBuiltin) {

src/sksl/sksl_pipeline.inc

@@ -1,3 +1,3 @@
 STRINGIFY(
-    half4 sample(fragmentProcessor fp, float2 coords);
+    half4 sample(fragmentProcessor fp);
 )

tools/viewer/SkSLSlide.cpp

@@ -8,7 +8,6 @@
 #include "tools/viewer/SkSLSlide.h"
 
 #include "include/effects/SkGradientShader.h"
-#include "include/effects/SkPerlinNoiseShader.h"
 #include "src/core/SkEnumerate.h"
 #include "tools/Resources.h"
 #include "tools/viewer/ImGuiLayer.h"
@@ -37,10 +36,10 @@ SkSLSlide::SkSLSlide() {
 
     fSkSL =
 
-        "in fragmentProcessor fp;\n"
+        "uniform half4 gColor;\n"
         "\n"
         "void main(float x, float y, inout half4 color) {\n"
-        "    color = sample(fp, float2(x, y));\n"
+        "    color = half4(half(x)*(1.0/255), half(y)*(1.0/255), gColor.b, 1);\n"
         "}\n";
 }
 
@@ -63,9 +62,6 @@ void SkSLSlide::load(SkScalar winWidth, SkScalar winHeight) {
     shader = GetResourceAsImage("images/mandrill_256.png")->makeShader();
     fShaders.push_back(std::make_pair("Mandrill", shader));
 
-    shader = SkPerlinNoiseShader::MakeImprovedNoise(0.025f, 0.025f, 3, 0.0f);
-    fShaders.push_back(std::make_pair("Perlin Noise", shader));
-
     this->rebuild();
 }