BasicFontRenderer: Fix various sizing and placement issues - #185
Merged
Conversation
In particular: - `getStringHeight` returned the height of the biggest character, but while `g` and `h` have similar height, the height of `gh` should be 1.5x that. - `getStringWidth` failed to include the width of glyphs without atlas bounds (such as the space character). - `getStringWidth` incorrectly adds between-letter spacing when the last character has no atlas bounds (e.g. space or unmapped or color code) - All glyphs were incorrectly offset on the y axis by 0.025em, and all their atlas coordinates on both axes by 0.5px. Together with other bugs these happened to cancel out with the Minecraft Five font, but not for any other font. Afaict these stem from an incorrect understanding of why the raw values did not match what one expected. See the docs on the newly added `shrinkGlyphsByHalfAPixel` method and the following point for where this confusion likely came from. - The provided Minecraft Five font actually has a base line height of 6 because most regular letters are actually placed at 0.5px above the baseline. This does not affect other fonts and must be an issue with the original font. - The string overall was rendered one higher than it should have been. Likely as a workaround to the above two points and the following pointwm. This magic offset has been removed now. - Glyphs were drawn at the wrong y position. The font file has its y origin at the bottom, while Minecraft has it at the top; this was taken into account in other places, but not for the y positioning. The old code happened to produce close to correct y positions when taking into account all the other bugs for most characters, but completely failed to do so for characters that aren't close to full-height, such as most punctuation. - The `_` in Minecraft Five was manually changed in 274b2d1 to appear at the correct Y position. This was done by assigning it an incorrect position in the file, which happened to come out at roughly the correct position after all it went through all the other bugs. With the other bugs fixed, this has now been reverted. Further issues that are not addressed by this commit: - The renderer almost completely ignores the x position and `advance` values of glyphs, hard-coding a single pixel spacing instead. This is kind of required given that the provided Minecraft Five font places glyphs in the middle of their allocated space instead of left-aligning them, and uses non-integer spacing. It does however ofc make it impossible to use a font where some characters have extra space around them. - The Minecraft Five font has some characters (e.g. `$`, `@`) which extend far below and/or above the regular line height. These are presently simply drawn out of bounds and may be cut off by scissor effects and similar. Elementa doesn't have any robust way to handle them (without substantially affecting layout), so there's no easy solution for them. - The font file format technically allows setting the y origin to be at the top. This continues to be unsupported by the renderer though.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In particular:
getStringHeightreturned the height of the biggest character, but whilegandhhave similar height, the height ofghshould be 1.5x that.getStringWidthfailed to include the width of glyphs without atlas bounds (such as the space character).getStringWidthincorrectly adds between-letter spacing when the last character has no atlas bounds (e.g. space or unmapped or color code)shrinkGlyphsByHalfAPixelmethod and the following point for where this confusion likely came from._in Minecraft Five was manually changed in 274b2d1 to appear at the correct Y position. This was done by assigning it an incorrect position in the file, which happened to come out at roughly the correct position after all it went through all the other bugs. With the other bugs fixed, this has now been reverted.Further issues that are not addressed by this commit:
advancevalues of glyphs, hard-coding a single pixel spacing instead. This is kind of required given that the provided Minecraft Five font places glyphs in the middle of their allocated space instead of left-aligning them, and uses non-integer spacing. It does however ofc make it impossible to use a font where some characters have extra space around them.$,@) which extend far below and/or above the regular line height. These are presently simply drawn out of bounds and may be cut off by scissor effects and similar. Elementa doesn't have any robust way to handle them (without substantially affecting layout), so there's no easy solution for them.