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

hhea.advanceWidthMax might be a bad value to rely on for monospace_max_advancewidth check #2749

Closed
arrowtype opened this issue Feb 4, 2020 · 7 comments
Assignees
Milestone

Comments

@arrowtype
Copy link
Contributor

Observed behaviour

In Recursive Mono, I get notified that This seems to be a monospaced font, so advanceWidth value should be the same across all glyphs, but 99.6% of them have a different value.

 >> com.google.fonts/check/monospace_max_advancewidth
    Monospace font has hhea.advanceWidthMax equal to each glyph's advanceWidth?
    with fonts_1.036/Static_OTF/RecursiveMonoLnr-ExtraBold.otf

    * WARN: This seems to be a monospaced font, so advanceWidth value should be the same across all glyphs, but 99.6% of them have a different value: space, uni00A0, uni2007, uni2008, uni2
009, uni200A, A, Agrave, Aacute, Acircumflex and 1232 more. [code: should-be-monospaced]
    * WARN: Double-width and/or zero-width glyphs were detected. These glyphs should be set to the same width as all others and then add GPOS single pos lookups that zeros/doubles the widt
hs as needed: uni02BC, uni0300, uni0301, uni0302, uni0303, uni0304, uni0306, uni0307, uni0308, uni0309 and 60 more. [code: variable-monospaced]

    Result: WARN

However, this assumes that the font's maximum width value is its primary width value, which is not the case here. Rather, the normal value is 600 units, and the maximum of 2400 comes from only a few code ligatures.

1099 of 1247 glyphs have a width of 600, meaning that only 11.87% of them have a different value. Moreover, a check shows that all glyph widths are divisible by the normal width of 600 with no remainder, so the font should theoretically have no issues of non-monospaced glyphs.

Of course, some terminals (e.g. the VS Code terminal) will not work with glyphs over the 600-unit width, but those also don't support dlig, so the only problems I've run into in the VS Code terminal are:

  1. The f.italic gets cut off because it goes beyond its sidebearings
  2. I occasionally get a warning about it being a non-monospaced font (but it still works, anyway)

Expected behaviour

Instead of basing the "correct" monospace value on hhea.advanceWidthMax, I think it might make sense to base it on something like the space or a glyph.

If there is reason to base it on the hhea.advanceWidthMax, we should at least update the check rationale to say why we are flagging it as a WARN, and perhaps calculate the "different value" metric.

Resources and exact process needed to replicate

Here's a recent version of Recursive which can be used for this check:

https://github.com/arrowtype/recursive/blob/1a1680256d89d6856f9e4eef06ceae247a804537/fonts_1.036/Static_TTF/RecursiveMonoLnr-ExtraBold.ttf

@arrowtype
Copy link
Contributor Author

