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

ufo: make build-time version number inline with the AFDKO #2195

Merged
merged 1 commit into from May 13, 2015

Conversation

adrientetar
Copy link
Member

#2193 replacement, with less changes.

The otspec says:

The string must contain a version number of the following form: one or more digits (0-9) of value less than 65,535, followed by a period, followed by one or more digits of value less than 65,535.

r? @frank-trampe

@frank-trampe
Copy link
Contributor

This does seem to accomplish the stated objective rather concisely. I'll copy @miguelsousa and @DavidRaymond once more so that they know that the padding question has moved to a new thread.

@adrientetar
Copy link
Member Author

Note that the spec allows it per OP and other fonts use this format already.

@frank-trampe
Copy link
Contributor

Legality and desirability are distinct qualities. I would like to avoid adding complexity (legal or not) unless this approach is measurably better in some way.

@adrientetar
Copy link
Member Author

It's only really for consistency because when this data is absent, the fallback attribute generated by ufo2fdk has padding zeroes.

@adrientetar adrientetar closed this Apr 9, 2015
@frank-trampe
Copy link
Contributor

Don't close the issue. It may well be the better way to do this. I just want to be sure that it is.

@frank-trampe frank-trampe reopened this Apr 9, 2015
@frank-trampe
Copy link
Contributor

@DavidRaymond, @miguelsousa, is it good to pad the version string like this (3 digits for the minor version)?

@miguelsousa
Copy link

@frank-trampe that's what the FDK has always done. @readroberts may be able to provide more details.

@davelab6
Copy link
Member

I think this is a good thing to do. Thanks @adrientetar !

@frank-trampe
Copy link
Contributor

@readroberts, what is the rule or system that you use for the version numbers? Do extra digits over 3 in the minor version get trimmed?

@frank-trampe
Copy link
Contributor

@readroberts?

@readroberts
Copy link

I suggest you keep the common tradition of interpreting at the head table fontRevision field as a 16.16 Fixed value, and round and pad the fractional part to 3 decimal places. 0x00010100L, aka decimal 1.001953125, should be represented as '1.002'

When I first started working on the FDK in 1998, there were several different implementations of the head table fontVersion field. In Both Apple and MS fonts, there are fonts which treated the fontRevision field as two separate short integer values, as well as a 16.16 Fixed value. In both MS and Apple system fonts, some reported the version in name ID 5 as being padded to 3 decimal places, and some didn't. A discussion on the OpenType list produced a consensus to do what is now common practice. The Open Type spec is now clear that the value is a Fixed value. It is also clear that you do NOT need to zero pad to any number of decimal places - "a period, followed by one or more digits". However, I personally, and quite a few other developers, find this practice useful.

@davelab6
Copy link
Member

davelab6 commented May 7, 2015

Thank you @readroberts !! I really appreciate you taking the time to participate in FontForge's development :)

@frank-trampe
Copy link
Contributor

It seems to me that the proposed changes follow the pattern suggested by @readroberts.

frank-trampe added a commit that referenced this pull request May 13, 2015
ufo: make build-time version number inline with the AFDKO
@frank-trampe frank-trampe merged commit ef83fd7 into fontforge:master May 13, 2015
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

Successfully merging this pull request may close these issues.

None yet

5 participants