From da65a8ea74d2748c93015fb5521da678e29e6548 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Mon, 30 Apr 2018 21:02:03 +0000 Subject: Saner memory managment in cttfontdesc, part 1. A subsequent patch will tackle the ad-hoc ref counting, but we can tidy this before going down that hole. Decouple CTTFontDesc creation from face setting. Remove union and treat single-entry case as vector's first element. Pass unique_ptr to prove memory ownership. Change-Id: Ic427798da04f3afbb65a56ee10045b9f22457a73 Reviewed-on: https://pdfium-review.googlesource.com/31730 Reviewed-by: dsinclair Commit-Queue: Tom Sepez --- core/fxge/cfx_fontmapper.cpp | 52 ++++++++++++++++++++++++-------------------- core/fxge/cfx_fontmgr.cpp | 47 +++++++++++++++++++++------------------ core/fxge/cfx_fontmgr.h | 5 +++-- core/fxge/cttfontdesc.cpp | 43 ++++++++++-------------------------- core/fxge/cttfontdesc.h | 26 ++++++++-------------- 5 files changed, 78 insertions(+), 95 deletions(-) diff --git a/core/fxge/cfx_fontmapper.cpp b/core/fxge/cfx_fontmapper.cpp index b3e88c00fe..c0b111a51e 100644 --- a/core/fxge/cfx_fontmapper.cpp +++ b/core/fxge/cfx_fontmapper.cpp @@ -751,22 +751,25 @@ FXFT_Face CFX_FontMapper::GetCachedTTCFace(void* hFont, const uint32_t tableTTCF, uint32_t ttc_size, uint32_t font_size) { - uint8_t buffer[1024]; - m_pFontInfo->GetFontData(hFont, tableTTCF, buffer, FX_ArraySize(buffer)); - uint32_t* pBuffer = reinterpret_cast(buffer); uint32_t checksum = 0; - for (int i = 0; i < 256; i++) - checksum += pBuffer[i]; - uint8_t* pFontData; - FXFT_Face face = m_pFontMgr->GetCachedTTCFace( - ttc_size, checksum, ttc_size - font_size, &pFontData); - if (!face) { - pFontData = FX_Alloc(uint8_t, ttc_size); - m_pFontInfo->GetFontData(hFont, tableTTCF, pFontData, ttc_size); - face = m_pFontMgr->AddCachedTTCFace(ttc_size, checksum, pFontData, ttc_size, - ttc_size - font_size); + { + uint8_t buffer[1024]; + m_pFontInfo->GetFontData(hFont, tableTTCF, buffer, sizeof(buffer)); + uint32_t* pBuffer = reinterpret_cast(buffer); + for (int i = 0; i < 256; i++) + checksum += pBuffer[i]; } - return face; + uint8_t* pIgnore = nullptr; + FXFT_Face face = m_pFontMgr->GetCachedTTCFace(ttc_size, checksum, + ttc_size - font_size, &pIgnore); + if (face) + return face; + + std::unique_ptr pFontData( + FX_Alloc(uint8_t, ttc_size)); + m_pFontInfo->GetFontData(hFont, tableTTCF, pFontData.get(), ttc_size); + return m_pFontMgr->AddCachedTTCFace(ttc_size, checksum, std::move(pFontData), + ttc_size, ttc_size - font_size); } FXFT_Face CFX_FontMapper::GetCachedFace(void* hFont, @@ -774,17 +777,18 @@ FXFT_Face CFX_FontMapper::GetCachedFace(void* hFont, int weight, bool bItalic, uint32_t font_size) { - uint8_t* pFontData; + uint8_t* pIgnore = nullptr; FXFT_Face face = - m_pFontMgr->GetCachedFace(SubstName, weight, bItalic, &pFontData); - if (!face) { - pFontData = FX_Alloc(uint8_t, font_size); - m_pFontInfo->GetFontData(hFont, 0, pFontData, font_size); - face = - m_pFontMgr->AddCachedFace(SubstName, weight, bItalic, pFontData, - font_size, m_pFontInfo->GetFaceIndex(hFont)); - } - return face; + m_pFontMgr->GetCachedFace(SubstName, weight, bItalic, &pIgnore); + if (face) + return face; + + std::unique_ptr pFontData( + FX_Alloc(uint8_t, font_size)); + m_pFontInfo->GetFontData(hFont, 0, pFontData.get(), font_size); + return m_pFontMgr->AddCachedFace(SubstName, weight, bItalic, + std::move(pFontData), font_size, + m_pFontInfo->GetFaceIndex(hFont)); } int PDF_GetStandardFontName(ByteString* name) { diff --git a/core/fxge/cfx_fontmgr.cpp b/core/fxge/cfx_fontmgr.cpp index 158b5d0cdb..2ee46373d5 100644 --- a/core/fxge/cfx_fontmgr.cpp +++ b/core/fxge/cfx_fontmgr.cpp @@ -126,19 +126,21 @@ FXFT_Face CFX_FontMgr::GetCachedFace(const ByteString& face_name, CTTFontDesc* pFontDesc = it->second.get(); *pFontData = pFontDesc->FontData(); pFontDesc->AddRef(); - return pFontDesc->SingleFace(); + return pFontDesc->GetFace(0); } -FXFT_Face CFX_FontMgr::AddCachedFace(const ByteString& face_name, - int weight, - bool bItalic, - uint8_t* pData, - uint32_t size, - int face_index) { +FXFT_Face CFX_FontMgr::AddCachedFace( + const ByteString& face_name, + int weight, + bool bItalic, + std::unique_ptr pData, + uint32_t size, + int face_index) { InitFTLibrary(); FXFT_Face face = nullptr; - int ret = FXFT_New_Memory_Face(m_FTLibrary, pData, size, face_index, &face); + int ret = + FXFT_New_Memory_Face(m_FTLibrary, pData.get(), size, face_index, &face); if (ret) return nullptr; @@ -146,10 +148,11 @@ FXFT_Face CFX_FontMgr::AddCachedFace(const ByteString& face_name, if (ret) return nullptr; - auto pFontDesc = pdfium::MakeUnique(pData, face); + auto pFontDesc = pdfium::MakeUnique(std::move(pData)); + pFontDesc->SetFace(0, face); CTTFontDesc* pResult = pFontDesc.get(); m_FaceMap[KeyNameFromFace(face_name, weight, bItalic)] = std::move(pFontDesc); - return pResult->SingleFace(); + return pResult->GetFace(0); } FXFT_Face CFX_FontMgr::GetCachedTTCFace(int ttc_size, @@ -163,22 +166,24 @@ FXFT_Face CFX_FontMgr::GetCachedTTCFace(int ttc_size, CTTFontDesc* pFontDesc = it->second.get(); *pFontData = pFontDesc->FontData(); int face_index = GetTTCIndex(pFontDesc->FontData(), ttc_size, font_offset); - if (!pFontDesc->TTCFace(face_index)) { - pFontDesc->SetTTCFace( + if (!pFontDesc->GetFace(face_index)) { + pFontDesc->SetFace( face_index, GetFixedFace(pFontDesc->FontData(), ttc_size, face_index)); } pFontDesc->AddRef(); - return pFontDesc->TTCFace(face_index); + return pFontDesc->GetFace(face_index); } -FXFT_Face CFX_FontMgr::AddCachedTTCFace(int ttc_size, - uint32_t checksum, - uint8_t* pData, - uint32_t size, - int font_offset) { - int face_index = GetTTCIndex(pData, ttc_size, font_offset); - FXFT_Face face = GetFixedFace(pData, ttc_size, face_index); - auto pFontDesc = pdfium::MakeUnique(pData, face_index, face); +FXFT_Face CFX_FontMgr::AddCachedTTCFace( + int ttc_size, + uint32_t checksum, + std::unique_ptr pData, + uint32_t size, + int font_offset) { + int face_index = GetTTCIndex(pData.get(), ttc_size, font_offset); + FXFT_Face face = GetFixedFace(pData.get(), ttc_size, face_index); + auto pFontDesc = pdfium::MakeUnique(std::move(pData)); + pFontDesc->SetFace(face_index, face); m_FaceMap[KeyNameFromSize(ttc_size, checksum)] = std::move(pFontDesc); return face; } diff --git a/core/fxge/cfx_fontmgr.h b/core/fxge/cfx_fontmgr.h index 6ca0970bdc..24c5ad3b6f 100644 --- a/core/fxge/cfx_fontmgr.h +++ b/core/fxge/cfx_fontmgr.h @@ -10,6 +10,7 @@ #include #include +#include "core/fxcrt/fx_memory.h" #include "core/fxge/fx_font.h" class CFX_FontMapper; @@ -31,7 +32,7 @@ class CFX_FontMgr { FXFT_Face AddCachedFace(const ByteString& face_name, int weight, bool bItalic, - uint8_t* pData, + std::unique_ptr pData, uint32_t size, int face_index); FXFT_Face GetCachedTTCFace(int ttc_size, @@ -40,7 +41,7 @@ class CFX_FontMgr { uint8_t** pFontData); FXFT_Face AddCachedTTCFace(int ttc_size, uint32_t checksum, - uint8_t* pData, + std::unique_ptr pData, uint32_t size, int font_offset); FXFT_Face GetFileFace(const char* filename, int face_index); diff --git a/core/fxge/cttfontdesc.cpp b/core/fxge/cttfontdesc.cpp index 97a12732e7..3d0b27b159 100644 --- a/core/fxge/cttfontdesc.cpp +++ b/core/fxge/cttfontdesc.cpp @@ -6,35 +6,26 @@ #include "core/fxge/cttfontdesc.h" +#include + #include "core/fxge/fx_freetype.h" #include "third_party/base/stl_util.h" -CTTFontDesc::CTTFontDesc(uint8_t* pData, FXFT_Face face) - : m_bIsTTC(false), m_pFontData(pData), m_SingleFace(face) {} - -CTTFontDesc::CTTFontDesc(uint8_t* pData, size_t index, FXFT_Face face) - : m_bIsTTC(true), m_pFontData(pData) { +CTTFontDesc::CTTFontDesc(std::unique_ptr pData) + : m_pFontData(std::move(pData)) { for (size_t i = 0; i < FX_ArraySize(m_TTCFaces); i++) m_TTCFaces[i] = nullptr; - SetTTCFace(index, face); } CTTFontDesc::~CTTFontDesc() { ASSERT(m_RefCount == 0); - if (m_bIsTTC) { - for (size_t i = 0; i < FX_ArraySize(m_TTCFaces); i++) { - if (m_TTCFaces[i]) - FXFT_Done_Face(m_TTCFaces[i]); - } - } else { - if (m_SingleFace) - FXFT_Done_Face(m_SingleFace); + for (size_t i = 0; i < FX_ArraySize(m_TTCFaces); i++) { + if (m_TTCFaces[i]) + FXFT_Done_Face(m_TTCFaces[i]); } - FX_Free(m_pFontData); } -void CTTFontDesc::SetTTCFace(size_t index, FXFT_Face face) { - ASSERT(m_bIsTTC); +void CTTFontDesc::SetFace(size_t index, FXFT_Face face) { ASSERT(index < FX_ArraySize(m_TTCFaces)); m_TTCFaces[index] = face; } @@ -45,24 +36,14 @@ void CTTFontDesc::AddRef() { } CTTFontDesc::ReleaseStatus CTTFontDesc::ReleaseFace(FXFT_Face face) { - if (m_bIsTTC) { - if (!pdfium::ContainsValue(m_TTCFaces, face)) - return kNotAppropriate; - } else { - if (m_SingleFace != face) - return kNotAppropriate; - } + if (!pdfium::ContainsValue(m_TTCFaces, face)) + return kNotAppropriate; + ASSERT(m_RefCount > 0); return --m_RefCount == 0 ? kReleased : kNotReleased; } -FXFT_Face CTTFontDesc::SingleFace() const { - ASSERT(!m_bIsTTC); - return m_SingleFace; -} - -FXFT_Face CTTFontDesc::TTCFace(size_t index) const { - ASSERT(m_bIsTTC); +FXFT_Face CTTFontDesc::GetFace(size_t index) const { ASSERT(index < FX_ArraySize(m_TTCFaces)); return m_TTCFaces[index]; } diff --git a/core/fxge/cttfontdesc.h b/core/fxge/cttfontdesc.h index bc0ea1737f..e73b03f6ec 100644 --- a/core/fxge/cttfontdesc.h +++ b/core/fxge/cttfontdesc.h @@ -7,6 +7,9 @@ #ifndef CORE_FXGE_CTTFONTDESC_H_ #define CORE_FXGE_CTTFONTDESC_H_ +#include + +#include "core/fxcrt/fx_memory.h" #include "core/fxcrt/fx_system.h" #include "core/fxge/fx_font.h" @@ -18,34 +21,23 @@ class CTTFontDesc { kNotReleased // Object still alive. }; - // Single face ctor. - CTTFontDesc(uint8_t* pData, FXFT_Face face); - - // TTC face ctor. - CTTFontDesc(uint8_t* pData, size_t index, FXFT_Face face); - + explicit CTTFontDesc(std::unique_ptr pData); ~CTTFontDesc(); - void SetTTCFace(size_t index, FXFT_Face face); + void SetFace(size_t index, FXFT_Face face); void AddRef(); // May not decrement refcount, depending on the value of |face|. ReleaseStatus ReleaseFace(FXFT_Face face); - uint8_t* FontData() const { return m_pFontData; } - - FXFT_Face SingleFace() const; - FXFT_Face TTCFace(size_t index) const; + uint8_t* FontData() const { return m_pFontData.get(); } + FXFT_Face GetFace(size_t index) const; private: - const bool m_bIsTTC; int m_RefCount = 1; - uint8_t* const m_pFontData; - union { - const FXFT_Face m_SingleFace; - FXFT_Face m_TTCFaces[16]; - }; + std::unique_ptr const m_pFontData; + FXFT_Face m_TTCFaces[16]; }; #endif // CORE_FXGE_CTTFONTDESC_H_ -- cgit v1.2.3