GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Replace ttconv by plain python for pdf subsetting by anntzer · Pull Request #18181 · matplotlib/matplotlib Conversation
|
Cool, I wanted to do that, except for it breaking images, like you've found. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I didn't completely finish the second commit yet, though it seems good.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| 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.
The reason will be displayed to describe this comment to others. Learn more.
Stylistically, this might be against PEP7.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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...
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I went for a slight variation which does respect pep7.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
586edd9 to
dd0482b
Compare
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Nice! Looks like much of the complexity is for bug-compatibility with ttconv, which I hope we can remove later.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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 an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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); |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
One thing I forgot to mention is that the Py_DTSF_ADD_DOT_0 change also fixes a separate bug in |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I'll merge since @anntzer is the main person concerned with images changing 😉 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Missing character upon savefig() with Free Serif font
There was an error while loading. Please reload this page.