8000 Port fontconfig's font weight detection to font_manager. by anntzer · Pull Request #16203 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
May 28, 2020
Merged

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

merged 3 commits into from
May 28, 2020

Conversation

anntzer
Copy link
Contributor
@anntzer anntzer commented Jan 13, 2020 •
edited
Loading

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

Choose a reason for hiding this comment

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

Seems a straightforward conversion, except for one thing.

@QuLogic
Copy link
Member
QuLogic commented May 22, 2020 •
edited
Loading

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
Copy link
Contributor Author
anntzer commented May 23, 2020

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
Copy link
Member
QuLogic commented May 25, 2020 •
edited
Loading

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
Copy link
Contributor Author
anntzer commented May 25, 2020

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
Copy link
Member
QuLogic commented May 25, 2020 •
edited
Loading

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

QuLogic
QuLogic approved these changes May 25, 2020
View reviewed changes
@QuLogic
Copy link
Member
QuLogic commented May 27, 2020

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

Don't forget this.

@anntzer
Copy link
Contributor Author
anntzer commented May 27, 2020

done

@jklymak
Copy link
Member
jklymak commented May 27, 2020

@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
Copy link
Member

If you think it is ok, merge it.

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

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
Copy link
Member
QuLogic commented Feb 4, 2022

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

@dschick dschick mentioned this pull request Apr 6, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuLogic QuLogic QuLogic approved these changes

@tacaswell tacaswell Awaiting requested review from tacaswell

Assignees
No one assigned
Labels
topic: text/fonts
Projects
None yet
Milestone
v3.3.0 < 440C /form>
Development

Successfully merging this pull request may close these issues.
5 participants
@anntzer @QuLogic @jklymak @tacaswell @AlvaroFurlani
0