Removing ref counting from GrXPFactory.

All GrXPFactory instances are static constexpr.

Change-Id: If1086b08534166201e53b3fd9379104e361eb5e6
Reviewed-on: https://skia-review.googlesource.com/6701
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

6 files changed, 89 insertions, 87 deletions

diff --git a/include/gpu/GrPaint.h b/include/gpu/GrPaint.h
index 6cdc2de09c..6bbd66cbce 100644
--- a/include/gpu/GrPaint.h
+++ b/include/gpu/GrPaint.h
@@ -84,13 +84,9 @@ public:
         setAllowSRGBInputs(gammaCorrect);
     }
 
-    void setXPFactory(sk_sp<GrXPFactory> xpFactory) {
-        fXPFactory = std::move(xpFactory);
-    }
+    void setXPFactory(const GrXPFactory* xpFactory) { fXPFactory = xpFactory; }
 
-    void setPorterDuffXPFactory(SkBlendMode mode) {
-        fXPFactory = GrPorterDuffXPFactory::Make(mode);
-    }
+    void setPorterDuffXPFactory(SkBlendMode mode) { fXPFactory = GrPorterDuffXPFactory::Get(mode); }
 
     void setCoverageSetOpXPFactory(SkRegion::Op, bool invertCoverage = false);
 
@@ -127,9 +123,7 @@ public:
     int numTotalFragmentProcessors() const { return this->numColorFragmentProcessors() +
                                               this->numCoverageFragmentProcessors(); }
 
