Require clone() be implemented by GrFragmentProcessor subclasses

Change-Id: I66ba0978e5748806d563ff4f26000e4e0095ed24
Reviewed-on: https://skia-review.googlesource.com/29042
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

9 files changed, 17 insertions, 56 deletions

diff --git a/src/core/SkNormalMapSource.cpp b/src/core/SkNormalMapSource.cpp
index d2ac97d258..dbbbf2ea48 100644
--- a/src/core/SkNormalMapSource.cpp
+++ b/src/core/SkNormalMapSource.cpp
@@ -34,11 +34,7 @@ public:
     const SkMatrix& invCTM() const { return fInvCTM; }
 
     sk_sp<GrFragmentProcessor> clone() const override {
-        auto child = this->childProcessor(0).clone();
-        if (!child) {
-            return nullptr;
-        }
-        return Make(std::move(child), fInvCTM);
+        return Make(this->childProcessor(0).clone(), fInvCTM);
     }
 
 private:
diff --git a/src/effects/SkArithmeticImageFilter.cpp b/src/effects/SkArithmeticImageFilter.cpp
index 6c6d6070c5..41bafb889c 100644
--- a/src/effects/SkArithmeticImageFilter.cpp
+++ b/src/effects/SkArithmeticImageFilter.cpp
@@ -224,11 +224,7 @@ public:
     }
 
     sk_sp<GrFragmentProcessor> clone() const override {
-        auto child = this->childProcessor(0).clone();
-        if (!child) {
-            return nullptr;
-        }
-        return Make(fK1, fK2, fK3, fK4, fEnforcePMColor, std::move(child));
+        return Make(fK1, fK2, fK3, fK4, fEnforcePMColor, this->childProcessor(0).clone());
     }
 
     float k1() const { return fK1; }
diff --git a/src/gpu/GrFragmentProcessor.cpp b/src/gpu/GrFragmentProcessor.cpp
index 8e795d5796..44b14add2d 100644
--- a/src/gpu/GrFragmentProcessor.cpp
+++ b/src/gpu/GrFragmentProcessor.cpp
@@ -305,8 +305,7 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::MakeInputPremulAndMulByOutput(
         const char* name() const override { return "Premultiply"; }
 
         sk_sp<GrFragmentProcessor> clone() const override {
-            auto child = this->childProcessor(0).clone();
-            return child ? Make(std::move(child)) : nullptr;
+            return Make(this->childProcessor(0).clone());
         }
 
     private:
@@ -376,8 +375,7 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::OverrideInput(sk_sp<GrFragmentPr
         const char* name() const override { return "Replace Color"; }
 
         sk_sp<GrFragmentProcessor> clone() const override {
-            auto child = this->childProcessor(0).clone();
-            return child ? Make(std::move(child), fColor) : nullptr;
+            return Make(this->childProcessor(0).clone(), fColor);
         }
 
     private:
diff --git a/src/gpu/GrFragmentProcessor.h b/src/gpu/GrFragmentProcessor.h
index 9c608086dd..0784031d30 100644
--- a/src/gpu/GrFragmentProcessor.h
+++ b/src/gpu/GrFragmentProcessor.h
@@ -88,11 +88,9 @@ public:
 
     /**
      * Makes a copy of this fragment processor that draws equivalently to the original.
-     * If the processor has child processors they are cloned as well. Currently this
-     * has a default implementation that fails. This is temporary until it can be implemented
-     * for all fragemnt processor leaf classes.
+     * If the processor has child processors they are cloned as well.
      */
-    virtual sk_sp<GrFragmentProcessor> clone() const { return nullptr; }
+    virtual sk_sp<GrFragmentProcessor> clone() const = 0;
 
     GrGLSLFragmentProcessor* createGLSLInstance() const;
 
diff --git a/src/gpu/effects/GrXfermodeFragmentProcessor.cpp b/src/gpu/effects/GrXfermodeFragmentProcessor.cpp
index 88a09e6d7a..490cb39990 100644
--- a/src/gpu/effects/GrXfermodeFragmentProcessor.cpp
+++ b/src/gpu/effects/GrXfermodeFragmentProcessor.cpp
@@ -199,9 +199,6 @@ sk_sp<GrFragmentProcessor> ComposeTwoFragmentProcessor::TestCreate(GrProcessorTe
 sk_sp<GrFragmentProcessor> ComposeTwoFragmentProcessor::clone() const {
     auto src = this->childProcessor(0).clone();
     auto dst = this->childProcessor(1).clone();
-    if (!src || !dst) {
-        return nullptr;
-    }
     return sk_sp<GrFragmentProcessor>(
             new ComposeTwoFragmentProcessor(std::move(src), std::move(dst), fMode));
 }
@@ -498,12 +495,8 @@ GrGLSLFragmentProcessor* ComposeOneFragmentProcessor::onCreateGLSLInstance() con
 }
 
 sk_sp<GrFragmentProcessor> ComposeOneFragmentProcessor::clone() const {
-    auto child = this->childProcessor(0).clone();
-    if (!child) {
-        return nullptr;
-    }
     return sk_sp<GrFragmentProcessor>(
-            new ComposeOneFragmentProcessor(std::move(child), fMode, fChild));
+            new ComposeOneFragmentProcessor(this->childProcessor(0).clone(), fMode, fChild));
 }
 
 //////////////////////////////////////////////////////////////////////////////
diff --git a/src/gpu/effects/GrYUVEffect.cpp b/src/gpu/effects/GrYUVEffect.cpp
index 93f268a13c..5b2eb7e3ae 100644
--- a/src/gpu/effects/GrYUVEffect.cpp
+++ b/src/gpu/effects/GrYUVEffect.cpp
@@ -250,11 +250,7 @@ public:
     const char* name() const override { return "RGBToYUV"; }
 
     sk_sp<GrFragmentProcessor> clone() const override {
-        auto child = this->childProcessor(0).clone();
-        if (!child) {
-            return nullptr;
-        }
-        return Make(std::move(child), fColorSpace, fOutputChannels);
+        return Make(this->childProcessor(0).clone(), fColorSpace, fOutputChannels);
     }
 
     SkYUVColorSpace getColorSpace() const { return fColorSpace; }
diff --git a/src/shaders/SkLightingShader.cpp b/src/shaders/SkLightingShader.cpp
index 88610a073c..97c5ac6ce4 100644
--- a/src/shaders/SkLightingShader.cpp
+++ b/src/shaders/SkLightingShader.cpp
@@ -122,14 +122,7 @@ public:
     const char* name() const override { return "LightingFP"; }
 
     sk_sp<GrFragmentProcessor> clone() const override {
-        // Currently we make the child clone here and pass it to the "copy" constructor. This is
-        // because clone() may fail for processor classes that haven't yet implemented it. Once all
-        // processors have an implementation the child can be cloned in a true copy constructor.
-        auto normalFPClone = this->childProcessor(0).clone();
-        if (!normalFPClone) {
-            return nullptr;
-        }
-        return sk_sp<GrFragmentProcessor>(new LightingFP(*this, std::move(normalFPClone)));
+        return sk_sp<GrFragmentProcessor>(new LightingFP(*this));
     }
 
     const SkTArray<SkLights::Light>& directionalLights() const { return fDirectionalLights; }
@@ -263,11 +256,11 @@ private:
         this->initClassID<LightingFP>();
     }
 
