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

Support FeatureWriter insert mode #351

Closed
madig opened this issue Nov 21, 2019 · 30 comments
Closed

Support FeatureWriter insert mode #351

madig opened this issue Nov 21, 2019 · 30 comments

Comments

@madig
Copy link
Collaborator

madig commented Nov 21, 2019

Situation: Noto Syriac contains hand-written feature files to define contextual kerning. Ideally, the mark and mkmk features are autogenerated, but they can currently only be appended to the preexisting feature file. The problem: the font requires that mark and mkmk are processed before kern, which is currently not possible and means that the feature have to be manually generated and hardcoded into the features. This is a waste of effort.

Maybe a FeatureWriter prepend mode could look for a human-placed placeholder string in the written feature file to insert the autogenerated features.

@anthrotype
Copy link
Member

i prefer "insert" mode, it's more general.

I know Glyphs.app does something similar. We need to find out what special strings it uses and do the same

@anthrotype
Copy link
Member

cc @schriftgestalt

@anthrotype anthrotype changed the title Support FeatureWriter prepend mode Support FeatureWriter insert mode Nov 21, 2019
@anthrotype
Copy link
Member

So, Glyphs.app uses a special comment # Automatic Code End

https://forum.glyphsapp.com/t/how-to-insert-a-lookup-feature-after-generated-mark-feature/12891/4

Perhaps also an # Automatic Code Start (I haven't tried)?

The special comment needs to be inside the block of the feature where one wants to insert the auto-generated lookups.

@anthrotype
Copy link
Member

This feature was added in https://glyphsapp.com/blog/glyphs-2-4-2-released

The changelog says "either start it with this comment line: # Automatic Code End. This will append your code after the automatic code. Or, to prepend your code, end the feature with this comment line: # Automatic Code Start".

I wonder what happens if both End and Start comments are used and there's manual code before and after the automatic one, whether that's supported or not..

@schriftgestalt
Copy link

I just had a look at the code. It seems that the position of the comment is not important. And if you have both, your code will be added twice. I would need to test this more.

@madig
Copy link
Collaborator Author

madig commented Nov 22, 2019

Can we maybe do something like

feature TAG {
    # Automatic Code Insert
    blah blah;
} TAG;

instead of Start/End? This would represent a singular insertion point for automatic feature code. Think e.g. putting mark and mkmk before any other lookup because later contextual lookups expect things to be in a certain state before they proceed?

Also, I don't understand how the current system works. Is the above equivalent to

feature TAG {
    # Automatic Code Start
    # Automatic Code End
    blah blah;
} TAG;

?

@anthrotype
Copy link
Member

only one of # Automatic Code Start and # Automatic Code End can be used for each feature, as @mekkablue also confirmed in https://forum.glyphsapp.com/t/how-to-insert-a-lookup-feature-after-generated-mark-feature/12891/12

I think in ufo2ft we should support the existing Start/End special comments.

We could also add a new one # Automatic Code Insert as an alias for the two above, though this would be only supported by ufo2ft and not by Glyphs.app (unless @schriftgestalt also implements it).

I agree the name "Insert" makes more sense and is more general, as it allows to put manual code before and after.

@schriftgestalt
Copy link

There is one thing you need to pay attention. When the automatic and manual code contains several script tags, at least makeOTF doesn't like it. It will fail with a "Script was set before" error. I split up the manual code and insert it with the automatic code to group the script. Later I learned that makeOTF allows duplicate features. That makes it much easier to combine code.

@khaledhosny
Copy link
Collaborator

So, if we were to implement this today, the following:

# Automatic Code End
pos A B -50;

would translate to:

kern {
  # genetated stuff
} kern;

# Automatic Code End

kern {
  pos A B -50;
} kern;

And vice versa?

@schriftgestalt
Copy link

That looks good

@twardoch
Copy link
Contributor

twardoch commented Dec 5, 2019

FontLab uses #> feature to indicate the start and #< feature to indicate the end of an autogenerated feature definition block, and similar keywords for autogenerated blocks of languagesystem declarations, class definitions etc.

@moyogo
Copy link
Collaborator

moyogo commented Dec 5, 2019

This should be standardized in some way in FEA syntax.

@twardoch
Copy link
Contributor

twardoch commented Dec 5, 2019

