Update GrRecordReplaceDraw to use SkTDynamicHash & add ReplaceDraw
authorrobertphillips <robertphillips@google.com>
Wed, 1 Oct 2014 16:24:06 +0000 (09:24 -0700)
committerCommit bot <commit-bot@chromium.org>
Wed, 1 Oct 2014 16:24:06 +0000 (09:24 -0700)
Having hoisted layers from different pictures invalidates the assumptions of the old GrReplacements object. This is fixed by switching to a SkTDynamicHash-based back-end.

Sub-picture-layers also require that the replacement drawing occur for the sub-pictures too. The ReplaceDraw object is added to make this happen and limit the replacement lookup to saveLayer draw commands.

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

BUG=skia:2315

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

include/core/SkPicture.h
src/gpu/GrLayerHoister.cpp
src/gpu/GrRecordReplaceDraw.cpp
src/gpu/GrRecordReplaceDraw.h
tests/RecordReplaceDrawTest.cpp

index 6e785d4..c23c5fa 100644 (file)
@@ -277,11 +277,7 @@ private:
     friend class GrLayerHoister;               // access to fRecord
     friend class CollectLayers;                // access to fRecord
     friend class SkPicturePlayback;            // to get fData
-    friend void GrRecordReplaceDraw(const SkPicture* picture,
-                                    SkCanvas* canvas,
-                                    const GrReplacements* replacements,
-                                    const SkMatrix& initialMatrix,
-                                    SkDrawPictureCallback* callback);
+    friend class ReplaceDraw;
 
     typedef SkRefCnt INHERITED;
 
