I'm Mr. Meeseek,@aitikgupta, Look at meee !

The last commit subsets and embeds Type 42 PS/EPS, here's the size difference in outputs:

nosub-dejavu.ps                ----> 1.2 MBsub-dejavu.ps                  ----> 8.5 kB

(To test it out on your machine, apply this patch fornosub-dejavu.ps:)

diff --git a/lib/matplotlib/backends/backend_ps.py b/lib/matplotlib/backends/backend_ps.pyindex 2a3ab64a1c..ed0af4ea85 100644--- a/lib/matplotlib/backends/backend_ps.py+++ b/lib/matplotlib/backends/backend_ps.py@@ -997,12 +997,8 @@ class FigureCanvasPS(FigureCanvasBase):                             ) as tmp:                                 tmp.write(fontdata)                                 tmp.seek(0, 0)-                                font = FT2Font(tmp.name)-                                glyph_ids = [-                                    font.get_char_index(c) for c in chars-                                ]                                 convert_ttf_to_ps(-                                    os.fsencode(tmp.name),+                                    os.fsencode(font_path),                                     fh,                                     fonttype,                                     glyph_ids,

The script:

importmatplotlib.pyplotaspltplt.rcParams["ps.fonttype"]=42plt.figtext(0.5,0.5,"hello")# plt.savefig('sub-dejavu.ps')                    # before applying the patch# plt.savefig('nosub-dejavu.ps')                  # after applying the patch

#20391 (comment): This appears to require testing.

I think it's difficult to testget_glyphs_subset, since it really depends on font files, and if we chose certain characters to test with a fixed font (lets say), there's no guarantee that the subset will be the same for different versions of the library.

Is there any other way to test this?

edit:@jklymak this isn't a draft anymore, just needed to address review comments 😄

You can mark as ready-to-review at any point. I just kick things to Draft so they don't show up in the queue if the PR requires action on the part of the author. (this one is still not passing the tests).

(this one is still not passing the tests).

Yeah, CI isn't installing fonttools, which is whyNoModuleFound is all over the place (even after adding it tominvers.txt:#20391 (comment))

Install is done with--no-deps, so hard dependencies also need to be listed here:https://github.com/matplotlib/matplotlib/blob/master/.github/workflows/tests.yml#L148minver.txt is only aconstraint on versions.

I think it's difficult to testget_glyphs_subset, since it really depends on font files, and if we chose certain characters to test with a fixed font (lets say), there's no guarantee that the subset will be the same for different versions of the library.

We ship DejaVu ourselves, so that should always be available for testing.

Is there any other way to test this?

A file with just 'A' embedded should be smaller than one with the full alphabet, say?
Also, a subsetted PDF should appear the same as one without subsetting (assuming Ghostscript doesn't happen to substitute the same glyphs.)

A file with just 'A' embedded should be smaller than one with the full alphabet, say?

Oh, if we just test the subset being 'smaller' yeah we can do it, but not with the exact number glyphs, since that will definitely vary. (we also setrecommeded_glyphs=True)

But even so, isn't that the 'full-time job' of the fonttools library itself? or in other words wouldn't testing theget_glyphs_subset function be the same as testing the library itself (that it does reduce the number of glyphs)?

Oh, and one more thing (sorry for not remembering this earlier): thePDF specification has some extra requirements for font subsets in section 9.6.4. The PostScript name of subsetted fonts needs to have a prepended tag of six random uppercase letters and a plus sign, e.g.EOODIA+Poetica if the original font name isPoetica. This is relevant in the case that multiple Matplotlib plots are combined in the same document, e.g. in a LaTeX paper with several figures. The random tags prevent collisions between the different versions of the same font.

extra requirements for font subsets ... PostScript name of subsetted fonts needs to have a prepended tag of six random uppercase letters and a plus sign

I think we don't do this even for Type 3 subsetting?
Since wealways try to subset those, I can probably just add a function to modify the name for Type 3 and Type 42 both.

This breaks thetest_determinism_check:

This is for obvious reasons, since we have random string inside font postscript name. (due to the specification)

Then the string shouldn't be random, but perhaps something derived from the subset of glyphs? Takehash(frozenset(glyphs)), convert it to base 26 and take the first six letters, or something like that.

The PDF specification only requires the tag for Type 1 and TrueType fonts. It probably doesn't hurt with Type 3 fonts either.

Then the string shouldn't be random, but perhaps something derived from the subset of glyphs?

That totally makes sense! Let me try this out 👍🏼

@sauerburger can you suggest a test for that, or is it already in#20633?

can you suggest a test for that, or is it already in#20633?

The test is already in, and that is exactly what is breaking the CI for this PR.
I'll rebase and push a commit, fixing the error..

It seems to me that the requested changes have been made.