Skip to content

Fix unwanted CJK-Latin spacing in text decorations#7521

Closed
Ri-Nai wants to merge 2 commits intotypst:mainfrom
Ri-Nai:fix/cjk-decorate-padding
Closed

Fix unwanted CJK-Latin spacing in text decorations#7521
Ri-Nai wants to merge 2 commits intotypst:mainfrom
Ri-Nai:fix/cjk-decorate-padding

Conversation

@Ri-Nai
Copy link
Copy Markdown
Contributor

@Ri-Nai Ri-Nai commented Dec 3, 2025

Description:
Related to #7475 where underlines and highlights extend into the automatic spacing added between CJK and Latin characters.

Implementation Details:
Instead of modifying the underlying shaping logic (which inserts the spacing directly into glyph advances/offsets), this PR adjusts the decoration rendering in deco.rs.

It introduces adjust_cjk_latin_spacing which heuristicsally detects if spacing was applied to the first/last glyphs of a run. It checks if the glyph is a CJK script and if the x-offset or advance includes the specific 0.25em padding used by the shaper.

It's just a temporary patch. While refactoring the CJK-Latin spacing implementation in the shaper might be the "pure" fix, it is a significant architectural change with high regression risks.

@Ri-Nai Ri-Nai force-pushed the fix/cjk-decorate-padding branch from 973b522 to a9a8da1 Compare December 3, 2025 15:22
@laurmaedje laurmaedje added waiting-on-review This PR is waiting to be reviewed. text Related to the text category, which is all about text handling, shaping, etc. fix A bug fix. labels Dec 4, 2025
@laurmaedje laurmaedje added cjk Chinese, Japanese, Korean typography. i18n About language- or script-specific features. May need attention from native speakers. labels Jan 7, 2026
@laurmaedje
Copy link
Copy Markdown
Member

It's just a temporary patch. While refactoring the CJK-Latin spacing implementation in the shaper might be the "pure" fix, it is a significant architectural change with high regression risks.

I must admit that I'm concerned with the maintainability of the inline layouter. It has accumulated quite a bit of somewhat hacky code, in particular for CJK handling, which makes it quite hard to cleanly refactor (especially without good understanding of CJK languages since I can barely judge whether something is an improvement or a regression).

@laurmaedje
Copy link
Copy Markdown
Member

Thanks for working on this, but I'll close it as I think this patch is too hacky and introduces too much maintainability burden. What it really shows me is that the CJK-Latin spacing should not be considered part of the glyph advance in the first place, but rather be handles as a separate spacing item or similar, that's inserted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cjk Chinese, Japanese, Korean typography. fix A bug fix. i18n About language- or script-specific features. May need attention from native speakers. text Related to the text category, which is all about text handling, shaping, etc. waiting-on-review This PR is waiting to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants