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

WIP: Compact Font Format (CFF) Version 2 #342

Open
wants to merge 41 commits into
base: main
Choose a base branch
from
Open

WIP: Compact Font Format (CFF) Version 2 #342

wants to merge 41 commits into from

Conversation

brianpopow
Copy link
Contributor

@brianpopow brianpopow commented Jul 14, 2023

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

This PR aims to add support for CFF2 format. Its still a work in progress.

The Specification can be found here.

Introduction to open type variable fonts: introducing-opentype-variable

Relevant tables for the font variations are:

Example font for CFF2:
RobotoFlex.zip

To see how the different variations settings affect the font the website wakamaifondue is usefull, just drop the example font there and go to the variations tab to experiment with different settings.

TODO:

  • The parsing of the ItemVariations data does not match what fontkit parses, see comment
  • We need some method to create a new font based on the variation settings a user chooses. For example: font.GetVariation(variationOptions). The equivalent in fontkit would be font.getVariation({wght: 300})
  • Font class needs a method to get the available variation axes that this font supports. Note: FontMetrics class already has a TryGetVariationAxes
  • Font class needs a method to get the the named variation instances that the font designer has specified.

@codecov
Copy link

codecov bot commented Jul 16, 2023

Codecov Report

Attention: Patch coverage is 60.91549% with 333 lines in your changes missing coverage. Please review.

Project coverage is 85%. Comparing base (977001a) to head (606f16b).
Report is 34 commits behind head on main.

Current head 606f16b differs from pull request most recent head ff606eb

Please upload reports for the commit ff606eb to get more accurate results.

Files Patch % Lines
src/SixLabors.Fonts/Tables/Cff/Cff2Parser.cs 0% 101 Missing ⚠️
...dTypographic/Variations/GlyphVariationProcessor.cs 31% 59 Missing and 1 partial ⚠️
src/SixLabors.Fonts/Tables/Cff/CffParserBase.cs 71% 31 Missing and 13 partials ⚠️
.../SixLabors.Fonts/Tables/Cff/CffEvaluationEngine.cs 17% 21 Missing and 2 partials ⚠️
src/SixLabors.Fonts/Tables/Cff/Cff2Table.cs 0% 17 Missing ⚠️
...s/AdvancedTypographic/Variations/TupleVariation.cs 61% 11 Missing and 2 partials ⚠️
...AdvancedTypographic/Variations/DeltaSetIndexMap.cs 57% 10 Missing and 2 partials ⚠️
...ancedTypographic/Variations/VariationRegionList.cs 68% 7 Missing and 3 partials ⚠️
...vancedTypographic/Variations/ItemVariationStore.cs 69% 5 Missing and 3 partials ⚠️
src/SixLabors.Fonts/StreamFontMetrics.cs 72% 2 Missing and 4 partials ⚠️
... and 14 more
Additional details and impacted files
@@          Coverage Diff           @@
##            main    #342    +/-   ##
======================================
- Coverage     86%     85%    -2%     
======================================
  Files        238     259    +21     
  Lines      15184   15865   +681     
  Branches    2101    2205   +104     
======================================
+ Hits       13088   13488   +400     
- Misses      1613    1871   +258     
- Partials     483     506    +23     
Flag Coverage Δ
unittests 85% <60%> (-2%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Aug 9, 2023

@brianpopow I had a quick look at the ItemVariationData and spotted an issue with the longwords check however I'm not sure how you tested the values here. How did you compare the values?

@brianpopow
Copy link
Contributor Author

@brianpopow I had a quick look at the ItemVariationData and spotted an issue with the longwords check however I'm not sure how you tested the values here. How did you compare the values?

I use the tagged version 1.8.0 of fontkit and the CFF2 font RobotoFlex.ttf linked above.
To debug fontkit, I use a unit test. As IDE, I do use webstorm. For example create a new file example.js in test folder of fontkit with the following code:

import fontkit from '../src';
import assert from 'assert';
import fs from 'fs';

describe('metadata', function() {
    it('fontkit test', function() {
        let font = fontkit.openSync(__dirname + '/data/RobotoFlex.ttf');
        var variationFont = font.getVariation({wght: 300})
        var run = font.layout("hello");
    });
});

This creates a new font variation with the weight 300 as an option. Then add a breakpoint, when the font is loaded and inspect font or variationFont. Look into the HVAR table and then into itemVariationStore -> itemVariationData.
For easier reading I usually do right click and Copy Json and past into into a text file.

@brianpopow
Copy link
Contributor Author

@JimBobSquarePants I think the ItemVariationsData now looks equal to fontkit, maybe you could double check, if I have missed something.

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Aug 11, 2023

image

Nice work. Looks like our indexes are arranged in a different order though?

@JimBobSquarePants
Copy link
Member

OK. Figured that out. The two arrays were being concatenated in the wrong order.

I'll have to do a fair bit of code reading to be able to be more useful. Will start asap.

@brianpopow brianpopow added this to the Future milestone Aug 11, 2023
@JimBobSquarePants
Copy link
Member

@brianpopow I've gone through and updated the PR to allow merging with the changes introduced during the submodule and target framework updates. The file-scoped namespace changes killed the ability to auto-merge. It looks like I've done everything correctly.

@avisra
Copy link

avisra commented Jun 22, 2024

did this get dropped or still being worked on

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Jun 22, 2024

Needs someone to finish it off. It’s a lot of work and we have very few contributors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants