+
Skip to content

[feaLib] Remove duplicates from class pair pos classes #3804

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

Merged
merged 1 commit into from
Apr 23, 2025

Conversation

khaledhosny
Copy link
Collaborator

AFDKO’s makeotf removes the duplicates and produces the following message:

NOTE: [line     3 char  29] Removing duplicate glyph <A>
NOTE: [line     3 char  29] Removing duplicate glyph <A>

While feaLib fails with:

Traceback (most recent call last):
  File "/fonttools/Lib/fontTools/feaLib/builder.py", line 875, in buildLookups_
    otLookups.append(l.build())
                     ^^^^^^^^^
  File "/fonttools/Lib/fontTools/otlLib/builder.py", line 1460, in build
    builder.addPair(glyphclass1, value1, glyphclass2, value2)
  File "/fonttools/Lib/fontTools/otlLib/builder.py", line 1340, in addPair
    self.classDef1_.add(gc1)
  File "/fonttools/Lib/fontTools/otlLib/builder.py", line 2656, in add
    raise OpenTypeLibError(
fontTools.otlLib.error.OpenTypeLibError: Glyph A is already present in class.

@@ -1471,7 +1471,9 @@ def add_class_pair_pos(self, location, glyphclass1, value1, glyphclass2, value2)
lookup = self.get_lookup_(location, PairPosBuilder)
v1 = self.makeOpenTypeValueRecord(location, value1, pairPosContext=True)
v2 = self.makeOpenTypeValueRecord(location, value2, pairPosContext=True)
lookup.addClassPair(location, glyphclass1, v1, glyphclass2, v2)
cls1 = tuple(sorted(set(glyphclass1)))
Copy link
Member

Choose a reason for hiding this comment

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

these glyphclass1/glyphclass2 are they supposed to be of type set or tuple? The docstring of addClassPair method says "set"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I use set, I get an error down the line that it it is unhashable type. They are used as a dict key.

Copy link
Member

@anthrotype anthrotype left a comment

Choose a reason for hiding this comment

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

LGTM (but see comment/question)

AFDKO’s makeotf removes the duplicates and produces the following
message:

    NOTE: [line     3 char  29] Removing duplicate glyph <A>
    NOTE: [line     3 char  29] Removing duplicate glyph <A>

While feaLib fails with:

    Traceback (most recent call last):
      File "/fonttools/Lib/fontTools/feaLib/builder.py", line 875, in buildLookups_
        otLookups.append(l.build())
                         ^^^^^^^^^
      File "/fonttools/Lib/fontTools/otlLib/builder.py", line 1460, in build
        builder.addPair(glyphclass1, value1, glyphclass2, value2)
      File "/fonttools/Lib/fontTools/otlLib/builder.py", line 1340, in addPair
        self.classDef1_.add(gc1)
      File "/fonttools/Lib/fontTools/otlLib/builder.py", line 2656, in add
        raise OpenTypeLibError(
    fontTools.otlLib.error.OpenTypeLibError: Glyph A is already present in class.
@khaledhosny khaledhosny force-pushed the fealib-duplicates-class-kerning branch from c2486c1 to 95b5747 Compare April 23, 2025 13:07
@khaledhosny khaledhosny merged commit 5ca8a27 into main Apr 23, 2025
11 checks passed
@khaledhosny khaledhosny deleted the fealib-duplicates-class-kerning branch April 23, 2025 13:17
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.

2 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载