Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark-to-mark glyph positioning incorrect #107

Closed
wezm opened this issue May 28, 2024 · 10 comments
Closed

Mark-to-mark glyph positioning incorrect #107

wezm opened this issue May 28, 2024 · 10 comments

Comments

@wezm
Copy link
Contributor

wezm commented May 28, 2024

Some marks in the Arabic text "ٱللَّهَ" are not positioned correctly.

Issue

The last glyph colored in "Blue" should be positioned similar to its corresponding glyph in the word below.

This is the layout details for the last glyph

"glyph_index": 104
"kerning": 0

// Layout Info.
"hori_advance": 0
"vert_advance": 0
"x_offset": 640
"y_offset": 1600

// Placement
"MarkAnchor": [
        3,
        {
          "x": 135,
          "y": 250
        },
        {
          "x": 225,
          "y": 0
        }
    ]
}

This is the **computed** x position for each glyph  5 glyphs
[-438, -2184, -1429, -1454, -1544] // negative for RTL
fn( i ) => -( hor_advance_sum( i ) - x_offset[ i ])

The hori_advance for each glyph from allsorts
[438, 1746, 0, 0, 0]

The x_offset for each glyph from allsorts
[0, 0, 755, 730, 640]

// Render Settings
font_size: 72
dpi: 96
upm: 2048
font: uthmanic_hafs_v20.ttf

I will attach the data collected from allsorts in the next comment.
Looking forward to hearing from you.

Originally posted by @solomancode in #106

@solomancode
Copy link

If anyone has an idea how to fix this issue, I'd be glad to help. Thank you.

@wezm
Copy link
Contributor Author

wezm commented May 28, 2024

If anyone has an idea how to fix this issue, I'd be glad to help. Thank you.

I have a fix, it's just working its way through internal review.

@solomancode
Copy link

@wezm That's great. Thanks for your efforts, Please let me know if I could be of any help.

wezm added a commit that referenced this issue May 29, 2024
Reported in #107

The selection of glyph pairs for mark-to-mark positioning was too eager.
For the text in question the ligature component position was not being
considered which was resulting in mark-to-mark positioning being applied
to marks 2 and 3 when it should not have. Considering the ligature
component position corrects this.

However, requiring the ligature component position to always match broke
some marks in Vietnamese. The solution was the introduction of a
ligature flag on RawGlyph so that a mark glyph can be tracked as a
ligature. This is used during GPOS to allow mark-to-mark positioning
between a pair of marks if one of them is a ligature despite having
differing ligature component positions.

This behaviour matches Harfbuzz:

https://github.com/harfbuzz/harfbuzz/blob/09a17a086c54e5c1b1aa674bba5b5449286e1480/src/OT/Layout/GPOS/MarkMarkPosFormat1.hh#L134-L138
@wezm
Copy link
Contributor Author

wezm commented May 29, 2024

This is the fix 1bf2db8

I'll work on a patch release now

@wezm wezm closed this as completed May 29, 2024
wezm added a commit that referenced this issue May 29, 2024
Reported in #107

The selection of glyph pairs for mark-to-mark positioning was too eager.
For the text in question the ligature component position was not being
considered which was resulting in mark-to-mark positioning being applied
to marks 2 and 3 when it should not have. Considering the ligature
component position corrects this.

However, requiring the ligature component position to always match broke
some marks in Vietnamese. The solution was the introduction of a
ligature flag on RawGlyph so that a mark glyph can be tracked as a
ligature. This is used during GPOS to allow mark-to-mark positioning
between a pair of marks if one of them is a ligature despite having
differing ligature component positions.

This behaviour matches Harfbuzz:

https://github.com/harfbuzz/harfbuzz/blob/09a17a086c54e5c1b1aa674bba5b5449286e1480/src/OT/Layout/GPOS/MarkMarkPosFormat1.hh#L134-L138
@wezm
Copy link
Contributor Author

wezm commented May 29, 2024

0.14.2 has been published.

@solomancode
Copy link

Thank you.

It's impressive your deep knowledge of opentype specs. even though the resources are scarce.
This fix was — Combining Ligature Marks. Am I correct ?

@solomancode
Copy link

image
😊

@wezm
Copy link
Contributor Author

wezm commented May 29, 2024

It's impressive your deep knowledge of opentype specs. even though the resources are scarce.

Unfortunately the OpenType specs are lacking specification for this type of thing so it mostly comes down to receiving bug reports like yours and then trying to work out what should be happening.

This fix was — Combining Ligature Marks. Am I correct ?

The fix for your specific issue was when performing mark-to-mark positioning to consider the ligature component position and only perform mark-to-mark positioning when the marks are on the same ligature component. However, applying this in isolation broke a Vietnamese test we had where one of the marks was itself a ligature so I mirrored what Harfbuzz does and allowed mark-to-mark positioning across ligature components in the case where one of the marks is a ligature.

@solomancode
Copy link

@wezm Thanks again for your efforts.

@wezm
Copy link
Contributor Author

wezm commented May 30, 2024

No worries, thanks for the clear bug report.

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

No branches or pull requests

2 participants