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

TypeError from _intSecAtan on previously valid arguments (valid before 4.40.0) #3287

Closed
stenson opened this issue Oct 5, 2023 · 4 comments · Fixed by #3288
Closed

TypeError from _intSecAtan on previously valid arguments (valid before 4.40.0) #3287

stenson opened this issue Oct 5, 2023 · 4 comments · Fixed by #3288

Comments

@stenson
Copy link

stenson commented Oct 5, 2023

Hi fontTools — long time fan, first time issue reporter — apologies if this has already been reported, but I've been using fontPen’s FlattenPen in coordination with ttf variable fonts for a number of years now with no issue, but when I recently updated a fontTools install (as a requirement of a fontmake install), I started getting this error whenever I tried to use the FlattenPen on glyphs with quadratic curves.

After some investigating, I narrowed the issue down to FlattenPen calling calcQuadraticArcLength and then, in turn, that function calling _intSecAtan — fontTools 4.39.0 works fine for all of my use cases, but 4.40.0 introduces the issue.

Here's an example traceback, with a minimal example (a direct call to calcQuadraticArcLength):

Traceback (most recent call last):
  File "/Users/robstenson/Type/flattentest/test_minimal.py", line 7, in <module>
    print(calcQuadraticArcLength((210, 333), (289, 333), (326.5, 290.5)))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "Lib/fontTools/misc/bezierTools.py", line 183, in fontTools.misc.bezierTools.calcQuadraticArcLength
  File "Lib/fontTools/misc/bezierTools.py", line 233, in fontTools.misc.bezierTools.calcQuadraticArcLengthC
  File "Lib/fontTools/misc/bezierTools.py", line 148, in fontTools.misc.bezierTools._intSecAtan
TypeError: must be real number, not complex

Not sure if it's of any value, but I put together a small repo to demonstrate the issue here: https://github.com/stenson/flattentest. My main issue is demonstrated in test.py, with this code snippet:

from fontTools.ttLib import TTFont
from fontTools.pens.recordingPen import RecordingPen
from fontPens.flattenPen import FlattenPen

glyph = TTFont("MutatorSans.ttf").getGlyphSet()["B"]

rp = RecordingPen()
fp = FlattenPen(rp, approximateSegmentLength=5)
glyph.draw(fp)

As far as I know, I'm not doing anything out of the ordinary here, but I get the TypeError with all recent versions of fontTools.

@stenson stenson changed the title TypeError from _intSecAtan on previously valid arguments (<=4.39.0) TypeError from _intSecAtan on previously valid arguments (valid before 4.40.0) Oct 6, 2023
@anthrotype
Copy link
Member

Seems like a regression, thanks for the report, I will take a look at it today

@justvanrossum
Copy link
Collaborator

I wonder if it relates to #3121. The failing code is optionally optimized by cython, but has otherwise not been changed in a while.

@anthrotype
Copy link
Member

argh.. yes seems to be related to cython, again..(6012643)

@anthrotype
Copy link
Member

actually in this case cython might be right.. the value is wrongly typed, it should have been typed as double, not as complex:

diff --git a/Lib/fontTools/misc/bezierTools.py b/Lib/fontTools/misc/bezierTools.py
index 7772a4bf8..21ab0a5d0 100644
--- a/Lib/fontTools/misc/bezierTools.py
+++ b/Lib/fontTools/misc/bezierTools.py
@@ -141,7 +141,7 @@ def _dot(v1, v2):
 @cython.cfunc
 @cython.inline
 @cython.returns(cython.double)
-@cython.locals(x=cython.complex)
+@cython.locals(x=cython.double)
 def _intSecAtan(x):
     # In : sympy.integrate(sp.sec(sp.atan(x)))
     # Out: x*sqrt(x**2 + 1)/2 + asinh(x)/2

this fixes it.

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