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.

Sign up for GitHub

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

Jump to bottom

[glyphs] Handle additional custom params #1144

Merged
merged 1 commit into from
Nov 28, 2024
Merged

[glyphs] Handle additional custom params #1144

merged 1 commit into from
Nov 28, 2024

Conversation

cmyr
Copy link
Member

@cmyr cmyr commented Nov 26, 2024

This is based on #1143

This handles additional custom parameters, specifically openTypeHeadLowestRecPPEM, and the various hhea & vhea caret properties (vhea isn't handled yet, but we can at least parse it now in preparation).

These params are kind of funny because you don't seem to be able to add them via the glyphs.app UI? But they exist in some fonts, and were causing crater failures.

@cmyr cmyr force-pushed the actually-add-more-params branch from 0b33566 to 298ae0d Compare November 26, 2024 17:52
@khaledhosny
Copy link
Contributor

These params are kind of funny because you don't seem to be able to add them via the glyphs.app UI?

You can add anything as Glyphs custom parameter. If Glyphs recognizes the name, it will give it the appropriate UI (e.g. a check box for boolean parameters), but otherwise you get a default text entry box that can have any value. Some of the parameters that Glyphs recognizes are hidden; if you search for them they will not show up, but if you go ahead and add it anyway it will get the appropriate UI.

They openType parameters, are, I think, present to parallel the corresponding UFO fontinfo keys, and I think Glyphs will use them when importing UFO files, rather than the “native” equivalent custom parameters.

rsheeter
rsheeter reviewed Nov 28, 2024
View reviewed changes
glyphs-reader/src/font.rs
@@ -520,7 +527,15 @@ impl RawCustomParameters {
{
macro_rules! add_and_report_issues {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a huge fan of the macro, lets at least add a comment explaining what precludes doing this without a macro

rsheeter
rsheeter reviewed Nov 28, 2024
View reviewed changes
glyphs-reader/src/font.rs Outdated Show resolved Hide resolved
@cmyr cmyr force-pushed the glyphs-parse-params-to-plist branch from 23146c9 to 17079c4 Compare November 28, 2024 23:35
Base automatically changed from glyphs-parse-params-to-plist to main November 28, 2024 23:40
@cmyr
[glyphs] Handle additional custom params
7a458d1 
Specifically openTypeHeadLowestRecPPEM, and the various hhea & vhea
caret properties.

(vhea isn't handled yet, but we can at least parse it now in preparation)
@cmyr cmyr force-pushed the actually-add-more-params branch from 298ae0d to 7a458d1 Compare November 28, 2024 23:47
@cmyr cmyr added this pull request to the merge queue Nov 28, 2024
Merged via the queue into main with commit a974d52 Nov 28, 2024
10 checks passed
@cmyr cmyr deleted the actually-add-more-params branch November 28, 2024 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rsheeter rsheeter rsheeter approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

handle openTypeLowestRecPPEM value in glyphs sources
3 participants
@cmyr @khaledhosny @rsheeter