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

[glyf] Support cubic curves #2988

Merged
merged 34 commits into from
Feb 22, 2023
Merged

[glyf] Support cubic curves #2988

merged 34 commits into from
Feb 22, 2023

Conversation

behdad
Copy link
Member

@behdad behdad commented Feb 7, 2023

@behdad
Copy link
Member Author

behdad commented Feb 7, 2023

Can be used with this patch to ufo2ft:

diff --git a/Lib/ufo2ft/preProcessor.py b/Lib/ufo2ft/preProcessor.py
index 1543916a..64c8f0d1 100644
--- a/Lib/ufo2ft/preProcessor.py
+++ b/Lib/ufo2ft/preProcessor.py
@@ -330,13 +330,13 @@ class TTFInterpolatablePreProcessor:
             for func in funcs:
                 func(ufo, glyphSet)
 
-        fonts_to_quadratic(
-            self.glyphSets,
-            max_err=self._conversionErrors,
-            reverse_direction=self._reverseDirection,
-            dump_stats=True,
-            remember_curve_type=self._rememberCurveType and self.inplace,
-        )
+        #fonts_to_quadratic(
+        #    self.glyphSets,
+        #    max_err=self._conversionErrors,
+        #    reverse_direction=self._reverseDirection,
+        #    dump_stats=True,
+        #    remember_curve_type=self._rememberCurveType and self.inplace,
+        #)
 
         # TrueType fonts cannot mix contours and components, so pick out all glyphs
         # that have contours (`bool(len(g)) == True`) and decompose their

tested with the -o=variable output.

With NotoSansArabic-VF, I see 10% size improvement.

@behdad behdad marked this pull request as draft February 7, 2023 23:52
@behdad behdad force-pushed the cubic-glyf branch 2 times, most recently from 7bf6d66 to 1e26af3 Compare February 8, 2023 00:07
@behdad
Copy link
Member Author

behdad commented Feb 8, 2023

See #2989
Currently I'm passing all offcurve point sequences to curveTo, which will interpret them as a "super-Bezier". Not what we want.

@behdad behdad force-pushed the cubic-glyf branch 6 times, most recently from d804877 to 36ad9ce Compare February 8, 2023 01:22
@behdad
Copy link
Member Author

behdad commented Feb 8, 2023

HarfBuzz counterpart: harfbuzz/harfbuzz#4107

@behdad behdad force-pushed the cubic-glyf branch 3 times, most recently from 8c90285 to d036ec5 Compare February 8, 2023 21:35
@behdad
Copy link
Member Author

behdad commented Feb 8, 2023

I've started a spec at:
https://github.com/harfbuzz/boring-expansion-spec/blob/main/glyf1-cubicOutlines.md

The full implementation handling all corner-cases needs more work (and is quite cumbersome). But I like feedback and whether this can move forward.

I like to land the HarfBuzz patch and ship it in HB 7 soon, even if it only handles simple cases.

@behdad behdad force-pushed the cubic-glyf branch 9 times, most recently from 2e31f7c to 919a551 Compare February 21, 2023 00:10

Verified

This commit was signed with the committer’s verified signature. The key has expired.
dtzWill Will Dietz
@behdad
Copy link
Member Author

behdad commented Feb 22, 2023

I also think the qu2cu.cli should have a flag to set Qu2CuPen all_cubic=True if one wants to force the output to be only cubic (could be useful for testing purposes)

I'll add.

cu2qu.cli can also gain an inverse of all_quadratic when I add the support to ufo.py...

Verified

This commit was signed with the committer’s verified signature. The key has expired.
dtzWill Will Dietz
@behdad
Copy link
Member Author

behdad commented Feb 22, 2023

how much work would adding support for all_quadratic=False be for cu2qu.ufo.fonts_to_quadratic as well? we can also file an issue and do later if/when needed

Done. Can you look into testing it?

@anthrotype
Copy link
Member

both curves_to_quadratic (with all_quadratic=False) and quadratic_to_curves (with all_cubic=False) return list of tuples of either length three (for quads) or lenght 4 (for cubics).. so the caller (be it a pen or the cu2qu.ufo module which also would like to know if it did actually modify a given curve segment) should be able use the lengths of the returned tuples to determine whether or not the function has actually done anything to the input

@anthrotype
Copy link
Member

Can you look into testing it?

yes

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@behdad
Copy link
Member Author

behdad commented Feb 22, 2023

cu2qu.cli can also gain an inverse of all_quadratic when I add the support to ufo.py...

What should I call that argument? --mixed? Umm.

@anthrotype
Copy link
Member

--mixed sounds good

@behdad
Copy link
Member Author

behdad commented Feb 22, 2023

I don't know why test_stats is failing on two bots only.

@behdad
Copy link
Member Author

behdad commented Feb 22, 2023

Nevermind. They are somehow stuck on old code. No idea why. I already fixed the code.

@behdad behdad force-pushed the cubic-glyf branch 2 times, most recently from 7281946 to 5c36bd6 Compare February 22, 2023 18:00
@behdad
Copy link
Member Author

behdad commented Feb 22, 2023

I think this is good to merge now.

@behdad behdad merged commit 4ffb9c7 into main Feb 22, 2023
@behdad behdad deleted the cubic-glyf branch February 22, 2023 18:45
@behdad
Copy link
Member Author

behdad commented Feb 22, 2023

Next step would be ufo2ft / fontmake support...

@anthrotype
Copy link
Member

ufo2ft / fontmake support...

basically we just need a way to pass down all_quadratic=False to cu2qu fonts_to_quadratic (to get the mixed quad+cubic curves)

@behdad
Copy link
Member Author

behdad commented Feb 23, 2023

ufo2ft / fontmake support...

basically we just need a way to pass down all_quadratic=False to cu2qu fonts_to_quadratic (to get the mixed quad+cubic curves)

Correct..

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

2 participants