Made clip mask cache reuse depend on mask size/bounds (instead of render target size)

http://codereview.appspot.com/6190067/

git-svn-id: http://skia.googlecode.com/svn/trunk@3903 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp
index f8c43b0..7672f5a 100644 (file)
--- a/src/gpu/GrClipMaskManager.cpp
+++ b/src/gpu/GrClipMaskManager.cpp
@@ -394,12 +394,6 @@ bool GrClipMaskManager::createAlphaClipMask(GrGpu* gpu,
     GrRenderTarget* rt = origDrawState->getRenderTarget();
     GrAssert(NULL != rt);
 
-    if (fAACache.canReuse(clipIn, rt->width(), rt->height())) {
-        *result = fAACache.getLastMask();
-        fAACache.getLastBound(resultBounds);
-        return true;
-    }
-
     GrRect rtRect;
     rtRect.setLTRB(0, 0,
                     GrIntToScalar(rt->width()), GrIntToScalar(rt->height()));
@@ -432,6 +426,14 @@ bool GrClipMaskManager::createAlphaClipMask(GrGpu* gpu,
     GrAssert(SkScalarIsInt(bounds.width()));
     GrAssert(SkScalarIsInt(bounds.height()));
 
+    if (fAACache.canReuse(clipIn, 
+                          SkScalarCeilToInt(bounds.width()),
+                          SkScalarCeilToInt(bounds.height()))) {
+        *result = fAACache.getLastMask();
+        fAACache.getLastBound(resultBounds);
+        return true;
+    }
+
     const GrTextureDesc desc = {
         kRenderTarget_GrTextureFlagBit|kNoStencil_GrTextureFlagBit,
         SkScalarCeilToInt(bounds.width()),
@@ -548,7 +550,7 @@ bool GrClipMaskManager::createAlphaClipMask(GrGpu* gpu,
         }
     }
 
-    fAACache.set(clipIn, rt->width(), rt->height(), accum, bounds);
+    fAACache.set(clipIn, accum, bounds);
     *result = accum;
     *resultBounds = bounds;
     SkSafeUnref(accum);     // fAACache still has a ref to accum

diff --git a/src/gpu/GrClipMaskManager.h b/src/gpu/GrClipMaskManager.h
index 6520567..9c72552 100644 (file)
--- a/src/gpu/GrClipMaskManager.h
+++ b/src/gpu/GrClipMaskManager.h
@@ -68,8 +68,9 @@ public:
 
         GrClipStackFrame* back = (GrClipStackFrame*) fStack.back();
 
-        if (back->fLastWidth >= width &&
-            back->fLastHeight >= height &&
+        if (back->fLastMask &&
+            back->fLastMask->width() >= width &&
+            back->fLastMask->height() >= height &&
             clip == back->fLastClip) {
             return true;
         }
@@ -77,8 +78,7 @@ public:
         return false;
     }
 
-    void set(const GrClip& clip, int width, int height, 
-             GrTexture* mask, const GrRect& bound) {
+    void set(const GrClip& clip, GrTexture* mask, const GrRect& bound) {
 
         if (fStack.empty()) {
             GrAssert(false);
@@ -87,8 +87,6 @@ public:
 
         GrClipStackFrame* back = (GrClipStackFrame*) fStack.back();
 
-        back->fLastWidth = width;
-        back->fLastHeight = height;
         back->fLastClip = clip;
         SkSafeRef(mask);
         back->fLastMask.reset(mask);
@@ -127,30 +125,6 @@ public:
         }
     }
 
-    int getLastWidth() const {
-
-        if (fStack.empty()) {
-            GrAssert(false);
-            return -1;
-        }
-
-        GrClipStackFrame* back = (GrClipStackFrame*) fStack.back();
-
-        return back->fLastWidth;
-    }
-
-    int getLastHeight() const {
-
-        if (fStack.empty()) {
-            GrAssert(false);
-            return -1;
-        }
-
-        GrClipStackFrame* back = (GrClipStackFrame*) fStack.back();
-
-        return back->fLastHeight;
-    }
-
     void getLastClip(GrClip* clip) const {
 
         if (fStack.empty()) {
@@ -254,19 +228,11 @@ private:
         }
 
         void reset () {
-            fLastWidth = -1;
-            fLastHeight = -1;
             fLastClip.setEmpty();
             fLastMask.reset(NULL);
             fLastBound.setEmpty();
         }
 
-        // fLastWidth & fLastHeight store the render target size used when
-        // creating the mask. They factor into the reuse decision (in canReuse)
-        // TODO: We should probably use the mask's width & height rather than
-        // the render target's width & height for reuse decisions
-        int                     fLastWidth;
-        int                     fLastHeight;
         GrClip                  fLastClip;
         // The mask's width & height values are used in setupDrawStateAAClip to 
         // correctly scale the uvs for geometry drawn with this mask

diff --git a/tests/ClipCacheTest.cpp b/tests/ClipCacheTest.cpp
index 999dc54..a8b220d 100644 (file)
--- a/tests/ClipCacheTest.cpp
+++ b/tests/ClipCacheTest.cpp
@@ -40,14 +40,9 @@ static GrTexture* createTexture(GrContext* context) {
 // verify that the top state of the stack matches the passed in state
 static void check_state(skiatest::Reporter* reporter,
                         const GrClipMaskCache& cache,
-                        int width,
-                        int height,
                         const GrClip& clip,
                         GrTexture* mask,
                         const GrRect& bound) {
-    REPORTER_ASSERT(reporter, width == cache.getLastWidth());
-    REPORTER_ASSERT(reporter, height == cache.getLastHeight());
-
     GrClip cacheClip;
     cache.getLastClip(&cacheClip);
     REPORTER_ASSERT(reporter, clip == cacheClip);
@@ -73,7 +68,7 @@ static void test_cache(skiatest::Reporter* reporter, GrContext* context) {
     emptyBound.setEmpty();
 
     // check initial state
-    check_state(reporter, cache, -1, -1, emptyClip, NULL, emptyBound);
+    check_state(reporter, cache, emptyClip, NULL, emptyBound);
 
     // set the current state
     GrRect bound1;
@@ -89,17 +84,17 @@ static void test_cache(skiatest::Reporter* reporter, GrContext* context) {
         return;
     }
 
-    cache.set(clip1, 128, 128, texture.get(), bound1);
+    cache.set(clip1, texture.get(), bound1);
 
     // check that the set took
-    check_state(reporter, cache, 128, 128, clip1, texture.get(), bound1);
+    check_state(reporter, cache, clip1, texture.get(), bound1);
     REPORTER_ASSERT(reporter, 2 == texture.get()->getRefCnt());
 
     // push the state
     cache.push();
 
     // verify that the pushed state is initially empty
-    check_state(reporter, cache, -1, -1, emptyClip, NULL, emptyBound);
+    check_state(reporter, cache, emptyClip, NULL, emptyBound);
     REPORTER_ASSERT(reporter, 2 == texture.get()->getRefCnt());
 
     // modify the new state
@@ -110,10 +105,10 @@ static void test_cache(skiatest::Reporter* reporter, GrContext* context) {
     clip2.setEmpty();
     clip2.setFromRect(bound2);
 
-    cache.set(clip2, 10, 10, texture.get(), bound2);
+    cache.set(clip2, texture.get(), bound2);
 
     // check that the changes took
-    check_state(reporter, cache, 10, 10, clip2, texture.get(), bound2);
+    check_state(reporter, cache, clip2, texture.get(), bound2);
     REPORTER_ASSERT(reporter, 3 == texture.get()->getRefCnt());
 
     // check to make sure canReuse works
@@ -124,14 +119,14 @@ static void test_cache(skiatest::Reporter* reporter, GrContext* context) {
     cache.pop();
 
     // verify that the old state is restored
-    check_state(reporter, cache, 128, 128, clip1, texture.get(), bound1);
+    check_state(reporter, cache, clip1, texture.get(), bound1);
     REPORTER_ASSERT(reporter, 2 == texture.get()->getRefCnt());
 
     // manually clear the state
     cache.reset();
 
     // verify it is now empty
-    check_state(reporter, cache, -1, -1, emptyClip, NULL, emptyBound);
+    check_state(reporter, cache, emptyClip, NULL, emptyBound);
     REPORTER_ASSERT(reporter, 1 == texture.get()->getRefCnt());
 
     // pop again - so there is no state