Skip to content

Parsing and writing support for variable font tables fvar, STAT and avar #576

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

Merged
merged 1 commit into from
Feb 28, 2023

Conversation

Connum
Copy link
Contributor

@Connum Connum commented Feb 24, 2023

Description

Motivation and Context

This PR lays the groundwork for variable fonts support. Actual coordinate scaling and normalization (see https://learn.microsoft.com/en-us/typography/opentype/spec/otvaroverview#CSN) for rendering is not yet implemented, but several tables needed for supporting it can now be parsed and written.

I decided against also implementing gvar, because it introduces a set of new concepts/data types and seems to be the most complex of them. That would have bloated the PR even more.

Further resources for future rendering implementation:

How Has This Been Tested?

Loading and saving different variable fonts, added tests.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I did npm run test and all tests passed green (including code styling checks).
  • I have added tests to cover my changes.
  • My change requires a change to the documentation.
  • I have updated the README accordingly.
  • I have read the CONTRIBUTING document.

@Connum Connum added enhancement Spec Related to the implementation of the Opentype specification writing support Anything related to writing support as opposed to parsing or rendering labels Feb 24, 2023
@Connum Connum added this to the Release 2.0.0 milestone Feb 24, 2023
@yne
Copy link
Contributor

yne commented Feb 26, 2023

very nice addition 👍

@Connum Connum requested a review from yne February 27, 2023 07:48
Copy link
Contributor

@yne yne left a comment

Choose a reason for hiding this comment

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

LGTM 🚀
don't forget to fix the import paths before merging 👍

@Connum
Copy link
Contributor Author

Connum commented Feb 27, 2023

You'll have to approve again before I can merge.
Is there a good way to catch missing file endings in imports, without having to manually test it in browser context? Maybe a linting rule?

@Connum Connum requested a review from yne February 27, 2023 22:44
@yne yne force-pushed the feature/variable-font-tables branch 2 times, most recently from f815010 to 475d8db Compare February 28, 2023 21:01
@yne yne force-pushed the feature/variable-font-tables branch from 475d8db to c17aa88 Compare February 28, 2023 21:03
@yne
Copy link
Contributor

yne commented Feb 28, 2023

@Connum I rebase-squashed your connum/feature/variable-font-tables on upstream/master + fixed lint + minor STAT->stat renaming

All lint+test are now passing. You can merge when you are okay (I can't since I'm the last committer)

@yne
Copy link
Contributor

yne commented Feb 28, 2023

Is there a good way to catch missing file endings in imports

Node / Deno implicitly lookup import with .js suffix, browser don't, so either we

  • add a eslint-plugin-import dependency + "import/extensions" : ["error", { "js": "always"}] rule

  • run a chrome in CI that <script type=module>import 'src/opentype.js'</script>

@Connum
Copy link
Contributor Author

Connum commented Feb 28, 2023

I can't approve because I'm the PR author! @ILOVEPIE? :)

@yne yne merged commit 90bf641 into opentypejs:master Feb 28, 2023
@Connum Connum mentioned this pull request Mar 2, 2023
8 tasks
@Connum
Copy link
Contributor Author

Connum commented Mar 2, 2023

  • add a eslint-plugin-import dependency + "import/extensions" : ["error", { "js": "always"}] rule

I'm all for that!

@Connum
Copy link
Contributor Author

Connum commented Mar 3, 2023

I just added the plugin and configured it, but strangely while VSCode does indeed show red lines under missing file extensions for import statements, the npm run lint command does not. When I change "always" to "never", it shows errors for all other import statements, however. I stumbled upon import-js/eslint-plugin-import#2319 which brought me to https://github.com/satazor/eslint-plugin-import-esm - but it's exactly the same, with VS Code doing what it's supposed to do, but the lint command failing. I have no clue what's happening here!

@yne
Copy link
Contributor

yne commented Mar 3, 2023

maybe the best testing would be to have in CI a chrome-headless test that import the /src/opentype.js :)

@Connum
Copy link
Contributor Author

Connum commented Mar 6, 2023

I just added #584 with a custom eslint rule for enforcing file extensions in import paths.

@Connum Connum deleted the feature/variable-font-tables branch March 6, 2023 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Spec Related to the implementation of the Opentype specification writing support Anything related to writing support as opposed to parsing or rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fvar table is parsed and can be generated, but is never actually written to a file
2 participants