googlefonts / fontmake

Compile fonts from sources (UFO, Glyphs) to binary (OpenType, TrueType).
Apache License 2.0
780 stars 93 forks source link

Tamil: UI font produced by the pipeline fails our UI font criteria #275

Open marekjez86 opened 7 years ago

marekjez86 commented 7 years ago

source: https://github.com/googlei18n/noto-source/tree/master/src/NotoSansTamil

font produced by the pipeline: https://github.com/googlei18n/noto-fonts/blob/master/alpha/from-pipeline/unhinted/otf/sans/NotoSansTamilUI-Regular.otf

font as expected: https://github.com/googlei18n/noto-fonts/blob/master/alpha/from-glyphsapp/unhinted/otf/sans/NotoSansTamilUI-Regular.otf

Issue: UI font produced by the pipeline fails our UI font criteria (smallest ymin should be -293 but is -321). Some notodiff results (Regular used during notodiff):

haiivowelsigntamil haprehalftamil hatamil

anthrotype commented 7 years ago

Yes, that's a known glyphsLib limitation.. We need to add support for transform custom parameters (in this case the outline is shifted up or down by some amount upon export).

marekjez86 commented 7 years ago

transformation-tamilui

Is this the option that we do not support at this point?

marekjez86 commented 7 years ago

@anthrotype @behdad : How difficult would it be to add a warning "Unimplemented custom parameter X"? This way I would know (and users of the pipeline would also know).

Also is there a complete list of unimplemented features like this? This way I could tell font designers, for now do not use X or Y in your fonts.

behdad commented 7 years ago

Hah, interesting. It will be a few day's work and then lots of testing. I wish we still had @jamesgk working on this stuff :(.

jamesgk commented 7 years ago

Ohh, this is pretty straightforward right? Just a little time-consuming. Or maybe that's the kind of stuff you're talking about :P

zjusbo commented 7 years ago

I'll take over this one.

Based on the lesson I learned from this thread, I'm writing the following design plan, discuss it with fellows, before actually starting to code.

Please correct me if I'm wrong. Or please even stop me if it involves to much work and I need to focus on other higher priority stuff.

Goal

Respect 'Filter' parameter in fontmake pipeline.

Background First, According to Glyphs HandBook page 184, Filter is a glyphapp specific custom parameter.

Second, In glyphsLib builder.py line 369, we dump all non-OpenType table entries (including glyphapp specific custom parameter) into ufo.lib, with a GLYPHS_PREFIX inserted at the start of the name. ufo.lib[GLYPHS_PREFIX + name] = value

Finally, neither GlyphsLib nor Fontmake respects the Filter value later, so as many other parameters.

For all glyphsApp specific custom parameters (those whose name are capitalized, for example: Master Name, Color Palettes are glyphsApp specific, but hheaDescender, underlineThickness are UFO spcific), we only respect Don't use Production Names, Glyphs.Export and Keep Glyphs so far. So all other glyphsApp specific custom parameters takes no effect in our pipeline (This list might not be accurate). I would recommend the designer not use these custom parameters for now until we have implement them.

Overview As the Glyphs Handbook page 184 says,

Filter: Triggers Glyphs filters or app functions in an instance, after decomposition of compound glyphs.

The decomposition of compound glyphs happens in fontmake.font_project.decompose_glyphs and is out of GlyphsLibs's control.

My plan is to add a function in GlyphsLib called 'apply_custom_parameter_filter(ufo)', which takes an ufo as parameter, read ufo.lib[GLYPHS_PREFIX + 'Filter'], analyze it and apply it to glyphs in ufo.

And fontmake (probably other applications) calls this function at a proper time.

Detailed Design

Modify UFO glyphs according to the filter so that we can safely remove filter parameter afterwards. I think I need to read Defcon as well as UFO 3 Spec to figure out which value should I modify. Any advises are appreciate if someone can point out which specific value do I need to modify or which specific section do I need to read regarding to applying the filter.

Extensible We can create more functions to handle other custom parameters and finally bundle them together into a single apply_custom_parameter.py file.

PS: the filter contains following elements • AddExtremes • HatchOutlineFilter; OriginX:x; OriginY:y; StepWidth:distance; Angle:angle; Offset:thickness • OffsetCurve; x; y; make stroke; position/auto • RemoveOverlap • Roughenizer; segment length; x; y; angle • RoundCorner; radius; visual correction • RoundedFont; stem • Transformations; LSB:±/shift; RSB:±/shift; Width:±shift; ScaleX:percent; ScaleY:percent; Slant:amount; SlantCorrection:bool; OffsetX:amount; OffsetY:amount; Origin:select

punchcutter commented 7 years ago

