Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix multiplication of fontinfo slant and italic angles #189

Merged
merged 1 commit into from
Jun 21, 2021

Conversation

madig
Copy link
Contributor

@madig madig commented Apr 16, 2020

A fontinfo's italicAngle and postscriptSlantAngle do not seem to interpolate correctly.

Initially found with this reproducer (the last assert fails):

from fontmake import instantiator
from fontTools.designspaceLib import DesignSpaceDocument
from ufoLib2 import Font

d = DesignSpaceDocument()
d.addAxisDescriptor(name="Weight", tag="wght", minimum=300, default=300, maximum=900)
d.addAxisDescriptor(name="Slant", tag="slnt", minimum=-20, default=0, maximum=0)
d.addSourceDescriptor(location={"Weight": 300, "Slant": 0}, font=Font())
d.addSourceDescriptor(location={"Weight": 500, "Slant": 0}, font=Font())
d.addSourceDescriptor(location={"Weight": 900, "Slant": 0}, font=Font())
d.addSourceDescriptor(location={"Weight": 300, "Slant": -20}, font=Font())
d.addSourceDescriptor(location={"Weight": 500, "Slant": -20}, font=Font())
d.addSourceDescriptor(location={"Weight": 900, "Slant": -20}, font=Font())
d.addInstanceDescriptor(styleName="1", location={"Weight": 400, "Slant": 0})
d.addInstanceDescriptor(styleName="2", location={"Weight": 400, "Slant": -12})
d.addInstanceDescriptor(styleName="3", location={"Weight": 400, "Slant": -20})
d.findDefault()

for s in d.sources:
    s.font.info.italicAngle = s.location["Slant"]

assert [s.font.info.italicAngle for s in d.sources] == [
    0.0,
    0.0,
    0.0,
    -20.0,
    -20.0,
    -20.0,
]

inst = instantiator.Instantiator.from_designspace(d)
for i in d.instances:
    i.font = inst.generate_instance(i)

instance_angles = [i.font.info.italicAngle for i in d.instances]
assert instance_angles == [0, -12, -20], instance_angles

After some digging with @belluzj, we found that test_mul fails on these two attributes if they're non-zero, as if they're skipped. More digging required.

@madig
Copy link
Contributor Author

madig commented Apr 16, 2020

So what's going on seems to be that both italicAngle and postscriptSlantAngle are handled by MathInfo._processMathTwoAngle, which calls factorAngle. If there is no anisotropy going on, it just returns the value that was passed in, when it should linearly interpolate the value.

@typesupply not quite sure what to do here. I'd assume that we want to interpolate the value like any other, but factor in anisotropy?

@typesupply
Copy link
Member

I defer to @LettError.

@LettError
Copy link
Member

LettError commented Apr 16, 2020

Interesting that this never came up before!

Here is a small change that seems to bring the expected results. I don't know where the factorAngle logic came from, I'd have to study it to see how it works. What are the expected results for anisotropic math?

13148c3

Simpler example that doesn't need fontmake.

import math
from fontParts.world import RFont

f = RFont()
f.info.italicAngle = 10
m = f.info.toMathInfo(guidelines=True)

m1 = m - m
r1 = RFont()
m1.extractInfo(r1.info)
assert r1.info.italicAngle == 0

m2 = m + m + m
r2 = RFont()
m2.extractInfo(r2.info)
assert r2.info.italicAngle == 30

m3 = 10 * m
r3 = RFont()
m3.extractInfo(r3.info)
assert r3.info.italicAngle == 100

m4 = m / 20
r4 = RFont()
m4.extractInfo(r4.info)
assert r4.info.italicAngle == 0.5

@benkiel
Copy link
Member

benkiel commented Sep 2, 2020

@madig Just checking in on this

@madig
Copy link
Contributor Author

madig commented Sep 3, 2020

Hi! 👋 need anything from me?

@madig madig force-pushed the slant-angle-multiplication branch from 1901c45 to 0dd3baf Compare June 21, 2021 16:15
@madig madig force-pushed the slant-angle-multiplication branch from 0dd3baf to a726893 Compare June 21, 2021 16:17
@madig
Copy link
Contributor Author

madig commented Jun 21, 2021

I stumbled over this again and eventually remembered that I opened this PR a year ago :D I implemented LettError's suggestion.

@benkiel @LettError

@madig madig marked this pull request as ready for review June 21, 2021 16:18
Copy link
Member

@benkiel benkiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but we should wait for @LettError to weigh in before merging, he's more familiar with the code than I am

@anthrotype anthrotype requested a review from LettError June 21, 2021 17:17
@LettError
Copy link
Member

LGTM. The test values are indeed what one would expect to see.

@benkiel benkiel merged commit ce02271 into robotools:master Jun 21, 2021
@madig madig deleted the slant-angle-multiplication branch June 21, 2021 20:57
benkiel added a commit that referenced this pull request Jun 22, 2021
…t is `False`). Strict means that line segments are not converted to curve segments with (0,0) control points and offcurves stacked on on-curves are not filtered out when extracting. (#235). Last version to support Python 2
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants