Replace CountedFaceCache with RetainPtr<CFX_FaceCache> Removes some add-hoc refcounting in favor of smart pointers. The combination of Retainable and Observable creates a cache entry that remains valid so long as there is at least one retain ptr outstanding. Change-Id: I14f3c0b3ec42d9b845b6fb3965b0d7e6f9fc1a28 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/54272 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fxge/cfx_facecache.h b/core/fxge/cfx_facecache.h index 224dd37..a99ddb3 100644 --- a/core/fxge/cfx_facecache.h +++ b/core/fxge/cfx_facecache.h
@@ -12,6 +12,8 @@ #include <tuple> #include "core/fxcrt/fx_string.h" +#include "core/fxcrt/observable.h" +#include "core/fxcrt/retain_ptr.h" #include "core/fxge/fx_freetype.h" #if defined _SKIA_SUPPORT_ || _SKIA_SUPPORT_PATHS_ @@ -24,10 +26,13 @@ class CFX_Matrix; class CFX_PathData; -class CFX_FaceCache { +class CFX_FaceCache : public Retainable, public Observable<CFX_FaceCache> { public: - explicit CFX_FaceCache(FXFT_Face face); - ~CFX_FaceCache(); + template <typename T, typename... Args> + friend RetainPtr<T> pdfium::MakeRetain(Args&&... args); + + ~CFX_FaceCache() override; + const CFX_GlyphBitmap* LoadGlyphBitmap(const CFX_Font* pFont, uint32_t glyph_index, bool bFontStyle, @@ -44,6 +49,8 @@ #endif private: + explicit CFX_FaceCache(FXFT_Face face); + using SizeGlyphCache = std::map<uint32_t, std::unique_ptr<CFX_GlyphBitmap>>; // <glyph_index, width, weight, angle, vertical> using PathMapKey = std::tuple<uint32_t, uint32_t, int, int, bool>;
diff --git a/core/fxge/cfx_font.cpp b/core/fxge/cfx_font.cpp index 79b57f0..2f39a0b 100644 --- a/core/fxge/cfx_font.cpp +++ b/core/fxge/cfx_font.cpp
@@ -581,18 +581,14 @@ return true; } -CFX_FaceCache* CFX_Font::GetFaceCache() const { +RetainPtr<CFX_FaceCache> CFX_Font::GetOrCreateFaceCache() const { if (!m_FaceCache) m_FaceCache = CFX_GEModule::Get()->GetFontCache()->GetCachedFace(this); - return m_FaceCache.Get(); + return m_FaceCache; } void CFX_Font::ClearFaceCache() { - if (!m_FaceCache) - return; - m_FaceCache = nullptr; - CFX_GEModule::Get()->GetFontCache()->ReleaseCachedFace(this); } void CFX_Font::AdjustMMParams(int glyph_index, @@ -713,17 +709,18 @@ uint32_t dest_width, int anti_alias, int* pTextFlags) const { - return GetFaceCache()->LoadGlyphBitmap(this, glyph_index, bFontStyle, matrix, - dest_width, anti_alias, pTextFlags); + return GetOrCreateFaceCache()->LoadGlyphBitmap(this, glyph_index, bFontStyle, + matrix, dest_width, anti_alias, + pTextFlags); } const CFX_PathData* CFX_Font::LoadGlyphPath(uint32_t glyph_index, uint32_t dest_width) const { - return GetFaceCache()->LoadGlyphPath(this, glyph_index, dest_width); + return GetOrCreateFaceCache()->LoadGlyphPath(this, glyph_index, dest_width); } #if defined _SKIA_SUPPORT_ || _SKIA_SUPPORT_PATHS_ CFX_TypeFace* CFX_Font::GetDeviceCache() const { - return GetFaceCache()->GetDeviceCache(this); + return GetOrCreateFaceCache()->GetDeviceCache(this); } #endif
diff --git a/core/fxge/cfx_font.h b/core/fxge/cfx_font.h index 8fe39ea..ab3fe6d 100644 --- a/core/fxge/cfx_font.h +++ b/core/fxge/cfx_font.h
@@ -126,7 +126,7 @@ #endif // PDF_ENABLE_XFA private: - CFX_FaceCache* GetFaceCache() const; + RetainPtr<CFX_FaceCache> GetOrCreateFaceCache() const; void DeleteFace(); void ClearFaceCache(); #if defined(OS_MACOSX) @@ -136,7 +136,7 @@ ByteString GetFamilyNameOrUntitled() const; mutable UnownedPtr<FXFT_FaceRec> m_Face; - mutable UnownedPtr<CFX_FaceCache> m_FaceCache; + mutable RetainPtr<CFX_FaceCache> m_FaceCache; std::unique_ptr<CFX_SubstFont> m_pSubstFont; std::unique_ptr<uint8_t, FxFreeDeleter> m_pGsubData; std::vector<uint8_t> m_pFontDataAllocation;
diff --git a/core/fxge/cfx_fontcache.cpp b/core/fxge/cfx_fontcache.cpp index 5ccac8c..b9300bc 100644 --- a/core/fxge/cfx_fontcache.cpp +++ b/core/fxge/cfx_fontcache.cpp
@@ -10,40 +10,26 @@ #include <utility> #include "core/fxge/cfx_facecache.h" +#include "core/fxge/cfx_font.h" #include "core/fxge/fx_font.h" #include "core/fxge/fx_freetype.h" #include "third_party/base/ptr_util.h" -CFX_FontCache::CountedFaceCache::CountedFaceCache() {} +CFX_FontCache::CFX_FontCache() = default; -CFX_FontCache::CountedFaceCache::~CountedFaceCache() {} +CFX_FontCache::~CFX_FontCache() = default; -CFX_FontCache::CFX_FontCache() {} - -CFX_FontCache::~CFX_FontCache() { - ASSERT(m_ExtFaceMap.empty()); - ASSERT(m_FTFaceMap.empty()); -} - -CFX_FaceCache* CFX_FontCache::GetCachedFace(const CFX_Font* pFont) { +RetainPtr<CFX_FaceCache> CFX_FontCache::GetCachedFace(const CFX_Font* pFont) { FXFT_Face face = pFont->GetFace(); const bool bExternal = !face; - CFX_FTCacheMap& map = bExternal ? m_ExtFaceMap : m_FTFaceMap; + auto& map = bExternal ? m_ExtFaceMap : m_FTFaceMap; auto it = map.find(face); - if (it != map.end()) { - CountedFaceCache* counted_face_cache = it->second.get(); - counted_face_cache->m_nCount++; - return counted_face_cache->m_Obj.get(); - } + if (it != map.end() && it->second) + return pdfium::WrapRetain(it->second.Get()); - auto counted_face_cache = pdfium::MakeUnique<CountedFaceCache>(); - counted_face_cache->m_nCount = 2; - auto new_cache = - pdfium::MakeUnique<CFX_FaceCache>(bExternal ? nullptr : face); - CFX_FaceCache* face_cache = new_cache.get(); - counted_face_cache->m_Obj = std::move(new_cache); - map[face] = std::move(counted_face_cache); - return face_cache; + auto new_cache = pdfium::MakeRetain<CFX_FaceCache>(face); + map[face].Reset(new_cache.Get()); + return new_cache; } #ifdef _SKIA_SUPPORT_ @@ -51,20 +37,3 @@ return GetCachedFace(pFont)->GetDeviceCache(pFont); } #endif - -void CFX_FontCache::ReleaseCachedFace(const CFX_Font* pFont) { - FXFT_Face face = pFont->GetFace(); - const bool bExternal = !face; - CFX_FTCacheMap& map = bExternal ? m_ExtFaceMap : m_FTFaceMap; - - auto it = map.find(face); - if (it == map.end()) - return; - - CountedFaceCache* counted_face_cache = it->second.get(); - if (counted_face_cache->m_nCount > 2) { - counted_face_cache->m_nCount--; - } else { - map.erase(it); - } -}
diff --git a/core/fxge/cfx_fontcache.h b/core/fxge/cfx_fontcache.h index 44a4576..516e779 100644 --- a/core/fxge/cfx_fontcache.h +++ b/core/fxge/cfx_fontcache.h
@@ -11,32 +11,24 @@ #include <memory> #include "core/fxcrt/fx_system.h" -#include "core/fxge/cfx_font.h" +#include "core/fxge/cfx_facecache.h" #include "core/fxge/fx_freetype.h" -class CFX_FaceCache; +class CFX_Font; class CFX_FontCache { public: CFX_FontCache(); ~CFX_FontCache(); - CFX_FaceCache* GetCachedFace(const CFX_Font* pFont); - void ReleaseCachedFace(const CFX_Font* pFont); + + RetainPtr<CFX_FaceCache> GetCachedFace(const CFX_Font* pFont); #ifdef _SKIA_SUPPORT_ CFX_TypeFace* GetDeviceCache(const CFX_Font* pFont); #endif private: - struct CountedFaceCache { - CountedFaceCache(); - ~CountedFaceCache(); - std::unique_ptr<CFX_FaceCache> m_Obj; - uint32_t m_nCount; - }; - - using CFX_FTCacheMap = std::map<FXFT_Face, std::unique_ptr<CountedFaceCache>>; - CFX_FTCacheMap m_FTFaceMap; - CFX_FTCacheMap m_ExtFaceMap; + std::map<FXFT_Face, CFX_FaceCache::ObservedPtr> m_FTFaceMap; + std::map<FXFT_Face, CFX_FaceCache::ObservedPtr> m_ExtFaceMap; }; #endif // CORE_FXGE_CFX_FONTCACHE_H_
diff --git a/core/fxge/win32/cfx_psrenderer.cpp b/core/fxge/win32/cfx_psrenderer.cpp index f2050c3..d684600 100644 --- a/core/fxge/win32/cfx_psrenderer.cpp +++ b/core/fxge/win32/cfx_psrenderer.cpp
@@ -664,11 +664,11 @@ << pObject2Device->e << " " << pObject2Device->f << "]cm\n"; CFX_FontCache* pCache = CFX_GEModule::Get()->GetFontCache(); - CFX_FaceCache* pFaceCache = pCache->GetCachedFace(pFont); + RetainPtr<CFX_FaceCache> pFaceCache = pCache->GetCachedFace(pFont); int last_fontnum = -1; for (int i = 0; i < nChars; i++) { int ps_fontnum, ps_glyphindex; - FindPSFontGlyph(pFaceCache, pFont, pCharPos[i], &ps_fontnum, + FindPSFontGlyph(pFaceCache.Get(), pFont, pCharPos[i], &ps_fontnum, &ps_glyphindex); if (last_fontnum != ps_fontnum) { buf << "/X" << ps_fontnum << " Ff " << font_size << " Fs Sf "; @@ -680,7 +680,6 @@ } buf << "Q\n"; WriteToStream(&buf); - pCache->ReleaseCachedFace(pFont); return true; }