Let them eat GrEffectRef.
Changes the remaining existing code that operates on naked GrEffects to GrEffectRef.
Review URL: https://codereview.appspot.com/7124058
git-svn-id: http://skia.googlecode.com/svn/trunk@7321 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/include/gpu/GrBackendEffectFactory.h b/include/gpu/GrBackendEffectFactory.h
index 437bcdd..a291387 100644
--- a/include/gpu/GrBackendEffectFactory.h
+++ b/include/gpu/GrBackendEffectFactory.h
@@ -23,7 +23,7 @@
of GrGLEffect.
*/
-class GrEffect;
+class GrEffectRef;
class GrEffectStage;
class GrGLEffect;
class GrGLCaps;
@@ -44,7 +44,7 @@
};
virtual EffectKey glEffectKey(const GrEffectStage&, const GrGLCaps&) const = 0;
- virtual GrGLEffect* createGLInstance(const GrEffect&) const = 0;
+ virtual GrGLEffect* createGLInstance(const GrEffectRef&) const = 0;
bool operator ==(const GrBackendEffectFactory& b) const {
return fEffectClassID == b.fEffectClassID;
diff --git a/include/gpu/GrEffect.h b/include/gpu/GrEffect.h
index 507bd57..0e6aab0 100644
--- a/include/gpu/GrEffect.h
+++ b/include/gpu/GrEffect.h
@@ -34,6 +34,9 @@
GrEffect* get() { return fEffect; }
const GrEffect* get() const { return fEffect; }
+ const GrEffect* operator-> () { return fEffect; }
+ const GrEffect* operator-> () const { return fEffect; }
+
void* operator new(size_t size);
void operator delete(void* target);
@@ -58,8 +61,11 @@
GrEffect subclass objects should be created by factory functions that return GrEffectRef.
There is no public way to wrap a GrEffect in a GrEffectRef. Thus, a factory should be a static
member function of a GrEffect subclass.
+
+ Because almost no code should ever handle a GrEffect outside of a GrEffectRef, we privately
+ inherit from GrRefCnt to help prevent accidental direct ref'ing/unref'ing of effects.
*/
-class GrEffect : public GrRefCnt {
+class GrEffect : private GrRefCnt {
public:
SK_DECLARE_INST_COUNT(GrEffect)
@@ -113,16 +119,16 @@
computed by the GrBackendEffectFactory:
effectA.getFactory().glEffectKey(effectA) == effectB.getFactory().glEffectKey(effectB).
*/
- bool isEqual(const GrEffect& other) const {
- if (&this->getFactory() != &other.getFactory()) {
+ bool isEqual(const GrEffectRef& other) const {
+ if (&this->getFactory() != &other->getFactory()) {
return false;
}
bool result = this->onIsEqual(other);
#if GR_DEBUG
if (result) {
- GrAssert(this->numTextures() == other.numTextures());
+ GrAssert(this->numTextures() == other->numTextures());
for (int i = 0; i < this->numTextures(); ++i) {
- GrAssert(*fTextureAccesses[i] == *other.fTextureAccesses[i]);
+ GrAssert(*fTextureAccesses[i] == *other->fTextureAccesses[i]);
}
}
#endif
@@ -154,6 +160,11 @@
void* operator new(size_t size);
void operator delete(void* target);
+ /** These use non-standard names because GrEffects should only be ref'ed an unref'ed deep in
+ the bowels. Rendering code should use GrEffectRef. */
+ void addRef() { this->ref(); }
+ void subRef() { this->unref(); }
+
protected:
/**
* Subclasses call this from their constructor to register GrTextureAcceses. The effect subclass
@@ -164,7 +175,8 @@
GrEffect() : fEffectRef(NULL) {};
- /** This should be called by GrEffect subclass factories */
+ /** This should be called by GrEffect subclass factories. See the comment on AutoEffectUnref for
+ an example factory function. */
static GrEffectRef* CreateEffectRef(GrEffect* effect) {
if (NULL == effect->fEffectRef) {
effect->fEffectRef = SkNEW_ARGS(GrEffectRef, (effect));
@@ -175,16 +187,41 @@
return effect->fEffectRef;
}
+ /** Helper used in subclass factory functions to unref the effect after it has been wrapped in a
+ GrEffectRef. E.g.:
+
+ class EffectSubclass : public GrEffect {
+ public:
+ GrEffectRef* Create(ParamType1 param1, ParamType2 param2, ...) {
+ AutoEffectUnref effect(SkNEW_ARGS(EffectSubclass, (param1, param2, ...)));
+ return CreateEffectRef(effect);
+ }
+ */
+ class AutoEffectUnref {
+ public:
+ AutoEffectUnref(GrEffect* effect) : fEffect(effect) { }
+ ~AutoEffectUnref() { fEffect->subRef(); }
+ operator GrEffect*() { return fEffect; }
+ private:
+ GrEffect* fEffect;
+ };
+
+ /** Helper for getting the GrEffect out of a GrEffectRef and down-casting to a GrEffect subclass
+ */
+ template <typename T>
+ static const T& CastEffect(const GrEffectRef& effectRef) {
+ GrAssert(NULL != effectRef.get());
+ return *static_cast<const T*>(effectRef.get());
+ }
+
private:
/** Subclass implements this to support isEqual(). It will only be called if it is known that
- the two effects are of the same subclass (i.e. they return the same object
- from getFactory()).*/
- virtual bool onIsEqual(const GrEffect& other) const = 0;
+ the two effects are of the same subclass (i.e. they return the same object from
+ getFactory()).*/
+ virtual bool onIsEqual(const GrEffectRef& other) const = 0;
- void EffectRefDestroyed() {
- fEffectRef = NULL;
- }
+ void EffectRefDestroyed() { fEffectRef = NULL; }
friend class GrEffectRef; // to call GrEffectRef destroyed
diff --git a/include/gpu/GrEffectStage.h b/include/gpu/GrEffectStage.h
index 0060152..c09cd45 100644
--- a/include/gpu/GrEffectStage.h
+++ b/include/gpu/GrEffectStage.h
@@ -39,7 +39,7 @@
return false;
}
- if (!this->getEffect()->isEqual(*other.getEffect())) {
+ if (!(*this->getEffect())->isEqual(*other.getEffect())) {
return false;
}
@@ -112,14 +112,7 @@
return EffectRef;
}
- // TODO: Push GrEffectRef deeper and make this getter return it rather than GrEffect.
- const GrEffect* getEffect() const {
- if (NULL != fEffectRef) {
- return fEffectRef->get();
- } else {
- return NULL;
- }
- }
+ const GrEffectRef* getEffect() const { return fEffectRef; }
private:
SkMatrix fCoordChangeMatrix;
@@ -129,4 +122,3 @@
};
#endif
-
diff --git a/include/gpu/GrTBackendEffectFactory.h b/include/gpu/GrTBackendEffectFactory.h
index 52dbc64..7ea7e39 100644
--- a/include/gpu/GrTBackendEffectFactory.h
+++ b/include/gpu/GrTBackendEffectFactory.h
@@ -34,7 +34,7 @@
const GrGLCaps& caps) const SK_OVERRIDE {
GrAssert(kIllegalEffectClassID != fEffectClassID);
EffectKey effectKey = GLEffect::GenKey(stage, caps);
- EffectKey textureKey = GLEffect::GenTextureKey(*stage.getEffect(), caps);
+ EffectKey textureKey = GLEffect::GenTextureKey(stage.getEffect(), caps);
#if GR_DEBUG
static const EffectKey kIllegalIDMask = (uint16_t) (~((1U << kEffectKeyBits) - 1));
GrAssert(!(kIllegalIDMask & effectKey));
@@ -48,7 +48,7 @@
/** Returns a new instance of the appropriate *GL* implementation class
for the given GrEffect; caller is responsible for deleting
the object. */
- virtual GLEffect* createGLInstance(const GrEffect& effect) const SK_OVERRIDE {
+ virtual GLEffect* createGLInstance(const GrEffectRef& effect) const SK_OVERRIDE {
return SkNEW_ARGS(GLEffect, (*this, effect));
}