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

Added Commissioner 1.000 #2561

Closed
wants to merge 2 commits into from
Closed

Added Commissioner 1.000 #2561

wants to merge 2 commits into from

Conversation

yanone
Copy link
Collaborator

@yanone yanone commented Jul 21, 2020

@m4rc1e
Copy link
Collaborator

m4rc1e commented Jul 21, 2020

Fontbakery report

Fontbakery version: 0.7.29

[1] Family checks
WARN: Is the command `ftxvalidator` (Apple Font Tool Suite) available?
--- Rationale ---

There's no reasonable (and legal) way to run the command `ftxvalidator` of the
Apple Font Tool Suite on a non-macOS machine. I.e. on GNU+Linux or Windows etc.

If Font Bakery is not running on an OSX machine, the machine running Font
Bakery could access `ftxvalidator` on OSX, e.g. via ssh or a remote procedure
call (rpc).

There's an ssh example implementation at:
https://github.com/googlefonts/fontbakery/blob/master/prebuilt/workarounds
/ftxvalidator/ssh-implementation/ftxvalidator


  • WARN Could not find ftxvalidator.

[8] Commissioner[FLAR,VOLM,slnt,wght].ttf
💔 ERROR: Check correctness of STAT table strings
--- Rationale ---

On the STAT table, the "Italic" keyword must not be used on AxisValues for
variation axes other than 'ital'.


  • 💔 ERROR Failed with AttributeError: AxisIndex
🔥 FAIL: Ensure VFs do not contain slnt or ital axes.
--- Rationale ---

The 'ital' axis is not supported yet in Google Chrome. The 'opsz' axis also has
patchy support.

For the time being, we need to ensure that VFs do not contain either of these
axes. Once browser support is better, we can deprecate this check.

For more info regarding ital and opsz browser support, see:
https://arrowtype.github.io/vf-slnt-test/


  • 🔥 FAIL The "slnt" axis is not yet well supported on Google Chrome. [code: unsupported-slnt]
WARN: DESCRIPTION.en_us.html should end in a linebreak.
--- Rationale ---

Some older text-handling tools sometimes misbehave if the last line of data in
a text file is not terminated with a newline character (also known as '\n').

We know that this is a very small detail, but for the sake of keeping all
DESCRIPTION.en_us.html files uniformly formatted throughout the GFonts
collection, we chose to adopt the practice of placing this final linebreak char
on them.


  • WARN The last characther on DESCRIPTION.en_us.html is not a line-break. Please add it. [code: missing-eof-linebreak]
WARN: Check copyright namerecords match license file.
--- Rationale ---

A known licensing description must be provided in the NameID 14 (LICENSE
DESCRIPTION) entries of the name table.

The source of truth for this check (to determine which license is in use) is a
file placed side-by-side to your font project including the licensing terms.

Depending on the chosen license, one of the following string snippets is
expected to be found on the NameID 13 (LICENSE DESCRIPTION) entries of the name
table:
- "This Font Software is licensed under the SIL Open Font License, Version 1.1.
This license is available with a FAQ at: https://scripts.sil.org/OFL"
- "Licensed under the Apache License, Version 2.0"
- "Licensed under the Ubuntu Font Licence 1.0."


Currently accepted licenses are Apache or Open Font License.
For a small set of legacy families the Ubuntu Font License may be acceptable as
well.

When in doubt, please choose OFL for new font projects.


  • WARN Please consider using HTTPS URLs at name table entry [plat=3, enc=1, name=13] [code: http-in-description]
  • WARN For now we're still accepting http URLs, but you should consider using https instead.
    [code: http]
WARN: License URL matches License text on name table?
--- Rationale ---

A known license URL must be provided in the NameID 14 (LICENSE INFO URL) entry
of the name table.

The source of truth for this check is the licensing text found on the NameID 13
entry (LICENSE DESCRIPTION).

The string snippets used for detecting licensing terms are:
- "This Font Software is licensed under the SIL Open Font License, Version 1.1.
This license is available with a FAQ at: https://scripts.sil.org/OFL"
- "Licensed under the Apache License, Version 2.0"
- "Licensed under the Ubuntu Font Licence 1.0."


Currently accepted licenses are Apache or Open Font License.
For a small set of legacy families the Ubuntu Font License may be acceptable as
well.

When in doubt, please choose OFL for new font projects.


  • WARN Please consider using HTTPS URLs at name table entry [plat=3, enc=1, name=13] [code: http-in-description]
  • WARN Please consider using HTTPS URLs at name table entry [plat=3, enc=1, name=13] [code: http-in-description]
  • WARN Please consider using HTTPS URLs at name table entry [plat=3, enc=1, name=13] [code: http-in-description]
  • WARN Please consider using HTTPS URLs at name table entry [plat=3, enc=1, name=14] [code: http-in-license-info]
  • WARN For now we're still accepting http URLs, but you should consider using https instead.
    [code: http]
