From 60a7bb036d9b39eb1a08512dd199db0960cbc4d1 Mon Sep 17 00:00:00 2001 From: Lee Salzman Date: Sun, 22 Sep 2019 19:56:00 +0000 Subject: [PATCH] Bug 1582749 - query FreeType glyph advance and bounds at the same time. r=jfkthame Cairo would normally query both the advance and other metrics at the same time, then store them in a glyph cache sitting on each cairo_scaled_font_t any time any of the extents were queried. Each cached scaled glyph metrics would require about 150 bytes of space and could thus use a horribly large amount of memory when a lot of glyphs were being used within a scaled font. This tries to duplicate the behavior of querying and storing both advance and bounds at the same time to effectively cut the number of glyph loads in half for most cases. This should only add another 8 bytes per hash entry to store the cached bounds, thus putting us way ahead on memory usage compared to what Cairo did under the hood. Further, Cairo would keep around cairo_scaled_font_t's in a holdover cache even after there are no existing references to them and the owning gfxFonts have long since died. This gives an artificial boost in successive runs of the benchmark, while not aiding in the performance of the first run. I don't believe the extra memory use would be justified to reproduce that particular behavior, especially since our expectations are that the glyph cache for a gfxFont dies when the gfxFont itself dies from the gfxFontCache. In any case, this should at least significantly boost our glyph metrics performance on a cold start, with the caveat about the warm start case. Differential Revision: https://phabricator.services.mozilla.com/D46726 --HG-- extra : moz-landing-system : lando --- gfx/thebes/gfxFT2FontBase.cpp | 86 ++++++++++++++++++++++++----------- gfx/thebes/gfxFT2FontBase.h | 48 ++++++++++++++++++- 2 files changed, 106 insertions(+), 28 deletions(-) diff --git a/gfx/thebes/gfxFT2FontBase.cpp b/gfx/thebes/gfxFT2FontBase.cpp index bcdd1d60a3e3..8bf1bf5fff69 100644 --- a/gfx/thebes/gfxFT2FontBase.cpp +++ b/gfx/thebes/gfxFT2FontBase.cpp @@ -138,6 +138,14 @@ static void SnapLineToPixels(gfxFloat& aOffset, gfxFloat& aSize) { aSize = snappedSize; } +static inline gfxRect ScaleGlyphBounds(const IntRect& aBounds, + gfxFloat aScale) { + return gfxRect(FLOAT_FROM_26_6(aBounds.x) * aScale, + FLOAT_FROM_26_6(aBounds.y) * aScale, + FLOAT_FROM_26_6(aBounds.width) * aScale, + FLOAT_FROM_26_6(aBounds.height) * aScale); +} + /** * Get extents for a simple character representable by a single glyph. * The return value is the glyph id of that glyph or zero if no such glyph @@ -148,10 +156,14 @@ uint32_t gfxFT2FontBase::GetCharExtents(char aChar, gfxFloat* aWidth, gfxRect* aBounds) { FT_UInt gid = GetGlyph(aChar); int32_t width; - if (gid && GetFTGlyphExtents(gid, &width, aBounds)) { + IntRect bounds; + if (gid && GetFTGlyphExtents(gid, &width, &bounds)) { if (aWidth) { *aWidth = FLOAT_FROM_16_16(width); } + if (aBounds) { + *aBounds = ScaleGlyphBounds(bounds, GetAdjustedSize() / mFTSize); + } return gid; } else { return 0; @@ -489,7 +501,7 @@ uint32_t gfxFT2FontBase::GetGlyph(uint32_t unicode, } FT_Vector gfxFT2FontBase::GetEmboldenStrength(FT_Face aFace) { - FT_Vector strength = { 0, 0 }; + FT_Vector strength = {0, 0}; if (!mEmbolden) { return strength; } @@ -508,7 +520,7 @@ FT_Vector gfxFT2FontBase::GetEmboldenStrength(FT_Face aFace) { } bool gfxFT2FontBase::GetFTGlyphExtents(uint16_t aGID, int32_t* aAdvance, - gfxRect* aBounds) { + IntRect* aBounds) { gfxFT2LockedFace face(this); MOZ_ASSERT(face.get()); if (!face.get()) { @@ -517,8 +529,7 @@ bool gfxFT2FontBase::GetFTGlyphExtents(uint16_t aGID, int32_t* aAdvance, return false; } - FT_Error ftError = Factory::LoadFTGlyph(face.get(), aGID, mFTLoadFlags); - if (ftError != FT_Err_Ok) { + if (Factory::LoadFTGlyph(face.get(), aGID, mFTLoadFlags) != FT_Err_Ok) { // FT_Face was somehow broken/invalid? Don't try to access glyph slot. // This probably shouldn't happen, but does: see bug 1440938. NS_WARNING("failed to load glyph!"); @@ -572,36 +583,59 @@ bool gfxFT2FontBase::GetFTGlyphExtents(uint16_t aGID, int32_t* aAdvance, x2 = (x2 + 63) & -64; y2 = (y2 + 63) & -64; } - *aBounds = gfxRect(FLOAT_FROM_26_6(x) * extentsScale, - FLOAT_FROM_26_6(y) * extentsScale, - FLOAT_FROM_26_6(x2 - x) * extentsScale, - FLOAT_FROM_26_6(y2 - y) * extentsScale); + *aBounds = IntRect(x, y, x2 - x, y2 - y); } return true; } +/** + * Get the cached glyph metrics for the glyph id if available. Otherwise, query + * FreeType for the glyph extents and initialize the glyph metrics. + */ +const gfxFT2FontBase::GlyphMetrics& gfxFT2FontBase::GetCachedGlyphMetrics( + uint16_t aGID, IntRect* aBounds) { + if (!mGlyphMetrics) { + mGlyphMetrics = + mozilla::MakeUnique>( + 128); + } + + if (const GlyphMetrics* metrics = mGlyphMetrics->GetValue(aGID)) { + return *metrics; + } + + GlyphMetrics& metrics = mGlyphMetrics->GetOrInsert(aGID); + IntRect bounds; + if (GetFTGlyphExtents(aGID, &metrics.mAdvance, &bounds)) { + metrics.SetBounds(bounds); + if (aBounds) { + *aBounds = bounds; + } + } + return metrics; +} + int32_t gfxFT2FontBase::GetGlyphWidth(uint16_t aGID) { - if (!mGlyphWidths) { - mGlyphWidths = - mozilla::MakeUnique>(128); - } - - int32_t width; - if (mGlyphWidths->Get(aGID, &width)) { - return width; - } - - if (!GetFTGlyphExtents(aGID, &width)) { - width = 0; - } - mGlyphWidths->Put(aGID, width); - - return width; + return GetCachedGlyphMetrics(aGID).mAdvance; } bool gfxFT2FontBase::GetGlyphBounds(uint16_t aGID, gfxRect* aBounds, bool aTight) { - return GetFTGlyphExtents(aGID, nullptr, aBounds); + IntRect bounds; + const GlyphMetrics& metrics = GetCachedGlyphMetrics(aGID, &bounds); + if (!metrics.HasValidBounds()) { + return false; + } + // Check if there are cached bounds and use those if available. Otherwise, + // fall back to directly querying the glyph extents. + if (metrics.HasCachedBounds()) { + bounds = metrics.GetBounds(); + } else if (bounds.IsEmpty() && !GetFTGlyphExtents(aGID, nullptr, &bounds)) { + return false; + } + // The bounds are stored unscaled, so must be scaled to the adjusted size. + *aBounds = ScaleGlyphBounds(bounds, GetAdjustedSize() / mFTSize); + return true; } // For variation fonts, figure out the variation coordinates to be applied diff --git a/gfx/thebes/gfxFT2FontBase.h b/gfx/thebes/gfxFT2FontBase.h index 63aac5dfc63f..429c5c6fabb2 100644 --- a/gfx/thebes/gfxFT2FontBase.h +++ b/gfx/thebes/gfxFT2FontBase.h @@ -48,7 +48,7 @@ class gfxFT2FontBase : public gfxFont { // Get advance (and optionally bounds) of a single glyph from FreeType, // and return true, or return false if we failed. bool GetFTGlyphExtents(uint16_t aGID, int32_t* aWidth, - gfxRect* aBounds = nullptr); + mozilla::gfx::IntRect* aBounds = nullptr); protected: void InitMetrics(); @@ -70,7 +70,51 @@ class gfxFT2FontBase : public gfxFont { // range reported by the face. nsTArray mCoords; - mozilla::UniquePtr> mGlyphWidths; + // Store cached glyph metrics for reasonably small glyph sizes. The bounds + // are stored unscaled to losslessly compress 26.6 fixed point to an int16_t. + // Larger glyphs are handled directly via GetFTGlyphExtents. + struct GlyphMetrics { + // Set the X coord to INT16_MIN to signal the bounds are invalid, or + // INT16_MAX to signal that the bounds would overflow an int16_t. + enum { INVALID = INT16_MIN, LARGE = INT16_MAX }; + + GlyphMetrics() : mAdvance(0), mX(INVALID), mY(0), mWidth(0), mHeight(0) {} + + bool HasValidBounds() const { return mX != INVALID; } + bool HasCachedBounds() const { return mX != LARGE; } + + // If the bounds can fit in an int16_t, set them. Otherwise, leave the + // bounds invalid to signal that GetFTGlyphExtents should be queried + // directly. + void SetBounds(const mozilla::gfx::IntRect& aBounds) { + if (aBounds.x > INT16_MIN && aBounds.x < INT16_MAX && + aBounds.y > INT16_MIN && aBounds.y < INT16_MAX && + aBounds.width <= UINT16_MAX && aBounds.height <= UINT16_MAX) { + mX = aBounds.x; + mY = aBounds.y; + mWidth = aBounds.width; + mHeight = aBounds.height; + } else { + mX = LARGE; + } + } + + mozilla::gfx::IntRect GetBounds() const { + return mozilla::gfx::IntRect(mX, mY, mWidth, mHeight); + } + + int32_t mAdvance; + int16_t mX; + int16_t mY; + uint16_t mWidth; + uint16_t mHeight; + }; + + const GlyphMetrics& GetCachedGlyphMetrics( + uint16_t aGID, mozilla::gfx::IntRect* aBounds = nullptr); + + mozilla::UniquePtr> + mGlyphMetrics; }; // Helper classes used for clearing out user font data when FT font