Skip to content
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.

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

[T1Lib] T1Font.saveAs() broken? #3238

Open
sl2c opened this issue Aug 3, 2023 · 15 comments · Fixed by #3240
Open

[T1Lib] T1Font.saveAs() broken? #3238

sl2c opened this issue Aug 3, 2023 · 15 comments · Fixed by #3240

Comments

@sl2c
Copy link

sl2c commented Aug 3, 2023

After doing:

t1 = T1Font('font.pfb')
t1.parse()
t1.saveAs('font-out.pfb', 'PFB')

and trying to open the resulting pfb file in fontforge I get an error from fontforge:

Didn't understand "/... " in blended font defn

This happens for every .pfb font I've tried (the original files are opened in fontforge with no problems at all).

Using the latest pip3 install of fontTools

@justvanrossum
Copy link
Collaborator

Can you post a before/after example? If I round-trip the pfb file in the repo ("Tests/t1Lib/data/TestT1-Regular.pfb") I don't find any significant differences.

blended font

This suggests your font is a Multiple Master font, and perhaps the bug is related to that. We don't have such a font as test data.

@sl2c
Copy link
Author

sl2c commented Aug 3, 2023

fonts.zip

justvanrossum added a commit that referenced this issue Aug 3, 2023
- fix when /Private follows /CharStrings instead of vv
- fix expected ND and PD values
- handle the case where lenIV is not 4 (writing)
- write /Subrs properly, not just the "standard 5"
@justvanrossum
Copy link
Collaborator

justvanrossum commented Aug 3, 2023

I've managed to fix the problems, but be aware that Type 1 is extremely Legacy, and fontTools.t1Lib is hardly maintained.

fontTools.t1Lib has had only limited use, and mostly for reading. Writing has hardly been tested in the field.

So while #3240 seems to address the issues exposed by this font, I'm not guaranteeing much support for the future.

Please reconsider wether you really really really need Type1 fonts in the first place.

@sl2c
Copy link
Author

sl2c commented Aug 4, 2023

Thanks form the prompt fix!

I've run your commit and the result causes the fontforge to output the same error. Not sure, what the problem is. At one point I thought that it's because I'm running it in python3 (the T1Font page on python docs says something like "python 2 only"), so I tried running under Python 2 and I get this:

Python 2.7.16 (default, Mar 25 2021, 18:52:10)
[GCC 4.2.1 Compatible Apple LLVM 10.0.1 (clang-1001.0.37.14)] on darwin
Type "help", "copyright", "credits" or "license" for more information.

from fontTools.t1Lib import T1Font
Traceback (most recent call last):
File "", line 1, in
File "/Users/user/Code/Python/lib/fontTools/init.py", line 2, in
from fontTools.misc.loggingTools import configLogger
File "/Users/user/Code/Python/lib/fontTools/misc/loggingTools.py", line 5, in
from collections.abc import Mapping, Callable
ImportError: No module named abc

Any ideas on how to troubleshoot this?

As for your:

Please reconsider wether you really really really need Type1 fonts in the first place.

well, the Type1 fonts may be retired by Adobe, but these are almost exclusively the fonts used in TeX/LaTeX typesetting. In other words, pretty much every scientific paper published to date (I mean those that were electronically typeset, and I exclude the ugly MS Word-typeset ones) and pretty much every paper that will be published in the next decade will be using Type1 fonts exclusively.

