Show kerning groups in the sidebar. #241
Conversation
xorgy
changed the title
|
I think this is good to go now. :+ ) |
xorgy
force-pushed
the
sidebar-show-kern-group
branch
from
6084ca0
to
a0b9f50
Compare
| let mut kern1_group = "".to_string(); | ||
| let mut kern2_group = "".to_string(); |
There was a problem hiding this comment.
I guess a few different things here:
- should these be
Optioninstead ofString? - you might prefer
String::new()for creating a new, emptyString.
At a higher level: I wonder if runebender shouldn't keep track of kern groups via some other mechanism, and only extract / write back to the UFO format as needed? trying to manipulate this directly just feels so awkward.
| @@ -762,10 +768,13 @@ mod lenses { | |||
| let outline = data.get_bezier(&glyph.name); | |||
| let is_placeholder = outline.is_none(); | |||
| let metrics = data.info.metrics.clone(); | |||
| let (kern1_group, kern2_group) = find_kern_groups(data, glyph); | |||
There was a problem hiding this comment.
lens get processed a lot, so I'm cautious of doing work in them that might get expensive. I guess this isn't too bad, being O(nm) over the number of glyphs and number of groups, which have practical upper bounds?
There was a problem hiding this comment.
Due to the way that it searches, I think it is always worst case on n, and the typical case for m is hard to say.
The better way to do this would be to have the core glyph datastructure hold the kern1 and kern2 group until it's time to save, since UFO demands there only be one of them per glyph anyway. The thing that makes that messy is that the glyphs as they come from Norad are not coupled to the font overall; this is nice for concurrent loading, but because UFO is kinda a mess, it makes it uglier to meet the constraints.
There was a problem hiding this comment.
so maybe a compromise would be to have something like a HashMap<GlyphName, [GroupName; 2]> in the runebender FontObject, or something?
There was a problem hiding this comment.
Hmm, that could work; it would definitely be faster. This seems like a problem that should be solved at the Norad level though, since it deals with maintaining a spec constraint.
There was a problem hiding this comment.
I'd prefer norad to be a fairly transparent mapping of the UFO object on disk; so while I agree it's a bit of a funny fit in runebender that's still my current preference? I'm not sure exactly what the rules/constraints are with kern groups though, so maybe @madig has an opinion?
There was a problem hiding this comment.
Maybe between Norad and Runebender, we need uhh... a federal bureaucracy.
There was a problem hiding this comment.
RE the constraints on kern groups: the UFO spec says that a given glyph must not appear in more than one kerning group per side.
There was a problem hiding this comment.
@madig how would you feel if we had, in norad, a representation of kerning groups that was distinct from what we load from and write to disk? This would just be some sort of map, where each glyph could have left and right kerning groups assigned. This seems better than exposing a literal representation of the UFO kern-groups to users?
There was a problem hiding this comment.
I'm fine with whatever fits Runebender best, but keep in mind that splitting data out of structures requires that you be careful when merging them back in. If you split out kerning groups into their own structure and the user somehow writes kerning groups into the original structure from out under you (scripting), you'll overwrite things or end up with inconsistent state.
Glyphs also has a UI where you set left and right group at glyph level and it serializes into a HashMap<String, HashMap<String, String>>.
No description provided.