I think there is still some expectation that the makeOTF lib would have to support everything that the FEA syntax describes (even though it's actually not the case). We have two more implementations (FontForge and feaLib), and at least one of them could be extended to support more aspects of the syntax, but since the work on the makeOTF compiler has been very slow, adoption of syntax extensions has also stalled.

There is

https://github.com/adobe-type-tools/afdko/labels/fea%20syntax%20extension

and there is

https://github.com/silnrsi/pysilfont/blob/master/docs/feaextensions.md

We could propose that FEAX (SIL’s FEA extensions) could become the new syntax. This is basically a bit like LaTeX and TeX. And standardize it there first.

@anthrotype
Copy link
Member

TBH, i don't like either of the two existing syntaxes from Glyphs.app and FontLab, and I prefer the one @madig proposed in #352. This is a single line comment (instead of two) that clearly means "INSERT". It is placed at the top level outside any feature blocks, and bears the tag of the automatic feature it is supposed to be replaced with, so it's very simple to match and replace with the automatic feature statements.

A feature writer may need to define standalone lookups and register them under specific scripts and language systems. So it needs to define separate feature blocks, can't reuse the user-defined one. It makes more sense to write these placeholder comments outside the feature blocks. The user could then define her own manual feature blocks before and/or after the automatic one. E.g.:

feature kern { pos a b -30; } kern;

### INSERT kern ###

feature kern { pos x x 10; } kern;

I think glyphsLib could be modified to convert between the Glyphs.app's notation to the one @madig proposed.

From:

feature kern {
    # Automatic Code End
    pos a b -30;
} kern;

To:

### INSERT kern ###

feature kern {
    pos a b -30;
} kern;

@khaledhosny
Copy link
Collaborator

https://xkcd.com/927/

@schriftgestalt
Copy link

@twardoch: What we are speaking about here is more a preprocessor functionality. So the feature compiler is not really involved with this.

@anthrotype You are right. Two comments are not needed. The handling in Glyphs is slightly different. The code as seen by the use never has the feature xxxx { that is added automatically. Standalone lookups go into its own place (Prefix). That is much easier in almost all cases but can be annoying at times. But because of this, having the feature name in the command is a duplication of info that might get out of sync and thous I like to avoid it.

@twardoch
Copy link
Contributor

I like the start and end comment because you can keep them in the final code, and the end-user always knows that has been inserted. That’s why FontLab 7 has these double indicators. Anything inside those special comments will be replaced, but as the end-user, you see what’s been inserted. And that’s important, because you may want to base your decisions as to what to add manually depending on what the preprocessor has added.

@kontur
Copy link
Contributor

kontur commented Feb 3, 2020

I'm not sure how much exactly this is part of the same problem, but I have a case where I'd like to add custom mark code that refers to some of the autogenerated markClass'es. Even if I can insert into the mark or kern feature with what you discuss above, this type of code would still not be possible, unless there is syntax to pre-insert the location of the yet to be generate markClass statements as well.

Did ufo2ft have an option to explicitly write out the mark and kern code before compile to manually operate on those kinds of additions after the generated markClass statements are added the feature code?

@anthrotype
Copy link
Member

I thought about this again, and I believe it's better that we don't invent yet another feature insertion method. I propose that ufo2ft use the one from FontLab 7 (see @twardoch's #351 (comment) above).
GlyphsLib can convert between Glyphs.app's block-nested # Automatic Code End and the FL7/ufo2ft insertion markers.

@anthrotype
Copy link
Member

I like the start and end comment because you can keep them in the final code, and the end-user always knows that has been inserted. That’s why FontLab 7 has these double indicators. Anything inside those special comments will be replaced, but as the end-user, you see what’s been inserted.

this is actually nice, and we should do the same.

@schriftgestalt
Copy link

schriftgestalt commented Jan 22, 2021

I Glyphs 3 the procedure is much easier:

There is only a # Automatic Code (it will recognize the old syntax as it just ignores the rest).

Depending Where you put the comment, the custom feature will be placed before or after the automatic one.

Example 1

custom kern code like this:

# Automatic Code

pos B B -100;

results in a final .fea file:

feature kern { # the automatic code, simplified
	pos A A -80;
} kern;

feature kern {
pos B B -100;
} kern;

Example 2

pos B B -100;

# Automatic Code

pos C C -100;

result:

feature kern {
pos B B -100;
} kern;

feature kern { # the automatic code, simplified
	pos A A -80;
} kern;

feature kern {
pos C C -100;
} kern;

Putting the code in individual feature blocks makes it much easier to deal with script and language settings.

(edited it after @moyogo comment below)

@moyogo
Copy link
Collaborator

moyogo commented Jan 22, 2021

@schriftgestalt If I'm not mistaken, the result in your example 1 has the wrong order, the automatic code comes first in the result as in the code. That’s what I see when I test Glyphs3.

@schriftgestalt
Copy link

Right, of cause. I fix it in the above example. I copied the wrong version of the code.

@madig
Copy link
Collaborator Author

madig commented Jan 22, 2021

How does the comment know whether to insert kern or mkmk or something else?

@schriftgestalt
Copy link

The comment and the code is inside a feature object that has the tag.

@madig
Copy link
Collaborator Author

madig commented Jan 22, 2021

Hm. I think we need to be able to insert outside feature blocks as well.

@schriftgestalt
Copy link

Do you have an example?

@moyogo
Copy link
Collaborator

moyogo commented Feb 19, 2021

Fixed by #458

@moyogo moyogo closed this as completed Feb 19, 2021
@khaledhosny
Copy link
Collaborator

khaledhosny commented Mar 11, 2021

I don’t think this is fixed. Glyphs 2 does not support # Automatic Code, it supports # Automatic Code End (and # Automatic Code Start but I’m fine with supporting only one).

# Automatic Code seems to be Glyphs 3-only, but Glyphs 3 format is not yet supported by glyphsLib (and will likely take sometimes before it is usable).

@schriftgestalt
Copy link

If the feature code is from Glyphs 2 (and contains the Start/End that can basically be ignored. So just look for # Automatic Code and ignore the rest of the line.

khaledhosny added a commit that referenced this issue Sep 23, 2024
Split the feature block into two and insert the generated code in the
middle.

See #351 (comment)
khaledhosny added a commit that referenced this issue Sep 23, 2024
Split the feature block into two and insert the generated code in the
middle.

See #351 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants