Fixing invalid text clipping on SkPicture playback

The bug was caused by an invalid assumption that a flattend object's
index is related to its array index in SkFlatDictionary::fData.
The data in SkFlatDictionary is sorted by flattened data content,
not by index number. Problem was solved by passing down the SkFlatData*
through addPaint, rather than the index value. The bug was causing
SkPictureRecord::addFontMetricsTopBottom to use cached font metrics
from the wrong SkPaint instance.

BUG=https://code.google.com/p/chromium/issues/detail?id=170964
Review URL: https://codereview.appspot.com/7178045

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

diff --git a/src/core/SkPictureFlat.h b/src/core/SkPictureFlat.h
index c01e9a37246456162664791eb0173a93359f19e7..6354e9fc2aaf00b2f546faa3f8366127316c2c04 100644 (file)
--- a/src/core/SkPictureFlat.h
+++ b/src/core/SkPictureFlat.h
@@ -339,7 +339,7 @@ public:
 
     // Returns fTopBot array, so it can be passed to a routine to compute them.
     // For efficiency, we assert that fTopBot have not been recorded yet.
-    SkScalar* writableTopBot() {
+    SkScalar* writableTopBot() const {
         SkASSERT(!this->isTopBotWritten());
         return fTopBot;
     }
@@ -355,10 +355,10 @@ private:
     int fIndex;
 
     // Cache of paint's FontMetrics fTop,fBottom
-    // initialied to [0,0] as a sentinel that they have not been recorded yet
+    // initialied to [NaN,NaN] as a sentinel that they have not been recorded yet
     //
     // This is *not* part of the key for search/sort
-    SkScalar fTopBot[2];
+    mutable SkScalar fTopBot[2];
 
     // marks fTopBot[] as unrecorded
     void setTopBotUnwritten() {
@@ -494,29 +494,6 @@ public:
         return array;
     }
 
-protected:
-    void (*fFlattenProc)(SkOrderedWriteBuffer&, const void*);
-    void (*fUnflattenProc)(SkOrderedReadBuffer&, void*);
-
-private:
-    void unflattenIntoArray(T* array) const {
-        const int count = fData.count();
-        const SkFlatData** iter = fData.begin();
-        for (int i = 0; i < count; ++i) {
-            const SkFlatData* element = iter[i];
-            int index = element->index() - 1;
-            SkASSERT((unsigned)index < (unsigned)count);
-            element->unflatten(&array[index], fUnflattenProc,
-                               fController->getBitmapHeap(),
-                               fController->getTypefacePlayback());
-        }
-    }
-
-
-    SkFlatController * const     fController;
-    int                          fNextIndex;
-    SkTDArray<const SkFlatData*> fData;
-
     const SkFlatData* findAndReturnFlat(const T& element) {
         SkFlatData* flat = SkFlatData::Create(fController, &element, fNextIndex, fFlattenProc);
 
@@ -545,6 +522,27 @@ private:
         return flat;
     }
 
+protected:
+    void (*fFlattenProc)(SkOrderedWriteBuffer&, const void*);
+    void (*fUnflattenProc)(SkOrderedReadBuffer&, void*);
+
+private:
+    void unflattenIntoArray(T* array) const {
+        const int count = fData.count();
+        const SkFlatData** iter = fData.begin();
+        for (int i = 0; i < count; ++i) {
+            const SkFlatData* element = iter[i];
+            int index = element->index() - 1;
+            SkASSERT((unsigned)index < (unsigned)count);
+            element->unflatten(&array[index], fUnflattenProc,
+                               fController->getBitmapHeap(),
+                               fController->getTypefacePlayback());
+        }
+    }
+
+    SkFlatController * const     fController;
+    int                          fNextIndex;
+    SkTDArray<const SkFlatData*> fData;
 
     enum {
         // Determined by trying diff values on picture-recording benchmarks
@@ -654,10 +652,6 @@ class SkPaintDictionary : public SkFlatDictionary<SkPaint> {
         fFlattenProc = &SkFlattenObjectProc<SkPaint>;
         fUnflattenProc = &SkUnflattenObjectProc<SkPaint>;
     }
-
-    SkFlatData* writableFlatData(int index) {
-        return const_cast<SkFlatData*>((*this)[index]);
-    }
 };
 
 class SkRegionDictionary : public SkFlatDictionary<SkRegion> {

diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp
index 3e481acf070efd162b15111de7304f52e8f993fc..9b05250ba82194ea1b595c506196e97603a3428b 100644 (file)
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -535,9 +535,8 @@ static void computeFontMetricsTopBottom(const SkPaint& paint, SkScalar topbot[2]
     topbot[1] = bounds.fBottom;
 }
 
-void SkPictureRecord::addFontMetricsTopBottom(const SkPaint& paint, int index,
+void SkPictureRecord::addFontMetricsTopBottom(const SkPaint& paint, const SkFlatData& flat,
                                               SkScalar minY, SkScalar maxY) {
-    SkFlatData* flat = fPaints.writableFlatData(index);
     if (!flat->isTopBotWritten()) {
         computeFontMetricsTopBottom(paint, flat->writableTopBot());
         SkASSERT(flat->isTopBotWritten());
@@ -551,12 +550,13 @@ void SkPictureRecord::drawText(const void* text, size_t byteLength, SkScalar x,
     bool fast = !paint.isVerticalText() && paint.canComputeFastBounds();
 
     addDraw(fast ? DRAW_TEXT_TOP_BOTTOM : DRAW_TEXT);
-    int paintIndex = addPaint(paint);
+    const SkFlatData* flatPaintData = addPaint(paint);
+    SkASSERT(flatPaintData);
     addText(text, byteLength);
     addScalar(x);
     addScalar(y);
     if (fast) {
-        addFontMetricsTopBottom(paint, paintIndex - 1, y, y);
+        addFontMetricsTopBottom(paint, *flatPaintData, y, y);
     }
     validate();
 }
@@ -597,7 +597,8 @@ void SkPictureRecord::drawPosText(const void* text, size_t byteLength,
     } else {
         addDraw(DRAW_POS_TEXT);
     }
-    int paintIndex = addPaint(paint);
+    const SkFlatData* flatPaintData = addPaint(paint);
+    SkASSERT(flatPaintData);
     addText(text, byteLength);
     addInt(points);
 
@@ -606,7 +607,7 @@ void SkPictureRecord::drawPosText(const void* text, size_t byteLength,
 #endif
     if (canUseDrawH) {
         if (fast) {
-            addFontMetricsTopBottom(paint, paintIndex - 1, pos[0].fY, pos[0].fY);
+            addFontMetricsTopBottom(paint, *flatPaintData, pos[0].fY, pos[0].fY);
         }
         addScalar(pos[0].fY);
         SkScalar* xptr = (SkScalar*)fWriter.reserve(points * sizeof(SkScalar));
@@ -616,7 +617,7 @@ void SkPictureRecord::drawPosText(const void* text, size_t byteLength,
     else {
         fWriter.writeMul4(pos, points * sizeof(SkPoint));
         if (fastBounds) {
-            addFontMetricsTopBottom(paint, paintIndex - 1, minY, maxY);
+            addFontMetricsTopBottom(paint, *flatPaintData, minY, maxY);
         }
     }
 #ifdef SK_DEBUG_SIZE
@@ -636,7 +637,8 @@ void SkPictureRecord::drawPosTextH(const void* text, size_t byteLength,
     bool fast = !paint.isVerticalText() && paint.canComputeFastBounds();
 
     addDraw(fast ? DRAW_POS_TEXT_H_TOP_BOTTOM : DRAW_POS_TEXT_H);
-    int paintIndex = this->addPaint(paint);
+    const SkFlatData* flatPaintData = addPaint(paint);
+    SkASSERT(flatPaintData);
     addText(text, byteLength);
     addInt(points);
 
@@ -644,7 +646,7 @@ void SkPictureRecord::drawPosTextH(const void* text, size_t byteLength,
     size_t start = fWriter.size();
 #endif
     if (fast) {
-        addFontMetricsTopBottom(paint, paintIndex - 1, constY, constY);
+        addFontMetricsTopBottom(paint, *flatPaintData, constY, constY);
     }
     addScalar(constY);
     fWriter.writeMul4(xpos, points * sizeof(SkScalar));
@@ -731,10 +733,11 @@ void SkPictureRecord::addMatrixPtr(const SkMatrix* matrix) {
     this->addInt(matrix ? fMatrices.find(*matrix) : 0);
 }
 
-int SkPictureRecord::addPaintPtr(const SkPaint* paint) {
-    int index = paint ? fPaints.find(*paint) : 0;
+const SkFlatData* SkPictureRecord::addPaintPtr(const SkPaint* paint) {
+    const SkFlatData* data = paint ? fPaints.findAndReturnFlat(*paint) : NULL;
+    int index = data ? data->index() : 0;
     this->addInt(index);
-    return index;
+    return data;
 }
 
 void SkPictureRecord::addPath(const SkPath& path) {
@@ -950,4 +953,3 @@ void SkPictureRecord::validateRegions() const {
     }
 }
 #endif
-

diff --git a/src/core/SkPictureRecord.h b/src/core/SkPictureRecord.h
index abaa22dd9d7feda0b63d15169044e5799ec71b7e..782ee43d9572b7e784420808293dda0ec950b7e1 100644 (file)
--- a/src/core/SkPictureRecord.h
+++ b/src/core/SkPictureRecord.h
@@ -75,7 +75,7 @@ public:
     virtual void drawData(const void*, size_t) SK_OVERRIDE;
     virtual bool isDrawingToLayer() const SK_OVERRIDE;
 
-    void addFontMetricsTopBottom(const SkPaint& paint, int paintIndex,
+    void addFontMetricsTopBottom(const SkPaint& paint, const SkFlatData*,
                                  SkScalar minY, SkScalar maxY);
 
     const SkTDArray<SkPicture* >& getPictureRefs() const {
@@ -122,8 +122,8 @@ private:
     void addBitmap(const SkBitmap& bitmap);
     void addMatrix(const SkMatrix& matrix);
     void addMatrixPtr(const SkMatrix* matrix);
-    int  addPaint(const SkPaint& paint) { return this->addPaintPtr(&paint); }
-    int  addPaintPtr(const SkPaint* paint);
+    const SkFlatData* addPaint(const SkPaint& paint) { return this->addPaintPtr(&paint); }
+    const SkFlatData* addPaintPtr(const SkPaint* paint);
     void addPath(const SkPath& path);
     void addPicture(SkPicture& picture);
     void addPoint(const SkPoint& point);