Implement CSS-style font size algorithm #5123

frenzibyte · 2022-04-23T04:49:20Z

Closes CSS-compatible font sizing #3271
Resolves the font size discrepancy in Discrepancy between element dimensions in pixels on figma designs and dimensions in game units osu#17940

Adds support for CSS-style font sizing, allowing for our sprite text sizes to match 1:1 with CSS.

As a starting point, the option is disabled by default to avoid breaking every existing sprite text in all games, but as we move forward (with migrating to new designs in osu!), we'll want to make the option enabled by default, and potentially remove the option (keeping it only based on whether the font's metrics are available).

For reviewing ease, I'll recommend going commit-by-commit as the overall diff is a bit messy to read through (mainly because of the addition of FontMetrics, addition of FontUsage.CssScaling, and addition of note font for testing purposes).

`FontMetrics`

Since the font binaries don't store metadata about the metrics (ascent, descent, em size), it is required for osu! and all consumers of the framework to manually extract them from the font and specify them in AddFont, similar to how it's done here:

@@ -161,19 +162,24 @@ private void load(FrameworkConfigManager config)
             // note that currently this means there could be two async font load operations.
             Fonts.AddStore(localFonts = new FontStore(useAtlas: false));

+            // Roboto and Font Awesome have different metrics for Windows and macOS.
+            // The ones used for Windows are used here for the time being.
+            var robotoMetrics = new FontMetrics(ascent: 1946, descent: 512, emSize: 2048);
+            var fontAwesomeMetrics = new FontMetrics(ascent: 460, descent: 84, emSize: 512);
+
             // Roboto (FrameworkFont.Regular)
-            addFont(localFonts, Resources, @"Fonts/Roboto/Roboto-Regular");
-            addFont(localFonts, Resources, @"Fonts/Roboto/Roboto-RegularItalic");
-            addFont(localFonts, Resources, @"Fonts/Roboto/Roboto-Bold");
-            addFont(localFonts, Resources, @"Fonts/Roboto/Roboto-BoldItalic");
+            addFont(localFonts, Resources, @"Fonts/Roboto/Roboto-Regular", robotoMetrics);
+            addFont(localFonts, Resources, @"Fonts/Roboto/Roboto-RegularItalic", robotoMetrics);
+            addFont(localFonts, Resources, @"Fonts/Roboto/Roboto-Bold", robotoMetrics);
+            addFont(localFonts, Resources, @"Fonts/Roboto/Roboto-BoldItalic", robotoMetrics);

             // RobotoCondensed (FrameworkFont.Condensed)
-            addFont(localFonts, Resources, @"Fonts/RobotoCondensed/RobotoCondensed-Regular");
-            addFont(localFonts, Resources, @"Fonts/RobotoCondensed/RobotoCondensed-Bold");
+            addFont(localFonts, Resources, @"Fonts/RobotoCondensed/RobotoCondensed-Regular", robotoMetrics);
+            addFont(localFonts, Resources, @"Fonts/RobotoCondensed/RobotoCondensed-Bold", robotoMetrics);

-            addFont(Fonts, Resources, @"Fonts/FontAwesome5/FontAwesome-Solid");
-            addFont(Fonts, Resources, @"Fonts/FontAwesome5/FontAwesome-Regular");
-            addFont(Fonts, Resources, @"Fonts/FontAwesome5/FontAwesome-Brands");
+            addFont(Fonts, Resources, @"Fonts/FontAwesome5/FontAwesome-Solid", fontAwesomeMetrics);
+            addFont(Fonts, Resources, @"Fonts/FontAwesome5/FontAwesome-Regular", fontAwesomeMetrics);
+            addFont(Fonts, Resources, @"Fonts/FontAwesome5/FontAwesome-Brands", fontAwesomeMetrics);

             dependencies.Cache(Fonts);

Note that different platforms use different metric tables, and I'm not entirely sure if we should use each metric based on the platform, as it turns out there can be differences. Therefore I continued with the one used by Windows.

This is still completely up to the consumer whether they want it to be platform-specific or not, but just mentioning this in regards to osu!.
I've kept the font metrics argument in AddFont as optional, for special fonts which don't have the concept of "font metrics" ("8-bit" fonts and the like).

I will write up more about this in the fonts wiki if the FontMetrics concept is agreed on, mainly about how to extract them and which values should be used, etc.

`FontUsage.CssScaling`

Going forward, osu! and all consumers should always append css: true to their font usages, potentially using a different static and prefixing the old non-CSS ones with Old:

diff --git a/osu.Game/Graphics/OsuFont.cs b/osu.Game/Graphics/OsuFont.cs
index edb484021c..62eb7492f8 100644
--- a/osu.Game/Graphics/OsuFont.cs
+++ b/osu.Game/Graphics/OsuFont.cs
@@ -19,7 +19,9 @@ public static class OsuFont

         public static FontUsage Numeric => GetFont(Typeface.Venera, weight: FontWeight.Bold);

-        public static FontUsage Torus => GetFont(Typeface.Torus, weight: FontWeight.Regular);
+        public static FontUsage OldTorus => GetFont(Typeface.Torus, weight: FontWeight.Regular);
+
+        public static FontUsage Torus => GetFont(Typeface.Torus, weight: FontWeight.Regular, css: true);

         public static FontUsage TorusAlternate => GetFont(Typeface.TorusAlternate, weight: FontWeight.Regular);

It could also go the other way around, by keeping the old one as-is, and defining a New font with css: true applied instead.

I've added behavioural test coverage, and also added a visual illustration in the sprite text test scene, by comparing against a picture of a CSS text element with an equal font size, in a properly aligned container:

Have also added sprite texts with CSS-style enabled, alongside the pre-existing non-CSS ones:

Also renames the field to become `useFullGlyphHeight` to better reflect what it actually does here, also matches with `SpriteText.UseFullGlyphHeight`. If I understand this properly, when setting this to false, it should take the glyph's texture height instead so that text get properly centered, but for some reason it's still including `YOffset` which could leave an empty area at the top of the text.

This one required a bit more work as CSS usually takes the "glyph scale" of the font specified in the `font-family` as far as my manual testing went. I think it should be fine to create a `GetFont` method returning `IGlyphStore`s for that purpose, as it's also required in visual testing to visualise the baseline position in a sprite text.

…te texts

This doesn't affect the test scene since the scaling is based on the name of the font specified for the `SpriteText`. Same for CSS as far as I've tested.

bdach · 2022-04-23T07:20:53Z

and addition of noto font for testing purposes

Why was this done? I think it would have sufficed to test Roboto.

frenzibyte · 2022-04-23T07:24:17Z

Just for the sake of adding more into the visualisation test, and also to confirm that the framework’s handling of multiple fonts in one sprite text also matches CSS.

osu.Framework/Graphics/Sprites/FontUsage.cs

osu.Framework/Text/ITexturedGlyphLookupStore.cs

osu.Framework/Text/TextBuilder.cs

Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>

osu.Framework/Text/IHasGlyphStores.cs

osu.Framework/Text/FontMetrics.cs

osu.Framework/IO/Stores/FontStore.cs

peppy · 2022-05-03T06:33:09Z

osu.Framework/Game.cs

@@ -167,19 +168,24 @@ private void load(FrameworkConfigManager config)
 // note that currently this means there could be two async font load operations.
 Fonts.AddStore(localFonts = new FontStore(useAtlas: false));

+ // Roboto and Font Awesome have different metrics for Windows and macOS.
+ // The ones used for Windows are used here for the time being.
+ var robotoMetrics = new FontMetrics(ascent: 1946, descent: 512, emSize: 2048);


Would be handy to link a guide or something to know how to obtain these. As a developer I'd probably end up here to figure how to add custom fonts, and have little idea how to make these.

Have mentioned that in PR description, but I plan to write up a section in the "Fonts" wiki page about metrics and how to extract them from fonts etc.

osu.Framework.Tests/TestGame.cs

smoogipoo · 2022-05-10T09:59:42Z

The test doesn't seem to be working for me:

I don't seem to have any local changes, so hopefully it's not just me?

frenzibyte · 2022-05-10T10:01:22Z

Indeed it regressed, will look into this immediately...

smoogipoo

Rest of the code looks fine at a glance, apart from the test not working.

smoogipoo · 2022-05-10T09:21:45Z

osu.Framework/Text/TextBuilderGlyph.cs

+ /// The size to draw the glyph at.
+ /// </summary>
+ /// <remarks>
+ /// Note that this can differ per font on one specified <see cref="FontUsage.Size"/>, depending on each one's font metrics.


I think sounds better:

For the same <see cref="FontUsage"/> size, this value can differ per-font depending on each font's metrics.

smoogipoo · 2022-05-10T09:27:52Z

osu.Framework/Text/TextBuilder.cs

+ float size = font.Size;
+
+ if (font.CssScaling && fontStoreGlyph.Metrics is FontMetrics metrics)
+ size *= metrics.GlyphScale;


Can this be extracted to a separate method, so that it's not duplicated in two places?

Do you expect something like getGlyphSize(FontUsage font, FontMetrics metrics)?

Just a getGlyphHeight(). I think everything you need is already part of the class-level variables.

smoogipoo · 2022-05-10T09:28:19Z

osu.Framework/Text/TextBuilder.cs

- /// <param name="useFontSizeAsHeight">True to use the provided <see cref="font"/> size as the height for each line. False if the height of each individual glyph should be used.</param>
+ /// <param name="useFullGlyphHeight">True to use <see cref="TextBuilderGlyph.Size"/> (full glyph size) as the height for each line. False if the height of each individual glyph (<see cref="TextBuilderGlyph.YOffset"/> + <see cref="TextBuilderGlyph.Height"/>) should be used.</param>


I think SpriteText's xmldoc reads better than this one. Possible to replace?

@bdach seeing that you struggled to read how this was worded initially (#5123 (comment)), what's your opinion on rewording this once again as suggested above, i.e.:

- /// <param name="useFullGlyphHeight">True to use <see cref="TextBuilderGlyph.Size"/> (full glyph size) as the height for each line. False if the height of each individual glyph (<see cref="TextBuilderGlyph.YOffset"/> + <see cref="TextBuilderGlyph.Height"/>) should be used.</param> + /// <param name="useFullGlyphHeight"> + /// True if the <see cref="TextBuilder"/>'s vertical bounds should be equal to the maximum full height of the glyphs (constant per font). + /// False to consider the maximum texture height of each individual glyph instead. + /// </param>

It's marginally better I guess. It still confuses me that "full height of the glyphs" is mentioned in one place and "height of each individual glyph" is in the other. The difference feels extremely subtle.

If I were to propose something then it'd probably be something along the lines of

/// <param name="useFullGlyphHeight"> /// When true, all texts that use a particular font will have a constant height, regardless of the letters used. /// When false, the height of a given text will be cropped to only fit the letters used in that particular text, /// and extra space above and below letters will be trimmed. /// </param>

osu.Framework/Graphics/Sprites/FontUsage.cs

frenzibyte · 2022-05-10T10:31:09Z

Apparently the test scene regressed because of 78d32b6 and I swear it wasn't broken to me the moment I tested it...

That commit was changing the metrics values to use the ones which Windows respects (typo), but the text images provided were taken on macOS (hhea), therefore causing this discrepancy.

The metrics provided by Windows lead to CSS scale of 1x ((880 + 120) / 1000 = 1), so I'm going to stick with the macOS ones with an inline comment and call it a day.

`inheritdoc` is a life saver.

bdach · 2022-05-10T14:23:33Z

That commit was changing the metrics values to use the ones which Windows respects (typo), but the text images provided were taken on macOS (hhea)

Well this is disconcerting, are you saying this can differ per-platform somehow, too? I would hope not...

frenzibyte · 2022-05-10T14:37:05Z

Well, some fonts render differently on browsers in Windows and macOS, due to there existing different metrics tables (most of the time the same values will be used on all metrics, but some fonts like Roboto and Noto Sans JP don't):

It is up for the framework consumer to decide whether they want their font to be rendered correctly based on the running platform (using RuntimeInfo.OS), or by sticking to one metrics table and call it a day.

For osu!, I think it may be best sticking to the Typo table, but we can consider rendering per-platform correctly if we want to. Just a matter of supplying the font with the correct metric values.

bdach · 2022-05-10T14:50:02Z

I think I'm just going to pretend I didn't ask and nod along. This is a rabbit hole the depths of which I do not dare to explore.

jai-x · 2022-05-10T14:54:37Z

For reference I found the relevant platform specific hacks that Chrome uses: https://github.com/chromium/chromium/blob/76769a9d7e911b4efa1fab3cc6e1b5df86f3d227/third_party/blink/renderer/platform/fonts/simple_font_data.cc#L88-L179

frenzibyte added 7 commits April 23, 2022 06:39

Introduce font metrics structure

3ac831a

Integrate the font metrics structure with the glyphs flow

9bd7ff8

Implement an optionally toggleable CSS scaling functionality

9c43cdf

Include font metrics for framework fonts

a8806ca

Add test coverage

d194b8d

frenzibyte added the area:fonts label Apr 23, 2022

pull-request-size bot added the size/XL label Apr 23, 2022

frenzibyte added 2 commits April 23, 2022 08:02

Add visual test illustrating the baseline position in font-mixed spri…

07cef08

…te texts

Include CSS-style sprite texts in test scene

b2370e5

frenzibyte force-pushed the css-font-algorithm branch from 947a8ff to b2370e5 Compare April 23, 2022 05:03

Fix wrong font metrics for test "Noto" font

78d32b6

This doesn't affect the test scene since the scaling is based on the name of the font specified for the `SpriteText`. Same for CSS as far as I've tested.

frenzibyte mentioned this pull request Apr 23, 2022

Update OsuFont to support CSS-style font sizing and supply osu! font with metrics ppy/osu#17946

Draft

2 tasks

bdach reviewed Apr 23, 2022

View reviewed changes

osu.Framework/Graphics/Sprites/FontUsage.cs Outdated Show resolved Hide resolved

bdach reviewed Apr 23, 2022

View reviewed changes

osu.Framework/Text/ITexturedGlyphLookupStore.cs Outdated Show resolved Hide resolved

osu.Framework/Text/TextBuilder.cs Outdated Show resolved Hide resolved

osu.Framework/Text/TextBuilder.cs Show resolved Hide resolved

frenzibyte and others added 5 commits April 23, 2022 19:41

Reference computation of "height of each individual glyph"

914f81b

Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>

Split GetFont lookup method to an internal interface

6c6ec9e

Remove no longer necessary implementation

6113bb3

Fix TestStore not implementing internal lookup interface

82ca32d

Merge branch 'master' into css-font-algorithm

2f01659

peppy reviewed May 3, 2022

View reviewed changes

frenzibyte added 3 commits May 3, 2022 10:59

Add note about noto font usage

252a680

Expose BASE_FONT_SIZE publicly

4cdf9a0

IHasGlyphStores -> IFontStore

aaca14b

smoogipoo reviewed May 10, 2022

View reviewed changes

Merge branch 'master' into css-font-algorithm

9aa1c7e

frenzibyte added 6 commits May 10, 2022 13:34

Revert font size metrics changes with note

cf7a7f6

Improve glyph size documentation

e34c364

Improve useFullGlyphHeight parameter documentation

aa38a93

Encapsulate font size calculation logic

f06c141

Fix out-of-sync constructor xmldoc

a6a41bf

`inheritdoc` is a life saver.

css -> cssScaling

fecd0ee

bdach approved these changes May 10, 2022

View reviewed changes

frenzibyte mentioned this pull request Oct 26, 2023

Update "Argon" skin gameplay counters to new design ppy/osu#25226

Merged

4 tasks

frenzibyte mentioned this pull request Nov 4, 2023

Add "Argon" performance points counter ppy/osu#25362

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement CSS-style font size algorithm #5123

Implement CSS-style font size algorithm #5123

frenzibyte commented Apr 23, 2022 •

edited

Loading

bdach commented Apr 23, 2022

frenzibyte commented Apr 23, 2022 •

edited

Loading

peppy May 3, 2022

frenzibyte May 3, 2022

smoogipoo commented May 10, 2022

frenzibyte commented May 10, 2022

smoogipoo left a comment

smoogipoo May 10, 2022

smoogipoo May 10, 2022

frenzibyte May 10, 2022

smoogipoo May 10, 2022

smoogipoo May 10, 2022

frenzibyte May 10, 2022 •

edited

Loading

bdach May 10, 2022

frenzibyte commented May 10, 2022

bdach commented May 10, 2022

frenzibyte commented May 10, 2022 •

edited

Loading

bdach commented May 10, 2022

jai-x commented May 10, 2022

		/// <param name="useFontSizeAsHeight">True to use the provided <see cref="font"/> size as the height for each line. False if the height of each individual glyph should be used.</param>
		/// <param name="useFullGlyphHeight">True to use <see cref="TextBuilderGlyph.Size"/> (full glyph size) as the height for each line. False if the height of each individual glyph (<see cref="TextBuilderGlyph.YOffset"/> + <see cref="TextBuilderGlyph.Height"/>) should be used.</param>

Implement CSS-style font size algorithm #5123

Are you sure you want to change the base?

Implement CSS-style font size algorithm #5123

Conversation

frenzibyte commented Apr 23, 2022 • edited Loading

FontMetrics

FontUsage.CssScaling

bdach commented Apr 23, 2022

frenzibyte commented Apr 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smoogipoo commented May 10, 2022

frenzibyte commented May 10, 2022

smoogipoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frenzibyte May 10, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frenzibyte commented May 10, 2022

bdach commented May 10, 2022

frenzibyte commented May 10, 2022 • edited Loading

bdach commented May 10, 2022

jai-x commented May 10, 2022

frenzibyte commented Apr 23, 2022 •

edited

Loading

`FontMetrics`

`FontUsage.CssScaling`

frenzibyte commented Apr 23, 2022 •

edited

Loading

frenzibyte May 10, 2022 •

edited

Loading

frenzibyte commented May 10, 2022 •

edited

Loading