Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open Corners allowed in Glyphs source? #288

Closed
punchcutter opened this issue Apr 11, 2017 · 36 comments
Closed

Open Corners allowed in Glyphs source? #288

punchcutter opened this issue Apr 11, 2017 · 36 comments

Comments

@punchcutter
Copy link

I was pretty sure I didn't have problems with open corners before, but right now I have some Glyphs sources with open corners in the outlines and the fonts generated with fontmake don't take care of the corners properly. I'm not sure if I broke something somewhere or is it just that open corners aren't supported at all?

@moyogo
Copy link
Collaborator

moyogo commented Apr 11, 2017

Overlaps in general are removed by default unless fontmake is called with --keep-overlaps. What kind of “open corner” are you talking about?

@anthrotype
Copy link
Member

No, they are not supported. We will need to figure out how these "open corners" are stored in the .glyphs source and derive the closed path from them by finding the intersection.

@anthrotype
Copy link
Member

sorry, maybe I'm confusing this with something else I saw at some fontlab VI presentation..

@punchcutter
Copy link
Author

I'm talking about in Glyphs you can select points and contextual menu Open Corner. It's a nice feature in some situations.

@moyogo
Copy link
Collaborator

moyogo commented Apr 11, 2017

Open corners can be inner or outer shapes. Like @anthrotype says, fontmake currently doesn’t support outer open corners. But inner open corners are just regular overlaps.

@punchcutter
Copy link
Author

Yes, that's what I'm talking about. The outer kind. In this case they just turn into little triangles on the outside:

opencorners

@anthrotype
Copy link
Member

indeed, removing the overlaps means making a union of all the contours in the glyph; if you open a corner and it falls outside, then it's not merged. I can't see another way to handle them.
How does Glyphs.app treat those when you remove overlap?

@moyogo
Copy link
Collaborator

moyogo commented Apr 11, 2017

@punchcutter These don’t translate to open corners that need to be removed in UFO.
@anthrotype I guess glyphsLib could add a key to the UFO to indicate to ufo2ft to delete the outer open corners.

Glyphs.app just removes the outer open corners when removing overlaps but only when they are in the right direction and their size is below a threshold relative to the neighbouring outline segments.

@anthrotype
Copy link
Member

Hm, I don't know, I'll have to play with it to understand what it does.
At first glance, this seems to go against the usual fill-in rules that clipper library uses to do the union of the paths.

@punchcutter
Copy link
Author

Looks like we need something like GSPathPen as shown here: https://forum.glyphsapp.com/t/how-to-detect-an-outside-open-corner/4117/11

For now I can just run a script in Glyphs to remove all the corners, but it would be nice to not alter the source files.

@anthrotype
Copy link
Member

anthrotype commented Apr 18, 2018

I think this way of clipping is at odds with the conventional non-zero or even-odd winding rules, and is too Glyphs.app specific.
The last time I tried to play with it in Glyphs.app, I remember that it seems to determine the inside/outside-ness of such self-intersecting, outward-going paths by checking whether the intersections fall within some threshold (t=0.5 or close to that) on both segments, and the two intersecting segments must are separated by only one intermediate segment, and this middle segment must be a line. These kind of rules are completely arbitrary from my point of view. No other font editor or clipping library works like that to my knowledge.
Sure, one could make a custom ufo2ft filter that simulates that. Though I don't think this feature will be implemented any time soon.
I would prefer if the noto sources be pruned of these from within Glyphs.app and its use discouraged for better portability of the outlines.

/cc @marekjez86

@belluzj
Copy link
Collaborator

belluzj commented Apr 18, 2018

Georg has explained the rule here: googlefonts/glyphsLib#255 (comment)

@marekjez86
Copy link
Contributor

@anthrotype : OK. we'll change the sources. Thanks for the feedback

@moyogo
Copy link
Collaborator

moyogo commented Apr 21, 2018

Would it make me sense for glyphsLib to either remove these outer overlaps or add a filter that does before generating an OTF?

@khaledhosny
Copy link
Collaborator

Apparently I’m incapable of figuring out how to detect these outer open corners. Anyone has a code snippet that can be used in e.g. a ufo2ft filter to remove these?

