microsoft / Font-Validator

Font Validator is a tool for testing fonts prior to release. This testing ensures that fonts meet Microsoft's high quality standards and perform exceptionally well on Microsoft's platform.
Other
117 stars 41 forks source link

OS/2: "The version number is valid, but less than 5" [W2106] #11

Open anthrotype opened 8 years ago

anthrotype commented 8 years ago

I'd recommend to downgrade this warning message to informative status. I think it's perfectly fine to have OS/2 table version less than the current one (version 5), as long as one does not attempt to use any of the flags which are only defined in subsequent versions of the table.

HinTak commented 8 years ago

Somewhat edited version of my previous comment ( https://github.com/HinTak/Font-Validator/issues/9#issuecomment-162055208 )

I am okay with that. It got bumped up from v3 to v5 because Behdad's CBLC/CBDT font sample has a v4 OS/2 table . So the recent change basically means warning on every font I have, as I don't actually have a v5 sample.

Two questions for Microsoft folks: (1) how keen they are to push people to update fonts to latest spec, as well as (2) their plans on their internal version in this matter, and anticipating divergence between the two.

anthrotype commented 8 years ago

I just found another issue with OS/2 table version.

My font has OS/2 table version 4 and the bit 7 (USE_TYPO_METRICS) set. FontVal is complaining that:

E2101:  There are undefined bits set in fsSelection field:  Bit(s) 7

However bit 7 is indeed defined in OS/2 version 4: http://www.microsoft.com/typography/otspec/os2ver4.htm

HinTak commented 8 years ago

The fsSelection field usage should be easy enough to update. Can you post a font sample for testing? When I bumped the version up from v3 to v5, I only added the larger table size of v5 as valid, and left a bunch of TODO comments all over; if I recall, the OS/2 codepage bits usage were massively updated - some 40? reserved bits were allocated, etc. That's a big TODO. Adding a few fsSelection bits should be easy.

anthrotype commented 8 years ago

this font has OS/2 format 4 and fsSelection bit 7 set: https://github.com/behdad/fonttools/blob/master/Lib/fontTools/ttLib/testdata/TestTTF-Regular.ttx

HinTak commented 8 years ago

@anthrotype I think this is the change to allow bit 7,8,9 for OS/2 v4+ . Can you give it a go? I'll have it in my private repo and test it when I next-sync the private repo with the repo for the next binary build, but that's weeks away and not happening any time soon. If you can give it a test that would be great.

diff --git a/OTFontFileVal/val_OS2.cs b/OTFontFileVal/val_OS2.cs
index 4738f4a..f8fd11e 100644
--- a/OTFontFileVal/val_OS2.cs
+++ b/OTFontFileVal/val_OS2.cs
@@ -641,8 +641,8 @@ namespace OTFontFileVal
                 }

                 // reserved bits
-                // TODO: Bit 7,8,9, v4
-                if ( (fsSelection & 0xFF80 ) != 0 )
+                if ( ( (fsSelection & 0xFF80 ) != 0 )
+                     || ((fsSelection & 0xFB00 ) != 0 && version > 3) )
                 {
                     // we need to look for Win 3.1 font pages
                     // Fonts with these
@@ -686,7 +686,8 @@ namespace OTFontFileVal
                                 for (int i=0; i<16; i++)
                                 {
                                     int nBitValue = 1<<i;
-                                    if ((nBitValue & 0xFF80) != 0)
+                                    if ( ((nBitValue & 0xFF80) != 0 && version < 4)
+                                         || ((fsSelection & 0xFB00) != 0) )
                                     {
                                         if ((fsSelection & nBitValue) != 0)
                                         {
@@ -711,7 +712,8 @@ namespace OTFontFileVal
                         for (int i=0; i<16; i++)
                         {
                             int nBitValue = 1<<i;
-                            if ((nBitValue & 0xFF80) != 0)
+                            if ( ((nBitValue & 0xFF80) != 0 && version < 4)
+                                 || ((nBitValue & 0xFB00) != 0) )
                             {
                                 if ((fsSelection & nBitValue) != 0)
                                 {
anthrotype commented 8 years ago

I can't apply the patch as Markdown screwed the formatting. You need three backtick quotes to embed a block of code in Markdown

``` `````` Why don't you just make a branch on your own fork? I can then `git checkout` it.
HinTak commented 8 years ago

@anthrotype : my markdown-fu is a bit lacking - anyway, the markdown is corrected now...

HinTak commented 8 years ago

Sorry about that... need to improve my markdown-fu :-). My private repo is a bit messy - the current pattern seems to work okay, I do changes, sit on it for a few days (or weeks...), when things don't break, scrub, collapse, and push it out just before I want to do a binary build...

Anyway, still going through my old e-mails...

anthrotype commented 8 years ago

I pulled your fork's master branch and applied that patch. Then rebuilt it with make clean && make, and tried again with my test font, which you can find here:

I run the FontValidator.exe like that:

mono bin/FontValidator.exe -file "/Users/cosimolupo/Desktop/TestTTF-Regular.ttf"

Unfortunately, the generated report is still complaining that the OS/2 table "version number is valid, but less than 5...". The "Details" field on the right says "3", but the version of the table in the actual font is "4"...

The test font has Bit 6 (REGULAR) and Bit 7 (USE_TYPO_METRICS) set. The latter was specified in OS/2 table v. 4. I don't think there's anything wrong with having an earlier version of a table, if you don't use any of the bit fields which are reserved for subsequent versions.

HinTak commented 8 years ago

The version check is near line 36 of val_OS2.cs, in Validate():

            if (v.PerformTest(T.OS_2_Version))
            {
                if (version == 0 || version == 1 || version == 2 || version == 3 || version == 4 )
                {
                    v.Warning(T.OS_2_Version, W.OS_2_W_Version_old, m_tag, version.ToString());
                }
                else if (version == 5)

Not sure how the "Details" part can says 3, as it should come out of version.ToString() (the 3rd argument of v.Warning()).

If you like to experiment with switching warning to informative, it might be just as simple as changing v.Warning() to v.Info() ; though I suspect it may or may not check that the 2nd argument is part of the W{} enum's and not the I{} enum's. Modifying the two lists of enums is hard, as they are auto-generated and sync'ed with the CHM from the GenerateFValData directory by the "make gendoc" target.

anthrotype commented 8 years ago

The font's OS/2 table is version 4 for sure, as you can see: https://github.com/behdad/fonttools/blob/master/Lib/fontTools/ttLib/testdata/TestTTF-Regular.ttx#L77 But I'm afraid I don't have the time to debug this issue right now. Thanks anyway.