GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Replace ttconv by plain python for pdf subsetting by anntzer · Pull Request #18181 · matplotlib/matplotlib

Conversation

@anntzer

@anntzer anntzer commented Aug 5, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

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.)

before:
old

after:
new

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

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/next_api_changes/* if API changed in a backward-incompatible way

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@anntzer anntzer mentioned this pull request Aug 5, 2020
Support scale in ttf composite glyphs #18081
Closed
2 of 6 tasks
@anntzer
anntzer force-pushed the unttpdf branch 5 times, most recently from 1da1a4c to 4a488fa Compare August 5, 2020 12:40
@anntzer anntzer added backend: pdf topic: text/fonts labels Aug 5, 2020
@QuLogic

QuLogic commented Aug 7, 2020

Copy link
Copy Markdown
Member

Instead of walking through FT_Outline ourselves, use FreeType's
FT_Outline_Decompose to decompose glyphs into paths.

Cool, I wanted to do that, except for it breaking images, like you've found.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

QuLogic
QuLogic reviewed Aug 7, 2020
View reviewed changes

@QuLogic QuLogic left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for 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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/ft2font.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/ft2font.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/ft2font.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/ft2font.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/_path.h Outdated
Comment on lines +1111 to +1113
while (q >= str && *q == '0') { --q; }
// If the end is a decimal point, delete that too
if (q >= str && *q == '.') { --q; }

@QuLogic QuLogic Aug 7, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for 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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@anntzer anntzer Aug 7, 2020

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for 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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@anntzer anntzer Aug 9, 2020

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for 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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@anntzer
anntzer force-pushed the unttpdf branch 2 times, most recently from 586edd9 to dd0482b Compare August 7, 2020 09:37
jkseppan
jkseppan approved these changes Aug 8, 2020
View reviewed changes

@jkseppan jkseppan left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for 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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@anntzer
Switch from a hand-written glyph outline decomposer to FreeType's one.
4469385
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.
@anntzer
anntzer force-pushed the unttpdf branch 2 times, most recently from d81d35b to 385894b Compare August 9, 2020 22:13
QuLogic
QuLogic reviewed Aug 11, 2020
View reviewed changes
Comment thread src/_path.h
--c;
}
try {
buffer.append(str, c + 1);

@QuLogic QuLogic Aug 11, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for 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...

Suggested change
buffer.append(str, c + 1);
buffer.append(str, c - str + 1);

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@anntzer anntzer Aug 12, 2020

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for 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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread lib/matplotlib/backends/backend_pdf.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@anntzer

anntzer commented Aug 12, 2020

Copy link
Copy Markdown
Contributor Author

One thing I forgot to mention is that the Py_DTSF_ADD_DOT_0 change also fixes a separate bug in __add_number, whereby passing precision = 0 would previously print values without the dot and then trailing but significant zeros would be incorrectly stripped out. This doesn't affect the patch here because this uses the separate bug-compat precision=-1 path, but the (future) backend_ps patch will use precision=0 and thus benefit from this.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@anntzer
Stop using ttconv for pdf.
19359c1
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).
@anntzer
anntzer force-pushed the unttpdf branch from 385894b to 19359c1 Compare August 12, 2020 22:40
QuLogic
QuLogic approved these changes Aug 13, 2020
View reviewed changes
@QuLogic

QuLogic commented Aug 13, 2020

Copy link
Copy Markdown
Member

Should we wait for the baseline image PR before merging this?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@anntzer

anntzer commented Aug 13, 2020

Copy link
Copy Markdown
Contributor Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jklymak jklymak added this to the v3.4.0 milestone Aug 13, 2020
@jklymak
jklymak merged commit c9bf76c into matplotlib:master Aug 13, 2020
@jklymak

jklymak commented Aug 13, 2020

Copy link
Copy Markdown
Member

I'll merge since @anntzer is the main person concerned with images changing 😉

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@anntzer
anntzer deleted the unttpdf branch August 13, 2020 16:10
@anntzer anntzer mentioned this pull request Aug 28, 2020
Move PostScript Type3 subsetting to pure python. #18370
Merged
7 tasks
@anntzer anntzer mentioned this pull request Feb 6, 2021
[bug] segmentation fault when try to savefig a pdf with Chinese font #19469
Closed
@QuLogic QuLogic mentioned this pull request Mar 18, 2021
DOC: Prepare What's new page for 3.4.0. #19713
Merged
5 tasks done
@anntzer anntzer mentioned this pull request May 1, 2025
Update FreeType to 2.13.3 #29816
Merged
5 tasks
@QuLogic QuLogic mentioned this pull request Jun 5, 2025
Remove ttconv backwards-compatibility code #30145
Merged
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkseppan jkseppan jkseppan approved these changes
@QuLogic QuLogic QuLogic approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

backend: pdf topic: text/fonts

Projects

None yet

Milestone

v3.4.0

Development

Successfully merging this pull request may close these issues.

Missing character upon savefig() with Free Serif font

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@anntzer @QuLogic @jklymak @jkseppan
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.