[SkiaPaths] Embolden SkFont if substitute font is bold or extra-bold.
This CL fixes the issue that SkiaPaths sometimes renders bold and
extra-bold fonts with bold style missing, and adds a pixel test which
contains bold and extra-bold styles for both PDFium-supported fonts
and fonts which need substitution.
1. Achieve regular bold font for substituted font just like AGG.
For a font which needs font substitution during rendering, if it has
a regular bold style indicated as "FONTNAME-Bold" or "FONTNAME,Bold",
the bold style is not passed when generating font face, which leads
to the SkTypeface missing bold font style for rendering.
To solve this issue, check whether substitute font weight is greater
than or equal to bold font weight (700). If yes, embolden SkFont to
achieve the bold style.
Currently, neither AGG or Acrobat supports "extra-bold" style (with
font name as "FONTNAME-Bold,Bold") when using substituted font. The
rendering results will just be regular bold for AGG, and not bold for
Acrobat. This CL will enable SkiaPaths to render in regular bold
style like AGG when encountering the same "extra-bold" substituted
fonts.
2. Enabling extra-boldness for fonts which are supported in PDFium.
For fonts which are supported in PDFium (See |g_Base14FontNames| and
|g_AltFontNames| in cfx_fontmapper.cpp), such as "Arial", regular
bold can currently be rendered correctly for SkiaPaths since the
exact font face with bold style built-in can be found and passed into
SkTypeface, and no substitute font info is loaded or needed.
However, there is no boldness difference between the rendering
results of font with extra-bold style, such as "Arial-Bold,Bold",
and regular-bold style, such as "Arial-Bold". Since for extra-bold
style, substituted font will be loaded with the heavier font weight,
this weight information can be used to indicate and apply the extra
boldness needed for SkFont during SkiaPaths rendering.
Bug: pdfium:846
Change-Id: I38451362ac818ca2b5b34d872823234443137615
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/67811
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Hui Yingst <nigi@chromium.org>
diff --git a/core/fxge/cfx_font.cpp b/core/fxge/cfx_font.cpp
index 67a80b6..2cef5ae 100644
--- a/core/fxge/cfx_font.cpp
+++ b/core/fxge/cfx_font.cpp
@@ -506,6 +506,11 @@
return m_Face && FXFT_Is_Face_fixedwidth(m_Face->GetRec()) != 0;
}
+bool CFX_Font::IsSubstFontBold() const {
+ CFX_SubstFont* subst_font = GetSubstFont();
+ return subst_font && subst_font->GetOriginalWeight() >= FXFONT_FW_BOLD;
+}
+
ByteString CFX_Font::GetPsName() const {
if (!m_Face)
return ByteString();
diff --git a/core/fxge/cfx_font.h b/core/fxge/cfx_font.h
index 9658ac2..d6fbc13 100644
--- a/core/fxge/cfx_font.h
+++ b/core/fxge/cfx_font.h
@@ -88,6 +88,7 @@
bool IsItalic() const;
bool IsBold() const;
bool IsFixedWidth() const;
+ bool IsSubstFontBold() const;
bool IsVertical() const { return m_bVertical; }
ByteString GetPsName() const;
ByteString GetFamilyName() const;
diff --git a/core/fxge/cfx_fontmapper.cpp b/core/fxge/cfx_fontmapper.cpp
index e40e083..da133d9 100644
--- a/core/fxge/cfx_fontmapper.cpp
+++ b/core/fxge/cfx_fontmapper.cpp
@@ -385,8 +385,7 @@
if (weight)
pSubstFont->m_Weight = weight;
if (FontFamilyIsRoman(pitch_family)) {
- pSubstFont->m_Weight = pSubstFont->m_Weight * 4 / 5;
- pSubstFont->m_Family = "Chrome Serif";
+ pSubstFont->UseChromeSerif();
if (!m_MMFaces[1]) {
m_MMFaces[1] = m_pFontMgr->NewFixedFace(
nullptr, m_pFontMgr->GetBuiltinFont(14).value(), 0);
diff --git a/core/fxge/cfx_substfont.cpp b/core/fxge/cfx_substfont.cpp
index b9936df..a2a3d41 100644
--- a/core/fxge/cfx_substfont.cpp
+++ b/core/fxge/cfx_substfont.cpp
@@ -9,3 +9,18 @@
CFX_SubstFont::CFX_SubstFont() = default;
CFX_SubstFont::~CFX_SubstFont() = default;
+
+int CFX_SubstFont::GetOriginalWeight() const {
+ int weight = m_Weight;
+
+ // Perform the inverse weight adjustment of UseChromeSerif() to get the
+ // original font weight.
+ if (m_Family == "Chrome Serif")
+ weight = weight * 5 / 4;
+ return weight;
+}
+
+void CFX_SubstFont::UseChromeSerif() {
+ m_Weight = m_Weight * 4 / 5;
+ m_Family = "Chrome Serif";
+}
diff --git a/core/fxge/cfx_substfont.h b/core/fxge/cfx_substfont.h
index c9ffa40..46ecc0a 100644
--- a/core/fxge/cfx_substfont.h
+++ b/core/fxge/cfx_substfont.h
@@ -15,6 +15,9 @@
CFX_SubstFont();
~CFX_SubstFont();
+ int GetOriginalWeight() const;
+ void UseChromeSerif();
+
ByteString m_Family;
int m_Charset = FX_CHARSET_ANSI;
int m_Weight = 0;
diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp
index faf40d6..9e2e423 100644
--- a/core/fxge/skia/fx_skia_device.cpp
+++ b/core/fxge/skia/fx_skia_device.cpp
@@ -885,6 +885,7 @@
}
if (Accumulator::kText != m_type) {
m_italicAngle = pFont->GetSubstFontItalicAngle();
+ m_isSubstFontBold = pFont->IsSubstFontBold();
m_charDetails.SetCount(0);
m_rsxform.setCount(0);
if (pFont->GetFaceRec())
@@ -963,6 +964,7 @@
if (m_pTypeFace) { // exclude placeholder test fonts
font.setTypeface(m_pTypeFace);
}
+ font.setEmbolden(m_isSubstFontBold);
font.setHinting(SkFontHinting::kNone);
font.setScaleX(m_scaleX);
font.setSkewX(tanf(m_italicAngle * FX_PI / 180.0));
@@ -1018,6 +1020,7 @@
m_drawMatrix = CFX_Matrix();
m_pFont = nullptr;
m_italicAngle = 0;
+ m_isSubstFontBold = false;
}
bool IsEmpty() const { return !m_commands.count(); }
@@ -1181,7 +1184,8 @@
return typeface != m_pTypeFace.get() || MatrixChanged(&matrix) ||
font_size != m_fontSize || scaleX != m_scaleX ||
color != m_fillColor ||
- pFont->GetSubstFontItalicAngle() != m_italicAngle;
+ pFont->GetSubstFontItalicAngle() != m_italicAngle ||
+ pFont->IsSubstFontBold() != m_isSubstFontBold;
}
bool MatrixChanged(const CFX_Matrix* pMatrix) const {
@@ -1520,6 +1524,7 @@
bool m_fillFullCover = false;
bool m_fillPath = false;
bool m_groupKnockout = false;
+ bool m_isSubstFontBold = false;
bool m_debugDisable = false; // turn off cache for debugging
#if SHOW_SKIA_PATH
public:
@@ -1684,6 +1689,7 @@
SkFont font;
font.setTypeface(typeface);
+ font.setEmbolden(pFont->IsSubstFontBold());
font.setHinting(SkFontHinting::kNone);
font.setSize(SkTAbs(font_size));
font.setSubpixel(true);
diff --git a/testing/resources/pixel/bug_846.in b/testing/resources/pixel/bug_846.in
new file mode 100644
index 0000000..e405d67
--- /dev/null
+++ b/testing/resources/pixel/bug_846.in
@@ -0,0 +1,160 @@
+{{header}}
+{{object 1 0}} <<
+ /Type /Catalog
+ /Pages 2 0 R
+>>
+endobj
+{{object 2 0}} <<
+ /Type /Pages
+ /Count 1
+ /Kids [3 0 R]
+>>
+endobj
+{{object 3 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+ /Resources 4 0 R
+ /Contents 15 0 R
+ /MediaBox [0 0 200 200]
+>>
+endobj
+{{object 4 0}} <<
+ /ProcSet [/PDF /Text]
+ /Font <<
+ /F1 5 0 R
+ /F2 7 0 R
+ /F3 9 0 R
+ /F4 11 0 R
+ >>
+>>
+endobj
+{{object 5 0}} <<
+ /Type /Font
+ /Subtype /TrueType
+ /BaseFont /Arial-Bold
+ /Encoding /WinAnsiEncoding
+ /FirstChar 65
+ /FontDescriptor 6 0 R
+ /LastChar 65
+ /Widths [778]
+>>
+endobj
+{{object 6 0}} <<
+ /Type /FontDescriptor
+ /Ascent 923
+ /CapHeight 687
+ /Descent -282
+ /Flags 34
+ /FontName /Arial-Bold
+ /FontBBox 14 0 R
+ /ItalicAngle 0
+ /StemV 140.269
+ /XHeight 0
+>>
+endobj
+{{object 7 0}} <<
+ /Type /Font
+ /Subtype /TrueType
+ /BaseFont /Arial-Bold,Bold
+ /Encoding /WinAnsiEncoding
+ /FirstChar 65
+ /FontDescriptor 8 0 R
+ /LastChar 65
+ /Widths [778]
+>>
+endobj
+{{object 8 0}} <<
+ /Type /FontDescriptor
+ /Ascent 923
+ /CapHeight 687
+ /Descent -282
+ /Flags 34
+ /FontName /Arial-Bold,Bold
+ /FontBBox 14 0 R
+ /ItalicAngle 0
+ /StemV 140.269
+ /XHeight 0
+>>
+endobj
+{{object 9 0}} <<
+ /Type /Font
+ /Subtype /TrueType
+ /BaseFont /DejaVuSans-Bold
+ /Encoding /WinAnsiEncoding
+ /FirstChar 58
+ /FontDescriptor 10 0 R
+ /LastChar 65
+ /Widths 13 0 R
+>>
+endobj
+{{object 10 0}} <<
+ /Type /FontDescriptor
+ /Ascent 923
+ /CapHeight 687
+ /Descent -282
+ /Flags 34
+ /FontName /DejaVuSans-Bold
+ /FontBBox 14 0 R
+ /ItalicAngle 0
+ /StemV 140.269
+ /XHeight 0
+>>
+endobj
+{{object 11 0}} <<
+ /Type /Font
+ /Subtype /TrueType
+ /BaseFont /DejaVuSans-Bold,Bold
+ /Encoding /WinAnsiEncoding
+ /FirstChar 58
+ /FontDescriptor 12 0 R
+ /LastChar 65
+ /Widths 13 0 R
+>>
+endobj
+{{object 12 0}} <<
+ /Type /FontDescriptor
+ /Ascent 923
+ /CapHeight 687
+ /Descent -282
+ /Flags 34
+ /FontName /DejaVuSans-Bold,Bold
+ /FontBBox 14 0 R
+ /ItalicAngle 0
+ /StemV 140.269
+ /XHeight 0
+>>
+endobj
+{{object 13 0}} [250 0 0 0 0 0 0 778]
+endobj
+{{object 14 0}} [-134 -337 1193 1021]
+endobj
+{{object 15 0}} <<
+ {{streamlen}}
+>>
+stream
+BT
+/F1 2 Tf
+18 0 0 18 50 120 Tm
+(A)Tj
+ET
+BT
+/F2 2 Tf
+18 0 0 18 120 120 Tm
+(A)Tj
+ET
+BT
+/F3 2 Tf
+18 0 0 18 50 60 Tm
+(A)Tj
+ET
+BT
+/F4 2 Tf
+18 0 0 18 120 60 Tm
+(A)Tj
+ET
+endstream
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/pixel/bug_846_expected.pdf.0.png b/testing/resources/pixel/bug_846_expected.pdf.0.png
new file mode 100644
index 0000000..2a57015
--- /dev/null
+++ b/testing/resources/pixel/bug_846_expected.pdf.0.png
Binary files differ
diff --git a/testing/resources/pixel/bug_846_expected_mac.pdf.0.png b/testing/resources/pixel/bug_846_expected_mac.pdf.0.png
new file mode 100644
index 0000000..3e0deb6
--- /dev/null
+++ b/testing/resources/pixel/bug_846_expected_mac.pdf.0.png
Binary files differ
diff --git a/testing/resources/pixel/bug_846_expected_skia.pdf.0.png b/testing/resources/pixel/bug_846_expected_skia.pdf.0.png
new file mode 100644
index 0000000..a13f154
--- /dev/null
+++ b/testing/resources/pixel/bug_846_expected_skia.pdf.0.png
Binary files differ
diff --git a/testing/resources/pixel/bug_846_expected_skia_mac.pdf.0.png b/testing/resources/pixel/bug_846_expected_skia_mac.pdf.0.png
new file mode 100644
index 0000000..4e71c4f
--- /dev/null
+++ b/testing/resources/pixel/bug_846_expected_skia_mac.pdf.0.png
Binary files differ