I am seeing that the com.google.fonts/check/monospace does provide more nuance:

 >> com.google.fonts/check/monospace
    Checking correctness of monospaced metadata.
    with fonts_1.036/Static_TTF/RecursiveMonoLnr-Regular.ttf

      Rationale:                                                                
      There are various metadata in the OpenType spec to specify if a font is   
      monospaced or not. If the font is not trully monospaced, then no          
      monospaced metadata should be set (as sometimes they mistakenly are...)   
                                                                                
      Monospace fonts must:                                                     
                                                                                
      * post.isFixedWidth "Set to 0 if the font is proportionally spaced,       
      non-zero if the font is not proportionally spaced (monospaced)"           
        www.microsoft.com/typography/otspec/post.htm                            
                                                                                
      * hhea.advanceWidthMax must be correct, meaning no glyph's width value is 
      greater.                                                                  
        www.microsoft.com/typography/otspec/hhea.htm                            
                                                                                
      * OS/2.panose.bProportion must be set to 9 (monospace). Spec says: "The   
      PANOSE definition contains ten digits each of which currently describes   
      up to sixteen variations. Windows uses bFamilyType, bSerifStyle and       
      bProportion in the font mapper to determine family type. It also uses     
      bProportion to determine if the font is monospaced."                      
        www.microsoft.com/typography/otspec/os2.htm#pan                         
        monotypecom-test.monotype.de/services/pan2                              
                                                                                
      * OS/2.xAverageWidth must be set accurately.                              
        "OS/2.xAverageWidth IS used when rendering monospaced fonts, at least   
      by Windows GDI"                                                           
        http://typedrawers.com/discussion/comment/15397/#Comment_15397          
                                                                                
      Also we should report an error for glyphs not of average width            

    * WARN: Font is monospaced but 148 glyphs (11.858974358974358%) have a different width. You should check the widths of: ['uni02BC', 'uni0300', 'uni0301', 'uni0302', 'uni0303', 'uni0304', 'uni0306', 'uni0307', 'uni0308', 'uni0309', 'uni030A', 'uni030B', 'uni030C', 'uni030F', 'uni0311', 'uni0312', 'uni0315', 'uni031B', 'uni0323', 'uni0324', 'uni0326', 'uni0327', 'uni0328', 'uni032E', 'uni0331', 'uni0335', 'uni200B', 'caronslovakcomb', 'dotsidecomb', 'horncombo', 'ogonekcombo', 'ringacute', 'uni0337', 'f_f', 'uniFB01', 'uniFB02', 'f_f_i', 'f_f_l', 'uni0302_uni0301', 'uni0302_uni0300', 'uni0302_hookabovecomb', 'uni0302_uni0303', 'uni0306_uni0301', 'uni0306_uni0300', 'uni0306_hookabovecomb', 'uni0306_uni0303', 'uni030C.alt', 'uni0300.case', 'uni0301.case', 'uni0302.case', 'uni0303.case', 'uni0304.case', 'uni0306.case', 'uni0307.case', 'uni0308.case', 'uni0309.case', 'uni030A.case', 'uni030B.case', 'uni030C.case', 'uni030F.case', 'uni0311.case', 'uni0312.case', 'uni0315.case', 'uni031B.case', 'uni0323.case', 'uni0326.case', 'uni0327.case', 'uni0328.case', 'acutecombviet.case', 'gravecombviet.case', 'horncombo.case', 'ogonekcombo.case', 'ringacute.case', 'uni0337.case', 'tildecombviet.case', 'uni01C6.italic', 'uni01C9.italic', 'uni01CC.italic', 'uni01C8.italic', 'uni01CB.italic', 'f_f.italic', 'f_f_i.italic', 'f_f_l.italic', 'uniFB01.italic', 'uniFB02.italic', 'f_f.mono', 'f_f_i.mono', 'f_f_l.mono', 'uniFB01.mono', 'uniFB02.mono', 'quotesingle_quotesingle_quotesingle.code', 'quotedbl_quotedbl_quotedbl.code', 'asterisk_asterisk.code', 'asterisk_asterisk_asterisk.code', 'asterisk_equal.code', 'asterisk_slash.code', 'plus_equal.code', 'plus_plus.code', 'plus_plus_plus.code', 'minus_equal.code', 'equal_equal.code', 'equal_equal_equal.code', 'equal_greater.code', 'equal_slash_equal.code', 'less_equal.code', 'less_exclam_hyphen_hyphen.code', 'less_hyphen.code', 'less_less.code', 'less_less_less.code', 'greater_equal.code', 'greater_greater.code', 'greater_greater_greater.code', 'greater_hyphen.code', 'f_quotesingle.code', 'underscore_underscore.code', 'hyphen_greater.code', 'hyphen_hyphen.code', 'hyphen_hyphen_greater.code', 'hyphen_hyphen_hyphen.code', 'hyphen_less.code', 'hyphen_space_bracketleft_space_bracketright.code', 'hyphen_space_bracketleft_x_bracketright.code', 'numbersign_numbersign.code', 'numbersign_numbersign_numbersign.code', 'numbersign_numbersign_numbersign_numbersign.code', 'ampersand_ampersand.code', 'ampersand_ampersand_ampersand.code', 'slash_asterisk.code', 'slash_equal.code', 'slash_slash.code', 'slash_slash_slash.code', 'backslash_b.code', 'backslash_n.code', 'backslash_r.code', 'backslash_t.code', 'backslash_v.code', 'bar_bar.code', 'bar_bar_bar.code', 'colon_colon.code', 'colon_slash_slash.code', 'exclam_equal.code', 'exclam_equal_equal.code', 'exclam_exclam.code', 'question_colon.code', 'question_period.code', 'question_question.code', 'dollar_braceleft.code', 'grave_grave_grave.code'] [code: mono-outliers]

