Skip to content

Resolve mark attachments after zeroing mark advances - #407

Merged
blikblum merged 2 commits into
foliojs:masterfrom
lineto:fix/attachments-after-zeroing
Oct 21, 2026
Merged

blikblum merged 2 commits into
foliojs:masterfrom
lineto:fix/attachments-after-zeroing

Conversation

@lehni

@lehni lehni commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Observed with real fonts: in Roboto Serif, a diaeresis stacked on an acute (x́̈) is offset by the acute's advance (fontkit −70, HarfBuzz 5). In our Akkurat Hebrew, every vowel and dot is shifted.

  • Attached marks were offset by the advances of the marks between them and their base, which shapers zero only after GPOS
  • Resolve cursive and mark attachments once, after the advances are final, as HarfBuzz does, instead of after every GPOS stage
  • Add a test with a Roboto Serif subset (OFL), matching hb-shape

lehni and others added 2 commits October 19, 2026 10:15
- Attached marks were offset by the advances of the marks between them and their base, which shapers zero only after GPOS, misplacing stacked marks in fonts whose marks have an advance (Roboto Serif, and all Hebrew vowels in Akkurat Hebrew)
- Resolve cursive and mark attachments once, after the advances are final, as HarfBuzz does, instead of after every GPOS stage
- Add a test with a Roboto Serif subset (OFL), matching hb-shape
super.applyFeatures(userFeatures, glyphs, advances);

for (var i = 0; i < this.glyphs.length; i++) {
fixAttachments(glyphs, positions) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

applyFeatures was not being used?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind. The super class still implements it

@blikblum
blikblum merged commit 3e0b7db into foliojs:master Oct 21, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants