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 c01e9a3..6354e9f 100644
--- a/src/core/SkPictureFlat.h
+++ b/src/core/SkPictureFlat.h
@@ -339,7 +339,7 @@
// 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 @@
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 @@
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 @@
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 @@
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 3e481ac..9b05250 100644
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -535,9 +535,8 @@
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 @@
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 @@
} 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 @@
#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 @@
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 @@
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 @@
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 @@
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 @@
}
}
#endif
-
diff --git a/src/core/SkPictureRecord.h b/src/core/SkPictureRecord.h
index abaa22d..782ee43 100644
--- a/src/core/SkPictureRecord.h
+++ b/src/core/SkPictureRecord.h
@@ -75,7 +75,7 @@
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 @@
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);