Move offset and CTM from LayerCache Key to per-hoisted-layer info

This CL reduces the amount of information used in the layer cache key:
- the stop value isn't needed since the start value uniquely identifies the layer in the picture.
- only the upper-left 2x2 portion of the CTM should be used as a key for looking up cached layers.
- individual layers can be redraw in different locations so the final offset cannot be a part of the key.

Since this data is no longer stored in the cached layer, but is still required to draw the cached layer, it is now stored in the per-layer information (i.e., HoistedLayer).

This is split out of (Fix sub-picture layer rendering bugs - https://codereview.chromium.org/597293002/).

BUG=skia:2315
R=egdaniel@google.com

Author: robertphillips@google.com

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

5 files changed, 65 insertions, 77 deletions

diff --git a/src/gpu/GrLayerCache.cpp b/src/gpu/GrLayerCache.cpp
index 11a97e4c66..6ec07580c8 100644
--- a/src/gpu/GrLayerCache.cpp
+++ b/src/gpu/GrLayerCache.cpp
@@ -14,7 +14,7 @@ DECLARE_SKMESSAGEBUS_MESSAGE(GrPictureDeletedMessage);
 #ifdef SK_DEBUG
 void GrCachedLayer::validate(const GrTexture* backingTexture) const {
     SkASSERT(SK_InvalidGenID != fKey.pictureID());
-    SkASSERT(fKey.start() > 0 && fKey.stop() > 0);
+    SkASSERT(fKey.start() > 0);
 
 
     if (fTexture) {
@@ -119,33 +119,30 @@ void GrLayerCache::freeAll() {
 
 GrCachedLayer* GrLayerCache::createLayer(uint32_t pictureID, 
                                          int start, int stop, 
-                                         const SkIPoint& offset,
                                          const SkMatrix& ctm,
                                          const SkPaint* paint) {
     SkASSERT(pictureID != SK_InvalidGenID && start > 0 && stop > 0);
 
-    GrCachedLayer* layer = SkNEW_ARGS(GrCachedLayer, (pictureID, start, stop, offset, ctm, paint));
+    GrCachedLayer* layer = SkNEW_ARGS(GrCachedLayer, (pictureID, start, stop, ctm, paint));
     fLayerHash.add(layer);
     return layer;
 }
 
 GrCachedLayer* GrLayerCache::findLayer(uint32_t pictureID,
-                                       int start, int stop, 
-                                       const SkIPoint& offset,
+                                       int start, 
                                        const SkMatrix& ctm) {
-    SkASSERT(pictureID != SK_InvalidGenID && start > 0 && stop > 0);
-    return fLayerHash.find(GrCachedLayer::Key(pictureID, start, stop, offset, ctm));
+    SkASSERT(pictureID != SK_InvalidGenID && start > 0);
+    return fLayerHash.find(GrCachedLayer::Key(pictureID, start, ctm));
 }
 
 GrCachedLayer* GrLayerCache::findLayerOrCreate(uint32_t pictureID,
                                                int start, int stop,
-                                               const SkIPoint& offset,
                                                const SkMatrix& ctm,
                                                const SkPaint* paint) {
     SkASSERT(pictureID != SK_InvalidGenID && start > 0 && stop > 0);
-    GrCachedLayer* layer = fLayerHash.find(GrCachedLayer::Key(pictureID, start, stop, offset, ctm));
+    GrCachedLayer* layer = fLayerHash.find(GrCachedLayer::Key(pictureID, start, ctm));
     if (NULL == layer) {
-        layer = this->createLayer(pictureID, start, stop, offset, ctm, paint);
+        layer = this->createLayer(pictureID, start, stop, ctm, paint);
     }
 
     return layer;
diff --git a/src/gpu/GrLayerCache.h b/src/gpu/GrLayerCache.h
index 38d4d9b500..790b82a8f3 100644
--- a/src/gpu/GrLayerCache.h
+++ b/src/gpu/GrLayerCache.h
@@ -51,44 +51,36 @@ struct GrCachedLayer {
 public:
     // For SkTDynamicHash
     struct Key {
-        Key(uint32_t pictureID, int start, int stop, const SkIPoint& offset, const SkMatrix& ctm) 
+        // TODO: the key needs to include the clip
+        Key(uint32_t pictureID, int start, const SkMatrix& ctm) 
         : fPictureID(pictureID)
-        , fStart(start)
-        , fStop(stop)
-        , fOffset(offset)
-        , fCTM(ctm) {
-            fCTM.getType(); // force initialization of type so hashes match
-
+        , fStart(start) {
+            fCTM[0] = ctm.getScaleX();
+            fCTM[1] = ctm.getSkewX();
+            fCTM[2] = ctm.getSkewY();
+            fCTM[3] = ctm.getScaleY();
             // Key needs to be tightly packed.
-            GR_STATIC_ASSERT(sizeof(Key) == sizeof(uint32_t) + 2 * sizeof(int) + 
-                                            2 * sizeof(int32_t) +
-                                            9 * sizeof(SkScalar) + sizeof(uint32_t));
+            GR_STATIC_ASSERT(sizeof(Key) == sizeof(uint32_t) +      // picture ID
+                                            sizeof(int) +           // start index
+                                            4 * sizeof(SkScalar));  // 2x2 from CTM
         }
 
         bool operator==(const Key& other) const {
             return fPictureID == other.fPictureID &&
                    fStart == other.fStart &&
-                   fStop == other.fStop &&
-                   fOffset == other.fOffset &&
-                   fCTM.cheapEqualTo(other.fCTM);
+                   0 == memcmp(fCTM, other.fCTM, sizeof(fCTM));
         }
 
         uint32_t pictureID() const { return fPictureID; }
         int start() const { return fStart; }
-        int stop() const { return fStop; }
-        const SkIPoint& offset() const { return fOffset; }
-        const SkMatrix& ctm() const { return fCTM; }
 
     private:
         // ID of the picture of which this layer is a part
         const uint32_t fPictureID;
-        // The range of commands in the picture this layer represents
+        // The the index of the saveLayer command in the picture
         const int      fStart;
-        const int      fStop;
-        // The offset of the layer in device space
-        const SkIPoint fOffset;
-        // The CTM applied to this layer in the picture
-        SkMatrix       fCTM;
+        // The 2x2 portion of the CTM applied to this layer in the picture
+        SkScalar       fCTM[4];
     };
 
     static const Key& GetKey(const GrCachedLayer& layer) { return layer.fKey; }
@@ -98,9 +90,9 @@ public:
 
     // GrCachedLayer proper
     GrCachedLayer(uint32_t pictureID, int start, int stop,
-                  const SkIPoint& offset, const SkMatrix& ctm,
-                  const SkPaint* paint)
-        : fKey(pictureID, start, stop, offset, ctm)
+                  const SkMatrix& ctm, const SkPaint* paint)
+        : fKey(pictureID, start, ctm)
+        , fStop(stop)
         , fPaint(paint ? SkNEW_ARGS(SkPaint, (*paint)) : NULL)
         , fTexture(NULL)
         , fRect(GrIRect16::MakeEmpty())
@@ -116,10 +108,8 @@ public:
 
     uint32_t pictureID() const { return fKey.pictureID(); }
     int start() const { return fKey.start(); }
-    int stop() const { return fKey.stop(); }
-    const SkIPoint& offset() const { return fKey.offset(); }
-    const SkMatrix& ctm() const { return fKey.ctm(); }
 
+    int stop() const { return fStop; }
     void setTexture(GrTexture* texture, const GrIRect16& rect) {
         SkRefCnt_SafeAssign(fTexture, texture);
         fRect = rect;
@@ -145,6 +135,9 @@ public:
 private:
     const Key       fKey;
 
+    // The final "restore" operation index of the cached layer
+    const int       fStop;
+
     // The paint used when dropping the layer down into the owning canvas.
     // Can be NULL. This class makes a copy for itself.
     const SkPaint*  fPaint;
@@ -187,11 +180,9 @@ public:
     // elements by the GrContext
     void freeAll();
 
-    GrCachedLayer* findLayer(uint32_t pictureID, int start, int stop, 
-                             const SkIPoint& offset, const SkMatrix& ctm);
+    GrCachedLayer* findLayer(uint32_t pictureID, int start, const SkMatrix& ctm);
     GrCachedLayer* findLayerOrCreate(uint32_t pictureID,
                                      int start, int stop, 
-                                     const SkIPoint& offset,
                                      const SkMatrix& ctm,
                                      const SkPaint* paint);
 
@@ -247,8 +238,7 @@ private:
 
     void initAtlas();
     GrCachedLayer* createLayer(uint32_t pictureID, int start, int stop, 
-                               const SkIPoint& offset, const SkMatrix& ctm,
-                               const SkPaint* paint);
+                               const SkMatrix& ctm, const SkPaint* paint);
 
     void purgeAll();
 
diff --git a/src/gpu/GrLayerHoister.cpp b/src/gpu/GrLayerHoister.cpp
index 91d439b304..f2e6560a18 100644
--- a/src/gpu/GrLayerHoister.cpp
+++ b/src/gpu/GrLayerHoister.cpp
@@ -56,6 +56,7 @@ bool GrLayerHoister::FindLayersToHoist(const SkPicture* topLevelPicture,
             continue;
         }
 
+        // TODO: ignore perspective projected layers here!
         // TODO: once this code is more stable unsuitable layers can
         // just be omitted during the optimization stage
         if (!info.fValid ||
@@ -84,7 +85,6 @@ bool GrLayerHoister::FindLayersToHoist(const SkPicture* topLevelPicture,
             GrCachedLayer* layer = layerCache->findLayerOrCreate(pict->uniqueID(),
                                                                  info.fSaveLayerOpID,
                                                                  info.fRestoreOpID,
-                                                                 info.fOffset,
                                                                  info.fOriginXform,
                                                                  info.fPaint);
 
@@ -102,16 +102,18 @@ bool GrLayerHoister::FindLayersToHoist(const SkPicture* topLevelPicture,
             }
 
             if (needsRendering) {
-                HoistedLayer* info;
+                HoistedLayer* hl;
 
                 if (layer->isAtlased()) {
-                    info = atlased->append();
+                    hl = atlased->append();
                 } else {
-                    info = nonAtlased->append();
+                    hl = nonAtlased->append();
                 }
 
-                info->fLayer = layer;
-                info->fPicture = pict;
+                hl->fLayer = layer;
+                hl->fPicture = pict;
+                hl->fOffset = info.fOffset;
+                hl->fCTM = info.fOriginXform;
             }
         }
     }
@@ -129,10 +131,12 @@ static void convert_layers_to_replacements(const SkTDArray<GrLayerHoister::Hoist
                                            GrReplacements* replacements) {
     // TODO: just replace GrReplacements::ReplacementInfo with GrCachedLayer?
     for (int i = 0; i < layers.count(); ++i) {
+        GrCachedLayer* layer = layers[i].fLayer;
+
         GrReplacements::ReplacementInfo* layerInfo = replacements->push();
-        layerInfo->fStart = layers[i].fLayer->start();
-        layerInfo->fStop = layers[i].fLayer->stop();
-        layerInfo->fPos = layers[i].fLayer->offset();;
+        layerInfo->fStart = layer->start();
+        layerInfo->fStop = layer->stop();
+        layerInfo->fPos = layers[i].fOffset;
 
         SkBitmap bm;
         wrap_texture(layers[i].fLayer->texture(),
@@ -172,6 +176,7 @@ void GrLayerHoister::DrawLayers(const SkTDArray<HoistedLayer>& atlased,
         for (int i = 0; i < atlased.count(); ++i) {
             GrCachedLayer* layer = atlased[i].fLayer;
             const SkPicture* pict = atlased[i].fPicture;
+            const SkIPoint offset = atlased[i].fOffset;
 
             atlasCanvas->save();
 
@@ -191,14 +196,14 @@ void GrLayerHoister::DrawLayers(const SkTDArray<HoistedLayer>& atlased,
             // Since this layer is atlased, the top/left corner needs
             // to be offset to the correct location in the backing texture.
             SkMatrix initialCTM;
-            initialCTM.setTranslate(SkIntToScalar(-layer->offset().fX), 
-                                    SkIntToScalar(-layer->offset().fY));
+            initialCTM.setTranslate(SkIntToScalar(-offset.fX), 
+                                    SkIntToScalar(-offset.fY));
             initialCTM.postTranslate(bound.fLeft, bound.fTop);
             
-            atlasCanvas->translate(SkIntToScalar(-layer->offset().fX), 
-                                   SkIntToScalar(-layer->offset().fY));
+            atlasCanvas->translate(SkIntToScalar(-offset.fX), 
+                                   SkIntToScalar(-offset.fY));
             atlasCanvas->translate(bound.fLeft, bound.fTop);
-            atlasCanvas->concat(layer->ctm());
+            atlasCanvas->concat(atlased[i].fCTM);
 
             SkRecordPartialDraw(*pict->fRecord.get(), atlasCanvas, bound,
                                 layer->start()+1, layer->stop(), initialCTM);
@@ -213,6 +218,7 @@ void GrLayerHoister::DrawLayers(const SkTDArray<HoistedLayer>& atlased,
     for (int i = 0; i < nonAtlased.count(); ++i) {
         GrCachedLayer* layer = nonAtlased[i].fLayer;
         const SkPicture* pict = nonAtlased[i].fPicture;
+        const SkIPoint offset = nonAtlased[i].fOffset;
 
         // Each non-atlased layer has its own GrTexture
         SkAutoTUnref<SkSurface> surface(SkSurface::NewRenderTargetDirect(
@@ -232,12 +238,12 @@ void GrLayerHoister::DrawLayers(const SkTDArray<HoistedLayer>& atlased,
         layerCanvas->clear(SK_ColorTRANSPARENT);
 
         SkMatrix initialCTM;
-        initialCTM.setTranslate(SkIntToScalar(-layer->offset().fX), 
-                                SkIntToScalar(-layer->offset().fY));
+        initialCTM.setTranslate(SkIntToScalar(-offset.fX), 
+                                SkIntToScalar(-offset.fY));
 
-        layerCanvas->translate(SkIntToScalar(-layer->offset().fX), 
-                               SkIntToScalar(-layer->offset().fY));
-        layerCanvas->concat(layer->ctm());
+        layerCanvas->translate(SkIntToScalar(-offset.fX), 
+                               SkIntToScalar(-offset.fY));
+        layerCanvas->concat(nonAtlased[i].fCTM);
 
         SkRecordPartialDraw(*pict->fRecord.get(), layerCanvas, bound,
                             layer->start()+1, layer->stop(), initialCTM);
@@ -262,7 +268,7 @@ static void unlock_layer_in_cache(GrLayerCache* layerCache,
 #endif
 }
 
-void GrLayerHoister::UnlockLayers(GrLayerCache* layerCache, 
+void GrLayerHoister::UnlockLayers(GrLayerCache* layerCache,
                                   const SkTDArray<HoistedLayer>& atlased,
                                   const SkTDArray<HoistedLayer>& nonAtlased) {
 
diff --git a/src/gpu/GrLayerHoister.h b/src/gpu/GrLayerHoister.h
index acf1901096..6aff2ea32a 100644
--- a/src/gpu/GrLayerHoister.h
+++ b/src/gpu/GrLayerHoister.h
@@ -26,6 +26,8 @@ public:
     struct HoistedLayer {
         const SkPicture* fPicture;
         GrCachedLayer*   fLayer;
+        SkIPoint         fOffset;
+        SkMatrix         fCTM;
     };
 
     /** Find the layers in 'topLevelPicture' that need hoisting. Note that the discovered
diff --git a/tests/GpuLayerCacheTest.cpp b/tests/GpuLayerCacheTest.cpp
index c9b88f5dfe..9e07056421 100644
--- a/tests/GpuLayerCacheTest.cpp
+++ b/tests/GpuLayerCacheTest.cpp
@@ -33,12 +33,11 @@ static void create_layers(skiatest::Reporter* reporter,
     for (int i = 0; i < numToAdd; ++i) {
         GrCachedLayer* layer = cache->findLayerOrCreate(picture.uniqueID(), 
                                                         idOffset+i+1, idOffset+i+2, 
-                                                        SkIPoint::Make(0, 0),
                                                         SkMatrix::I(),
                                                         NULL);
         REPORTER_ASSERT(reporter, layer);
-        GrCachedLayer* temp = cache->findLayer(picture.uniqueID(), idOffset+i+1, idOffset+i+2, 
-                                               SkIPoint::Make(0, 0), SkMatrix::I());
+        GrCachedLayer* temp = cache->findLayer(picture.uniqueID(), idOffset+i+1,
+                                               SkMatrix::I());
         REPORTER_ASSERT(reporter, temp == layer);
 
         REPORTER_ASSERT(reporter, TestingAccess::NumLayers(cache) == idOffset + i + 1);
@@ -46,7 +45,6 @@ static void create_layers(skiatest::Reporter* reporter,
         REPORTER_ASSERT(reporter, picture.uniqueID() == layer->pictureID());
         REPORTER_ASSERT(reporter, layer->start() == idOffset + i + 1);
         REPORTER_ASSERT(reporter, layer->stop() == idOffset + i + 2);
-        REPORTER_ASSERT(reporter, layer->ctm() == SkMatrix::I());
         REPORTER_ASSERT(reporter, NULL == layer->texture());
         REPORTER_ASSERT(reporter, NULL == layer->paint());
         REPORTER_ASSERT(reporter, !layer->isAtlased());
@@ -103,8 +101,7 @@ DEF_GPUTEST(GpuLayerCache, reporter, factory) {
         create_layers(reporter, &cache, *picture, kInitialNumLayers, 0);
 
         for (int i = 0; i < kInitialNumLayers; ++i) {
-            GrCachedLayer* layer = cache.findLayer(picture->uniqueID(), i+1, i+2, 
-                                                   SkIPoint::Make(0, 0), SkMatrix::I());
+            GrCachedLayer* layer = cache.findLayer(picture->uniqueID(), i+1, SkMatrix::I());
             REPORTER_ASSERT(reporter, layer);
 
             lock_layer(reporter, &cache, layer);
@@ -121,15 +118,13 @@ DEF_GPUTEST(GpuLayerCache, reporter, factory) {
 
         // Unlock the textures
         for (int i = 0; i < kInitialNumLayers; ++i) {
-            GrCachedLayer* layer = cache.findLayer(picture->uniqueID(), i+1, i+2, 
-                                                   SkIPoint::Make(0, 0), SkMatrix::I());
+            GrCachedLayer* layer = cache.findLayer(picture->uniqueID(), i+1, SkMatrix::I());
             REPORTER_ASSERT(reporter, layer);
             cache.unlock(layer);
         }
 
         for (int i = 0; i < kInitialNumLayers; ++i) {
-            GrCachedLayer* layer = cache.findLayer(picture->uniqueID(), i+1, i+2, 
-                                                   SkIPoint::Make(0, 0), SkMatrix::I());
+            GrCachedLayer* layer = cache.findLayer(picture->uniqueID(), i+1, SkMatrix::I());
             REPORTER_ASSERT(reporter, layer);
 
             REPORTER_ASSERT(reporter, !layer->locked());
@@ -149,8 +144,7 @@ DEF_GPUTEST(GpuLayerCache, reporter, factory) {
             // will force out the first atlased layer
             create_layers(reporter, &cache, *picture, 1, kInitialNumLayers);
             GrCachedLayer* layer = cache.findLayer(picture->uniqueID(), 
-                                                   kInitialNumLayers+1, kInitialNumLayers+2, 
-                                                   SkIPoint::Make(0, 0), SkMatrix::I());
+                                                   kInitialNumLayers+1, SkMatrix::I());
             REPORTER_ASSERT(reporter, layer);
 
             lock_layer(reporter, &cache, layer);
@@ -158,8 +152,7 @@ DEF_GPUTEST(GpuLayerCache, reporter, factory) {
         }
 
         for (int i = 0; i < kInitialNumLayers+1; ++i) {
-            GrCachedLayer* layer = cache.findLayer(picture->uniqueID(), i+1, i+2, 
-                                                   SkIPoint::Make(0, 0), SkMatrix::I());
+            GrCachedLayer* layer = cache.findLayer(picture->uniqueID(), i+1, SkMatrix::I());
             // 3 old layers plus the new one should be in the atlas.
             if (1 == i || 2 == i || 3 == i || 5 == i) {
                 REPORTER_ASSERT(reporter, layer);