Replace ttconv by plain python for pdf subsetting #18181
Conversation
1da1a4c to
4a488fa
Compare
Cool, I wanted to do that, except for it breaking images, like you've found. |
src/_path.h
Outdated
| while (q >= str && *q == '0') { --q; } | ||
| // If the end is a decimal point, delete that too | ||
| if (q >= str && *q == '.') { --q; } |
There was a problem hiding this comment.
Stylistically, this might be against PEP7.
There was a problem hiding this comment.
I know, but I think splitting things on their own lines (as was done before) makes the code really harder to follow because the information density is so low...
There was a problem hiding this comment.
I went for a slight variation which does respect pep7.
586edd9 to
dd0482b
Compare
Instead of walking through FT_Outline ourselves, use FreeType's FT_Outline_Decompose to decompose glyphs into paths. Fixes incorrect positioning of first and last control points of certain contours; compare e.g. ```python from pylab import * from matplotlib.textpath import TextPath from matplotlib.patches import PathPatch rcParams["mathtext.fontset"] = "stixsans" path = TextPath((0, 0), r"$\delta$", size=72) plot(*path.vertices.T, ".-", lw=.5) gca().add_patch(PathPatch(path, alpha=.5)) gca().set(aspect="equal") show() ``` before and after the patch (before the patch, the inner loop was not properly closed). (Technically, I believe the earlier bug affected start/end-of-contours that are implicitly created per the ttf format at the midpoint between two conic control points.) A few SVG files need to be updated too.
d81d35b to
385894b
Compare
| --c; | ||
| } | ||
| try { | ||
| buffer.append(str, c + 1); |
There was a problem hiding this comment.
Second parameter is size, isn't it? Or is cppreference wrong, since it seems to compile...
| buffer.append(str, c + 1); | |
| buffer.append(str, c - str + 1); |
There was a problem hiding this comment.
This is using the
template< class InputIt >
basic_string& append( InputIt first, InputIt last );
overload, not the
basic_string& append( const CharT* s, size_type count );
one.
|
One thing I forgot to mention is that the Py_DTSF_ADD_DOT_0 change also fixes a separate bug in |
Quite a bit of work goes into reproducing quirks of the old ttconv code in order not to update baseline images, but this can easily be dropped once the new baseline images system goes in. figure_align_label.pdf gets modified because of tiny differences in outline extraction (due to the ttconv emulation); there's already the svg test that tests the fixed-dpi case so I feel OK deleting it. This also fixes ttc subsetting for pdf output (cf. changes in test_font_manager.py).
|
Should we wait for the baseline image PR before merging this? |
|
I put in all the bug-compat code explicitly to minimize the image diffs; I'd say the remaining svg diffs are not too large, but I'm not in a big hurry either. |
|
I'll merge since @anntzer is the main person concerned with images changing 😉 |
PR Summary
First commit:
Switch from a hand-written glyph outline decomposer to FreeType's one.
Instead of walking through FT_Outline ourselves, use FreeType's
FT_Outline_Decompose to decompose glyphs into paths.
Fixes incorrect positioning of first and last control points of certain
contours; compare e.g.
before and after the patch (before the patch, the inner loop was not
properly closed). (Technically, I believe the earlier bug affected
start/end-of-contours that are implicitly created per the ttf format at
the midpoint between two conic control points.)
before:
after:
A few SVG files need to be updated too.
Second commit:
Stop using ttconv for pdf.
Quite a bit of work goes into reproducing quirks of the old ttconv code
in order not to update baseline images, but this can easily be dropped
once the new baseline images system goes in.
figure_align_label.pdf gets modified because of tiny differences in
outline extraction (due to the ttconv emulation); there's already the
svg test that tests the fixed-dpi case so I feel OK deleting it.
This also fixes ttc subsetting for pdf output (cf. changes in
test_font_manager.py).
(PostScript is not done yet, but it's basically the same idea, it's just the header that's different.)
Fixes #17197. I'll skip adding a new font for the test, because in essence we're "just" relying on FreeType handing us the right outlines.
PR Checklist