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..25aaed77a6e3c459ccdcfba16d85fa6262ad6a3d 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,
+        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,12 @@ 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 initGlyphIdFrom(const SkGlyph& glyph) {
+        this->initCommon(glyph.fID);
     }
 
     /**
@@ -94,18 +106,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);
     }
 
     static unsigned ID2SubX(uint32_t id) {
@@ -134,11 +150,16 @@ struct SkGlyph {
         x = FixedToSub(x);
         y = FixedToSub(y);
         return (x << (kSubShift + kSubShiftX)) |
-               (y << (kSubShift + kSubShiftY)) |
-               code;
+            (y << (kSubShift + kSubShiftY)) |
+            code;
     }
 
-    void toMask(SkMask* mask) const;
+  // FIXME - This is needed because the Android frame work directly
+  // accesses fID. Remove when fID accesses are cleaned up.

[mtklein, 2015/02/24 23:33:48]

As far as dirty hacks go, I don't think this makes things any dirtier, but won't
this prevent you from doing the +1, -1 later?

Seems like we need to add a public getter (hopefully not a setter?), switch
android over to that, then make the field private?

[scroggo, 2015/02/25 14:08:00]

Android does not need a setter. That seems like the right approach...
If the plan is to modify it later, then perhaps the getter won't work. Android
stores the fID and then uses it later on. If Android still *should* be looking
at the original fID, we don't have a problem. I don't know enough about this
code, or Android's use of it to make that call though.

+#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
+  public:
+#endif
+    uint32_t    fID;
 };
 
 #endif