Always have GrLayer's rect be valid

This CL just makes atlasing easier/clearer since there is a separate atlased query method. Not using the rect as a signal also simplifies the rendering of the layer in SkGpuDevice.cpp.

This is calved off from (Add atlased layer purging - https://codereview.chromium.org/367073002/)

R=bsalomon@google.com

Author: robertphillips@google.com

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

4 files changed, 119 insertions, 48 deletions

diff --git a/src/gpu/GrLayerCache.cpp b/src/gpu/GrLayerCache.cpp
index 62c97208cc..9688cac650 100644
--- a/src/gpu/GrLayerCache.cpp
+++ b/src/gpu/GrLayerCache.cpp
@@ -10,7 +10,7 @@
 #include "GrLayerCache.h"
 
 /**
- *  PictureLayerKey just wraps a saveLayer's id in the picture for GrTHashTable.
+ *  PictureLayerKey just wraps a saveLayer's id in a picture for GrTHashTable.
  */
 class GrLayerCache::PictureLayerKey {
 public:
@@ -41,6 +41,44 @@ private:
     int      fLayerID;
 };
 
+#ifdef SK_DEBUG
+void GrCachedLayer::validate(GrTexture* backingTexture) const {
+    SkASSERT(SK_InvalidGenID != fPictureID);
+    SkASSERT(-1 != fLayerID);
+
+    if (NULL != fTexture) {
+        // If the layer is in some texture then it must occupy some rectangle
+        SkASSERT(!fRect.isEmpty());
+        if (!this->isAtlased()) {
+            // If it isn't atlased then the rectangle should start at the origin
+            SkASSERT(0.0f == fRect.fLeft && 0.0f == fRect.fTop);
+        }
+    } else {
+        SkASSERT(fRect.isEmpty());
+    }
+}
+
+class GrAutoValidateLayer : ::SkNoncopyable {
+public:
+    GrAutoValidateLayer(GrTexture* backingTexture, const GrCachedLayer* layer) 
+        : fBackingTexture(backingTexture)
+        , fLayer(layer) {
+        if (NULL != fLayer) {
+            fLayer->validate(backingTexture);
+        }
+    }
+    ~GrAutoValidateLayer() {
+        if (NULL != fLayer) {
+            fLayer->validate(fBackingTexture);
+        }
+    }
+
+private:
+    GrTexture* fBackingTexture;
+    const GrCachedLayer* fLayer;
+};
+#endif
+
 GrLayerCache::GrLayerCache(GrContext* context)
     : fContext(context) {
     this->initAtlas();
@@ -112,11 +150,12 @@ GrCachedLayer* GrLayerCache::findLayerOrCreate(const SkPicture* picture, int lay
 }
 
 bool GrLayerCache::lock(GrCachedLayer* layer, const GrTextureDesc& desc) {
+    SkDEBUGCODE(GrAutoValidateLayer avl(fAtlas->getTexture(), layer);)
 
     if (NULL != layer->texture()) {
         // This layer is already locked
 #ifdef SK_DEBUG
-        if (!layer->rect().isEmpty()) {
+        if (layer->isAtlased()) {
             // It claims to be atlased
             SkASSERT(layer->rect().width() == desc.fWidth);
             SkASSERT(layer->rect().height() == desc.fHeight);
@@ -132,32 +171,61 @@ bool GrLayerCache::lock(GrCachedLayer* layer, const GrTextureDesc& desc) {
         GrIRect16 bounds = GrIRect16::MakeXYWH(loc.fX, loc.fY, 
                                                SkToS16(desc.fWidth), SkToS16(desc.fHeight));
         layer->setTexture(fAtlas->getTexture(), bounds);
+        layer->setAtlased(true);
         return false;
     }
 #endif
 
+    // The texture wouldn't fit in the cache - give it it's own texture.
     // This path always uses a new scratch texture and (thus) doesn't cache anything.
     // This can yield a lot of re-rendering
     layer->setTexture(fContext->lockAndRefScratchTexture(desc, GrContext::kApprox_ScratchTexMatch),
-                      GrIRect16::MakeEmpty());
+                      GrIRect16::MakeWH(SkToS16(desc.fWidth), SkToS16(desc.fHeight)));
     return false;
 }
 
 void GrLayerCache::unlock(GrCachedLayer* layer) {
+    SkDEBUGCODE(GrAutoValidateLayer avl(fAtlas->getTexture(), layer);)
+
     if (NULL == layer || NULL == layer->texture()) {
         return;
     }
 
-    // The atlas doesn't currently use a scratch texture (and we would have
-    // to free up space differently anyways)
-    // TODO: unlock atlas space when a recycling rectanizer is available
-    if (layer->texture() != fAtlas->getTexture()) {
+    if (layer->isAtlased()) {
+        // The atlas doesn't currently use a scratch texture (and we would have
+        // to free up space differently anyways)
+        // TODO: unlock atlas space when a recycling rectanizer is available
+    } else {
         fContext->unlockScratchTexture(layer->texture());
         layer->setTexture(NULL, GrIRect16::MakeEmpty());
     }
 }
 
+#ifdef SK_DEBUG
+void GrLayerCache::validate() const {
+    const SkTDArray<GrCachedLayer*>& layerArray = fLayerHash.getArray();
+    for (int i = 0; i < fLayerHash.count(); ++i) {
+        layerArray[i]->validate(fAtlas->getTexture());
+    }
+}
+
+class GrAutoValidateCache : ::SkNoncopyable {
+public:
+    explicit GrAutoValidateCache(GrLayerCache* cache)
+        : fCache(cache) {
+        fCache->validate();
+    }
+    ~GrAutoValidateCache() {
+        fCache->validate();
+    }
+private:
+    GrLayerCache* fCache;
+};
+#endif
+
 void GrLayerCache::purge(const SkPicture* picture) {
+    SkDEBUGCODE(GrAutoValidateCache avc(this);)
+
     // This is somewhat of an abuse of GrTHashTable. We need to find all the
     // layers associated with 'picture' but the usual hash calls only look for
     // exact key matches. This code peeks into the hash table's innards to
diff --git a/src/gpu/GrLayerCache.h b/src/gpu/GrLayerCache.h
index 27479193aa..566a738279 100644
--- a/src/gpu/GrLayerCache.h
+++ b/src/gpu/GrLayerCache.h
@@ -21,16 +21,15 @@ class SkPicture;
 
 // GrCachedLayer encapsulates the caching information for a single saveLayer.
 //
-// Atlased layers get a ref to their atlas GrTexture and 'fRect' contains
-// their absolute location in the backing texture.
-//
-// Non-atlased layers get a ref to the GrTexture in which they reside. Their
-// 'fRect' will be empty.
+// Atlased layers get a ref to the backing GrTexture while non-atlased layers
+// get a ref to the GrTexture in which they reside. In both cases 'fRect' 
+// contains the layer's extent in its texture.
 //
 // TODO: can we easily reuse the empty space in the non-atlased GrTexture's?
 struct GrCachedLayer {
 public:
-    GrCachedLayer(uint32_t pictureID, int layerID) {
+    GrCachedLayer(uint32_t pictureID, int layerID) 
+        : fAtlased(false) {
         fPictureID = pictureID;
         fLayerID = layerID;
         fTexture = NULL;
@@ -52,8 +51,15 @@ public:
     GrTexture* texture() { return fTexture; }
     const GrIRect16& rect() const { return fRect; }
 
+    void setAtlased(bool atlased) { fAtlased = atlased; }
+    bool isAtlased() const { return fAtlased; }
+
+    SkDEBUGCODE(void validate(GrTexture* backingTexture) const;)
+
 private:
+    // ID of the picture of which this layer is a part
     uint32_t        fPictureID;
+
     // fLayerID is only valid when fPicture != kInvalidGenID in which case it
     // is the index of this layer in the picture (one of 0 .. #layers).
     int             fLayerID;
@@ -63,8 +69,12 @@ private:
     // non-NULL, that means that the texture is locked in the texture cache.
     GrTexture*      fTexture;
 
-    // For non-atlased layers 'fRect' is empty otherwise it is the bound of
-    // the layer in the atlas.
+    // True if this layer is in an atlas; false otherwise.
+    bool            fAtlased;
+
+    // For both atlased and non-atlased layers 'fRect' contains the  bound of
+    // the layer in whichever texture it resides. It is empty when 'fTexture'
+    // is NULL.
     GrIRect16       fRect;
 };
 
@@ -99,6 +109,8 @@ public:
     // Remove all the layers (and unlock any resources) associated with 'picture'
     void purge(const SkPicture* picture);
 
+    SkDEBUGCODE(void validate() const;)
+
 private:
     GrContext*                fContext;  // pointer back to owning context
     SkAutoTDelete<GrAtlas>    fAtlas;    // TODO: could lazily allocate
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index 9d6ff8b26b..b06046a557 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -1991,21 +1991,18 @@ bool SkGpuDevice::EXPERIMENTAL_drawPicture(SkCanvas* canvas, const SkPicture* pi
 
             layerInfo->fBM = SkNEW(SkBitmap);  // fBM is allocated so ReplacementInfo can be POD
             wrap_texture(layer->texture(), 
-                         layer->rect().isEmpty() ? desc.fWidth : layer->texture()->width(),
-                         layer->rect().isEmpty() ? desc.fHeight : layer->texture()->height(),
+                         !layer->isAtlased() ? desc.fWidth : layer->texture()->width(),
+                         !layer->isAtlased() ? desc.fHeight : layer->texture()->height(),
                          layerInfo->fBM);
 
             SkASSERT(info.fPaint);
             layerInfo->fPaint = info.fPaint;
 
-            if (layer->rect().isEmpty()) {
-                layerInfo->fSrcRect = SkIRect::MakeWH(desc.fWidth, desc.fHeight);
-            } else {
-                layerInfo->fSrcRect = SkIRect::MakeXYWH(layer->rect().fLeft,
-                                                        layer->rect().fTop,
-                                                        layer->rect().width(),
-                                                        layer->rect().height());
-            }
+            layerInfo->fSrcRect = SkIRect::MakeXYWH(layer->rect().fLeft,
+                                                    layer->rect().fTop,
+                                                    layer->rect().width(),
+                                                    layer->rect().height());
+
 
             if (needsRendering) {
                 SkAutoTUnref<SkSurface> surface(SkSurface::NewRenderTargetDirect(
@@ -2015,14 +2012,12 @@ bool SkGpuDevice::EXPERIMENTAL_drawPicture(SkCanvas* canvas, const SkPicture* pi
 
                 SkCanvas* canvas = surface->getCanvas();
 
-                if (!layer->rect().isEmpty()) {
-                    // Add a rect clip to make sure the rendering doesn't
-                    // extend beyond the boundaries of the atlased sub-rect
-                    SkRect bound = SkRect::MakeXYWH(SkIntToScalar(layer->rect().fLeft),
-                                                    SkIntToScalar(layer->rect().fTop),
-                                                    SkIntToScalar(layer->rect().width()),
-                                                    SkIntToScalar(layer->rect().height()));
-                    canvas->clipRect(bound);
+                // Add a rect clip to make sure the rendering doesn't
+                // extend beyond the boundaries of the atlased sub-rect
+                SkRect bound = SkRect::Make(layerInfo->fSrcRect);
+                canvas->clipRect(bound);
+
+                if (layer->isAtlased()) {
                     // Since 'clear' doesn't respect the clip we need to draw a rect
                     // TODO: ensure none of the atlased layers contain a clear call!
                     SkPaint paint;
@@ -2032,16 +2027,12 @@ bool SkGpuDevice::EXPERIMENTAL_drawPicture(SkCanvas* canvas, const SkPicture* pi
                     canvas->clear(SK_ColorTRANSPARENT);
                 }
 
-                canvas->setMatrix(info.fCTM);
-
-                if (!layer->rect().isEmpty()) {
-                    // info.fCTM maps the layer's top/left to the origin.
-                    // Since this layer is atlased the top/left corner needs
-                    // to be offset to some arbitrary location in the backing 
-                    // texture.
-                    canvas->translate(SkIntToScalar(layer->rect().fLeft),
-                                      SkIntToScalar(layer->rect().fTop));
-                } 
+                // info.fCTM maps the layer's top/left to the origin.
+                // If this layer is atlased the top/left corner needs
+                // to be offset to some arbitrary location in the backing 
+                // texture.
+                canvas->translate(bound.fLeft, bound.fTop);
+                canvas->concat(info.fCTM);
 
                 SkPictureRangePlayback rangePlayback(picture,
                                                      info.fSaveLayerOpID, 
diff --git a/tests/GpuLayerCacheTest.cpp b/tests/GpuLayerCacheTest.cpp
index 02917e475f..d5c2dde4ad 100644
--- a/tests/GpuLayerCacheTest.cpp
+++ b/tests/GpuLayerCacheTest.cpp
@@ -39,7 +39,7 @@ static void create_layers(skiatest::Reporter* reporter,
         REPORTER_ASSERT(reporter, picture.uniqueID() == layers[i]->pictureID());
         REPORTER_ASSERT(reporter, layers[i]->layerID() == i);
         REPORTER_ASSERT(reporter, NULL == layers[i]->texture());
-        REPORTER_ASSERT(reporter, layers[i]->rect().isEmpty());
+        REPORTER_ASSERT(reporter, !layers[i]->isAtlased());
     }
 
 }
@@ -83,10 +83,10 @@ DEF_GPUTEST(GpuLayerCache, reporter, factory) {
         // The first 4 layers should be in the atlas (and thus have non-empty
         // rects)
         if (i < 4) {
-            REPORTER_ASSERT(reporter, !layer->rect().isEmpty());
+            REPORTER_ASSERT(reporter, layer->isAtlased());
         } else {
 #endif
-            REPORTER_ASSERT(reporter, layer->rect().isEmpty());
+            REPORTER_ASSERT(reporter, !layer->isAtlased());
 #if USE_ATLAS
         }
 #endif
@@ -109,11 +109,11 @@ DEF_GPUTEST(GpuLayerCache, reporter, factory) {
         // currently unlock). The final layer should be unlocked.
         if (i < 4) {
             REPORTER_ASSERT(reporter, NULL != layer->texture());
-            REPORTER_ASSERT(reporter, !layer->rect().isEmpty());
+            REPORTER_ASSERT(reporter, layer->isAtlased());
         } else {
 #endif
             REPORTER_ASSERT(reporter, NULL == layer->texture());
-            REPORTER_ASSERT(reporter, layer->rect().isEmpty());
+            REPORTER_ASSERT(reporter, !layer->isAtlased());
 #if USE_ATLAS
         }
 #endif