WARN: METADATA.pb: Fontfamily is listed on Google Fonts API?
WARN: Are there caret positions declared for every ligature?
--- Rationale ---

All ligatures in a font must have corresponding caret (text cursor) positions
defined in the GDEF table, otherwhise, users may experience issues with caret
rendering.

If using GlyphsApp, ligature carets can be set directly on canvas by accessing
the `Glyph -> Set Anchors` menu option or by pressing the `Cmd+U` keyboard
shortcut.


  • WARN This font lacks caret position values for ligature glyphs on its GDEF table. [code: lacks-caret-pos]
WARN: Is there kerning info for non-ligated sequences?
--- Rationale ---

Fonts with ligatures should have kerning on the corresponding non-ligated
sequences for text where ligatures aren't used (eg
https://github.com/impallari/Raleway/issues/14).


  • WARN GPOS table lacks kerning info for the following non-ligated sequences:

    • f + f
    • f + i
    • i + f
    • f + l
    • l + f
    • i + l

    [code: lacks-kern-info]


Summary

💔 ERROR 🔥 FAIL ⚠ WARN 💤 SKIP ℹ INFO 🍞 PASS 🔎 DEBUG
1 1 7 35 9 124 0
1% 1% 4% 20% 5% 70% 0%

Note: The following loglevels were omitted in this report:

  • SKIP
  • INFO
  • PASS
  • DEBUG

Diff images: commisioner_qa.zip

@yanone
Copy link
Collaborator Author

yanone commented Jul 21, 2020

I tried to dig into the ERROR, but no success. A non-existing AxisValue attribute is being reported, when it really seems to exist. I aborted that attempt.

@graphicore
Copy link
Collaborator

I tried to dig into the ERROR, but no success. A non-existing AxisValue attribute is being reported, when it really seems to exist. I aborted that attempt.

If it's an ERROR it must be fixed in Font Bakery and the problem is a not existing attribute:

Failed with AttributeError: AxisIndex

I suggest to file an issue at https://github.com/googlefonts/fontbakery/issues

@yanone
Copy link
Collaborator Author

yanone commented Jul 21, 2020

Okay, will do it

@m4rc1e
Copy link
Collaborator

m4rc1e commented Jul 22, 2020

The i's sidebearings seem a bit off in the darker weights.

Screenshot 2020-07-22 at 10 32 54

@yanone
Copy link
Collaborator Author

yanone commented Jul 22, 2020

I can't confirm that from looking at the sources. I shall try without hinting. How did you get to this preview image?

@RosaWagner RosaWagner self-assigned this Aug 26, 2020
@RosaWagner
Copy link
Contributor

@m4rc1e I run a browser previews on the statics and I couldn't replicate this spacing issue. If it concerns de variable, it should be already unhinted. In any case, such weight shouldn't be use under 30pt… what do you think ?

@RosaWagner RosaWagner removed their assignment Aug 28, 2020
@davelab6
Copy link
Member

davelab6 commented Sep 3, 2020

such weight shouldn't be use under 30pt… what do you think ?

I think users will still use it at 18 and notice XD I'd like to get to the bottom of why this happens as Yanone saiid he couldn't see it in the source files...

@kosbarts any suggestions? :)

@davelab6
Copy link
Member

davelab6 commented Sep 3, 2020

@m4rc1e I need to get the axis registry updated with the custom axes, too. Marc are you able to make a separate PR for that?

@kosbarts
Copy link

kosbarts commented Sep 3, 2020 via email

@RosaWagner
Copy link
Contributor

So I re-run a browser previews on the variable. The bug appears in Mac OS High Sierra Safari preview:

Capture d’écran 2020-09-04 à 11 19 11

But not in Windows 10 Chrome:

Capture d’écran 2020-09-04 à 11 18 43

When I play in Safari (Mas OS Mojave), the spacing do have a weird behaviour, which in not the case in Chrome:
video-commissioner.zip

@RosaWagner RosaWagner added -- Needs Upstream Resolution Upstream fix required before moving forward and removed Ready to push labels Sep 4, 2020
@RosaWagner RosaWagner self-assigned this Sep 4, 2020
@RosaWagner RosaWagner mentioned this pull request Sep 4, 2020
4 tasks
@RosaWagner
Copy link
Contributor

So, this spacing issue is apparently a bug of Safari with all composites. It is fixed in Catalina apparently. We could satisfy the diffbrowser preview by decomposing the /i. Although it bothers a lot the designer because it would require a special script in his production process. And what about the other composites? He would prefer to let it like that, or to decompose everything (but 16 masters in multiscripts would make the the file heavier).

@davelab6
Copy link
Member

davelab6 commented Sep 9, 2020

I'm OK to proceed

@RosaWagner
Copy link
Contributor

Made a new PR #2675 with regenerated fonts and added informations.

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.

6 participants