Conservative SkTextBlob bounds.

src/core/SkTextBlob.cpp

 /*
  * Copyright 2014 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkTextBlob.h"
 
 #include "SkReadBuffer.h"
+#include "SkTypeface.h"
 #include "SkWriteBuffer.h"
 
 //
 // Textblob data is laid out into externally-managed storage as follows:
 //
 //    -----------------------------------------------------------------------------
 //   | SkTextBlob | RunRecord | Glyphs[] | Pos[] | RunRecord | Glyphs[] | Pos[] | ...
 //    -----------------------------------------------------------------------------
 //
 //  Each run record describes a text blob run, and can be used to determine the (implicit)
@@ skipping 276 matching lines @@
 }
 
 SkTextBlobBuilder::~SkTextBlobBuilder() {
     if (NULL != fStorage.get()) {
         // We are abandoning runs and must destruct the associated font data.
         // The easiest way to accomplish that is to use the blob destructor.
         build()->unref();
     }
 }
 
+SkRect SkTextBlobBuilder::tightRunBounds(const SkTextBlob::RunRecord& run) {
+    SkRect bounds;
+
+    if (SkTextBlob::kDefault_Positioning == run.positioning()) {
+        run.font().measureText(run.glyphBuffer(), run.glyphCount() * sizeof(uint16_t), &bounds);
+        return bounds;
+    }
+
+    SkASSERT(SkTextBlob::kFull_Positioning == run.positioning() ||
+             SkTextBlob::kHorizontal_Positioning == run.positioning());
+
+    SkAutoSTArray<16, SkRect> glyphBounds(run.glyphCount());
+    run.font().getTextWidths(run.glyphBuffer(),
+                             run.glyphCount() * sizeof(uint16_t),
+                             NULL,
+                             glyphBounds.get());
+
+    bounds = SkRect::MakeEmpty();
+    SkScalar* glyphPos = run.posBuffer();
+    for (unsigned i = 0; i < run.glyphCount(); ++i) {
+        if (SkTextBlob::kFull_Positioning == run.positioning()) {
+            // [ x, y, x, y... ]
+            glyphBounds[i].offset(glyphPos[0], glyphPos[1]);
+            SkASSERT(2 == SkTextBlob::ScalarsPerGlyph(run.positioning()));
+            glyphPos += 2;
+        } else {
+            // [ x, x, x... ], const y applied by runBounds.offset(run->offset()) later.
+            glyphBounds[i].offset(glyphPos[0], 0);
+            SkASSERT(1 == SkTextBlob::ScalarsPerGlyph(run.positioning()));
+            glyphPos += 1;
+        }
+
+        bounds.join(glyphBounds[i]);
+    }
+
+    SkASSERT((void*)glyphPos <= SkTextBlob::RunRecord::Next(&run));
+
+    bounds.offset(run.offset());
+
+    return bounds;
+}
+
+SkRect SkTextBlobBuilder::conservativeRunBounds(const SkTextBlob::RunRecord& run) {
+    const SkScalar* glyphPos = run.posBuffer();
+    int posScalars = SkTextBlob::ScalarsPerGlyph(run.positioning());
+
+    SkASSERT(1 == posScalars || 2 == posScalars);
+    SkASSERT(run.glyphCount() > 0);
+    SkASSERT((void*)(glyphPos + run.glyphCount() * posScalars) <= SkTextBlob::RunRecord::Next(&run));
+
+    // First, compute the glyph position bbox.
+    SkRect bounds = SkRect::MakeXYWH(glyphPos[0], (2 == posScalars) ? glyphPos[1] : 0, 0, 0);
+    for (unsigned i = 1; i < run.glyphCount(); ++i) {
+        bounds.growToInclude(glyphPos[i * posScalars],
+                             (2 == posScalars) ? glyphPos[i * posScalars + 1] : 0);
+    }
+
+    SkScalar glyphScale = run.font().getTextSize();
+    SkRect typefaceBounds;
+    if (SkToBool(run.font().getTypeface())) {
+        typefaceBounds = run.font().getTypeface()->getBounds();
+    } else {
+        SkAutoTUnref<SkTypeface> typeface(SkTypeface::RefDefault());
+        typefaceBounds = typeface->getBounds();
+    }
+
+    // Expand by typeface glyph bounds.
+    bounds.fLeft   += glyphScale * typefaceBounds.left();
+    bounds.fTop    += glyphScale * typefaceBounds.top();
+    bounds.fRight  += glyphScale * typefaceBounds.right();
+    bounds.fBottom += glyphScale * typefaceBounds.bottom();
+
+    // Offset by run position.
+    return bounds.makeOffset(run.offset().x(), run.offset().y());
+}

[f(malita), 2015/01/28 14:46:19]

Here used to live an awesome assert: conservative bounds should always contain
the tight bounds.

Except this awesome assert was firing in Chromium for certain pages, because of
minor vertical tight box snapping due to hinting (as Ben explained).

So, err, the awesome assert no longer lives here...

[mtklein, 2015/01/28 14:58:21]

I actually had the same sort of problem over in the bbh building world (no
surprise), and fixing it was the reason we added getBounds() to SkTypeface. 
Turns out, SkTypeface::getBounds() isn't quite good enough in all cases.  It
doesn't account for the paint effects on that box, like x scale and skew.  This
led to a few rendering issues where small skewed text was dropped.

Luckily, we now have SkPaint::getFontBounds(), which applies the scale and skew
and should return the rectangle you actually want.  gm/fontmgr demos it.  I'm
not really sure why I haven't made use of it yet over in the BBH code...

[f(malita), 2015/01/28 15:14:37]

Yay for getFontBounds() - thanks for the tip! Done.

+
 void SkTextBlobBuilder::updateDeferredBounds() {
     SkASSERT(!fDeferredBounds || fRunCount > 0);
 
     if (!fDeferredBounds) {
         return;
     }
 
     SkASSERT(fLastRun >= sizeof(SkTextBlob));
     SkTextBlob::RunRecord* run = reinterpret_cast<SkTextBlob::RunRecord*>(fStorage.get() +
                                                                           fLastRun);
     SkASSERT(SkPaint::kGlyphID_TextEncoding == run->font().getTextEncoding());
 
-    SkRect runBounds = SkRect::MakeEmpty();
+    SkRect runBounds;
+#ifdef SK_SUPPORT_LEGACY_BLOB_BOUNDS
+    runBounds = tightRunBounds(*run);
+#else
+    // FIXME: conservative bounds for default positioning?
     if (SkTextBlob::kDefault_Positioning == run->positioning()) {
-        run->font().measureText(run->glyphBuffer(),
-                                run->glyphCount() * sizeof(uint16_t),
-                                &runBounds);
+        runBounds = tightRunBounds(*run);
     } else {
-        SkASSERT(SkTextBlob::kFull_Positioning == run->positioning() ||
-                 SkTextBlob::kHorizontal_Positioning == run->positioning());
-
-        SkAutoSTArray<16, SkRect> glyphBounds(run->glyphCount());
-        run->font().getTextWidths(run->glyphBuffer(),
-                                  run->glyphCount() * sizeof(uint16_t),
-                                  NULL,
-                                  glyphBounds.get());
-
-        SkScalar* glyphOffset = run->posBuffer();
-        for (unsigned i = 0; i < run->glyphCount(); ++i) {
-            if (SkTextBlob::kFull_Positioning == run->positioning()) {
-                // [ x, y, x, y... ]
-                glyphBounds[i].offset(glyphOffset[0], glyphOffset[1]);
-                SkASSERT(2 == SkTextBlob::ScalarsPerGlyph(run->positioning()));
-                glyphOffset += 2;
-            } else {
-                // [ x, x, x... ], const y applied by runBounds.offset(run->offset()) later.
-                glyphBounds[i].offset(glyphOffset[0], 0);
-                SkASSERT(1 == SkTextBlob::ScalarsPerGlyph(run->positioning()));
-                glyphOffset += 1;
-            }
-
-            runBounds.join(glyphBounds[i]);
-        }
-
-        SkASSERT((void*)glyphOffset <= SkTextBlob::RunRecord::Next(run));
+        runBounds = conservativeRunBounds(*run);
     }
-
-    runBounds.offset(run->offset());
+#endif
 
     fBounds.join(runBounds);
     fDeferredBounds = false;
 }
 
 void SkTextBlobBuilder::reserve(size_t size) {
     // We don't currently pre-allocate, but maybe someday...
     if (fStorageUsed + size <= fStorageSize) {
         return;
     }
@@ skipping 152 matching lines @@
 
     fStorageUsed = 0;
     fStorageSize = 0;
     fRunCount = 0;
     fLastRun = 0;
     fBounds.setEmpty();
 
     return blob;
 }