@punchcutter
Copy link
Author

Georg posted something like this a long time ago and I've used it to remove the open corners before running fontmake.

from GlyphsApp import GSPathPen, currentFont
font = currentFont()

for g in font.glyphs:
    for layer in g.layers:
        paths = [p for p in layer.paths]
        layer.paths = []
        for path in paths:
            pen = GSPathPen.alloc().init()
            path.drawInPen_(pen)
            path = pen.layer().paths
            path = path[0]
            layer.addPath_(path)
            layer.cleanUpPaths()

@khaledhosny
Copy link
Collaborator

Thanks.

IIUC, this has to be run inside Glyphs app and depends on a Glyphs feature, but I’m looking for something that can be run by fontmake at build time to keep the sources unchanged.

@gorjious
Copy link

gorjious commented Apr 7, 2020

I am also running into this issue of outside open corners remaining as triangles which is making testing difficult. Outside corners can be just as important as inside corners for maintaining the flexibility of individual line segments.

open_corners_issue

L: Glyphs R: FontGoggles

@chrissimpkins

@anthrotype
Copy link
Member

anthrotype commented Apr 7, 2020

I think what we need here is 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.

Then we use this filterPen in a ufo2ft preProcessor filter that we run only when building from .glyphs sources (where it is expected). For UFO workflows one could enable the filter manually by setting the relevant filter lib key.

@anthrotype
Copy link
Member

although I am not sure whether this open corner trimming should do that unconditionally or only for those corners that lie outside of a contour (as opposed to regular, inner overlaps), nor I know how to detect the latter situation in a reliable efficient way.

@simoncozens
Copy link
Contributor

simoncozens commented Feb 17, 2021

If I'm allowed to use beziers.py, I have a neat solution to this.

from beziers.path.representations.fontparts import FontParts
from beziers.path.representations.Segment import SegmentRepresentation

class EraseOpenCornersFilter(BaseFilter):
    def filter(self, glyph):
        if not len(glyph):
            return False

        paths = FontParts.fromFontpartsGlyph(glyph)
        altered = False
        for p in paths:
            p.closed = True
            segs = p.asSegments()
            newsegs = []
            for ix, seg in enumerate(segs):
                prevseg = segs[ix - 1]
                nextIx = (ix + 1) % len(segs)
                intersection = segs[ix - 1].intersections(segs[nextIx])
                if intersection and (
                    intersection[0].t1 > 0.5 and intersection[0].t2 < 0.5
                ):
                    (segs[ix - 1], _) = segs[ix - 1].splitAtTime(intersection[0].t1)
                    if newsegs:
                        newsegs[-1] = segs[ix - 1]
                    (_, segs[nextIx]) = segs[nextIx].splitAtTime(intersection[0].t2)
                    altered = True
                    continue
                newsegs.append(seg)
            p.activeRepresentation = SegmentRepresentation(p, newsegs)
        if altered:
            glyph.clearContours()
            for p in paths:
                FontParts.drawToFontpartsGlyph(glyph, p)
        return altered

(Yes, it says fontParts but it works for ufolib and defcon objects too.)

I have no idea how I would do all the intersection testing and curve splitting without a beziers library.

@anthrotype
Copy link
Member

nice!
in this case, what do we need beziers.py for specifically? I think fontTools.misc.bezierTools should contain all that's needed to compute intersections or splitting beziers. Ideally a generic segment pen that only depends on fonttools would be preferable. Unless I hear a more compelling reason to add an additional dependency.

By the way, you should already be able to load that custom filter without needing to modify the core ufo2ft library. We designed the filters API to allow dynamically loading such custom filter classes from external modules. Yeah, documentation..

@simoncozens
Copy link
Contributor

Ok, will look at bezierTools tomorrow!

@anthrotype
Copy link
Member

