Fix GrAtlasTextBlob bounds management
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1605013002
Committed: https://skia.googlesource.com/skia/+/ae473fdfc3db2d9dd82b05b2568767d6d4038fcd
Review URL: https://codereview.chromium.org/1605013002
diff --git a/src/gpu/batches/GrAtlasTextBatch.cpp b/src/gpu/batches/GrAtlasTextBatch.cpp
index ce6b2ea..b2251a4 100644
--- a/src/gpu/batches/GrAtlasTextBatch.cpp
+++ b/src/gpu/batches/GrAtlasTextBatch.cpp
@@ -413,6 +413,19 @@
size_t byteCount = info.byteCount();
memcpy(currVertex, blob->fVertices + info.vertexStartIndex(), byteCount);
+#ifdef SK_DEBUG
+ // bounds sanity check
+ SkRect rect;
+ rect.setLargestInverted();
+ SkPoint* vertex = (SkPoint*) ((char*)blob->fVertices + info.vertexStartIndex());
+ rect.growToInclude(vertex, vertexStride, kVerticesPerGlyph * info.glyphCount());
+
+ if (this->usesDistanceFields()) {
+ fBatch.fViewMatrix.mapRect(&rect);
+ }
+ SkASSERT(fBounds.contains(rect));
+#endif
+
currVertex += byteCount;
}
diff --git a/src/gpu/batches/GrAtlasTextBatch.h b/src/gpu/batches/GrAtlasTextBatch.h
index 8f20313..f4008c9 100644
--- a/src/gpu/batches/GrAtlasTextBatch.h
+++ b/src/gpu/batches/GrAtlasTextBatch.h
@@ -84,14 +84,35 @@
fBatch.fViewMatrix = geo.fBlob->fViewMatrix;
// We don't yet position distance field text on the cpu, so we have to map the vertex bounds
- // into device space
+ // into device space.
+ // We handle vertex bounds differently for distance field text and bitmap text because
+ // the vertex bounds of bitmap text are in device space. If we are flushing multiple runs
+ // from one blob then we are going to pay the price here of mapping the rect for each run.
const Run& run = geo.fBlob->fRuns[geo.fRun];
+ SkRect bounds = run.fSubRunInfo[geo.fSubRun].vertexBounds();
if (run.fSubRunInfo[geo.fSubRun].drawAsDistanceFields()) {
- SkRect bounds = run.fVertexBounds;
+ // Distance field text is positioned with the (X,Y) as part of the glyph position,
+ // and currently the view matrix is applied on the GPU
+ bounds.offset(geo.fBlob->fX - geo.fBlob->fInitialX,
+ geo.fBlob->fY - geo.fBlob->fInitialY);
fBatch.fViewMatrix.mapRect(&bounds);
this->setBounds(bounds);
} else {
- this->setBounds(run.fVertexBounds);
+ // Bitmap text is fully positioned on the CPU, and offset by an (X,Y) translate in
+ // device space.
+ SkMatrix boundsMatrix = geo.fBlob->fInitialViewMatrixInverse;
+
+ boundsMatrix.postTranslate(-geo.fBlob->fInitialX, -geo.fBlob->fInitialY);
+
+ boundsMatrix.postTranslate(geo.fBlob->fX, geo.fBlob->fY);
+
+ boundsMatrix.postConcat(geo.fBlob->fViewMatrix);
+ boundsMatrix.mapRect(&bounds);
+
+ // Due to floating point numerical inaccuracies, we have to round out here
+ SkRect roundedOutBounds;
+ bounds.roundOut(&roundedOutBounds);
+ this->setBounds(roundedOutBounds);
}
}
diff --git a/src/gpu/text/GrAtlasTextBlob.cpp b/src/gpu/text/GrAtlasTextBlob.cpp
index 05fb5ad..bf8f286 100644
--- a/src/gpu/text/GrAtlasTextBlob.cpp
+++ b/src/gpu/text/GrAtlasTextBlob.cpp
@@ -63,7 +63,7 @@
subRun->setMaskFormat(format);
- run.fVertexBounds.joinNonEmptyArg(positions);
+ subRun->joinGlyphBounds(positions);
subRun->setColor(color);
intptr_t vertex = reinterpret_cast<intptr_t>(this->fVertices + subRun->vertexEndIndex());
@@ -219,7 +219,6 @@
(*outTransY) = y - fY;
}
-
// If we can reuse the blob, then make sure we update the blob's viewmatrix, and x/y
// offsets. Note, we offset the vertex bounds right before flushing
fViewMatrix = viewMatrix;
@@ -383,7 +382,6 @@
drawFilter, viewMatrix, clipBounds, x, y);
continue;
}
- fRuns[run].fVertexBounds.offset(transX, transY);
this->flushRun(dc, &pipelineBuilder, run, color,
transX, transY, skPaint, props,
distanceAdjustTable, context->getBatchFontCache());
@@ -419,8 +417,8 @@
const SkPaint& skPaint, const SkSurfaceProps& props,
const GrDistanceFieldAdjustTable* distanceAdjustTable,
GrBatchFontCache* cache) {
- const GrAtlasTextBlob::Run::SubRunInfo& info = fRuns[0].fSubRunInfo[0];
- return this->createBatch(info, glyphCount, 0, 0, color, transX, transY, skPaint,
+ const GrAtlasTextBlob::Run::SubRunInfo& info = fRuns[run].fSubRunInfo[subRun];
+ return this->createBatch(info, glyphCount, run, subRun, color, transX, transY, skPaint,
props, distanceAdjustTable, cache);
}
diff --git a/src/gpu/text/GrAtlasTextBlob.h b/src/gpu/text/GrAtlasTextBlob.h
index 1c5ea5e..af6441e 100644
--- a/src/gpu/text/GrAtlasTextBlob.h
+++ b/src/gpu/text/GrAtlasTextBlob.h
@@ -210,13 +210,11 @@
// We use this color vs the SkPaint color because it has the colorfilter applied.
void initReusableBlob(GrColor color, const SkMatrix& viewMatrix, SkScalar x, SkScalar y) {
fPaintColor = color;
- fViewMatrix = viewMatrix;
- fX = x;
- fY = y;
+ this->setupViewMatrix(viewMatrix, x, y);
}
- void initThrowawayBlob(const SkMatrix& viewMatrix) {
- fViewMatrix = viewMatrix;
+ void initThrowawayBlob(const SkMatrix& viewMatrix, SkScalar x, SkScalar y) {
+ this->setupViewMatrix(viewMatrix, x, y);
}
GrDrawBatch* test_createBatch(int glyphCount, int run, int subRun,
@@ -249,6 +247,20 @@
SkDrawFilter* drawFilter, const SkMatrix& viewMatrix,
const SkIRect& clipBounds, SkScalar x, SkScalar y);
+ // This function will only be called when we are regenerating a blob from scratch. We record the
+ // initial view matrix and initial offsets(x,y), because we record vertex bounds relative to
+ // these numbers. When blobs are reused with new matrices, we need to return to model space so
+ // we can update the vertex bounds appropriately.
+ void setupViewMatrix(const SkMatrix& viewMatrix, SkScalar x, SkScalar y) {
+ fViewMatrix = viewMatrix;
+ if (!viewMatrix.invert(&fInitialViewMatrixInverse)) {
+ fInitialViewMatrixInverse = SkMatrix::I();
+ SkDebugf("Could not invert viewmatrix\n");
+ }
+ fX = fInitialX = x;
+ fY = fInitialY = y;
+ }
+
/*
* Each Run inside of the blob can have its texture coordinates regenerated if required.
* To determine if regeneration is necessary, fAtlasGeneration is used. If there have been
@@ -276,7 +288,6 @@
Run()
: fInitialized(false)
, fDrawAsPaths(false) {
- fVertexBounds.setLargestInverted();
// To ensure we always have one subrun, we push back a fresh run here
fSubRunInfo.push_back();
}
@@ -290,10 +301,13 @@
, fColor(GrColor_ILLEGAL)
, fMaskFormat(kA8_GrMaskFormat)
, fDrawAsDistanceFields(false)
- , fUseLCDText(false) {}
+ , fUseLCDText(false) {
+ fVertexBounds.setLargestInverted();
+ }
SubRunInfo(const SubRunInfo& that)
: fBulkUseToken(that.fBulkUseToken)
, fStrike(SkSafeRef(that.fStrike.get()))
+ , fVertexBounds(that.fVertexBounds)
, fAtlasGeneration(that.fAtlasGeneration)
, fVertexStartIndex(that.fVertexStartIndex)
, fVertexEndIndex(that.fVertexEndIndex)
@@ -338,6 +352,11 @@
fVertexEndIndex = prev.vertexEndIndex();
}
+ const SkRect& vertexBounds() const { return fVertexBounds; }
+ void joinGlyphBounds(const SkRect& glyphBounds) {
+ fVertexBounds.joinNonEmptyArg(glyphBounds);
+ }
+
// df properties
void setUseLCDText(bool useLCDText) { fUseLCDText = useLCDText; }
bool hasUseLCDText() const { return fUseLCDText; }
@@ -347,6 +366,7 @@
private:
GrBatchAtlas::BulkUseTokenUpdater fBulkUseToken;
SkAutoTUnref<GrBatchTextStrike> fStrike;
+ SkRect fVertexBounds;
uint64_t fAtlasGeneration;
size_t fVertexStartIndex;
size_t fVertexEndIndex;
@@ -368,7 +388,6 @@
}
static const int kMinSubRuns = 1;
SkAutoTUnref<SkTypeface> fTypeface;
- SkRect fVertexBounds;
SkSTArray<kMinSubRuns, SubRunInfo> fSubRunInfo;
SkAutoDescriptor fDescriptor;
@@ -423,7 +442,10 @@
SkTArray<BigGlyph> fBigGlyphs;
Key fKey;
SkMatrix fViewMatrix;
+ SkMatrix fInitialViewMatrixInverse;
GrColor fPaintColor;
+ SkScalar fInitialX;
+ SkScalar fInitialY;
SkScalar fX;
SkScalar fY;
diff --git a/src/gpu/text/GrAtlasTextContext.cpp b/src/gpu/text/GrAtlasTextContext.cpp
index e286aa6..eef4f0b 100644
--- a/src/gpu/text/GrAtlasTextContext.cpp
+++ b/src/gpu/text/GrAtlasTextContext.cpp
@@ -281,7 +281,7 @@
int glyphCount = skPaint.countText(text, byteLength);
GrAtlasTextBlob* blob = fCache->createBlob(glyphCount, 1, GrAtlasTextBlob::kGrayTextVASize);
- blob->initThrowawayBlob(viewMatrix);
+ blob->initThrowawayBlob(viewMatrix, x, y);
if (GrTextUtils::CanDrawAsDistanceFields(skPaint, viewMatrix, fSurfaceProps,
*fContext->caps()->shaderCaps())) {
@@ -304,7 +304,7 @@
int glyphCount = skPaint.countText(text, byteLength);
GrAtlasTextBlob* blob = fCache->createBlob(glyphCount, 1, GrAtlasTextBlob::kGrayTextVASize);
- blob->initThrowawayBlob(viewMatrix);
+ blob->initThrowawayBlob(viewMatrix, offset.x(), offset.y());
if (GrTextUtils::CanDrawAsDistanceFields(skPaint, viewMatrix, fSurfaceProps,
*fContext->caps()->shaderCaps())) {
@@ -393,8 +393,10 @@
gTextContext->createDrawTextBlob(grPaint, skPaint, viewMatrix, text,
static_cast<size_t>(textLen), 0, 0));
- SkScalar transX = static_cast<SkScalar>(random->nextU());
- SkScalar transY = static_cast<SkScalar>(random->nextU());
+ // We'd like to be able to test this with random translations, but currently the vertex
+ // bounds and vertices will get out of sync
+ SkScalar transX = 0.f;//SkIntToScalar(random->nextU());
+ SkScalar transY = 0.f;//SkIntToScalar(random->nextU());
return blob->test_createBatch(textLen, 0, 0, color, transX, transY, skPaint,
gSurfaceProps, gTextContext->dfAdjustTable(),
context->getBatchFontCache());