fix(charset): Implemented charset according to specification

[ln-north]

Description

Implemented Charsets in accordance with the specification

• Enable retrieval of corresponding CID for GID in CID fonts
• Utilize Predefined Charsets from Adobe TN #5176

Motivation and Context

• In CID fonts, we use the CID obtained from Charsets for glyph names. Without this, development and rendering of CID fonts become difficult.
• Aligned with fonttools implementation as it was significantly different.

How Has This Been Tested?

• Tested that in CID Keyed fonts, the glyph name changes from gid2 to cid00002.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• I did npm run test and all tests passed green (including code styling checks).
• I have added tests to cover my changes.
• My change requires a change to the documentation.
• I have updated the README accordingly.
• I have read the CONTRIBUTING document.

[Connum]

I did a quick review and it looks fine to me, however due to limited time at the moment I might have overlooked something and would like to have another reviewer do a more thorough review and testing.

[Connum]

I had hoped that this might also fix #117, but unfortunately it does not. Could you take a look at this and see if there's an easy fix for this or if it's a separete problem?

[ln-north]

Thanks for your review, I got it. I will try it.

[Connum]

I just added the dummy text used to #117 for debugging/fixing

[Connum]

btw, #117 had a different cause and has meanwhile been fixed

[ln-north]

Understood, this PR is now awaiting review.

[yne]

can't test right now
but found some unnecessary spaces/tabs/newline
and also some unrelated " to ' changes that would be cleaner if removed.
but otherwise seems good 👍

[ln-north]

Thanks for the review. I've got it done.

[Connum]

What specification was that change from gid* to cid000...* based on?
We are now failing some of the unicode tests that we passed before, because the tests actually expect a name of "gid*":
CFF-1 and CFF-2 are no longer passing, despite the paths matching, because of the difference in glyph names.

[yne]

@Connum shall we add those test to the CI ? npm run test went fine.

[Connum]

I already have a workflow file prepared on my fork that will run the tests for PRs and add a comment with the results, as well as save the report as a build artifact. I just have to think of a way to make sure that it will mark the workflow as failed when a test fails that previously passed. We'll have to store this info somehow. Could do it by automatically updating a file, but that would pollute the commit history, and I'm not sure if we want that.

How shall we go about the naming issue? I think we should stay in line with all the other implementations, right?

[yne]

I like the idea of a larger test set (even if non-fully-passing)

but there is no need for a file to store the delta. just run the test twice:

• one on the latest PR commit
• one on master

(more failure = CI return fail)

no opinion on the naming, maybe @ln-north have more insight

[Connum]

That's a good idea. We just have to make sure that it catches when something lets a new test pass but breaks an old one, which would result in no change in numbers, so we'll have to compare the actual test results.

[ln-north]

What specification was that change from gid* to cid000...* based on?

The notation of 'cid%05d' follows the convention used in major libraries like afdko and fonttools.
It appears that zero-padding is done to make it sortable.

https://github.com/adobe-type-tools/afdko/blob/master/python/afdko/ttxn.py#L2099-L2101

[Connum]

I mean we could probably adapt the script used for rendering the glyphs for the test suite to generate the gid* names for the output, but that doesn't seem to be necessary for any of the other engines. I just want to make sure we don't break anything.