Make fID and MixedID calculations private

src/core/SkGlyph.h

 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SkGlyph_DEFINED
 #define SkGlyph_DEFINED
 
 #include "SkTypes.h"
 #include "SkFixed.h"
 #include "SkMask.h"
 
 class SkPath;
+class SkGlyphCache;
 
 // needs to be != to any valid SkMask::Format
 #define MASK_FORMAT_UNKNOWN         (0xFF)
 #define MASK_FORMAT_JUST_ADVANCE    MASK_FORMAT_UNKNOWN
 
 #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;
 
     uint8_t     fMaskFormat;
     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);
     }
 
     /**
      *  Compute the rowbytes for the specified width and mask-format.
      */
     static unsigned ComputeRowBytes(unsigned width, SkMask::Format format) {
         unsigned rb = width;
         if (SkMask::kBW_Format == format) {
             rb = (rb + 7) >> 3;
         } else if (SkMask::kARGB32_Format == format) {
@@ skipping 35 matching lines @@
     }
 
     size_t computeImageSize() const;
 
     /** Call this to set all of the metrics fields to 0 (e.g. if the scaler
         encounters an error measuring a glyph). Note: this does not alter the
         fImage, fPath, fID, fMaskFormat fields.
      */
     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) {
         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) {
         return code;
     }
 
     static uint32_t MakeID(unsigned code, SkFixed x, SkFixed y) {
         SkASSERT(code <= kCodeMask);
         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