font/shaper: Fix CoreText position.y for some scripts #10179
Merged
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
Contributor
|
Thanks again for the diligence in following up with this <3 |
This was referenced Jan 12, 2026
Merged
mitchellh
added a commit
that referenced
this pull request
Jan 15, 2026
… glyphs incorrectly (#10295) This PR implements a heuristic for detecting ligatures that then prevents resetting to the cell grid prematurely, avoiding position errors from glyphs that follow ligatures. The inspiration for this was the example from #10179 that was off. See the giant comment for an explanation of the heuristic. The tests for this are the following: ### Tai Tham The Tai Tham example from #10179 got updated since now the marking glyph is attached to the ligature cell correctly. Browser: ᩉ᩠ᨿᩩ Before: <img width="970" height="110" alt="CleanShot 2026-01-12 at 10 02 42@2x" src="https://github.com/user-attachments/assets/1315bbeb-0541-420a-91a1-bac39897efe3" /> After: <img width="962" height="122" alt="CleanShot 2026-01-12 at 10 02 57@2x" src="https://github.com/user-attachments/assets/ca2b3e1d-0785-462f-993c-f482d875a1e9" /> ### Javanese Browser: ꦤ꧀ꦲꦸ Before: <img width="962" height="98" alt="CleanShot 2026-01-12 at 10 05 43@2x" src="https://github.com/user-attachments/assets/74002334-7140-4646-806e-2194457e56c5" /> After: <img width="962" height="110" alt="CleanShot 2026-01-12 at 10 05 23@2x" src="https://github.com/user-attachments/assets/49c08ac9-98af-409c-9b0e-49a34da93597" /> There does seem to be a small maybe single pixel difference here, and I didn't investigate why. ### Chakma Browser: 𑄝𑄖𑄳𑄠𑄬 Before: <img width="1406" height="104" alt="CleanShot 2026-01-12 at 10 07 52@2x" src="https://github.com/user-attachments/assets/3ba7a9d8-d2a2-4e47-976a-8c7702462a49" /> After: <img width="1408" height="104" alt="CleanShot 2026-01-12 at 10 08 08@2x" src="https://github.com/user-attachments/assets/c25c4a41-b651-4109-8ded-69983fc77267" /> ### Bengali Browser: রাষ্ট্রে Before: <img width="1370" height="84" alt="CleanShot 2026-01-12 at 10 10 42@2x" src="https://github.com/user-attachments/assets/c59e5fc9-0e9e-4c8f-b937-025ad28866d2" /> After: <img width="1370" height="96" alt="CleanShot 2026-01-12 at 10 11 00@2x" src="https://github.com/user-attachments/assets/d108dcda-b0b5-4824-b3e4-d1e2ddb6d6de" /> This one doesn't match the browser, but it seems like a CoreText or font issue. ### Log output I've got a `log.txt` with 15k lines from "cell_offset.cluster differs from cluster (potential ligature detected)" logs when I run [ttylang](https://github.com/jacobsandlund/ttylang) (printing the Universal Declaration of Human Rights in various languages). But right now when I try to create a gist with it, I get: <img width="1554" height="584" alt="CleanShot 2026-01-12 at 10 15 29@2x" src="https://github.com/user-attachments/assets/b8a5dd0c-c034-45f6-a2fb-80c67f93f26f" />
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 PR simplifies and corrects the logic for placing a glyph vertically, by using the
position.yfromCoreTextdirectly, instead of using an offset from the cell's startingy. The logic was incorrect from the beginning, always treating the first glyph of a cell as being atyof zero. We only need to be subtracting the cell's startingxto align the glyphs to the cell grid.Enabling the commented out logging, I found no instances of
position.y differs from old offset.ylines withJetBrains Monowith ligatures turned on, but running ttylang (printing the Universal Declaration of Human Rights in various languages) revealed 676 instances of this, with many only slightly off.An example log from some Tai Tham text is the following, and this PR adds a test based on this:
Browsers display this as:
ᩉ᩠ᨿᩩ
mainis printing:this PR prints:
Since this is a ligature of two different grapheme clusters, Ghostty ends up subtracting too much of the
xvalue with thecell_offset.x(starting x), so neither of the screenshots above are correct, but the second is closer and gets theyvalue right.AI disclaimer: I didn't use AI for the code, but did ask it about this Tai Tham text and why it wasn't a single grapheme cluster: https://ampcode.com/threads/T-019b8ea2-1822-75bb-a8eb-55a9ddb9f7ea