FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Port fontconfig's font weight detection to font_manager. by anntzer · Pull Request #16203 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Port fontconfig's font weight detection to font_manager. - #16203

Merged
jklymak merged 3 commits into
matplotlib:masterfrom
anntzer:fontweight
May 28, 2020
ConversationCommits (3)ChecksFiles changed
Merged

Port fontconfig's font weight detection to font_manager.#16203
jklymak merged 3 commits into
matplotlib:masterfrom
anntzer:fontweight

Conversation

anntzer commented Jan 13, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Summary

This (should) fix the various font weight detection problems (#4822 #8551 #8607 #8788 #8800) by reusing fontconfig's font weight detection algorithm (specifically https://gitlab.freedesktop.org/fontconfig/fontconfig/blob/452be81/src/fcfreetype.c#L1859 https://gitlab.freedesktop.org/fontconfig/fontconfig/blob/452be81/src/fcfreetype.c#L1928 https://gitlab.freedesktop.org/fontconfig/fontconfig/blob/452be81/src/fcfreetype.c#L2006 https://gitlab.freedesktop.org/fontconfig/fontconfig/blob/452be81/src/fcfreetype.c#L2044).

The first commit is a straight port of fontconfig's algorithm, and outputs fontconfig weights; the second commit reverts to using CSS/OpenType weights, which is what we were using so far (see discussion in #10249).

To test this, delete your last fontlist-v310.json and regen one by importing matplotlib.font_manager with or without this PR and setting the PYTHONHASHSEED envvar to a fixed value -- this ensures that the json dumps stay in the same order, as we use sets, whose order depend on the hash seed.

(If we agree on the PR I'll need to bump the fontlist version number as well.)

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 force-pushed the fontweight branch 2 times, most recently from 3bf657f to 70d9103 Compare January 13, 2020 18:34
tacaswell self-requested a review January 13, 2020 20:55
tacaswell added this to the v3.3.0 milestone Jan 13, 2020
anntzer mentioned this pull request Feb 27, 2020
Closed
QuLogic mentioned this pull request May 5, 2020
Closed
3 tasks done
anntzer force-pushed the fontweight branch 2 times, most recently from ad78710 to 4a395e6 Compare May 11, 2020 13:56
QuLogic reviewed May 13, 2020
View reviewed changes
Comment thread lib/matplotlib/font_manager.py Outdated
QuLogic reviewed May 22, 2020
View reviewed changes

QuLogic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Seems a straightforward conversion, except for one thing.

Comment thread lib/matplotlib/font_manager.py Outdated
Comment thread lib/matplotlib/font_manager.py

QuLogic commented May 22, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

I tried with Fira Sans, which has a bunch of styles, and it went (heaviest to lightest) from:

FiraSans-Black black 900
FiraSans-BlackItalic black 900
FiraSans-Bold 700
FiraSans-BoldItalic 700
FiraSans-ExtraBold bold 700
FiraSans-ExtraBoldItalic bold 700
FiraSans-SemiBold semibold 600
FiraSans-SemiBoldItalic semibold 600
FiraSans-Medium medium 500
FiraSans-MediumItalic medium 500
FiraSans-Regular regular 400
FiraSans-Italic 400
FiraSans-Thin 400
FiraSans-ThinItalic 400
FiraSans-Light light 200
FiraSans-LightItalic light 200
FiraSans-ExtraLight light 200
FiraSans-ExtraLightItalic light 200

to:

FiraSans-Black 900
FiraSans-BlackItalic 900
FiraSans-ExtraBold 800
FiraSans-ExtraBoldItalic 800
FiraSans-Bold 700
FiraSans-BoldItalic 700
FiraSans-SemiBold 600
FiraSans-SemiBoldItalic 600
FiraSans-Medium 500
FiraSans-MediumItalic 500
FiraSans-Regular 400
FiraSans-Italic 400
FiraSans-Light 300
FiraSans-LightItalic 300
FiraSans-ExtraLight 275
FiraSans-ExtraLightItalic 275
FiraSans-Thin 250
FiraSans-ThinItalic 250

This is certainly better in terms of distinct values, and it matches the order shown on Google Fonts. However, Google Fonts calls ExtraLight 200, and Thin 100, and I think, so does fontconfig.

anntzer commented May 23, 2020

Copy link
Copy Markdown
Contributor Author

This is what I get:

python -c 'from matplotlib.font_manager import ft2font, ttfFontProperty; from pathlib import Path; [print(path, ttfFontProperty(ft2font.FT2Font(str(path))).weight) for path in Path("/usr/share/fonts/TTF").glob("FiraSans-*.ttf")]' | sort -n -k2
/usr/share/fonts/TTF/FiraSans-EightItalic.ttf 100
/usr/share/fonts/TTF/FiraSans-Eight.ttf 100
/usr/share/fonts/TTF/FiraSans-FourItalic.ttf 100
/usr/share/fonts/TTF/FiraSans-Four.ttf 100
/usr/share/fonts/TTF/FiraSans-HairItalic.ttf 100
/usr/share/fonts/TTF/FiraSans-Hair.ttf 100
/usr/share/fonts/TTF/FiraSans-ThinItalic.ttf 100
/usr/share/fonts/TTF/FiraSans-Thin.ttf 100
/usr/share/fonts/TTF/FiraSans-TwoItalic.ttf 100
/usr/share/fonts/TTF/FiraSans-Two.ttf 100
/usr/share/fonts/TTF/FiraSans-ExtraLightItalic.ttf 200
/usr/share/fonts/TTF/FiraSans-ExtraLight.ttf 200
/usr/share/fonts/TTF/FiraSans-UltraLightItalic.ttf 200
/usr/share/fonts/TTF/FiraSans-UltraLight.ttf 200
/usr/share/fonts/TTF/FiraSans-LightItalic.ttf 300
/usr/share/fonts/TTF/FiraSans-Light.ttf 300
/usr/share/fonts/TTF/FiraSans-BookItalic.ttf 350
/usr/share/fonts/TTF/FiraSans-Book.ttf 350
/usr/share/fonts/TTF/FiraSans-Italic.ttf 400
/usr/share/fonts/TTF/FiraSans-Regular.ttf 400
/usr/share/fonts/TTF/FiraSans-MediumItalic.ttf 500
/usr/share/fonts/TTF/FiraSans-Medium.ttf 500
/usr/share/fonts/TTF/FiraSans-SemiBoldItalic.ttf 600
/usr/share/fonts/TTF/FiraSans-SemiBold.ttf 600
/usr/share/fonts/TTF/FiraSans-BoldItalic.ttf 700
/usr/share/fonts/TTF/FiraSans-Bold.ttf 700
/usr/share/fonts/TTF/FiraSans-ExtraBoldItalic.ttf 800
/usr/share/fonts/TTF/FiraSans-ExtraBold.ttf 800
/usr/share/fonts/TTF/FiraSans-HeavyItalic.ttf 900
/usr/share/fonts/TTF/FiraSans-Heavy.ttf 900
/usr/share/fonts/TTF/FiraSans-UltraItalic.ttf 950
/usr/share/fonts/TTF/FiraSans-Ultra.ttf 950

that looks correct?

QuLogic commented May 25, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

I pulled these fonts from Google Fonts, but maybe they're wrong?

The Fedora-packaged version are .otf, so I'm not sure if they're equivalent, but if I use those, the weights are the same as yours, except for ExtraLight, which is 250 instead of 200.

If I run fc-scan FiraSans-Thin.{ttf,otf}, then the otf has weight:0 while the ttf has weight:45.5.

anntzer commented May 25, 2020

Copy link
Copy Markdown
Contributor Author

Mine were from Arch's distro packages.
I agree with your observations re google's fonts. However the weight values are just directly read (in this case) from the OS/2 table's usWeightClass entry (https://docs.microsoft.com/en-us/typography/opentype/spec/os2#usweightclass), which is the first thing FreeType looks at. So I'm quite tempted to just claim that google's fonts have incorrect metadata...

QuLogic commented May 25, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

I also confirmed in FontForge, so I guess I'll report a bug on Google Fonts.

QuLogic approved these changes May 25, 2020
View reviewed changes
Comment thread lib/matplotlib/font_manager.py Outdated

QuLogic commented May 27, 2020

Copy link
Copy Markdown
Member

(If we agree on the PR I'll need to bump the fontlist version number as well.)

Don't forget this.

anntzer commented May 27, 2020

Copy link
Copy Markdown
Contributor Author

done

jklymak commented May 27, 2020

Copy link
Copy Markdown
Member

@tacaswell you self-requested a review on this. Did you want it to wait?

I'd merge on the basis of @anntzer and @QuLogic combined expertise, and because it seems an improvement, not because I particularly understand the guts of the font weight detection.

tacaswell commented May 28, 2020

Copy link
Copy Markdown
Member

If you think it is ok, merge it.

jklymak merged commit a6f31e3 into matplotlib:master May 28, 2020
anntzer deleted the fontweight branch May 28, 2020 07:16
This was referenced May 28, 2020
Closed
Closed
Closed
Closed
Closed
Closed

AlvaroFurlani commented Feb 4, 2022

Copy link
Copy Markdown

Is there any resolution for Google Fonts? I'm trying to use Public Sans and the weight in matplotlib for the Thin and ExtraLight styles are both 250, instead of 100 and 200, respectively.

QuLogic commented Feb 4, 2022

Copy link
Copy Markdown
Member

Please open a new issue with an explanation of what you mean.

dschick mentioned this pull request Apr 6, 2023
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

QuLogic QuLogic approved these changes
tacaswell Awaiting requested review from tacaswell

Assignees

No one assigned

Labels

topic: text/fonts

Projects

None yet

Milestone

v3.3.0

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL