googlefonts / glyphsLib

A bridge from Glyphs source files (.glyphs) to UFOs
Apache License 2.0
178 stars 51 forks source link

No width linking of brace layers #985

Closed schriftgestalt closed 2 months ago

schriftgestalt commented 4 months ago

it was getting it form the master they are attached too and that is supposed to have a different width

anthrotype commented 4 months ago

it's not clear to me what this is doing (or no longer doing), can you please clarify?

schriftgestalt commented 4 months ago

This is mostly a hot fix for a specific project. Brace layer almost always have a different width than the master layers they are attached too. Linking to that width will mess things up. There might be some extra layer that might benefit from linking, so this needs more testing. Until then, this is a draft.

belluzj commented 2 months ago

I rebased this branch on top of main and will add a test file that has a bad case and gets solved correctly by this branch. I think the required scenario is this (font with Weight and ROUND axes):

Previously, the advance width of the brace layer {500, 1} would be forcibly taken from the metricsSource of the master layer Regular ROUND (400, 1) to which the brace layer {500, 1} was attached, and so it would get 100 instead of 200.

With this patch, the advance width of the layer {500, 1} will not be changed, because the the layer {500, 1} does not define itself a metricsSource.

@schriftgestalt is that an accurate description of what this patch does?

schriftgestalt commented 2 months ago

That is correct.

belluzj commented 2 months ago

@anthrotype @simoncozens This is ready for review. I added a test, and I checked locally that the test was failing before the patch (the glyph had width == 100) and is green after (width == 200 as specified in the file).