commit 01d6e5f462d1d52203ee1a6660415877e4cf2dde
author robertphillips <robertphillips@google.com> Mon Dec 01 09:09:27 2014 -0800
committer Commit bot <commit-bot@chromium.org> Mon Dec 01 09:09:27 2014 -0800
tree ddbfc5b976c891adf2e318008043a76e9c4a7f8d
parent e05162d15741eb642080a31f28c38a99b3142dfb

Use variable length key (rather than accumulated matrix) as save layer hoisting key

Adding the rendering canvas' CTM to the layer hoisting key (i.e., Add support for hoisting layers in pictures drawn with a matrix - https://codereview.chromium.org/748853002/) has increased the cache miss rate due to accumulated floating point error. This CL fixes part of the issue by using the chain of operation indices leading to each saveLayer as the key. The canvas' CTM must still form part of the key but should be less subject to accumulated error.

BUG=skia:2315

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

src/core/SkLayerInfo.h

diff --git a/src/core/SkLayerInfo.h b/src/core/SkLayerInfo.h
index dd0eaf0..6ed77c9 100644
--- a/src/core/SkLayerInfo.h
+++ b/src/core/SkLayerInfo.h
@@ -18,8 +18,8 @@
     // Information about a given saveLayer/restore block in an SkPicture
     class BlockInfo {
     public:
-        BlockInfo() : fPicture(NULL), fPaint(NULL) {}
-        ~BlockInfo() { SkSafeUnref(fPicture); SkDELETE(fPaint); }
+        BlockInfo() : fPicture(NULL), fPaint(NULL), fKey(NULL), fKeySize(0) {}
+        ~BlockInfo() { SkSafeUnref(fPicture); SkDELETE(fPaint); SkDELETE_ARRAY(fKey); }
 
         // The picture owning the layer. If the owning picture is the top-most
         // one (i.e., the picture for which this SkLayerInfo was created) then
@@ -50,6 +50,11 @@
         bool    fHasNestedLayers;
         // True if this saveLayer is nested within another. False otherwise.
         bool    fIsNested;
+        // The variable length key for this saveLayer block. It stores the
+        // thread of drawPicture and saveLayer operation indices that lead to this
+        // saveLayer (including its own op index). The BlockInfo owns this memory.
+        int*    fKey;
+        int     fKeySize;  // # of ints
     };
 
     SkLayerInfo(Key key) : INHERITED(key) { }

src/core/SkMultiPictureDraw.cpp

diff --git a/src/core/SkMultiPictureDraw.cpp b/src/core/SkMultiPictureDraw.cpp
index 51feae5..bd1e8fe 100644
--- a/src/core/SkMultiPictureDraw.cpp
+++ b/src/core/SkMultiPictureDraw.cpp
@@ -170,8 +170,8 @@
 
             GrReplacements replacements;
 
-            GrLayerHoister::ConvertLayersToReplacements(needRendering, &replacements);
-            GrLayerHoister::ConvertLayersToReplacements(recycled, &replacements);
+            GrLayerHoister::ConvertLayersToReplacements(picture, needRendering, &replacements);
+            GrLayerHoister::ConvertLayersToReplacements(picture, recycled, &replacements);
 
             // Render the entire picture using new layers
             GrRecordReplaceDraw(picture, canvas, &replacements, initialMatrix, NULL);

src/core/SkRecordDraw.cpp

diff --git a/src/core/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp
index 5e1fe7f..5407480 100644
--- a/src/core/SkRecordDraw.cpp
+++ b/src/core/SkRecordDraw.cpp
@@ -692,6 +692,13 @@
             dst.fRestoreOpID = src.fRestoreOpID;
             dst.fHasNestedLayers = src.fHasNestedLayers;
             dst.fIsNested = fSaveLayersInStack > 0 || src.fIsNested;
+
+            // Store 'saveLayer ops from enclosing picture' + drawPict op + 'ops from sub-picture'
+            dst.fKeySize = fSaveLayerOpStack.count() + src.fKeySize + 1;
+            dst.fKey = SkNEW_ARRAY(int, dst.fKeySize);
+            memcpy(dst.fKey, fSaveLayerOpStack.begin(), fSaveLayerOpStack.count() * sizeof(int));
+            dst.fKey[fSaveLayerOpStack.count()] = fFillBounds.currentOp();
+            memcpy(&dst.fKey[fSaveLayerOpStack.count()+1], src.fKey, src.fKeySize * sizeof(int));
         }
     }
 
@@ -724,6 +731,7 @@
         if (isSaveLayer) {
             this->updateStackForSaveLayer();
             ++fSaveLayersInStack;
+            fSaveLayerOpStack.push(fFillBounds.currentOp());
         }
 
         fSaveLayerStack.push(SaveLayerInfo(fFillBounds.currentOp(), isSaveLayer, paint));
@@ -735,6 +743,8 @@
             return;
         }
 
+        SkASSERT(fSaveLayersInStack == fSaveLayerOpStack.count());
+
         SaveLayerInfo sli;
         fSaveLayerStack.pop(&sli);
 
@@ -758,12 +768,20 @@
         block.fRestoreOpID = fFillBounds.currentOp();
         block.fHasNestedLayers = sli.fHasNestedSaveLayer;
         block.fIsNested = fSaveLayersInStack > 0;
+
+        block.fKeySize = fSaveLayerOpStack.count();
+        block.fKey = SkNEW_ARRAY(int, block.fKeySize);
+        memcpy(block.fKey, fSaveLayerOpStack.begin(), block.fKeySize * sizeof(int));
+
+        fSaveLayerOpStack.pop();
     }
 
     // Used to collect saveLayer information for layer hoisting
-    int                   fSaveLayersInStack;
+    int                      fSaveLayersInStack;
     SkTDArray<SaveLayerInfo> fSaveLayerStack;
-    SkLayerInfo*          fAccelData;
+    // The op code indices of all the currently active saveLayers
+    SkTDArray<int>           fSaveLayerOpStack;
+    SkLayerInfo*             fAccelData;
     const SkPicture::SnapshotArray* fPictList;
 
     SkRecords::FillBounds fFillBounds;