Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Fallback '.notdef' Glyph for Improved Cross-Platform TrueType Font Support #1352

Merged
merged 14 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ This can also be enabled programmatically with `warnings.simplefilter('default',
* support for [`v_align` at the row level in tables](https://py-pdf.github.io/fpdf2/Tables.html#setting-vertical-alignment-of-text-in-cells)
* documentation on [verifying provenance of `fpdf2` releases](https://py-pdf.github.io/fpdf2/#verifying-provenance)
* documentation on [`fpdf2` internals](https://py-pdf.github.io/fpdf2/Internals.html)
* support for adding TrueType fonts that are missing the .notdef glyph - [issue #1161](https://github.com/py-pdf/fpdf2/issues/1161)
### Fixed
* [`FPDF.write_html()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.write_html): Fixed rendering of content following `<a>` tags; now correctly resets emphasis style post `</a>` tag: hyperlink styling contained within the tag authority. - [Issue #1311](https://github.com/py-pdf/fpdf2/issues/1311)

Expand Down
38 changes: 38 additions & 0 deletions fpdf/fonts.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"""

import re, warnings
import logging

from bisect import bisect_left
from collections import defaultdict
Expand All @@ -16,6 +17,7 @@
from typing import List, Optional, Tuple, Union

from fontTools import ttLib
from fontTools.pens.ttGlyphPen import TTGlyphPen

try:
import uharfbuzz as hb
Expand All @@ -37,6 +39,8 @@ def __deepcopy__(self, _memo):
from .syntax import Name, PDFObject
from .util import escape_parens

LOGGER = logging.getLogger(__name__)


@dataclass
class FontFace:
Expand Down Expand Up @@ -267,6 +271,40 @@ def __init__(self, fpdf, font_file_path, fontkey, style):
)

self.scale = 1000 / self.ttfont["head"].unitsPerEm

# check if the font is a TrueType and missing a .notdef glyph
# if it is missing, provide a fallback glyph
if "glyf" in self.ttfont and ".notdef" not in self.ttfont["glyf"]:
LOGGER.warning(
(
"TrueType Font '%s' is missing the '.notdef' glyph. "
"Fallback glyph will be provided."
),
self.fontkey,
)
# draw a diagonal cross .notdef glyph
(xMin, xMax, yMin, yMax) = (
self.ttfont["head"].xMin,
self.ttfont["head"].xMax,
self.ttfont["head"].yMin,
self.ttfont["head"].yMax,
)
pen = TTGlyphPen(self.ttfont["glyf"])
pen.moveTo((xMin, yMin))
pen.lineTo((xMax, yMin))
pen.lineTo((xMax, yMax))
pen.lineTo((xMin, yMax))
pen.closePath()
pen.moveTo((xMin, yMin))
pen.lineTo((xMax, yMax))
pen.closePath()
pen.moveTo((xMax, yMin))
pen.lineTo((xMin, yMax))
pen.closePath()

self.ttfont["glyf"][".notdef"] = pen.glyph()
self.ttfont["hmtx"][".notdef"] = (xMax - xMin, yMax - yMin)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe having the fixed 0,0 to 600,600 would look weird on different fonts.
Maybe having your coordinates set to (ttfont['head'].xMin, ttfont['head'].yMax), (ttfont['head'].xMax, ttfont['head'].yMax) would generate a more consistent notdef outline.

default_width = round(self.scale * self.ttfont["hmtx"].metrics[".notdef"][0])

os2_table = self.ttfont["OS/2"]
Expand Down
Binary file added test/fonts/Roboto-Regular-without-notdef.ttf
Binary file not shown.
Binary file not shown.
10 changes: 10 additions & 0 deletions test/fonts/test_add_font.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,16 @@ def test_add_font_uppercase():
assert pdf.fonts is not None and len(pdf.fonts) != 0 # fonts add successful


def test_add_font_missing_notdef_glyph(caplog):
pdf = FPDF()
pdf.add_font(family="Roboto", fname=HERE / "Roboto-Regular-without-notdef.ttf")
assert pdf.fonts is not None and len(pdf.fonts) != 0 # fonts add successful
assert (
"TrueType Font 'roboto' is missing the '.notdef' glyph. "
"Fallback glyph will be provided."
) in caplog.text


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Times New Roman is a commercial font owned by Monotype, so we can't include it in our repository.

I recommend getting a OFL font and renaming the .notdef glyph to something else to create your test font.

something like this:

from fontTools import ttLib

font = ttLib.TTFont("NotoSans-Regular.ttf")
glyphnames = font.getGlyphOrder()
glyphnames[0] = "dummy"
font = ttLib.TTFont("NotoSans-Regular.ttf")
font.setGlyphOrder(glyphnames)
post = font['post']
font.save("NotoSans-Regular-without-notdef.ttf")

Copy link
Author

@spacegaori spacegaori Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

post = font['post']

What does this line do?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

post = font['post']

What does this line do?

If you don't read the 'post' table, fontTools will never apply the change on this table and the change won't be saved on the new font. It's because of the lazy loading they do to improve performance.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to include the "NotoSans-Regular-without-notdef.ttf" - you can create the font and commit the ttf into the repository.

Also, the CI is failing because test/font/test_charmap.py will create a test file with the first 999 chars of every font on test/font, so you need to run it with generate=True once to create a reference file for the new font.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I'll run it once.

You don't need to include the "NotoSans-Regular-without-notdef.ttf" - you can create the font and commit the ttf into the repository.

Do you mean that I don't need to include the process of creating "NotoSans-Regular-without-notdef.ttf" and just include the created font?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't read the 'post' table, fontTools will never apply the change on this table and the change won't be saved on the new font. It's because of the lazy loading they do to improve performance.

Maybe it would be worth adding a comment explaining that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

post = font['post']

FYI, this line causes a linter error.

test/fonts/test_add_font.py:158:4: W0612: Unused variable 'post' (unused-variable)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you included the TTF font already, I'd say just remove generating the font again (remove lines 151 to 161).

It should be good to merge after that.

def test_font_missing_glyphs(caplog):
pdf = FPDF()
pdf.add_page()
Expand Down