@zjusbo I like that idea. As far as I can tell not all of these Filters are supported yet in customParameter inside of Glyphs. Better check with @schriftgestalt to be sure. I've tried Transformations and RoundCorner together and that seems to work. Here's a ridiculous test I did:

Define an instance Custom Parameter called Filter with value

Transformations;OffsetX:33;OffsetY:40;Origin:4;LSB:+100;RSB:+100;ScaleX:120;ScaleY:120;Slant:20;SlantCorrection:0

and then another Custom Parameter with value

RoundCorner;180;0

The syntax isn't 100% clear because Transformations has key/value pairs, but RoundCorner just takes the semi-colon separated values.

This particular situation for Tamil UI is a pretty elegant solution to creating UI instances from the same master file as the document version since it only needs to be shifted slightly. For a super simple X/Y shift like this it's easy in Defcon to do:

for glyph in ufo:
    glyph.move((0, -51))
schriftgestalt commented 7 years ago

The filter on export support was one of the big issues I had when asked about building a CLI version of the glyphs font export. The default filters can be handled by had coding them in the pipeline. But people can install custom Filters in Glyphs and use them on export, too. So if might be a good idea to keep the plugin/modul structure (as it is implemented in Glyphs) to make it as extendable as possible. And maybe even support glyphs plugins (at least the ones written in python) directly? That would suggest to apply them in glyphsLib.builder.

The format of the argument string is a list of semicolon delimited values. The format of the individual values is defined by the plugin. For plugins that only ever have one or two arguments it doesn't need 'key:value' settings.

And keep in mind that most of the filters support a 'include|exclude:list, of, glyph, names' as last argument.

anthrotype commented 7 years ago

Thanks Bo for the thorough analysis and for stepping up to do this.

I was thinking that we could try to make this filters support not necessarily tied with Glyphs.app and glyphsLib. I mean, would be nice if one could apply similar filters whether input is .glyphs or .ufo.

To that end, I think we could have ufo2ft instead of glyphsLib apply the actual filters on the Defcon glyphs before these are converted to fonttools objects. If we do that, then both fontmake and other UFO based applications that use ufo2ft (e.g. Trufont) could take advantage of such on-export filters.

The job of glyphsLib would just be parsing the Filter custom parameters in the .glyphs source and writing that out to something that ufo2ft can consume, in the UFO lib. We could define a private com.github.googlei18n.ufo2ft.filters UFO lib key, and use that to store these filter parameters.

The default filters would live in a sub-package, e.g. ufo2ft.filters, each in its own module. They could be based on the PointPen API (see ufoLib/pointPen.py), which is the standard way in the UFO world to manipulate outlines. They would work like "filter" pens, i.e. pens that take another pen that actually does the drawing, and before passing drawing commands to the inner pen, they do some transformations (the fonttools TransformPen is an example).

And after we implement the basic default filters, we could see how to make this extendable. Popular python applications like pytest or tox use the setuptools' entry_points feature to allow loading external plugins (e.g. http://doc.pytest.org/en/latest/writing_plugins.html#setuptools-entry-points). But we can look at that later, I think.

To answer Georg's suggestion about supporting existing third-party plugins written for Glyphs.app, I am not sure how that could be done from glyphsLib, which is supposed to run outside of and independently of Glyphs.app.

I've never written a Glyphs plugin, to be honest. However, looking at the documentation, it seems that one needs to import from GlyphsApp.plugins import FilterWithoutDialog, and this GlyphsApp module would not be accessible when running glyphsLib in a pure Python environment outside Glyphs.app.

https://github.com/schriftgestalt/GlyphsSDK/tree/master/Python%20Templates/Filter%20without%20Dialog

Let's think more about all this. Have a nice weekend!

JelleBosmaMT commented 7 years ago

Sorry to make your life more complicated, but.... the possible use of a custom parameter that shifts UI instances has been documented last year in the specification for .glyphs deliverables. A reminder was added to the design proposal of Tamil last summer which already mentioned "Filter = Transformations; OffsetY: 51; Origin: 4; ".

