Skip to content

Add support for variable COLR tables using VarIndexBase and DeltaSetIndexMap#88

Merged
justvanrossum merged 6 commits intofontra:mainfrom
anthrotype:variable-colr
Jul 6, 2022
Merged

Add support for variable COLR tables using VarIndexBase and DeltaSetIndexMap#88
justvanrossum merged 6 commits intofontra:mainfrom
anthrotype:variable-colr

Conversation

@anthrotype
Copy link
Copy Markdown
Contributor

@anthrotype anthrotype commented May 9, 2022

Towards #53

Requires the https://github.com/fonttools/fonttools/compare/wip-variable-colr?expand=1 branch of fonttools to work

EDIT: the branch is now called https://github.com/fonttools/fonttools/compare/variable-colr?expand=1 (without the "wip-")

@anthrotype
Copy link
Copy Markdown
Contributor Author

anthrotype commented May 9, 2022

oops, clicked the green button too soon 😅
well, i'll mark it as a draft for it requires stuff that hasn't been merged in fonttools yet

@anthrotype anthrotype changed the title Add support for variable COLR tables using VarIndexBase and DeltaSetI… Add support for variable COLR tables using VarIndexBase and DeltaSetIndexMap May 9, 2022
@anthrotype anthrotype marked this pull request as draft May 9, 2022 10:57
@justvanrossum
Copy link
Copy Markdown
Member

Thanks for getting this started! Much apreciated.

the offset (from VarIndexBase + {offset}) is implicit in the ordering of the variable attributes, thus we use enumerate to construct the dict.
@anthrotype anthrotype marked this pull request as ready for review July 6, 2022 11:19
@anthrotype
Copy link
Copy Markdown
Contributor Author

I suppose this PR needs tests to be added. I am not familiar with the test suite. Mind leaving that to you? I'll be on leave for a couple weeks from tomorrow.

@justvanrossum
Copy link
Copy Markdown
Member

Thanks! Are there any Variable COLRv1 test fonts around?

@justvanrossum
Copy link
Copy Markdown
Member

I suppose this PR needs tests to be added. I am not familiar with the test suite. Mind leaving that to you?

Absolutely. Thank you so much for the implementation!

@anthrotype
Copy link
Copy Markdown
Contributor Author

if you want a really simple one you can cd in fontTools/Tests/varLib/data and run fonttools varLib TestVariableCOLR.designspace and you get a simple TestVariableCOLR-VF.ttf with two color glyphs glyph, A and B that do very simple things.
@drott is working on more test fonts in googlefonts/color-fonts#102

@justvanrossum
Copy link
Copy Markdown
Member

I added the fonttools test font + glyphs to the test suite. Can you briefly verify that the images in the expected output are as you expect them to be?

@anthrotype
Copy link
Copy Markdown
Contributor Author

Can you briefly verify that the images in the expected output are as you expect them to be?

yes, they look correct, thanks!

Copy link
Copy Markdown
Member

@justvanrossum justvanrossum left a comment

Choose a reason for hiding this comment

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

Fantastic! More test cases would be nice, but let's do that later.

@justvanrossum justvanrossum merged commit 530caa3 into fontra:main Jul 6, 2022
@anthrotype anthrotype deleted the variable-colr branch July 6, 2022 13:52
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.

2 participants