GrClip no longer translates its clips (to better mimic SkClipStack's behavior)

http://codereview.appspot.com/6445052/



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

7 files changed, 117 insertions, 91 deletions

diff --git a/include/gpu/GrClip.h b/include/gpu/GrClip.h
index d6655dd8e0..bb07c28de1 100644
--- a/include/gpu/GrClip.h
+++ b/include/gpu/GrClip.h
@@ -23,11 +23,7 @@ class GrClip {
 public:
     GrClip();
     GrClip(const GrClip& src);
-    /**
-     *  The conservativeBounds parameter already takes (tx,ty) into account.
-     */
-    GrClip(GrClipIterator* iter, GrScalar tx, GrScalar ty,
-           const GrRect& conservativeBounds);
+    GrClip(GrClipIterator* iter, const GrRect& conservativeBounds);
     explicit GrClip(const GrIRect& rect);
     explicit GrClip(const GrRect& rect);
 
@@ -142,20 +138,16 @@ public:
         }
     }
 
-    // FIXME: This word "empty" is confusing. It means that the clip has no
-    // elements (it is the infinite plane) not that it has no area.
-    bool isEmpty() const { return 0 == fList.count(); }
+    // isWideOpen returns true if the clip has no elements (it is the 
+    // infinite plane) not that it has no area.
+    bool isWideOpen() const { return 0 == fList.count(); }
 
     /**
      *  Resets this clip to be empty
      */
     void setEmpty();
 
-    /**
-     *  If specified, the bounds parameter already takes (tx,ty) into account.
-     */
-    void setFromIterator(GrClipIterator* iter, GrScalar tx, GrScalar ty,
-                         const GrRect& conservativeBounds);
+    void setFromIterator(GrClipIterator* iter, const GrRect& conservativeBounds);
     void setFromRect(const GrRect& rect);
     void setFromIRect(const GrIRect& rect);
 
diff --git a/src/gpu/GrClip.cpp b/src/gpu/GrClip.cpp
index 9ce89f3f20..a3905baefb 100644
--- a/src/gpu/GrClip.cpp
+++ b/src/gpu/GrClip.cpp
@@ -30,9 +30,8 @@ GrClip::GrClip(const GrRect& rect) {
     this->setFromRect(rect);
 }
 
-GrClip::GrClip(GrClipIterator* iter, GrScalar tx, GrScalar ty,
-               const GrRect& bounds) {
-    this->setFromIterator(iter, tx, ty, bounds);
+GrClip::GrClip(GrClipIterator* iter, const GrRect& bounds) {
+    this->setFromIterator(iter, bounds);
 }
 
 GrClip::~GrClip() {}
@@ -92,7 +91,7 @@ static void intersectWith(SkRect* dst, const SkRect& src) {
     }
 }
 
