Godot: OpenType font spacing differs after update of FreeType from 2.9.1 to 2.10.0 (Overpass font)

[ForLoveOfCats]

Godot versions:
Godot Mono 3.1.0 Beta 11 and Godot Mono 3.1.1 RC1

OS/device including version:
Manjaro Linux x64 w/Intel Sandybridge Mobile iGPU running Mesa 19.0.2-1

Issue description:
Upon upgrading from Godot Mono 3.1.0 Beta 11 to Godot Mono 3.1.1 RC1 my game's buttons went from this:

to this:

Switching between GLES2 and GLES3 does not effect either version. For the sake of completeness I will mention that these are both Mono builds but that should (hopefully) not be related to this issue.

Here is a reproduction project: ButtonSizeBug.zip
Godot Mono 3.1.0 Beta 11:

Godot Mono 3.1.1 RC1:

[ForLoveOfCats]

I possible I would like to attempt to fix this myself as I am trying to become more familiar with Godot's codebase and contribute more.

Edit: The hardware I am stuck with ATM is not up to compiling Godot quite a few times in order to find the root commit

[hpvb]

Can you please check the difference between 3.1.0-stable and 3.1.1-rc1?

[ForLoveOfCats]

Issue is not present in Godot Mono 3.1.0 Stable

[hpvb]

OK, so this is a difference between 3.1.0-stable and 3.1.1-rc1?

[ForLoveOfCats]

The issue is not present in 3.1.0 Stable but is present in 3.1.1 RC1

[hpvb]

If you want to look at this yourself use git bisect between 3.1.0-stable and the tip of the 3.1 branch. You should find it with about 7 compilations currently. Then we know what backport caused it and we can see if there's a fix for it already or revert it in 3.1 before release of 3.1.1-stable.

[ForLoveOfCats]

Oof I'm stuck on this under powered laptop for the time being... While I would like to try to fix this myself that just isn't practical. Editing my original comment about doing this myself.

[akien-mga]

I don't see any difference with the attached project between 3.1-stable and current 3.1 HEAD (476e179) (non Mono build).

Edit: I do get the issue in 3.1.1-rc1 though. Testing a release_debug build locally to see if it's related.

[akien-mga]

Tested so far, all non-Mono on X11 64-bit:

• 3.1-stable (release_debug, LTO): OK
• 3.1.1-rc1 aka 39f1a11 (release_debug, LTO): bug
• 39f1a11 local build (debug, no LTO): OK
• 4764e17 local build (debug, no LTO): OK
• 4764e17 local build (release_debug, no LTO): OK
• 4764e17 local build (release_debug, LTO): OK

So not sure what's left to test, but something is wrong only in the official 3.1.1-rc1 build.

[hpvb]

This is very puzzling as no changes were made to the build containers at all.

[akien-mga]

This is probably due to the freetype version change.

[hpvb]

I'll test with the freetype version reverted.

[akien-mga]

I can confirm that the update of the builtin freetype version is the problem. I've reverted it in the 3.1 branch, so the bug is fixed there, but we still need to figure out the issue for the master branch (especially as it means that linking against system freetype 2.10.0+ may trigger the issue again for 3.1.x or even 3.0.x).

[volzhs]

| Overpass font | freetype 2.9 | freetype 2.10 |
|---------------|--------------|---------------|
| ascent | 16 | 12 |
| descent | 5 | 4 |

| NotoSansUI font | freetype 2.9 | freetype 2.10 |
|---------------|--------------|---------------|
| ascent | 18 | 18 |
| descent | 5 | 5 |

I tested freetype 2.9 and 2.10
the ascent and descent value are different with specific fonts.
as a result, the overpass font looks stuck to the top.

I looked over various values inside FT_Face and changes log.
but couldn't find a clue to fix it.

@akien-mga would it be better to revert freetype 2.10?

[Zireael07]

IIRC freetype 2.10 has been reverted?

[akien-mga]

would it be better to revert freetype 2.10?

No, we need to find a way to configure things to work the same on 2.9 and 2.10. Distros for example will typically unbundle freetype and build against their system version, and we can't freeze on 2.9 forever.

If we can't find a config option to fix this, we should probably contact upstream FreeType as this would be a compatibility breakage in 2.10.

[volzhs]

dug up for several hours more but still no clue.
no time for investigating this issue.

[akien-mga]

@hpvb OK with defaulting all stable branches to using builtin freetype on Linux for the time being?

I've already done that on master as we now need libpng16 (which Ubuntu < 18.10 and matching Debian still don't have out of the box), and if we use builtin libpng then we also need to make freetype and zlib builtin.

I think we should do the same on stable branches to be on the safe side, and so that users making custom builds don't get hit by this issue. Doesn't change much for distros who will have to opt-in for zlib, libpng and freetype, like they already have to do for all other vendored deps.

[akien-mga]

OK with defaulting all stable branches to using builtin freetype on Linux for the time being?

Done that for all stable branches via cherry-picks of #29998.

[Anutrix]

These might possibly useless/unrelated info.
Maybe these?
https://github.com/golang/freetype/issues/34#issuecomment-308912650

2018-08-08  Alexei Podtelezhnikov  <[email protected]>

[pcf] Massive unsigning (part 1).

Unofficial specifications hesitate to use unsigned 32-bit integers. Negative values caused a lot of trouble in the past and it is safer and easier to treat some properties as unsigned.

 * src/pcf/pcf.h (PCF_AccelRec): Use unsigned values for `fontAscent',
`fontDescent', and `maxOverlap'.
* src/pcf/pcfread.c (pcf_load_font, pcf_get_accel): Updated.
* src/pcf/pcfdrivr.c (PCF_Glyph_Load, PCF_Size_Select,
PCF_Size_Request): Updated.

