Modified the "missing glyph" warning to also print out the name of the font the glyph is missing from. This is mostly to help me debug why#26854 is failing on CI but working locally but there have been times where I've gotten this error and would have found it helpful to have the added context.

~~second commit adds a :filter-warning: option to the..plot directive~~ spun out into#27076

PR checklist

[n/a] "closes #0000" is in the body of the PR description tolink the related issue
new and changed code istested
[n/a]Plotting related features are demonstrated in anexample
[n/a]New Features andAPI Changes are noted with adirective and release note
[n/a] Documentation complies withgeneral anddocstring guidelines

story645 added topic: text/fonts Maintenance labels

Oct 3, 2023

story645 changed the title~~print fontname in missing glyph warning~~MNT: print fontname in missing glyph warning

Oct 3, 2023

Copy link

Member

tacaswell commentedOct 3, 2023

How does this interact with the fallback list? Presumable we would want to show all of the fonts that were considered?

Copy link

MemberAuthor

story645 commentedOct 4, 2023•
edited
Loading

How does this interact with the fallback list? Presumable we would want to show all of the fonts that were considered?

Currently fallback suppresses the missing font and if fallback doesn't work only prints the first font

eta: I think b/c it's marked as a flag once and load_char_with_fallaback is kinda recursive.

matplotlib/src/ft2font.cpp

