[tx][libxml2-improved] Move FDArray from fontinfo.plist to lib.plist

[kaydeearts]

Description

Fixes #1542 and #1539
Does what #1380 intended, but rewritten for the new libxml2 improvements + other considerations.
🚀

To-Dos:

• ufowrite: write FDArray in lib.plist instead of fontinfo.plist
• uforead: parse FDArray from lib.plist as well as fontinfo.plist (to support old & new files)
• .glif files: current solution is that iFD & CID is held in a CIDMap in lib.plist. Is this what we want?
• update tests to reflect the changes
• Fix test failures
• Remove support for FDArray in fontinfo.plist
• Edit CIDMap to only have cids
• Move iFD information into a group in groups.plist
• Add tests for groups.plist parsing
• Add tests for CIDMap in lib.plist parsing
• Fix Windows heap corruption fails

Checklist:

• I have followed the Contribution Guidelines
• I have added test code and data to prove that my code functions correctly
• I have verified that new and existing tests pass locally with my changes
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation

[kaydeearts]

Question (mainly for @skef ): Last year, Zachary and I had structured this so that the cid and iFD mappings would be moved from each .glif file to a CIDMap in lib.plist.

With you more recently we have discussed moving the iFD mappings to groups.plist in #1539 . I'm wondering if we should keep the CIDMap as well as have the iFD group, or move the CIDMap to groups.plist, or have only one of them?

For now, I kept the CIDMap in lib.plist in this PR as I mainly focused on refactoring my PR from last year.
I can now make changes as needed since tests pass.

[skef]

The CIDmap isn't a group so it can't/shouldn't go in groups.plist.

I don't see a problem with separating the two (keeping the map in lib.plist and having the group in groups.plist). Some LGC fonts will have FD mappings but no CIDmap.

[kaydeearts]

Question about the key naming: Shall we discuss if we want keys to have "com.adobe.type" or not? Currently, the code should support keys that do and don't have it (since what we currently have in the FDK are the keys without the com.adobe.type in them, and I didn't want to break older files.)

[skef]

Was the previous code ever really in production?

[kaydeearts]

Was the previous code ever really in production?

@skef No, it only ever stayed in PR #1380 and we never ended up merging. Then the plan became to rewrite it after the libxml2 updates. However, Zachary used that branch to release Source Han Serif last year and still uses it for font development.

[punchcutter]

I already started updating to use the new way. No reason at all to keep the old way. iFD should go into groups.plist and CIDs in the CIDMap in lib.plist without all the com.adobe stuff because that's irrelevant. The top level com.adobe.postscriptCIDMap or whatever that name is (writing on my phone) has com.adobe and that should just be a dict with names mapped to CID. The only reason it had com.adobe or even "CID" was because I originally made it a dict with CID and iFD. Since we are moving iFD nothing needs to be named iFD or CID.

[skef]

I got a bit into this but I think it would be more useful for me to wait until the older conventions are removed from the code.

[lgtm-com]

This pull request introduces 1 alert when merging fbfc499 into 6b6487f - view on LGTM.com

new alerts:

• 1 for Lossy pointer cast

---

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

[skef]

I only reviewed the changes since my last review because I remember it being in good shape otherwise. So while I think you should probably get a separate sign-off from @punchcutter I'm approving this from my end.

[punchcutter]

The one big thing that I see right now is that going tx -ufo -o test.ufo test.pfa doesn't create a groups.plist so any further tx'ing on the UFO fails

tx: (ufr) Warning: FDArraySelect not defined for cid-keyed font
tx: (ufr) glyph 'cid00000' missing FDArray index within <lib> dict

We could merge this and then address that. That second message is also unclear because lib.plist does have FDArray, but what's actually missing is groups.plist with an FDArraySelect containing that glyph.

[punchcutter]

LGTM. Let's merge it!