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

Unexported glyphs being added to mark filtering set #721

Open
simoncozens opened this issue Mar 6, 2023 · 9 comments
Open

Unexported glyphs being added to mark filtering set #721

simoncozens opened this issue Mar 6, 2023 · 9 comments

Comments

@simoncozens
Copy link
Contributor

Sorry, this is my fault. In #720, we grovel the UFO for spacing glyphs and put them into a mark filtering set. Unfortunately, we don't check if they're exporting or not. We should use the glyphset:

            # We only want to filter the spacing marks
            marks = set(self.context.gdefClasses.mark) & set(self.context.glyphSet)
            spacing = []

But I just tried that, and now I have a merge failure in the GDEF table, which I don't really understand, caused by different glyph class definitions in each master; I don't know if this is related or not. (Mark class definitions would make sense, but I'm seeing some glyphs in GDEF in some masters but not in others.)

@anthrotype
Copy link
Member

it might be that different masters have different glyph export status. I think ufo2ft checks for skipExportGlyphs at the designspace.lib level, for that info should be considered global to the VF, not master-specific

kwargs["skipExportGlyphs"] = designSpaceDoc.lib.get("public.skipExportGlyphs", [])

@anthrotype
Copy link
Member

in designspace v5 where there can be more than one VF per .designspace file, ideally we'd source that list from each specific <variable-font>'s lib element (but I guess that's a different issue).

Perhaps glyphsLib doesn't even set that lib key at the designspace.lib level? /cc @madig who added that piece of logic in ufo2ft

@simoncozens
Copy link
Contributor Author

Nah, it's completely my fault. I just did a bit of bisecting and the merge failure only happens after cc02f39, so it's also being caused by #720.

@simoncozens
Copy link
Contributor Author

Oh, urgh. It's because some masters have width==0 for a spacing combining glyph and some masters don't. I knew it was quite a brittle test (but the best we could do), but I didn't expect that failure mode.

Chalk another one up to "treating each master as an independent font".

@anthrotype
Copy link
Member

ouch. how do we fix this one now..

@khaledhosny
Copy link
Collaborator

That was fast. My suggestion, if these are Glyphs sources, is to explicitly check for category and subCategory (which I believe we keep in lib).

@anthrotype
Copy link
Member

My suggestion, if these are Glyphs sources, is to explicitly check for category and subCategory (which I believe we keep in lib).

hm yeah, we could do that but it'd be weird for ufo2ft to explicitly check for the presence of a lib key that's Glyphs' specific..
We could also do the GSUB closure (similarly to how we group glyphs in RTL scripts etc.) to get all glyphs that are mapped (via cmap or GSUB substituions) to Unicode codepoints whose category is "Mc" aka "Spacing Combining Marks".

@anthrotype
Copy link
Member

while we discuss this, maybe we should revert #720 ?

@simoncozens
Copy link
Contributor Author

I'm uneasy about doing a GSUB closure. You may wish to make a mark into a spacing mark for design reasons. Real example: Noto Serif Telugu (until recently) didn't support the stacked -jnya conjunct:
shape
Instead it anchors the nya alongside the first conjunct:
shape
but of course it needs to be spacing so that it doesn't end up underneath the క.

There's nothing in Unicode which says the the -nya conjunct should be spacing or non-spacing; doing a GSUB closure won't help you.

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

3 participants