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

[merger] Cursive attachment merging fails when all anchors are NULL #3247

Closed
simoncozens opened this issue Aug 7, 2023 · 4 comments · Fixed by #3248
Closed

[merger] Cursive attachment merging fails when all anchors are NULL #3247

simoncozens opened this issue Aug 7, 2023 · 4 comments · Fixed by #3248

Comments

@simoncozens
Copy link
Collaborator

Carried over from googlefonts/fontmake#1022.

When all anchors in all masters are of the form:

    pos cursive _0 <anchor NULL> <anchor 25 0>;

the merger creates an ExitEntryAnchor like this:

        rec = ot.EntryExitRecord()
        rec.EntryAnchor = ot.Anchor()
        rec.EntryAnchor.Format = 1
        rec.ExitAnchor = ot.Anchor()
        rec.ExitAnchor.Format = 1
        self.EntryExitRecord.append(rec)

This means that deep inside mergeThings, when trying to merge the entry anchors, we hit this code path:

            if allNone(lst):
                if out is not None:
                    raise FoundANone(self, got=lst)

All entry anchors are None, but the target entry anchor is an Anchor so merging fails.

@anthrotype
Copy link
Member

is this perhaps a regression introduced with #3209 three weeks ago?

@simoncozens
Copy link
Collaborator Author

Confirmed, it works at 56731a9 but not at b14a29c.

Bad Behdad. ;-)

@anthrotype
Copy link
Member

how does one reproduces exactly? could you push a failing test case in varLib/merger_test.py so Behdad can fix it

@simoncozens
Copy link
Collaborator Author

Will do. I could probably fix it myself, TBH; it just needs a custom merger for EntryAnchor/ExitAnchors.

simoncozens added a commit that referenced this issue Aug 7, 2023
simoncozens added a commit that referenced this issue Aug 7, 2023
* A failing test for #3247

* Fix up NULL entry/exit records on merge, fixes #3247
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 a pull request may close this issue.

3 participants