Rewriting the FreeType implementation#37
Open
longtran2904 wants to merge 1 commit into4coder-community:masterfrom
Open
Rewriting the FreeType implementation#37longtran2904 wants to merge 1 commit into4coder-community:masterfrom
longtran2904 wants to merge 1 commit into4coder-community:masterfrom
Conversation
The first issue is DPI scaling. When requesting a font size, 4coder does not provide horiResolution or vertResolution, causing FreeType to fall back to a default. In recent versions, this default is 96 DPI, which is what we want. However, 4coder uses an older FreeType version where the default is 72 DPI, leading to incorrect sizing calculations. The second issue is the codepoint lookup table. The way the code iterates through all the glyphs is incorrect, the starting size for the table is incorrect, and the way the code checks for the zeroth glyph is also incorrect. The third issue is library initialization. The code never checks whether FT_Init_FreeType succeeds, and it always calls FT_Done_FreeType unconditionally, even if initialization failed. The fourth issue is how glyph data (bounds and advance) is stored. 4coder places it in a large array spanning from codepoint zero to the font’s maximum codepoint. Since many codepoints have no associated glyph, most of the entries go unused. My fix replaces this with a system that stores glyph data only for codepoints that actually exist. I also made some minor fixes like checking for pixels greater than 128 (instead of just non-zero) in 1-bit monochrome, removing the unused white glyph, and resetting the arena whenever an error occurs.
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.
The first issue is DPI scaling. When requesting a font size:
4coder does not setsize.horiResolutionorsize.vertResolution, so FreeType falls back to its default. In recent versions, this default is 96 DPI, which is what we want. However, 4coder uses an older FreeType version where the default is 72 DPI, leading to incorrect sizing calculations.[Edit: While pulling this out into a standalone article, I noticed that this part is slightly incorrect. The default value is always 72 dpi, but
scale_factoris defined relative to 96 dpi (so1.0equals 96 dpi). That’s why the calculation comes out wrong. The correct approach is to stop premultiplying the size withscale_factorand instead pass the actual dpi directly.]The second issue is the codepoint lookup table. The way the code iterates through all the glyphs is incorrect. It should stop when
indexis zero, not whencounter == glyph_count,glyph_countis typically way larger than the number of mappable glyphs anyway. This also means that the starting size for the table is too much:glyph_count * 2is enough. Finally, the check for the zeroth glyph is also incomplete:FT_Get_First_Charcould return the null codepoint, bypassing the check.The third issue is library initialization. The code never checks whether
FT_Init_FreeTypesucceeds, and it always callsFT_Done_FreeTypeunconditionally, even if initialization failed.The fourth issue is inefficient glyph storage. Glyph data (bounds and advance) is packed in a large array covering every codepoint up to the font’s maximum. Most of these entries are unused, since many codepoints don’t have an associated glyph. I replace this with a system that only stores glyph data for codepoints that actually exist.
I also made some minor fixes, like checking for pixels greater than 128 (instead of just non-zero) in 1-bit monochrome, ignoring the unused white glyph, and resetting the arena whenever an error occurs. This commit also includes my fix for issue #35.
Edit: Forgot to mention that I also simplified the texture packing code into a single function. I could go further and inline that function too, but I prefer to keep it separate and more general, to signal that this is an additional (and optional) step, and allow other code to reuse it in the future.