robotools / fontParts

The replacement for RoboFab
MIT License
132 stars 44 forks source link

kerning.round results in float values #702

Open frankrolf opened 11 months ago

frankrolf commented 11 months ago

I have noticed (with @punchcutter) that kerning.scale() followed by kerning.round() may write floats into the kerning data. These floats are all .0, so there’s no data problem – still, wondering about the “why”.

In the attached project, all kerning values are integers. After doing this, all values are converted to floats:

from fontParts.world import RFont
import sys

f = RFont(sys.argv[-1])

original_kerning = f.kerning.copy()
for key, value in f.kerning.items():
    if isinstance(value, float):
        # in the attached project, all kerning pairs are int
        print('hey, float!', key, value)

f.kerning.scaleBy(2)
f.kerning.round()

for key in original_kerning.keys():
    value_original = original_kerning[key]
    value_scaled = f.kerning[key]
    if isinstance(value_scaled, float):
        print(key)
        print(value_original, value_scaled)
        print()

output is

('public.kern1.LAT_K', 'public.kern2.LAT_O')
-24 -48.0

('public.kern1.LAT_K', 'public.kern2.LAT_Y')
-10 -20.0

('public.kern1.LAT_K', 'public.kern2.LAT_Z')
10 20.0

('public.kern1.LAT_O', 'public.kern2.LAT_E')
-18 -36.0

# (… more lines)

When changing the scale factor to 2.2, only a few pairs (the ones ending up as .0) become floats, the remaining pairs are rounded to int:

('public.kern1.LAT_K', 'public.kern2.LAT_Y')
-10 -22.0

('public.kern1.LAT_K', 'public.kern2.LAT_Z')
10 22.0

('public.kern1.LAT_O', 'public.kern2.LAT_O')
10 22.0

('public.kern1.LAT_O', 'public.kern2.LAT_Y')
-40 -88.0

('public.kern1.LAT_Z', 'public.kern2.LAT_Y')
-10 -22.0

I have double-checked https://github.com/robotools/fontParts/blob/master/Lib/fontParts/base/kerning.py#L93-L115, as well as https://github.com/robotools/fontParts/blob/master/Lib/fontParts/base/normalizers.py#L1094-L1108 – it’s impossible for these to return a float. normalizers.normalizeKerningValue should just pass the value through – I wonder if there’s some comparison happening while setting the values (which would return True for 10.0 == 10)?

kerning_example.ufo.zip

justvanrossum commented 11 months ago

I can confirm the behavior, but looking at the code I don't understand how it's happening.

justvanrossum commented 11 months ago

The reason is here: https://github.com/robotools/defcon/blob/32c956cabe3ca6aaeea69ddeaade5c6d0903ae91/Lib/defcon/objects/base.py#L524 If the new value is equal to the old value, defcon will not write the new value. Since 123.0 == 123 this checks out, and causes the behavior you observe.

frankrolf commented 11 months ago

Thank you Just! :-)

Would it be reasonable to check for the type of the value there? Or, perhaps kerning.round() should just completely replace the whole kerning object instead of re-writing pairs one-by-one?