SkPDF: Always get advances at unitsPerEm.

  * Work around BUG=chromium:696356
  * SkTestScalerContext needs a return a em-size.
  * SkPDFFont::MakeVectorCache which always produces a glyph
    cache at emsize.  Replaces vector_cache().
  * Stop looking at fLastGlyphID and fEmSize in TypefaceMetrics.

Change-Id: I28d93b8f62d461a60fa046e9aaf7fa6d116a7ee5
Reviewed-on: https://skia-review.googlesource.com/9324
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Hal Canary <halcanary@google.com>

diff --git a/src/core/SkAdvancedTypefaceMetrics.h b/src/core/SkAdvancedTypefaceMetrics.h
index 17255ab..61f1f1e 100644 (file)
--- a/src/core/SkAdvancedTypefaceMetrics.h
+++ b/src/core/SkAdvancedTypefaceMetrics.h
@@ -26,8 +26,6 @@ public:
     SkAdvancedTypefaceMetrics()
         : fType(SkAdvancedTypefaceMetrics::kOther_Font)
         , fFlags((FontFlags)0)
-        , fLastGlyphID(0)
-        , fEmSize(0)
         , fStyle((StyleFlags)0)
         , fItalicAngle(0)
         , fAscent(0)
@@ -59,9 +57,6 @@ public:
     };
     FontFlags fFlags;  // Global font flags.
 