These?
https://github.com/godotengine/godot/blob/4f5a7ebaecfcf00cf1e5c4af4b20034f0dcecd29/thirdparty/freetype/src/pcf/README#L58

[akien-mga]

Good finds, thanks. I'd suggest to step through Error DynamicFontAtSize::_load() with a debugger and compare values for the different metrics when loading a project in the master branch (using FreeType 2.10.0) and in the 3.1 branch (using FreeType 2.9.1).

[Anutrix]

Info so far:
Branch - master : 3.1
metrics.ascender Ratio - 3 : 4
metrics.descender Ratio - 4 : 5

Whenever this if block executes, ascender and decender values(not sure about other values) set in 3.1 and master:
https://github.com/godotengine/godot/blob/2293c612e6c0913b6999080c9f2fa77227348818/scene/resources/dynamic_font.cpp#L163

Update: These ratios were co-incidence.

[Anutrix]

Finally found the different values.
The face->ascender and face->descender in 2.9.1 is HHead values.
The face->ascender and face->descender in 2.10.0 is Typo values.
The following image should make it apparent.

I have manually confirmed the values by using debug messages in source files.
Notice that it is same in Notosans and hence doesn't issues in either.
First point in Miscellaneous section of CHANGELOG.

Here is the same Overpass-SemiBold.ttf with just that checkbox "Really use Typo metrics" unchecked and saved. This has same button height in both 3.1 and master.
Full Test Project(because the one in OP has path issues): ButtonSizeBug2.zip.

[volzhs]

so you modified ttf file instead of code?

[volzhs]

@akien-mga freetype released new 2.10.1 version.
it's better to test with the 2.10.1 version to see if solve this issue.
I like to do it myself, but I don't have much time these days.
I know you are also busy these days too. 😃

[Anutrix]

so you modified ttf file instead of code?

The changes are on Freetype's side so I couldn't do anything.
Note(just bragging): I changed Freetype's code about hundred times to print debugging face->ascender values at different times while testing.
Note: I don't know anything about how fonts work.

[Anutrix]

@akien-mga freetype released new 2.10.1 version.
it's better to test with the 2.10.1 version to see if solve this issue.
I like to do it myself, but I don't have much time these days.
I know you are also busy these days too. 😃

I tested Godot with Freetype 2.10.1 stable source today. Issue persists just like 2.10.0 . Still not sure if this is their bug or feature.

[Zireael07]

Probably a compat-breaking feature, judging from their changelog notes?

[volzhs]

@Zireael07 the problem is font is not vertically center aligned comparing to freetype 2.9

[akien-mga]

Nice work @Anutrix.

So in the end it's an issue specific to OpenType fonts using the Really use typo metrics flag, and it's documented in the FreeType changelog:

• The logic for computing the global ascender, descender, and
  height of OpenType fonts has been slightly adjusted for
  consistency.

  • If the useTypoMetrics' flag (i.e., bit 7 in thefsSelection'
    field) in the OS/2' table is set, use thesTypo' fields in
    `OS/2' unconditionally.
  • Otherwise use the metrics data from the `hhea' table (if not
    zero).
  • Otherwise use the sTypo' fields from theOS/2' table (if not
    zero).
  • Otherwise use the usWin' data from theOS/2' table as a last
    resort.

Is there anything we can do at the engine side to mitigate breaking compat for those fonts, or should we simply document this as a change in 3.2 and how to adapt fonts whose behavior changed?

[volzhs]

I had looked in various way to solve it on our side.
but couldn't find it.

there is a workaround for those font in the font settings.

[Anutrix]

Disclaimer: I don't much about fonts. Just started reading on it recently.
I would like someone to test all fonts having this issue. According to what I have read, the values of the 3 versions of ascents/descents should be really close. Hence, only few fonts seem to have this issue.
Eg. Overpass 750 vs 982
Reporting the issues to respective font developers to be good IMHO.

[akien-mga]

As per @Anutrix's debugging, it seems to be an expected consequence of changes in FreeType's handling of some OpenType font metrics, which introduce this visual difference for fonts with invalid metrics.

So that's an issue for font providers to handle on their end (or negotiate with FreeType devs if they feel this 2.10 change was incorrect). From our end, I think we should just document this in the 3.2 changelog (cc @Calinou), ideally with a workaround for affected users (e.g. how to fix the font, or adding extra spacing as shown by @volzhs).

[apodtele]

This is the change that you question:
https://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/?id=f72b00746c64e13625cf8371f031411fbd0d6161

And the long discussion about it:
https://marc.info/?t=154508255300008&r=1&w=2

There is no reason for FreeType to disobey the font flags.

[akien-mga]

Thanks for digging this up :)

We can close this as "not a bug" once this FreeType change is mentioned in our release notes (cc @Calinou). This is strictly something for font providers to handle so that their font metrics work as expected by the OpenType spec and FreeType implementation.

[Calinou]

@akien-mga I added a mention of this change in the changelog.

[akien-mga]

Thanks, closing then.

[madig]

The typo metrics have some semantics attached to them, so it is the responsibility of the text layout engine to properly handle them: https://docs.microsoft.com/en-us/typography/opentype/spec/recom#tad. The appearance of text being "shifted up" usually comes from the typo metrics providing a more accurate but tighter bounding box, while the head metrics usually carry the line gap at the top for compatibility reasons. Take that away and you get a "shift upwards".

The UseTypoMetrics flag really signals that the layout code _must_ to take over. Trying to make vertical metrics the same across platforms is impossible, it is much better to expose controls to the designer to fully control line-spacing.