Smart component speedup

[simoncozens]

Using pickle was indeed faster than using deepcopy, but it's still slower than shallow-cloning each node. This should speed up the regression tests significantly (and anything that uses smart components).

Also a slightly more helpful error message when a smart component is not interpolatable.

[anthrotype]

regression tests still taking too long...

[anthrotype]

in the other PR #828, the regression tests failed after 111 minutes with a suspicious error related to smart components, which might be relevant for here:

https://github.com/googlefonts/glyphsLib/actions/runs/3410863952/jobs/5674343376

  File "/home/runner/work/glyphsLib/glyphsLib/Lib/glyphsLib/builder/glyph.py", line 164, in to_ufo_glyph
    self.to_ufo_components(ufo_glyph, layer)  # .components
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/glyphsLib/glyphsLib/Lib/glyphsLib/builder/components.py", line 70, in to_ufo_components
    to_ufo_smart_component(self, layer, component, pen)
  File "/home/runner/work/glyphsLib/glyphsLib/Lib/glyphsLib/builder/smart_components.py", line 123, in to_ufo_smart_component
    new_coords = model.interpolateFromMasters(normalized_location, coordinates)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/glyphsLib/glyphsLib/generate/lib/python3.11/site-packages/fontTools/varLib/models.py", line 494, in interpolateFromMasters
    deltas = self.getDeltas(masterValues, round=round)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/glyphsLib/glyphsLib/generate/lib/python3.11/site-packages/fontTools/varLib/models.py", line 457, in getDeltas
    delta -= out[j]
  File "/home/runner/work/glyphsLib/glyphsLib/generate/lib/python3.11/site-packages/fontTools/ttLib/tables/_g_l_y_f.py", line 1779, in __isub__
    assert len(a) == len(other)
AssertionError

[simoncozens]

That suspicious error is why this branch has better error messages.

As for them still taking too long, remember that the tests/tools/generate_regression_test_files.py script is run on main, not on this branch.

[anthrotype]

the tests/tools/generate_regression_test_files.py script is run on main, not on this branch.

ok but what does that have to do with them being slow?

[anthrotype]

oh I see, this PR will speed them up but only after it gets merged.. but how faster? have you tried offline?

[simoncozens]

Wrapping generate_from_glyphs in a try block so it completes even with bad inputs, on my M1 laptop it converts the files in 189s.

[anthrotype]

i suppose the github runners are slower and support less parallelism than your laptop, but I'd be quite surprised if the runtime goes down from 1 hour and a half to a few minutes just because of the changes in this PR..

[simoncozens]

Prepare to be surprised. I'm still waiting for the version on main to finish.

[simoncozens]

With this patch: fontmake -o ufo -g DelaGothic.glyphs finished (with an error) after 9s.
Without this patch: I gave up and hit ctrl-c after 8 minutes.

[anthrotype]

that's impressive but I doubt it has to do with (avoiding the use of) pickle itself, more like the fact that you're now not deepcopying the unneeded attributes, right?

[simoncozens]

The problem is that there is a cyclic link between a GSPath.node and GSNode._parent. Deepcopy doesn't do well copying cyclic dependencies.

[schriftgestalt]

couldn’t that be avoided by implementing __copy__ and __deepcopy__?

[anthrotype]

yes maybe, but I think the purpose here was to make a more selective copy by excluding attributes that aren't needed for the task at hand, am I right @simoncozens? Perhaps the method name .clone() does not make that clear