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

Bug in table 'loca' when subsetting #32

Closed
gettalong opened this issue Nov 1, 2016 · 9 comments
Closed

Bug in table 'loca' when subsetting #32

gettalong opened this issue Nov 1, 2016 · 9 comments

Comments

@gettalong
Copy link
Member

The issue prawnpdf/prawn#972 describes a problem with Prawn when subsetting a TTF font into a PDF.

@Siyfion provided a test case for tracking down the bug. If one substitutes "A" for the text to be output (instead of "ABCDEFG...") everything works fine, the subset font is valid. However, if one uses "D", nothing is shown and loading the subset font in FontForge yields an error.

The problem stems from ttfunk supporting the 'short' type for the 'loca' table. Using this type offsets aren't stored directly, only their value divided by two is stored. This works fine as long all glyph data has a length that is even. In case of the "D" glyph, however, the length is odd, leading to an invalid offset and and invalid 'loca' table.

The method Loca.encode at https://github.com/prawnpdf/ttfunk/blob/master/lib/ttfunk/table/loca.rb#L15 needs to be changed to the following to always output a valid table:

      def self.encode(offsets)
        { :type => 1, :table => offsets.pack("N*") }
      end

The only disadvantage of this change is that the 'loca' table might be larger than needed.

@Siyfion
Copy link

Siyfion commented Nov 2, 2016

I have just tested this change with a variety of different cases that were failing previously and they are all now working correctly. Any idea if/when we could look at getting this merged, as this is causing havoc with my production servers!

@Siyfion
Copy link

Siyfion commented Mar 4, 2017

@practicingruby @smetana Any chance of an update on this... still have the issue, and it's really annoying! 👍

@Siyfion
Copy link

Siyfion commented Apr 10, 2017

Is there any reason why the fix to this issue isn't being merged into the latest releases? @packetmonkey @smetana @pointlessone ??

@pointlessone
Copy link
Member

@Siyfion Yes. Lack of time.

@Siyfion
Copy link

Siyfion commented Apr 10, 2017

@pointlessone But there's been releases made, and there's a 1 line fix above...?

@pointlessone
Copy link
Member

pointlessone commented Apr 10, 2017

@Siyfion One line does not mean no time to validate and integrate the fix.

You are not forced to use upstream version. Feel free to fork, apply the fix and use your fork in your project.

@Siyfion
Copy link

Siyfion commented Apr 10, 2017

@pointlessone Oh I appreciate that, I've got open-source projects too!

It's just that 16 commits have been made since this issue, including adding TTC support, which I imagine is a hell of a lot more work, testing and code than fixing this, which at its most fundamental level, stops TTF fonts from rendering... ie. Quite a serious issue

Now I can, as you say, fork, apply and use that (and I do currently), but as someone using this library in production currently, there's a great deal of pressure on me to make sure that we are using the "best" (not always the easiest thing to measure) libraries, that are well-supported, maintained, etc. and this doesn't help my cause!

@prawnpdf prawnpdf locked and limited conversation to collaborators Apr 10, 2017
@pointlessone
Copy link
Member

@Siyfion Thank you for your contribution to the discussion. I'm locking this issue now as it heads into a somewhat unproductive direction. Please feel welcome to open a PR with a detailed explanation why the fix is appropriate along with the links to spec. Saving maintainers research time will help a lot handling the PR in a timely manner.

@pointlessone
Copy link
Member

This is fixed in 1.5.1.

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

No branches or pull requests

3 participants