Fix the rendering of scientific symbols correctly
The file in the bug contains text which has to be rendered
in "Symbol" font. This font is not embedded in the pdf file.
When rendering call comes for text that corresponds to Symbol font, the
font is fetched from the system. Even though symbol font is present on
the system, it is not picked up.
Currently, the font is fetched by following the two steps:
1. Find whether there is a matching font in the installed fonts on
the system. This call returns font face name if found in the list of
installed fonts.
2. Getting the font face for the required font.
This call again iterates over all the installed fonts
and returns the font face for the best matching font.
For the case in question:
Incorrect font whose name is a prefix of desired font is being picked up
due to GetSimilarValue() yielding the same result.
To fix this, check if the family name matches the font facename exactly,
then increase |iSimilarValue| by 4.
Bug: pdfium:1449
Change-Id: I23d5c3f5e4f26981fc6a9af2ccd0024c3f9b6102
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/64610
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Himadri Kakar <hikakar@microsoft.com>
diff --git a/core/fxge/BUILD.gn b/core/fxge/BUILD.gn
index 2b90c21..f8b7d4e 100644
--- a/core/fxge/BUILD.gn
+++ b/core/fxge/BUILD.gn
@@ -196,6 +196,7 @@
pdfium_unittest_source_set("unittests") {
sources = [
+ "cfx_folderfontinfo_unittest.cpp",
"cfx_fontmapper_unittest.cpp",
"dib/cfx_cmyk_to_srgb_unittest.cpp",
"dib/cfx_dibitmap_unittest.cpp",
diff --git a/core/fxge/cfx_folderfontinfo.cpp b/core/fxge/cfx_folderfontinfo.cpp
index c93b1f0..c0dbcc7 100644
--- a/core/fxge/cfx_folderfontinfo.cpp
+++ b/core/fxge/cfx_folderfontinfo.cpp
@@ -109,8 +109,13 @@
int32_t GetSimilarValue(int weight,
bool bItalic,
int pitch_family,
- uint32_t style) {
+ uint32_t style,
+ bool bMatchName,
+ size_t familyNameLength,
+ size_t bsNameLength) {
int32_t iSimilarValue = 0;
+ if (bMatchName && (familyNameLength == bsNameLength))
+ iSimilarValue += 4;
if (FontStyleIsForceBold(style) == (weight > 400))
iSimilarValue += 16;
if (FontStyleIsItalic(style) == bItalic)
@@ -311,8 +316,11 @@
if (bMatchName && !bsName.Contains(bsFamily))
continue;
+ size_t familyNameLength = strlen(family);
+ size_t bsNameLength = bsName.GetLength();
int32_t iSimilarValue =
- GetSimilarValue(weight, bItalic, pitch_family, pFont->m_Styles);
+ GetSimilarValue(weight, bItalic, pitch_family, pFont->m_Styles,
+ bMatchName, familyNameLength, bsNameLength);
if (iSimilarValue > iBestSimilar) {
iBestSimilar = iSimilarValue;
pFind = pFont;
diff --git a/core/fxge/cfx_folderfontinfo.h b/core/fxge/cfx_folderfontinfo.h
index 4fd516d..a8da990 100644
--- a/core/fxge/cfx_folderfontinfo.h
+++ b/core/fxge/cfx_folderfontinfo.h
@@ -38,6 +38,8 @@
bool GetFontCharset(void* hFont, int* charset) override;
protected:
+ friend class CFX_FolderFontInfoTest;
+
class FontFaceInfo {
public:
FontFaceInfo(ByteString filePath,
diff --git a/core/fxge/cfx_folderfontinfo_unittest.cpp b/core/fxge/cfx_folderfontinfo_unittest.cpp
new file mode 100644
index 0000000..e3626da
--- /dev/null
+++ b/core/fxge/cfx_folderfontinfo_unittest.cpp
@@ -0,0 +1,90 @@
+// Copyright 2019 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/fxge/cfx_folderfontinfo.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/base/ptr_util.h"
+#include "third_party/base/stl_util.h"
+
+namespace {
+constexpr char kArial[] = "Arial";
+constexpr char kTimesNewRoman[] = "TimesNewRoman";
+constexpr char kSymbol[] = "Symbol";
+constexpr char kBookshelfSymbol7[] = "Bookshelf Symbol 7";
+constexpr char kCalibri[] = "Calibri";
+constexpr char kBookshelf[] = "Bookshelf";
+} // namespace
+
+class CFX_FolderFontInfoTest : public ::testing::Test {
+ public:
+ CFX_FolderFontInfoTest() {
+ auto arial_info = pdfium::MakeUnique<CFX_FolderFontInfo::FontFaceInfo>(
+ /*filePath=*/"", kArial, /*fontTables=*/"",
+ /*fontOffset=*/0, /*fileSize=*/0);
+ arial_info->m_Charsets = 2;
+ auto times_new_roman_info =
+ pdfium::MakeUnique<CFX_FolderFontInfo::FontFaceInfo>(
+ /*filePath=*/"", kTimesNewRoman, /*fontTables=*/"",
+ /*fontOffset=*/0, /*fileSize=*/0);
+ auto bookshelf_symbol7_info =
+ pdfium::MakeUnique<CFX_FolderFontInfo::FontFaceInfo>(
+ /*filePath=*/"", kBookshelfSymbol7, /*fontTables=*/"",
+ /*fontOffset=*/0, /*fileSize=*/0);
+ bookshelf_symbol7_info->m_Charsets = 2;
+ auto symbol_info = pdfium::MakeUnique<CFX_FolderFontInfo::FontFaceInfo>(
+ /*filePath=*/"", kSymbol, /*fontTables=*/"",
+ /*fontOffset=*/0, /*fileSize=*/0);
+ symbol_info->m_Charsets = 2;
+
+ font_info_.m_FontList[kArial] = std::move(arial_info);
+ font_info_.m_FontList[kTimesNewRoman] = std::move(times_new_roman_info);
+ font_info_.m_FontList[kBookshelfSymbol7] =
+ std::move(bookshelf_symbol7_info);
+ font_info_.m_FontList[kSymbol] = std::move(symbol_info);
+ }
+
+ void* FindFont(int weight,
+ bool bItalic,
+ int charset,
+ int pitch_family,
+ const char* family,
+ bool bMatchName) {
+ return font_info_.FindFont(weight, bItalic, charset, pitch_family, family,
+ bMatchName);
+ }
+
+ ByteString GetFaceName(void* font) {
+ return static_cast<CFX_FolderFontInfo::FontFaceInfo*>(font)->m_FaceName;
+ }
+
+ private:
+ CFX_FolderFontInfo font_info_;
+};
+
+TEST_F(CFX_FolderFontInfoTest, TestFindFont) {
+ // Find "Symbol" font
+ void* font = FindFont(/*weight=*/0, /*bItalic=*/false, /*charset=*/2,
+ /*pitch_family=*/2, kSymbol, /*bMatchName=*/true);
+ ASSERT_TRUE(font);
+ EXPECT_EQ(GetFaceName(font), kSymbol);
+
+ // Find "Calibri" font that is not present in the installed fonts
+ EXPECT_FALSE(FindFont(/*weight=*/0, /*bItalic=*/false, /*charset=*/2,
+ /*pitch_family=*/2, kCalibri,
+ /*bMatchName=*/true));
+
+ // Find the closest matching font to "Bookself" font that is present in the
+ // installed fonts
+ font = FindFont(/*weight=*/0, /*bItalic=*/false, /*charset=*/2,
+ /*pitch_family=*/2, kBookshelf, /*bMatchName=*/true);
+ ASSERT_TRUE(font);
+ EXPECT_EQ(GetFaceName(font), kBookshelfSymbol7);
+
+ // Find "Symbol" font when name matching is false
+ font = FindFont(/*weight=*/0, /*bItalic=*/false, /*charset=*/2,
+ /*pitch_family=*/2, kSymbol, /*bMatchName=*/false);
+ ASSERT_TRUE(font);
+ EXPECT_EQ(GetFaceName(font), kArial);
+}
diff --git a/testing/SUPPRESSIONS b/testing/SUPPRESSIONS
index d8e0a2b..ae9cec0 100644
--- a/testing/SUPPRESSIONS
+++ b/testing/SUPPRESSIONS
@@ -340,6 +340,9 @@
# TODO(chromium:451366): Remove after associated bug is fixed
bug_451366.in * * *
+# TODO(pdfium:1466): Remove after associated bug is fixed
+bug_1449.in linux * *
+
# xfa_specific
# TODO(pdfium:1107): Remove after associated bug is fixed
diff --git a/testing/resources/pixel/bug_1449.in b/testing/resources/pixel/bug_1449.in
new file mode 100644
index 0000000..603eb56
--- /dev/null
+++ b/testing/resources/pixel/bug_1449.in
@@ -0,0 +1,47 @@
+{{header}}
+{{object 1 0}} <<
+ /Type /Catalog
+ /Pages 2 0 R
+>>
+endobj
+{{object 2 0}} <<
+ /Type /Pages
+ /MediaBox [ 0 0 100 100 ]
+ /Count 1
+ /Kids [ 3 0 R ]
+>>
+endobj
+{{object 3 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /Resources <<
+ /Font <<
+ /F0 4 0 R
+ >>
+ >>
+ /Contents 5 0 R
+>>
+endobj
+{{object 4 0}} <<
+ /Type /Font
+ /Subtype /TrueType
+ /BaseFont /Symbol
+ /FirstChar 31
+ /LastChar 255
+ /Name /F0
+>>
+endobj
+{{object 5 0}} <<
+ {{streamlen}}
+>>
+stream
+BT
+25 25 Td /F0 12 Tf
+-0.036 Tc (\265) Tj
+ET
+endstream
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/pixel/bug_1449_expected.pdf.0.png b/testing/resources/pixel/bug_1449_expected.pdf.0.png
new file mode 100644
index 0000000..a9fcdbf
--- /dev/null
+++ b/testing/resources/pixel/bug_1449_expected.pdf.0.png
Binary files differ
diff --git a/testing/resources/pixel/bug_1449_expected_mac.pdf.0.png b/testing/resources/pixel/bug_1449_expected_mac.pdf.0.png
new file mode 100644
index 0000000..0a4e266
--- /dev/null
+++ b/testing/resources/pixel/bug_1449_expected_mac.pdf.0.png
Binary files differ
