Consolidate CFX_Face::Open() callers
This changeset consolidates the code in callers of CFX_Face::Open(). The
functions CFX_Font::LoadFile() and LoadFace() in cfgas_fontmgr.cpp are
two callers which share nearly identical logic. Though with fewer shared
aspects, android/cfpf_skiafontmgr.cpp also uses the consolidated code.
The code is consolidated into a function called OpenFromStream() and an
edited Open() function, with the former invoking the latter. These
functions are utilised in the callers to continue working toward the
goals of freetype encapsulation.
Bug: 452087741
Change-Id: Ic745d6b3c29b0736c38aaddafcc76e815976ac96
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/137550
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fxge/cfx_face.cpp b/core/fxge/cfx_face.cpp
index 602265a..995affc 100644
--- a/core/fxge/cfx_face.cpp
+++ b/core/fxge/cfx_face.cpp
@@ -18,7 +18,7 @@
#include "core/fxcrt/numerics/clamped_math.h"
#include "core/fxcrt/numerics/safe_conversions.h"
#include "core/fxcrt/numerics/safe_math.h"
-#include "core/fxcrt/span.h"
+#include "core/fxcrt/unowned_ptr.h"
#include "core/fxge/cfx_font.h"
#include "core/fxge/cfx_fontmgr.h"
#include "core/fxge/cfx_gemodule.h"
@@ -283,7 +283,24 @@
}
NOTREACHED();
}
-
+#if BUILDFLAG(IS_ANDROID) || defined(PDF_ENABLE_XFA)
+unsigned long FTStreamRead(FXFT_StreamRec* stream,
+ unsigned long offset,
+ unsigned char* buffer,
+ unsigned long count) {
+ if (count == 0) {
+ return 0;
+ }
+ IFX_SeekableReadStream* pFile =
+ static_cast<IFX_SeekableReadStream*>(stream->descriptor.pointer);
+ // SAFETY: caller ensures `buffer` points to at least `count` bytes.
+ return pFile && pFile->ReadBlockAtOffset(
+ UNSAFE_BUFFERS(pdfium::span(buffer, count)), offset)
+ ? count
+ : 0;
+}
+void FTStreamClose(FXFT_StreamRec* stream) {}
+#endif
} // namespace
// static
@@ -306,6 +323,9 @@
RetainPtr<CFX_Face> CFX_Face::Open(FT_Library library,
const FT_Open_Args* args,
FT_Long face_index) {
+ if (!library) {
+ return nullptr;
+ }
FXFT_FaceRec* pRec = nullptr;
if (FT_Open_Face(library, args, face_index, &pRec) != 0) {
return nullptr;
@@ -314,6 +334,40 @@
// Private ctor.
return pdfium::WrapRetain(new CFX_Face(pRec, nullptr));
}
+
+RetainPtr<CFX_Face> CFX_Face::OpenFromStream(
+ FT_Library library,
+ const RetainPtr<IFX_SeekableReadStream>& font_stream,
+ FT_Long face_index) {
+ if (!font_stream) {
+ return nullptr;
+ }
+ auto ft_stream = std::make_unique<FXFT_StreamRec>();
+ *ft_stream = {}; // Aggregate initialization.
+ static_assert(
+ std::is_aggregate_v<std::remove_pointer_t<decltype(ft_stream.get())>>);
+ ft_stream->base = nullptr;
+ ft_stream->descriptor.pointer = static_cast<void*>(font_stream.Get());
+ ft_stream->pos = 0;
+ ft_stream->size = static_cast<unsigned long>(font_stream->GetSize());
+ ft_stream->read = FTStreamRead;
+ ft_stream->close = FTStreamClose;
+
+ FT_Open_Args ft_args = {}; // Aggregate initialization.
+ static_assert(std::is_aggregate_v<decltype(ft_args)>);
+ ft_args.flags = FT_OPEN_STREAM;
+ ft_args.stream = ft_stream.get();
+
+ RetainPtr<CFX_Face> face = Open(library, &ft_args, face_index);
+ if (!face) {
+ return nullptr;
+ }
+ face->SetPixelSize(0, 64);
+ face->owned_font_stream_ = std::move(font_stream);
+ face->owned_stream_rec_ = std::move(ft_stream);
+ return face;
+}
+
#endif
bool CFX_Face::HasGlyphNames() const {
diff --git a/core/fxge/cfx_face.h b/core/fxge/cfx_face.h
index ee10f1c..de77cd9 100644
--- a/core/fxge/cfx_face.h
+++ b/core/fxge/cfx_face.h
@@ -15,6 +15,7 @@
#include "build/build_config.h"
#include "core/fxcrt/bytestring.h"
#include "core/fxcrt/fx_coordinates.h"
+#include "core/fxcrt/fx_stream.h"
#include "core/fxcrt/observed_ptr.h"
#include "core/fxcrt/retain_ptr.h"
#include "core/fxcrt/span.h"
@@ -68,6 +69,10 @@
static RetainPtr<CFX_Face> Open(FT_Library library,
const FT_Open_Args* args,
FT_Long face_index);
+ static RetainPtr<CFX_Face> OpenFromStream(
+ FT_Library library,
+ const RetainPtr<IFX_SeekableReadStream>& font_stream,
+ FT_Long face_index);
#endif
bool HasGlyphNames() const;
@@ -154,6 +159,10 @@
void AdjustVariationParams(int glyph_index, int dest_width, int weight);
std::optional<std::array<uint8_t, 2>> GetOs2Panose();
+ // `owned_font_stream_` must outlive `owned_stream_rec_`.
+ RetainPtr<IFX_SeekableReadStream> owned_font_stream_;
+ // `owned_stream_rec_` must outlive `rec_`.
+ std::unique_ptr<FXFT_StreamRec> owned_stream_rec_;
ScopedFXFTFaceRec const rec_;
RetainPtr<Retainable> const desc_;
};
diff --git a/core/fxge/cfx_font.cpp b/core/fxge/cfx_font.cpp
index 06caf1c..a6730ee 100644
--- a/core/fxge/cfx_font.cpp
+++ b/core/fxge/cfx_font.cpp
@@ -72,28 +72,6 @@
right * 1000 / x_scale, bottom * 1000 / y_scale);
}
-#ifdef PDF_ENABLE_XFA
-unsigned long FTStreamRead(FXFT_StreamRec* stream,
- unsigned long offset,
- unsigned char* buffer,
- unsigned long count) {
- if (count == 0) {
- return 0;
- }
-
- IFX_SeekableReadStream* pFile =
- static_cast<IFX_SeekableReadStream*>(stream->descriptor.pointer);
-
- // SAFETY: caller ensures `buffer` points to at least `count` bytes.
- return pFile && pFile->ReadBlockAtOffset(
- UNSAFE_BUFFERS(pdfium::span(buffer, count)), offset)
- ? count
- : 0;
-}
-
-void FTStreamClose(FXFT_StreamRec* stream) {}
-#endif // PDF_ENABLE_XFA
-
bool ShouldAppendStyle(const ByteString& style) {
return !style.IsEmpty() && style != "Regular";
}
@@ -197,29 +175,9 @@
bool CFX_Font::LoadFile(RetainPtr<IFX_SeekableReadStream> pFile,
int nFaceIndex) {
object_tag_ = 0;
-
- auto pStreamRec = std::make_unique<FXFT_StreamRec>();
- pStreamRec->base = nullptr;
- pStreamRec->size = static_cast<unsigned long>(pFile->GetSize());
- pStreamRec->pos = 0;
- pStreamRec->descriptor.pointer = static_cast<void*>(pFile.Get());
- pStreamRec->close = FTStreamClose;
- pStreamRec->read = FTStreamRead;
-
- FT_Open_Args args;
- args.flags = FT_OPEN_STREAM;
- args.stream = pStreamRec.get();
-
- face_ = CFX_Face::Open(CFX_GEModule::Get()->GetFontMgr()->GetFTLibrary(),
- &args, nFaceIndex);
- if (!face_) {
- return false;
- }
-
- owned_file_ = std::move(pFile);
- owned_stream_rec_ = std::move(pStreamRec);
- face_->SetPixelSize(0, 64);
- return true;
+ face_ = CFX_Face::OpenFromStream(
+ CFX_GEModule::Get()->GetFontMgr()->GetFTLibrary(), pFile, nFaceIndex);
+ return !!face_;
}
#if !BUILDFLAG(IS_WIN)
diff --git a/core/fxge/cfx_font.h b/core/fxge/cfx_font.h
index 465503a..e709524 100644
--- a/core/fxge/cfx_font.h
+++ b/core/fxge/cfx_font.h
@@ -22,7 +22,6 @@
#include "core/fxcrt/span.h"
#include "core/fxcrt/unowned_ptr_exclusion.h"
#include "core/fxge/cfx_face.h"
-#include "core/fxge/freetype/fx_freetype.h"
#if defined(PDF_USE_SKIA)
#include "core/fxge/fx_font.h"
@@ -141,12 +140,6 @@
#endif
ByteString GetFamilyNameOrUntitled() const;
-#if defined(PDF_ENABLE_XFA)
- // |owned_file_| must outlive |owned_stream_rec_|.
- RetainPtr<IFX_SeekableReadStream> owned_file_;
- std::unique_ptr<FXFT_StreamRec> owned_stream_rec_; // Must outlive |face_|.
-#endif
-
mutable RetainPtr<CFX_Face> face_;
mutable RetainPtr<CFX_GlyphCache> glyph_cache_;
std::unique_ptr<CFX_SubstFont> subst_font_;
diff --git a/xfa/fgas/font/cfgas_fontmgr.cpp b/xfa/fgas/font/cfgas_fontmgr.cpp
index b36b618..9dda582 100644
--- a/xfa/fgas/font/cfgas_fontmgr.cpp
+++ b/xfa/fgas/font/cfgas_fontmgr.cpp
@@ -417,31 +417,6 @@
return fxcrt::GetUInt16MSBFirst(data.subspan(offset).first<2u>());
}
-extern "C" {
-
-unsigned long ftStreamRead(FXFT_StreamRec* stream,
- unsigned long offset,
- unsigned char* buffer,
- unsigned long count) {
- if (count == 0) {
- return 0;
- }
-
- IFX_SeekableReadStream* pFile =
- static_cast<IFX_SeekableReadStream*>(stream->descriptor.pointer);
-
- // SAFETY: required from caller.
- if (!pFile->ReadBlockAtOffset(
- UNSAFE_BUFFERS(pdfium::span(buffer, count), offset))) {
- return 0;
- }
- return count;
-}
-
-void ftStreamClose(FXFT_StreamRec* stream) {}
-
-} // extern "C"
-
std::vector<WideString> GetNames(pdfium::span<const uint8_t> name_table) {
std::vector<WideString> results;
if (name_table.empty()) {
@@ -514,46 +489,12 @@
return nullptr;
}
-struct FTStreamRecAndFace {
- std::unique_ptr<FXFT_StreamRec> ft_stream; // Must outlive `face`.
- RetainPtr<CFX_Face> face;
-};
-
-FTStreamRecAndFace LoadFace(
+RetainPtr<CFX_Face> LoadFace(
const RetainPtr<IFX_SeekableReadStream>& font_stream,
int32_t iFaceIndex) {
- if (!font_stream) {
- return {nullptr, nullptr};
- }
-
CFX_FontMgr* font_mgr = CFX_GEModule::Get()->GetFontMgr();
FXFT_LibraryRec* library = font_mgr->GetFTLibrary();
- if (!library) {
- return {nullptr, nullptr};
- }
-
- auto ftStream = std::make_unique<FXFT_StreamRec>();
- *ftStream = {}; // Aggregate initialization.
- static_assert(
- std::is_aggregate_v<std::remove_pointer_t<decltype(ftStream.get())>>);
- ftStream->base = nullptr;
- ftStream->descriptor.pointer = static_cast<void*>(font_stream.Get());
- ftStream->pos = 0;
- ftStream->size = static_cast<unsigned long>(font_stream->GetSize());
- ftStream->read = ftStreamRead;
- ftStream->close = ftStreamClose;
-
- FT_Open_Args ftArgs = {}; // Aggregate initialization.
- static_assert(std::is_aggregate_v<decltype(ftArgs)>);
- ftArgs.flags |= FT_OPEN_STREAM;
- ftArgs.stream = ftStream.get();
-
- RetainPtr<CFX_Face> pFace = CFX_Face::Open(library, &ftArgs, iFaceIndex);
- if (!pFace) {
- return {nullptr, nullptr};
- }
- pFace->SetPixelSize(0, 64);
- return {std::move(ftStream), std::move(pFace)};
+ return CFX_Face::OpenFromStream(library, font_stream, iFaceIndex);
}
bool VerifyUnicodeForFontDescriptor(CFGAS_FontDescriptor* pDesc,
@@ -564,14 +505,11 @@
if (!pFileRead) {
return false;
}
- FTStreamRecAndFace ft_stream_and_face =
- LoadFace(pFileRead, pDesc->face_index_);
- if (!ft_stream_and_face.face) {
+ RetainPtr<CFX_Face> ft_face = LoadFace(pFileRead, pDesc->face_index_);
+ if (!ft_face) {
return false;
}
-
- pDesc->ft_stream_ = std::move(ft_stream_and_face.ft_stream);
- pDesc->face_ = std::move(ft_stream_and_face.face);
+ pDesc->face_ = std::move(ft_face);
}
return pDesc->face_->SelectCharMap(fxge::FontEncoding::kUnicode) &&
pDesc->face_->GetCharIndex(wcUnicode);
@@ -818,8 +756,7 @@
int32_t index = 0;
int32_t num_faces = 0;
do {
- FTStreamRecAndFace ft_stream_and_face = LoadFace(font_stream, index++);
- RetainPtr<CFX_Face>& face = ft_stream_and_face.face;
+ RetainPtr<CFX_Face> face = LoadFace(font_stream, index++);
if (!face) {
continue;
}
diff --git a/xfa/fgas/font/cfgas_fontmgr.h b/xfa/fgas/font/cfgas_fontmgr.h
index e70a021..fd39700 100644
--- a/xfa/fgas/font/cfgas_fontmgr.h
+++ b/xfa/fgas/font/cfgas_fontmgr.h
@@ -20,7 +20,6 @@
#include "core/fxcrt/retain_ptr.h"
#include "core/fxcrt/widestring.h"
#include "core/fxge/cfx_face.h"
-#include "core/fxge/freetype/fx_freetype.h"
class CFGAS_GEFont;
class IFX_SeekableReadStream;
@@ -63,7 +62,6 @@
int32_t face_index_ = 0;
uint32_t font_styles_ = 0;
WideString face_name_;
- std::unique_ptr<FXFT_StreamRec> ft_stream_; // Must outlive `face_`.
RetainPtr<CFX_Face> face_;
std::vector<WideString> family_names_;
std::array<uint32_t, 4> usb_ = {};