(The reason is that in TeX/LaTeX, there are gazillion lines of tex code written for packages that heavily rely on the fonts being in the TFM/PFB combo; It's hard to imagine how a transition may occur to anything other than that. There's, of course, XeTeX/XeLaTeX, which uses TTF fonts, but it's incompatible on the package level, and so despite some initial promise, its use by the scientific community is negligible)

So, I am sorry to say that as far as publishing science is concerned, Type1 fonts are to stay for at least another decade (subjectively).

And as for whether I personally really need these — yes, I'm myself a scientist and I'm writing a tool that will migrate pre-Type1 era scientific papers to the Type1 era, so to say, by replacing the Type3 fonts with Type1 — kinda like what the https://en.wikipedia.org/wiki/Pkfix and https://ctan.org/pkg/pkfix-helper?lang=en do, but for PDFs instead of Postscript files.

Why not move one step further: Type3 --> TTF. Well, because these old fonts are only available in Type1, and some of them — not even in Type1, but in the form of the METAFONT sources (https://en.wikipedia.org/wiki/Metafont). And for these I am using a tool called mftrace (https://ctan.org/pkg/mftrace?lang=en) that vector-traces bitmaps with the help of potrace (https://potrace.sourceforge.net/) to produce, you guessed it, Type1 PFB files.

Now, if you know of a way to convert Type1 to TTF I would love to use it of course — that would add another step to all the processes I described above, and in the end my code would go for a direct replacement of Type3 --> TrueType inside PDFs. Otherwise, I guess I am stuck with Type1 and with asking you to fix more bugs.

An alternative would be for me to read the Adobe Type1 font spec, figure out what the current code in T1Lib does, and join the ranks of its developers. But frankly, I am already overstretched as it is, given the mess I mentioned above.

And this is why I am really really asking for help here. And I think the cause is worth the effort.

Btw, when in my first successful test a file is converted from using raster fonts at 300dpi to using vector Type1 fonts (with proper hinting) the results look just amazing. Let me know if you have time for watching a few beautiful glyphs in the formulas in scientific papers, I might upload a few. If not just take my word for it — this might give new life to tons of old papers, for which no TeX/LaTeX sources exist anymore. And which look uglier than they should.

@justvanrossum
Copy link
Collaborator

I've run your commit and the result causes the fontforge to output the same error.

No idea — are you sure you are running the patched fonttools?

so I tried running under Python 2

FontTools does not support Python 2.

@sl2c
Copy link
Author

sl2c commented Aug 4, 2023

I've run your commit and the result causes the fontforge to output the same error.

No idea — are you sure you are running the patched fonttools?

I am: I cloned master, but it for some reason had the old version of the file (so I assumed the commit was to a branch other than master, but I couldn't figure out which), so what I did was: I opened the commit page:

2808d99

then clicked browse files:

https://github.com/fonttools/fonttools/tree/2808d995b2e66969d84470450b168d928c0336d7

then clicked Code and chose to download ZIP.

After unpacking I made sure the new t1Lib/init.py reflected the changes you made (all this bc I have no idea how to use github properly, I suppose). But none of that helped.

so I tried running under Python 2

FontTools does not support Python 2.

https://fonttools.readthedocs.io/en/latest/t1Lib.html — I meant the header here.

@justvanrossum
Copy link
Collaborator

https://fonttools.readthedocs.io/en/latest/t1Lib.html — I meant the header here.

Ah, that's a stale comment. I've removed it in my PR.

What you can do to check whether you are actually running the patched module is to introduce an error, say add raise AssertionError("test") at some place in the patched code that you know must run. For example in the T1Font.__init__().

@sl2c
Copy link
Author

sl2c commented Aug 4, 2023

Did that, and it raises the error, as expected.

I've attached the before/after pfb files, as well as the init.py file I got.
fonts.zip

Maybe my sequence

from fontTools.t1Lib import T1Font
t1 = T1Font('file.pfb')
t1.parse()
t1.save('file-out.pfb','PFB')

is missing some steps?

UPD. Oh, if I remove the t1.parse() step it produces a correct file.

UPD2: Here's what I see from the code in __init__.py: T1Font.__init__() loads original PFB file data to self.data. T1Font.parse() parses self.data into self.font, and self.data is deleted. T1Font.saveAs() checks if self.data exists and if not it regenerates it with T1Font.createData() from .font. So, either .parse() is broken, or .createData() is broken.

@justvanrossum
Copy link
Collaborator

I don't have fontforge, but another tool opens the "-out" font just fine (which it didn't with your initial fonts.zip). There still may be something wrong with the font, but I'm out of capacity to debug this further. If you can find out specifically what makes fontforge choke on this font we can perhaps work out a further fix.

@sl2c
Copy link
Author

sl2c commented Aug 4, 2023

I tried to use the resulting file with pdftex (it embeds pfb files into PDF, basically). The output:

!pdfTeX error: pdftex (file ./file-out.pfb): 138039 bytes more than expected
==> Fatal error occurred, no output PDF file produced!

@justvanrossum
Copy link
Collaborator

I merged #3240, but will reopen this as the issues aren't completely resolved.

@justvanrossum justvanrossum reopened this Aug 4, 2023
@miguelsousa
Copy link
Collaborator

@sl2c with this script you can convert a Type 1 font into an OpenType-CFF font.
If you need a TrueType font you can use otf2ttf.py to convert the OTF font.

@miguelsousa
Copy link
Collaborator

@justvanrossum I used the AFDKO's detype1 tool to convert the font-out.pfb file into a PFA and the charstrings are garbled. If I use font.pfb instead the charstrings look correct.

@justvanrossum
Copy link
Collaborator

Is the out font you’re looking at made with fonttools after my initial set of fixes? Because I did fix a lot, but apparently not enough. But I believe the charstrings survive now.

@miguelsousa
Copy link
Collaborator

But I believe the charstrings survive now.

Confirmed. My feedback was based on a file that was posted at the start of the thread.
I've updated to 4.42.1, round-tripped the PFB font, and the output looks good. Sorry for the false alarm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants