Close open corners in UFO

[simoncozens]

Requires fonttools/fonttools#2192 to be merged. Fixes #255 and googlefonts/fontmake#288

[simoncozens]

Of course the tests are going to fail, because I'm living in the future and GitHub Actions isn't.

[simoncozens]

This is causing some problems with fontmake and certain variable fonts. Trying to track it down.

[simoncozens]

The heuristic @anthrotype suggested in googlefonts/fontmake#288 was

a filter pen that iterates over triplets (prev, curr, next) of segments in each contour, computes intersection between previous and next segment, and if intersection falls after the mid-point of the previous segment and before the mid-point of the next segment, then the filter pen should draw with the wrapped pen only the portions of previous and next segments that we want, thus excluding the current segment and surrounding sub-segments that make up the corner shape.

Unfortunately that doesn't deal with "fat" open corners like this one:

where the intersections are at (t1=0.46, t2=0.13) and (t1=0.86,t2=t2=0.53).

I guess one approach would be to reorder things and run RemoveOverlap first, to get rid of this kind of open corner, leaving only the more simple ones. But that doesn't work in the general case as RemoveOverlap does unhelpful things to "outside" corners, putting them into a separate contour:

So we either need to

1. Detect the difference between "inside" and "outside" open corners (inside ones being dealt with by RemoveOverlaps), or
2. Come up with a better heuristic for removing all open corners.

[schriftgestalt]

The open corner processing is only done for outside corners. That means it only does sorting if the previous and next point is on the left of the line segment (as seen from that segment).

[simoncozens]

That means it only does sorting if the previous and next point is on the left of the line segment (as seen from that segment).

I don't understand this.

For the open corner at the bottom, the "previous point" is the start point, right? (Where the 2 is) And the "next point" is the other selected point? They are on different sides of the line segment.

[schriftgestalt]

I might have confused the direction. But both selected nodes are on the same side of the line going through the two points on the bottom (this segment goes from right to left, so the selected points are on the right of that line (when you look RTL))

[simoncozens]

In the diagram above, both nodes are on the same side of the line, so it is an outside corner?

In the diagram below, both nodes are on the same side of the line, but it is an inside corner.

[schriftgestalt]

If both are on the right (not left) side when looking in the direction of the short line segment.

[simoncozens]

Ah, ok, when the short line segment is aligned clockwise. Thanks, I’ll try working with that.

[schriftgestalt]

I use this function:

BOOL GSPointIsLeftOfLine(NSPoint a, NSPoint b, NSPoint aPoint) {
	return ((b.x - a.x) * (aPoint.y - a.y) - (b.y - a.y) * (aPoint.x - a.x)) >= 0;
}

and if both points are NOT left of the line, and the both t vales are below 0.5.

[simoncozens]

(Not my test failure - pip-compile is being too strict and pip is being stupid about resolving dependencies.)

[simoncozens]

(Not my test failure - pip-compile is being too strict and pip is being stupid about resolving dependencies.)

Specifically, the current release of ufolib2 hard-requires fontTools 4.19.0 and we need 4.20.0. We need a new release of ufolib2 to unblock. (Or just stop using pip-compile which is creating too-strict dependency chains.)

[anthrotype]

ufolib2 hard-requires fontTools 4.19.0

You sure? Where did you see that. In setup.cfg, ufoLib2 only seems to require fonttools >= 4.0.0: https://github.com/fonttools/ufoLib2/blob/1201353d31ce463b13ac12cd12253c4d5a18bded/setup.cfg#L29

you should be able to re-pip-compile the requirements with pip-compile -U setup.py (@madig am I right)

stop using pip-compile which is creating too-strict dependency chains

that was to make the test runs more predictable. I'm ok if you want to ditch that and keep all deps open-ended and install latest. It means you may find yourself having to deal with sudden unrelated breakages in the middle of your PR

[simoncozens]

you should be able to re-pip-compile the requirements with pip-compile -U setup.py (@madig am I right)

I already did that in fab5d27.

ufolib2 hard-requires fontTools 4.19.0
You sure? Where did you see that.

Hmm, no, I'm not sure. Something is pulling in 4.19.0 as you can see in the test failure. But pip isn't very good at telling me what it is.

[simoncozens]

Ack, it's the requirements-dev.

[simoncozens]

OK, that failing test now is mine, but I don't know how to fix it. We are adding a ufo2ft filter on export to UFO, but I am not sure how that should be round-tripped.

[madig]

Please update the requirements files with the newest pip-tools to avoid needless diffs.

[madig]

