Clean up glyph id handling.

Extract SkPackedID and its strongly typed subclasses SkPackedGlyphID and
SkPackedUnicharID out of SkGlyph. This simplifies the code handling
these types, as well as making it clearer that we wouuld eventually like
to get away from this scheme.

Changes SkScalerContext::getPath to take SkPackedGlyphID.
Changes SkScalerContext::generatePath to take SkGlyphID.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4722

Change-Id: I365c0c618b7ae0d348272155fac7761a69faa920
Reviewed-on: https://skia-review.googlesource.com/4722
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>

15 files changed, 196 insertions, 178 deletions

diff --git a/src/core/SkGlyph.h b/src/core/SkGlyph.h
index 04f9296b55..73f8704f64 100644
--- a/src/core/SkGlyph.h
+++ b/src/core/SkGlyph.h
@@ -22,8 +22,9 @@ class SkGlyphCache;
 
 #define kMaxGlyphWidth (1<<13)
 
-SK_BEGIN_REQUIRE_DENSE
-class SkGlyph {
+/** (glyph-index or unicode-point) + subpixel-pos */
+struct SkPackedID {
+    static constexpr uint32_t kImpossibleID = ~0;
     enum {
         kSubBits = 2,
         kSubMask = ((1 << kSubBits) - 1),
@@ -34,6 +35,95 @@ class SkGlyph {
         kSubShiftY = 0
     };
 
+    SkPackedID(uint32_t code) {
+        SkASSERT(code <= kCodeMask);
+        SkASSERT(code != kImpossibleID);
+        fID = code;
+    }
+
+    SkPackedID(uint32_t code, SkFixed x, SkFixed y) {
+        SkASSERT(code <= kCodeMask);
+        x = FixedToSub(x);
+        y = FixedToSub(y);
+        uint32_t ID = (x << (kSubShift + kSubShiftX)) |
+                      (y << (kSubShift + kSubShiftY)) |
+                      code;
+        SkASSERT(ID != kImpossibleID);
+        fID = ID;
+    }
+
+    constexpr SkPackedID() : fID(kImpossibleID) {}
+
+    bool operator==(const SkPackedID& that) const {
+        return fID == that.fID;
+    }
+    bool operator!=(const SkPackedID& that) const {
+        return !(*this == that);
+    }
+
+    uint32_t code() const {
+        return fID & kCodeMask;
+    }
+
+    SkFixed getSubXFixed() const {
+        return SubToFixed(ID2SubX(fID));
+    }
+
+    SkFixed getSubYFixed() const {
+        return SubToFixed(ID2SubY(fID));
+    }
+
+    uint32_t hash() const {
+        return SkChecksum::CheapMix(fID);
+    }
+
+// FIXME - This is needed because the Android framework directly accesses fID.
+// Remove when fID accesses are cleaned up.
+#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
+    operator uint32_t() const { return fID; }
+#endif
+
+private:
+    static unsigned ID2SubX(uint32_t id) {
+        return id >> (kSubShift + kSubShiftX);
+    }
+
+    static unsigned ID2SubY(uint32_t id) {
+        return (id >> (kSubShift + kSubShiftY)) & kSubMask;
+    }
+
+    static unsigned FixedToSub(SkFixed n) {
+        return (n >> (16 - kSubBits)) & kSubMask;
+    }
+
+    static SkFixed SubToFixed(unsigned sub) {
+        SkASSERT(sub <= kSubMask);
+        return sub << (16 - kSubBits);
+    }
+
+    uint32_t fID;
+};
+
+struct SkPackedGlyphID : public SkPackedID {
+    SkPackedGlyphID(SkGlyphID code) : SkPackedID(code) { }
+    SkPackedGlyphID(SkGlyphID code, SkFixed x, SkFixed y) : SkPackedID(code, x, y) { }
+    SkPackedGlyphID() : SkPackedID() { }
+    SkGlyphID code() const {
+        return SkTo<SkGlyphID>(SkPackedID::code());
+    }
+};
+
+struct SkPackedUnicharID : public SkPackedID {
+    SkPackedUnicharID(SkUnichar code) : SkPackedID(code) { }
+    SkPackedUnicharID(SkUnichar code, SkFixed x, SkFixed y) : SkPackedID(code, x, y) { }
+    SkPackedUnicharID() : SkPackedID() { }
+    SkUnichar code() const {
+        return SkTo<SkUnichar>(SkPackedID::code());
+    }
+};
+
+SK_BEGIN_REQUIRE_DENSE
+class SkGlyph {
     // Support horizontal and vertical skipping strike-through / underlines.
     // The caller walks the linked list looking for a match. For a horizontal underline,
     // the fBounds contains the top and bottom of the underline. The fInterval pair contains the
@@ -51,9 +141,7 @@ class SkGlyph {
     };
 
 public:
-    static const SkFixed kSubpixelRound = SK_FixedHalf >> SkGlyph::kSubBits;
-    // A value that can never be generated by MakeID.
-    static const uint32_t kImpossibleID = ~0;
+    static const SkFixed kSubpixelRound = SK_FixedHalf >> SkPackedID::kSubBits;
     void*       fImage;
     PathData*   fPathData;
     float       fAdvanceX, fAdvanceY;
@@ -65,16 +153,12 @@ public:
     int8_t      fRsbDelta, fLsbDelta;  // used by auto-kerning
     int8_t      fForceBW;
 
-    void initWithGlyphID(uint32_t glyph_id) {
-        this->initCommon(MakeID(glyph_id));
-    }
-
-    void initGlyphIdFrom(const SkGlyph& glyph) {
-        this->initCommon(glyph.fID);
-    }
-
-    void initGlyphFromCombinedID(uint32_t combined_id) {
-      this->initCommon(combined_id);
+    void initWithGlyphID(SkPackedGlyphID glyph_id) {
+        fID             = glyph_id;
+        fImage          = nullptr;
+        fPathData       = nullptr;
+        fMaskFormat     = MASK_FORMAT_UNKNOWN;
+        fForceBW        = 0;
     }
 
     /**
@@ -106,20 +190,20 @@ public:
         return MASK_FORMAT_JUST_ADVANCE != fMaskFormat;
     }
 
-    uint16_t getGlyphID() const {
-        return ID2Code(fID);
+    SkGlyphID getGlyphID() const {
+        return fID.code();
     }
 
-    unsigned getSubX() const {
-        return ID2SubX(fID);
+    SkPackedGlyphID getPackedID() const {
+        return fID;
     }
 
     SkFixed getSubXFixed() const {
-        return SubToFixed(ID2SubX(fID));
+        return fID.getSubXFixed();
     }
 
     SkFixed getSubYFixed() const {
-        return SubToFixed(ID2SubY(fID));
+        return fID.getSubYFixed();
     }
 
     size_t computeImageSize() const;
@@ -134,11 +218,11 @@ public:
 
     class HashTraits {
     public:
-        static uint32_t GetKey(const SkGlyph& glyph) {
+        static SkPackedGlyphID GetKey(const SkGlyph& glyph) {
             return glyph.fID;
         }
-        static uint32_t Hash(uint32_t glyphId) {
-            return SkChecksum::CheapMix(glyphId);
+        static uint32_t Hash(SkPackedGlyphID glyphId) {
+            return glyphId.hash();
         }
     };
 
@@ -146,58 +230,12 @@ public:
     // TODO(herb) remove friend statement after SkGlyphCache cleanup.
     friend class SkGlyphCache;
 
-    void initCommon(uint32_t id) {
-        fID             = id;
-        fImage          = nullptr;
-        fPathData       = nullptr;
-        fMaskFormat     = MASK_FORMAT_UNKNOWN;
-        fForceBW        = 0;
-    }
-
-    static unsigned ID2Code(uint32_t id) {
-        return id & kCodeMask;
-    }
-
-    static unsigned ID2SubX(uint32_t id) {
-        return id >> (kSubShift + kSubShiftX);
-    }
-
-    static unsigned ID2SubY(uint32_t id) {
-        return (id >> (kSubShift + kSubShiftY)) & kSubMask;
-    }
-
-    static unsigned FixedToSub(SkFixed n) {
-        return (n >> (16 - kSubBits)) & kSubMask;
-    }
-
-    static SkFixed SubToFixed(unsigned sub) {
-        SkASSERT(sub <= kSubMask);
-        return sub << (16 - kSubBits);
-    }
-
-    static uint32_t MakeID(unsigned code) {
-        SkASSERT(code <= kCodeMask);
-        SkASSERT(code != kImpossibleID);
-        return code;
-    }
-
-    static uint32_t MakeID(unsigned code, SkFixed x, SkFixed y) {
-        SkASSERT(code <= kCodeMask);
-        x = FixedToSub(x);
-        y = FixedToSub(y);
-        uint32_t ID = (x << (kSubShift + kSubShiftX)) |
-                      (y << (kSubShift + kSubShiftY)) |
-                      code;
-        SkASSERT(ID != kImpossibleID);
-        return ID;
-    }
-
-    // FIXME - This is needed because the Android frame work directly
-  // accesses fID. Remove when fID accesses are cleaned up.
+// FIXME - This is needed because the Android frame work directly accesses fID.
+// Remove when fID accesses are cleaned up.
 #ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
   public:
 #endif
-    uint32_t    fID;
+    SkPackedGlyphID fID;
 };
 SK_END_REQUIRE_DENSE
 
diff --git a/src/core/SkGlyphCache.cpp b/src/core/SkGlyphCache.cpp
index 7526bbf256..c1abcd1f26 100644
--- a/src/core/SkGlyphCache.cpp
+++ b/src/core/SkGlyphCache.cpp
@@ -60,19 +60,12 @@ SkGlyphCache::~SkGlyphCache() {
     });
 }
 
-SkGlyphCache::CharGlyphRec* SkGlyphCache::getCharGlyphRec(PackedUnicharID packedUnicharID) {
-    if (nullptr == fPackedUnicharIDToPackedGlyphID.get()) {
-        // Allocate the array.
-        fPackedUnicharIDToPackedGlyphID.reset(kHashCount);
-        // Initialize array to map character and position with the impossible glyph ID. This
-        // represents no mapping.
-        for (int i = 0; i <kHashCount; ++i) {
-            fPackedUnicharIDToPackedGlyphID[i].fPackedUnicharID = SkGlyph::kImpossibleID;
-            fPackedUnicharIDToPackedGlyphID[i].fPackedGlyphID = 0;
-        }
+SkGlyphCache::CharGlyphRec* SkGlyphCache::getCharGlyphRec(SkPackedUnicharID packedUnicharID) {
+    if (!fPackedUnicharIDToPackedGlyphID) {
+        fPackedUnicharIDToPackedGlyphID.reset(new CharGlyphRec[kHashCount]);
     }
 
-    return &fPackedUnicharIDToPackedGlyphID[SkChecksum::CheapMix(packedUnicharID) & kHashMask];
+    return &fPackedUnicharIDToPackedGlyphID[packedUnicharID.hash() & kHashMask];
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -83,24 +76,24 @@ SkGlyphCache::CharGlyphRec* SkGlyphCache::getCharGlyphRec(PackedUnicharID packed
 #define VALIDATE()
 #endif
 
-uint16_t SkGlyphCache::unicharToGlyph(SkUnichar charCode) {
+SkGlyphID SkGlyphCache::unicharToGlyph(SkUnichar charCode) {
     VALIDATE();
-    PackedUnicharID packedUnicharID = SkGlyph::MakeID(charCode);
+    SkPackedUnicharID packedUnicharID(charCode);
     CharGlyphRec* rec = this->getCharGlyphRec(packedUnicharID);
 
     if (rec->fPackedUnicharID == packedUnicharID) {
         // The glyph exists in the unichar to glyph mapping cache. Return it.
-        return SkGlyph::ID2Code(rec->fPackedGlyphID);
+        return rec->fPackedGlyphID.code();
     } else {
         // The glyph is not in the unichar to glyph mapping cache. Insert it.
         rec->fPackedUnicharID = packedUnicharID;
-        uint16_t glyphID = fScalerContext->charToGlyphID(charCode);
-        rec->fPackedGlyphID = SkGlyph::MakeID(glyphID);
+        SkGlyphID glyphID = fScalerContext->charToGlyphID(charCode);
+        rec->fPackedGlyphID = SkPackedGlyphID(glyphID);
         return glyphID;
     }
 }
 
-SkUnichar SkGlyphCache::glyphToUnichar(uint16_t glyphID) {
+SkUnichar SkGlyphCache::glyphToUnichar(SkGlyphID glyphID) {
     return fScalerContext->glyphIDToChar(glyphID);
 }
 
@@ -121,7 +114,7 @@ const SkGlyph& SkGlyphCache::getUnicharAdvance(SkUnichar charCode) {
 
 const SkGlyph& SkGlyphCache::getGlyphIDAdvance(uint16_t glyphID) {
     VALIDATE();
-    PackedGlyphID packedGlyphID = SkGlyph::MakeID(glyphID);
+    SkPackedGlyphID packedGlyphID(glyphID);
     return *this->lookupByPackedGlyphID(packedGlyphID, kJustAdvance_MetricsType);
 }
 
@@ -139,32 +132,27 @@ const SkGlyph& SkGlyphCache::getUnicharMetrics(SkUnichar charCode, SkFixed x, Sk
 
 const SkGlyph& SkGlyphCache::getGlyphIDMetrics(uint16_t glyphID) {
     VALIDATE();
-    PackedGlyphID packedGlyphID = SkGlyph::MakeID(glyphID);
+    SkPackedGlyphID packedGlyphID(glyphID);
     return *this->lookupByPackedGlyphID(packedGlyphID, kFull_MetricsType);
 }
 
 const SkGlyph& SkGlyphCache::getGlyphIDMetrics(uint16_t glyphID, SkFixed x, SkFixed y) {
     VALIDATE();
-    PackedGlyphID packedGlyphID = SkGlyph::MakeID(glyphID, x, y);
+    SkPackedGlyphID packedGlyphID(glyphID, x, y);
     return *this->lookupByPackedGlyphID(packedGlyphID, kFull_MetricsType);
 }
 
 SkGlyph* SkGlyphCache::lookupByChar(SkUnichar charCode, MetricsType type, SkFixed x, SkFixed y) {
-    PackedUnicharID id = SkGlyph::MakeID(charCode, x, y);
+    SkPackedUnicharID id(charCode, x, y);
     CharGlyphRec* rec = this->getCharGlyphRec(id);
     if (rec->fPackedUnicharID != id) {
-        // this ID is based on the UniChar
         rec->fPackedUnicharID = id;
-        // this ID is based on the glyph index
-        PackedGlyphID combinedID = SkGlyph::MakeID(fScalerContext->charToGlyphID(charCode), x, y);
-        rec->fPackedGlyphID = combinedID;
-        return this->lookupByPackedGlyphID(combinedID, type);
-    } else {
-        return this->lookupByPackedGlyphID(rec->fPackedGlyphID, type);
+        rec->fPackedGlyphID = SkPackedGlyphID(fScalerContext->charToGlyphID(charCode), x, y);
     }
+    return this->lookupByPackedGlyphID(rec->fPackedGlyphID, type);
 }
 
-SkGlyph* SkGlyphCache::lookupByPackedGlyphID(PackedGlyphID packedGlyphID, MetricsType type) {
+SkGlyph* SkGlyphCache::lookupByPackedGlyphID(SkPackedGlyphID packedGlyphID, MetricsType type) {
     SkGlyph* glyph = fGlyphMap.find(packedGlyphID);
 
     if (nullptr == glyph) {
@@ -177,13 +165,13 @@ SkGlyph* SkGlyphCache::lookupByPackedGlyphID(PackedGlyphID packedGlyphID, Metric
     return glyph;
 }
 
-SkGlyph* SkGlyphCache::allocateNewGlyph(PackedGlyphID packedGlyphID, MetricsType mtype) {
+SkGlyph* SkGlyphCache::allocateNewGlyph(SkPackedGlyphID packedGlyphID, MetricsType mtype) {
     fMemoryUsed += sizeof(SkGlyph);
 
     SkGlyph* glyphPtr;
     {
         SkGlyph glyph;
-        glyph.initGlyphFromCombinedID(packedGlyphID);
+        glyph.initWithGlyphID(packedGlyphID);
         glyphPtr = fGlyphMap.set(glyph);
     }
 
@@ -194,7 +182,7 @@ SkGlyph* SkGlyphCache::allocateNewGlyph(PackedGlyphID packedGlyphID, MetricsType
         fScalerContext->getMetrics(glyphPtr);
     }
 
-    SkASSERT(glyphPtr->fID != SkGlyph::kImpossibleID);
+    SkASSERT(glyphPtr->fID != SkPackedGlyphID());
     return glyphPtr;
 }
 
@@ -226,7 +214,7 @@ const SkPath* SkGlyphCache::findPath(const SkGlyph& glyph) {
             const_cast<SkGlyph&>(glyph).fPathData = pathData;
             pathData->fIntercept = nullptr;
             SkPath* path = pathData->fPath = new SkPath;
-            fScalerContext->getPath(glyph, path);
+            fScalerContext->getPath(glyph.getPackedID(), path);
             fMemoryUsed += sizeof(SkPath) + path->countPoints() * sizeof(SkPoint);
         }
     }
diff --git a/src/core/SkGlyphCache.h b/src/core/SkGlyphCache.h
index 863b2c82fb..8cb5337737 100644
--- a/src/core/SkGlyphCache.h
+++ b/src/core/SkGlyphCache.h
@@ -39,7 +39,7 @@ public:
         getGlyphIDMetrics instead.
     */
     const SkGlyph& getUnicharAdvance(SkUnichar);
-    const SkGlyph& getGlyphIDAdvance(uint16_t);
+    const SkGlyph& getGlyphIDAdvance(SkGlyphID);
 
     /** Returns a glyph with all fields valid except fImage and fPath, which may be null. If they
         are null, call findImage or findPath for those. If they are not null, then they are valid.
@@ -48,7 +48,7 @@ public:
         fAdvance/fDevKern fields, call those instead.
     */
     const SkGlyph& getUnicharMetrics(SkUnichar);
-    const SkGlyph& getGlyphIDMetrics(uint16_t);
+    const SkGlyph& getGlyphIDMetrics(SkGlyphID);
 
     /** These are variants that take the device position of the glyph. Call these only if you are
         drawing in subpixel mode. Passing 0, 0 is effectively the same as calling the variants
@@ -60,11 +60,11 @@ public:
     /** Return the glyphID for the specified Unichar. If the char has already been seen, use the
         existing cache entry. If not, ask the scalercontext to compute it for us.
     */
-    uint16_t unicharToGlyph(SkUnichar);
+    SkGlyphID unicharToGlyph(SkUnichar);
 
     /** Map the glyph to its Unicode equivalent. Unmappable glyphs map to a character code of zero.
     */
-    SkUnichar glyphToUnichar(uint16_t);
+    SkUnichar glyphToUnichar(SkGlyphID);
 
     /** Returns the number of glyphs for this strike.
     */
@@ -187,12 +187,9 @@ private:
         kHashMask           = kHashCount - 1
     };
 
-    typedef uint32_t PackedGlyphID;    // glyph-index + subpixel-pos
-    typedef uint32_t PackedUnicharID;  // unichar + subpixel-pos
-
     struct CharGlyphRec {
-        PackedUnicharID    fPackedUnicharID;
-        PackedGlyphID      fPackedGlyphID;
+        SkPackedUnicharID fPackedUnicharID;
+        SkPackedGlyphID fPackedGlyphID;
     };
 
     SkGlyphCache(const SkDescriptor*, std::unique_ptr<SkScalerContext>);
@@ -201,19 +198,19 @@ private:
     // Return the SkGlyph* associated with MakeID. The id parameter is the
     // combined glyph/x/y id generated by MakeID. If it is just a glyph id
     // then x and y are assumed to be zero.
-    SkGlyph* lookupByPackedGlyphID(PackedGlyphID packedGlyphID, MetricsType type);
+    SkGlyph* lookupByPackedGlyphID(SkPackedGlyphID packedGlyphID, MetricsType type);
 
     // Return a SkGlyph* associated with unicode id and position x and y.
     SkGlyph* lookupByChar(SkUnichar id, MetricsType type, SkFixed x = 0, SkFixed y = 0);
 
     // Return a new SkGlyph for the glyph ID and subpixel position id. Limit the amount
     // of work using type.
-    SkGlyph* allocateNewGlyph(PackedGlyphID packedGlyphID, MetricsType type);
+    SkGlyph* allocateNewGlyph(SkPackedGlyphID packedGlyphID, MetricsType type);
 
     static bool DetachProc(const SkGlyphCache*, void*) { return true; }
 
     // The id arg is a combined id generated by MakeID.
-    CharGlyphRec* getCharGlyphRec(PackedUnicharID id);
+    CharGlyphRec* getCharGlyphRec(SkPackedUnicharID id);
 
     static void OffsetResults(const SkGlyph::Intercept* intercept, SkScalar scale,
                               SkScalar xPos, SkScalar* array, int* count);
@@ -236,11 +233,11 @@ private:
     SkPaint::FontMetrics   fFontMetrics;
 
     // Map from a combined GlyphID and sub-pixel position to a SkGlyph.
-    SkTHashTable<SkGlyph, PackedGlyphID, SkGlyph::HashTraits> fGlyphMap;
+    SkTHashTable<SkGlyph, SkPackedGlyphID, SkGlyph::HashTraits> fGlyphMap;
 
     SkChunkAlloc           fGlyphAlloc;
 
-    SkAutoTArray<CharGlyphRec> fPackedUnicharIDToPackedGlyphID;
+    std::unique_ptr<CharGlyphRec[]> fPackedUnicharIDToPackedGlyphID;
 
     // used to track (approx) how much ram is tied-up in this cache
     size_t                 fMemoryUsed;
diff --git a/src/core/SkScalerContext.cpp b/src/core/SkScalerContext.cpp
index 581e102034..c06222d4d8 100644
--- a/src/core/SkScalerContext.cpp
+++ b/src/core/SkScalerContext.cpp
@@ -131,7 +131,7 @@ void SkScalerContext::getMetrics(SkGlyph* glyph) {
         SkPath      devPath, fillPath;
         SkMatrix    fillToDevMatrix;
 
-        this->internalGetPath(*glyph, &fillPath, &devPath, &fillToDevMatrix);
+        this->internalGetPath(glyph->getPackedID(), &fillPath, &devPath, &fillToDevMatrix);
 
         if (fRasterizer) {
             SkMask  mask;
@@ -462,7 +462,7 @@ void SkScalerContext::getImage(const SkGlyph& origGlyph) {
              SkMask::kARGB32_Format != origGlyph.fMaskFormat);
 
     if (fMaskFilter) {   // restore the prefilter bounds
-        tmpGlyph.initGlyphIdFrom(origGlyph);
+        tmpGlyph.initWithGlyphID(origGlyph.getPackedID());
 
         // need the original bounds, sans our maskfilter
         SkMaskFilter* mf = fMaskFilter.release();   // temp disable
@@ -487,7 +487,7 @@ void SkScalerContext::getImage(const SkGlyph& origGlyph) {
         SkMatrix    fillToDevMatrix;
         SkMask      mask;
 
-        this->internalGetPath(*glyph, &fillPath, &devPath, &fillToDevMatrix);
+        this->internalGetPath(glyph->getPackedID(), &fillPath, &devPath, &fillToDevMatrix);
         glyph->toMask(&mask);
 
         if (fRasterizer) {
@@ -564,8 +564,8 @@ void SkScalerContext::getImage(const SkGlyph& origGlyph) {
     }
 }
 
-void SkScalerContext::getPath(const SkGlyph& glyph, SkPath* path) {
-    this->internalGetPath(glyph, nullptr, path, nullptr);
+void SkScalerContext::getPath(SkPackedGlyphID glyphID, SkPath* path) {
+    this->internalGetPath(glyphID, nullptr, path, nullptr);
 }
 
 void SkScalerContext::getFontMetrics(SkPaint::FontMetrics* fm) {
@@ -578,14 +578,14 @@ SkUnichar SkScalerContext::generateGlyphToChar(uint16_t glyph) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-void SkScalerContext::internalGetPath(const SkGlyph& glyph, SkPath* fillPath,
-                                  SkPath* devPath, SkMatrix* fillToDevMatrix) {
+void SkScalerContext::internalGetPath(SkPackedGlyphID glyphID, SkPath* fillPath,
+                                      SkPath* devPath, SkMatrix* fillToDevMatrix) {
     SkPath  path;
-    generatePath(glyph, &path);
+    generatePath(glyphID.code(), &path);
 
     if (fRec.fFlags & SkScalerContext::kSubpixelPositioning_Flag) {
-        SkFixed dx = glyph.getSubXFixed();
-        SkFixed dy = glyph.getSubYFixed();
+        SkFixed dx = glyphID.getSubXFixed();
+        SkFixed dy = glyphID.getSubYFixed();
         if (dx | dy) {
             path.offset(SkFixedToScalar(dx), SkFixedToScalar(dy));
         }
@@ -850,7 +850,7 @@ protected:
         glyph->zeroMetrics();
     }
     void generateImage(const SkGlyph& glyph) override {}
-    void generatePath(const SkGlyph& glyph, SkPath* path) override {}
+    void generatePath(SkGlyphID glyph, SkPath* path) override {}
     void generateFontMetrics(SkPaint::FontMetrics* metrics) override {
         if (metrics) {
             sk_bzero(metrics, sizeof(*metrics));
diff --git a/src/core/SkScalerContext.h b/src/core/SkScalerContext.h
index 61f94ad006..7bfdb92a52 100644
--- a/src/core/SkScalerContext.h
+++ b/src/core/SkScalerContext.h
@@ -8,13 +8,13 @@
 #ifndef SkScalerContext_DEFINED
 #define SkScalerContext_DEFINED
 
+#include "SkGlyph.h"
 #include "SkMask.h"
 #include "SkMaskGamma.h"
 #include "SkMatrix.h"
 #include "SkPaint.h"
 #include "SkTypeface.h"
 
-class SkGlyph;
 class SkDescriptor;
 class SkMaskFilter;
 class SkPathEffect;
@@ -248,7 +248,7 @@ public:
     void        getAdvance(SkGlyph*);
     void        getMetrics(SkGlyph*);
     void        getImage(const SkGlyph&);
-    void        getPath(const SkGlyph&, SkPath*);
+    void        getPath(SkPackedGlyphID, SkPath*);
     void        getFontMetrics(SkPaint::FontMetrics*);
 
     /** Return the size in bytes of the associated gamma lookup table
@@ -309,11 +309,8 @@ protected:
     /** Sets the passed path to the glyph outline.
      *  If this cannot be done the path is set to empty;
      *  this is indistinguishable from a glyph with an empty path.
-     *  This does not set glyph.fPath.
-     *
-     *  TODO: path is always glyph.fPath, no reason to pass separately.
      */
-    virtual void generatePath(const SkGlyph& glyph, SkPath* path) = 0;
+    virtual void generatePath(SkGlyphID glyphId, SkPath* path) = 0;
 
     /** Retrieves font metrics. */
     virtual void generateFontMetrics(SkPaint::FontMetrics*) = 0;
@@ -350,7 +347,7 @@ private:
     // calling generateImage.
     bool fGenerateImageFromPath;
 
-    void internalGetPath(const SkGlyph& glyph, SkPath* fillPath,
+    void internalGetPath(SkPackedGlyphID id, SkPath* fillPath,
                          SkPath* devPath, SkMatrix* fillToDevMatrix);
 
     // SkMaskGamma::PreBlend converts linear masks to gamma correcting masks.
diff --git a/src/fonts/SkGScalerContext.cpp b/src/fonts/SkGScalerContext.cpp
index 53d385437e..9da05492e6 100644
--- a/src/fonts/SkGScalerContext.cpp
+++ b/src/fonts/SkGScalerContext.cpp
@@ -53,7 +53,7 @@ protected:
     void generateAdvance(SkGlyph*) override;
     void generateMetrics(SkGlyph*) override;
     void generateImage(const SkGlyph&) override;
-    void generatePath(const SkGlyph&, SkPath*) override;
+    void generatePath(SkGlyphID, SkPath*) override;
     void generateFontMetrics(SkPaint::FontMetrics*) override;
 
 private:
@@ -89,7 +89,7 @@ void SkGScalerContext::generateMetrics(SkGlyph* glyph) {
     glyph->fAdvanceY = SkScalarToFloat(advance.fY);
 
     SkPath path;
-    fProxy->getPath(*glyph, &path);
+    fProxy->getPath(glyph->getPackedID(), &path);
     path.transform(fMatrix);
 
     SkRect storage;
@@ -109,7 +109,7 @@ void SkGScalerContext::generateMetrics(SkGlyph* glyph) {
 void SkGScalerContext::generateImage(const SkGlyph& glyph) {
     if (SkMask::kARGB32_Format == glyph.fMaskFormat) {
         SkPath path;
-        fProxy->getPath(glyph, &path);
+        fProxy->getPath(glyph.getPackedID(), &path);
 
         SkBitmap bm;
         bm.installPixels(SkImageInfo::MakeN32Premul(glyph.fWidth, glyph.fHeight),
@@ -126,8 +126,8 @@ void SkGScalerContext::generateImage(const SkGlyph& glyph) {
     }
 }
 
-void SkGScalerContext::generatePath(const SkGlyph& glyph, SkPath* path) {
-    fProxy->getPath(glyph, path);
+void SkGScalerContext::generatePath(SkGlyphID glyph, SkPath* path) {
+    fProxy->getPath(SkPackedGlyphID(glyph), path);
     path->transform(fMatrix);
 }
 
diff --git a/src/fonts/SkRandomScalerContext.cpp b/src/fonts/SkRandomScalerContext.cpp
index 96a2619876..55c7fb30d4 100644
--- a/src/fonts/SkRandomScalerContext.cpp
+++ b/src/fonts/SkRandomScalerContext.cpp
@@ -25,7 +25,7 @@ protected:
     void generateAdvance(SkGlyph*) override;
     void generateMetrics(SkGlyph*) override;
     void generateImage(const SkGlyph&) override;
-    void generatePath(const SkGlyph&, SkPath*) override;
+    void generatePath(SkGlyphID, SkPath*) override;
     void generateFontMetrics(SkPaint::FontMetrics*) override;
 
 private:
@@ -84,7 +84,7 @@ void SkRandomScalerContext::generateMetrics(SkGlyph* glyph) {
     }
     if (SkMask::kARGB32_Format == format) {
         SkPath path;
-        fProxy->getPath(*glyph, &path);
+        fProxy->getPath(glyph->getPackedID(), &path);
 
         SkRect storage;
         const SkPaint& paint = this->getRandomTypeface()->paint();
@@ -101,7 +101,7 @@ void SkRandomScalerContext::generateMetrics(SkGlyph* glyph) {
         SkPath      devPath, fillPath;
         SkMatrix    fillToDevMatrix;
 
-        this->internalGetPath(*glyph, &fillPath, &devPath, &fillToDevMatrix);
+        this->internalGetPath(glyph->getPackedID(), &fillPath, &devPath, &fillToDevMatrix);
 
         // just use devPath
         const SkIRect ir = devPath.getBounds().roundOut();
@@ -154,7 +154,7 @@ void SkRandomScalerContext::generateImage(const SkGlyph& glyph) {
     if (!fFakeIt) {
         if (SkMask::kARGB32_Format == glyph.fMaskFormat) {
             SkPath path;
-            fProxy->getPath(glyph, &path);
+            fProxy->getPath(glyph.getPackedID(), &path);
 
             SkBitmap bm;
             bm.installPixels(SkImageInfo::MakeN32Premul(glyph.fWidth, glyph.fHeight),
@@ -175,8 +175,8 @@ void SkRandomScalerContext::generateImage(const SkGlyph& glyph) {
     }
 }
 
-void SkRandomScalerContext::generatePath(const SkGlyph& glyph, SkPath* path) {
-    fProxy->getPath(glyph, path);
+void SkRandomScalerContext::generatePath(SkGlyphID glyph, SkPath* path) {
+    fProxy->generatePath(glyph, path);
 }
 
 void SkRandomScalerContext::generateFontMetrics(SkPaint::FontMetrics* metrics) {
diff --git a/src/fonts/SkTestScalerContext.cpp b/src/fonts/SkTestScalerContext.cpp
index 7a97ca8bba..d84571d310 100644
--- a/src/fonts/SkTestScalerContext.cpp
+++ b/src/fonts/SkTestScalerContext.cpp
@@ -137,8 +137,8 @@ void SkTestTypeface::getMetrics(SkGlyph* glyph) {
     glyph->fAdvanceY = 0;
 }
 
-void SkTestTypeface::getPath(const SkGlyph& glyph, SkPath* path) {
-    *path = *fTestFont->fPaths[glyph.getGlyphID()];
+void SkTestTypeface::getPath(SkGlyphID glyph, SkPath* path) {
+    *path = *fTestFont->fPaths[glyph];
 }
 
 void SkTestTypeface::onFilterRec(SkScalerContextRec* rec) const {
@@ -236,7 +236,7 @@ protected:
         glyph->fAdvanceY = SkScalarToFloat(advance.fY);
 
         SkPath path;
-        this->getTestTypeface()->getPath(*glyph, &path);
+        this->getTestTypeface()->getPath(glyph->getGlyphID(), &path);
         path.transform(fMatrix);
 
         SkRect storage;
@@ -254,7 +254,7 @@ protected:
 
     void generateImage(const SkGlyph& glyph) override {
         SkPath path;
-        this->getTestTypeface()->getPath(glyph, &path);
+        this->getTestTypeface()->getPath(glyph.getGlyphID(), &path);
 
         SkBitmap bm;
         bm.installPixels(SkImageInfo::MakeN32Premul(glyph.fWidth, glyph.fHeight),
@@ -270,7 +270,7 @@ protected:
         canvas.drawPath(path, paint);
     }
 
-    void generatePath(const SkGlyph& glyph, SkPath* path) override {
+    void generatePath(SkGlyphID glyph, SkPath* path) override {
         this->getTestTypeface()->getPath(glyph, path);
         path->transform(fMatrix);
     }
diff --git a/src/fonts/SkTestScalerContext.h b/src/fonts/SkTestScalerContext.h
index 90945970fa..217e57c2a4 100644
--- a/src/fonts/SkTestScalerContext.h
+++ b/src/fonts/SkTestScalerContext.h
@@ -60,7 +60,7 @@ public:
     void getAdvance(SkGlyph* glyph);
     void getFontMetrics(SkPaint::FontMetrics* metrics);
     void getMetrics(SkGlyph* glyph);
-    void getPath(const SkGlyph& glyph, SkPath* path);
+    void getPath(SkGlyphID glyph, SkPath* path);
 protected:
     SkScalerContext* onCreateScalerContext(const SkScalerContextEffects&,
                                            const SkDescriptor* desc) const override;
diff --git a/src/gpu/GrPathRendering.cpp b/src/gpu/GrPathRendering.cpp
index ae5bcf643a..18d000a177 100644
--- a/src/gpu/GrPathRendering.cpp
+++ b/src/gpu/GrPathRendering.cpp
@@ -58,9 +58,7 @@ public:
     }
 
     void generatePath(int glyphID, SkPath* out) override {
-        SkGlyph skGlyph;
-        skGlyph.initWithGlyphID(glyphID);
-        fScalerContext->getPath(skGlyph, out);
+        fScalerContext->getPath(glyphID, out);
     }
 #ifdef SK_DEBUG
     bool isEqualTo(const SkDescriptor& desc) const override { return *fDesc == desc; }
diff --git a/src/ports/SkFontHost_FreeType.cpp b/src/ports/SkFontHost_FreeType.cpp
index 2eaf2bf06e..0a4e63688d 100644
--- a/src/ports/SkFontHost_FreeType.cpp
+++ b/src/ports/SkFontHost_FreeType.cpp
@@ -195,7 +195,7 @@ protected:
     void generateAdvance(SkGlyph* glyph) override;
     void generateMetrics(SkGlyph* glyph) override;
     void generateImage(const SkGlyph& glyph) override;
-    void generatePath(const SkGlyph& glyph, SkPath* path) override;
+    void generatePath(SkGlyphID glyphID, SkPath* path) override;
     void generateFontMetrics(SkPaint::FontMetrics*) override;
     SkUnichar generateGlyphToChar(uint16_t glyph) override;
 
@@ -1212,7 +1212,7 @@ void SkScalerContext_FreeType::generateImage(const SkGlyph& glyph) {
 }
 
 
-void SkScalerContext_FreeType::generatePath(const SkGlyph& glyph, SkPath* path) {
+void SkScalerContext_FreeType::generatePath(SkGlyphID glyphID, SkPath* path) {
     SkAutoMutexAcquire  ac(gFTMutex);
 
     SkASSERT(path);
@@ -1226,11 +1226,11 @@ void SkScalerContext_FreeType::generatePath(const SkGlyph& glyph, SkPath* path)
     flags |= FT_LOAD_NO_BITMAP; // ignore embedded bitmaps so we're sure to get the outline
     flags &= ~FT_LOAD_RENDER;   // don't scan convert (we just want the outline)
 
-    FT_Error err = FT_Load_Glyph( fFace, glyph.getGlyphID(), flags);
+    FT_Error err = FT_Load_Glyph(fFace, glyphID, flags);
 
     if (err != 0) {
         SkDEBUGF(("SkScalerContext_FreeType::generatePath: FT_Load_Glyph(glyph:%d flags:%d) returned 0x%x\n",
-                    glyph.getGlyphID(), flags, err));
+                  glyphID, flags, err));
         path->reset();
         return;
     }
diff --git a/src/ports/SkFontHost_mac.cpp b/src/ports/SkFontHost_mac.cpp
index a9b9a2a70e..a9f6bb0a6e 100644
--- a/src/ports/SkFontHost_mac.cpp
+++ b/src/ports/SkFontHost_mac.cpp
@@ -702,7 +702,7 @@ protected:
     void generateAdvance(SkGlyph* glyph) override;
     void generateMetrics(SkGlyph* glyph) override;
     void generateImage(const SkGlyph& glyph) override;
-    void generatePath(const SkGlyph& glyph, SkPath* path) override;
+    void generatePath(SkGlyphID glyph, SkPath* path) override;
     void generateFontMetrics(SkPaint::FontMetrics*) override;
 
 private:
@@ -1373,7 +1373,7 @@ void SkScalerContext_Mac::generateImage(const SkGlyph& glyph) {
  */
 #define kScaleForSubPixelPositionHinting (4.0f)
 
-void SkScalerContext_Mac::generatePath(const SkGlyph& glyph, SkPath* path) {
+void SkScalerContext_Mac::generatePath(SkGlyphID glyph, SkPath* path) {
     AUTO_CG_LOCK();
 
     SkScalar scaleX = SK_Scalar1;
@@ -1408,7 +1408,7 @@ void SkScalerContext_Mac::generatePath(const SkGlyph& glyph, SkPath* path) {
         xform = CGAffineTransformConcat(fTransform, scale);
     }
 
-    CGGlyph cgGlyph = (CGGlyph)glyph.getGlyphID();
+    CGGlyph cgGlyph = SkTo<CGGlyph>(glyph);
     AutoCFRelease<CGPathRef> cgPath(CTFontCreatePathForGlyph(fCTFont, cgGlyph, &xform));
 
     path->reset();
diff --git a/src/ports/SkFontHost_win.cpp b/src/ports/SkFontHost_win.cpp
index 77465ceff4..9229be6d30 100644
--- a/src/ports/SkFontHost_win.cpp
+++ b/src/ports/SkFontHost_win.cpp
@@ -546,11 +546,11 @@ protected:
     void generateAdvance(SkGlyph* glyph) override;
     void generateMetrics(SkGlyph* glyph) override;
     void generateImage(const SkGlyph& glyph) override;
-    void generatePath(const SkGlyph& glyph, SkPath* path) override;
+    void generatePath(SkGlyphID glyph, SkPath* path) override;
     void generateFontMetrics(SkPaint::FontMetrics*) override;
 
 private:
-    DWORD getGDIGlyphPath(const SkGlyph& glyph, UINT flags,
+    DWORD getGDIGlyphPath(SkGlyphID glyph, UINT flags,
                           SkAutoSTMalloc<BUFFERSIZE, uint8_t>* glyphbuf);
 
     HDCOffscreen fOffscreen;
@@ -1589,22 +1589,22 @@ static bool sk_path_from_gdi_paths(SkPath* path, const uint8_t* glyphbuf, DWORD
     return true;
 }
 
-DWORD SkScalerContext_GDI::getGDIGlyphPath(const SkGlyph& glyph, UINT flags,
-                                               SkAutoSTMalloc<BUFFERSIZE, uint8_t>* glyphbuf)
+DWORD SkScalerContext_GDI::getGDIGlyphPath(SkGlyphID glyph, UINT flags,
+                                           SkAutoSTMalloc<BUFFERSIZE, uint8_t>* glyphbuf)
 {
     GLYPHMETRICS gm;
 
-    DWORD total_size = GetGlyphOutlineW(fDDC, glyph.getGlyphID(), flags, &gm, BUFFERSIZE, glyphbuf->get(), &fMat22);
+    DWORD total_size = GetGlyphOutlineW(fDDC, glyph, flags, &gm, BUFFERSIZE, glyphbuf->get(), &fMat22);
     // Sometimes GetGlyphOutlineW returns a number larger than BUFFERSIZE even if BUFFERSIZE > 0.
     // It has been verified that this does not involve a buffer overrun.
     if (GDI_ERROR == total_size || total_size > BUFFERSIZE) {
         // GDI_ERROR because the BUFFERSIZE was too small, or because the data was not accessible.
         // When the data is not accessable GetGlyphOutlineW fails rather quickly,
         // so just try to get the size. If that fails then ensure the data is accessible.
-        total_size = GetGlyphOutlineW(fDDC, glyph.getGlyphID(), flags, &gm, 0, nullptr, &fMat22);
+        total_size = GetGlyphOutlineW(fDDC, glyph, flags, &gm, 0, nullptr, &fMat22);
         if (GDI_ERROR == total_size) {
             LogFontTypeface::EnsureAccessible(this->getTypeface());
-            total_size = GetGlyphOutlineW(fDDC, glyph.getGlyphID(), flags, &gm, 0, nullptr, &fMat22);
+            total_size = GetGlyphOutlineW(fDDC, glyph, flags, &gm, 0, nullptr, &fMat22);
             if (GDI_ERROR == total_size) {
                 // GetGlyphOutlineW is known to fail for some characters, such as spaces.
                 // In these cases, just return that the glyph does not have a shape.
@@ -1614,10 +1614,10 @@ DWORD SkScalerContext_GDI::getGDIGlyphPath(const SkGlyph& glyph, UINT flags,
 
         glyphbuf->reset(total_size);
 
-        DWORD ret = GetGlyphOutlineW(fDDC, glyph.getGlyphID(), flags, &gm, total_size, glyphbuf->get(), &fMat22);
+        DWORD ret = GetGlyphOutlineW(fDDC, glyph, flags, &gm, total_size, glyphbuf->get(), &fMat22);
         if (GDI_ERROR == ret) {
             LogFontTypeface::EnsureAccessible(this->getTypeface());
-            ret = GetGlyphOutlineW(fDDC, glyph.getGlyphID(), flags, &gm, total_size, glyphbuf->get(), &fMat22);
+            ret = GetGlyphOutlineW(fDDC, glyph, flags, &gm, total_size, glyphbuf->get(), &fMat22);
             if (GDI_ERROR == ret) {
                 SkASSERT(false);
                 return 0;
@@ -1627,7 +1627,7 @@ DWORD SkScalerContext_GDI::getGDIGlyphPath(const SkGlyph& glyph, UINT flags,
     return total_size;
 }
 
-void SkScalerContext_GDI::generatePath(const SkGlyph& glyph, SkPath* path) {
+void SkScalerContext_GDI::generatePath(SkGlyphID glyph, SkPath* path) {
     SkASSERT(path);
     SkASSERT(fDDC);
 
diff --git a/src/ports/SkScalerContext_win_dw.cpp b/src/ports/SkScalerContext_win_dw.cpp
index c83542f47a..10afb5a0ca 100644
--- a/src/ports/SkScalerContext_win_dw.cpp
+++ b/src/ports/SkScalerContext_win_dw.cpp
@@ -887,7 +887,7 @@ void SkScalerContext_DW::generateImage(const SkGlyph& glyph) {
     }
 }
 
-void SkScalerContext_DW::generatePath(const SkGlyph& glyph, SkPath* path) {
+void SkScalerContext_DW::generatePath(SkGlyphID glyph, SkPath* path) {
     SkASSERT(path);
 
     path->reset();
@@ -895,7 +895,7 @@ void SkScalerContext_DW::generatePath(const SkGlyph& glyph, SkPath* path) {
     SkTScopedComPtr<IDWriteGeometrySink> geometryToPath;
     HRVM(SkDWriteGeometrySink::Create(path, &geometryToPath),
          "Could not create geometry to path converter.");
-    uint16_t glyphId = glyph.getGlyphID();
+    UINT16 glyphId = SkTo<UINT16>(glyph);
     {
         SkAutoExclusive l(DWriteFactoryMutex);
         //TODO: convert to<->from DIUs? This would make a difference if hinting.
diff --git a/src/ports/SkScalerContext_win_dw.h b/src/ports/SkScalerContext_win_dw.h
index bcb7ab47db..7b45c320ca 100644
--- a/src/ports/SkScalerContext_win_dw.h
+++ b/src/ports/SkScalerContext_win_dw.h
@@ -34,7 +34,7 @@ protected:
     void generateAdvance(SkGlyph* glyph) override;
     void generateMetrics(SkGlyph* glyph) override;
     void generateImage(const SkGlyph& glyph) override;
-    void generatePath(const SkGlyph& glyph, SkPath* path) override;
+    void generatePath(SkGlyphID glyph, SkPath* path) override;
     void generateFontMetrics(SkPaint::FontMetrics*) override;
 
 private: