Skip to content

Add a last resort font for missing glyphs #29356

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 25, 2025
Merged

Add a last resort font for missing glyphs #29356

merged 1 commit into from
Apr 25, 2025

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Dec 20, 2024

PR summary

Some time ago, I came across https://github.com/unicode-org/last-resort-font. Through some charmap magic, the "High Efficiency" font from this project is able to provide a glyph for every Unicode codepoint while only being about 550K. (The expanded version in that repo in 8.6MB.)

This is a proof-of-concept for applying this font as final fallback at (almost) all times. I have added an rcParam to turn it off, mostly for one test to work, but we may not want to use that. I'm also not sure if it's been inserted in the best place. We may instead want to insert the glyph at the point where we have none, so that we can warn about it too.

These glyphs show a representative character from the Unicode block, a square frame, and if large enough, the block name and code point range:

import matplotlib.pyplot as plt

text = '\N{Bengali Digit Zero}\N{Hiragana Letter A}\ufdd0'
sizes = [
    (0.90, 6),
    (0.85, 8),
    (0.80, 10),
    (0.75, 12),
    (0.70, 16),
    (0.63, 20),
    (0.55, 24),
    (0.45, 32),
    (0.30, 48),
    (0.10, 64),
]

fig = plt.figure()
for y, size in sizes:
    fig.text(0.01, y, f'{size}pt: {text}', fontsize=size)
plt.show()

Figure_1

PR checklist

@story645
Copy link
Member

would this also address #27232 ?

@QuLogic
Copy link
Member Author

QuLogic commented Dec 20, 2024

I haven't tested it, but I think so.

@QuLogic
Copy link
Member Author

QuLogic commented Feb 22, 2025

IIRC, our conversation in a previous meeting concluded with moving this fallback to the missing-glyph warning trigger. While this is possible with some refactoring, I've realized we reach a problem with font size. This would not have been set for the last-resort font, and it's a bit difficult to copy that information from another font due to the way FreeType stores it (it's stored in some font units, not the original call units.) If we have a global object for it, then we'd also have to worry about threading issues if propagating the size. So overall, this may not be the best course of action.

However, that does mean we'd need to find an alternative means of ensuring the missing-glyph warning appears.

@QuLogic QuLogic added this to the v3.11.0 milestone Feb 25, 2025
@QuLogic QuLogic marked this pull request as ready for review February 25, 2025 00:28
@QuLogic
Copy link
Member Author

QuLogic commented Feb 25, 2025 •
edited
Loading

So in the end, I just added a flag to the Last Resort FT2Font to indicate whether to warn or not. That's simpler than trying to reach back into Python somehow to ask if it should.

I don't recall if we concluded that the rcParam was unnecessary, but I've left it in for now.

@QuLogic
Copy link
Member Author

QuLogic commented Feb 25, 2025

I think I'm missing something super-obvious, as I don't know why the What's new plot isn't displaying anything.

@QuLogic
Copy link
Member Author

QuLogic commented Mar 25, 2025

OK, I've figured out why the plot is missing. The warning is preserved here, and since the doc builds set -Werror, this causes an exception in savefig.

This is supposed to be caught:

matplotlib/lib/matplotlib/sphinxext/plot_directive.py

Lines 700 to 713 in a9dc9ac

for fmt, dpi in formats:
try:
figman.canvas.figure.savefig(img.filename(fmt), dpi=dpi)
if fmt == formats[0][0] and config.plot_srcset:
# save a 2x, 3x etc version of the default...
srcset = _parse_srcset(config.plot_srcset)
for mult, suffix in srcset.items():
fm = f'{suffix}.{fmt}'
img.formats.append(fm)
figman.canvas.figure.savefig(img.filename(fm),
dpi=int(dpi * mult))
except Exception as err:
raise PlotError(traceback.format_exc()) from err
img.formats.append(fmt)

and reported:

matplotlib/lib/matplotlib/sphinxext/plot_directive.py

Lines 868 to 875 in a9dc9ac

except PlotError as err:
reporter = state.memo.reporter
sm = reporter.system_message(
2, "Exception occurred in plotting {}\n from {}:\n{}".format(
output_base, source_file_name, err),
line=lineno)
results = [(code, [])]
errors = [sm]

but I've never seen this output.

Because the exception occurs in savefig (which is outside of the plot directive), there is nothing we can do from inside the plot code itself. So I'm going to just un-error these warnings globally.

@github-actions github-actions bot added the Documentation: build building the docs label Mar 25, 2025
@QuLogic QuLogic force-pushed the last-resort-font branch 3 times, most recently from 6499c51 to b8b8e5a Compare March 28, 2025 06:18
@QuLogic
Copy link
Member Author

QuLogic commented Apr 3, 2025

While testing out fallbacks with libraqm, I noticed that these don't work in PDF for some reason. Still need to investigate that a bit closer.

@QuLogic QuLogic marked this pull request as draft April 3, 2025 01:32
@QuLogic QuLogic force-pushed the last-resort-font branch from b8b8e5a to 6f8cf65 Compare April 3, 2025 05:24
@QuLogic
Copy link
Member Author

QuLogic commented Apr 3, 2025

So instead of using the existing font objects, the PDF backend tracks fonts by filename, and then loads them again individually when writing the final font subsets. What this meant is that a) the Last Resort font was loaded without selecting the right charmap and b) the Last Resort fallback was added to Last Resort if it was explicitly asked for (and only this fallback had the right charmap).

