Removes outdated baseline/y-offset correction in adjustFont#20
Open
uguraslan wants to merge 1 commit into
Open
Removes outdated baseline/y-offset correction in adjustFont#20uguraslan wants to merge 1 commit into
uguraslan wants to merge 1 commit into
Conversation
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.
This fixes a silent but real bug affecting the vertical alignment of every font this tool has generated since October 2025 (
v1.2.0through the currentv1.3.0). Text is currently sitting slightly too low and this PR corrects it.Background
msdf-bmfont-xmlhad a bug: it was placing the baseline and every character's vertical position slightly wrong. He added a small correction for it here, inadjustFont.ts. At the same time, he also opened this (Fix baseline alignment soimy/msdf-bmfont-xml#93) to fix the actual bug at its source.He also added a
@remarkin the source code:msdf-generator/src/adjustFont.ts
Lines 30 to 35 in ae716ca
[February 2025] : that upstream PR was merged. From this point on, any new version of
msdf-bmfont-xmlbuilt from theirmainbranch no longer has the bug.[October 2025] : PR #14 updated our dependency to
msdf-bmfont-xml@2.8.0, which includes that upstream fix. This was a good and necessary update. But after that point our ownadjustFont.tscorrection (written for the old, broken version) has started to double fix something that was already fixed.Changes
This PR removes the redundant correction in
adjustFont.ts. The function still does its other job (extracting and writing font metrics), only the baseline/y-offset math is removed.Verification
msdf-bmfont-xml, before ouradjustFont.tscorrection is applied.adjustFont.tscorrection on top of that old, unfixed library.msdf-bmfont-xmlfixes the baseline issue on its own, without our correction.So by removing the extra adjustment, we are returning to state 3.
Visual impact of this fix
This fix will visibly shift text in every app that uses this package, by a small amount (a couple of pixels based on the text configuration). It's not dramatic and that's exactly why this went unnoticed for months. But it is a real, visible change to existing layouts. Anyone relying on the current (incorrect) text position may notice things shift slightly, to the correct baseline.