Feature/ccmp

[TonyJR]

Support noto-emoji

Description

add a new contextChecker for ccmp table in tokenizeText()
add a new substitutionFormat for 'lookup sub_5'

Motivation and Context

This PR resoved multi-character emoji #511.
noto-emoji uses the 'ccmp' table to solve the 'multi-character for one emoji' problem, which is a 'Language forms' feature that should be executed before all other features. When parsing the noto-emoji font, I found that 'lookup sub_5' could not be parsed correctly, so I added a substitutionFormat to handle this issue.

How Has This Been Tested?

The unit tests passed and I added a test case for this feature.

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]

This looks like a great addition! Please check why the build is failing: https://github.com/opentypejs/opentype.js/actions/runs/8388243929/job/22972963564?pr=688
This PR also implements some additional substitution formats/types, which might conflict with some other outstanding PRs, but we'll have to check.

[TonyJR]

okay, i removed the file header and fixed a code error.

[Connum]

There are still some lining issues, can you please resolve them? Then you can run npm run lint locally before committing in order to check if everything is OK.

[Connum]

Please also add tests for the ligature substitution format 3 and the substitution type 21 (which should catch the mistake I mentioned in the comment I just added to the associated code)

[TonyJR]

Please also add tests for the ligature substitution format 3 and the substitution type 21 (which should catch the mistake I mentioned in the comment I just added to the associated code)

I'm trying to add tests for ligature with sub_5, it doesn't work. Maybe I need a few days to fix it. This may modify the code related to "liga" and "rlig".
Any suggestions?

[TonyJR]

Please also add tests for the ligature substitution format 3 and the substitution type 21 (which should catch the mistake I mentioned in the comment I just added to the associated code)

I removed substitution type 21 and ligatureSubstitutionFormat3. As an alternative, use chainSubstitutionFormat3 to process the sub5 table. Sub5 may exist in liga and ccmp, both of which are processed using ligatureSubstitutionFormat3. I added test code to ensure they work properly.

[Connum]

Thanks for your work! Two more things, then we'll hopefully be ready for review:

1. please add the information for the newly added test fonts to the test/fonts/LICENSE file (alphabetic order)
2. Instead of adding a new liga.js test file, please add the tests for the two new lookup formats to test/featureQuery.js in analog form as the existing tests.

Big thanks again!

[TonyJR]

@Connum Do you have a todo list of the LookupTypes? I want to implement this part of the code, but I am concerned that it may overlap with your work. So, do you have any suggestions?

[Connum]

Great to see someone being interested in contributing! There is an issue with an overview of TODOs: #374
It might not be up to date, so some things might have been implemented meanwhile. You could also search the code base for instances of "not supported" or "not yet supported".

[TonyJR]

Great to see someone being interested in contributing! There is an issue with an overview of TODOs: #374 It might not be up to date, so some things might have been implemented meanwhile. You could also search the code base for instances of "not supported" or "not yet supported".

Thanks for your suggestion. I'll finsh the format 5.1,5.2,5.3 in this PR, please merge them later.

[TonyJR]

@Connum ,

I have added support for 5.1.
For 5.2, I encountered difficulties. I can't find the relevant font, and besides that, I need to read the code related to GlyphClass first. So I'm not sure when I can finish it.
Can you review and merge the current modifications first?

[TonyJR]

By the way, 5.1 and 5.3 are currently only effective in the 'ccmp' table and cannot be recognized in the 'liga' table. I have tested their performance in Figma (halfbuzz+freetype) and they can take effect anywhere. I think this is caused by an error in the 'language tag', and I will describe it in the issues later

[Connum]

I added a few comments for things that need clarification or fixing.

[Connum]

I guess this should be 'DFLT'?

[forresto]

This is the only place in the codebase with 'delf'.

[Connum]

Sorry this made it through the review process, I didn't follow up because it was marked as resolved. Good catch, thanks!

@TonyJR could you comment on this please? I think it should be DFLT for the default script. And for the future, please let reviewers resolve the conversations. :)

[Connum]

Another thing I noticed: The 'ccmp' tag was already present in the requiredThaiFeatures:

opentype.js/test/bidi.js

Line 114 in 6408975

tags: ['liga', 'rlig', 'ccmp']

Should this be present in requiredArabicFeatures and latinFeatures as well?

opentype.js/test/bidi.js

Line 25 in 6408975

tags: ['init', 'medi', 'fina', 'rlig']

opentype.js/test/bidi.js

Line 40 in 6408975

tags: ['liga', 'rlig']

Or is it maybe not needed at all, because the 'ccmp' is always applied? And if so, does the code make sure of that?

This might also relate to your comment #688 (comment) ? I think you forgot to describe the issue with the language tag for us. Could it be related to the wrong 'delf' script tag I hinted at in the review comment?