index 8143bdc..2e42ead 100644 (file)
@@ -135,9 +135,12 @@ static void convert_layers_to_replacements(const SkTDArray<GrLayerHoister::Hoist
     // TODO: just replace GrReplacements::ReplacementInfo with GrCachedLayer?
     for (int i = 0; i < layers.count(); ++i) {
         GrCachedLayer* layer = layers[i].fLayer;
+        const SkPicture* picture = layers[i].fPicture;
 
-        GrReplacements::ReplacementInfo* layerInfo = replacements->push();
-        layerInfo->fStart = layer->start();
+        GrReplacements::ReplacementInfo* layerInfo =
+                    replacements->newReplacement(picture->uniqueID(),
+                                                 layer->start(),
+                                                 layers[i].fCTM);
         layerInfo->fStop = layer->stop();
         layerInfo->fPos = layers[i].fOffset;
 
index acdc63f..c8b2670 100644 (file)
@@ -6,49 +6,34 @@
  */
 
 #include "GrRecordReplaceDraw.h"
+#include "SkCanvasPriv.h"
 #include "SkImage.h"
 #include "SkRecordDraw.h"
-
-GrReplacements::ReplacementInfo* GrReplacements::push() {
-    SkDEBUGCODE(this->validate());
-    return fReplacements.push();
+#include "SkRecords.h"
+
+GrReplacements::ReplacementInfo* GrReplacements::newReplacement(uint32_t pictureID, 
+                                                                unsigned int start,
+                                                                const SkMatrix& ctm) {
+    ReplacementInfo* replacement = SkNEW_ARGS(ReplacementInfo, (pictureID, start, ctm));
+    fReplacementHash.add(replacement);
+    return replacement;
 }
 
 void GrReplacements::freeAll() {
-    for (int i = 0; i < fReplacements.count(); ++i) {
-        fReplacements[i].fImage->unref();
-        SkDELETE(fReplacements[i].fPaint);
-    }
-    fReplacements.reset();
-}
+    SkTDynamicHash<ReplacementInfo, ReplacementInfo::Key>::Iter iter(&fReplacementHash);
 
-#ifdef SK_DEBUG
-void GrReplacements::validate() const {
-    // Check that the ranges are monotonically increasing and non-overlapping
-    if (fReplacements.count() > 0) {
-        SkASSERT(fReplacements[0].fStart < fReplacements[0].fStop);
-
-        for (int i = 1; i < fReplacements.count(); ++i) {
-            SkASSERT(fReplacements[i].fStart < fReplacements[i].fStop);
-            SkASSERT(fReplacements[i - 1].fStop < fReplacements[i].fStart);
-        }
+    for (; !iter.done(); ++iter) {
+        ReplacementInfo* replacement = &(*iter);
+        SkDELETE(replacement);
     }
+
+    fReplacementHash.reset();
 }
-#endif
-
-const GrReplacements::ReplacementInfo*
-GrReplacements::lookupByStart(size_t start, int* searchStart) const {
-    SkDEBUGCODE(this->validate());
-    for (int i = *searchStart; i < fReplacements.count(); ++i) {
-        if (start == fReplacements[i].fStart) {
-            *searchStart = i + 1;
-            return &fReplacements[i];
-        } else if (start < fReplacements[i].fStart) {
-            return NULL;  // the ranges are monotonically increasing and non-overlapping
-        }
-    }
 
-    return NULL;
+const GrReplacements::ReplacementInfo* GrReplacements::lookupByStart(uint32_t pictureID, 
+                                                                     size_t start, 
+                                                                     const SkMatrix& ctm) const {
+    return fReplacementHash.find(ReplacementInfo::Key(pictureID, start, ctm));
 }
 
 static inline void draw_replacement_bitmap(const GrReplacements::ReplacementInfo* ri,
@@ -66,65 +51,130 @@ static inline void draw_replacement_bitmap(const GrReplacements::ReplacementInfo
     canvas->restore();
 }
 
-void GrRecordReplaceDraw(const SkPicture* picture,
-                         SkCanvas* canvas,
-                         const GrReplacements* replacements,
-                         const SkMatrix& initialMatrix,
-                         SkDrawPictureCallback* callback) {
-    SkAutoCanvasRestore saveRestore(canvas, true /*save now, restore at exit*/);
-
-    const SkBBoxHierarchy* bbh = picture->fBBH.get();
-    const SkRecord* record = picture->fRecord.get();
-    if (NULL == record) {
-        return;
+// Used by GrRecordReplaceDraw. It intercepts nested drawPicture calls and
+// also draws them with replaced layers.
+class ReplaceDraw : public SkRecords::Draw {
+public:
+    ReplaceDraw(SkCanvas* canvas,
+                const SkPicture* picture,
+                const GrReplacements* replacements,
+                const SkMatrix& initialMatrix,
+                SkDrawPictureCallback* callback)
+        : INHERITED(canvas)
+        , fCanvas(canvas)
+        , fPicture(picture)
+        , fReplacements(replacements)
+        , fInitialMatrix(initialMatrix)
+        , fCallback(callback)
+        , fIndex(0) {
     }
 
-    SkRecords::Draw draw(canvas);
-    const GrReplacements::ReplacementInfo* ri = NULL;
-    int searchStart = 0;
-
-    if (bbh) {
-        // Draw only ops that affect pixels in the canvas's current clip.
-        // The SkRecord and BBH were recorded in identity space.  This canvas
-        // is not necessarily in that same space.  getClipBounds() returns us
-        // this canvas' clip bounds transformed back into identity space, which
-        // lets us query the BBH.
-        SkRect query = { 0, 0, 0, 0 };
-        (void)canvas->getClipBounds(&query);
-
-        SkTDArray<void*> ops;
-        bbh->search(query, &ops);
-
-        for (int i = 0; i < ops.count(); i++) {
-            if (callback && callback->abortDrawing()) {
-                return;
+    void draw() {
+        const SkBBoxHierarchy* bbh = fPicture->fBBH.get();
+        const SkRecord* record = fPicture->fRecord.get();
+        if (NULL == record) {
+            return;
+        }
+
+        fOps.rewind();
+
+        if (bbh) {
+            // Draw only ops that affect pixels in the canvas's current clip.
+            // The SkRecord and BBH were recorded in identity space.  This canvas
+            // is not necessarily in that same space.  getClipBounds() returns us
+            // this canvas' clip bounds transformed back into identity space, which
+            // lets us query the BBH.
+            SkRect query = { 0, 0, 0, 0 };
+            (void)fCanvas->getClipBounds(&query);
+
+            bbh->search(query, &fOps);
+
+            for (fIndex = 0; fIndex < fOps.count(); ++fIndex) {
+                if (fCallback && fCallback->abortDrawing()) {
+                    return;
+                }
+
+                record->visit<void>((uintptr_t)fOps[fIndex], *this);
             }
-            ri = replacements->lookupByStart((uintptr_t)ops[i], &searchStart);
-            if (ri) {
-                draw_replacement_bitmap(ri, canvas, initialMatrix);
 
-                while ((uintptr_t)ops[i] < ri->fStop) {
-                    ++i;
+        } else {
+            for (fIndex = 0; fIndex < (int) record->count(); ++fIndex) {
+                if (fCallback && fCallback->abortDrawing()) {
+                    return;
                 }
-                SkASSERT((uintptr_t)ops[i] == ri->fStop);
-                continue;
+
+                record->visit<void>(fIndex, *this);
             }
+        }
+    }
+
+    // Same as Draw for all ops except DrawPicture and SaveLayer.
+    template <typename T> void operator()(const T& r) {
+        this->INHERITED::operator()(r);
+    }
+    void operator()(const SkRecords::DrawPicture& dp) {
+        SkAutoCanvasMatrixPaint acmp(fCanvas, dp.matrix, dp.paint, dp.picture->cullRect());
 
-            record->visit<void>((uintptr_t)ops[i], draw);
+        // Draw sub-pictures with the same replacement list but a different picture
+        ReplaceDraw draw(fCanvas, dp.picture, fReplacements, fInitialMatrix, fCallback);
+
+        draw.draw();
+    }
+    void operator()(const SkRecords::SaveLayer& sl) {
+        
+        // For a saveLayer command, check if it can be replaced by a drawBitmap
+        // call and, if so, draw it and then update the current op index accordingly.
+        size_t startOffset;
+        if (fOps.count()) {
+            startOffset = (uintptr_t)fOps[fIndex];
+        } else {
+            startOffset = fIndex;
         }
-    } else {
-        for (unsigned int i = 0; i < record->count(); ++i) {
-            if (callback && callback->abortDrawing()) {
-                return;
-            }
-            ri = replacements->lookupByStart(i, &searchStart);
-            if (ri) {
-                draw_replacement_bitmap(ri, canvas, initialMatrix);
-                i = ri->fStop;
-                continue;
-            }
 
-            record->visit<void>(i, draw);
+        const GrReplacements::ReplacementInfo* ri = fReplacements->lookupByStart(
+                                                            fPicture->uniqueID(), 
+                                                            startOffset, 
+                                                            fCanvas->getTotalMatrix());
+
+        if (ri) {
+            draw_replacement_bitmap(ri, fCanvas, fInitialMatrix);
+
+            if (fPicture->fBBH.get()) {
+                while ((uintptr_t)fOps[fIndex] < ri->fStop) {
+                    ++fIndex;
+                }
+                SkASSERT((uintptr_t)fOps[fIndex] == ri->fStop);
+            } else {
+                fIndex = ri->fStop;
+            }
+            return;
         }
+
+        // This is a fail for layer hoisting
+        this->INHERITED::operator()(sl);
     }
+
+private:
+    SkCanvas*              fCanvas;
+    const SkPicture*       fPicture;
+    const GrReplacements*  fReplacements;
+    const SkMatrix         fInitialMatrix;
+    SkDrawPictureCallback* fCallback;
+
+    SkTDArray<void*>       fOps;
+    int                    fIndex;
+
+    typedef Draw INHERITED;
+};
+
+void GrRecordReplaceDraw(const SkPicture* picture,
+                         SkCanvas* canvas,
+                         const GrReplacements* replacements,
+                         const SkMatrix& initialMatrix,
+                         SkDrawPictureCallback* callback) {
+    SkAutoCanvasRestore saveRestore(canvas, true /*save now, restore at exit*/);
+
+    ReplaceDraw draw(canvas, picture, replacements, initialMatrix, callback);
+
+    draw.draw();
 }
index 4900e0d..b47b077 100644 (file)
@@ -8,9 +8,11 @@
 #ifndef GrRecordReplaceDraw_DEFINED
 #define GrRecordReplaceDraw_DEFINED
 
+#include "SkChecksum.h"
 #include "SkDrawPictureCallback.h"
+#include "SkImage.h"
 #include "SkRect.h"
-#include "SkTDArray.h"
+#include "SkTDynamicHash.h"
 
 class SkBBoxHierarchy;
 class SkBitmap;
@@ -26,9 +28,53 @@ class SkRecord;
 class GrReplacements {
 public:
     // All the operations between fStart and fStop (inclusive) will be replaced with
-    // a single drawBitmap call using fPos, fBM and fPaint.
-    struct ReplacementInfo {
-        unsigned        fStart;
+    // a single drawBitmap call using fPos, fImage and fPaint.
+    class ReplacementInfo {
+    public:
+        struct Key {
+            Key(uint32_t pictureID, unsigned int start, const SkMatrix& ctm)
+            : fPictureID(pictureID)
+            , fStart(start)
+            , fCTM(ctm) {
+                fCTM.getType(); // force initialization of type so hashes match
+
+                // Key needs to be tightly packed.
+                GR_STATIC_ASSERT(sizeof(Key) == sizeof(uint32_t) +      // picture ID
+                                                sizeof(int) +           // start
+                                                9 * sizeof(SkScalar)    // 3x3 from CTM
+                                                +sizeof(uint32_t));     // matrix's type
+            }
+
+            bool operator==(const Key& other) const { 
+                return fPictureID == other.fPictureID &&
+                       fStart == other.fStart &&
+                       fCTM.cheapEqualTo(other.fCTM); // TODO: should be fuzzy
+            }
+
+            uint32_t     pictureID() const { return fPictureID; }
+            unsigned int start() const { return fStart; }
+
+        private:
+            const uint32_t     fPictureID;
+            const unsigned int fStart;
+            const SkMatrix     fCTM;
+        };
+
+        static const Key& GetKey(const ReplacementInfo& layer) { return layer.fKey; }
+        static uint32_t Hash(const Key& key) {
+            return SkChecksum::Murmur3(reinterpret_cast<const uint32_t*>(&key), sizeof(Key));
+        }
+
+        ReplacementInfo(uint32_t pictureID, unsigned int start, const SkMatrix& ctm)
+            : fKey(pictureID, start, ctm)
+            , fImage(NULL)
+            , fPaint(NULL) {
+        }
+        ~ReplacementInfo() { fImage->unref(); SkDELETE(fPaint); }
+
+        unsigned int start() const { return fKey.start(); }
+
+        const Key       fKey;
         unsigned        fStop;
         SkIPoint        fPos;
         SkImage*        fImage;  // Owns a ref
@@ -39,25 +85,18 @@ public:
 
     ~GrReplacements() { this->freeAll(); }
 
-    // Add a new replacement range. The replacement ranges should be
-    // sorted in increasing order and non-overlapping (esp. no nested
-    // saveLayers).
-    ReplacementInfo* push();
+    // Add a new replacement range.
+    ReplacementInfo* newReplacement(uint32_t pictureID, unsigned int start, const SkMatrix& ctm);
 
-    // look up a replacement range by its start offset.
-    // lastLookedUp is an in/out parameter that is used to speed up the search.
-    // It should be initialized to 0 on the first call and then passed back in
-    // unmodified on subsequent calls.
-    const ReplacementInfo* lookupByStart(size_t start, int* lastLookedUp) const;
+    // look up a replacement range by its pictureID, start offset and the CTM
+    // TODO: also need to add clip to lookup
+    const ReplacementInfo* lookupByStart(uint32_t pictureID, size_t start, 
+                                         const SkMatrix& ctm) const;
 
 private:
-    SkTDArray<ReplacementInfo> fReplacements;
+    SkTDynamicHash<ReplacementInfo, ReplacementInfo::Key> fReplacementHash;
 
     void freeAll();
-
-#ifdef SK_DEBUG
-    void validate() const;
-#endif
 };
 
 // Draw an SkPicture into an SkCanvas replacing saveLayer/restore blocks with
@@ -65,7 +104,7 @@ private:
 void GrRecordReplaceDraw(const SkPicture*,
                          SkCanvas*,
                          const GrReplacements*,
-                         const SkMatrix&,
+                         const SkMatrix& initialMatrix,
                          SkDrawPictureCallback*);
 
 #endif // GrRecordReplaceDraw_DEFINED
index 5b6d12c..e93b622 100644 (file)
@@ -112,13 +112,12 @@ void test_replacements(skiatest::Reporter* r, bool useBBH) {
         canvas->restore();
         canvas->drawRect(SkRect::MakeWH(SkIntToScalar(kWidth / 2), SkIntToScalar(kHeight / 2)),
                          SkPaint());
-
         pic.reset(recorder.endRecording());
     }
 
     GrReplacements replacements;
-    GrReplacements::ReplacementInfo* ri = replacements.push();
-    ri->fStart = 0;
+    GrReplacements::ReplacementInfo* ri = replacements.newReplacement(pic->uniqueID(), 
+                                                                      0, SkMatrix::I());
     ri->fStop = 2;
     ri->fPos.set(0, 0);
     ri->fImage = make_image(SK_ColorRED);