Reduce GrGlyph's functionality

The idea here is to move the mapping of GrGlyph to atlas location into the GrAtlasManager.

The only thing left in the GrGlyph will be the PackedGlyphID and the width/height.

Bug: 1056730
Change-Id: I6f85780eddbab701100599198b70edfed0c434dc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/279915
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Herb Derby <herb@google.com>

src/gpu/GrGlyph.h

@@ -16,12 +16,8 @@
 #include "include/private/SkChecksum.h"
 #include "include/private/SkFixed.h"
 
-struct GrGlyph {
+class GrGlyph {
-    enum MaskStyle {
+public:
-        kCoverage_MaskStyle,
-        kDistance_MaskStyle
-    };
-
     static GrMaskFormat FormatFromSkGlyph(SkMask::Format format) {
         switch (format) {
             case SkMask::kBW_Format:
@@ -35,58 +31,23 @@ struct GrGlyph {
                 return kA565_GrMaskFormat;
             case SkMask::kARGB32_Format:
                 return kARGB_GrMaskFormat;
-            default:
-                SkDEBUGFAIL("unsupported SkMask::Format");
-                return kA8_GrMaskFormat;
-        }
         }
 
-    static MaskStyle MaskStyleFromSkGlyph(const SkGlyph& skGlyph) {
+        SkUNREACHABLE;
-        return skGlyph.maskFormat() == SkMask::kSDF_Format
-           ? GrGlyph::MaskStyle::kDistance_MaskStyle
-           : GrGlyph::MaskStyle::kCoverage_MaskStyle;
     }
 
     GrGlyph(const SkGlyph& skGlyph)
             : fPackedID{skGlyph.getPackedID()}
-        , fMaskFormat{FormatFromSkGlyph(skGlyph.maskFormat())}
+            , fWidthHeight(SkIPoint16::Make(skGlyph.width(), skGlyph.height())) {
-        , fMaskStyle{MaskStyleFromSkGlyph(skGlyph)}
-        , fBounds{GrIRect16::Make(skGlyph.iRect())} {}
-
-
-    SkRect destRect(SkPoint origin) {
-        return SkRect::MakeXYWH(
-                SkIntToScalar(fBounds.fLeft) + origin.x(),
-                SkIntToScalar(fBounds.fTop)  + origin.y(),
-                SkIntToScalar(fBounds.width()),
-                SkIntToScalar(fBounds.height()));
     }
 
-    SkRect destRect(SkPoint origin, SkScalar textScale) {
+    int width() const { return fWidthHeight.fX; }
-        if (fMaskStyle == kCoverage_MaskStyle) {
+    int height() const { return fWidthHeight.fY; }
-            return SkRect::MakeXYWH(
-                    SkIntToScalar(fBounds.fLeft)    * textScale + origin.x(),
-                    SkIntToScalar(fBounds.fTop)     * textScale + origin.y(),
-                    SkIntToScalar(fBounds.width())  * textScale,
-                    SkIntToScalar(fBounds.height()) * textScale);
-        } else {
-            return SkRect::MakeXYWH(
-                    (SkIntToScalar(fBounds.fLeft) + SK_DistanceFieldInset) * textScale + origin.x(),
-                    (SkIntToScalar(fBounds.fTop)  + SK_DistanceFieldInset) * textScale + origin.y(),
-                    (SkIntToScalar(fBounds.width())  - 2 * SK_DistanceFieldInset) * textScale,
-                    (SkIntToScalar(fBounds.height()) - 2 * SK_DistanceFieldInset) * textScale);
-        }
-    }
-
-    int width() const { return fBounds.width(); }
-    int height() const { return fBounds.height(); }
     uint32_t pageIndex() const { return GrDrawOpAtlas::GetPageIndexFromID(fPlotLocator); }
-    MaskStyle maskStyle() const { return fMaskStyle; }
 
     const SkPackedGlyphID      fPackedID;
-    const GrMaskFormat         fMaskFormat;
+    const SkIPoint16           fWidthHeight{0, 0};
-    const MaskStyle            fMaskStyle;
+
-    const GrIRect16            fBounds;
     SkIPoint16                 fAtlasLocation{0, 0};
     GrDrawOpAtlas::PlotLocator fPlotLocator{GrDrawOpAtlas::kInvalidPlotLocator};
 };

src/gpu/text/GrAtlasManager.cpp

@@ -28,9 +28,9 @@ void GrAtlasManager::freeAll() {
     }
 }
 
-bool GrAtlasManager::hasGlyph(GrGlyph* glyph) {
+bool GrAtlasManager::hasGlyph(GrMaskFormat format, GrGlyph* glyph) {
     SkASSERT(glyph);
-    return this->getAtlas(glyph->fMaskFormat)->hasID(glyph->fPlotLocator);
+    return this->getAtlas(format)->hasID(glyph->fPlotLocator);
 }
 
 // add to texture atlas that matches this format
@@ -44,11 +44,11 @@ GrDrawOpAtlas::ErrorCode GrAtlasManager::addToAtlas(
 }
 
 void GrAtlasManager::addGlyphToBulkAndSetUseToken(GrDrawOpAtlas::BulkUseTokenUpdater* updater,
-                                                  GrGlyph* glyph,
+                                                  GrMaskFormat format, GrGlyph* glyph,
                                                   GrDeferredUploadToken token) {
     SkASSERT(glyph);
     if (updater->add(glyph->fPlotLocator)) {
-        this->getAtlas(glyph->fMaskFormat)->setLastUseToken(glyph->fPlotLocator, token);
+        this->getAtlas(format)->setLastUseToken(glyph->fPlotLocator, token);
     }
 }
 

src/gpu/text/GrAtlasManager.h

@@ -13,7 +13,7 @@
 #include "src/gpu/GrOnFlushResourceProvider.h"
 #include "src/gpu/GrProxyProvider.h"
 
-struct GrGlyph;
+class GrGlyph;
 class GrTextStrike;
 
 //////////////////////////////////////////////////////////////////////////////////////////////////
@@ -57,14 +57,14 @@ public:
 
     void freeAll();
 
-    bool hasGlyph(GrGlyph* glyph);
+    bool hasGlyph(GrMaskFormat, GrGlyph*);
 
     // To ensure the GrDrawOpAtlas does not evict the Glyph Mask from its texture backing store,
     // the client must pass in the current op token along with the GrGlyph.
     // A BulkUseTokenUpdater is used to manage bulk last use token updating in the Atlas.
     // For convenience, this function will also set the use token for the current glyph if required
     // NOTE: the bulk uploader is only valid if the subrun has a valid atlasGeneration
-    void addGlyphToBulkAndSetUseToken(GrDrawOpAtlas::BulkUseTokenUpdater*, GrGlyph*,
+    void addGlyphToBulkAndSetUseToken(GrDrawOpAtlas::BulkUseTokenUpdater*, GrMaskFormat, GrGlyph*,
                                       GrDeferredUploadToken);
 
     void setUseTokenBulk(const GrDrawOpAtlas::BulkUseTokenUpdater& updater,

src/gpu/text/GrStrikeCache.cpp

@@ -165,11 +165,11 @@ GrDrawOpAtlas::ErrorCode GrTextStrike::addGlyphToAtlas(const SkGlyph& skGlyph,
     expectedMaskFormat = fullAtlasManager->resolveMaskFormat(expectedMaskFormat);
     int bytesPerPixel = GrMaskFormatBytesPerPixel(expectedMaskFormat);
 
-    bool isSDFGlyph = grGlyph->maskStyle() == GrGlyph::kDistance_MaskStyle;
+    bool isSDFGlyph = skGlyph.maskFormat() == SkMask::kSDF_Format;
     // Add 1 pixel padding around grGlyph if needed.
     bool addPad = isScaledGlyph && !isSDFGlyph;
-    const int width = addPad ? grGlyph->width() + 2 : grGlyph->width();
+    const int width = addPad ? skGlyph.width() + 2 : skGlyph.width();
-    const int height = addPad ? grGlyph->height() + 2 : grGlyph->height();
+    const int height = addPad ? skGlyph.height() + 2 : skGlyph.height();
     int rowBytes = width * bytesPerPixel;
     size_t size = height * rowBytes;
 
@@ -208,15 +208,3 @@ GrGlyph* GrTextStrike::getGlyph(const SkGlyph& skGlyph) {
     return grGlyph;
 }
 
-GrGlyph*
-GrTextStrike::getGlyph(SkPackedGlyphID packed, SkBulkGlyphMetricsAndImages* metricsAndImages) {
-    GrGlyph* grGlyph = fCache.findOrNull(packed);
-    if (grGlyph == nullptr) {
-        // We could return this to the caller, but in practice it adds code complexity for
-        // potentially little benefit(ie, if the glyph is not in our font cache, then its not
-        // in the atlas and we're going to be doing a texture upload anyways).
-        grGlyph = fAlloc.make<GrGlyph>(*metricsAndImages->glyph(packed));
-        fCache.set(grGlyph);
-    }
-    return grGlyph;
-}

src/gpu/text/GrStrikeCache.h

@@ -33,12 +33,6 @@ public:
 
     GrGlyph* getGlyph(const SkGlyph& skGlyph);
 
-    // This variant of the above function is called by GrAtlasTextOp. At this point, it is possible
-    // that the maskformat of the glyph differs from what we expect.  In these cases we will just
-    // draw a clear square.
-    // skbug:4143 crbug:510931
-    GrGlyph* getGlyph(SkPackedGlyphID packed, SkBulkGlyphMetricsAndImages* metricsAndImages);
-
     // returns true if glyph successfully added to texture atlas, false otherwise.  If the glyph's
     // mask format has changed, then addGlyphToAtlas will draw a clear box.  This will almost never
     // happen.

src/gpu/text/GrTextBlob.cpp

@@ -74,6 +74,35 @@ GrTextBlob::SubRun::SubRun(GrTextBlob* textBlob, const SkStrikeSpec& strikeSpec)
     textBlob->insertSubRun(this);
 }
 
+
+static SkRect dest_rect(const SkGlyph& g, SkPoint origin) {
+    return SkRect::MakeXYWH(
+            SkIntToScalar(g.left()) + origin.x(),
+            SkIntToScalar(g.top())  + origin.y(),
+            SkIntToScalar(g.width()),
+            SkIntToScalar(g.height()));
+}
+
+static bool is_SDF(const SkGlyph& skGlyph) {
+    return skGlyph.maskFormat() == SkMask::kSDF_Format;
+}
+
+static SkRect dest_rect(const SkGlyph& g, SkPoint origin, SkScalar textScale) {
+    if (!is_SDF(g)) {
+        return SkRect::MakeXYWH(
+                SkIntToScalar(g.left())   * textScale + origin.x(),
+                SkIntToScalar(g.top())    * textScale + origin.y(),
+                SkIntToScalar(g.width())  * textScale,
+                SkIntToScalar(g.height()) * textScale);
+    } else {
+        return SkRect::MakeXYWH(
+                (SkIntToScalar(g.left()) + SK_DistanceFieldInset) * textScale + origin.x(),
+                (SkIntToScalar(g.top())  + SK_DistanceFieldInset) * textScale + origin.y(),
+                (SkIntToScalar(g.width())  - 2 * SK_DistanceFieldInset) * textScale,
+                (SkIntToScalar(g.height()) - 2 * SK_DistanceFieldInset) * textScale);
+    }
+}
+
 void GrTextBlob::SubRun::appendGlyphs(const SkZip<SkGlyphVariant, SkPoint>& drawables) {
     GrTextStrike* grStrike = fStrike.get();
     SkScalar strikeToSource = fStrikeSpec.strikeToSourceRatio();
@@ -91,9 +120,9 @@ void GrTextBlob::SubRun::appendGlyphs(const SkZip<SkGlyphVariant, SkPoint>& draw
         SkRect dstRect;
         if (!this->needsTransform()) {
             pos = {SkScalarFloorToScalar(pos.x()), SkScalarFloorToScalar(pos.y())};
-            dstRect = grGlyph->destRect(pos);
+            dstRect = dest_rect(*skGlyph, pos);
         } else {
-            dstRect = grGlyph->destRect(pos, strikeToSource);
+            dstRect = dest_rect(*skGlyph, pos, strikeToSource);
         }
 
         this->joinGlyphBounds(dstRect);
@@ -236,8 +265,8 @@ void GrTextBlob::SubRun::updateTexCoords(int begin, int end) {
         GrGlyph* glyph = this->fGlyphs[i];
         SkASSERT(glyph != nullptr);
 
-        int width = glyph->fBounds.width();
+        int width = glyph->width();
-        int height = glyph->fBounds.height();
+        int height = glyph->height();
         uint16_t u0, v0, u1, v1;
         if (this->drawAsDistanceFields()) {
             u0 = glyph->fAtlasLocation.fX + SK_DistanceFieldInset;
@@ -787,11 +816,13 @@ std::tuple<bool, int> GrTextBlob::VertexRegenerator::updateTextureCoordinates(
     int i = begin;
     for (; i < end; i++) {
         GrGlyph* grGlyph = fSubRun->fGlyphs[i];
-        SkASSERT(grGlyph && grGlyph->fMaskFormat == fSubRun->maskFormat());
+        SkASSERT(grGlyph);
 
-        if (!fFullAtlasManager->hasGlyph(grGlyph)) {
+        if (!fFullAtlasManager->hasGlyph(fSubRun->maskFormat(), grGlyph)) {
             const SkGlyph& skGlyph = *fMetricsAndImages->glyph(grGlyph->fPackedID);
-            if (skGlyph.image() == nullptr) { return {false, 0}; }
+            if (skGlyph.image() == nullptr) {
+                return {false, 0};
+            }
             code = grStrike->addGlyphToAtlas(skGlyph,
                     fSubRun->maskFormat(),
                     fSubRun->needsTransform(),
@@ -801,7 +832,8 @@ std::tuple<bool, int> GrTextBlob::VertexRegenerator::updateTextureCoordinates(
             }
         }
         fFullAtlasManager->addGlyphToBulkAndSetUseToken(
-                fSubRun->bulkUseToken(), grGlyph, tokenTracker->nextDrawToken());
+                fSubRun->bulkUseToken(),  fSubRun->maskFormat(), grGlyph,
+                tokenTracker->nextDrawToken());
     }
     int glyphsPlacedInAtlas = i - begin;
 

src/gpu/text/GrTextBlob.h

@@ -28,7 +28,7 @@
 
 class GrAtlasManager;
 class GrAtlasTextOp;
-struct GrGlyph;
+class GrGlyph;
 
 class SkTextBlob;
 class SkTextBlobRunIterator;