SkPDF: Glyph validation change

src/pdf/SkPDFFont.cpp

Index: src/pdf/SkPDFFont.cpp
diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index 389df62fd5da133041c66a5d8f81a446ad3cabf5..82ca0b7e6b2bd2eb554c99cee774345fd91d3d9a 100644
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -18,7 +18,6 @@
 #include "SkRefCnt.h"
 #include "SkScalar.h"
 #include "SkStream.h"
-#include "SkTypefacePriv.h"
 #include "SkTypes.h"
 #include "SkUtils.h"
 
@@ -148,40 +147,7 @@ static bool can_subset(const SkAdvancedTypefaceMetrics& metrics) {
 }
 #endif
 
-int SkPDFFont::glyphsToPDFFontEncoding(SkGlyphID* glyphIDs, int numGlyphs) const {
-    // A font with multibyte glyphs will support all glyph IDs in a single font.
-    if (this->multiByteGlyphs()) {
-        return numGlyphs;
-    }
-
-    for (int i = 0; i < numGlyphs; i++) {
-        if (glyphIDs[i] == 0) {
-            continue;
-        }
-        if (glyphIDs[i] < fFirstGlyphID || glyphIDs[i] > fLastGlyphID) {
-            return i;
-        }
-        glyphIDs[i] -= (fFirstGlyphID - 1);
-    }
-
-    return numGlyphs;
-}
-
-int SkPDFFont::glyphsToPDFFontEncodingCount(const SkGlyphID* glyphIDs,
-                                            int numGlyphs) const {
-    if (this->multiByteGlyphs()) {    // A font with multibyte glyphs will
-        return numGlyphs;             // support all glyph IDs in a single font.
-    }
-    for (int i = 0; i < numGlyphs; i++) {
-        if (glyphIDs[i] != 0 &&
-            (glyphIDs[i] < fFirstGlyphID || glyphIDs[i] > fLastGlyphID)) {
-            return i;
-        }
-    }
-    return numGlyphs;
-}
-
-
+// returns null only when typeface is bad

[bungeman-skia, 2016/08/25 19:28:44]

/** Returns .... */

[hal.canary, 2016/08/26 15:27:04]

