Fix SkGPipe drawing, and turn it on by default.

A recent change broke SkGPipe. Fix it, and turn on pipe drawing
in GM by default so we will catch these in the future.

We already had a bug where SkGPipeWriter had to use its Cross Process
flag to work, so for a quick fix, force the reader to use the Cross
Process flag as well. The bug to allow both cross and non cross process
is http://code.google.com/p/skia/issues/detail?id=663

Review URL: https://codereview.appspot.com/6333071

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

3 files changed, 175 insertions, 67 deletions

diff --git a/src/pipe/SkGPipePriv.h b/src/pipe/SkGPipePriv.h
index 4ce8a4126f..82e6927c63 100644
--- a/src/pipe/SkGPipePriv.h
+++ b/src/pipe/SkGPipePriv.h
@@ -71,6 +71,7 @@ enum DrawOps {
     kDef_Bitmap_DrawOp,
 
     // these are signals to playback, not drawing verbs
+    kReportFlags_DrawOp,
     kDone_DrawOp,
 };
 
@@ -138,6 +139,13 @@ enum {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+static inline bool shouldFlattenBitmaps(uint32_t flags) {
+    return flags & SkGPipeWriter::kCrossProcess_Flag
+            && !(flags & SkGPipeWriter::kSharedAddressSpace_SkGPipeFlag);
+}
+
+///////////////////////////////////////////////////////////////////////////////
+
 enum PaintOps {
     kReset_PaintOp,     // no arg
     
diff --git a/src/pipe/SkGPipeRead.cpp b/src/pipe/SkGPipeRead.cpp
index 7f05b31414..a3afc03d05 100644
--- a/src/pipe/SkGPipeRead.cpp
+++ b/src/pipe/SkGPipeRead.cpp
@@ -67,9 +67,20 @@ public:
     SkGPipeState();
     ~SkGPipeState();
 
+    void setFlags(unsigned flags) {
+        if (fFlags != flags) {
+            fFlags = flags;
+            this->updateReader();
+        }
+    }
+
+    unsigned getFlags() const {
+        return fFlags;
+    }
+
     void setReader(SkFlattenableReadBuffer* reader) {
         fReader = reader;
-        fReader->setFactoryArray(&fFactoryArray);
+        this->updateReader();
     }
 
     const SkPaint& paint() const { return fPaint; }
@@ -91,9 +102,7 @@ public:
     void addBitmap(int index) {
         SkASSERT(fBitmaps.count() == index);
         SkBitmap* bm = new SkBitmap();
-        size_t size = fReader->readU32();
-        SkOrderedReadBuffer readBuffer(fReader->skip(size), size);
-        bm->unflatten(readBuffer);
+        bm->unflatten(*fReader);
         *fBitmaps.append() = bm;
     }
 
@@ -111,14 +120,27 @@ public:
         paint->setTypeface(id ? fTypefaces[id - 1] : NULL);
     }
     
-    SkFlattenableReadBuffer* fReader;
-
 private:
+    void updateReader() {
+        if (NULL == fReader) {
+            return;
+        }
+        bool crossProcess = SkToBool(fFlags & SkGPipeWriter::kCrossProcess_Flag);
+        fReader->setFlags(SkSetClearMask(fReader->getFlags(), crossProcess,
+                                         SkFlattenableReadBuffer::kCrossProcess_Flag));
+        if (crossProcess) {
+            fReader->setFactoryArray(&fFactoryArray);
+        } else {
+            fReader->setFactoryArray(NULL);
+        }
+    }
+    SkFlattenableReadBuffer*  fReader;
     SkPaint                   fPaint;
     SkTDArray<SkFlattenable*> fFlatArray;
     SkTDArray<SkTypeface*>    fTypefaces;
     SkTDArray<SkFlattenable::Factory> fFactoryArray;
     SkTDArray<SkBitmap*>      fBitmaps;
+    unsigned                  fFlags;
 };
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -345,9 +367,17 @@ static void drawTextOnPath_rp(SkCanvas* canvas, SkReader32* reader, uint32_t op3
 
 ///////////////////////////////////////////////////////////////////////////////
 
+static const SkBitmap* getBitmap(SkReader32* reader, uint32_t op32, SkGPipeState* state) {
+    if (shouldFlattenBitmaps(state->getFlags())) {
+        unsigned index = DrawOp_unpackData(op32);
+        return state->getBitmap(index);
+    }
+    return static_cast<const SkBitmap*>(reader->readPtr());
+}
+
 static void drawBitmap_rp(SkCanvas* canvas, SkReader32* reader, uint32_t op32,
                           SkGPipeState* state) {
-    const SkBitmap* bm(static_cast<const SkBitmap*>(reader->readPtr()));
+    const SkBitmap* bm = getBitmap(reader, op32, state);
     bool hasPaint = reader->readBool();
     SkScalar left = reader->readScalar();
     SkScalar top = reader->readScalar();
@@ -361,7 +391,7 @@ static void drawBitmapMatrix_rp(SkCanvas* canvas, SkReader32* reader, uint32_t o
 
 static void drawBitmapNine_rp(SkCanvas* canvas, SkReader32* reader,
                               uint32_t op32, SkGPipeState* state) {
-    const SkBitmap* bm(static_cast<const SkBitmap*>(reader->readPtr()));
+    const SkBitmap* bm = getBitmap(reader, op32, state);
     bool hasPaint = reader->readBool();
     const SkIRect* center = skip<SkIRect>(reader);
     const SkRect* dst = skip<SkRect>(reader);
@@ -371,7 +401,7 @@ static void drawBitmapNine_rp(SkCanvas* canvas, SkReader32* reader,
 
 static void drawBitmapRect_rp(SkCanvas* canvas, SkReader32* reader,
                               uint32_t op32, SkGPipeState* state) {
-    const SkBitmap* bm(static_cast<const SkBitmap*>(reader->readPtr()));
+    const SkBitmap* bm = getBitmap(reader, op32, state);
     bool hasPaint = reader->readBool();
     bool hasSrc = reader->readBool();
     const SkIRect* src;
@@ -386,7 +416,7 @@ static void drawBitmapRect_rp(SkCanvas* canvas, SkReader32* reader,
 
 static void drawSprite_rp(SkCanvas* canvas, SkReader32* reader, uint32_t op32,
                           SkGPipeState* state) {
-    const SkBitmap* bm(static_cast<const SkBitmap*>(reader->readPtr()));
+    const SkBitmap* bm = getBitmap(reader, op32, state);
     bool hasPaint = reader->readBool();
     const SkIPoint* point = skip<SkIPoint>(reader);
     canvas->drawSprite(*bm, point->fX, point->fY, hasPaint ? &state->paint() : NULL);
@@ -483,6 +513,12 @@ static void skip_rp(SkCanvas*, SkReader32* reader, uint32_t op32, SkGPipeState*)
     (void)reader->skip(bytes);
 }
 
+static void reportflags_rp(SkCanvas*, SkReader32*, uint32_t op32,
+                           SkGPipeState* state) {
+    unsigned flags = DrawOp_unpackFlags(op32);
+    state->setFlags(flags);
+}
+
 static void done_rp(SkCanvas*, SkReader32*, uint32_t, SkGPipeState*) {}
 
 typedef void (*ReadProc)(SkCanvas*, SkReader32*, uint32_t op32, SkGPipeState*);
@@ -524,12 +560,13 @@ static const ReadProc gReadTable[] = {
     def_PaintFlat_rp,
     def_Bitmap_rp,
 
+    reportflags_rp,
     done_rp
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 
-SkGPipeState::SkGPipeState() {}
+SkGPipeState::SkGPipeState(): fReader(0), fFlags(0) {}
 
 SkGPipeState::~SkGPipeState() {
     fTypefaces.safeUnrefAll();
diff --git a/src/pipe/SkGPipeWrite.cpp b/src/pipe/SkGPipeWrite.cpp
index b97b98d08a..15adea8935 100644
--- a/src/pipe/SkGPipeWrite.cpp
+++ b/src/pipe/SkGPipeWrite.cpp
@@ -68,7 +68,7 @@ static size_t writeTypeface(SkWriter32* writer, SkTypeface* typeface) {
  */
 class Heap {
 public:
-    Heap() {}
+    Heap(bool shallow) : fCanDoShallowCopies(shallow) {}
     ~Heap() {
         for (int i = 0; i < fBitmaps.count(); i++) {
             delete fBitmaps[i].fBitmap;
@@ -88,18 +88,23 @@ public:
                 return fBitmaps[i].fBitmap;
             }
         }
-        // TODO: Use a flag to determine whether we need the bitmap to be
-        // in shared cross process address space. If not, we can do a shallow
-        // copy.
-        SkBitmap* copy = new SkBitmap();
-        if (bm.copyTo(copy, bm.getConfig())) {
-            BitmapInfo* info = fBitmaps.append();
-            info->fBitmap = copy;
-            info->fGenID = genID;
-            return copy;
+        SkBitmap* copy;
+        // If the bitmap is mutable, we still need to do a deep copy, since the
+        // caller may modify it afterwards. That said, if the bitmap is mutable,
+        // but has no pixelRef, the copy constructor actually does a deep copy.
+        if (fCanDoShallowCopies && (bm.isImmutable() || !bm.pixelRef())) {
+            copy = new SkBitmap(bm);
+        } else {
+            copy = new SkBitmap();
+            if (!bm.copyTo(copy, bm.getConfig())) {
+                delete copy;
+                return NULL;
+            }
         }
-        delete copy;
-        return NULL;
+        BitmapInfo* info = fBitmaps.append();
+        info->fBitmap = copy;
+        info->fGenID = genID;
+        return copy;
     }
 private:
     struct BitmapInfo {
@@ -109,14 +114,15 @@ private:
         // for comparing.
         uint32_t fGenID;
     };
-    SkTDArray<BitmapInfo>fBitmaps;
+    SkTDArray<BitmapInfo>   fBitmaps;
+    const bool              fCanDoShallowCopies;
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 
 class SkGPipeCanvas : public SkCanvas {
 public:
-    SkGPipeCanvas(SkGPipeController*, SkWriter32*, SkFactorySet*);
+    SkGPipeCanvas(SkGPipeController*, SkWriter32*, SkFactorySet*, uint32_t flags);
     virtual ~SkGPipeCanvas();
 
     void finish() {
@@ -187,6 +193,7 @@ private:
     size_t      fBlockSize; // amount allocated for writer
     size_t      fBytesNotified;
     bool        fDone;
+    uint32_t    fFlags;
 
     SkRefCntSet fTypefaceSet;
 
@@ -246,10 +253,12 @@ private:
 };
 
 int SkGPipeCanvas::flattenToIndex(const SkBitmap & bitmap) {
+    SkASSERT(shouldFlattenBitmaps(fFlags));
     SkOrderedWriteBuffer tmpWriter(1024);
-    // FIXME: Rather than forcing CrossProcess, we should create an SkRefCntSet
-    // so that we can store a pointer to a bitmap's pixels during flattening.
-    tmpWriter.setFlags(SkFlattenableWriteBuffer::kCrossProcess_Flag);
+    tmpWriter.setFlags((SkFlattenableWriteBuffer::Flags)
+                       (SkFlattenableWriteBuffer::kInlineFactoryNames_Flag
+                        | SkFlattenableWriteBuffer::kCrossProcess_Flag));
+    tmpWriter.setFactoryRecorder(fFactorySet);
     bitmap.flatten(tmpWriter);
 
     size_t len = tmpWriter.size();
@@ -271,9 +280,8 @@ int SkGPipeCanvas::flattenToIndex(const SkBitmap & bitmap) {
         // for a NULL bitmap (unlike with paint flattenables).
         copy->fIndex = fBitmapArray.count();
         *fBitmapArray.insert(index) = copy;
-        if (this->needOpBytes(len + sizeof(uint32_t))) {
+        if (this->needOpBytes(len)) {
             this->writeOp(kDef_Bitmap_DrawOp, 0, copy->fIndex);
-            fWriter.write32(len);
             fWriter.write(copy->data(), len);
         }
     }
@@ -288,13 +296,15 @@ int SkGPipeCanvas::flattenToIndex(SkFlattenable* obj, PaintFlats paintflat) {
 
     SkOrderedWriteBuffer tmpWriter(1024);
     
-    // Needs to be cross process so a bitmap shader will be preserved
-    // FIXME: Rather than forcing CrossProcess, we should create an SkRefCntSet
-    // so that we can store a pointer to a bitmap's pixels during flattening.
-    tmpWriter.setFlags((SkFlattenableWriteBuffer::Flags)
-                       (SkFlattenableWriteBuffer::kInlineFactoryNames_Flag
-                       | SkFlattenableWriteBuffer::kCrossProcess_Flag));
-    tmpWriter.setFactoryRecorder(fFactorySet);
+    if (fFlags & SkGPipeWriter::kCrossProcess_Flag) {
+        tmpWriter.setFlags((SkFlattenableWriteBuffer::Flags)
+                           (SkFlattenableWriteBuffer::kInlineFactoryNames_Flag
+                           | SkFlattenableWriteBuffer::kCrossProcess_Flag));
+        tmpWriter.setFactoryRecorder(fFactorySet);
+    } else {
+        // Needed for bitmap shaders.
+        tmpWriter.setFlags(SkFlattenableWriteBuffer::kForceFlattenBitmapPixels_Flag);
+    }
 
     tmpWriter.writeFlattenable(obj);
     size_t len = tmpWriter.size();
@@ -330,8 +340,8 @@ int SkGPipeCanvas::flattenToIndex(SkFlattenable* obj, PaintFlats paintflat) {
 #define MIN_BLOCK_SIZE  (16 * 1024)
 
 SkGPipeCanvas::SkGPipeCanvas(SkGPipeController* controller,
-                             SkWriter32* writer, SkFactorySet* fset)
-        : fWriter(*writer) {
+                             SkWriter32* writer, SkFactorySet* fset, uint32_t flags)
+: fWriter(*writer), fFlags(flags), fHeap(!(flags & SkGPipeWriter::kCrossProcess_Flag)) {
     fFactorySet = fset;
     fController = controller;
     fDone = false;
@@ -346,6 +356,10 @@ SkGPipeCanvas::SkGPipeCanvas(SkGPipeController* controller,
     bitmap.setConfig(SkBitmap::kARGB_8888_Config, 32767, 32767);
     SkDevice* device = SkNEW_ARGS(SkDevice, (bitmap));
     this->setDevice(device)->unref();
+    // Tell the reader the appropriate flags to use.
+    if (this->needOpBytes()) {
+        this->writeOp(kReportFlags_DrawOp, fFlags, 0);
+    }
 }
 
 SkGPipeCanvas::~SkGPipeCanvas() {
@@ -596,9 +610,16 @@ void SkGPipeCanvas::drawPath(const SkPath& path, const SkPaint& paint) {
 
 void SkGPipeCanvas::drawBitmap(const SkBitmap& bm, SkScalar left, SkScalar top,
                                    const SkPaint* paint) {
-    const void* ptr(fHeap.addBitmap(bm));
-    if (NULL == ptr) {
-        return;
+    bool flatten = shouldFlattenBitmaps(fFlags);
+    const void* ptr = 0;
+    int bitmapIndex = 0;
+    if (flatten) {
+        bitmapIndex = this->flattenToIndex(bm);
+    } else {
+        ptr = fHeap.addBitmap(bm);
+        if (NULL == ptr) {
+            return;
+        }
     }
 
     NOTIFY_SETUP(this);
@@ -606,9 +627,15 @@ void SkGPipeCanvas::drawBitmap(const SkBitmap& bm, SkScalar left, SkScalar top,
         this->writePaint(*paint);
     }
 
-    if (this->needOpBytes(sizeof(SkScalar) * 2 + sizeof(bool) + sizeof(void*))) {
-        this->writeOp(kDrawBitmap_DrawOp, 0, 0);
-        fWriter.writePtr(const_cast<void*>(ptr));
+    size_t opBytesNeeded = sizeof(SkScalar) * 2 + sizeof(bool);
+    if (!flatten) {
+        opBytesNeeded += sizeof(void*);
+    }
+    if (this->needOpBytes(opBytesNeeded)) {
+        this->writeOp(kDrawBitmap_DrawOp, 0, bitmapIndex);
+        if (!flatten) {
+            fWriter.writePtr(const_cast<void*>(ptr));
+        }
         fWriter.writeBool(paint != NULL);
         fWriter.writeScalar(left);
         fWriter.writeScalar(top);
@@ -617,9 +644,16 @@ void SkGPipeCanvas::drawBitmap(const SkBitmap& bm, SkScalar left, SkScalar top,
 
 void SkGPipeCanvas::drawBitmapRect(const SkBitmap& bm, const SkIRect* src,
                                        const SkRect& dst, const SkPaint* paint) {
-    const void* ptr(fHeap.addBitmap(bm));
-    if (NULL == ptr) {
-        return;
+    bool flatten = shouldFlattenBitmaps(fFlags);
+    const void* ptr = 0;
+    int bitmapIndex = 0;
+    if (flatten) {
+        bitmapIndex = this->flattenToIndex(bm);
+    } else {
+        ptr = fHeap.addBitmap(bm);
+        if (NULL == ptr) {
+            return;
+        }
     }
 
     NOTIFY_SETUP(this);
@@ -627,14 +661,19 @@ void SkGPipeCanvas::drawBitmapRect(const SkBitmap& bm, const SkIRect* src,
         this->writePaint(*paint);
     }
 
-    size_t opBytesNeeded = sizeof(SkRect) + sizeof(bool) * 2 + sizeof(void*);
+    size_t opBytesNeeded = sizeof(SkRect) + sizeof(bool) * 2;
     bool hasSrc = src != NULL;
     if (hasSrc) {
         opBytesNeeded += sizeof(int32_t) * 4;
     }
+    if (!flatten) {
+        opBytesNeeded += sizeof(void*);
+    }
     if (this->needOpBytes(opBytesNeeded)) {
-        this->writeOp(kDrawBitmapRect_DrawOp, 0, 0);
-        fWriter.writePtr(const_cast<void*>(ptr));
+        this->writeOp(kDrawBitmapRect_DrawOp, 0, bitmapIndex);
+        if (!flatten) {
+            fWriter.writePtr(const_cast<void*>(ptr));
+        }
         fWriter.writeBool(paint != NULL);
         fWriter.writeBool(hasSrc);
         if (hasSrc) {
@@ -654,9 +693,16 @@ void SkGPipeCanvas::drawBitmapMatrix(const SkBitmap&, const SkMatrix&,
 
 void SkGPipeCanvas::drawBitmapNine(const SkBitmap& bm, const SkIRect& center,
                                    const SkRect& dst, const SkPaint* paint) {
-    const void* ptr(fHeap.addBitmap(bm));
-    if (NULL == ptr) {
-        return;
+    bool flatten = shouldFlattenBitmaps(fFlags);
+    const void* ptr = 0;
+    int bitmapIndex = 0;
+    if (flatten) {
+        bitmapIndex = this->flattenToIndex(bm);
+    } else {
+        ptr = fHeap.addBitmap(bm);
+        if (NULL == ptr) {
+            return;
+        }
     }
 
     NOTIFY_SETUP(this);
@@ -664,10 +710,15 @@ void SkGPipeCanvas::drawBitmapNine(const SkBitmap& bm, const SkIRect& center,
         this->writePaint(*paint);
     }
 
-    if (this->needOpBytes(sizeof(int32_t) * 4 + sizeof(bool)
-                          + sizeof(SkRect) + sizeof(void*))) {
-        this->writeOp(kDrawBitmapNine_DrawOp, 0, 0);
-        fWriter.writePtr(const_cast<void*>(ptr));
+    size_t opBytesNeeded = sizeof(int32_t) * 4 + sizeof(bool) + sizeof(SkRect);
+    if (!flatten) {
+        opBytesNeeded += sizeof(void*);
+    }
+    if (this->needOpBytes(opBytesNeeded)) {
+        this->writeOp(kDrawBitmapNine_DrawOp, 0, bitmapIndex);
+        if (!flatten) {
+            fWriter.writePtr(const_cast<void*>(ptr));
+        }
         fWriter.writeBool(paint != NULL);
         fWriter.write32(center.fLeft);
         fWriter.write32(center.fTop);
@@ -679,9 +730,16 @@ void SkGPipeCanvas::drawBitmapNine(const SkBitmap& bm, const SkIRect& center,
 
 void SkGPipeCanvas::drawSprite(const SkBitmap& bm, int left, int top,
                                    const SkPaint* paint) {
-    const void* ptr(fHeap.addBitmap(bm));
-    if (NULL == ptr) {
-        return;
+    bool flatten = shouldFlattenBitmaps(fFlags);
+    const void* ptr = 0;
+    int bitmapIndex = 0;
+    if (flatten) {
+        bitmapIndex = this->flattenToIndex(bm);
+    } else {
+        ptr = fHeap.addBitmap(bm);
+        if (NULL == ptr) {
+            return;
+        }
     }
 
     NOTIFY_SETUP(this);
@@ -689,9 +747,15 @@ void SkGPipeCanvas::drawSprite(const SkBitmap& bm, int left, int top,
         this->writePaint(*paint);
     }
 
-    if (this->needOpBytes(sizeof(int32_t) * 2 + sizeof(bool) + sizeof(void*))) {
-        this->writeOp(kDrawSprite_DrawOp, 0, 0);
-        fWriter.writePtr(const_cast<void*>(ptr));
+    size_t opBytesNeeded = sizeof(int32_t) * 2 + sizeof(bool);
+    if (!flatten) {
+        opBytesNeeded += sizeof(void*);
+    }
+    if (this->needOpBytes(opBytesNeeded)) {
+        this->writeOp(kDrawSprite_DrawOp, 0, bitmapIndex);
+        if (!flatten) {
+            fWriter.writePtr(const_cast<void*>(ptr));
+        }
         fWriter.writeBool(paint != NULL);
         fWriter.write32(left);
         fWriter.write32(top);
@@ -955,14 +1019,13 @@ SkGPipeWriter::~SkGPipeWriter() {
     SkSafeUnref(fCanvas);
 }
 
-SkCanvas* SkGPipeWriter::startRecording(SkGPipeController* controller,
-                                        uint32_t flags) {
+SkCanvas* SkGPipeWriter::startRecording(SkGPipeController* controller, uint32_t flags) {
     if (NULL == fCanvas) {
         fWriter.reset(NULL, 0);
         fFactorySet.reset();
         fCanvas = SkNEW_ARGS(SkGPipeCanvas, (controller, &fWriter,
                                              (flags & kCrossProcess_Flag) ?
-                                             &fFactorySet : NULL));
+                                             &fFactorySet : NULL, flags));
     }
     return fCanvas;
 }