@felipesanches felipesanches self-assigned this Feb 5, 2020
@felipesanches felipesanches added this to the 0.7.upcoming milestone Feb 5, 2020
@felipesanches felipesanches modified the milestones: 0.7.upcoming, 0.7.19 Feb 5, 2020
@felipesanches
Copy link
Collaborator

According to the specs, hhea.advanceWidthMax is a calculated value:

Captura de Tela 2020-02-07 às 02 47 49

I think that the problem with com.google.fonts/check/monospace_max_advancewidth is the false assumption that all glyphs in a monospaced font would have the same advanceWidth as the maximum value.

If the purpose of the check was to ensure that hhea.advanceWidthMax is properly calculated, then the wording of the log messages convey the wrong idea by suggesting the problem is with the advanceWidths of a bunch of glyphs.

Also, we already have a check for ensuring hhea.advanceWidthMax is properly calculated: That's com.google.fonts/check/maxadvancewidth and it looks fine.

And we also have a check for the advanceWidths of all glyphs that carefully determines which is the largest group of glyphs with the same value and then ensures such group corresponds to more than 80% of the glyphset, which is pretty reasonable to accommodate the eventual outliers. That's com.google.fonts/check/monospace, as @thundernixon mentioned above.


So, my conclusion is that com.google.fonts/check/monospace_max_advancewidth was a mistake and has no actual usefulness, so it will be deprecated.

If anyone knows of an actual value of keeping it (even if including changes to it's implementation) please let me know and I'll be glad to revert the deprecation, as long as we document the rationale for keeping it.

@arrowtype
Copy link
Contributor Author

So, there are instances in which the max advance width can cause issues if it is too wide, especially in some older code/terminal environments. For example:

googlefonts/Inconsolata#42

I know that Fira Code and Hasklig use a surprising approach to code ligatures, to ensure that everything stays within the same single-letter width.

So, I don’t think it’s necessarily a terrible check, but the rationale definitely could use an upgrade.

@felipesanches
Copy link
Collaborator

thanks, @thundernixon!
If we can craft a short rationale text describing the actual purpose of the check, we can open a new issue to either bring back the check or to implement a new, reasonable one.

@arrowtype
Copy link
Contributor Author

Good plan! I’ll try to help think of something.

Found some detail from the Fira Code author on why the libraries have single width, plus blank spaces: it makes it easier for programs to insert the cursor at the proper place.

I know that even the terminal in VS Code gets messed up from proportional glyphs (somewhat surprising, because the code editor part has no trouble), though I think it probably uses average width rather than max width.

https://www.patreon.com/posts/fira-code-inside-28074541

Input Mono describes some issues with editors and terminals that don’t support non-fixed-width fonts. This is probably slightly out of date, but likely some/many of the issues still exist.

https://input.fontbureau.com/workarounds/

Fonts that aren’t marked as isFixedPitch will not show I up in many Windows software font menus (google/fonts#1824). So, perhaps there is another check for that, but if not, this check could maybe suggest that people set that to 1 if they intend their font to be used for code.

@arrowtype
Copy link
Contributor Author

Maybe the rationale could be something like:

INFO: The hhea.advanceMaxWidth is X, but the average width is Y. This can cause layout and cursor-placement issues in some terminals and code editors. If this font is intended for code, make sure that isFixedPitch is set to 1, and consider whether glyphs can be given a uniform width. For a way to make code ligatures with fixed widths, see https://www.patreon.com/posts/fira-code-inside-28074541.

@arrowtype
Copy link
Contributor Author

(Though, this ignores that I think code ligatures can be better as dlig than as calt. I need to try the Fira code width hack with dlig, to see whether that can work. If so, I could write a document on GitHub about the workflow, and we could link to that.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants