search

fonttools / fontbakery

🧁 A font quality assurance tool for everyone
https://fontbakery.readthedocs.io
Apache License 2.0
549 stars 101 forks source link

nameID edgecase #1096

Closed alexeiva closed 7 years ago

alexeiva commented 8 years ago

https://github.com/alexeiva/Arsenal/issues/4

Arsenal is generating this error, although the naming is proper. @m4rc1e rechecked on his machine and confirmed the issue. 

Font lacks entry with nameId=17 (TYPOGRAPHIC_SUBFAMILY_NAME)  
ERROR    [FONT_FAMILY_NAME(1):WINDOWS(3)] entry: expected 'Arsenal' but got 'Arsenal Bold'

TTFs here

davelab6 commented 8 years ago

No, the naming is wrong; for the RIBBI 4 styles, nameID 1 should be the family name only, and there should not be nameIDs 16 or 17

alexeiva commented 8 years ago

@davelab6 you are wrong, sir

https://gist.github.com/alexeiva/b3c5c063a3a9cc837d4ec17bf45b94e5

alexeiva commented 8 years ago

Arsenal-Bold.TTX

davelab6 commented 8 years ago

@felipesanches confirmed this

felipesanches commented 8 years ago

@davelab6 I don't get what you mean by "Felipe confirmed this." I did not confirm this. Also, I tested it in my computer and it works for me.

I can't reproduce this issue. Please reopen if any of you still get it, but then I'll need someone to provide a more detailed description of how to replicate the issue.

davelab6 commented 8 years ago

Sorry I meant that I confirmed this :)

davelab6 commented 8 years ago

@alexeiva are you using the very latest version?

felipesanches commented 8 years ago

oh! I got it:

ERROR    Font lacks entry with nameId=17 (TYPOGRAPHIC_SUBFAMILY_NAME)  
ERROR    [FONT_FAMILY_NAME(1):WINDOWS(3)] entry: expected 'Arsenal' but got 'Arsenal Bold'

I don't know why I didn't see it the first time.

So... what is the expected behaviour in this case, @davelab6 ?

felipesanches commented 8 years ago

Also, there are 2 error messages. Both of them are problematic?

felipesanches commented 8 years ago