-void GrClip::setFromIterator(GrClipIterator* iter, GrScalar tx, GrScalar ty,
+void GrClip::setFromIterator(GrClipIterator* iter,
                              const GrRect& conservativeBounds) {
     fList.reset();
     fRequiresAA = false;
@@ -116,9 +115,6 @@ void GrClip::setFromIterator(GrClipIterator* iter, GrScalar tx, GrScalar ty,
             switch (e.fType) {
                 case kRect_ClipType:
                     iter->getRect(&e.fRect);
-                    if (tx || ty) {
-                        e.fRect.offset(tx, ty);
-                    }
                     ++rectCount;
                     if (isectRectValid) {
                         if (SkRegion::kIntersect_Op == e.fOp) {
@@ -137,9 +133,6 @@ void GrClip::setFromIterator(GrClipIterator* iter, GrScalar tx, GrScalar ty,
                     break;
                 case kPath_ClipType:
                     e.fPath = *iter->getPath();
-                    if (tx || ty) {
-                        e.fPath.offset(tx, ty);
-                    }
                     e.fPathFill = iter->getPathFill();
                     isectRectValid = false;
                     break;
@@ -252,6 +245,12 @@ void GrClip::Iter::reset(const GrClip& stack, IterStart startLoc) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+/**
+ * getConservativeBounds returns the conservative bounding box of the clip
+ * in device (as opposed to canvas) coordinates. If the bounding box is
+ * the result of purely intersections of rects (with an initial replace)
+ * isIntersectionOfRects will be set to true.
+ */
 void GrClipData::getConservativeBounds(const GrSurface* surface,
                                        GrIRect* result,
                                        bool* isIntersectionOfRects) const {
@@ -266,10 +265,18 @@ void GrClipData::getConservativeBounds(const GrSurface* surface,
                                    SkIntToScalar(surface->height()));
 
     GrRect conservativeBounds = fClipStack->getConservativeBounds();
+
+    conservativeBounds.offset(SkIntToScalar(-fOrigin.fX), 
+                              SkIntToScalar(-fOrigin.fY));
+
     if (!conservativeBounds.intersect(temp)) {
         conservativeBounds.setEmpty();
     }
 
     conservativeBounds.roundOut(result);
+
+    if (NULL != isIntersectionOfRects) {
+        *isIntersectionOfRects = fClipStack->isRect();
+    }
 }
 
diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp
index ea5a2cd693..97c102f66b 100644
--- a/src/gpu/GrClipMaskManager.cpp
+++ b/src/gpu/GrClipMaskManager.cpp
@@ -20,6 +20,18 @@
 //#define GR_AA_CLIP 1
 //#define GR_SW_CLIP 1
 
+GrClipMaskCache::GrClipMaskCache()
+    : fContext(NULL)
+    , fStack(sizeof(GrClipStackFrame)) {
+    // We need an initial frame to capture the clip state prior to 
+    // any pushes
+    SkNEW_PLACEMENT(fStack.push_back(), GrClipStackFrame);
+}
+
+void GrClipMaskCache::push() {
+    SkNEW_PLACEMENT(fStack.push_back(), GrClipStackFrame);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 namespace {
 // set up the draw state to enable the aa clipping mask. Besides setting up the 
@@ -136,7 +148,7 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn) {
     fCurrClipMaskType = kNone_ClipMaskType;
 
     GrDrawState* drawState = fGpu->drawState();
-    if (!drawState->isClipState() || clipDataIn->fClipStack->isEmpty()) {
+    if (!drawState->isClipState() || clipDataIn->fClipStack->isWideOpen()) {
         fGpu->disableScissor();
         this->setGpuStencil();
         return true;
@@ -162,7 +174,7 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn) {
     // Otherwise check if we should just create the entire clip mask 
     // in software (this will only happen if the clip mask is anti-aliased
     // and too complex for the gpu to handle in its entirety)
-    if (0 == rt->numSamples() && 
+    if (0 == rt->numSamples() &&
         requiresAA && 
         this->useSWOnlyPath(*clipDataIn->fClipStack)) {
         // The clip geometry is complex enough that it will be more
@@ -214,14 +226,14 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn) {
 
     // If the clip is a rectangle then just set the scissor. Otherwise, create
     // a stencil mask.
-    if (clipDataIn->fClipStack->isRect()) {
+    if (isIntersectionOfRects) {
         fGpu->enableScissor(bounds);
         this->setGpuStencil();
         return true;
     }
 
     // use the stencil clip if we can't represent the clip as a rectangle.
-    bool useStencil = !clipDataIn->fClipStack->isEmpty() && !bounds.isEmpty();
+    bool useStencil = !clipDataIn->fClipStack->isWideOpen() && !bounds.isEmpty();
 
     if (useStencil) {
         this->createStencilClipMask(*clipDataIn, bounds);
@@ -248,17 +260,20 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn) {
 namespace {
 /**
  * Does "container" contain "containee"? If either is empty then
- * no containment is possible.
+ * no containment is possible. "container" is in canvas coordinates while
+ * "containee" is in device coordiates. "origin" provides the mapping between
+ * the two.
  */
-bool contains(const SkRect& container, const SkIRect& containee) {
+bool contains(const SkRect& container, 
+              const SkIRect& containee,
+              const SkIPoint& origin) {
     return  !containee.isEmpty() && !container.isEmpty() &&
-            container.fLeft <= SkIntToScalar(containee.fLeft) && 
-            container.fTop <= SkIntToScalar(containee.fTop) &&
-            container.fRight >= SkIntToScalar(containee.fRight) && 
-            container.fBottom >= SkIntToScalar(containee.fBottom);
+            container.fLeft <= SkIntToScalar(containee.fLeft+origin.fX) && 
+            container.fTop <= SkIntToScalar(containee.fTop+origin.fY) &&
+            container.fRight >= SkIntToScalar(containee.fRight+origin.fX) && 
+            container.fBottom >= SkIntToScalar(containee.fBottom+origin.fY);
 }
 
-
 ////////////////////////////////////////////////////////////////////////////////
 // determines how many elements at the head of the clip can be skipped and
 // whether the initial clear should be to the inside- or outside-the-clip value,
@@ -267,7 +282,8 @@ const GrClip::Iter::Clip* process_initial_clip_elements(
                                   GrClip::Iter* iter,
                                   const GrIRect& bounds,
                                   bool* clearToInside,
-                                  SkRegion::Op* firstOp) {
+                                  SkRegion::Op* firstOp,
+                                  const GrClipData& clipData) {
 
     GrAssert(NULL != iter && NULL != clearToInside && NULL != firstOp);
 
@@ -294,7 +310,8 @@ const GrClip::Iter::Clip* process_initial_clip_elements(
             case SkRegion::kIntersect_Op:
                 // if this element contains the entire bounds then we
                 // can skip it.
-                if (NULL != clip->fRect && contains(*clip->fRect, bounds)) {
+                if (NULL != clip->fRect &&
+                    contains(*clip->fRect, bounds, clipData.fOrigin)) {
                     break;
                 }
                 // if everything is initially clearToInside then intersect is
@@ -360,7 +377,6 @@ const GrClip::Iter::Clip* process_initial_clip_elements(
 
 }
 
-
 namespace {
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -450,7 +466,7 @@ bool GrClipMaskManager::drawClipShape(GrTexture* target,
     if (NULL != clip->fRect) {
         if (clip->fDoAA) {
             getContext()->getAARectRenderer()->fillAARect(fGpu, fGpu,
-                                                          *clip->fRect, 
+                                                          *clip->fRect,
                                                           true);
         } else {
             fGpu->drawSimpleRect(*clip->fRect, NULL);
@@ -537,6 +553,7 @@ bool GrClipMaskManager::clipMaskPreamble(const GrClipData& clipDataIn,
 
     // unlike the stencil path the alpha path is not bound to the size of the
     // render target - determine the minimum size required for the mask
+    // Note: intBounds is in device (as opposed to canvas) coordinates
     GrIRect intBounds;
     clipDataIn.getConservativeBounds(rt, &intBounds);
 
@@ -573,6 +590,8 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn,
         return true;
     }
 
+    // Note: 'resultBounds' is in device (as opposed to canvas) coordinates
+
     GrTexture* accum = fAACache.getLastMask();
     if (NULL == accum) {
         fAACache.reset();
@@ -584,25 +603,25 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn,
 
     GrDrawTarget::AutoGeometryPush agp(fGpu);
 
-    if (0 != resultBounds->fTop || 0 != resultBounds->fLeft) {
+    if (0 != resultBounds->fTop || 0 != resultBounds->fLeft ||
+        0 != clipDataIn.fOrigin.fX || 0 != clipDataIn.fOrigin.fY) {
         // if we were able to trim down the size of the mask we need to 
         // offset the paths & rects that will be used to compute it
-        GrMatrix m;
-
-        m.setTranslate(SkIntToScalar(-resultBounds->fLeft), 
-                       SkIntToScalar(-resultBounds->fTop));
-
-        drawState->setViewMatrix(m);
+        drawState->viewMatrix()->setTranslate(
+                SkIntToScalar(-resultBounds->fLeft-clipDataIn.fOrigin.fX), 
+                SkIntToScalar(-resultBounds->fTop-clipDataIn.fOrigin.fY));
     }
 
     bool clearToInside;
     SkRegion::Op firstOp = SkRegion::kReplace_Op; // suppress warning
 
-    GrClip::Iter iter(*clipDataIn.fClipStack, GrClip::Iter::kBottom_IterStart);
+    GrClip::Iter iter(*clipDataIn.fClipStack, 
+                      GrClip::Iter::kBottom_IterStart);
     const GrClip::Iter::Clip* clip = process_initial_clip_elements(&iter,
                                                               *resultBounds,
                                                               &clearToInside,
-                                                              &firstOp);
+                                                              &firstOp,
+                                                              clipDataIn);
 
     fGpu->clear(NULL, 
                 clearToInside ? 0xffffffff : 0x00000000, 
@@ -633,9 +652,8 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn,
         } else if (SkRegion::kReverseDifference_Op == op ||
                    SkRegion::kIntersect_Op == op) {
             // there is no point in intersecting a screen filling rectangle.
-            if (SkRegion::kIntersect_Op == op &&
-                NULL != clip->fRect &&
-                contains(*clip->fRect, *resultBounds)) {
+            if (SkRegion::kIntersect_Op == op && NULL != clip->fRect &&
+                contains(*clip->fRect, *resultBounds, clipDataIn.fOrigin)) {
                 continue;
             }
 
@@ -654,13 +672,11 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn,
             // TODO: rather than adding these two translations here
             // compute the bounding box needed to render the texture
             // into temp
-            if (0 != resultBounds->fTop || 0 != resultBounds->fLeft) {
-                GrMatrix m;
-
-                m.setTranslate(SkIntToScalar(resultBounds->fLeft), 
-                               SkIntToScalar(resultBounds->fTop));
-
-                drawState->preConcatViewMatrix(m);
+            if (0 != resultBounds->fTop || 0 != resultBounds->fLeft ||
+                0 != clipDataIn.fOrigin.fX || 0 != clipDataIn.fOrigin.fY) {
+                // In order for the merge of the temp clip into the accumulator
+                // to work we need to disable the translation
+                drawState->viewMatrix()->reset();
             }
 
             // Now draw into the accumulator using the real operation
@@ -668,13 +684,11 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn,
             setup_boolean_blendcoeffs(drawState, op);
             this->drawTexture(accum, temp.texture());
 
-            if (0 != resultBounds->fTop || 0 != resultBounds->fLeft) {
-                GrMatrix m;
-
-                m.setTranslate(SkIntToScalar(-resultBounds->fLeft), 
-                               SkIntToScalar(-resultBounds->fTop));
-
-                drawState->preConcatViewMatrix(m);
+            if (0 != resultBounds->fTop || 0 != resultBounds->fLeft ||
+                0 != clipDataIn.fOrigin.fX || 0 != clipDataIn.fOrigin.fY) {
+                drawState->viewMatrix()->setTranslate(
+                        SkIntToScalar(-resultBounds->fLeft-clipDataIn.fOrigin.fX), 
+                        SkIntToScalar(-resultBounds->fTop-clipDataIn.fOrigin.fY));
             }
 
         } else {
@@ -691,7 +705,8 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn,
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// Create a 1-bit clip mask in the stencil buffer
+// Create a 1-bit clip mask in the stencil buffer. 'bounds' are in device 
+// (as opposed to canvas) coordinates
 bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn,
                                               const GrIRect& bounds) {
 
@@ -730,6 +745,17 @@ bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn,
         drawState->setRenderTarget(rt);
         GrDrawTarget::AutoGeometryPush agp(fGpu);
 
+        if (0 != clipDataIn.fOrigin.fX || 0 != clipDataIn.fOrigin.fY) {
+            // Add the saveLayer's offset to the view matrix rather than 
+            // offset each individual draw
+            GrMatrix m;
+
+            m.setTranslate(SkIntToScalar(-clipDataIn.fOrigin.fX), 
+                           SkIntToScalar(-clipDataIn.fOrigin.fY));
+
+            drawState->setViewMatrix(m);
+        }
+
 #if !VISUALIZE_COMPLEX_CLIP
         drawState->enableState(GrDrawState::kNoColorWrites_StateBit);
 #endif
@@ -749,7 +775,8 @@ bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn,
         const GrClip::Iter::Clip* clip = process_initial_clip_elements(&iter,
                                                   rtRect,
                                                   &clearToInside,
-                                                  &firstOp);
+                                                  &firstOp,
+                                                  clipDataIn);
 
         fGpu->clearStencilClip(bounds, clearToInside);
         bool first = true;
@@ -791,7 +818,7 @@ bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn,
                 // there is no point in intersecting a screen filling
                 // rectangle.
                 if (SkRegion::kIntersect_Op == op &&
-                    contains(*clip->fRect, rtRect)) {
+                    contains(*clip->fRect, rtRect, oldClipData->fOrigin)) {
                     continue;
                 }
             } else if (NULL != clip->fPath) {
@@ -862,11 +889,15 @@ bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn,
                     }
                 } else {
                     SET_RANDOM_COLOR
+                    // 'bounds' is already in device coordinates so the 
+                    // translation in the view matrix is inappropriate. 
+                    // Apply the inverse translation so the drawn rect will
+                    // be in the correct location
                     GrRect rect = GrRect::MakeLTRB(
-                            SkIntToScalar(bounds.fLeft),
-                            SkIntToScalar(bounds.fTop),
-                            SkIntToScalar(bounds.fRight),
-                            SkIntToScalar(bounds.fBottom));
+                            SkIntToScalar(bounds.fLeft+clipDataIn.fOrigin.fX),
+                            SkIntToScalar(bounds.fTop+clipDataIn.fOrigin.fY),
+                            SkIntToScalar(bounds.fRight+clipDataIn.fOrigin.fX),
+                            SkIntToScalar(bounds.fBottom+clipDataIn.fOrigin.fY));
                     fGpu->drawSimpleRect(rect, NULL);
                 }
             }
@@ -880,6 +911,7 @@ bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn,
     return true;
 }
 
+
 // mapping of clip-respecting stencil funcs to normal stencil funcs
 // mapping depends on whether stencil-clipping is in effect.
 static const GrStencilFunc 
@@ -1105,16 +1137,21 @@ bool GrClipMaskManager::createSoftwareClipMask(const GrClipData& clipDataIn,
 
     GrSWMaskHelper helper(this->getContext());
 
-    helper.init(*resultBounds, NULL);
+    GrMatrix matrix;
+    matrix.setTranslate(SkIntToScalar(-clipDataIn.fOrigin.fX), 
+                        SkIntToScalar(-clipDataIn.fOrigin.fY));
+    helper.init(*resultBounds, &matrix);
 
     bool clearToInside;
     SkRegion::Op firstOp = SkRegion::kReplace_Op; // suppress warning
 
-    GrClip::Iter iter(*clipDataIn.fClipStack, GrClip::Iter::kBottom_IterStart);
+    GrClip::Iter iter(*clipDataIn.fClipStack, 
+                      GrClip::Iter::kBottom_IterStart);
     const GrClip::Iter::Clip* clip = process_initial_clip_elements(&iter,
                                               *resultBounds,
                                               &clearToInside,
-                                              &firstOp);
+                                              &firstOp,
+                                              clipDataIn);
 
     helper.clear(clearToInside ? 0xFF : 0x00);
 
diff --git a/src/gpu/GrClipMaskManager.h b/src/gpu/GrClipMaskManager.h
index 0551b8cf2a..81be5ed919 100644
--- a/src/gpu/GrClipMaskManager.h
+++ b/src/gpu/GrClipMaskManager.h
@@ -33,13 +33,7 @@ class GrDrawState;
  */
 class GrClipMaskCache : public GrNoncopyable {
 public:
-    GrClipMaskCache()
-    : fContext(NULL)
-    , fStack(sizeof(GrClipStackFrame)) {
-        // We need an initial frame to capture the clip state prior to 
-        // any pushes
-        SkNEW_PLACEMENT(fStack.push_back(), GrClipStackFrame);
-    }
+    GrClipMaskCache();
 
     ~GrClipMaskCache() {
 
@@ -86,9 +80,7 @@ public:
      * TODO: can we take advantage of the nested nature of the clips to
      * reduce the mask creation cost?
      */
-    void push() {
-        SkNEW_PLACEMENT(fStack.push_back(), GrClipStackFrame);
-    }
+    void push();
 
     void pop() {
         //GrAssert(!fStack.empty());
diff --git a/src/gpu/GrTextContext.cpp b/src/gpu/GrTextContext.cpp
index 36e6646307..ab04bb48f5 100644
--- a/src/gpu/GrTextContext.cpp
+++ b/src/gpu/GrTextContext.cpp
@@ -88,6 +88,8 @@ GrTextContext::GrTextContext(GrContext* context,
     const GrClipData* clipData = context->getClip();
 
     GrRect conservativeBound = clipData->fClipStack->getConservativeBounds();
+    conservativeBound.offset(SkIntToScalar(-clipData->fOrigin.fX), 
+                             SkIntToScalar(-clipData->fOrigin.fY));
 
     if (!fExtMatrix.isIdentity()) {
         GrMatrix inverse;
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index 3e5bba0ce2..dcfb57b030 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -415,17 +415,13 @@ static void convert_matrixclip(GrContext* context, const SkMatrix& matrix,
 
     SkRect bounds;
     bool isIntersectionOfRects = false;
-    clipStack.getConservativeBounds(-origin.fX,
-                                    -origin.fY,
+    clipStack.getConservativeBounds(0, 0,
                                     renderTargetWidth,
                                     renderTargetHeight,
                                     &bounds,
                                     &isIntersectionOfRects);
 
-    result->setFromIterator(&iter,
-                            GrIntToScalar(-origin.x()),
-                            GrIntToScalar(-origin.y()),
-                            bounds);
+    result->setFromIterator(&iter, bounds);
 
     GrAssert(result->isRect() == isIntersectionOfRects);
 
diff --git a/tests/ClipCacheTest.cpp b/tests/ClipCacheTest.cpp
index e3ff24551f..516a6141c8 100644
--- a/tests/ClipCacheTest.cpp
+++ b/tests/ClipCacheTest.cpp
@@ -83,7 +83,7 @@ static void test_clip_bounds(skiatest::Reporter* reporter, GrContext* context) {
     iter.reset(stack);
 
     GrClip clip;
-    clip.setFromIterator(&iter, 0, 0, stackBounds);
+    clip.setFromIterator(&iter, stackBounds);
 
     const GrRect& grBound = clip.getConservativeBounds();