-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add translate anchors filter #514
base: main
Are you sure you want to change the base?
Conversation
class OptimizeAnchorsFilter(TransformationsFilter): | ||
def set_context(self, font, glyphSet): | ||
# Skip over transformations filter to base filter | ||
return super(TransformationsFilter, self).set_context(font, glyphSet) |
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.
better to explicitly call the BaseFilter.set_context(self, font, glyphSet)
since that is what you are after.
Note to self: the warn should be a debug |
We definitely should not assume that by default. It's legit to leave marks to have correction placement for a bunch of base glyphs and hence not require attachment. |
In general, moving glyphs around like that is not something a build tool should do by default; and this particular one feels rather fragile to me. |
This does nothing by default. It’s a filter that’s activated via a UFO’s lib, which can also include/exclude specific glyphs. |
I understand. Thanks. |
80629c2
to
626701a
Compare
return False | ||
# Also skip over marks which are deliberately positioned over the | ||
# previous glyphs | ||
if glyph.getBounds().xMax < 0: |
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.
as a heuristic as to guess whether a mark was "deliberately positioned over the previous glyph", I think this is a bit too fragile. It's conceivable that the xMax still be >= 0 while the mark being intentionally designed to fall on top of another (e.g. narrow) base glyph
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.
given this is turned off by default and opt-in only, and given one can filter the glyphs to include/exclude if one wishes, I think it's ok to merge this in its current state.
We should probably update TransformationsFilter at some point so it updates composite glyphs using transformed glyphs as components. Possibly with a new flag that could be enabled in OptimizeAnchorsFilter. |
This is another experimental filter.
Mark glyphs with anchor attachment points can kind of be positioned anywhere; they're going to be repositioned when the mark attachment is done. (There's an assumption here that you probably need to test - what if a mark glyph "gets through" without being attached to anything?)
Because of this, we can translate the whole glyph such that the position of their first attaching anchor (
_whatever
) is (0,0). If a lot of anchors end up being zero, they all end up sharing a very small Anchor subtable. This naturally leads to big space savings.