-
Notifications
You must be signed in to change notification settings - Fork 16
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
._glyph does not exists anymore, go for the new _getGlyphAndOffset() … #52
Conversation
…callback this will not be backwards compatible with older fontTools...
You should not use those internal attrs/methods at all. It was previously wrong, and this PR doesn't fix it. Do not use internal stuff. If you need access to a glyph object from the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @typemytype, I have been bitten by this change in a couple of my own modules ...
Lib/extractor/formats/opentype.py
Outdated
@@ -146,7 +146,7 @@ def extractGlyphPrograms(source, destination): | |||
return | |||
glyph_set = source.getGlyphSet() | |||
for name in glyph_set.keys(): | |||
glyph = glyph_set[name]._glyph | |||
glyph, offset = glyph_set[name]._getGlyphAndOffset() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps better like this?
glyph = glyph_set.glyfTable[name]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, just get the glyf
table from the ttfont. Why even use getGlyphSet at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
glyph_set.glyfTable
should also be private.
And probably you should raise the fonttools dependency in setup to |
Just do this: def extractGlyphPrograms(source, destination):
"""
Extract the TrueType pre-program to the font lib.
"""
if "glyf" not in source:
return
glyph_table = source["glyf"]
for name in glyph_table.keys():
glyph = glyph_table[name]
dest_glyph = destination[name]
if glyph.isComposite():
# Extract composite flags
_extractCompositeFlags(glyph, dest_glyph)
if not hasattr(glyph, "program"):
continue
hash_pen = HashPointPen(dest_glyph.width, destination)
dest_glyph.drawPoints(hash_pen)
lib = dest_glyph.lib[TRUETYPE_INSTRUCTIONS_KEY] = {
"formatVersion": "1",
"id": hash_pen.hash,
}
lib["assembly"] = _byteCodeToTtxAssembly(glyph.program)
|
I was probably under the impression that the glyphset was the preferred way of getting at the glyphs because |
With @justvanrossum’s suggestion, the dependency can of course stay as it was. |
thanks @justvanrossum |
But using attrs starting with underscores should trigger all alarm bells...
*) And since recently (related to the refactoring that triggered this PR) it supports |
…callback
this will not be backwards compatible with older fontTools...
@jenskutilek