-    LightingFP(const LightingFP& that, sk_sp<GrFragmentProcessor> normalFPClone)
+    LightingFP(const LightingFP& that)
             : INHERITED(kPreservesOpaqueInput_OptimizationFlag)
             , fDirectionalLights(that.fDirectionalLights)
             , fAmbientColor(that.fAmbientColor) {
-        this->registerChildProcessor(std::move(normalFPClone));
+        this->registerChildProcessor(that.childProcessor(0).clone());
         this->initClassID<LightingFP>();
     }
 
diff --git a/tests/GLProgramsTest.cpp b/tests/GLProgramsTest.cpp
index aca548cdf8..891d1cca46 100644
--- a/tests/GLProgramsTest.cpp
+++ b/tests/GLProgramsTest.cpp
@@ -111,8 +111,7 @@ public:
     GrGLSLFragmentProcessor* onCreateGLSLInstance() const override { return new GLFP; }
 
     sk_sp<GrFragmentProcessor> clone() const override {
-        auto child = this->childProcessor(0).clone();
-        return child ? Make(std::move(child)) : nullptr;
+        return Make(this->childProcessor(0).clone());
     }
 
 private:
diff --git a/tests/ProcessorTest.cpp b/tests/ProcessorTest.cpp
index 3bfd62e3a4..e2625a7e31 100644
--- a/tests/ProcessorTest.cpp
+++ b/tests/ProcessorTest.cpp
@@ -84,15 +84,7 @@ public:
     }
 
     sk_sp<GrFragmentProcessor> clone() const override {
-        sk_sp<GrFragmentProcessor> child;
-        if (this->numChildProcessors()) {
-            SkASSERT(1 == this->numChildProcessors());
-            child = this->childProcessor(0).clone();
-            if (!child) {
-                return nullptr;
-            }
-        }
-        return sk_sp<GrFragmentProcessor> (new TestFP(*this, std::move(child)));
+        return sk_sp<GrFragmentProcessor>(new TestFP(*this));
     }
 
 private:
@@ -120,7 +112,7 @@ private:
         this->registerChildProcessor(std::move(child));
     }
 
-    explicit TestFP(const TestFP& that, sk_sp<GrFragmentProcessor> child)
+    explicit TestFP(const TestFP& that)
             : INHERITED(that.optimizationFlags()), fSamplers(4), fBuffers(4), fImages(4) {
         this->initClassID<TestFP>();
         for (int i = 0; i < that.fSamplers.count(); ++i) {
@@ -135,8 +127,8 @@ private:
             fImages.emplace_back(that.fImages[i]);
             this->addImageStorageAccess(&fImages.back());
         }
-        if (child) {
-            this->registerChildProcessor(std::move(child));
+        for (int i = 0; i < that.numChildProcessors(); ++i) {
+            this->registerChildProcessor(that.childProcessor(i).clone());
         }
     }
 
@@ -554,8 +546,8 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ProcessorCloneTest, reporter, ctxInfo) {
         for (int j = 0; j < kTimesToInvokeFactory; ++j) {
             auto fp = GrFragmentProcessorTestFactory::MakeIdx(i, &testData);
             auto clone = fp->clone();
-            // Currently clone() is allowed to fail.
             if (!clone) {
+                ERRORF(reporter, "Clone of processor %s failed.", fp->name());
                 continue;
             }
             if (!fp->instantiate(context->resourceProvider()) ||