Avoid re-rendering stencil clip for every draw with reducable clip stack

Fixes the cases where clip stack reduction would cause clip to be
re-rendered to stencil for each draw call. This causes unneeded
slowdown.

Stencil cache would not be used because the clip stack generation id communicated
by the clip stack element list would be invalid. This happended due to

 a) clip stack reduction creating new elements in the element list.

 b) purging logic removing the generation id, but reduction logic
    selecting already purged element, and thus the generation id, as
    the representative state of the clip.

Cases of a) where reduction would flatten the stack to a single new
element were fixed by assigning the generation id of the top-most
element of the clip stack as the generation id of the new
element. This is not strictly minimal, but enables more caching than
using invalid id.

Cases of a) where reduction would substitute a stack element with a
new element the generation id of the substituted element is used.

The b) part was fixed by removing the purging logic. It was not
exactly correct, as the previously purged states were actually
used. The purging was not used for anything.

Changes SkClipStack API to highlight that invalid generation id is
never returned by SkClipStack. Empty stacks are wide open. Changes the
clients to reflect this.

Fixes a crash when not passing anti-alias out parameter to
GrReducedClip::ReduceClipStack. The crash is not exercised in the
current code.

Committed: http://code.google.com/p/skia/source/detail?r=12084

R=bsalomon@google.com, robertphillips@google.com

Author: kkinnunen@nvidia.com

Review URL: https://codereview.chromium.org/48593003

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

7 files changed, 53 insertions, 45 deletions

diff --git a/src/gpu/GrClipMaskCache.h b/src/gpu/GrClipMaskCache.h
index 97b4b515cc..213e2823e3 100644
--- a/src/gpu/GrClipMaskCache.h
+++ b/src/gpu/GrClipMaskCache.h
@@ -36,10 +36,6 @@ public:
         SkASSERT(clipGenID != SkClipStack::kWideOpenGenID);
         SkASSERT(clipGenID != SkClipStack::kEmptyGenID);
 
-        if (SkClipStack::kInvalidGenID == clipGenID) {
-            return false;
-        }
-
         GrClipStackFrame* back = (GrClipStackFrame*) fStack.back();
 
         // We could reuse the mask if bounds is a subset of last bounds. We'd have to communicate
diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp
index 0f915666fb..3aef3dee67 100644
--- a/src/gpu/GrClipMaskManager.cpp
+++ b/src/gpu/GrClipMaskManager.cpp
@@ -113,6 +113,7 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn,
     fCurrClipMaskType = kNone_ClipMaskType;
 
     ElementList elements(16);
+    int32_t genID;
     InitialState initialState;
     SkIRect clipSpaceIBounds;
     bool requiresAA;
@@ -132,6 +133,7 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn,
         ReduceClipStack(*clipDataIn->fClipStack,
                         clipSpaceRTIBounds,
                         &elements,
+                        &genID,
                         &initialState,
                         &clipSpaceIBounds,
                         &requiresAA);
