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>
diff --git a/src/core/SkNormalMapSource.cpp b/src/core/SkNormalMapSource.cpp
index d2ac97d..dbbbf2e 100644
--- a/src/core/SkNormalMapSource.cpp
+++ b/src/core/SkNormalMapSource.cpp
@@ -34,11 +34,7 @@
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 6c6d607..41bafb8 100644
--- a/src/effects/SkArithmeticImageFilter.cpp
+++ b/src/effects/SkArithmeticImageFilter.cpp
@@ -224,11 +224,7 @@
}
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 8e795d5..44b14ad 100644
--- a/src/gpu/GrFragmentProcessor.cpp
+++ b/src/gpu/GrFragmentProcessor.cpp
@@ -305,8 +305,7 @@
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 @@
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 9c60808..0784031 100644
--- a/src/gpu/GrFragmentProcessor.h
+++ b/src/gpu/GrFragmentProcessor.h
@@ -88,11 +88,9 @@
/**
* 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 88a09e6..490cb39 100644
--- a/src/gpu/effects/GrXfermodeFragmentProcessor.cpp
+++ b/src/gpu/effects/GrXfermodeFragmentProcessor.cpp
@@ -199,9 +199,6 @@
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 @@
}
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 93f268a..5b2eb7e 100644
--- a/src/gpu/effects/GrYUVEffect.cpp
+++ b/src/gpu/effects/GrYUVEffect.cpp
@@ -250,11 +250,7 @@
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 88610a0..97c5ac6 100644
--- a/src/shaders/SkLightingShader.cpp
+++ b/src/shaders/SkLightingShader.cpp
@@ -122,14 +122,7 @@
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 @@
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 aca548c..891d1cc 100644
--- a/tests/GLProgramsTest.cpp
+++ b/tests/GLProgramsTest.cpp
@@ -111,8 +111,7 @@
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 3bfd62e..e2625a7 100644
--- a/tests/ProcessorTest.cpp
+++ b/tests/ProcessorTest.cpp
@@ -84,15 +84,7 @@
}
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 @@
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 @@
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 @@
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()) ||