Make fMaskFormate private

Change-Id: I20e652f2b6f9bf606b03c6dd4e346c3439ea8a0b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/220876
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>

src/core/SkGlyph.cpp

@@ -67,6 +67,18 @@ static size_t format_rowbytes(int width, SkMask::Format format) {
                                         : width * format_alignment(format);
 }
 
+SkGlyph::SkGlyph(const SkGlyphPrototype& p)
+    : fWidth{p.width}
+    , fHeight{p.height}
+    , fTop{p.top}
+    , fLeft{p.left}
+    , fAdvanceX{p.advanceX}
+    , fAdvanceY{p.advanceY}
+    , fMaskFormat{(uint8_t)p.maskFormat}
+    , fForceBW{p.forceBW}
+    , fID{p.id}
+    {}
+
 size_t SkGlyph::formatAlignment() const {
     auto format = static_cast<SkMask::Format>(fMaskFormat);
     return format_alignment(format);

src/core/SkGlyph.h

@@ -114,13 +114,16 @@ private:
     uint32_t fID;
 };
 
-class SkGlyph {
+struct SkGlyphPrototype;
-    struct PathData;
 
+class SkGlyph {
 public:
-    constexpr explicit SkGlyph(SkPackedGlyphID id) : fID{id} {}
     static constexpr SkFixed kSubpixelRound = SK_FixedHalf >> SkPackedGlyphID::kSubBits;
 
+    constexpr explicit SkGlyph(SkPackedGlyphID id) : fID{id} {}
+    explicit SkGlyph(const SkGlyphPrototype& p);
+
+
     SkVector advanceVector() const { return SkVector{fAdvanceX, fAdvanceY}; }
     SkScalar advanceX() const { return fAdvanceX; }
     SkScalar advanceY() const { return fAdvanceY; }
@@ -174,6 +177,10 @@ public:
     // path was previously set.
     const SkPath* path() const;
 
+    // Format
+    bool isColor() const { return fMaskFormat == SkMask::kARGB32_Format; }
+    SkMask::Format maskFormat() const { return static_cast<SkMask::Format>(fMaskFormat); }
+
     int maxDimension() const {
         // width and height are only defined if a metrics call was made.
         SkASSERT(fMaskFormat != MASK_FORMAT_UNKNOWN);
@@ -203,20 +210,21 @@ public:
     int16_t   fTop  = 0,
               fLeft = 0;
 
-    // This is a combination of SkMask::Format and SkGlyph state. The SkGlyph can be in one of two
-    // states, just the advances have been calculated, and all the metrics are available. The
-    // illegal mask format is used to signal that only the advances are available.
-    uint8_t   fMaskFormat = MASK_FORMAT_UNKNOWN;
-
 private:
     // There are two sides to an SkGlyph, the scaler side (things that create glyph data) have
     // access to all the fields. Scalers are assumed to maintain all the SkGlyph invariants. The
     // consumer side has a tighter interface.
+    friend class RandomScalerContext;
+    friend class SkScalerContext;
+    friend class SkScalerContextProxy;
+    friend class SkScalerContext_Empty;
     friend class SkScalerContext_FreeType;
+    friend class SkScalerContext_FreeType_Base;
     friend class SkScalerContext_DW;
     friend class SkScalerContext_GDI;
     friend class SkScalerContext_Mac;
     friend class SkStrikeClient;
+    friend class SkStrikeServer;
     friend class SkTestScalerContext;
     friend class SkTestSVGScalerContext;
     friend class TestSVGTypeface;
@@ -226,7 +234,7 @@ private:
     // 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
     // beginning and end of of the intersection of the bounds and the glyph's path.
-    // If interval[0] >= interval[1], no intesection was found.
+    // If interval[0] >= interval[1], no intersection was found.
     struct Intercept {
         Intercept* fNext;
         SkScalar   fBounds[2];    // for horz underlines, the boundaries in Y
@@ -251,6 +259,11 @@ private:
     float     fAdvanceX = 0,
               fAdvanceY = 0;
 
+    // This is a combination of SkMask::Format and SkGlyph state. The SkGlyph can be in one of two
+    // states, just the advances have been calculated, and all the metrics are available. The
+    // illegal mask format is used to signal that only the advances are available.
+    uint8_t   fMaskFormat = MASK_FORMAT_UNKNOWN;
+
     // Used by the DirectWrite scaler to track state.
     int8_t    fForceBW = 0;
 
@@ -259,4 +272,23 @@ private:
     SkPackedGlyphID fID;
 };
 
+struct SkGlyphPrototype {
+    SkPackedGlyphID id;
+
+    float           advanceX = 0,
+                    advanceY = 0;
+
+    // The width and height of the glyph mask.
+    uint16_t        width  = 0,
+                    height = 0;
+
+    // The offset from the glyphs origin on the baseline to the top left of the glyph mask.
+    int16_t         left = 0,
+                    top  = 0;
+
+    SkMask::Format  maskFormat = SkMask::kBW_Format;
+
+    bool            forceBW = 0;
+};
+
 #endif

src/core/SkGlyphRunPainter.cpp

@@ -166,7 +166,7 @@ void SkGlyphRunListPainter::drawForBitmapDevice(
                 SkPoint position = glyphPos.position;
                 if (check_glyph_position(position)
                     && !glyph.isEmpty()
-                    && glyph.fMaskFormat != SkMask::kARGB32_Format
+                    && !glyph.isColor()
                     && glyph.path() != nullptr)
                 {
                     // Only draw a path if it exists, and this is not a color glyph.
@@ -385,16 +385,16 @@ void SkGlyphRunListPainter::processGlyphRunList(const SkGlyphRunList& glyphRunLi
                 // The SDF scaler context system ensures that a glyph is empty, kSDF_Format, or
                 // kARGB32_Format. The following if statements use this assumption.
                 SkASSERT(glyph.isEmpty()
-                         || glyph.fMaskFormat == SkMask::kSDF_Format
+                         || glyph.maskFormat() == SkMask::kSDF_Format
-                         || glyph.fMaskFormat == SkMask::kARGB32_Format);
+                         || glyph.isColor());
 
                 if (glyph.isEmpty()) {
                     // do nothing
-                } else if (glyph.fMaskFormat == SkMask::kSDF_Format
+                } else if (glyph.maskFormat() == SkMask::kSDF_Format
                            && glyph.maxDimension() <= SkStrikeCommon::kSkSideTooBigForAtlas) {
                     // SDF mask will work.
                     fGlyphPos[glyphsWithMaskCount++] = glyphPos;
-                } else if (glyph.fMaskFormat != SkMask::kARGB32_Format
+                } else if (!glyph.isColor()
                            && glyph.path() != nullptr) {
                     // If not color but too big, use a path.
                     fPaths.push_back(glyphPos);
@@ -456,7 +456,7 @@ void SkGlyphRunListPainter::processGlyphRunList(const SkGlyphRunList& glyphRunLi
                 SkPoint position = glyphPos.position;
                 if (glyph.isEmpty()) {
                     // do nothing
-                } else if (glyph.fMaskFormat != SkMask::kARGB32_Format
+                } else if (!glyph.isColor()
                            && glyph.path() != nullptr) {
                     // Place paths in fGlyphPos
                     fGlyphPos[glyphsWithPathCount++] = glyphPos;
@@ -514,7 +514,7 @@ void SkGlyphRunListPainter::processGlyphRunList(const SkGlyphRunList& glyphRunLi
 
                 if (glyph.maxDimension() <= SkStrikeCommon::kSkSideTooBigForAtlas) {
                     fGlyphPos[glyphsWithMaskCount++] = glyphPos;
-                } else if (glyph.fMaskFormat != SkMask::kARGB32_Format
+                } else if (!glyph.isColor()
                            && glyph.path() != nullptr) {
                     fPaths.push_back(glyphPos);
                 } else {

src/core/SkRemoteGlyphCache.cpp

@@ -449,7 +449,7 @@ static void writeGlyph(SkGlyph* glyph, Serializer* serializer) {
     serializer->write<uint16_t>(glyph->fHeight);
     serializer->write<int16_t>(glyph->fTop);
     serializer->write<int16_t>(glyph->fLeft);
-    serializer->write<uint8_t>(glyph->fMaskFormat);
+    serializer->write<uint8_t>(glyph->maskFormat());
 }
 
 void SkStrikeServer::SkGlyphCacheState::writePendingGlyphs(Serializer* serializer) {

src/core/SkStrike.cpp

@@ -65,6 +65,16 @@ SkGlyph* SkStrike::glyph(SkGlyphID glyphID) {
     return this->glyph(SkPackedGlyphID{glyphID});
 }
 
+SkGlyph* SkStrike::glyphFromPrototype(const SkGlyphPrototype& p) {
+    SkGlyph* glyph = fGlyphMap.findOrNull(p.id);
+    if (glyph == nullptr) {
+        fMemoryUsed += sizeof(SkGlyph);
+        glyph = fAlloc.make<SkGlyph>(p);
+        fGlyphMap.set(glyph);
+    }
+    return glyph;
+}
+
 SkGlyph* SkStrike::glyphOrNull(SkPackedGlyphID id) const {
     return fGlyphMap.findOrNull(id);
 }

src/core/SkStrike.h

@@ -63,8 +63,9 @@ public:
 
     // Return a glyph. Create it if it doesn't exist, and initialize the glyph with metrics and
     // advances.
-    SkGlyph* glyph(SkPackedGlyphID id);
+    SkGlyph* glyph(SkPackedGlyphID packedID);
-    SkGlyph* glyph(SkGlyphID);
+    SkGlyph* glyph(SkGlyphID glyphID);
+    SkGlyph* glyphFromPrototype(const SkGlyphPrototype& p);
 
     // Return a glyph or nullptr if it does not exits in the strike.
     SkGlyph* glyphOrNull(SkPackedGlyphID id) const;

src/gpu/GrGlyph.h

@@ -23,8 +23,7 @@ struct GrGlyph {
     };
 
     static GrMaskFormat FormatFromSkGlyph(const SkGlyph& glyph) {
-        SkMask::Format format = static_cast<SkMask::Format>(glyph.fMaskFormat);
+        switch (glyph.maskFormat()) {
-        switch (format) {
             case SkMask::kBW_Format:
             case SkMask::kSDF_Format:
                 // fall through to kA8 -- we store BW and SDF glyphs in our 8-bit cache
@@ -50,7 +49,7 @@ struct GrGlyph {
     }
 
     static MaskStyle MaskStyleFromSkGlyph(const SkGlyph& skGlyph) {
-        return (SkMask::Format)skGlyph.fMaskFormat == SkMask::kSDF_Format
+        return skGlyph.maskFormat() == SkMask::kSDF_Format
            ? GrGlyph::MaskStyle::kDistance_MaskStyle
            : GrGlyph::MaskStyle::kCoverage_MaskStyle;
     }

src/gpu/text/GrStrikeCache.cpp

@@ -129,7 +129,7 @@ static bool get_packed_glyph_image(SkStrike* cache, const SkGlyph& glyph, int wi
     // The windows font host sometimes has BW glyphs in a non-BW strike. So it is important here to
     // check the glyph's format, not the strike's format, and to be able to convert to any of the
     // GrMaskFormats.
-    if (SkMask::kBW_Format == glyph.fMaskFormat) {
+    if (glyph.maskFormat() == SkMask::kBW_Format) {
         // expand bits to our mask type
         const uint8_t* bits = reinterpret_cast<const uint8_t*>(src);
         switch (expectedMaskFormat) {

src/ports/SkFontHost_mac.cpp

@@ -1047,13 +1047,13 @@ CGRGBPixel* Offscreen::getCG(const SkScalerContext_Mac& context, const SkGlyph&
     bool doAA = false;
     bool doLCD = false;
 
-    if (SkMask::kBW_Format != glyph.fMaskFormat) {
+    if (SkMask::kBW_Format != glyph.maskFormat()) {
         doLCD = true;
         doAA = true;
     }
 
     // FIXME: lcd smoothed un-hinted rasterization unsupported.
-    if (!generateA8FromLCD && SkMask::kA8_Format == glyph.fMaskFormat) {
+    if (!generateA8FromLCD && SkMask::kA8_Format == glyph.maskFormat()) {
         doLCD = false;
         doAA = true;
     }
@@ -1061,7 +1061,7 @@ CGRGBPixel* Offscreen::getCG(const SkScalerContext_Mac& context, const SkGlyph&
     // If this font might have color glyphs, disable LCD as there's no way to support it.
     // CoreText doesn't tell us which format it ended up using, so we can't detect it.
     // A8 will end up black on transparent, but TODO: we can detect gray and set to A8.
-    if (SkMask::kARGB32_Format == glyph.fMaskFormat) {
+    if (SkMask::kARGB32_Format == glyph.maskFormat()) {
         doLCD = false;
     }
 
@@ -1076,7 +1076,7 @@ CGRGBPixel* Offscreen::getCG(const SkScalerContext_Mac& context, const SkGlyph&
 
         rowBytes = fSize.fWidth * sizeof(CGRGBPixel);
         void* image = fImageStorage.reset(rowBytes * fSize.fHeight);
-        const CGImageAlphaInfo alpha = (SkMask::kARGB32_Format == glyph.fMaskFormat)
+        const CGImageAlphaInfo alpha = (glyph.isColor())
                                      ? kCGImageAlphaPremultipliedFirst
                                      : kCGImageAlphaNoneSkipFirst;
         const CGBitmapInfo bitmapInfo = kCGBitmapByteOrder32Host | alpha;
@@ -1120,7 +1120,7 @@ CGRGBPixel* Offscreen::getCG(const SkScalerContext_Mac& context, const SkGlyph&
     image += (fSize.fHeight - glyph.fHeight) * fSize.fWidth;
 
     // Erase to white (or transparent black if it's a color glyph, to not composite against white).
-    uint32_t bgColor = (SkMask::kARGB32_Format != glyph.fMaskFormat) ? 0xFFFFFFFF : 0x00000000;
+    uint32_t bgColor = (!glyph.isColor()) ? 0xFFFFFFFF : 0x00000000;
     sk_memset_rect32(image, bgColor, glyph.fWidth, glyph.fHeight, rowBytes);
 
     float subX = 0;

tests/SkRemoteGlyphCacheTest.cpp

@@ -845,12 +845,9 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
         auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
 
         auto fallbackCache = strikeCache.findOrCreateStrikeExclusive(*desc, effects, *clientTf);
-        auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID);
+        SkGlyphPrototype proto = {lostGlyphID, 0.f, 0.f, 2, 1, 0, 0, SkMask::kA8_Format, false};
-        glyph->fMaskFormat = SkMask::kA8_Format;
+        SkGlyph* glyph = fallbackCache->glyphFromPrototype(proto);
-        glyph->fHeight = 1;
-        glyph->fWidth = 2;
         fallbackCache->initializeImage(glyphImage, glyph->computeImageSize(), glyph);
-        glyph->fImage = (void *)glyphImage;
     }
 
     // Make sure we can find the fall back cache.
@@ -892,7 +889,7 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
 
         REPORTER_ASSERT(reporter, lostGlyph.fHeight == 1);
         REPORTER_ASSERT(reporter, lostGlyph.fWidth == 2);
-        REPORTER_ASSERT(reporter, lostGlyph.fMaskFormat == SkMask::kA8_Format);
+        REPORTER_ASSERT(reporter, lostGlyph.maskFormat() == SkMask::kA8_Format);
         REPORTER_ASSERT(reporter, memcmp(lostGlyph.fImage, glyphImage, sizeof(glyphImage)) == 0);
     }
 
@@ -904,7 +901,7 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
 
         REPORTER_ASSERT(reporter, lostGlyph.fHeight == 1);
         REPORTER_ASSERT(reporter, lostGlyph.fWidth == 2);
-        REPORTER_ASSERT(reporter, lostGlyph.fMaskFormat == SkMask::kA8_Format);
+        REPORTER_ASSERT(reporter, lostGlyph.maskFormat() == SkMask::kA8_Format);
         REPORTER_ASSERT(reporter, memcmp(lostGlyph.fImage, glyphImage, sizeof(glyphImage)) == 0);
     }
 
@@ -940,8 +937,8 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
 
     auto lostGlyphID = SkPackedGlyphID(1, SK_FixedHalf, SK_FixedHalf);
     const uint8_t glyphImage[] = {0xFF, 0xFF};
-    uint32_t realMask;
+    SkMask::Format realMask;
-    uint32_t fakeMask;
+    SkMask::Format fakeMask;
 
     SkStrikeCache strikeCache;
 
@@ -959,8 +956,7 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
         auto context = serverTf->createScalerContext(effects, desc, false);
         SkGlyph glyph{lostGlyphID};
         context->getMetrics(&glyph);
-        realMask = glyph.fMaskFormat;
+        realMask = glyph.maskFormat();
-        REPORTER_ASSERT(reporter, realMask != MASK_FORMAT_UNKNOWN);
     }
 
     // Build a fallback cache.
@@ -976,11 +972,9 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
         auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
 
         auto fallbackCache = strikeCache.findOrCreateStrikeExclusive(*desc, effects, *clientTf);
-        auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID);
         fakeMask = (realMask == SkMask::kA8_Format) ? SkMask::kBW_Format : SkMask::kA8_Format;
-        glyph->fMaskFormat = fakeMask;
+        SkGlyphPrototype proto = {lostGlyphID, 0.f, 0.f, 2, 1, 0, 0, fakeMask, false};
-        glyph->fHeight = 1;
+        SkGlyph* glyph = fallbackCache->glyphFromPrototype(proto);
-        glyph->fWidth = 2;
         fallbackCache->initializeImage(glyphImage, glyph->computeImageSize(), glyph);
     }
 
@@ -1017,7 +1011,7 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
         auto fallbackCache = strikeCache.findStrikeExclusive(*desc);
         REPORTER_ASSERT(reporter, fallbackCache.get() != nullptr);
         auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID);
-        REPORTER_ASSERT(reporter, glyph->fMaskFormat == fakeMask);
+        REPORTER_ASSERT(reporter, glyph->maskFormat() == fakeMask);
         REPORTER_ASSERT(reporter,
                         memcmp(glyph->fImage, glyphImage, glyph->computeImageSize()) == 0);
     }