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

Fix mark to base bug #815

Merged
merged 2 commits into from
Jun 20, 2019
Merged

Fix mark to base bug #815

merged 2 commits into from
Jun 20, 2019

Conversation

readroberts
Copy link
Contributor

No description provided.

@miguelsousa
Copy link
Member

@readroberts can the tests be for makeotfexe rather than makeotf? Tests against the former run faster as there's much less overhead.

@miguelsousa
Copy link
Member

Also, should HOT_VERSION get bumped?

Fix afdko issue Bug in building MarkToBase lookup type 4 #811.

In fillMarkToBase(), If a base record does not have anchor for a given mark class, the logic assigns it an offset of 0. Later, in writeMarkToBase(), an offset of 0 for a base anchor record is used as a flag to write a NULL anchor record.

There are several parts to how the bug happens. First, the offset to the first mark anchor record is always zero. This is not a problem for the array of offsets to the mark anchor records, as the logic assumes these are never NULL. Second, the the anchor offset array is filled first with the mark anchor records, and then extended with the base anchor records. Third, when the base anchor records are processed, the code tries to find a match for the current base anchor record's coords in the anchor record array; if a match is found, the offset for that anchor record is used. This means that if a base anchor record's coords match the coords of the first mark anchor record, the offset for the baser anchor record will be set to the same as that of the first mark anchor record - which is zero. In writeMarkToBase(), this valid offset is then interpreted as flagging a NULL anchor.

I think this bug was added by me on 5/9/2016 commit [20af6dd], when I changed the handling of empty mark records.
@readroberts
Copy link
Contributor Author

@miguelsousa Agreed on both comments. Moved test to makeotfexe_test.py, bumped HOT_VERSION. Force-pushed clean branch.

Copy link
Member

@miguelsousa miguelsousa left a comment

Choose a reason for hiding this comment

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

I verified that feaLib produces the same exact GPOS table.

@cjchapman cjchapman merged commit d2b7dea into develop Jun 20, 2019
@cjchapman cjchapman deleted the fix-MarkToBase-bug branch June 20, 2019 16:29
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.

3 participants