OpenTTD
OpenTTD copied to clipboard
Fix #10151: Use otmAscent instead of otmTextMetrics.tmAscent [Windows]
Motivation / Problem
Windows font rendering provides two sets of metrics for ascent and descent.
We have always used otmTextMetrics.tmAscent and otmTextMetrics.tmDescent, however an alternative is to use otmAscent and otmDescent instead.
Description
These metrics actually more closely match the the sprite font, but do result in a more cramped line-height.
These screenshots also show the viewport sign padding change, although it's also different due to the metrics change.
Limitations
Use of these metrics probably doesn't match macOS and Freetype rendering, but I can't confirm this.
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.
- The bug fix is important enough to be backported? (label: 'backport requested')
- This PR touches english.txt or translations? Check the guidelines
- This PR affects the save game format? (label 'savegame upgrade')
- This PR affects the GS/AI API? (label 'needs review: Script API')
- ai_changelog.hpp, gs_changelog.hpp need updating.
- The compatibility wrappers (compat_*.nut) need updating.
- This PR affects the NewGRF API? (label 'needs review: NewGRF')
- newgrf_debug_data.h may need updating.
- PR must be added to API tracker
This could use a more thorough before/after comparison to show station signs with multiple fonts and sizes, like https://github.com/OpenTTD/OpenTTD/issues/10151#issuecomment-1311110957
At max zoom out, so it's expected they overlap really...
Arial at 2* 6px:
Arial at 1* 10px:
I can't test with Microsoft Himalaya as on my system it ends up using the fallback font instead.
The other comparison is unspaced lists. Reference sprite font:
Arial bold default size in master:
Arial bold default size with PR:
It matches the sprite font better, but whether that's desirable is another matter :-)
Of course it all still depends on the font used, so randomly adding a pixel might space it nicer for Arial, but be too much for another font. 🤷
Okay, I managed to test with Himalaya by only having it set as small font. It still has wonky font metrics.
An alternative method to determine ascender and descender is to manually render a few select upper case and lower case characters, and test where they start and end. Probably bad for non-latin languages.
This could be available for all truetype fontcache implementations though.
I cannot see any alternative metrics fields in FreeType for this. and not even tried looking in macOS.
ChatGPT tells me (yes, I am really doing this):
"In general, it is usually better to use the tmAscent and tmDescent fields from the TEXTMETRIC structure, as these give the ascent and descent values for the font as it is currently being used (taking into account any scaling or other transformations that have been applied). The otmAscent and otmDescent fields, on the other hand, give the maximum possible ascent and descent values for the font, which may not be accurate if the font is being scaled or transformed in some way."
No clue if it is true, but it sounds true :P It has more to say about this PR, if you want to know:
Again, no clue if it is true. But I wanted to share :)
I've been doing some reading... Using otmAscent and otmDescent will work most of the time, but it isn't a robust solution. These are the maximum extent of the glyph outlines, but position changes to glyphs (like contextual position changes, eg. multiple combining diacritics, and things like superscript position) can put glyphs outside this range. Rendering a line height of otmAscent + otmDescent might clip some repositioned glyphs.
However, as there is not a consistent way to define a recommended line spacing, otmTextMetrics.tmAscent and otmTextMetrics.tmDescent are often (ab)used to define a recommended line spacing rather than strictly full ascent/descent. Rendering a line height of less than otmTextMetrics.tmAscent + otmTextMetrics.tmDescent will not necessarily clip any glyphs.
A rendered height of otmTextMetrics.tmAscent + otmTextMetrics.tmDescent drawn, overlapping, with a line spacing of otmAscent + otmDescent should be good. I think...
tmAscent/tmDescent is what we use at the moment, which is what results in the higher than desired line height currently.
Yup, I think we're in agreement(?)
otmTextMetrics.tmAscent+Descent > otmAscent+Descent. So drawing the string in a rectangle with height otmTextMetrics.tmAscent+Descent (a large canvas), but then arranging those in the GUI vertically separated by otmAscent+Descent (more tightly spaced) should make the lines more closely spaced without risking glyph clipping. (Unless I'm totally confused!)
But, perhaps this isn't worth worrying about now I got the OpenTTD Sans line height correct under Windows.
This PR does what you are suggesting (although we draw strings glyph by glyph). The full glyph is still rendered, this only changes how tall we claim it is. Any clipping is because all widgets clip contents so they can't overdraw their boundary.
Is this still relevant for OpenTTD Sans? Or does it only affect other fonts?
This change isn't necessary for the OpenTTD TTFs. I've successfully wrangled the geometry and reported metrics so the line heights are correct on Windows (they already were correct on Linux/Mac). ie. I think I've managed to make otmTextMetrics.tmAscent+Descent = otmAscent+Descent...
Well, I've never seen the font on Windows. I believe Zephyris did a good job making it match but it might not be exact.
And it still might be beneficial (or maybe worse?) for other fonts, although with OpenTTD Sans in play, I think we only have to pay attention to non-Latin fonts.
The challenge languages are things with lots (including stacked) diacritics, like Vietnamese. Reduced line height might lose some accents, so problematic. CJK are probably safe to use the reduced line height.