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

Error subsetting CFF font with no Subrs #42

Open
jahewson opened this issue Aug 18, 2016 · 5 comments
Open

Error subsetting CFF font with no Subrs #42

jahewson opened this issue Aug 18, 2016 · 5 comments

Comments

@jahewson
Copy link

jahewson commented Aug 18, 2016

Attempting to subset a CFF font which does not contain a Subrs entry in its Private dictionary results in an exception in subsetSubrs. The problem code is here.

@kblomster
Copy link
Contributor

kblomster commented Aug 15, 2017

I believe I ran into this when attempting to render a PDF with Futura PT using pdfmake. Is there any known workaround? I guess I could convert to TTF, but I'm unsure if the font license allows that.

/Users/kblomster/Code/redacted/api/node_modules/fontkit/index.js:13405
    for (var i = 0; i < subrs.length; i++) {
                             ^

TypeError: Cannot read property 'length' of null
    at CFFSubset.subsetSubrs (/Users/kblomster/Code/redacted/api/node_modules/fontkit/index.js:13405:30)
    at CFFSubset.createCIDFontdict (/Users/kblomster/Code/redacted/api/node_modules/fontkit/index.js:13498:30)
    at CFFSubset.encode (/Users/kblomster/Code/redacted/api/node_modules/fontkit/index.js:13548:12)
    at /Users/kblomster/Code/redacted/api/node_modules/fontkit/index.js:13027:13
    at _combinedTickCallback (internal/process/next_tick.js:73:7)
    at process._tickCallback (internal/process/next_tick.js:104:9)

@kblomster
Copy link
Contributor

kblomster commented Aug 15, 2017

I tried adding a simple if (!subrs) return res; in CFFSubset.js after line 35, and that appears to "work" in that it prevents the error above and appears to render my PDF, but I have absolutely no idea if this is actually a viable solution or not. Maybe it helps someone else?

@Pomax
Copy link
Contributor

Pomax commented Aug 15, 2017

another option is to change the function signature to have a default argument that causes the for loop to immediately exit:

subsetSubrs(subrs = [], used) {
  ...
}

This introduces less code, but also avoids an early return and early returns are great for ensuring code is fast (in that vein, an early return if (!subrs) return []; rather than allocating a res symbol would be even faster)

@kblomster
Copy link
Contributor

kblomster commented Aug 17, 2017

If that solution doesn't introduce problems, I can send a pull request for it. After looking at the code I think it won't break things, since the requested subset does not necessarily need to use any subrs anyway (I think?), but I certainly can't claim to understand the actual mechanics of subsetting a CFF font in any detail.

Getting this fixed would be nice for me.

@jahewson
Copy link
Author

jahewson commented Aug 18, 2017

The original issue which I opened against the old coffee script source no longer exists, as there is now a check for Subrs in subsetFontdict().

However, the corresponding line in createCIDFontdict() is missing this check and that seems to be the cause of your issue, looking at the stack trace. Presumably that's because you have a CID font, while I had a Type 1-equivalent font.

The advantage of adding the missing null check vs. the proposed fix above is that it won't unnecessarily create an empty Subrs dictionary in the subset font. No Subrs dictionary is needed at all.

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

No branches or pull requests

3 participants