googlefonts / glyphsLib

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

Issue with components from intermediate layers #954

Closed anthrotype closed 3 months ago

anthrotype commented 7 months ago

If a font contains a mixed glyph (with outlines and components) that should get decomposed at build time, and any of the components in turn reference a glyph that has intermediate ("brace") layers, currently the mixed glyph has to have the same set of intermediate layers as its components' base glyphs, otherwise the latter are ignored as the mixed glyph is decomposed and thus the originally mixed now decomposed glyph will not display as expected (i.e. with the expected intermediate layers) in the generated font. Glyphs.app itself does not impose this, and the mixed-composite glyph needs not replicate the intermediate layers of the component glyphs.

The following test file, IntermediateLayer.glyphs.zip, contains glyph "i" which is composed of "idotless" and an outline for the i's dot. The "idotless" glyph has an additional intermediate layer (drawn as a triangle to make it clear if the intermediate is working or not when rendered). If the "{700}" intermediate layer is removed from the glyph "i", the font compiled with fontmake (as well as fontc) will render the glyph "i" incorrectly at wght=700, the stem of the i rendered as a rectangle instead of a triangle (like in idotless at wght=700).

Screenshot 2023-11-08 at 13 35 01 Screenshot 2023-11-08 at 13 35 06
anthrotype commented 7 months ago

I noticed this issue while working on #859 where @RosaWagner's test font similarly uses a _part.line component in a bunch of ABCDE etc. glyphs which also contain regular outlines thus should be decomposed. The workaround there was to add all the intermediate layers to the composite glyphs (manually one by one..)

schriftgestalt commented 7 months ago

Adding the extra intermediate layer to the composites what needed in Glyphs 2 but in Glyphs 3 it is not any more.

anthrotype commented 7 months ago

Could you explain how this is supposed to work? For each glyph that has a mixture of outlines and components, for each of the components, if the component's base glyph has any intermediate layers that the parent glyph has not, then a new intermediate layer is added to the parent glyph containing decomposed outlines interpolated at that intermediate layer's location?

schriftgestalt commented 7 months ago

The short explanation is: Look at all components, if the base glyphs has brace layers. If so, make a as many copies of the layer for each brace location (if not already present). Then re-interpolate the paths and then decompose the components (getting the outlines from the base glyphs brace layers).

RosaWagner commented 7 months ago

Ah came here to report the same. The issue is blocking the onboarding of the brand fonts by Reddit.

anthrotype commented 7 months ago

this is going to take some time to implement, it's non trivial fix, I suggest you create the intermediate layers in the sources for all the composite glyphs that use a component that in turn has some intermediate layers

bghryct commented 7 months ago

we're running into the same issue with https://github.com/Afrotype/danfobase/issues/2

simoncozens commented 5 months ago

I've just hit this with my grand Get Rid Of SemiBold In Noto plan, but from the other way around: A glyph with an intermediate layer uses a component which does not have such a layer will not compile because fontmake can't find a layer for the component. So I'm up for implementing it.

simoncozens commented 5 months ago

Now I can't actually reproduce your bug. In your glyphs file, i has the intermediate layer, but iacute does not. So according to your description, iacute should "render incorrectly at wght=700, the stem of the i rendered as a rectangle instead of a triangle". But they are behaving identically:

Screenshot 2024-01-10 at 13 58 09
anthrotype commented 5 months ago

Now I can't actually reproduce your bug.

Sorry my fault, I should have been clearer. In the original comment I wrote

If the "{700}" intermediate layer is removed from the glyph "i", ...

Basically the .glyphs file in the uploaded archive works fine, exactly because both the composite glyph (i) and the glyph used as component (idotless) have the same set of intermediates; you actually need to modify the file and remove the {700} layer from "i", save the file and the compile with fontmake, to be able to trigger the issue.

anthrotype commented 5 months ago

This one below will show the issue when compiled with fontmake:

IntermediateLayer-bad.glyphs.zip

When exported directly from Glyphs.app it works just like the original .glyphs file which contained explicit matching intermediate layers

yanone commented 5 months ago

I have GlyphsApp-native code for this. Shall I attempt a PR?

Edit: Link pointed to wrong file. Corrected it.

anthrotype commented 5 months ago

thanks but we don't want to expand the intermediate layers to full masters like that script seems to be doing. We only want to sync composites' and respective components' intermediate layers. #971 was one side of the problem, we need to do the second part. Maybe @simoncozens will be able to tackle this?

anthrotype commented 5 months ago

@yanone I can now see the correct file you linked to. The tricky part was the 'reinterpolate' part which Simon addressed in #971 already.

yanone commented 5 months ago

The tricky part was the 'reinterpolate' part which Simon addressed in https://github.com/googlefonts/glyphsLib/pull/971 already.

What do you mean exactly with "reinterpolate part"?

The outlines of my font interpolate correctly, I think. My problem is that the composite glyphs show no adjusted width at those designspace locations.

anthrotype commented 5 months ago

What do you mean exactly with "reinterpolate part"?

when inserting a new brace layer we need to interpolate the new layer at that location

anthrotype commented 3 months ago

I will soon have a fix for this in https://github.com/googlefonts/ufo2ft/pull/826. Once that's merged and released, we can revert the changes in https://github.com/googlefonts/glyphsLib/pull/971, which will no longer be needed since ufo2ft will take care of "aligning", so to speak, the composite glyphs and referenced components intermediate locations as needed (i.e. only when the composite glyphs are actually going to be decomposed). Right now, glyphsLib is unconditonally interpolating intermediate layers for glyphs used as components in composite glyphs that have additional masters, not present in the components' glyphs. But that may lead to unnecessary additional delta sets in gvar if these composite glyphs end up not being decomposed any more at compile time.

anthrotype commented 3 months ago

This should be now fixed for good by fontmake 3.9.0 https://github.com/googlefonts/fontmake/releases/tag/v3.9.0

please test and let me know if that's not the case!