Skip to content
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

font.renameGlyph() does not rename components #97

Open
khaledhosny opened this issue Sep 3, 2020 · 8 comments
Open

font.renameGlyph() does not rename components #97

khaledhosny opened this issue Sep 3, 2020 · 8 comments

Comments

@khaledhosny
Copy link
Collaborator

I’m not sure if this is intentional, but font.renameGlyph() leave components with baseGlyph using the old name.

@madig
Copy link
Collaborator

madig commented Sep 3, 2020

I remember this also being an issue with TruFont/defcon but I can't remember.

@anthrotype
Copy link
Member

anthrotype commented Sep 9, 2020

yes I think we should support renaming components as well.

glifLib's GlyphSet has a getComponentReferences method that returns a dict keyed by composite glyph names pointing to lists of base component names (empty lists for simple glyphs).
(defcon exposes that as componentReferences property for Layer and Font classes so we may wish to do the same in ufoLib2)
These component references are basically a directed graph which we can reverse to obtain for each component glyph a list of composite glyphs that reference it. Then we can rename not only the single glyph but also, if it's used as a component, its Component.baseGlyph attribute for all the composite glyphs that references it.

@anthrotype
Copy link
Member

a directed graph which we can reverse

by traversing the graph while building a new one with edges reversed

def reverse_graph(g):
    rev = {}
    for v in g:
        for e in g[v]:
            rev.setdefault(e, []).append(v)
    return rev

@anthrotype
Copy link
Member

and we should probably cache these so that we don't have to rebuild every time we rename one glyph

@madig
Copy link
Collaborator

madig commented Sep 9, 2020

Which would mean that now we have to manage the cache. I think we're veering dangereously close to defcon territory here.

@anthrotype
Copy link
Member

Hm yeah
How about a new renameGlyphs method that takes a mapping from old to new names and does the renaming in batch so that we compute the component graph only once?

@khaledhosny
Copy link
Collaborator Author

khaledhosny commented Sep 9, 2020

How about a new renameGlyphs method that takes a mapping from old to new names and does the renaming in batch so that we compute the component graph only once?

That would be best. I currently do this (names is a dict mapping old to new names):

for name in list(font.keys()):
    new = names.get(name)
    if new and new != name:
        font.renameGlyph(name, new)

for glyph in font:
    for component in glyph.components:
        name = component.baseGlyph
        new = names.get(name)
        if new and new != name:
            component.baseGlyph = new

@NightFurySL2001
Copy link

This would certainly be helpful.

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

No branches or pull requests

4 participants