I've modified _get_font so that is more resilient to these cases and always sets the charmap for Last Resort, as well as not duplicating a fallback for it. Also added a small test image for the results just in case we change PDF/SVG.

@QuLogic QuLogic marked this pull request as ready for review April 3, 2025 06:19
@QuLogic
Copy link
Member Author

QuLogic commented Apr 3, 2025

Also, I had to bump the GitHub cache key so that test_get_font_names passed; this is because we have a new font in mpl-data/fonts. Does that warrant bumping FontManager.__version__? Or not, because it's only a developer problem?

@QuLogic QuLogic force-pushed the last-resort-font branch 2 times, most recently from 4fe3a07 to c872584 Compare April 17, 2025 19:40
tacaswell
tacaswell reviewed Apr 18, 2025
View reviewed changes
src/ft2font.cpp
: ft_glyph_warn(warn), image(), face(nullptr)
FT2Font::WarnFunc warn, bool warn_if_used)
: ft_glyph_warn(warn), warn_if_used(warn_if_used), image(), face(nullptr),
hinting_factor(hinting_factor_),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
hinting_factor(hinting_factor_),
// set a default fontsize 12 pt at 72dpi
hinting_factor(hinting_factor_),

and presumably the comment should be removed below?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, that had nothing to do with the hinting factor, which is why I left it there.

tacaswell
tacaswell reviewed Apr 18, 2025
View reviewed changes
src/ft2font.cpp
@@ -268,12 +271,7 @@ FT2Font::FT2Font(FT_Open_Args &open_args,
throw_ft_error("Can not load face", error);
}

// set default kerning factor to 0, i.e., no kerning manipulation
kerning_factor = 0;

// set a default fontsize 12 pt at 72dpi
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// set a default fontsize 12 pt at 72dpi

tacaswell
tacaswell approved these changes Apr 18, 2025
View reviewed changes
Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

Left one minor comment about moving a comment.

story645
story645 requested changes Apr 23, 2025
View reviewed changes
@story645 story645 merged commit 4407d1d into matplotlib:main Apr 25, 2025
39 of 41 checks passed
@QuLogic QuLogic deleted the last-resort-font branch April 25, 2025 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tacaswell tacaswell tacaswell approved these changes

@story645 story645 story645 approved these changes

Assignees
No one assigned
Labels
Documentation: build building the docs topic: rcparams topic: text/fonts
Projects
None yet
Milestone
v3.11.0
Development

Successfully merging this pull request may close these issues.
3 participants
@QuLogic @story645 @tacaswell