if I understood correctly, the commit above (https://github.com/googlefonts/fontbakery/pull/1101/commits/050da91fe8f9c845526b9492cac3a13594f819b7) fixes the detection of a RIBBI style consequently fixing the first error message, while the second message (expected 'Arsenal' but got 'Arsenal Bold') is actually correct and the font should be fixed in that case. (My rationale is based on Dave's comment here: https://github.com/googlefonts/fontbakery/issues/1096#issuecomment-251445795)

davelab6 commented 8 years ago

I said,

for the RIBBI 4 styles, nameID 1 should be the family name only

The name table is

  <name>
    <namerecord nameID="0" platformID="1" platEncID="0" langID="0x0" unicode="True">
      Copyright 2012 The Arsenal Project Authors (andrij@typedesign.org.ua)
    </namerecord>
    <namerecord nameID="1" platformID="1" platEncID="0" langID="0x0" unicode="True">
      Arsenal
    </namerecord>
    <namerecord nameID="2" platformID="1" platEncID="0" langID="0x0" unicode="True">
      Bold
    </namerecord>
    <namerecord nameID="3" platformID="1" platEncID="0" langID="0x0" unicode="True">
      2.000;MYFO;Arsenal-Bold
    </namerecord>
    <namerecord nameID="4" platformID="1" platEncID="0" langID="0x0" unicode="True">
      Arsenal Bold
    </namerecord>
    <namerecord nameID="5" platformID="1" platEncID="0" langID="0x0" unicode="True">
      Version 2.000
    </namerecord>
    <namerecord nameID="6" platformID="1" platEncID="0" langID="0x0" unicode="True">
      Arsenal-Bold
    </namerecord>
    <namerecord nameID="8" platformID="1" platEncID="0" langID="0x0" unicode="True">
      Stairsfor
    </namerecord>
    <namerecord nameID="9" platformID="1" platEncID="0" langID="0x0" unicode="True">
      Andrij Shevchenko
    </namerecord>
    <namerecord nameID="11" platformID="1" platEncID="0" langID="0x0" unicode="True">
      http://stairsfor.com/
    </namerecord>
    <namerecord nameID="12" platformID="1" platEncID="0" langID="0x0" unicode="True">
      http://andrij.com.ua/
    </namerecord>
    <namerecord nameID="13" platformID="1" platEncID="0" langID="0x0" unicode="True">
      This Font Software is licensed under the SIL Open Font License, Version 1.1. This license is available with a FAQ at: http://scripts.sil.org/OFL
    </namerecord>
    <namerecord nameID="14" platformID="1" platEncID="0" langID="0x0" unicode="True">
      http://scripts.sil.org/OFL
    </namerecord>
    <namerecord nameID="0" platformID="3" platEncID="1" langID="0x409">
      Copyright 2012 The Arsenal Project Authors (andrij@typedesign.org.ua)
    </namerecord>
    <namerecord nameID="1" platformID="3" platEncID="1" langID="0x409">
      Arsenal
    </namerecord>
    <namerecord nameID="2" platformID="3" platEncID="1" langID="0x409">
      Bold
    </namerecord>
    <namerecord nameID="3" platformID="3" platEncID="1" langID="0x409">
      2.000;MYFO;Arsenal-Bold
    </namerecord>
    <namerecord nameID="4" platformID="3" platEncID="1" langID="0x409">
      Arsenal Bold
    </namerecord>
    <namerecord nameID="5" platformID="3" platEncID="1" langID="0x409">
      Version 2.000
    </namerecord>
    <namerecord nameID="6" platformID="3" platEncID="1" langID="0x409">
      Arsenal-Bold
    </namerecord>
    <namerecord nameID="8" platformID="3" platEncID="1" langID="0x409">
      Stairsfor
    </namerecord>
    <namerecord nameID="9" platformID="3" platEncID="1" langID="0x409">
      Andrij Shevchenko
    </namerecord>
    <namerecord nameID="11" platformID="3" platEncID="1" langID="0x409">
      http://stairsfor.com/
    </namerecord>
    <namerecord nameID="12" platformID="3" platEncID="1" langID="0x409">
      http://andrij.com.ua/
    </namerecord>
    <namerecord nameID="13" platformID="3" platEncID="1" langID="0x409">
      This Font Software is licensed under the SIL Open Font License, Version 1.1. This license is available with a FAQ at: http://scripts.sil.org/OFL
    </namerecord>
    <namerecord nameID="14" platformID="3" platEncID="1" langID="0x409">
      http://scripts.sil.org/OFL
    </namerecord>
  </name>

This is correct. Therefore FB is wrong.

felipesanches commented 8 years ago

I'm sorry that I ended up git tagging fontbakery v0.1.0 and closing the milestone without really fixing this one.

Linux torvalds appologized for "the same" today: https://linux.slashdot.org/story/16/10/05/210227/linus-torvalds-says-buggy-crap-made-it-into-linux-48 :-P

I'll resume work on this issue tomorrow.

felipesanches commented 8 years ago

Running ttx on Arsenal-BoldItalic.ttf in my local checkout (on git commit https://github.com/alexeiva/Arsenal/commit/506a7434a411dc9886c50f7b1e863c0b932b8edb) results in this entry in the name table:

    <namerecord nameID="1" platformID="3" platEncID="1" langID="0x409">
      Arsenal Bold
    </namerecord>

My understanding is that FontBakery correctly reports this as a naming error that should be fixed in the font file.

felipesanches commented 8 years ago

Do you agree, @alexeiva and @davelab6 ?

alexeiva commented 8 years ago

Hm, no. Here is what I get when I run ttx on BoldItalic

<namerecord nameID="1" platformID="1" platEncID="0" langID="0x0" unicode="True">
      Arsenal
    </namerecord>

running f-b on Arsenal Bold Italic:

ERROR    [FONT_FAMILY_NAME(1):WINDOWS(3)] entry: expected 'Arsenal' but got 'Arsenal Bold'
felipesanches commented 8 years ago

Here I get 2 nameID="1" entries. The good one for plat=MAC(1) and the bad one for plat=WIN(3):

felipe@guarana:~/devel/github_felipesanches/Arsenal/fonts/ttf (master)*$ ttx -t name Arsenal-BoldItalic.ttf
Dumping "Arsenal-BoldItalic.ttf" to "Arsenal-BoldItalic.ttx"...
Dumping 'name' table...
    <namerecord nameID="1" platformID="1" platEncID="0" langID="0x0" unicode="True">
      Arsenal
    </namerecord>

    [...]

    <namerecord nameID="1" platformID="3" platEncID="1" langID="0x409">
      Arsenal Bold
    </namerecord>
felipesanches commented 8 years ago

@alexeiva could you please take a second look at your ttx output to make sure you're really missing that second nameID="1" entry? (the important entry is the one with platformID="3")

felipesanches commented 8 years ago

There's realy nothing I can do here unless someone can provide evidence of an actual bug. The error message clearly states that a bad nameID=1/plat=3 entry was found. Checking with TTX I can see the bad entry is indeed there. The supposedly good entry that @alexeiva said he's got from TTX is plat=1 instead, so perhaps the bad entry (plat=3) is also there further down the TTX file.

I'm pretty sure this is an invalid issue so I'll close it. Please only reopen this if strong evidence of an actual fontbakery bug can be provided.

alexeiva commented 8 years ago

For the record, this error is related to a Glyphs update. /cc @schriftgestalt

Previous versions:

<namerecord nameID="1" platformID="3" platEncID="1" langID="0x409">
      Arsenal
    </namerecord>

Glyphs Version 2.4b (926):

<namerecord nameID="1" platformID="3" platEncID="1" langID="0x409">
      Arsenal Bold
    </namerecord>
davelab6 commented 8 years ago

For the record, this error is related to a Glyphs update.

YIKES! :(

schriftgestalt commented 8 years ago

This is not a bug in Glyphs but a misconfiguration. The Bold Italic instance has a Bold in the link to field. This field should be empty.

alexeiva commented 8 years ago

Okay. This was an oversight on my part. My apologies.

Was: screen shot 2016-10-12 at 14 28 02

Result:

<namerecord nameID="1" platformID="3" platEncID="1" langID="0x409">
      Arsenal Bold
    </namerecord>

Fixed:

screen shot 2016-10-12 at 14 31 28

Result:

<namerecord nameID="1" platformID="3" platEncID="1" langID="0x409">
      Arsenal
    </namerecord>
davelab6 commented 8 years ago

@schriftgestalt 👑

schriftgestalt commented 8 years ago

Please leave the field empty. Regular is inferred.

alexeiva commented 8 years ago

It's time to coin the term #fontbakeryhack : doing something random for the sake of passing fontbakery

felipesanches commented 8 years ago

There's no value in "passing fontbakery" other than conforming to the quality standards criteria that FB is built on top of. :-)

Also, it is not really ideal to do "random" tweaks if one does not understand the rationale behind the needed adjustments in the tools.

I understand that this specific issue has lead us to a better understanding of how Glyphs deals with these specific name table entries.

alexeiva commented 8 years ago

I just inspected the problematic Arsenal sources and regenerated them with Marc's fix gf specs script — it does a fine job, and passes fb.

Original error was related to a fault in my Italic naming. I had 'instance of Bold' instead if (blank). That is why the Roman file looked fine, and we couldn't figure out the problem.

Thanks everyone for a fun, and educational discussion. Now, back to real work.

Result of Marc's script. The proper way to do it. screen shot 2016-10-12 at 15 49 24

Glyphs handbook: screen shot 2016-10-12 at 15 49 44

alexeiva commented 8 years ago

So now for https://github.com/alexeiva/Merriweather I did the style linking as suggested by Dave in https://github.com/davelab6/glyphs-export.

It's a four weight family, and I get this warning for Merriweather-Black.ttf:

ERROR    fontforge did print these messages to stderr:
Warning: Mac string is a subset of the Windows string in the 'name' table
 for the Family string in the English (US) language.
Warning: Mac and Windows entries in the 'name' table differ for the
 SubFamily string in the language English (US)
 Mac String: Black
Windows String: Regular

Am I doing something wrong, or should this be just ignored?

davelab6 commented 7 years ago

@m4rc1e and @felipesanches what is the current status of this

davelab6 commented 7 years ago

@m4rc1e and @felipesanches what is the current status of this

felipesanches commented 7 years ago

@davelab6 I have to think about it. I'll check it out and provide a status update on this in a while.

davelab6 commented 7 years ago

In this case I will deprioritize this

felipesanches commented 7 years ago

We inherit a few check routines from FontForge. I'll take a look at these two specific ones pointed out by @alexeiva to make sure what FontForge is validating makes sense in this case. But giving Dave's deprioritization of this, I'll only review this later. (Moving on now to the top-priority milestone 0.2.0 issues :-D)

alexeiva commented 7 years ago

Makes sense. Not all fonts need to inherit legacy metrics. It is always a case-to-case basis. For example adding tall Vietnamese glyphs may require vertical readjustments. Or original metrics may have been set unthoughtful or incorrect. The glyphs script is a very easy fix, and gfregression tool is good for diagnosing.

Adding a fontbakery warning that v metrics have diverged from server api would be a useful option. I would then go to gfregression for examination.

On Nov 21, 2016 2:01 PM, "Felipe Corrêa da Silva Sanches" < notifications@github.com> wrote:

We inherit a few check routines from FontForge. I'll take a look at these two specific ones pointed out by @alexeiva https://github.com/alexeiva to make sure what FontForge is validating makes sense in this case. But giving Dave's deprioritization of this, I'll only review this later. (Moving on now to the top-priority milestone 0.2.0 issues :-D)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/googlefonts/fontbakery/issues/1096#issuecomment-261931148, or mute the thread https://github.com/notifications/unsubscribe-auth/AP3Q6dcJRimNRkPKn45ZR-shFHNNlUPVks5rAZY8gaJpZM4KN5LG .

davelab6 commented 7 years ago

I am pretty sure this can be closed; Glyphsapp was generating name tables we didn't like, and it was updated to generate name tables we do like.