line-line intersection is trivial; curve-to-line fonttools can do it (it's a matter of translating/rotating the curve and line such that the line is reduced to the x or the y axes then solve the bezier polynomial to get the ts for the intersections); curve-to-curve maybe we don't have it in fonttools but we can add something

@simoncozens
Copy link
Contributor

By the way, you should already be able to load that custom filter without needing to modify the core ufo2ft library.

I'm trying to work out where to plumb this into the toolchain. If we want this filter to fire on all UFOs generated from Glyphs sources, it should probably be added to the lib on output in glyphsLib.builder somewhere.

If we don't want that, and we want fontmake -g to work without having to stop and hand-hack the UFOs in the middle, then we also need a way for fontmake to push arbitrary filters to ufo2ft.

In fact, I should have picked this up when I added flattenComponents. Rather than Yet Another Kwarg on saveOTFs and _build_interpolatable_masters and ufo2ft.compileTTF and ufo2ft.TTFInterpolatablePreProcessor etc.:

        removeOverlaps=removeOverlaps,
        flattenComponents=flattenComponents,
        reverseDirection=reverseDirection,
        eraseOpenCorners=eraseOpenCorners

it would be a whole lot cleaner just to pass a list of filters around. Then we can add new filters as a command line arg to fontmake.

@anthrotype
Copy link
Member

I don't think we want a new command-line argument for fontmake, since this is the default behaviour for glyphs-generated sources and nobody wants to build a font with those open corners anyway.
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.
Also I don't think we need to add a new kwarg to the ufo2ft compile functions for the filters. Filters stored in the lib should be enough to do what you want.

@anthrotype
Copy link
Member

maybe I misunderstood. are you suggesting we add a generic --filters option to fontmake CLI, similar to --feature-writers with which one can load custom writers from the CLI in addition to from the UFO lib? I guess that could be handy.. but in general one should be able to achieve the same result by injecting a filter definition in the UFO libs.

glyphsLib should be responsible for adding the filter

maybe fontmake is best placed to inject the open courner filter, as the filter is only useful when compiling to a binary font, whereas one may use glyphsLib for source-to-source translation.
As for where the filter should be defined, we could add a module in the ufo2ft.filters package (the new submodule may need to import stuff from ufo2ft), but not enable it by default for all UFOs. We only want to enable it for UFOs that have been generated from a .glyphs source.

@simoncozens
Copy link
Contributor

are you suggesting we add a generic --filters option to fontmake CLI,

Right, exactly. And if we can pass filters=[...] from fontmake to ufo2ft then all the horrible kwarg passing goes away.

injecting a filter definition in the UFO libs.

But if you are doing fontmake -g, you don't get to stop and manipulate the UFO libs. (For arbitrary filters, I mean. We can easily make glyphsLib add an erase corner filter if that's the way you want to go.)

As for where the filter should be defined, we could add a module in the ufo2ft.filters package .

Bah, you just said to define it inside glyphsLib since it is Glyphs-specific. (I agree!) The code does need to import ufo2ft but the filter only gets run by ufo2ft, so it doesn't matter. I added a ufo2ft dev-dependency to glyphsLib for running the tests.

@anthrotype
Copy link
Member

I added a ufo2ft dev-dependency

that's what I was trying to avoid.. having glyphsLib depend on ufo2ft

@simoncozens
Copy link
Contributor

Only for dev and only for running test. Not a big deal, is it?

@anthrotype
Copy link
Member

there are precedents for ufo2ft.filters that are only defined but not enabled by default, e.g. the Transformations filter (which is also very glyphs-like in the naming of the various parameters, as it was designed to implement the Glyphs.app's Transformations custom parameter).

@anthrotype
Copy link
Member

the filter only gets run by ufo2ft, so it doesn't matter

I see what you mean.. yeah, it's fine then. As long as we don't add a runtime dep on ufo2ft in glyphsLib

@moyogo
Copy link
Collaborator

moyogo commented Feb 18, 2021

Having a fontmake --filters=... is handy just like --feature-writers is. fonttools/fontbakery#2961 (comment)
There are workflows where people call fontmake without adding extra lib keys to UFOs.

@anthrotype
Copy link
Member

alright

@moyogo
Copy link
Collaborator

moyogo commented Feb 18, 2021

I think the main reason is that there is no tool to add or edit ufo2ft libkeys.

@simoncozens
Copy link
Contributor

This is all done, now, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants