fix(font): Improve FreeType glyph measurements and add unit tests for face metrics #8738
Conversation
|
Pushed an update with a better design for sharing FreeType load flags between measurements and rendering. The previous CI failures seemed to be a matter of CI resource constraints, not an issue with the code. |
src/font/Collection.zig
Outdated
| .ascent = 12.3046875, | ||
| .descent = -3.6953125, | ||
| .line_gap = 0.0, | ||
| .underline_position = -1.2265625, |
There was a problem hiding this comment.
comparing floats like this is going to be super sketchy. We should use std.math.approxEqRel. It's going to be tedious to pull them all out but that will be significantly more robust.
There was a problem hiding this comment.
I know, never compare floats for equality. I was actually surprised when the values came out exactly equal in CoreText and FreeType (modulo the ones affected by hinting), but given that they did I figured the calculations producing these values don't have a lot of potential for reassociations that would change the exact result (basically, they are all some variant of value_in_integer_design_units * px_per_unit, so to reassociate you'd probably have to split up the px_per_unit ratio), so I went with the quick and dirty.
Will fix.
There was a problem hiding this comment.
Pushed an update that addresses this.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
|
Anything else I should do to get this ready? Merging the tests isn't that urgent/consequential of course, but the bounding box measurements fix is necessary to make #8720 work as intended (the metrics-based bounding box can be quite a bit too large). |
danielwe
changed the title
… face metrics (#8738) Follow-up to #8720 adding * Two improvements to FreeType glyph measurements: - Ensuring that glyphs are measured with the same hinting as they are rendered, ref [#8720#issuecomment-3305408157](#8720 (comment)); - For outline glyphs, using the outline bbox instead of the built-in metrics, like `renderGlyph()`. * Basic unit tests for face metrics and their estimators, using the narrowest and widest fonts from the resource directory, Cozette Vector and Geist Mono. --- I also made one unrelated change to `freetype.zig`, replacing `@alignCast(@ptrCast(...))` with `@ptrCast(@alignCast(...))` on line 173. Autoformatting has been making this change on every save for weeks, and reverting the hunk before each commit is getting old, so I hope it's OK that I use this PR to upstream this decree from the formatter.
Follow-up to #8720 adding
renderGlyph().I also made one unrelated change to
freetype.zig, replacing@alignCast(@ptrCast(...))with@ptrCast(@alignCast(...))on line 173. Autoformatting has been making this change on every save for weeks, and reverting the hunk before each commit is getting old, so I hope it's OK that I use this PR to upstream this decree from the formatter.