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

A simpler representation of an axis location? #417

Open
cmyr opened this issue Aug 23, 2023 · 2 comments
Open

A simpler representation of an axis location? #417

cmyr opened this issue Aug 23, 2023 · 2 comments

Comments

@cmyr
Copy link
Member

cmyr commented Aug 23, 2023

the problem

Currently we store axis locations (in both fea-rs and in fontc) as some sort of map (hash or b-tree) of tag->value. In the font binary, these end up stored as a (sparse) array of values, in axis-order.

I'm finding our current representation a bit difficult to work with, and I'm wondering if there would be some benefit to using a representation closer to what ends up in the binary. The idea would be that we have a Location type that is always normalized and is internally stored as Vec<f32>, and then we enforce that these can only be constructed if you have an AxisOrdering (or something) type, which enforces the invariant that the length of the vector equals the number of axes, and they are in the correct order.

benefits

I think this has a number of benefits:

  • we don't need to worry about OrderedFloat
  • we only need to validate/construct once, and conversion to the final representation is trivial
  • storing locations uses less memory (definitely, no storing tags)
  • we're probably computationally more efficient, since computers love arrays (not sure how significant this is, but if we're doing tons of hashing it could be relevant)
  • we don't have to worry as much about the possibility of an unexpected tag or similar, since we're 'valid by construction'

I'm writing this up right now because I'm running into some annoyances in moving to using f32 instead of Fixed, and it's making me revisit some older design ideas I've had. This doesn't need to block the f32 changes; it's more a longer-term evolution idea.

drawbacks

It's possible that there are drawbacks here that I'm not considering?

  • maybe we want to construct locations in places where we do not have access to the axis order?
  • maybe there are implementation challenges to implementing the constructor that I'm not thinking about (what does this API actually look like, how does this work in an avar2 world, etc)

in any case, I think this would likely be at least worth investigating.

@rsheeter
Copy link
Contributor

The drawback of needing a global thing that has to be involved to produce a Location is fairly significant; it's moving toward what Javaland would call the LocationFactory. It complicates every bit of code that ever needs to interact with a Location.

Also, suppose I have some code that wants to work with two sources at once, or even just want to alter the axis order. Now I have several active AxisOrdering. Which one did any given location come from? I guess it has to know? - I fear this gets ugly real quick.

@rsheeter
Copy link
Contributor

A very achievable step that would make my life better is to have just one Location type; receiving fea_rs::compile::variations::AxisLocation not the compilers location type is a nuisance. I propose - again - fea-rs gets to see fontdrasil and we move some stuff from fontir/src/variations.rs there.

I think that might let us avoid the callback interaction via VariationInfo, we could just give the compiler enough information to do unit conversions and compute deltas because it could directly interact with VariationModel.

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

No branches or pull requests

2 participants