-
Notifications
You must be signed in to change notification settings - Fork 14
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
Explicitly distinguish design and user coordinates and locations #47
Conversation
As I noted in #46, in designspace axes element, the map goes the other way around: i.e. from user to design |
@@ -60,7 +59,7 @@ pub type DesignSpaceLocation = BTreeMap<String, OrderedFloat<f32>>; | |||
#[derive(Serialize, Deserialize, Debug, PartialEq)] | |||
pub struct Glyph { | |||
pub name: String, | |||
pub sources: HashMap<DesignSpaceLocation, GlyphInstance>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually I think for these we don't want UserSpaceLocation. Sources in designspace are always in design-space, not user-space. Instances can avail themselves of being optionally defined in user-space, but sources (aka the designer's "masters") are always design locations because that's what the designer works with, and nobody requested to define sources with user-space location so far.
So I don't think it helps anything to have Glyph::sources keyed by user-space location.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had in mind to leave designspace behind when creating IR and work exclusively in user and normalized space. Is that a bad idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder for self, per f2f I believe the user of internal coordinates was acceptable here.
@@ -50,7 +53,7 @@ impl Cache { | |||
fn new( | |||
global_metadata_sources: StateSet, | |||
glyph_names: HashSet<String>, | |||
locations: HashMap<PathBuf, Vec<DesignSpaceLocation>>, | |||
locations: HashMap<PathBuf, Vec<UserSpaceLocation>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so yeah that's incorrect, the locations of sources in designspace (in this particular case, locations
contains the location of each glif file) are in design-space coordinates, not user-space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My rationale was that for IR generation the userspace location would be what you want?
I think I've now changed this so much it makes sense to make a new PR against #56 |
Introduce explicit user vs design coord and location types and begin to use them in the right places. Conversion is currently entirely bogus, and explicitly noted so.
Intent is that in the end we will explicitly move between them using the mapping type added in #46. I current imagine building a design:user map and storing in StaticMetadata.
Builds on #43. Step toward #22.