[SkTextBlob] Custom run font record

src/core/SkTextBlob.cpp

Index: src/core/SkTextBlob.cpp
diff --git a/src/core/SkTextBlob.cpp b/src/core/SkTextBlob.cpp
index 3ffc41e9ba6677b5eb1904da834a4281b26800f4..bcf91e24afba073b217f34b64cd4a602ed121cf4 100644
--- a/src/core/SkTextBlob.cpp
+++ b/src/core/SkTextBlob.cpp
@@ -11,6 +11,73 @@
 #include "SkTypeface.h"
 #include "SkWriteBuffer.h"
 
+namespace {
+
+// TODO(fmalita): replace with SkFont.
+class RunFont : SkNoncopyable {

[mtklein, 2015/04/09 09:44:19]

Woah, if this adds size, let's not do that.  Let's just hide the copy and
assignment constructors manually.

I've always assumed empty base class optimization would apply for SkNoncopyable.
 It's bizarre that it adds size.

[f(malita), 2015/04/09 13:07:09]

Done.
Yeah, I'd like to understand what's going on. This doesn't make sense to me
either.

[f(malita), 2015/04/09 13:18:31]

OK, I see what's going on: this is triggered by the first member being an
SkAutoTUnref, which itself is derived from SkNoncopyable => it inhibits EBCO.

I can easily move fTypeface around here, but I wonder whether this affects other
Skia classes.

+public:
+    RunFont(const SkPaint& paint)
+        : fTypeface(SkSafeRef(paint.getTypeface()))
+        , fSize(paint.getTextSize())
+        , fScaleX(paint.getTextScaleX())
+        , fSkewX(paint.getTextSkewX())
+        , fHinting(paint.getHinting())
+        , fFlags(paint.getFlags() & kFlagsMask) { }
+
+    void applyToPaint(SkPaint* paint) const {
+        paint->setTextEncoding(SkPaint::kGlyphID_TextEncoding);
+        paint->setTypeface(fTypeface.get());
+        paint->setTextSize(fSize);
+        paint->setTextScaleX(fScaleX);
+        paint->setTextSkewX(fSkewX);
+        paint->setHinting(static_cast<SkPaint::Hinting>(fHinting));
+
+        paint->setFlags((paint->getFlags() & ~kFlagsMask) | fFlags);
+    }
+
+    bool operator==(const RunFont& other) const {
+        return fTypeface == other.fTypeface
+            && fSize == other.fSize
+            && fScaleX == other.fScaleX
+            && fSkewX == other.fSkewX
+            && fHinting == other.fHinting
+            && fFlags == other.fFlags;
+    }
+
+    bool operator!=(const RunFont& other) const {
+        return !(*this == other);
+    }
+private:
+    const static uint32_t kFlagsMask =
+        SkPaint::kAntiAlias_Flag          |
+        SkPaint::kUnderlineText_Flag      |
+        SkPaint::kStrikeThruText_Flag     |
+        SkPaint::kFakeBoldText_Flag       |
+        SkPaint::kLinearText_Flag         |
+        SkPaint::kSubpixelText_Flag       |
+        SkPaint::kDevKernText_Flag        |
+        SkPaint::kLCDRenderText_Flag      |
+        SkPaint::kEmbeddedBitmapText_Flag |
+        SkPaint::kAutoHinting_Flag        |
+        SkPaint::kVerticalText_Flag       |
+        SkPaint::kGenA8FromLCD_Flag       |
+        SkPaint::kDistanceFieldTextTEMP_Flag;
+
+    SkAutoTUnref<SkTypeface> fTypeface;
+    SkScalar                 fSize;
+    SkScalar                 fScaleX;
+    SkScalar                 fSkewX;
+
+    SK_COMPILE_ASSERT(SkPaint::kFull_Hinting < 4, insufficient_hinting_bits);
+    uint32_t                 fHinting : 2;
+    SK_COMPILE_ASSERT((kFlagsMask & 0xffff) == kFlagsMask, insufficient_flags_bits);
+    uint32_t                 fFlags : 16;
+
+    typedef SkNoncopyable INHERITED;
+};
+
+} // anonymous namespace
+
 //
 // Textblob data is laid out into externally-managed storage as follows:
 //