Thanks, I'll try to get to reviewing this this week... out of curiosity, are there use cases for this pen outside a Glyphs context? I wonder if FontLab implements this differently.

[madig]

Seems fine to me.

Is it maybe worth making it work on multiple paths at once (i.e. be able to process all contours of a glyph and output trimmed ones without having to re-instantiate the pen between each)?

[anthrotype]

I wonder if FontLab implements this differently.

oh, does Fontlab also have similarly "open corners"?

[simoncozens]

It doesn't look like fontLab's "loop corners" do the same thing on the outside as Glyphs does.

[anthrotype]

I think self.is_closed should be reset at the end, before a new contour is drawn with the same pen, otherwise it stays True

[anthrotype]

I understand this behaviour is the default in Glyphs.app, however I'm a bit concerned that this filter will then be enabled for all .glyphs sources that are built via fontmake, especially given that the segment-segment intersection code in fonttools bezierTools is quite recent and does not have a complete test suite.
How about we have this behind a (yet-another-I-know) flag? One can argue that the default fontmake behavior so far has been to not erase glyphs-style open corners..

[simoncozens]

One can argue that the default fontmake behavior so far has been to not erase glyphs-style open corners.

Yes, one can. But one can also argue that this is a mistake and counterintuitive. The expected behaviour is that fonts produced by glyphsLib and fonts produced by Glyphs look the same. Currently they don't, which is why we have designers opening issues saying that fontmake is making their fonts look horrible, and why we have to tell people not to use open corners in fonts they submit.

If the problem is that the fontTools code doesn't have a good enough test suite, let's fix that problem, but that's not this problem. We shouldn't need to enable an optional flag to get expected behaviour. The program should do what's expected by default.

[moyogo]

It doesn't look like fontLab's "loop corners" do the same thing on the outside as Glyphs does.

FontLab does this, it's called "looped corners", but it's hidden behind a setting: Font Info > Other Values > Contour Properties > Unfill looped corners.
Checkout https://www.youtube.com/watch?v=iQ3FkgDLVOA
The latest version also has "Suggest distance X" and "Suggest distance Y" values.

[anthrotype]

@moyogo cool, so it works the same way but more user-configurable

[moyogo]

Should the erase open corner filter be merged in ufo2ft instead?

[simoncozens]

If fontlabLib existed, then yes, absolutely it makes sense to put this at the next level in the pipeline. But it doesn't. (Well, it does, it's called babelfont. ;-) So for practical purposes it is only Glyphs which uses open corners.

[moyogo]

I guess it doesn't matter where it is if I want to use it in a UFO workflow, since fontmake requires glyphsLib anyway.

[simoncozens]

Discussed here: googlefonts/fontmake#288 (comment)

[moyogo]

Discussed here: googlefonts/fontmake#288 (comment)

So I think glyphsLib should be responsible for adding the filter to the UFO libs. It should probably be defined inside glyphsLib itself, being a glyphs-specific feature.

So is the Propagate Anchors Filter or the Transformations Filter, yet they are in ufo2ft because it's a useful filter to have in a variety of workflows. There are open corners in Noto Sans Georgian UFO sources for example. FontLab can still export UFOs to be used in fontmake.

I don't mind having filters in glyphsLib, I just don't understand what's the logic for that exception.

[anthrotype]

There are open corners in Noto Sans Georgian UFO sources for example

yeah, but that's because it was designed inside Glyphs.app then converted to UFO. I don't think there's any native UFO editor that allows one to draw/view these so-called "open corners". It's a font-editor-specific feature, a Glyphs.app-ism, hence it lives in glyphsLib. If it were to become something that is more widely used in UFO-based workflows then we may reconsider moving this to ufo2ft proper.

[madig]

@schriftgestalt how does Glyphs.app handle Sofia Sans's kgreenlandic in the DSFix layer?

Moved aside to show the contour:

[simoncozens]

and if both points are NOT left of the line, and the both t vales are below 0.5.

@schriftgestalt, I'm confused by this. In this case:

the t value of the intersection on both line segments is < 0.5, so that's detected as an open corner. OK, that makes sense.

But in this case:

the t value of the intersection on the selected line segment is way > 0.5, but it's still detected as an open corner!

[schriftgestalt]

this is the actual code:

if (((t1 < 0.5 && t2 < 0.5) || t1 < 0.3 || t2 < 0.3) && t1 > 0.0001 && t2 > 0.0001) {

So if one of the edges id smaller than 0.3, it will still do it.

[simoncozens]

Aha! Thank you!