Done.

 const SkAdvancedTypefaceMetrics* SkPDFFont::GetMetrics(SkTypeface* typeface,
                                                        SkPDFCanon* canon) {
     SkFontID id = SkTypeface::UniqueID(typeface);
@@ -189,20 +155,22 @@ const SkAdvancedTypefaceMetrics* SkPDFFont::GetMetrics(SkTypeface* typeface,
         return *ptr;
     }
     sk_sp<SkTypeface> defaultFace;

[bungeman-skia, 2016/08/25 19:28:43]

not used anymore?

[hal.canary, 2016/08/26 15:27:04]

Done.

-    if (!typeface) {
-        defaultFace = SkTypeface::MakeDefault();
-        typeface = defaultFace.get();
-    }
-    sk_sp<SkAdvancedTypefaceMetrics> metrics(
-            typeface->getAdvancedTypefaceMetrics(
-                    SkTypeface::kGlyphNames_PerGlyphInfo | SkTypeface::kToUnicode_PerGlyphInfo,
-                    nullptr, 0));
-    if (!metrics) {
-        if (typeface->countGlyphs() > 0) {
+    SkASSERT(typeface);
+    sk_sp<SkAdvancedTypefaceMetrics> metrics;
+    SkPDFFont::TypefaceInfo faceInfo = SkPDFFont::GetTypefaceInfo(typeface);
+    if (faceInfo.fGood) {
+        metrics.reset(
+                typeface->getAdvancedTypefaceMetrics(
+                        SkTypeface::kGlyphNames_PerGlyphInfo | SkTypeface::kToUnicode_PerGlyphInfo,
+                        nullptr, 0));
+        if (!metrics) {
             metrics = sk_make_sp<SkAdvancedTypefaceMetrics>();
+            metrics->fLastGlyphID = faceInfo.fMaxGlyphID;
         } else {
-            SkDEBUGF(("SkPDF: SkTypeface:getAdvancedTypefaceMetrics() returned null.\n"));
+            SkASSERT(metrics->fLastGlyphID == faceInfo.fMaxGlyphID);
         }
+    } else {
+        SkDEBUGF(("SkPDF: SkTypeface:countGlyphs() bad value.\n"));
     }
     // May cache null to skip this check.  use SkSafeUnref.
     return *canon->fTypefaceMetrics.set(id, metrics.release());
@@ -216,7 +184,7 @@ SkAdvancedTypefaceMetrics::FontType font_type(const SkAdvancedTypefaceMetrics& m
     return metrics.fType;
 }
 
-static SkGlyphID first_glyph_for_single_byte_encoding(SkGlyphID gid) {
+static SkGlyphID first_nonzero_glyph_for_single_byte_encoding(SkGlyphID gid) {
     return gid != 0 ? gid - (gid - 1) % 255 : 1;
 }
 
@@ -224,40 +192,38 @@ SkPDFFont* SkPDFFont::GetFontResource(SkPDFCanon* canon,
                                       SkTypeface* face,
                                       SkGlyphID glyphID) {
     SkASSERT(canon);
+    SkASSERT(face);  // All SkPDFDevice::internalDrawText ensures this.
     const SkAdvancedTypefaceMetrics* fontMetrics = SkPDFFont::GetMetrics(face, canon);
-    if (!fontMetrics) {
-        return nullptr;  // bad font, return early.
-    }
+    SkASSERT(fontMetrics);  // SkPDFDevice::internalDrawText ensures the typeface is good.
+                            // GetMetrics only returns null to signify a bad typeface.
     const SkAdvancedTypefaceMetrics& metrics = *fontMetrics;
     SkAdvancedTypefaceMetrics::FontType type = font_type(metrics);
     bool multibyte = SkPDFFont::IsMultiByte(type);
-    SkGlyphID firstGlyph = multibyte ? 0 : first_glyph_for_single_byte_encoding(glyphID);
-    uint64_t fontID = (SkTypeface::UniqueID(face) << 16) | firstGlyph;
+    SkGlyphID subsetCode = multibyte ? 0 : first_nonzero_glyph_for_single_byte_encoding(glyphID);
+    uint64_t fontID = (SkTypeface::UniqueID(face) << 16) | subsetCode;
 
     if (SkPDFFont** found = canon->fFontMap.find(fontID)) {
-        SkASSERT(multibyte == (*found)->multiByteGlyphs());
-        return SkRef(*found);
+        SkPDFFont* foundFont = *found;
+        SkASSERT(foundFont && multibyte == foundFont->multiByteGlyphs());
+        return SkRef(foundFont);
     }
 
-    sk_sp<SkTypeface> typeface(face ? sk_ref_sp(face) : SkTypeface::MakeDefault());
+    sk_sp<SkTypeface> typeface(sk_ref_sp(face));
     SkASSERT(typeface);
-    int glyphCount = typeface->countGlyphs();
-    // Validate typeface + glyph;
-    if (glyphCount < 1 ||  // typeface lacks even a NOTDEF glyph.
-        glyphCount > 1 + SK_MaxU16 ||  // invalid glyphCount
-        glyphID >= glyphCount) {  // invalid glyph
-        return nullptr;
-    }
+
+    SkGlyphID lastGlyph = metrics.fLastGlyphID;
+    SkASSERT(typeface->countGlyphs() == SkToInt(1 + metrics.fLastGlyphID));
+
+    // should be caught by SkPDFDevice::internalDrawText
+    SkASSERT(glyphID <= lastGlyph);
 
     SkGlyphID firstNonZeroGlyph;
-    SkGlyphID lastGlyph;
     if (multibyte) {
         firstNonZeroGlyph = 1;
-        lastGlyph = SkToU16(glyphCount - 1);
     } else {
-        firstNonZeroGlyph = firstGlyph;
-        lastGlyph = SkToU16(SkTMin<int>(glyphCount - 1,
-                                        254 + (int)firstGlyph));
+        firstNonZeroGlyph = subsetCode;
+        lastGlyph = SkToU16(SkTMin<int>((int)lastGlyph,
+                                        254 + (int)subsetCode));

[bungeman-skia, 2016/08/25 19:28:43]

maybe fits on one line now?

[hal.canary, 2016/08/26 15:27:03]

Done.

     }
     SkPDFFont::Info info = {std::move(typeface), firstNonZeroGlyph, lastGlyph, type};
     sk_sp<SkPDFFont> font;