As a reminder: when generating hinted OTFs: do not forget to transform the alignment zones. This is the one part where Glyphs.app let me down when I verified the shift while preparing the design proposal. (I reported the omission and it may be fixed in the mean time, I haven't checked.)

jamesgk commented 7 years ago

To answer Georg's suggestion about supporting existing third-party plugins written for Glyphs.app, I am not sure how that could be done from glyphsLib, which is supposed to run outside of and independently of Glyphs.app.

I've never written a Glyphs plugin, to be honest. However, looking at the documentation, it seems that one needs to import from GlyphsApp.plugins import FilterWithoutDialog, and this GlyphsApp module would not be accessible when running glyphsLib in a pure Python environment outside Glyphs.app.

It might be possible to stub these out and recreate Glyphs' python API in glyphsLib. It goes back to the old non-trivial task of making everything in glyphsLib class-based. But I just looked and saw that work has finally started on this!

schriftgestalt commented 7 years ago

Please have a look at the class branch. The test do pass. Please don't wait to long to merge it before the main branch moves away.

anthrotype commented 7 years ago

@schriftgestalt Right, I'll review it first thing tomorrow.

marekjez86 commented 7 years ago

@zjusbo : it looks that this might be fixed. Please find out

anthrotype commented 7 years ago

it looks that this might be fixed

@marekjez86 Actually, no.. Support for Filter custom parameters is not implemented yet anywhere in the pipeline.

zjusbo commented 7 years ago

Thanks all for the feedbacks and instructions!

Here is a brief summary of where we are currently:

  1. Not all of these Filters are supported yet. At least Transforms and RoundCorner seems to work. Be careful about the unclear syntax in customParameter. - punchcutter
  2. Keep the plugin/module structure(What does it mean?). Be careful about supporting 'include|exclude:list, of, glyph, names' as last argument. - schriftgestalt
  3. Implement Filter feature in ufo2ft lib so that both glyphsLib, ufo and other apps can use it. - anthrotype
  4. Transform the alignment zooms when generating hinted OTFs - JelleBosmaMT
  5. Make everything in glyphsLib class based. - jamesgk
  6. Cosimo and Georg will talk about the class branch in Berlin.

Plug-in/class based approach is still under discussion between Cosimo and Georg so I'm focusing on a straightforward fix for this issue for now. (And @marekjez86 is also waiting for this to be fix ASAP.)

I like the idea that this filter could be implemented in ufo2ft so that other applications can also use it.

My concern is that the syntax of 'Filter' is defined in glyphsApp manual therefore glyphsLib should be responsible to handle it. (IMHO, the goal of glyphsLib is to perform a lossless conversion from .glyphs to UFOs and .designspace (Is it possible?)) The glyphsLib will depend on the correctness of ufo2ft if we let ufo2ft handle the glyphsApp functionality. And it sort of breaks the modularization I guess.

Alternatively, if we really want to make some filter/custom parameters globally usable, we define ufo2ft's own filter specification, so that every app that want to use ufo2ft to compile the font must respect ufo2ft specification and provide ufo2ft recognizable ufo files.

As @punchcutter pointed out, offsetX and offsetY is fairly simple in defcon:

for glyph in ufo:
    glyph.move((0, -51))

I think let glyphsLibs to handle it is straightforward from my understanding. But all other comments are welcome.

jamesgk commented 7 years ago

I like the idea that this filter could be implemented in ufo2ft so that other applications can also use it.

My concern is that the syntax of 'Filter' is defined in glyphsApp manual therefore glyphsLib should be responsible to handle it. (IMHO, the goal of glyphsLib is to perform a lossless conversion from .glyphs to UFOs and .designspace (Is it possible?)) The glyphsLib will depend on the correctness of ufo2ft if we let ufo2ft handle the glyphsApp functionality. And it sort of breaks the modularization I guess.

This is a question I faced a lot working on fontmake. The counter-argument is that while this feature originates in Glyphs, it doesn't necessarily need to be tied to Glyphs. We can embrace it as a pattern for font compilation by supporting similar UFO data and converting to that as Cosimo suggests. And I tend to listen to the folks from Dalton Maag as they're more in tune than I am with needs of font developers generally :) But I think the bottom line is either way works.

As for lossless conversion, it's possible but not always practical. I had that as a goal originally but wouldn't suggest focusing on it at this point.

zjusbo commented 7 years ago

Thanks James. At the meantime of waiting for inputs from Dalto Maag, I'm following Cosimo's and your advice, looking into the ufo2ft codebase and see how I can fix it there.

[edited on 4:29 02/21/2017] As a side note, the UFO does not store everything. It has following limitations:

  1. UFOs are single masters
  2. many Glyphs-specific settings, like enabled or disabled automatic alignment of components, or things like the bracket trick, cannot be stored in UFO fiels.

For 1, we can convert a single glyphs file into multiple UFOs with designspace file. For 2, we are currently store them into ufo.lib (if I'm correct), and does not handle them.

behdad commented 7 years ago

@anthrotype what's the status of this?

anthrotype commented 7 years ago

@behdad Sorry, I was sidetracked by other stuff. I'm now working on a patch based on Bo's pull request. I'll focus on implementing the built-in Transformations filter first; maybe later we can work on an extensible plugin interface.

behdad commented 7 years ago

Hey no worries. Was just cleaning up my tabs.

marekjez86 commented 7 years ago

@anthrotype @behdad another bug (feature requests) that causes that we cannot release Tamil UI