Improve TrueType font fallback check for glyph ID 0.
Change the heuristics used to check for font fallback when drawing
TrueType fonts with glyph ID 0. In some cases, 0 means use glyph 0, and
in other cases, it means the glyph is invalid. Right now, the code tries
to use a fallback font, and if that fails, go back to the original font.
Instead of doing this back-and-forth, look at the heuristics and decide
one way or the other.
Bug: chromium:1271578
Change-Id: I71a0a6607162602d8e3fba07e80546ed3a1ce548
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/91831
Reviewed-by: Nigi <nigi@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/render/charposlist.cpp b/core/fpdfapi/render/charposlist.cpp
index 6c171d2..c07ddbe 100644
--- a/core/fpdfapi/render/charposlist.cpp
+++ b/core/fpdfapi/render/charposlist.cpp
@@ -12,6 +12,32 @@
#include "core/fxge/cfx_substfont.h"
#include "core/fxge/text_char_pos.h"
+namespace {
+
+bool ShouldUseExistingFont(const CPDF_Font* font,
+ uint32_t glyph_id,
+ bool has_to_unicode) {
+ // Check for invalid glyph ID.
+ if (glyph_id == static_cast<uint32_t>(-1))
+ return false;
+
+ if (!font->IsTrueTypeFont())
+ return true;
+
+ // For TrueType fonts, a glyph ID of 0 may be invalid.
+ //
+ // When a "ToUnicode" entry exists in the font dictionary, it indicates
+ // a "ToUnicode" mapping file is used to convert from CIDs (which
+ // begins at decimal 0) to Unicode code. (See ToUnicode Mapping File
+ // Tutorial - Adobe
+ // https://www.adobe.com/content/dam/acom/en/devnet/acrobat/pdfs/5411.ToUnicode.pdf
+ // and
+ // https://www.freetype.org/freetype2/docs/tutorial/step1.html#section-6)
+ return glyph_id != 0 || has_to_unicode;
+}
+
+} // namespace
+
std::vector<TextCharPos> GetCharPosList(pdfium::span<const uint32_t> char_codes,
pdfium::span<const float> char_pos,
CPDF_Font* font,
@@ -43,42 +69,19 @@
? text_char_pos.m_ExtGID
: text_char_pos.m_GlyphIndex;
#endif
- bool is_invalid_glyph = glyph_id == static_cast<uint32_t>(-1);
- bool is_true_type_zero_glyph = glyph_id == 0 && font->IsTrueTypeFont();
- bool use_fallback_font = false;
- if (is_invalid_glyph || is_true_type_zero_glyph) {
- text_char_pos.m_FallbackFontPosition =
- font->FallbackFontFromCharcode(char_code);
- text_char_pos.m_GlyphIndex = font->FallbackGlyphFromCharcode(
- text_char_pos.m_FallbackFontPosition, char_code);
- if (is_true_type_zero_glyph &&
- text_char_pos.m_GlyphIndex == static_cast<uint32_t>(-1)) {
- // For a TrueType font character, when finding the glyph from the
- // fallback font fails, switch back to using the original font.
-
- // When keyword "ToUnicode" exists in the PDF file, it indicates
- // a "ToUnicode" mapping file is used to convert from CIDs (which
- // begins at decimal 0) to Unicode code. (See ToUnicode Mapping File
- // Tutorial - Adobe
- // https://www.adobe.com/content/dam/acom/en/devnet/acrobat/pdfs/5411.ToUnicode.pdf
- // and
- // https://www.freetype.org/freetype2/docs/tutorial/step1.html#section-6)
- if (has_to_unicode)
- text_char_pos.m_GlyphIndex = 0;
- } else {
- use_fallback_font = true;
- }
- }
CFX_Font* current_font;
- if (use_fallback_font) {
- current_font =
- font->GetFontFallback(text_char_pos.m_FallbackFontPosition);
+ if (ShouldUseExistingFont(font, glyph_id, has_to_unicode)) {
+ current_font = font->GetFont();
+ text_char_pos.m_FallbackFontPosition = -1;
+ } else {
+ int32_t fallback_position = font->FallbackFontFromCharcode(char_code);
+ current_font = font->GetFontFallback(fallback_position);
+ text_char_pos.m_FallbackFontPosition = fallback_position;
+ text_char_pos.m_GlyphIndex =
+ font->FallbackGlyphFromCharcode(fallback_position, char_code);
#if BUILDFLAG(IS_APPLE)
text_char_pos.m_ExtGID = text_char_pos.m_GlyphIndex;
#endif
- } else {
- current_font = font->GetFont();
- text_char_pos.m_FallbackFontPosition = -1;
}
if (!font->IsEmbedded() && !font->IsCIDFont())
diff --git a/testing/SUPPRESSIONS b/testing/SUPPRESSIONS
index a0840b1..ee548b2 100644
--- a/testing/SUPPRESSIONS
+++ b/testing/SUPPRESSIONS
@@ -334,9 +334,6 @@
# TODO(pdfium:1747): Remove after associated bug is fixed
bug_1258634.in * * * skia
-# TODO(chromium:1271578): Remove after associated bug is fixed
-bug_1271578.in * * * *
-
# TODO(pdfium:1748): Remove after associated bug is fixed
bug_1286.in * * * skia
diff --git a/testing/resources/pixel/bug_1271578_expected_mac.pdf.0.png b/testing/resources/pixel/bug_1271578_expected_mac.pdf.0.png
new file mode 100644
index 0000000..7318ef2
--- /dev/null
+++ b/testing/resources/pixel/bug_1271578_expected_mac.pdf.0.png
Binary files differ
