googlefonts / roboto-flex

SIL Open Font License 1.1
467 stars 32 forks source link

xHeight metric not scaling with `YTLC` #378

Closed drott closed 1 year ago

drott commented 1 year ago

The xHeight metric of Roboto flex (tested with https://github.com/googlefonts/roboto-flex/releases/tag/3.100) does not scale with the YTLC axis.

Spun off from https://bugs.chromium.org/p/chromium/issues/detail?id=1458629

No MVAR table is found, so the metric can't be varied.

Dumping "roboto-flex-fonts/fonts/variable/RobotoFlex[GRAD,XOPQ,XTRA,YOPQ,YTAS,YTDE,YTFI,YTLC,YTUC,opsz,slnt,wdth,wght].ttf" to "-"...

<?xml version="1.0" encoding="UTF-8"?>
<ttFont sfntVersion="\x00\x01\x00\x00" ttLibVersion="4.37">

Dumping 'OS/2' table...
  <OS_2>
[...]
    <sxHeight value="1052"/>
[...]
  </OS_2>

No 'MVAR' table found.
</ttFont>
drott commented 1 year ago

CC @argyleink

m4rc1e commented 1 year ago

Thanks for the report.

I've just updated the dependencies so the MVAR table gets built but it still isn't working. I suspect the values in the font sources need updating. Lemme check that.

cc @sannorozco

m4rc1e commented 1 year ago

Looks like the values do differ for the sources.

Marcs-Air-2:roboto-flex marcfoley$ grep -rni --color "xHeight" -C 1 sources/1A-drawings/Parametric\ Axes/*YTLC*
sources/1A-drawings/Parametric Axes/RobotoFlex_YTLC416.ufo/fontinfo.plist:257:    <key>xHeight</key>
sources/1A-drawings/Parametric Axes/RobotoFlex_YTLC416.ufo/fontinfo.plist-258-    <real>852.0</real>
--
sources/1A-drawings/Parametric Axes/RobotoFlex_YTLC570.ufo/fontinfo.plist:247:    <key>xHeight</key>
sources/1A-drawings/Parametric Axes/RobotoFlex_YTLC570.ufo/fontinfo.plist-248-    <real>1168.0</real>

I'll see if they're being included in the MVAR table.

m4rc1e commented 1 year ago

mvar-test.zip

Attached is the test font with MVAR and ttx dump.

m4rc1e commented 1 year ago

Even with the new MVAR, I can't see a difference if I take @argyleink's test page and change the YTLC val and overlay with the original.

Screenshot 2023-06-28 at 12 14 24
m4rc1e commented 1 year ago

Just checked in Samsa and the test font appears to be working.

mvar

Values being produced from the slider for x-height look good to me and match the sources.

m4rc1e commented 1 year ago

@argyleink could you try and use the fonts in https://github.com/googlefonts/roboto-flex/issues/378#issuecomment-1611212158 please? if it works, I'll cut a new release.

drott commented 1 year ago

Thanks for the quick action on this @m4rc1e.

davelab6 commented 1 year ago

For context, we didn't include mvar tables as they actually caused problems for windows 10 users, but I believe that's been fixed for long enough that it's worth Marc doing a review and identifying all gf VFs that vary any Y proportions and rebuilding with correct mvar data.

@vv-monsalve is interested in this for Playpen

davelab6 commented 1 year ago

In the original crbug, @argyleink proposed

ensure any variable fonts that have an xheight adjustment setting are observed for client modifications and perform recalculations on the ex length if needed. aka: the current algorithm appears to measure the xheight for the ex unit length based on an unadjusted font metric

I wonder if it's still worth doing this, given mvar data is unreliable, or especially, missing altogether, @drott ?

At least I guess the devtool console could better explain why this CSS won't work because the font is missing an mvar table, even if chrome won't recalculate on the fly :)

drott commented 1 year ago

I wonder if it's still worth doing this, given mvar data is unreliable, or especially, missing altogether, @drott ?

I don't understand this question. Why would it not be worth fixing the font? I am quite sure we do get from the font backends and use xHeight with variations applied if they were in the font.

At least I guess the devtool console could better explain why this CSS won't work because the font is missing an mvar table, even if chrome won't recalculate on the fly :)

I don't think that's the level of detail that the devtool team will be exposing, given that there's a bunch of lower hanging font tooling fruit that have not been harvested.

argyleink commented 1 year ago

I downloaded the font from the release, uploaded it to Codepen, forked the original demo, used the new font, and still see the same bug:

https://codepen.io/argyleink/pen/ZEmerQB/47e4fffa1574b8a8acb4381445428053

Screenshot 2023-06-28 at 9 59 39 AM

Did I do something wrong, or this was the test y'all wanted me to check? Thanks for checkin this out!

drott commented 1 year ago

I have now been trying to debug that for a while based on your content @argyleink , but I find it hard to minimize the content. I assume what you're trying to achieve is to place the red line at the x-height of Roboto Flex? Could you provide a more reduced example without animations, and without external imports other than the font? I do see the red line moving relative to the top horizontal part of the capital G, but interestingly, with both fonts, the original and the updated one.

drott commented 1 year ago

PS: On Linux, (and in FF on Mac) the red line does not move with the new font and the link above. We do have Apple specific code that measures the x glyph if OS/2 x-height is zero.

argyleink commented 1 year ago

I have now been trying to debug that for a while based on your content @argyleink , but I find it hard to minimize the content. I assume what you're trying to achieve is to place the red line at the x-height of Roboto Flex? Could you provide a more reduced example without animations, and without external imports other than the font? I do see the red line moving relative to the top horizontal part of the capital G, but interestingly, with both fonts, the original and the updated one.

here's a more minimal version https://codepen.io/argyleink/pen/bGQWbRb plus a slider that changes the YTLC. You can watch as YTLC changes the font but the value of 1ex (the red bar) doesn't change with it.

Lorp commented 1 year ago

Looks perfect to me in Chrome and Safari. Firefox shows no red element.

https://github.com/googlefonts/roboto-flex/assets/587440/372b9cca-a07f-4a5f-b42e-aee35eeb24ef

argyleink commented 1 year ago

Looks perfect to me in Chrome and Safari. Firefox shows no red element.

red-bar-YTLC-Roboto-Flex.mov

oh dang, it does work! I'm on CrOS 114 today and I'm not getting that result, but that's definitely what I'm lookin for! Can try MacOS later today.

drott commented 1 year ago

On Chrome Linux Beta, I don't see the red box scaling, reopening https://crbug.com/1458629 and will investigate there.

argyleink commented 1 year ago

confirmed working as expected in MacOS Chrome 114 👍🏻

https://github.com/googlefonts/roboto-flex/assets/1134620/eb130294-cad3-4719-915b-a82d6d487099

drott commented 1 year ago

here's a more minimal version https://codepen.io/argyleink/pen/bGQWbRb plus a slider that changes the YTLC. You can watch as YTLC changes the font but the value of 1ex (the red bar) doesn't change with it.

As @bungeman pointed out in comment 6 on the Chromium bug the test content in https://github.com/googlefonts/roboto-flex/issues/378#issuecomment-1613357136 did not include the font that has the mvar table.

When uploading the font that @m4rc1e made and modifying the test content to use that font, as here: https://codepen.io/drott_chrome/pen/abQWJWO Then this works as expected on Linux as well.

image image

@bungeman also confirmed that Skia then reports correct mvar-adjusted x-height on Windows, through DirectWrite, too.

@m4rc1e from my point of view, this looks fixed in the font and cutting a new release would be helpful.

m4rc1e commented 1 year ago

Thanks @drott. I'll push a new release.

Lorp commented 1 year ago

@davelab6:

mvar data is unreliable

What do you mean by this? Unreliable data in fonts, or data being unreliably processed by browser clients?