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

Flattening components does not handle transformed components well #442

Closed
khaledhosny opened this issue Dec 15, 2020 · 4 comments · Fixed by #450
Closed

Flattening components does not handle transformed components well #442

khaledhosny opened this issue Dec 15, 2020 · 4 comments · Fixed by #450

Comments

@khaledhosny
Copy link
Collaborator

In Test.glyphs I have many rotated, translated, or flipped components, but flattening components seems to not handle all of them correctly.

In the Test-Regular.ttf (built with fontmake -f), many components are mis positioned.

Below bad font (built with -f fontmake option) and above the good font (built without the option):
image

@khaledhosny
Copy link
Collaborator Author

(These are activated with numr feature).

@moyogo
Copy link
Collaborator

moyogo commented Dec 15, 2020

The issue is :

tr = tr.transform(component.transformation)

Applying scale transforms to translation transforms breaks the translations.

We should do something along those lines:

@@ -44,7 +44,9 @@ def _flattenComponent(glyphSet, component):
     for nested in glyph.components:
         flattened_components = _flattenComponent(glyphSet, nested)
         for i, (_, tr) in enumerate(flattened_components):
-            tr = tr.transform(component.transformation)
-            flattened_components[i] = (flattened_components[i][0], tr)
+            flat_tr = Transform()
+            flat_tr = flat_tr.translate(*component.transformation[4:]).translate(*tr[4:])
+            flat_tr = flat_tr.transform(Transform(*component.transformation[:4])).transform(Transform(*tr[:4]))
+            flattened_components[i] = (flattened_components[i][0], flat_tr)
         all_flattened_components.extend(flattened_components)
     return all_flattened_components

@khaledhosny
Copy link
Collaborator Author

This seems to work for me:

diff --git a/Lib/ufo2ft/filters/flattenComponents.py b/Lib/ufo2ft/filters/flattenComponents.py
index aa234b6..72ebdc0 100644
--- a/Lib/ufo2ft/filters/flattenComponents.py
+++ b/Lib/ufo2ft/filters/flattenComponents.py
@@ -43,8 +43,10 @@ def _flattenComponent(glyphSet, component):
     all_flattened_components = []
     for nested in glyph.components:
         flattened_components = _flattenComponent(glyphSet, nested)
-        for i, (_, tr) in enumerate(flattened_components):
-            tr = tr.transform(component.transformation)
-            flattened_components[i] = (flattened_components[i][0], tr)
+        for i, (name, tr) in enumerate(flattened_components):
+            flat_tr = Transform(*component.transformation)
+            flat_tr = flat_tr.translate(tr.dx, tr.dy)
+            flat_tr = flat_tr.transform((tr.xx, tr.xy, tr.yx, tr.yy, 0, 0))
+            flattened_components[i] = (name, flat_tr)
         all_flattened_components.extend(flattened_components)
     return all_flattened_components

@arrowtype
Copy link

arrowtype commented Dec 17, 2020

Not sure it’s useful to say, but I’ve run into problems here, as well.

image

I can’t easily offer a simple reproduction case, but this the result of using decomposeTransformedComponents and flattenComponents at https://github.com/arrowtype/recursive/tree/b61385b2e6d40e4684fcc042957ec468863a4e94.

Related issues:

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 a pull request may close this issue.

3 participants