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

Vertical alignment is inconsistent between variable and non-variable files #377

Closed
tay1orjones opened this issue Jun 17, 2021 · 13 comments · Fixed by #519
Closed

Vertical alignment is inconsistent between variable and non-variable files #377

tay1orjones opened this issue Jun 17, 2021 · 13 comments · Fixed by #519
Assignees
Labels
future severity: 4 Affects minor functionality, no workaround needed

Comments

@tay1orjones
Copy link
Member

There's been a longstanding issue open regarding the Carbon website and the vertical alignment of text with certain components, carbon-design-system/carbon#3922

After some further debugging, it appears the cause is the usage of the variable font. When I flip between the variable and the non-variable font family, a noticable shift happens. So far I've seen this in iOS Safari, Chrome, and Firefox. In the videos below I override the font family to be non-vf, wait for the build to finish which automatically updates the browser after a few seconds - you can see the text shift at the end of each video.

small.shift.in.chrome.mov
firefox.shift.mov
shifting.mov
@mjabbink
Copy link
Contributor

@BoldMonday Will this be resolved in the next release?

@BoldMonday
Copy link
Collaborator

This is hard to solve currently because variable and static fonts are really different beasts. The static Plex fonts have been tailored to work on a large number of applications and platforms including ones that do not (and probably will not) support variable fonts. If we change the vertical alignment settings in the static fonts we will upset a large number of users. To change the vertical alignment settings in the variable fonts is a hack that will create problems in the future I'm afraid.

In the long run we can perhaps decide for the static fonts to drop support for legacy platforms and applications. But now is not that time I think.

@vpicone
Copy link
Contributor

vpicone commented Jul 2, 2021

@BoldMonday I believe this might be a problem with the variable font itself. The Google fonts quality assurance tool font bakery uncovered four errors, the second one in particular looks like it's related.


description: Checking OS/2 usWinAscent & usWinDescent.
result: FAIL
rationale: 
        A font's winAscent and winDescent values should be greater than the head table's yMax, abs(yMin) values. If they are less than these values, clipping can occur on Windows platforms (https://github.com/RedHatBrand/Overpass/issues/33).

        If the font includes tall/deep writing systems such as Arabic or Devanagari, the winAscent and winDescent can be greater than the yMax and abs(yMin) to accommodate vowel marks.

        When the win Metrics are significantly greater than the upm, the linespacing can appear too loose. To counteract this, enabling the OS/2 fsSelection bit 7 (Use_Typo_Metrics), will force Windows to use the OS/2 typo values instead. This means the font developer can control the linespacing with the typo values, whilst avoiding clipping by setting the win values to values greater than the yMax and abs(yMin).
    
logs: [
  {
    "message": "OS/2.usWinAscent value should be equal or greater than 1119, but got 1025 instead [code: ascent]",
    "status": "FAIL",
    "traceback": null
  }
]
---

description: Checking OS/2 Metrics match hhea Metrics.
result: FAIL
rationale: 
        OS/2 and hhea vertical metric values should match. This will produce the same linespacing on Mac, GNU+Linux and Windows.

        - Mac OS X uses the hhea values.
        - Windows uses OS/2 or Win, depending on the OS or fsSelection bit value.

        When OS/2 and hhea vertical metrics match, the same linespacing results on macOS, GNU+Linux and Windows. Unfortunately as of 2018, Google Fonts has released many fonts with vertical metrics that don't match in this way. When we fix this issue in these existing families, we will create a visible change in line/paragraph layout for either Windows or macOS users, which will upset some of them.

        But we have a duty to fix broken stuff, and inconsistent paragraph layout is unacceptably broken when it is possible to avoid it.

        If users complain and prefer the old broken version, they have the freedom to take care of their own situation.
    
logs: [
  {
    "message": "OS/2 sTypoAscender (780) and hhea ascent (1025) must be equal. [code: ascender]",
    "status": "FAIL",
    "traceback": null
  }
]
---

description: Are there unwanted tables?
result: FAIL
rationale: 
        Some font editors store source data in their own SFNT tables, and these can sometimes sneak into final release files, which should only have OpenType spec tables.
    
logs: [
  {
    "message": "The following unwanted font tables were found:\nTable: MVAR\nReason: Produces a bug in DirectWrite which causes https://bugzilla.mozilla.org/show_bug.cgi?id=1492477, https://github.com/google/fonts/issues/2085\n\nThey can be removed with the gftools fix-unwanted-tables script. [code: unwanted-tables]",
    "status": "FAIL",
    "traceback": null
  }
]
---

description: Does the font have a DSIG table?
result: FAIL
rationale: 
        Microsoft Office 2013 and below products expect fonts to have a digital signature declared in a DSIG table in order to implement OpenType features. The EOL date for Microsoft Office 2013 products is 4/11/2023. This issue does not impact Microsoft Office 2016 and above products.

        This checks verifies that this signature is available in the font.

        A fake signature is enough to address this issue. If needed, a dummy table can be added to the font with the `gftools fix-dsig` script available at https://github.com/googlefonts/gftools

        Reference: https://github.com/googlefonts/fontbakery/issues/1845
    
logs: [
  {
    "message": "This font lacks a digital signature (DSIG table). Some applications may require one (even if only a dummy placeholder) in order to work properly. You can add a DSIG table by running the `gftools fix-dsig` script. [code: lacks-signature]",
    "status": "FAIL",
    "traceback": null
  }
]
---

@vpicone
Copy link
Contributor

vpicone commented Jul 2, 2021

Screen Shot 2021-07-02 at 11 03 55 AM

https://ttqq5.csb.app/

At the moment the VF behaves so differently from the base font, it can't be used with the same styles in our products/component library.

@mjabbink
Copy link
Contributor

@BoldMonday Any solution for this in a future release?

@BoldMonday
Copy link
Collaborator

@mjabbink We can have a look how to solve this in Q3. But as I wrote above this is hard to solve and we cannot guarantee a short-time fix.

@mjabbink mjabbink added severity: 4 Affects minor functionality, no workaround needed future labels May 21, 2022
@mjabbink
Copy link
Contributor

@BoldMonday We need to get this on the schedule as it blocking us from taking advantage of the variable fonts in our digital experiences. Let’s discuss on next Plex Math call.

@ddegan
Copy link

ddegan commented May 3, 2023

Hi all! Are there any news regarding this issue?

@mjabbink
Copy link
Contributor

mjabbink commented May 3, 2023

@ddegan I believe this issue has been resolved in the latest versions that have been released. @BoldMonday can confirm any details.

@BoldMonday
Copy link
Collaborator

@mjabbink We delivered an update that should fix the issue but this repository still contains the previous version. Can someone push an update?

@ddegan
Copy link

ddegan commented May 4, 2023

Yeah I confirm that the issue seems to be there also for version 6.2.0.

@tay1orjones
Copy link
Member Author

@BoldMonday Sorry for the delay, we've got it planned to release next week

@tay1orjones
Copy link
Member Author

Hey there! v6.3.0 was just released that references this issue.

Comment here or join the release discussion to provide feedback or voice concerns. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future severity: 4 Affects minor functionality, no workaround needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants