Make fID and MixedID calculations private

src/core/SkGlyph.h

Index: src/core/SkGlyph.h
diff --git a/src/core/SkGlyph.h b/src/core/SkGlyph.h
index 9abefa84c7578f450cce93460a109e95e3f0bca4..0217902a094715293421fb185dabd16f6946320a 100644
--- a/src/core/SkGlyph.h
+++ b/src/core/SkGlyph.h
@@ -13,6 +13,7 @@
 #include "SkMask.h"
 
 class SkPath;
+class SkGlyphCache;
 
 // needs to be != to any valid SkMask::Format
 #define MASK_FORMAT_UNKNOWN         (0xFF)
@@ -20,12 +21,23 @@ class SkPath;
 
 #define kMaxGlyphWidth (1<<13)
 
-struct SkGlyph {
+class SkGlyph {
+    enum {
+        kSubBits = 2,
+        kSubMask = ((1 << kSubBits) - 1),
+        kSubShift = 24, // must be large enough for glyphs and unichars
+        kCodeMask = ((1 << kSubShift) - 1),
+        // relative offsets for X and Y subpixel bits
+            kSubShiftX = kSubBits,

[mtklein, 2015/02/23 17:30:36]

funky indent?

[herb_g, 2015/02/23 23:24:53]

Done.

+        kSubShiftY = 0
+    };
+
+ public:
+    static const SkFixed kSubpixelRound = SK_FixedHalf >> SkGlyph::kSubBits;
     void*       fImage;
     SkPath*     fPath;
     SkFixed     fAdvanceX, fAdvanceY;
 
-    uint32_t    fID;
     uint16_t    fWidth, fHeight;
     int16_t     fTop, fLeft;
 
@@ -33,12 +45,16 @@ struct SkGlyph {
     int8_t      fRsbDelta, fLsbDelta;  // used by auto-kerning
     int8_t      fForceBW;
 
-    void init(uint32_t id) {
-        fID             = id;
-        fImage          = NULL;
-        fPath           = NULL;
-        fMaskFormat     = MASK_FORMAT_UNKNOWN;
-        fForceBW        = 0;
+    void initWithGlyphID(uint32_t glyph_id) {
+        this->initCommon(MakeID(glyph_id));
+    }
+
+    void initWithGlyphIDXY(uint32_t glyph_id, SkFixed x, SkFixed y) {

[mtklein, 2015/02/23 17:30:36]

Huh, unused.  Must be only SkGlyphCache does this?  Let's leave it out if we're
going to keep SkGlyphCache as a superfriend.

[herb_g, 2015/02/23 23:24:53]

Done.

+        this->initCommon(MakeID(glyph_id, x, y));
+    }
+
+    void initFromGlyph(const SkGlyph& glyph) {

[mtklein, 2015/02/23 17:30:36]

maybe copyFrom() ?

[herb_g, 2015/02/23 23:24:53]

This is not really copying the whole structure, but only the glyph id. It's
really an init. Tried a better name.

+        this->initCommon(glyph.fID);
     }
 
     /**
@@ -94,18 +110,22 @@ struct SkGlyph {
      */
     void zeroMetrics();
 
-    enum {
-        kSubBits = 2,
-        kSubMask = ((1 << kSubBits) - 1),
-        kSubShift = 24, // must be large enough for glyphs and unichars
-        kCodeMask = ((1 << kSubShift) - 1),
-        // relative offsets for X and Y subpixel bits
-        kSubShiftX = kSubBits,
-        kSubShiftY = 0
-    };
 
+    void toMask(SkMask* mask) const;
+
+ private:
+    // TODO(herb) remove friend statement after SkGlyphCache cleanup.
+    friend class SkGlyphCache;
+
+    void initCommon(uint32_t id) {
+        fID             = id;
+        fImage          = NULL;
+        fPath           = NULL;
+        fMaskFormat     = MASK_FORMAT_UNKNOWN;
+        fForceBW        = 0;
+    }
     static unsigned ID2Code(uint32_t id) {
-        return id & kCodeMask;
+        return (id & kCodeMask) - 1;

[mtklein, 2015/02/23 17:30:36]

Now that it's clear this is safe, let's undo this for this CL.  It's weird to
land it without the extra context of why we're trying to carve zero out as an
invalid value.  Really, when SkGlyphCache is no longer a friend, we'll probably
want an SkGlyph::initInvalid() to hide away how we do that too.  It may be
clearer, for example, to use ~0 instead of 0 as the invalid value.

[herb_g, 2015/02/23 23:24:53]

If you use ~0, then one record will always be at the end of the array, so you
will always be adjust an extra record. With sentinal == 0 then you will never
move the extra record.

     }
 
     static unsigned ID2SubX(uint32_t id) {
@@ -126,7 +146,7 @@ struct SkGlyph {
     }
 
     static uint32_t MakeID(unsigned code) {
-        return code;
+        return code + 1;
     }
 
     static uint32_t MakeID(unsigned code, SkFixed x, SkFixed y) {
@@ -134,11 +154,11 @@ struct SkGlyph {
         x = FixedToSub(x);
         y = FixedToSub(y);
         return (x << (kSubShift + kSubShiftX)) |
-               (y << (kSubShift + kSubShiftY)) |
-               code;
+            (y << (kSubShift + kSubShiftY)) |
+            (code + 1);
     }
 
-    void toMask(SkMask* mask) const;
+    uint32_t    fID;
 };
 
 #endif