From 5fa68b4399d96d1b422af4e54336a0257931791c Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Tue, 31 Mar 2020 08:34:22 -0400 Subject: [PATCH] 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 Reviewed-by: Herb Derby --- src/gpu/GrGlyph.h | 57 ++++++--------------------------- src/gpu/text/GrAtlasManager.cpp | 8 ++--- src/gpu/text/GrAtlasManager.h | 6 ++-- src/gpu/text/GrStrikeCache.cpp | 18 ++--------- src/gpu/text/GrStrikeCache.h | 6 ---- src/gpu/text/GrTextBlob.cpp | 48 ++++++++++++++++++++++----- src/gpu/text/GrTextBlob.h | 2 +- 7 files changed, 60 insertions(+), 85 deletions(-) diff --git a/src/gpu/GrGlyph.h b/src/gpu/GrGlyph.h index a49fa337a9..015a757046 100644 --- a/src/gpu/GrGlyph.h +++ b/src/gpu/GrGlyph.h @@ -16,12 +16,8 @@ #include "include/private/SkChecksum.h" #include "include/private/SkFixed.h" -struct GrGlyph { - enum MaskStyle { - kCoverage_MaskStyle, - kDistance_MaskStyle - }; - +class GrGlyph { +public: 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) { - return skGlyph.maskFormat() == SkMask::kSDF_Format - ? GrGlyph::MaskStyle::kDistance_MaskStyle - : GrGlyph::MaskStyle::kCoverage_MaskStyle; + SkUNREACHABLE; } GrGlyph(const SkGlyph& skGlyph) - : fPackedID{skGlyph.getPackedID()} - , fMaskFormat{FormatFromSkGlyph(skGlyph.maskFormat())} - , 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())); + : fPackedID{skGlyph.getPackedID()} + , fWidthHeight(SkIPoint16::Make(skGlyph.width(), skGlyph.height())) { } - SkRect destRect(SkPoint origin, SkScalar textScale) { - if (fMaskStyle == kCoverage_MaskStyle) { - 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(); } + int width() const { return fWidthHeight.fX; } + int height() const { return fWidthHeight.fY; } uint32_t pageIndex() const { return GrDrawOpAtlas::GetPageIndexFromID(fPlotLocator); } - MaskStyle maskStyle() const { return fMaskStyle; } const SkPackedGlyphID fPackedID; - const GrMaskFormat fMaskFormat; - const MaskStyle fMaskStyle; - const GrIRect16 fBounds; + const SkIPoint16 fWidthHeight{0, 0}; + SkIPoint16 fAtlasLocation{0, 0}; GrDrawOpAtlas::PlotLocator fPlotLocator{GrDrawOpAtlas::kInvalidPlotLocator}; }; diff --git a/src/gpu/text/GrAtlasManager.cpp b/src/gpu/text/GrAtlasManager.cpp index bf4f0f34bf..c0bbeeed05 100644 --- a/src/gpu/text/GrAtlasManager.cpp +++ b/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); } } diff --git a/src/gpu/text/GrAtlasManager.h b/src/gpu/text/GrAtlasManager.h index c1434008d9..57b5c6c4be 100644 --- a/src/gpu/text/GrAtlasManager.h +++ b/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, diff --git a/src/gpu/text/GrStrikeCache.cpp b/src/gpu/text/GrStrikeCache.cpp index bb1ac7032d..46fa61d712 100644 --- a/src/gpu/text/GrStrikeCache.cpp +++ b/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 height = addPad ? grGlyph->height() + 2 : grGlyph->height(); + const int width = addPad ? skGlyph.width() + 2 : skGlyph.width(); + 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(*metricsAndImages->glyph(packed)); - fCache.set(grGlyph); - } - return grGlyph; -} diff --git a/src/gpu/text/GrStrikeCache.h b/src/gpu/text/GrStrikeCache.h index 7ff65593b9..ad320ebeea 100644 --- a/src/gpu/text/GrStrikeCache.h +++ b/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. diff --git a/src/gpu/text/GrTextBlob.cpp b/src/gpu/text/GrTextBlob.cpp index 7c2603fa52..a4cc0ca74b 100644 --- a/src/gpu/text/GrTextBlob.cpp +++ b/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& drawables) { GrTextStrike* grStrike = fStrike.get(); SkScalar strikeToSource = fStrikeSpec.strikeToSourceRatio(); @@ -91,9 +120,9 @@ void GrTextBlob::SubRun::appendGlyphs(const SkZip& 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 height = glyph->fBounds.height(); + int width = glyph->width(); + 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 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 GrTextBlob::VertexRegenerator::updateTextureCoordinates( } } fFullAtlasManager->addGlyphToBulkAndSetUseToken( - fSubRun->bulkUseToken(), grGlyph, tokenTracker->nextDrawToken()); + fSubRun->bulkUseToken(), fSubRun->maskFormat(), grGlyph, + tokenTracker->nextDrawToken()); } int glyphsPlacedInAtlas = i - begin; diff --git a/src/gpu/text/GrTextBlob.h b/src/gpu/text/GrTextBlob.h index c9b81ecf58..355be0b206 100644 --- a/src/gpu/text/GrTextBlob.h +++ b/src/gpu/text/GrTextBlob.h @@ -28,7 +28,7 @@ class GrAtlasManager; class GrAtlasTextOp; -struct GrGlyph; +class GrGlyph; class SkTextBlob; class SkTextBlobRunIterator;