robotools / fontMath

A collection of objects that implement fast font, glyph, etc. math.
MIT License
42 stars 17 forks source link

Fix multiplication of fontinfo slant and italic angles #189

Closed madig closed 3 years ago

madig commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

I defer to @LettError.

LettError commented 4 years ago

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?

https://github.com/robotools/fontMath/commit/13148c362d5376eca18a7ccdaff9d1aa562fb78d

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 commented 4 years ago

@madig Just checking in on this

madig commented 4 years ago

Hi! :wave: need anything from me?

madig commented 3 years ago

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

@benkiel @LettError

LettError commented 3 years ago

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