Lines 514 to 518 in21ce592

	bool was_found =load_char_with_fallback(ft_object_with_glyph, glyph_index, glyphs,
	char_to_font, glyph_to_font, codepoints[n], flags,
	charcode_error, glyph_error,false);
	if (!was_found) {
	ft_glyph_warn((FT_ULong)codepoints[n]);

story645 force-pushed thewarning-fontname branch froma5d314f toc952d15Compare

October 4, 2023 04:28

Copy link

MemberAuthor

story645 commentedOct 4, 2023

: I think b/c it's marked as a flag once and load_char_with_fallaback is kinda recursive.

Which also means I can't think of a clean way to only print the warnings if the fallback fails w/o maintaining a separate stack of warnings & I think that rabbit hole is out of scope for this PR.

story645 force-pushed thewarning-fontname branch 2 times, most recently from22e8b01 toe16b88dCompare

October 4, 2023 18:56

Copy link

MemberAuthor

story645 commentedOct 4, 2023•
edited
Loading

Added a :filter-warning: to the plot directive so docs can build cleanly, but I'm really unsure that raising the warnings is all that helpful to someone using fallback-presumably they're using it b/c they expect that not all the fonts will work.

story645 force-pushed thewarning-fontname branch fromc5648ce tob613bd7Compare

October 4, 2023 19:39

Copy link

Member

tacaswell commentedOct 4, 2023

When we exhaust the list of fallback fonts, do we have anyway to get back to the top of the list and walk it again collecting all of their names?

Copy link

MemberAuthor

story645 commentedOct 4, 2023•
edited
Loading

When we exhaust the list of fallback fonts, do we have anyway to get back to the top of the list and walk it again collecting all of their names?

Kinda maybe? if we exhaust the list, it means i==fallbacks.size() and if that works that's definitely a cleaner solution than the changes I've been making of passing around a cache object...but also currently pybind doesn't want to work for me so I can't build anything.

matplotlib/src/ft2font.cpp

Lines 672 to 680 in6ba7d5f

	for (size_t i =0; i < fallbacks.size(); ++i) {
	bool was_found = fallbacks[i]->load_char_with_fallback(
	ft_object_with_glyph, final_glyph_index, parent_glyphs, parent_char_to_font,
	parent_glyph_to_font, charcode, flags, charcode_error, glyph_error,override);
	if (was_found) {
	returntrue;
	}
	}
	returnfalse;

story645 force-pushed thewarning-fontname branch fromd0908d8 todc2366aCompare

October 4, 2023 23:10

Copy link

MemberAuthor

story645 commentedOct 12, 2023

So I was wrong in that it'll miss the first glyph when there's an override.

story645 force-pushed thewarning-fontname branch fromdc2366a to4aaf163Compare

October 12, 2023 21:23

Copy link

MemberAuthor

story645 commentedOct 12, 2023

Got this to work by adding an array to F2tFont to store the names of the fonts. Reason is because if you can't find anything, you need to printfont + fallback_list

story645 force-pushed thewarning-fontname branch from4aaf163 to8ca754aCompare

October 12, 2023 21:27

story645 requested a review fromtacaswell

October 12, 2023 21:28

story645 force-pushed thewarning-fontname branch 3 times, most recently fromc66bfd9 tof9274f3Compare

October 12, 2023 22:02

story645 mentioned this pull request

Oct 12, 2023

DOC: plot-directive warning filter option#27076

Closed

5 tasks

story645 force-pushed thewarning-fontname branch 3 times, most recently fromdf5d2ae toc719d89Compare

October 15, 2023 01:41

story645 mentioned this pull request

Oct 15, 2023

[ENH] xkcd.mplstyle w/ quasi parsing of patheffects.{functions}#26854

Open

4 tasks

ksunden reviewed

Oct 16, 2023

View reviewed changes

src/ft2font.cpp OutdatedShow resolvedHide resolved

story645 force-pushed thewarning-fontname branch from16a3fb9 to09584ffCompare

October 16, 2023 20:43

ksunden reviewed

Oct 16, 2023

View reviewed changes

src/ft2font.cpp OutdatedShow resolvedHide resolved

story645 force-pushed thewarning-fontname branch 4 times, most recently fromb3dd35d to1ff3b42Compare

October 17, 2023 15:23

ksunden reviewed

Oct 17, 2023

View reviewed changes

lib/matplotlib/tests/test_backend_pdf.py OutdatedShow resolvedHide resolved

story645 force-pushed thewarning-fontname branch 2 times, most recently from0a39542 to296977bCompare

October 17, 2023 19:56

tacaswell reviewed

Oct 18, 2023

View reviewed changes

src/ft2font.h OutdatedShow resolvedHide resolved

tacaswell previously requested changes

Oct 18, 2023

View reviewed changes

Copy link

Member

tacaswell left a comment

Choose a reason for hiding this comment

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

I think this can be done without adding any new state to the objects.

story645 force-pushed thewarning-fontname branch from296977b toeee55abCompare

October 19, 2023 01:51

Copy link

MemberAuthor

story645 commentedOct 19, 2023

changed to parameter into function

story645 requested a review fromtacaswell

October 19, 2023 20:39

story645 added this to thev3.8.1 milestone

Oct 22, 2023

tacaswell reviewed

Oct 26, 2023

View reviewed changes

lib/matplotlib/tests/test_ft2font.py OutdatedShow resolvedHide resolved

tacaswell reviewed

Oct 26, 2023

View reviewed changes

src/ft2font.h OutdatedShow resolvedHide resolved

tacaswell dismissed theirstale review

October 26, 2023 20:30

passing cache through works.

story645 force-pushed thewarning-fontname branch fromeee55ab to249f4beCompare

October 27, 2023 05:01

Copy link

MemberAuthor

story645 commentedOct 27, 2023•
edited
Loading

one warning w/ all the fonts instead of n warnings w/ one font per@tacaswell's request - and I'm 99% sure that the set can be passed from c++ to python directly but since this is for a string I figure there's no harm in doing the strcat on the C++ side since passing the set over would I think involve copying the set to the python set object. I figure this can get cleaned up as part of the pybind port if it's important enough.

story645 force-pushed thewarning-fontname branch 2 times, most recently from22c8c97 to6470890Compare

October 27, 2023 06:00

prints font name in missing glyph warning

4ece8ab

lists all fallback fonts if glyph missing in all fontsremoved ft_get_char_index_or_warnadded a test for otf fonts on pdf + some windows fonts to testsCo-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>Co-authored-by: Thomas A Caswell <tcaswell@gmail.com>

story645 force-pushed thewarning-fontname branch from6470890 to4ece8abCompare

October 27, 2023 16:25

mend

15c44a5