-    uint16_t fLastGlyphID; // The last valid glyph ID in the font.
-    uint16_t fEmSize;  // The size of the em box (defines font units).
-
     // These enum values match the values used in the PDF file format.
     enum StyleFlags : uint32_t {
         kFixedPitch_Style  = 0x00000001,

diff --git a/src/fonts/SkTestScalerContext.cpp b/src/fonts/SkTestScalerContext.cpp
index 2af8c88..db72637 100644 (file)
--- a/src/fonts/SkTestScalerContext.cpp
+++ b/src/fonts/SkTestScalerContext.cpp
@@ -154,7 +154,6 @@ SkAdvancedTypefaceMetrics* SkTestTypeface::onGetAdvancedTypefaceMetrics(
     SkAdvancedTypefaceMetrics* info = new SkAdvancedTypefaceMetrics;
     info->fFontName.set(fTestFont->fName);
     int glyphCount = this->onCountGlyphs();
-    info->fLastGlyphID = SkToU16(glyphCount - 1);
 
     SkTDArray<SkUnichar>& toUnicode = info->fGlyphToUnicode;
     toUnicode.setCount(glyphCount);

diff --git a/src/fonts/SkTestScalerContext.h b/src/fonts/SkTestScalerContext.h
index 20cce91..5b2ec4f 100644 (file)
--- a/src/fonts/SkTestScalerContext.h
+++ b/src/fonts/SkTestScalerContext.h
@@ -84,8 +84,7 @@ protected:
     }
 
     int onGetUPEM() const override {
-        SkASSERT(0);  // don't expect to get here
-        return 1;
+        return 2048;
     }
 
     void onGetFamilyName(SkString* familyName) const override;

diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index 26dd09f..4e637bf 100644 (file)
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -1307,7 +1307,12 @@ void SkPDFDevice::internalDrawText(
     }
     bool defaultPositioning = (positioning == SkTextBlob::kDefault_Positioning);
     paint.setHinting(SkPaint::kNo_Hinting);
-    SkAutoGlyphCache glyphCache(paint, nullptr, nullptr);
+
+    int emSize;
+    SkAutoGlyphCache glyphCache = SkPDFFont::MakeVectorCache(typeface, &emSize);
+
+    SkScalar textSize = paint.getTextSize();
+    SkScalar advanceScale = textSize * paint.getTextScaleX() / emSize;
 
     SkPaint::Align alignment = paint.getTextAlign();
     float alignmentFactor = SkPaint::kLeft_Align   == alignment ?  0.0f :
@@ -1316,7 +1321,7 @@ void SkPDFDevice::internalDrawText(
     if (defaultPositioning && alignment != SkPaint::kLeft_Align) {
         SkScalar advance = 0;
         for (int i = 0; i < glyphCount; ++i) {
-            advance += glyphCache->getGlyphIDAdvance(glyphs[i]).fAdvanceX;
+            advance += advanceScale * glyphCache->getGlyphIDAdvance(glyphs[i]).fAdvanceX;
         }
         offset.offset(alignmentFactor * advance, 0);
     }
@@ -1325,13 +1330,13 @@ void SkPDFDevice::internalDrawText(
         return;
     }
     SkDynamicMemoryWStream* out = &content.entry()->fContent;
-    SkScalar textSize = paint.getTextSize();
     const SkTDArray<SkUnichar>& glyphToUnicode = metrics->fGlyphToUnicode;
 
     out->writeText("BT\n");
     SK_AT_SCOPE_EXIT(out->writeText("ET\n"));
 
-    const SkGlyphID maxGlyphID = metrics->fLastGlyphID;
+    const SkGlyphID maxGlyphID = SkToU16(typeface->countGlyphs() - 1);
+
     bool multiByteGlyphs = SkPDFFont::IsMultiByte(SkPDFFont::FontType(*metrics));
     if (clusterator.reversedChars()) {
         out->writeText("/ReversedChars BMC\n");
@@ -1404,7 +1409,7 @@ void SkPDFDevice::internalDrawText(
             SkPoint xy{0, 0};
             SkScalar advance{0};
             if (!defaultPositioning) {
-                advance = glyphCache->getGlyphIDAdvance(gid).fAdvanceX;
+                advance = advanceScale * glyphCache->getGlyphIDAdvance(gid).fAdvanceX;
                 xy = SkTextBlob::kFull_Positioning == positioning
                    ? SkPoint{pos[2 * index], pos[2 * index + 1]}
                    : SkPoint{pos[index], 0};

diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index 93ea69b..41b0d4d 100644 (file)
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -25,6 +25,24 @@
     #include "sample/chromium/font_subsetter.h"
 #endif
 
+SkAutoGlyphCache SkPDFFont::MakeVectorCache(SkTypeface* face, int* size) {
+    SkPaint tmpPaint;
+    tmpPaint.setHinting(SkPaint::kNo_Hinting);
+    tmpPaint.setTypeface(sk_ref_sp(face));
+    int unitsPerEm = face->getUnitsPerEm();
+    if (unitsPerEm <= 0) {
+        unitsPerEm = 1024;
+    }
+    if (size) {
+        *size = unitsPerEm;
+    }
+    tmpPaint.setTextSize((SkScalar)unitsPerEm);
+    const SkSurfaceProps props(0, kUnknown_SkPixelGeometry);
+    SkAutoGlyphCache glyphCache(tmpPaint, &props, nullptr);
+    SkASSERT(glyphCache.get());
+    return glyphCache;
+}
+
 namespace {
 // PDF's notion of symbolic vs non-symbolic is related to the character set, not
 // symbols vs. characters.  Rarely is a font the right character set to call it
@@ -58,22 +76,6 @@ struct SkPDFType3Font final : public SkPDFFont {
 // File-Local Functions
 ///////////////////////////////////////////////////////////////////////////////
 
-static SkAutoGlyphCache vector_cache(SkTypeface* face, SkScalar size = 0) {
-    SkPaint tmpPaint;
-    tmpPaint.setHinting(SkPaint::kNo_Hinting);
-    tmpPaint.setTypeface(sk_ref_sp(face));
-    if (0 == size) {
-        SkASSERT(face);
-        tmpPaint.setTextSize((SkScalar)face->getUnitsPerEm());
-    } else {
-        tmpPaint.setTextSize(size);
-    }
-    const SkSurfaceProps props(0, kUnknown_SkPixelGeometry);
-    SkAutoGlyphCache glyphCache(tmpPaint, &props, nullptr);
-    SkASSERT(glyphCache.get());
-    return glyphCache;
-}
-
 // scale from em-units to base-1000, returning as a SkScalar
 SkScalar from_font_units(SkScalar scaled, uint16_t emSize) {
     if (emSize == 1000) {
@@ -155,9 +157,7 @@ const SkAdvancedTypefaceMetrics* SkPDFFont::GetMetrics(SkTypeface* typeface,
                     nullptr, 0));
     if (!metrics) {
         metrics = sk_make_sp<SkAdvancedTypefaceMetrics>();
-        metrics->fLastGlyphID = SkToU16(count - 1);
     }
-    SkASSERT(metrics->fLastGlyphID == SkToU16(count - 1));
     return *canon->fTypefaceMetrics.set(id, metrics.release());
 }
 
@@ -197,8 +197,7 @@ SkPDFFont* SkPDFFont::GetFontResource(SkPDFCanon* canon,
     sk_sp<SkTypeface> typeface(sk_ref_sp(face));
     SkASSERT(typeface);
 
-    SkGlyphID lastGlyph = metrics.fLastGlyphID;
-    SkASSERT(typeface->countGlyphs() == SkToInt(1 + metrics.fLastGlyphID));
+    SkGlyphID lastGlyph = SkToU16(typeface->countGlyphs() - 1);
 
     // should be caught by SkPDFDevice::internalDrawText
     SkASSERT(glyphID <= lastGlyph);
@@ -244,8 +243,8 @@ SkPDFFont::SkPDFFont(SkPDFFont::Info info)
 
 static void  add_common_font_descriptor_entries(SkPDFDict* descriptor,
                                                 const SkAdvancedTypefaceMetrics& metrics,
+                                                uint16_t emSize,
                                                 int16_t defaultWidth) {
-    const uint16_t emSize = metrics.fEmSize;
     descriptor->insertName("FontName", metrics.fFontName);
     descriptor->insertInt("Flags", (size_t)(metrics.fStyle | kPdfSymbolic));
     descriptor->insertScalar("Ascent",
@@ -258,7 +257,7 @@ static void  add_common_font_descriptor_entries(SkPDFDict* descriptor,
             scaleFromFontUnits(metrics.fCapHeight, emSize));
     descriptor->insertInt("ItalicAngle", metrics.fItalicAngle);
     descriptor->insertObject(
-            "FontBBox", makeFontBBox(metrics.fBBox, metrics.fEmSize));
+            "FontBBox", makeFontBBox(metrics.fBBox, emSize));
     if (defaultWidth > 0) {
         descriptor->insertScalar("MissingWidth",
                 scaleFromFontUnits(defaultWidth, emSize));
@@ -364,7 +363,8 @@ void SkPDFType0Font::getFontSubset(SkPDFCanon* canon) {
     SkASSERT(face);
 
     auto descriptor = sk_make_sp<SkPDFDict>("FontDescriptor");
-    add_common_font_descriptor_entries(descriptor.get(), metrics, 0);
+    uint16_t emSize = SkToU16(this->typeface()->getUnitsPerEm());
+    add_common_font_descriptor_entries(descriptor.get(), metrics, emSize , 0);
 
     int ttcIndex;
     std::unique_ptr<SkStreamAsset> fontAsset(face->openStream(&ttcIndex));
@@ -430,17 +430,17 @@ void SkPDFType0Font::getFontSubset(SkPDFCanon* canon) {
     sysInfo->insertInt("Supplement", 0);
     newCIDFont->insertObject("CIDSystemInfo", std::move(sysInfo));
 
-    uint16_t emSize = metrics.fEmSize;
     int16_t defaultWidth = 0;
     {
-        SkAutoGlyphCache glyphCache = vector_cache(face);
+        int emSize;
+        SkAutoGlyphCache glyphCache = SkPDFFont::MakeVectorCache(face, &emSize);
         sk_sp<SkPDFArray> widths = SkPDFMakeCIDGlyphWidthsArray(
-                glyphCache.get(), &this->glyphUsage(), emSize, &defaultWidth);
+                glyphCache.get(), &this->glyphUsage(), SkToS16(emSize), &defaultWidth);
         if (widths && widths->size() > 0) {
             newCIDFont->insertObject("W", std::move(widths));
         }
         newCIDFont->insertScalar(
-                "DW", scaleFromFontUnits(defaultWidth, emSize));
+                "DW", scaleFromFontUnits(defaultWidth, SkToS16(emSize)));
     }
 
     ////////////////////////////////////////////////////////////////////////////
@@ -472,7 +472,8 @@ static sk_sp<SkPDFDict> make_type1_font_descriptor(
         SkTypeface* typeface,
         const SkAdvancedTypefaceMetrics& info) {
     auto descriptor = sk_make_sp<SkPDFDict>("FontDescriptor");
-    add_common_font_descriptor_entries(descriptor.get(), info, 0);
+    uint16_t emSize = SkToU16(typeface->getUnitsPerEm());
+    add_common_font_descriptor_entries(descriptor.get(), info, emSize, 0);
     if (!can_embed(info)) {
         return descriptor;
     }
@@ -507,14 +508,14 @@ static void populate_type_1_font(SkPDFDict* font,
     font->insertInt("FirstChar", (size_t)0);
     font->insertInt("LastChar", (size_t)glyphCount);
     {
-        SkAutoGlyphCache glyphCache = vector_cache(typeface);
+        int emSize;
+        SkAutoGlyphCache glyphCache = SkPDFFont::MakeVectorCache(typeface, &emSize);
         auto widths = sk_make_sp<SkPDFArray>();
         SkScalar advance = glyphCache->getGlyphIDAdvance(0).fAdvanceX;
-        const uint16_t emSize = info.fEmSize;
-        widths->appendScalar(from_font_units(advance, emSize));
+        widths->appendScalar(from_font_units(advance, SkToU16(emSize)));
         for (unsigned gID = firstGlyphID; gID <= lastGlyphID; gID++) {
             advance = glyphCache->getGlyphIDAdvance(gID).fAdvanceX;
-            widths->appendScalar(from_font_units(advance, emSize));
+            widths->appendScalar(from_font_units(advance, SkToU16(emSize)));
         }
         font->insertObject("Widths", std::move(widths));
     }
@@ -592,7 +593,6 @@ private:
 static void add_type3_font_info(SkPDFCanon* canon,
                                 SkPDFDict* font,
                                 SkTypeface* typeface,
-                                SkScalar emSize,
                                 const SkBitSet& subset,
                                 SkGlyphID firstGlyphID,
                                 SkGlyphID lastGlyphID) {
@@ -603,8 +603,9 @@ static void add_type3_font_info(SkPDFCanon* canon,
     while (lastGlyphID > firstGlyphID && !subset.has(lastGlyphID)) {
         --lastGlyphID;
     }
-    SkASSERT(emSize > 0.0f);
-    SkAutoGlyphCache cache = vector_cache(typeface, emSize);
+    int unitsPerEm;
+    SkAutoGlyphCache cache = SkPDFFont::MakeVectorCache(typeface, &unitsPerEm);
+    SkScalar emSize = (SkScalar)unitsPerEm;
     font->insertName("Subtype", "Type3");
     // Flip about the x-axis and scale by 1/emSize.
     SkMatrix fontMatrix;
@@ -712,12 +713,7 @@ SkPDFType3Font::SkPDFType3Font(SkPDFFont::Info info,
     : SkPDFFont(std::move(info)) {}
 
 void SkPDFType3Font::getFontSubset(SkPDFCanon* canon) {
-    const SkAdvancedTypefaceMetrics* info =
-        SkPDFFont::GetMetrics(this->typeface(), canon);
-    SkASSERT(info);
-    uint16_t emSize = info->fEmSize > 0 ? info->fEmSize : 1000;
-    add_type3_font_info(canon, this, this->typeface(), (SkScalar)emSize,
-                        this->glyphUsage(),
+    add_type3_font_info(canon, this, this->typeface(), this->glyphUsage(),
                         this->firstGlyphID(), this->lastGlyphID());
 }
 

diff --git a/src/pdf/SkPDFFont.h b/src/pdf/SkPDFFont.h
index a14ae63..3858b64 100644 (file)
--- a/src/pdf/SkPDFFont.h
+++ b/src/pdf/SkPDFFont.h
@@ -15,6 +15,7 @@
 #include "SkTDArray.h"
 #include "SkTypeface.h"
 
+class SkAutoGlyphCache;
 class SkPDFCanon;
 class SkPDFFont;
 
@@ -47,6 +48,8 @@ public:
                type == SkAdvancedTypefaceMetrics::kTrueType_Font;
     }
 
+    static SkAutoGlyphCache MakeVectorCache(SkTypeface*, int* sizeOut);
+
     /** Returns true if this font encoding supports glyph IDs above 255.
      */
     bool multiByteGlyphs() const { return SkPDFFont::IsMultiByte(this->getType()); }

diff --git a/src/ports/SkFontHost_FreeType.cpp b/src/ports/SkFontHost_FreeType.cpp
index 1c228ea..eee76d5 100644 (file)
--- a/src/ports/SkFontHost_FreeType.cpp
+++ b/src/ports/SkFontHost_FreeType.cpp
@@ -560,8 +560,6 @@ SkAdvancedTypefaceMetrics* SkTypeface_FreeType::onGetAdvancedTypefaceMetrics(
     if (!canSubset(face)) {
         info->fFlags |= SkAdvancedTypefaceMetrics::kNotSubsettable_FontFlag;
     }
-    info->fLastGlyphID = face->num_glyphs - 1;
-    info->fEmSize = 1000;
 
     const char* fontType = FT_Get_X11_Font_Format(face);
     if (strcmp(fontType, "Type 1") == 0) {
@@ -572,10 +570,6 @@ SkAdvancedTypefaceMetrics* SkTypeface_FreeType::onGetAdvancedTypefaceMetrics(
         info->fType = SkAdvancedTypefaceMetrics::kCFF_Font;
     } else if (strcmp(fontType, "TrueType") == 0) {
         info->fType = SkAdvancedTypefaceMetrics::kTrueType_Font;
-        TT_Header* ttHeader;
-        if ((ttHeader = (TT_Header*)FT_Get_Sfnt_Table(face, ft_sfnt_head)) != nullptr) {
-            info->fEmSize = ttHeader->Units_Per_EM;
-        }
     } else {
         info->fType = SkAdvancedTypefaceMetrics::kOther_Font;
     }

diff --git a/src/ports/SkFontHost_mac.cpp b/src/ports/SkFontHost_mac.cpp
index 21017e1..a9b8ccd 100644 (file)
--- a/src/ports/SkFontHost_mac.cpp
+++ b/src/ports/SkFontHost_mac.cpp
@@ -1501,8 +1501,6 @@ SkAdvancedTypefaceMetrics* SkTypeface_Mac::onGetAdvancedTypefaceMetrics(
     }
 
     CFIndex glyphCount = CTFontGetGlyphCount(ctFont.get());
-    info->fLastGlyphID = SkToU16(glyphCount - 1);
-    info->fEmSize = CTFontGetUnitsPerEm(ctFont.get());
 
     if (perGlyphInfo & kToUnicode_PerGlyphInfo) {
         populate_glyph_to_unicode(ctFont.get(), glyphCount, &info->fGlyphToUnicode);

diff --git a/src/ports/SkFontHost_win.cpp b/src/ports/SkFontHost_win.cpp
index fbcd01a..296b9d3 100644 (file)
--- a/src/ports/SkFontHost_win.cpp
+++ b/src/ports/SkFontHost_win.cpp
@@ -1761,8 +1761,6 @@ SkAdvancedTypefaceMetrics* LogFontTypeface::onGetAdvancedTypefaceMetrics(
     glyphCount = calculateGlyphCount(hdc, fLogFont);
 
     info = new SkAdvancedTypefaceMetrics;
-    info->fEmSize = otm.otmEMSquare;
-    info->fLastGlyphID = SkToU16(glyphCount - 1);
     tchar_to_skstring(lf.lfFaceName, &info->fFontName);
     // If bit 1 is set, the font may not be embedded in a document.
     // If bit 1 is clear, the font can be embedded.

diff --git a/src/ports/SkTypeface_win_dw.cpp b/src/ports/SkTypeface_win_dw.cpp
index e955665..3acdd21 100644 (file)
--- a/src/ports/SkTypeface_win_dw.cpp
+++ b/src/ports/SkTypeface_win_dw.cpp
@@ -328,8 +328,6 @@ SkAdvancedTypefaceMetrics* DWriteFontTypeface::onGetAdvancedTypefaceMetrics(
     fDWriteFontFace->GetMetrics(&dwfm);
 
     info = new SkAdvancedTypefaceMetrics;
-    info->fEmSize = dwfm.designUnitsPerEm;
-    info->fLastGlyphID = SkToU16(glyphCount - 1);
 
     info->fAscent = SkToS16(dwfm.ascent);
     info->fDescent = SkToS16(dwfm.descent);