@@ -156,7 +158,6 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn,
 
     // If MSAA is enabled we can do everything in the stencil buffer.
     if (0 == rt->numSamples() && requiresAA) {
-        int32_t genID = clipDataIn->fClipStack->getTopmostGenID();
         GrTexture* result = NULL;
 
         if (this->useSWOnlyPath(elements)) {
@@ -207,7 +208,8 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn,
 
     // use the stencil clip if we can't represent the clip as a rectangle.
     SkIPoint clipSpaceToStencilSpaceOffset = -clipDataIn->fOrigin;
-    this->createStencilClipMask(initialState,
+    this->createStencilClipMask(genID,
+                                initialState,
                                 elements,
                                 clipSpaceIBounds,
                                 clipSpaceToStencilSpaceOffset);
@@ -390,11 +392,11 @@ void GrClipMaskManager::getTemp(int width, int height, GrAutoScratchTexture* tem
 // Handles caching & allocation (if needed) of a clip alpha-mask texture for both the sw-upload
 // or gpu-rendered cases. Returns true if there is no more work to be done (i.e., we got a cache
 // hit)
-bool GrClipMaskManager::getMaskTexture(int32_t clipStackGenID,
+bool GrClipMaskManager::getMaskTexture(int32_t elementsGenID,
                                        const SkIRect& clipSpaceIBounds,
                                        GrTexture** result,
                                        bool willUpload) {
-    bool cached = fAACache.canReuse(clipStackGenID, clipSpaceIBounds);
+    bool cached = fAACache.canReuse(elementsGenID, clipSpaceIBounds);
     if (!cached) {
 
         // There isn't a suitable entry in the cache so we create a new texture to store the mask.
@@ -412,7 +414,7 @@ bool GrClipMaskManager::getMaskTexture(int32_t clipStackGenID,
             desc.fConfig = kAlpha_8_GrPixelConfig;
         }
 
-        fAACache.acquireMask(clipStackGenID, desc, clipSpaceIBounds);
+        fAACache.acquireMask(elementsGenID, desc, clipSpaceIBounds);
     }
 
     *result = fAACache.getLastMask();
@@ -421,14 +423,14 @@ bool GrClipMaskManager::getMaskTexture(int32_t clipStackGenID,
 
 ////////////////////////////////////////////////////////////////////////////////
 // Create a 8-bit clip mask in alpha
-GrTexture* GrClipMaskManager::createAlphaClipMask(int32_t clipStackGenID,
+GrTexture* GrClipMaskManager::createAlphaClipMask(int32_t elementsGenID,
                                                   InitialState initialState,
                                                   const ElementList& elements,
                                                   const SkIRect& clipSpaceIBounds) {
     SkASSERT(kNone_ClipMaskType == fCurrClipMaskType);
 
     GrTexture* result;
-    if (this->getMaskTexture(clipStackGenID, clipSpaceIBounds, &result, false)) {
+    if (this->getMaskTexture(elementsGenID, clipSpaceIBounds, &result, false)) {
         fCurrClipMaskType = kAlpha_ClipMaskType;
         return result;
     }
@@ -569,7 +571,8 @@ GrTexture* GrClipMaskManager::createAlphaClipMask(int32_t clipStackGenID,
 ////////////////////////////////////////////////////////////////////////////////
 // Create a 1-bit clip mask in the stencil buffer. 'devClipBounds' are in device
 // (as opposed to canvas) coordinates
-bool GrClipMaskManager::createStencilClipMask(InitialState initialState,
+bool GrClipMaskManager::createStencilClipMask(int32_t elementsGenID,
+                                              InitialState initialState,
                                               const ElementList& elements,
                                               const SkIRect& clipSpaceIBounds,
                                               const SkIPoint& clipSpaceToStencilOffset) {
@@ -587,11 +590,10 @@ bool GrClipMaskManager::createStencilClipMask(InitialState initialState,
     if (NULL == stencilBuffer) {
         return false;
     }
-    int32_t genID = elements.tail()->getGenID();
 
-    if (stencilBuffer->mustRenderClip(genID, clipSpaceIBounds, clipSpaceToStencilOffset)) {
+    if (stencilBuffer->mustRenderClip(elementsGenID, clipSpaceIBounds, clipSpaceToStencilOffset)) {
 
-        stencilBuffer->setLastClip(genID, clipSpaceIBounds, clipSpaceToStencilOffset);
+        stencilBuffer->setLastClip(elementsGenID, clipSpaceIBounds, clipSpaceToStencilOffset);
 
         // Set the matrix so that rendered clip elements are transformed from clip to stencil space.
         SkVector translate = {
@@ -921,14 +923,14 @@ void GrClipMaskManager::adjustStencilParams(GrStencilSettings* settings,
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-GrTexture* GrClipMaskManager::createSoftwareClipMask(int32_t clipStackGenID,
+GrTexture* GrClipMaskManager::createSoftwareClipMask(int32_t elementsGenID,
                                                      GrReducedClip::InitialState initialState,
                                                      const GrReducedClip::ElementList& elements,
                                                      const SkIRect& clipSpaceIBounds) {
     SkASSERT(kNone_ClipMaskType == fCurrClipMaskType);
 
     GrTexture* result;
-    if (this->getMaskTexture(clipStackGenID, clipSpaceIBounds, &result, true)) {
+    if (this->getMaskTexture(elementsGenID, clipSpaceIBounds, &result, true)) {
         return result;
     }
 
diff --git a/src/gpu/GrClipMaskManager.h b/src/gpu/GrClipMaskManager.h
index 015c801d54..f44a8e7b22 100644
--- a/src/gpu/GrClipMaskManager.h
+++ b/src/gpu/GrClipMaskManager.h
@@ -103,18 +103,19 @@ private:
     GrClipMaskCache fAACache;       // cache for the AA path
 
     // Draws the clip into the stencil buffer
-    bool createStencilClipMask(GrReducedClip::InitialState initialState,
+    bool createStencilClipMask(int32_t elementsGenID,
+                               GrReducedClip::InitialState initialState,
                                const GrReducedClip::ElementList& elements,
                                const SkIRect& clipSpaceIBounds,
                                const SkIPoint& clipSpaceToStencilOffset);
     // Creates an alpha mask of the clip. The mask is a rasterization of elements through the
     // rect specified by clipSpaceIBounds.
-    GrTexture* createAlphaClipMask(int32_t clipStackGenID,
+    GrTexture* createAlphaClipMask(int32_t elementsGenID,
                                    GrReducedClip::InitialState initialState,
                                    const GrReducedClip::ElementList& elements,
                                    const SkIRect& clipSpaceIBounds);
     // Similar to createAlphaClipMask but it rasterizes in SW and uploads to the result texture.
-    GrTexture* createSoftwareClipMask(int32_t clipStackGenID,
+    GrTexture* createSoftwareClipMask(int32_t elementsGenID,
                                       GrReducedClip::InitialState initialState,
                                       const GrReducedClip::ElementList& elements,
                                       const SkIRect& clipSpaceIBounds);
@@ -122,7 +123,7 @@ private:
     // Gets a texture to use for the clip mask. If true is returned then a cached mask was found
     // that already contains the rasterization of the clip stack, otherwise an uninitialized texture
     // is returned. 'willUpload' is set when the alpha mask needs to be uploaded from the CPU.
-    bool getMaskTexture(int32_t clipStackGenID,
+    bool getMaskTexture(int32_t elementsGenID,
                         const SkIRect& clipSpaceIBounds,
                         GrTexture** result,
                         bool willUpload);
diff --git a/src/gpu/GrReducedClip.cpp b/src/gpu/GrReducedClip.cpp
index a5f4519ea7..8480e041b8 100644
--- a/src/gpu/GrReducedClip.cpp
+++ b/src/gpu/GrReducedClip.cpp
@@ -17,6 +17,7 @@ namespace GrReducedClip {
 void reduced_stack_walker(const SkClipStack& stack,
                           const SkRect& queryBounds,
                           ElementList* result,
+                          int32_t* resultGenID,
                           InitialState* initialState,
                           bool* requiresAA);
 
@@ -30,11 +31,17 @@ take a rect in case the caller knows a bound on what is to be drawn through this
 void ReduceClipStack(const SkClipStack& stack,
                      const SkIRect& queryBounds,
                      ElementList* result,
+                     int32_t* resultGenID,
                      InitialState* initialState,
                      SkIRect* tighterBounds,
                      bool* requiresAA) {
     result->reset();
 
+    // The clip established by the element list might be cached based on the last
+    // generation id. When we make early returns, we do not know what was the generation
+    // id that lead to the state. Make a conservative guess.
+    *resultGenID = stack.getTopmostGenID();
+
     if (stack.isWideOpen()) {
         *initialState = kAllIn_InitialState;
         return;
@@ -70,7 +77,9 @@ void ReduceClipStack(const SkClipStack& stack,
                 SkRect scalarTighterBounds = SkRect::Make(*tighterBounds);
                 if (scalarTighterBounds == isectRect) {
                     // the round-out didn't add any area outside the clip rect.
-                    *requiresAA = false;
+                    if (NULL != requiresAA) {
+                        *requiresAA = false;
+                    }
                     *initialState = kAllIn_InitialState;
                     return;
                 }
@@ -123,12 +132,17 @@ void ReduceClipStack(const SkClipStack& stack,
 
     // Now that we have determined the bounds to use and filtered out the trivial cases, call the
     // helper that actually walks the stack.
-    reduced_stack_walker(stack, scalarBounds, result, initialState, requiresAA);
+    reduced_stack_walker(stack, scalarBounds, result, resultGenID, initialState, requiresAA);
+
+    // The list that was computed in this function may be cached based on the gen id of the last
+    // element.
+    SkASSERT(SkClipStack::kInvalidGenID != *resultGenID);
 }
 
 void reduced_stack_walker(const SkClipStack& stack,
                           const SkRect& queryBounds,
                           ElementList* result,
+                          int32_t* resultGenID,
                           InitialState* initialState,
                           bool* requiresAA) {
 
@@ -312,6 +326,11 @@ void reduced_stack_walker(const SkClipStack& stack,
                 break;
         }
         if (!skippable) {
+            if (0 == result->count()) {
+                // This will be the last element. Record the stricter genID.
+                *resultGenID = element->getGenID();
+            }
+
             // if it is a flip, change it to a bounds-filling rect
             if (isFlip) {
                 SkASSERT(SkRegion::kXOR_Op == element->getOp() ||
@@ -417,5 +436,13 @@ void reduced_stack_walker(const SkClipStack& stack,
     if (NULL != requiresAA) {
         *requiresAA = numAAElements > 0;
     }
+
+    if (0 == result->count()) {
+        if (*initialState == kAllIn_InitialState) {
+            *resultGenID = SkClipStack::kWideOpenGenID;
+        } else {
+            *resultGenID = SkClipStack::kEmptyGenID;
+        }
+    }
 }
 } // namespace GrReducedClip
diff --git a/src/gpu/GrReducedClip.h b/src/gpu/GrReducedClip.h
index abfc244f20..0b79f2c7f5 100644
--- a/src/gpu/GrReducedClip.h
+++ b/src/gpu/GrReducedClip.h
@@ -20,7 +20,8 @@ enum InitialState {
 
 /**
  * This function takes a clip stack and a query rectangle and it produces a reduced set of
- * SkClipStack::Elements that are equivalent to applying the full stack to the rectangle. The
+ * SkClipStack::Elements that are equivalent to applying the full stack to the rectangle. The clip
+ * stack generation id that represents the list of elements is returned in resultGenID. The
  * initial state of the query rectangle before the first clip element is applied is returned via
  * initialState. Optionally, the caller can request a tighter bounds on the clip be returned via
  * tighterBounds. If not NULL, tighterBounds will always be contained by queryBounds after return.
@@ -33,6 +34,7 @@ enum InitialState {
 void ReduceClipStack(const SkClipStack& stack,
                      const SkIRect& queryBounds,
                      ElementList* result,
+                     int32_t* resultGenID,
                      InitialState* initialState,
                      SkIRect* tighterBounds = NULL,
                      bool* requiresAA = NULL);
diff --git a/src/gpu/GrStencilBuffer.h b/src/gpu/GrStencilBuffer.h
index 3765a4c657..37d40f16ba 100644
--- a/src/gpu/GrStencilBuffer.h
+++ b/src/gpu/GrStencilBuffer.h
@@ -43,8 +43,7 @@ public:
     bool mustRenderClip(int32_t clipStackGenID,
                         const SkIRect& clipSpaceRect,
                         const SkIPoint clipSpaceToStencilOffset) const {
-        return SkClipStack::kInvalidGenID == clipStackGenID ||
-               fLastClipStackGenID != clipStackGenID ||
+        return fLastClipStackGenID != clipStackGenID ||
                fLastClipSpaceOffset != clipSpaceToStencilOffset ||
                !fLastClipStackRect.contains(clipSpaceRect);
     }
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index 4041c411fc..7630ccd08c 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -344,34 +344,15 @@ void SkGpuDevice::writePixels(const SkBitmap& bitmap, int x, int y,
                                config, bitmap.getPixels(), bitmap.rowBytes(), flags);
 }
 
-namespace {
-void purgeClipCB(int genID, void* ) {
-
-    if (SkClipStack::kInvalidGenID == genID ||
-        SkClipStack::kEmptyGenID == genID ||
-        SkClipStack::kWideOpenGenID == genID) {
-        // none of these cases will have a cached clip mask
-        return;
-    }
-
-}
-};
-
 void SkGpuDevice::onAttachToCanvas(SkCanvas* canvas) {
     INHERITED::onAttachToCanvas(canvas);
 
     // Canvas promises that this ptr is valid until onDetachFromCanvas is called
     fClipData.fClipStack = canvas->getClipStack();
-
-    fClipData.fClipStack->addPurgeClipCallback(purgeClipCB, fContext);
 }
 
 void SkGpuDevice::onDetachFromCanvas() {
     INHERITED::onDetachFromCanvas();
-
-    // TODO: iterate through the clip stack and clean up any cached clip masks
-    fClipData.fClipStack->removePurgeClipCallback(purgeClipCB, fContext);
-
     fClipData.fClipStack = NULL;
 }