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

Clarifying postscriptUnderlinePosition #217

Closed
benkiel opened this issue Jun 26, 2023 · 18 comments
Closed

Clarifying postscriptUnderlinePosition #217

benkiel opened this issue Jun 26, 2023 · 18 comments

Comments

@benkiel
Copy link
Contributor

benkiel commented Jun 26, 2023

The UFO spec leaves ambiguity for postscriptUnderlinePosition.

The post version of this data is different from the CFF/Type1 version of the data: https://learn.microsoft.com/en-us/typography/opentype/spec/post

It seems like the FDK accounts for this (adobe-type-tools/afdko#1531) but fontMake does not.

Either this value should be clarified as to what it means to both tables, or we should add a key for the post table. I don't think a key should be added for the same type of information, so I think we should clarify this value.

By name the implication is that the value is the center of the underline stroke. Should we go with this and say that the post table needs to calculate from that, or to we go with the post definition of it being the top of the stroke?

Either way, some tools will need to change.

Does anyone have an opinion on this? @anthrotype @justvanrossum @typesupply @schriftgestalt @josh-hadley @kaydeearts @skef

@benkiel benkiel changed the title Clarifying underlinePosition Clarifying postscriptUnderlinePosition Jun 26, 2023
@skef
Copy link

skef commented Jun 26, 2023

Might it make the most sense to do both? That is, add an optional keyword for post for specifying the value, but also clarify how the value should be calculated from postscriptUnderlinePosition if that keyword is absent? It often turns out that calculated values work 99% of the time but there are valid exceptions.

@schriftgestalt
Copy link

I’m for clarifying this. I can’t see why the value in CFF and post should not agree.

@typesupply
Copy link
Contributor

I would much prefer to clarify over adding a new key/value to fontinfo.plist. If we do need a new key/value I'd prefer doing it with a public key in the lib.

@benkiel
Copy link
Contributor Author

benkiel commented Jun 26, 2023

@schriftgestalt The two values will always be different, as they are specified differently:

  • post & cff2 tables: position value is the top of the underline
  • CFF table & Type1 fonts: position value is the middle of the underline

For sure it sounds like we should clarify the value (right now it says the value is for CFF/post/Type1) as being tied to how CFF specifies it. @skef do you think it is worth adding a public lib key for the times the calculation doesn't work? I'm trying to think of when that would be if you go from CFF to post (I understand that you could have an odd underline height that wouldn't work going from post to CFF).

Thanks all for chiming in!

@skef
Copy link

skef commented Jun 27, 2023

Since we need to clarify regardless, maybe we should do that first and then see if anyone thinks the calculation we come up with would ever fall short.

AFDKO's calculation is cff_underline_position + cff_underline_thickness / 2.

@benkiel
Copy link
Contributor Author

benkiel commented Jun 27, 2023

@skef thank you: agree that this is the way to at least start.

@nicksherman
Copy link

I'm glad this is being addressed, thank you all!

For what it's worth, it seems confusing to me to use the old (basically now obsolete) CFF logic as the basis for the UFO spec if the values that will be stored in the newer / more relevant post tables will be different.

@benkiel
Copy link
Contributor Author

benkiel commented Jun 27, 2023

@nicksherman: because the info attribute is tied to postscript (CFF/Type1) changing the behavior of the attribute to be how post specifies the value is a breaking change. If this ends up having too many edge cases, we’ll add a public key for the post/CFF2 tables —as discussed above.

Closed with #218

@skef
Copy link

skef commented Jun 27, 2023

Whoops! Sorry, I was reading the code wrong. It looks like the calculation was cff_underline_position - cff_underline_thickness / 2 . I wrote "+" earlier. Sorry for the think-o

@skef
Copy link

skef commented Jun 27, 2023

Oh, dammit, wait never mind, I was right the first time! I just confused myself by looking at the inverse calculation. What you have is fine!

I guess I need coffee or something.

@skef
Copy link

skef commented Jun 27, 2023

@anthrotype anthrotype reopened this Jul 3, 2023
@anthrotype
Copy link
Member

anthrotype commented Jul 3, 2023

I was away while this was discussed so I didn't have the chance to chime in. Sorry but I don't agree with the changes that went in #218. We should have simply akwnoledged that the interpretation of this field is ambiguous and implementation dependent, and may produce different result depending on whether a compiler uses the post's definition (fontmake/ufo2ft) or the Type1/CFF definition (AFDKO's makeotf). To make the interpretation unambiguous going forward, without needing a major version bump, a new lib key could be specified so that new compiler versions can read and apply either definitions. Existing sources should continue to produce the same binary fonts, whether they were built using fontmake or AFDKO, and not suddenly change because the UFO spec got "clarified", without even an explicit version format change (and this one arguably counts as major change so UFO 4, rather than 3.1).

@benkiel
Copy link
Contributor Author

benkiel commented Jul 3, 2023

@anthrotype yes, agree, will fix. Apologies, I was too hasty.

For now, what do you think about the a public lib key being underlinePosition defined as being the top of the underline. In UFO4 this would become part of info, but for now, if that key is there, the compiler would know the value for post/CFF2, and would calculate CFF/Type1 if not defined, or use postscriptUnderlinePosition for CFF/Type1. I would be happy to PR that logic for ufo2ft/ufo2fdk.

@anthrotype
Copy link
Member

sounds good, thanks. Maybe I would call the new public lib key (and future fontinfo attribute) openTypePostUnderlinePosition to match the current approach of prefixing with openType{Tag} any attributes pertaining to open type tables.

@typesupply
Copy link
Contributor

public.openTypePostUnderlinePosition sounds like a good lib key to me with openTypePostUnderlinePosition being the eventual fontinfo.plist key.

@benkiel
Copy link
Contributor Author

benkiel commented Jul 3, 2023

Ok, I'll PR that.

benkiel added a commit that referenced this issue Jul 3, 2023
benkiel added a commit that referenced this issue Jul 3, 2023
@benkiel
Copy link
Contributor Author

benkiel commented Jul 3, 2023

PRs made

@benkiel
Copy link
Contributor Author

benkiel commented Jul 3, 2023

And accepted. Closing.

@benkiel benkiel closed this as completed Jul 3, 2023
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

6 participants