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

should decoder sort tags alphabetically in table directory? #3

Closed
wants to merge 2 commits into from

Conversation

anthrotype
Copy link
Member

Hello,

I'm working on writing a Python version of the woff2 encoder/decoder for @behdad's fontTools package.
I'm not a C/C++ programmer, so apologies if my code is a bit naïve -- it's simply meant to show roughly what I mean.

The problem has to do with the order of tags in the table directory, and whether it should be responsibility of the encoder or the decoder.

According to WOFF2 specs, section "2. General Requirements":

When WOFF2 file is decompressed, the decoder MUST sort the tags in the table directory in ascending alphabetical order

Later on, at section "4. Table directory format", it adds:

the WOFF2 table directory entries define the physical order of tables in which they have been processed and encoded as part of the compressed font data stream. It is a decoder responsibility to sort and reorder the table directory when the font file is decompressed.

Now, the woff2_dec.cc does not seem to do that. It just keeps the same order as the input file's table directory.

It's actually the encoder module woff2_enc.cc that changes the offsets fields in the table headers so that the table data are reordered by increasing tag values.

So apparently, rather than the decoder reordering the the table directory alphabetically, here it's the encoder which reorders the table data according to the input TTF/OTF table directory (which in turn must be sorted alphabetically as per OT specs).

By reading the WOFF2 specs, it seems to me that it should actually be the other way around: that is, the encoder defining the table directory as "the physical order", and the decoder reordering the table directory alphabetically.

But maybe I'm just missing something else...
Anyway, thank you and... merry christmas.

Cosimo


PS: Interestingly, the OpenType Sanitizer refuses to load a woff2 file whose table directory is not sorted alphabetically. I presume that's because it uses a very similar woff2 decoder that does not reorder the tags (cc @khaledhosny).

@behdad
Copy link

behdad commented Dec 27, 2014

cc @raphlinus

@khaledhosny
Copy link
Contributor

That is my reading of the spec as well, and one of the issues I wanted to ask the web fonts working group about, so it would be nice to clarify this and update the implementations and the spec (I will update OTS if this will be needed, though it won’t be of much help until Chrome switches over to my fork).

@behdad
Copy link

behdad commented Dec 29, 2014

I also agree that this sounds like the right thing to do. Thanks! @rsheeter

@khaledhosny
Copy link
Contributor

So, what is the conclusion on this?

@rsheeter
Copy link
Contributor

Good catch. Agreed our implementation needs an update. I aspire to review and hopefully merge this week.

@khaledhosny
Copy link
Contributor

@rsheeter Did you have time to review this change? I’d like to incorporate it into OTS.

@rsheeter
Copy link
Contributor

Sorry, got side-tracked. It looks fine to me.

@anthrotype, if you haven't already, you need to sign the Google Individual Contributor License Agreement(CLA), before I can accept. I believe you can do this at https://cla.developers.google.com/about/google-individual.

@anthrotype
Copy link
Member Author

I'm happy you think this way, but I don't feel like taking credit for the patch code -- its purpose was to serve as an example.
You can reuse/modify the code as you wish.

Besides, this patch only modifies the decoder, but does not change the way the current woff2 encoder still reorders the table data alphabetically (for which there's no explicit requirement).

@anthrotype anthrotype closed this Feb 11, 2015
@anthrotype anthrotype reopened this Feb 11, 2015
@rsheeter
Copy link
Contributor

Made changes as suggested. Note that I anticipate having to update ordering again as TTC spec changes are integrated (glyf and loca fly together).

@rsheeter rsheeter closed this Feb 12, 2015
@anthrotype
Copy link
Member Author

@rsheeter
There is a problem in the latest woff2 implementation which makes the OpenType Sanitiser in Chrome and Firefox reject WOFF2 (as well as TTF/OTF) fonts as generated by woff2_compress and woff2_decompress tools.

ERROR: table directory not correctly ordered
Failed to sanitise file!

The problem is that, as I've already noted in this thread, the woff2 decoder does not properly reorder the tags in the table directory by ascending alphabetical order.

It turns out that, in one of the latest commits where you introduced TTC support 1c97553,
you have reverted that portion of code which reordered the table directory of decoded fonts. Besides, in the same commit, you have added a new OutputOrderedTags() function in font.cc which

alphabetize (not required), then put loca immediately after glyf (required)

Now, by just moving loca table away from its alphabetically determined position, you make OTS reject the font.
It is true that the WOFF2 specs says that the encoder must make sure that loca follows glyf in the WOFF2 table directory--however this only applies to WOFF2 fonts, not to decoded OTF/TTF fonts, for which the alphabetically sorted table directory still holds.

So it's not OTS which is picky, but it's the woff2 decoder which is not doing what it is required to do by both WOFF2 and OT specs.

cc @khaledhosny

@rsheeter
Copy link
Contributor

rsheeter commented Mar 2, 2015

Good catch, thank you! Will fix in decoder while leaving the encoding ordering shuffled to match woff2 spec.

@khaledhosny, I wonder if this is a problem OTS could fix rather than reject? (I'll fix woff2 either way)

@khaledhosny
Copy link
Contributor

Yes, I think OTS should just fix the table order instead of rejecting the font.

@khaledhosny
Copy link
Contributor

Fixed with khaledhosny/ots@e779d45.

@rsheeter
Copy link
Contributor

rsheeter commented Mar 2, 2015

FYI, fix is being circulated for review internally. @anthrotype, as finder (first!) of a bug in TTC I owe you a t-shirt! To collect, please email shirt size (they run slightly on the small side) and address as I should use for shipping to rsheeter AT google.com :)

@rsheeter
Copy link
Contributor

rsheeter commented Mar 4, 2015

498c637 is intended to address the need to sort the table directory when decoding while retaining the new woff2 ordering spec. I believe there could still be an issue with older versions of OTS (eg the one in shipping browsers) rejecting fonts encoded with newer versions of woff2.

My current theory is we should remove the requirement to order other than as per otspec when encoding a non-collection font. I will raise the issue with the working group.

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 this pull request may close these issues.

4 participants