-    GrXPFactory* getXPFactory() const {
-        return fXPFactory.get();
-    }
+    const GrXPFactory* getXPFactory() const { return fXPFactory; }
 
     GrFragmentProcessor* getColorFragmentProcessor(int i) const {
         return fColorFragmentProcessors[i].get();
@@ -173,7 +167,7 @@ public:
 private:
     bool internalIsConstantBlendedColor(GrColor paintColor, GrColor* constantColor) const;
 
-    mutable sk_sp<GrXPFactory>                fXPFactory;
+    const GrXPFactory*                        fXPFactory;
     SkSTArray<4, sk_sp<GrFragmentProcessor>>  fColorFragmentProcessors;
     SkSTArray<2, sk_sp<GrFragmentProcessor>>  fCoverageFragmentProcessors;
 
diff --git a/include/gpu/GrProcessorUnitTest.h b/include/gpu/GrProcessorUnitTest.h
index d398aae9e4..0826e3d653 100644
--- a/include/gpu/GrProcessorUnitTest.h
+++ b/include/gpu/GrProcessorUnitTest.h
@@ -18,6 +18,7 @@ class GrContext;
 class GrRenderTargetContext;
 struct GrProcessorTestData;
 class GrTexture;
+class GrXPFactory;
 
 namespace GrProcessorUnitTest {
 
@@ -65,7 +66,7 @@ struct GrProcessorTestData {
 class GrProcessor;
 class GrTexture;
 
-template <class Processor> class GrProcessorTestFactory : SkNoncopyable {
+template <class Processor> class GrProcessorTestFactory : private SkNoncopyable {
 public:
     typedef sk_sp<Processor> (*MakeProc)(GrProcessorTestData*);
 
@@ -88,20 +89,44 @@ public:
     /** Use factory function at Index idx to create a processor. */
     static sk_sp<Processor> MakeIdx(int idx, GrProcessorTestData* data) {
         GrProcessorTestFactory<Processor>* factory = (*GetFactories())[idx];
-        return factory->fMakeProc(data);
+        sk_sp<Processor> processor = factory->fMakeProc(data);
+        SkASSERT(processor);
+        return processor;
     }
 
-    /*
+private:
+    /**
      * A test function which verifies the count of factories.
      */
     static void VerifyFactoryCount();
 
-private:
     MakeProc fMakeProc;
 
     static SkTArray<GrProcessorTestFactory<Processor>*, true>* GetFactories();
 };
 
+class GrXPFactoryTestFactory : private SkNoncopyable {
+public:
+    using GetFn = const GrXPFactory*(GrProcessorTestData*);
+
+    GrXPFactoryTestFactory(GetFn* getProc) : fGetProc(getProc) { GetFactories()->push_back(this); }
+
+    static const GrXPFactory* Get(GrProcessorTestData* data) {
+        VerifyFactoryCount();
+        SkASSERT(GetFactories()->count());
+        uint32_t idx = data->fRandom->nextRangeU(0, GetFactories()->count() - 1);
+        const GrXPFactory* xpf = (*GetFactories())[idx]->fGetProc(data);
+        SkASSERT(xpf);
+        return xpf;
+    }
+
+private:
+    static void VerifyFactoryCount();
+
+    GetFn* fGetProc;
+    static SkTArray<GrXPFactoryTestFactory*, true>* GetFactories();
+};
+
 /** GrProcessor subclasses should insert this macro in their declaration to be included in the
  *  program generation unit test.
  */
@@ -114,21 +139,21 @@ private:
     static sk_sp<GrFragmentProcessor> TestCreate(GrProcessorTestData*)
 
 #define GR_DECLARE_XP_FACTORY_TEST                                                                 \
-    static GrProcessorTestFactory<GrXPFactory> gTestFactory SK_UNUSED;                             \
-    static sk_sp<GrXPFactory> TestCreate(GrProcessorTestData*)
+    static GrXPFactoryTestFactory gTestFactory SK_UNUSED;                                          \
+    static const GrXPFactory* TestGet(GrProcessorTestData*)
 
 /** GrProcessor subclasses should insert this macro in their implementation file. They must then
  *  also implement this static function:
  *      GrProcessor* TestCreate(GrProcessorTestData*);
  */
 #define GR_DEFINE_FRAGMENT_PROCESSOR_TEST(Effect)                                                  \
-    GrProcessorTestFactory<GrFragmentProcessor> Effect :: gTestFactory(Effect :: TestCreate)
-
-#define GR_DEFINE_XP_FACTORY_TEST(Factory)                                                         \
-    GrProcessorTestFactory<GrXPFactory> Factory :: gTestFactory(Factory :: TestCreate)
+    GrProcessorTestFactory<GrFragmentProcessor> Effect::gTestFactory(Effect::TestCreate)
 
 #define GR_DEFINE_GEOMETRY_PROCESSOR_TEST(Effect)                                                  \
-    GrProcessorTestFactory<GrGeometryProcessor> Effect :: gTestFactory(Effect :: TestCreate)
+    GrProcessorTestFactory<GrGeometryProcessor> Effect::gTestFactory(Effect::TestCreate)
+
+#define GR_DEFINE_XP_FACTORY_TEST(Factory)                                                         \
+    GrXPFactoryTestFactory Factory::gTestFactory(Factory::TestGet)
 
 #else // !SK_ALLOW_STATIC_GLOBAL_INITIALIZERS
 
@@ -140,15 +165,15 @@ private:
 
 // The unit test relies on static initializers. Just declare the TestCreate function so that
 // its definitions will compile.
-#define GR_DECLARE_XP_FACTORY_TEST                                                                 \
-    static sk_sp<GrXPFactory> TestCreate(GrProcessorTestData*)
-#define GR_DEFINE_XP_FACTORY_TEST(X)
-
-// The unit test relies on static initializers. Just declare the TestCreate function so that
-// its definitions will compile.
 #define GR_DECLARE_GEOMETRY_PROCESSOR_TEST                                                         \
     static sk_sp<GrGeometryProcessor> TestCreate(GrProcessorTestData*)
 #define GR_DEFINE_GEOMETRY_PROCESSOR_TEST(X)
 
+// The unit test relies on static initializers. Just declare the TestGet function so that
+// its definitions will compile.
+#define GR_DECLARE_XP_FACTORY_TEST                                                                 \
+    const GrXPFactory* TestGet(GrProcessorTestData*)
+#define GR_DEFINE_XP_FACTORY_TEST(X)
+
 #endif // !SK_ALLOW_STATIC_GLOBAL_INITIALIZERS
 #endif
diff --git a/include/gpu/GrXferProcessor.h b/include/gpu/GrXferProcessor.h
index e97c0b92c7..17cd2c7bf3 100644
--- a/include/gpu/GrXferProcessor.h
+++ b/include/gpu/GrXferProcessor.h
@@ -289,8 +289,22 @@ GR_MAKE_BITFIELD_OPS(GrXferProcessor::OptFlags);
  * Before the XP is created, the XPF is able to answer queries about what functionality the XPs it
  * creates will have. For example, can it create an XP that supports RGB coverage or will the XP
  * blend with the destination color.
+ *
+ * GrXPFactories are intended to be static immutable objects. We pass them around as raw pointers
+ * and expect the pointers to always be valid and for the factories to be reusable and thread safe.
+ * Equality is tested for using pointer comparison. GrXPFactory destructors must be no-ops.
  */
-class GrXPFactory : public SkRefCnt {
+
+// In order to construct GrXPFactory subclass instances as constexpr the subclass, and therefore
+// GrXPFactory, must be a literal type. One requirement is having a trivial destructor. This is ok
+// since these objects have no need for destructors. However, GCC and clang throw a warning when a
+// class has virtual functions and a non-virtual destructor. We suppress that warning here and
+// for the subclasses.
+#if defined(__GNUC__) || defined(__clang)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wnon-virtual-dtor"
+#endif
+class GrXPFactory {
 public:
     typedef GrXferProcessor::DstTexture DstTexture;
     GrXferProcessor* createXferProcessor(const GrPipelineAnalysis&,
@@ -317,29 +331,8 @@ public:
 
     bool willNeedDstTexture(const GrCaps& caps, const GrPipelineAnalysis&) const;
 
-    bool isEqual(const GrXPFactory& that) const {
-        if (this->classID() != that.classID()) {
-            return false;
-        }
-        return this->onIsEqual(that);
-    }
-
-    /**
-      * Helper for down-casting to a GrXPFactory subclass
-      */
-    template <typename T> const T& cast() const { return *static_cast<const T*>(this); }
-
-    uint32_t classID() const { SkASSERT(kIllegalXPFClassID != fClassID); return fClassID; }
-
 protected:
-    GrXPFactory() : fClassID(kIllegalXPFClassID) {}
-
-    template <typename XPF_SUBCLASS> void initClassID() {
-         static uint32_t kClassID = GenClassID();
-         fClassID = kClassID;
-    }
-
-    uint32_t fClassID;
+    constexpr GrXPFactory() {}
 
 private:
     virtual GrXferProcessor* onCreateXferProcessor(const GrCaps& caps,
@@ -347,34 +340,17 @@ private:
                                                    bool hasMixedSamples,
                                                    const DstTexture*) const = 0;
 
-    virtual bool onIsEqual(const GrXPFactory&) const = 0;
-
     bool willReadDstColor(const GrCaps&, const GrPipelineAnalysis&) const;
+
     /**
      *  Returns true if the XP generated by this factory will explicitly read dst in the fragment
      *  shader.
      */
     virtual bool onWillReadDstColor(const GrCaps&, const GrPipelineAnalysis&) const = 0;
-
-    static uint32_t GenClassID() {
-        // fCurrXPFactoryID has been initialized to kIllegalXPFactoryID. The
-        // atomic inc returns the old value not the incremented value. So we add
-        // 1 to the returned value.
-        uint32_t id = static_cast<uint32_t>(sk_atomic_inc(&gCurrXPFClassID)) + 1;
-        if (!id) {
-            SkFAIL("This should never wrap as it should only be called once for each GrXPFactory "
-                   "subclass.");
-        }
-        return id;
-    }
-
-    enum {
-        kIllegalXPFClassID = 0,
-    };
-    static int32_t gCurrXPFClassID;
-
-    typedef GrProgramElement INHERITED;
 };
+#if defined(__GNUC__) || defined(__clang)
+#pragma GCC diagnostic pop
+#endif
 
 #endif
 
diff --git a/include/gpu/effects/GrCoverageSetOpXP.h b/include/gpu/effects/GrCoverageSetOpXP.h
index 2aae7be41d..ca71abc4e0 100644
--- a/include/gpu/effects/GrCoverageSetOpXP.h
+++ b/include/gpu/effects/GrCoverageSetOpXP.h
@@ -14,6 +14,12 @@
 
 class GrProcOptInfo;
 
+// See the comment above GrXPFactory's definition about this warning suppression.
+#if defined(__GNUC__) || defined(__clang)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wnon-virtual-dtor"
+#endif
+
 /**
  * This xfer processor directly blends the the src coverage with the dst using a set operator. It is
  * useful for rendering coverage masks using CSG. It can optionally invert the src coverage before
@@ -21,13 +27,13 @@ class GrProcOptInfo;
  */
 class GrCoverageSetOpXPFactory : public GrXPFactory {
 public:
-    static sk_sp<GrXPFactory> Make(SkRegion::Op regionOp, bool invertCoverage = false);
+    static const GrXPFactory* Get(SkRegion::Op regionOp, bool invertCoverage = false);
 
     void getInvariantBlendedColor(const GrProcOptInfo& colorPOI,
                                   GrXPFactory::InvariantBlendedColor*) const override;
 
 private:
-    GrCoverageSetOpXPFactory(SkRegion::Op regionOp, bool invertCoverage);
+    constexpr GrCoverageSetOpXPFactory(SkRegion::Op regionOp, bool invertCoverage);
 
     GrXferProcessor* onCreateXferProcessor(const GrCaps&,
                                            const GrPipelineAnalysis&,
@@ -38,11 +44,6 @@ private:
         return false;
     }
 
-    bool onIsEqual(const GrXPFactory& xpfBase) const override {
-        const GrCoverageSetOpXPFactory& xpf = xpfBase.cast<GrCoverageSetOpXPFactory>();
-        return fRegionOp == xpf.fRegionOp;
-    }
-
     GR_DECLARE_XP_FACTORY_TEST;
 
     SkRegion::Op fRegionOp;
@@ -50,5 +51,8 @@ private:
 
     typedef GrXPFactory INHERITED;
 };
+#if defined(__GNUC__) || defined(__clang)
+#pragma GCC diagnostic pop
+#endif
 #endif
 
diff --git a/include/gpu/effects/GrCustomXfermode.h b/include/gpu/effects/GrCustomXfermode.h
index a8c868e034..54309ddafc 100644
--- a/include/gpu/effects/GrCustomXfermode.h
+++ b/include/gpu/effects/GrCustomXfermode.h
@@ -20,7 +20,7 @@ class GrXPFactory;
  */
 namespace GrCustomXfermode {
     bool IsSupportedMode(SkBlendMode mode);
-    sk_sp<GrXPFactory> MakeXPFactory(SkBlendMode mode);
+    const GrXPFactory* Get(SkBlendMode mode);
 };
 
 #endif
diff --git a/include/gpu/effects/GrPorterDuffXferProcessor.h b/include/gpu/effects/GrPorterDuffXferProcessor.h
index 29d56ab29a..ca14275926 100644
--- a/include/gpu/effects/GrPorterDuffXferProcessor.h
+++ b/include/gpu/effects/GrPorterDuffXferProcessor.h
@@ -14,9 +14,14 @@
 
 class GrProcOptInfo;
 
+// See the comment above GrXPFactory's definition about this warning suppression.
+#if defined(__GNUC__) || defined(__clang)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wnon-virtual-dtor"
+#endif
 class GrPorterDuffXPFactory : public GrXPFactory {
 public:
-    static sk_sp<GrXPFactory> Make(SkBlendMode mode);
+    static const GrXPFactory* Get(SkBlendMode blendMode);
 
     void getInvariantBlendedColor(const GrProcOptInfo& colorPOI,
                                   GrXPFactory::InvariantBlendedColor*) const override;
@@ -51,7 +56,7 @@ public:
     static bool SrcOverWillNeedDstTexture(const GrCaps&, const GrPipelineAnalysis&);
 
 private:
-    GrPorterDuffXPFactory(SkBlendMode);
+    constexpr GrPorterDuffXPFactory(SkBlendMode);
 
     GrXferProcessor* onCreateXferProcessor(const GrCaps& caps,
                                            const GrPipelineAnalysis&,
@@ -60,18 +65,16 @@ private:
 
     bool onWillReadDstColor(const GrCaps&, const GrPipelineAnalysis&) const override;
 
-    bool onIsEqual(const GrXPFactory& xpfBase) const override {
-        const GrPorterDuffXPFactory& xpf = xpfBase.cast<GrPorterDuffXPFactory>();
-        return fXfermode == xpf.fXfermode;
-    }
-
     GR_DECLARE_XP_FACTORY_TEST;
     static void TestGetXPOutputTypes(const GrXferProcessor*, int* outPrimary, int* outSecondary);
 
-    SkBlendMode fXfermode;
+    SkBlendMode fBlendMode;
 
     friend class GrPorterDuffTest; // for TestGetXPOutputTypes()
     typedef GrXPFactory INHERITED;
 };
+#if defined(__GNUC__) || defined(__clang)
+#pragma GCC diagnostic pop
+#endif
 
 #endif