search

source-foundry / Slice

An open-source, cross-platform GUI app to generate custom font design spaces from variable fonts
https://slice-gui.netlify.app/
GNU General Public License v3.0
157 stars 9 forks source link

Support restricted variable axis range sub-spaces at Level 3 #31

Closed chrissimpkins closed 3 years ago

chrissimpkins commented 3 years ago

This app currently supports the following sub-space output fonts from input variable fonts:

I've received requests to support variable font outputs that include variable axes with a reduced axis range compared with the input font. This is not currently supported in the fonttools library that we rely on for the slicing operation in this project. (this is partially supported at level 3, see https://github.com/source-foundry/Slice/issues/31#issuecomment-828063680, documentation updated in https://github.com/fonttools/fonttools/pull/2280)

fonttools related issues:

https://github.com/fonttools/fonttools/blob/cf8deef42045fb0a4141e43025a92ae2a203813d/Lib/fontTools/varLib/instancer/__init__.py#L1151

TODO:

cjdunn commented 3 years ago

@chrissimpkins Thank you for adding this! Hereʻs the issue I was following regarding the varLib.instancer road map, specifically this comment: https://github.com/fonttools/fonttools/issues/1537#issuecomment-474983825

chrissimpkins commented 3 years ago

I stand corrected on this issue. L3 instancing is available in fonttools. We can add support here for subsetting an axis range to a smaller range with the current restriction that you must include the axis default value in the new range. L4 support for changing the default value of a range is not currently available in fonttools.

Edit: Pushed PR to fonttools with documentation update to indicate that L3 is supported https://github.com/fonttools/fonttools/pull/2280

chrissimpkins commented 3 years ago

Also https://github.com/fonttools/fonttools/issues/2157 for L4 support

chrissimpkins commented 3 years ago

Ideas about the idiom to use in the application to specify the range:

The first option would follow the fonttools command line definition, but it is simple enough to reformat if something else is more intuitive

cjdunn commented 3 years ago

I like the first option best, it reads to me unambiguously as 100 through 300. And following the fonttools definition makes a lot of sense.

Option 2 could read as 100 and 300. And option 3 isnʻt very intuitive to me. My only other thought would be to consider (100 - 300). But Iʻm not convinced this is better than option 1.

chrissimpkins commented 3 years ago

Sgtm! Coming soon.

chrissimpkins commented 3 years ago

@cjdunn The initial implementation of L3 sub-space support is available in this dmg installer if you'd like to have a look:

https://drive.google.com/file/d/1Kxs9_X-xk0OEMmzwOgtzeYn68Uu4fvUk/view?usp=sharing

This will install v0.7.0-dev1. I used the min_value:max_value syntax that you suggested. It is reasonably permissive syntax that (should) accept spaces (e.g., 200 : 400) and reversed max:min order (e.g., 400:200).

Let me know what you think

Note to self: compiled @ 2855c7727204acbf38f33a7183281a56db3dabf3

cjdunn commented 3 years ago

@chrissimpkins this is awesome! Thank you for sharing this dev version.

The min_value:max_value syntax makes sense, and glad to hear it is reasonable permissive regarding spaces and order.

One suggestion/request: could it also be permissive regarding parens? My first attempt was to type: (300:400) because that's how your default values are presented. Eg. for Recursive you show:(300.0: 1000.0) [300.0] so I was trying to match that formatting. Or if you don't want parens I'd suggest changing the example formatting that you show.

Also, would it make sense to allow the user to repeat the default value in brackets? Eg (300:400) [300] That way they can follow exactly the formatting you have. And at a future date when changing the default axis value is supported the syntax could stay as is? Perhaps now if they type a value other than the current default they get a message saying "changing the default is not yet supported". Does that make sense?

Either way, thank you for the prompt addition of the L3 sub-space support, this is super useful!

chrissimpkins commented 3 years ago

if you don't want parens I'd suggest changing the example formatting that you show.

In the name of keeping it simple and preventing more entry than necessary to get the job done, let's drop the parens altogether.

Thoughts about this format for the axis range display? This bags the commas and parens, replaces with the colon delimiter that will be used for range restriction definitions.

2021-05-03_10-23-15

I like that you are thinking about future-proofing for L4 support! Thoughts about the displayed default value bracket format vs. parens (vs. some other set of delimiters?).

chrissimpkins commented 3 years ago

Maybe better with a space between values:

2021-05-03_10-29-50
chrissimpkins commented 3 years ago

And centered text improves it further depending upon the data in the row:

2021-05-03_10-34-59
cjdunn commented 3 years ago

I think the value range Min : Maxworks great without parens. And brackets make sense for the [Default] Nice work!

chrissimpkins commented 3 years ago

Perfect. Thanks!

Notes to self

Will go with the following regex to parse input. 

(?P<start>\-?\d+(\.\d+)?)\s*\:\s*(?P<end>\-?\d+(\.\d+)?)\s*(\[\s*(?P<default>\-?\d+(\.?\d+)?)\s*\])?

This supports lots of spacing/integer + float combinations and will support the definition of a default axis value when L4 is available in upstream fonttools. For now a default defined with this approach will simply be ignored.

Possible valid input values:

chrissimpkins commented 3 years ago

Updated implementation in 50009fc723ebea70cc6735fca3bf838c7c022fd1 (whoops, missed negative axis range value / negative default values in the parser, added in 14077eef64e58ad09b2b0e30a8d207b82c5e8465) Revised axis value range and default header strings in ec49583f55831e494550ce3c8143a9ea8d6c2f5b

chrissimpkins commented 3 years ago

This will be released in v0.7.0.

chrissimpkins commented 3 years ago

Level 3 VF sub-spacing support was released in v0.7.0. Docs (including doc images) were updated with the new min:max range restriction syntax.

Thanks so much for all of your help with this @cjdunn! Enjoy and please let me know if you come across any issues. Here's hoping that L4 support happens soon!

cc @djrrb

Related: https://github.com/source-foundry/Slice/issues/30#issue-869128327