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

Decompose scaled or rotated components #399

Merged

Conversation

moyogo
Copy link
Collaborator

@moyogo moyogo commented Aug 15, 2020

This adds a filter to decomposed rotated or scaled components. Some TrueType implementations don’t deal well with those.

@madig
Copy link
Collaborator

madig commented Aug 16, 2020

Some TrueType implementations don’t deal well with those.

Which ones? :O

@moyogo moyogo force-pushed the decomposeScaledOrRotatedComponents branch from 2eee156 to 8a00c37 Compare August 17, 2020 04:48
@anthrotype
Copy link
Member

Shouldn't this filter class live in its own separate module?

I think that's what is expectated for the getFilterClass function used by loadFilters to load the filters from the lib.plist:

def getFilterClass(filterName, pkg="ufo2ft.filters"):
"""Given a filter name, import and return the filter class.
By default, filter modules are searched within the ``ufo2ft.filters``
package.
"""
# TODO add support for third-party plugin discovery?
# if filter name is 'Foo Bar', the module should be called 'fooBar'
filterName = filterName.replace(" ", "")
moduleName = filterName[0].lower() + filterName[1:]
module = importlib.import_module(".".join([pkg, moduleName]))
# if filter name is 'Foo Bar', the class should be called 'FooBarFilter'
className = filterName[0].upper() + filterName[1:] + "Filter"
return getattr(module, className)

@anthrotype
Copy link
Member

I believe Denis is referring to those transformations (like flipping horizontally/vertically) that reverse the component's contour direction, and thus may create white gaps if such transformed components overlap with other components. Am I right?

There's code in ufo2ft.util.deepCopyContours that checks for those transforms (a negative determinant is the trigger):

ufo2ft/Lib/ufo2ft/util.py

Lines 192 to 197 in e1c08f6

pen = TransformPen(parent.getPen(), transformation)
# if the transformation has a negative determinant, it will
# reverse the contour direction of the component
xx, xy, yx, yy = transformation[:4]
if xx * yy - xy * yx < 0:
pen = ReverseContourPen(pen)

Perhaps the issue is that by default we only decomposing glyphs that are "mixed" with both contour and components; whereas Denis would like to trigger this for such problematic composite glyphs as well.

There's also a related fontmake issue: googlefonts/fontmake#253 (comment)

@anthrotype
Copy link
Member

I don't think there's anything wrong with merely having a component scaled or rotated, I've never come across an implementation that doesn't render such components properly. It's when they overlap other components and have transforms that flip their direction that you get rendering issues (as exemplified in the linked fontmake issue)

@moyogo
Copy link
Collaborator Author

moyogo commented Aug 17, 2020

Some TrueType implementations don’t deal well with those.

Which ones? :O

Any implementation of the TrueType implementation that doesn’t understand the component flags SCALED_COMPONENT_OFFSET or UNSCALED_COMPONENT_OFFSET for example, or any implementation that expects the component flag WE_HAVE_A_SCALE to apply scaling.

We probably should set more flags from https://docs.microsoft.com/en-us/typography/opentype/spec/glyf#composite-glyph-description.
But even then, there’s still the issue of implementations that are not aware of the SCALED_COMPONENT_OFFSET or UNSCALED_COMPONENT_OFFSET flags, for example if they follow only https://developer.apple.com/fonts/TrueType-Reference-Manual/RM06/Chap6glyf.html which doesn’t mention those flags.

@moyogo moyogo force-pushed the decomposeScaledOrRotatedComponents branch from 8a00c37 to 740279b Compare August 17, 2020 20:31
@moyogo
Copy link
Collaborator Author

moyogo commented Aug 18, 2020

Shouldn't this filter class live in its own separate module?

I moved the filter to its own module.

http://pfaedit.sourceforge.net/Composites/ (pfaedit is the super old name of Fontforge) also documents the issue about how Apple’s and Microsoft’s understanding of scaled components differs.

@khaledhosny
Copy link
Collaborator

I have been using scaled and rotated components for almost 10 years, if there is a problem I would have known by now.

The only issue I had with components was Mac OS X not liking nested components (they get displaced), so for a long time I had code to de-nest them, but I dropped this recently because I can no longer reproduce it, no complaints so far.

@khaledhosny
Copy link
Collaborator

(nested and transformed components, that is it)

@moyogo
Copy link
Collaborator Author

moyogo commented Aug 18, 2020

There are several printers that have issues with nested components. See for example JulietaUla/Montserrat#108 which I suspect is the issue, since Udieresis has a nested component.
Dalton Maag had clients that had the issue a couple of times ; in fact one of the printers from this decade at their office even has the nested component issue (or at least when I was there).
The flattenComponentsFilter can be used to flatten nested components to basic components.

For scaled components, this merge request is for a module that is optional, like the flattenComponentsFilter one is. I have no intention in making fontmake/ufo2ft using it be default.
There are a few workflows where it can be useful:

@anthrotype
Copy link
Member

Yes, makes sense to have such an optional filter available.
Can we name it "Decompose Transformed Components" instead of just "Scaled"?

Note that Glyphs.app has a custom parameter "Keep Transformed Components" which off by default; so by default Glyphs.app will decompose "distorted and mirrored components", unless that parameter is set. See https://glyphsapp.com/blog/new-features-in-glyphs-2-5

@moyogo moyogo force-pushed the decomposeScaledOrRotatedComponents branch from 740279b to b4d4707 Compare August 26, 2020 19:52
@moyogo
Copy link
Collaborator Author

moyogo commented Aug 26, 2020

I’ve renamed it "Decompose Transformed Components".

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.

None yet

4 participants