Simplify and unify character tracking in pdf and ps backends (with linked fonts) #15686
Conversation
Instead of trying to resolve font paths to absolute files and key off by inode(!), just track fonts using whatever names they use, and simplify used_characters to be a straight mapping of filenames to character ids (making the attribute private -- with a backcompat shim) at the same time). The previous approach would avoid embedding the same file twice if it is given under two different filenames (hardlinks to the same file...), but it would fail if the user passes a relative path, chdir()s to another directory, and passes another different font with the same filename, because of the lru_cache(). None of these seem likely to happen in practice, and in any case we can cover most of it by making the font paths absolute before passing them to FreeType (which is going to open the file anyways, so the cost of making them absolute doesn't matter).
|
Why remove the resolve() in font_manager? If two fonts are resolving to the same path, it's still good to be able to collapse them, no? |
|
In the In I thought the idea of #15320 was to ignore that fact that links might resolve to the same file, and embed the font twice in this case. If we want to collapse these fonts, we need to do this consistently when tracking the used characters and fonts. |
|
Good point, but the idea was also to still get some of the dedupe by resolving early. I guess you can put a resolve() call (or even do a full dedupe based on st_inode (and st_dev if you want to take care of the multiple-mounted-filesystems case...)) on the output of findSystemFonts to get most of this benefits? |
b8e93dc to
5e030c8
Compare
|
I've move the With this solution we get the following benefits:
The optimization is not sensitive to hard links, but I think this was already sacrificed in #15320. My test setup with linked fonts produces valid PDFs when using this implementation: example.pdf, CI run. |
fe2b338 to
adb8412
Compare
The commit ensures that the realpath of linked fonts is used across the library. This prevents embedding linked fonts twice and makes sure that linked fonts are referenced consistently in PDF files. The link resolution cannot happend in addfont() since this would break the *directory* argument of findfont().
adb8412 to
6a126a4
Compare
|
Thanks and congratulations on your first contribution to Matplotlib! Hope to see you back some time. |
PR Summary
This PR builds on the implementation of #15320 but also considers the issues from #15629.
This PR resolves #15628.
The code produces a valid PDF with unicode and regular characters if linked fonts are used:
example.pdf CI run
PR Checklist