Skip to content

textpath encoding#12928

Merged
tacaswell merged 4 commits intomatplotlib:masterfrom
anntzer:textpath-encoding
Apr 2, 2019
Merged

textpath encoding#12928
tacaswell merged 4 commits intomatplotlib:masterfrom
anntzer:textpath-encoding

Conversation

@anntzer
Copy link
Copy Markdown
Contributor

@anntzer anntzer commented Dec 3, 2018 •
edited
Loading

PR Summary

Consider the following example.

import matplotlib.pyplot as plt
plt.rcParams['text.usetex'] = True
plt.rcParams['text.latex.preamble'] = r'\usepackage{siunitx}'
plt.rcParams['text.hinting_factor'] = 1
plt.text(.5, .5, r'$\si{\degree}$')
plt.text(.5, .4, r'ff\textwon')
plt.gca().set_axis_off()
plt.savefig('/tmp/plot.svg')
plt.savefig('/tmp/plot.pdf')
plt.savefig('/tmp/plot.png')
plt.show()

In the svg output, one sees that the \degree and \textwon characters
(which come from a different font that the ff ligature) are now
correctly loaded, but at a too small size -- this still needs to be
fixedn but is already better than not loading the glyphs at all.
(pdf and png output are unaffected.)
screenshot_20181203_174540

Supersedes #8286, #8415 (from which a test is taken).
Fixes part of #8068, #8280 (the glyph-loading part; the sizing issue is a bug this PR newly exposes).

Likely the bad font sizing has something to do with the use, in textpath.py of self.FONT_SCALE rather than prop.get_size_in_points()/dvifont.size...

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/api_changes.rst if API changed in a backward-incompatible way

@anntzer anntzer added this to the v3.1 milestone Dec 3, 2018
@anntzer anntzer force-pushed the textpath-encoding branch 2 times, most recently from 04dfa60 to 83f2d64 Compare December 3, 2018 20:41
anntzer
anntzer commented Dec 4, 2018
View reviewed changes
lib/matplotlib/textpath.py Outdated
# If psfonts.map specifies no encoding, the indices directly map to
# the font's builtin charmap (see the pdftex manual, section 6.1
# -- Map files); so don't use the FreeType-synthesized charmap but
# the native ones (we can't directly identify it but it's typically
Copy link
Copy Markdown
Contributor Author

@anntzer anntzer Dec 4, 2018

Choose a reason for hiding this comment

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

See also http://lists.nongnu.org/archive/html/freetype/2018-12/msg00001.html.

@anntzer
Copy link
Copy Markdown
Contributor Author

anntzer commented Dec 6, 2018 •
edited
Loading

Very empirically, it looks like glyphs are correctly sized (across multiple of font sizes) with

diff --git i/lib/matplotlib/textpath.py w/lib/matplotlib/textpath.py
index 8ef6187bd..d306e82c8 100644
--- i/lib/matplotlib/textpath.py
+++ w/lib/matplotlib/textpath.py
@@ -315,7 +315,7 @@ class TextToPath(object):
 
             if char_id not in glyph_map:
                 font.clear()
-                font.set_size(self.FONT_SCALE, self.DPI)
+                font.set_size(self.FONT_SCALE ** 2 / dvifont.size, self.DPI)
                 # See comments in _get_ps_font_and_encoding.
                 if enc is not None:
                     index = font.get_name_index(enc[glyph])

which doesn't look completely nuts, but I haven't tracked down all the resizings either.

@anntzer anntzer force-pushed the textpath-encoding branch 2 times, most recently from 533b875 to 2077dc9 Compare December 7, 2018 14:57
jklymak
jklymak approved these changes Mar 4, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

Not clear on all details, but this seems to work...

@anntzer
Copy link
Copy Markdown
Contributor Author

anntzer commented Mar 4, 2019

rebased

@anntzer anntzer force-pushed the textpath-encoding branch from 2077dc9 to 976e779 Compare March 4, 2019 20:57
tacaswell
tacaswell reviewed Apr 2, 2019
View reviewed changes
lib/matplotlib/textpath.py Outdated
glyph0 = font.load_char(charcode, flags=ft2font_flag)
# See comments in _get_ps_font_and_encoding.
if enc is not None:
index = font.get_name_index(enc[glyph])
Copy link
Copy Markdown
Member

@tacaswell tacaswell Apr 2, 2019

Choose a reason for hiding this comment

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

this used to handle the case where glyph not in enc where as this looks like it will now raise KeyError. Is this a change we want to make?

Copy link
Copy Markdown
Contributor Author

@anntzer anntzer Apr 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Restored handling of glyph not in enc (in the last commit). Not sure why I removed this in the first place.

tacaswell
tacaswell approved these changes Apr 2, 2019
View reviewed changes
Copy link
Copy Markdown
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.

modulo the question about changing .get -> [].

@anntzer anntzer force-pushed the textpath-encoding branch from 976e779 to 51436bf Compare April 2, 2019 09:35
anntzer and others added 4 commits April 2, 2019 13:54
@anntzer
Fix loading of encoded fonts in textpath.
2c0f5ec 
Consider the following example.

    import matplotlib.pyplot as plt
    plt.rcParams['text.usetex'] = True
    plt.rcParams['text.latex.preamble'] = r'\usepackage{siunitx}'
    plt.rcParams['text.hinting_factor'] = 1
    plt.text(.5, .5, r'$\si{\degree}$')
    plt.text(.5, .4, r'ff\textwon')
    plt.gca().set_axis_off()
    plt.savefig('/tmp/plot.svg')
    plt.savefig('/tmp/plot.pdf')
    plt.savefig('/tmp/plot.png')
    plt.show()

In the svg output, one sees that the \degree and \textwon characters
(which come from a different font that the ff ligature) are now
correctly loaded, *but* at a too small size -- this still needs to be
fixed.
(pdf and png output are unaffected.)
@anntzer
Added test encoding issue in SVG backend
88e1b53
@anntzer
Fix the cherry-picked test.
c5f2bbe
@anntzer
Properly warn when glyph is not in encoding.
722de6f
@anntzer anntzer force-pushed the textpath-encoding branch from 51436bf to 722de6f Compare April 2, 2019 11:54
@tacaswell tacaswell merged commit a2277af into matplotlib:master Apr 2, 2019
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Apr 2, 2019
@tacaswell
Backport PR matplotlib#12928: textpath encoding
4ffbeeb
@meeseeksmachine meeseeksmachine mentioned this pull request Apr 2, 2019
Merged
@anntzer anntzer deleted the textpath-encoding branch April 2, 2019 13:25
dstansby added a commit that referenced this pull request Apr 2, 2019
@dstansby
Merge pull request #13841 from meeseeksmachine/auto-backport-of-pr-12…
9f39678 
…928-on-v3.1.x

Backport PR #12928 on branch v3.1.x (textpath encoding)
This was referenced May 7, 2019
Closed
Merged
Closed
This was referenced Jul 5, 2019
Closed
Closed
@QuLogic QuLogic mentioned this pull request Apr 7, 2023
Closed
@ksunden ksunden mentioned this pull request Apr 9, 2023
Closed
@anntzer anntzer mentioned this pull request Mar 22, 2025
Closed
@anntzer anntzer mentioned this pull request Apr 1, 2025
Merged
5 tasks
@anntzer anntzer mentioned this pull request May 23, 2025
Merged
5 tasks
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

@jklymak jklymak jklymak approved these changes

Assignees

No one assigned

Labels

backend: svg topic: text/usetex

Projects

None yet

Milestone

v3.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@anntzer @tacaswell @jklymak