@@ -41,7 +108,7 @@ public:
         return fOffset;
     }
 
-    const SkPaint& font() const {
+    const RunFont& font() const {
         return fFont;
     }
 
@@ -102,7 +169,7 @@ private:
 
     uint32_t         fCount;
     SkPoint          fOffset;
-    SkPaint          fFont;
+    RunFont          fFont;
     GlyphPositioning fPositioning;
 
     SkDEBUGCODE(unsigned fMagic;)
@@ -262,29 +329,7 @@ SkTextBlob::GlyphPositioning SkTextBlob::RunIterator::positioning() const {
 void SkTextBlob::RunIterator::applyFontToPaint(SkPaint* paint) const {
     SkASSERT(!this->done());
 
-    const SkPaint& font = fCurrentRun->font();
-
-    paint->setTypeface(font.getTypeface());
-    paint->setTextEncoding(font.getTextEncoding());
-    paint->setTextSize(font.getTextSize());
-    paint->setTextScaleX(font.getTextScaleX());
-    paint->setTextSkewX(font.getTextSkewX());
-    paint->setHinting(font.getHinting());
-
-    uint32_t flagsMask = SkPaint::kAntiAlias_Flag
-                       | SkPaint::kUnderlineText_Flag
-                       | SkPaint::kStrikeThruText_Flag
-                       | SkPaint::kFakeBoldText_Flag
-                       | SkPaint::kLinearText_Flag
-                       | SkPaint::kSubpixelText_Flag
-                       | SkPaint::kDevKernText_Flag
-                       | SkPaint::kLCDRenderText_Flag
-                       | SkPaint::kEmbeddedBitmapText_Flag
-                       | SkPaint::kAutoHinting_Flag
-                       | SkPaint::kVerticalText_Flag
-                       | SkPaint::kGenA8FromLCD_Flag
-                       | SkPaint::kDistanceFieldTextTEMP_Flag;
-    paint->setFlags((paint->getFlags() & ~flagsMask) | (font.getFlags() & flagsMask));
+    fCurrentRun->font().applyToPaint(paint);
 }
 
 SkTextBlobBuilder::SkTextBlobBuilder()
@@ -308,7 +353,9 @@ SkRect SkTextBlobBuilder::TightRunBounds(const SkTextBlob::RunRecord& run) {
     SkASSERT(SkTextBlob::kDefault_Positioning == run.positioning());
 
     SkRect bounds;
-    run.font().measureText(run.glyphBuffer(), run.glyphCount() * sizeof(uint16_t), &bounds);
+    SkPaint paint;
+    run.font().applyToPaint(&paint);
+    paint.measureText(run.glyphBuffer(), run.glyphCount() * sizeof(uint16_t), &bounds);
 
     return bounds.makeOffset(run.offset().x(), run.offset().y());
 }
@@ -346,7 +393,9 @@ SkRect SkTextBlobBuilder::ConservativeRunBounds(const SkTextBlob::RunRecord& run
     }
 
     // Expand by typeface glyph bounds.
-    const SkRect fontBounds = run.font().getFontBounds();
+    SkPaint paint;
+    run.font().applyToPaint(&paint);
+    const SkRect fontBounds = paint.getFontBounds();
     bounds.fLeft   += fontBounds.left();
     bounds.fTop    += fontBounds.top();
     bounds.fRight  += fontBounds.right();
@@ -366,7 +415,6 @@ void SkTextBlobBuilder::updateDeferredBounds() {
     SkASSERT(fLastRun >= sizeof(SkTextBlob));
     SkTextBlob::RunRecord* run = reinterpret_cast<SkTextBlob::RunRecord*>(fStorage.get() +
                                                                           fLastRun);
-    SkASSERT(SkPaint::kGlyphID_TextEncoding == run->font().getTextEncoding());
 
     // FIXME: we should also use conservative bounds for kDefault_Positioning.
     SkRect runBounds = SkTextBlob::kDefault_Positioning == run->positioning() ?