From a8f80de2bc17672b4b6f26d3cf6b38123ac850c9 Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Sat, 7 Jan 2017 09:37:13 -0500 Subject: 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 Commit-Queue: Brian Salomon --- include/gpu/GrPaint.h | 14 ++---- include/gpu/GrProcessorUnitTest.h | 59 ++++++++++++++++------- include/gpu/GrXferProcessor.h | 64 ++++++++----------------- include/gpu/effects/GrCoverageSetOpXP.h | 18 ++++--- include/gpu/effects/GrCustomXfermode.h | 2 +- include/gpu/effects/GrPorterDuffXferProcessor.h | 19 ++++---- 6 files changed, 89 insertions(+), 87 deletions(-) (limited to 'include') 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 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 fXPFactory; + const GrXPFactory* fXPFactory; SkSTArray<4, sk_sp> fColorFragmentProcessors; SkSTArray<2, sk_sp> 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 GrProcessorTestFactory : SkNoncopyable { +template class GrProcessorTestFactory : private SkNoncopyable { public: typedef sk_sp (*MakeProc)(GrProcessorTestData*); @@ -88,20 +89,44 @@ public: /** Use factory function at Index idx to create a processor. */ static sk_sp MakeIdx(int idx, GrProcessorTestData* data) { GrProcessorTestFactory* factory = (*GetFactories())[idx]; - return factory->fMakeProc(data); + sk_sp 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*, 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* 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 TestCreate(GrProcessorTestData*) #define GR_DECLARE_XP_FACTORY_TEST \ - static GrProcessorTestFactory gTestFactory SK_UNUSED; \ - static sk_sp 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 Effect :: gTestFactory(Effect :: TestCreate) - -#define GR_DEFINE_XP_FACTORY_TEST(Factory) \ - GrProcessorTestFactory Factory :: gTestFactory(Factory :: TestCreate) + GrProcessorTestFactory Effect::gTestFactory(Effect::TestCreate) #define GR_DEFINE_GEOMETRY_PROCESSOR_TEST(Effect) \ - GrProcessorTestFactory Effect :: gTestFactory(Effect :: TestCreate) + GrProcessorTestFactory Effect::gTestFactory(Effect::TestCreate) + +#define GR_DEFINE_XP_FACTORY_TEST(Factory) \ + GrXPFactoryTestFactory Factory::gTestFactory(Factory::TestGet) #else // !SK_ALLOW_STATIC_GLOBAL_INITIALIZERS @@ -138,17 +163,17 @@ private: static sk_sp TestCreate(GrProcessorTestData*) #define GR_DEFINE_FRAGMENT_PROCESSOR_TEST(X) -// 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 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 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 const T& cast() const { return *static_cast(this); } - - uint32_t classID() const { SkASSERT(kIllegalXPFClassID != fClassID); return fClassID; } - protected: - GrXPFactory() : fClassID(kIllegalXPFClassID) {} - - template 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(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 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(); - 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 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 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(); - 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 -- cgit v1.2.3