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

Decouple the closure based glyph segmentation functionality from GlyphSegmentation. #98

Merged
merged 1 commit into from
Mar 4, 2025

Conversation

garretrieger
Copy link
Contributor

@garretrieger garretrieger commented Feb 24, 2025

  • GlyphSegmentation contains common structures around representing a segmentation.
  • The specific closure based implementation of glyph keyed segmentation is moved to it's own file and own util closure_glyph_segmenter/closure_glyph_keyed_segmenter_util.

@garretrieger garretrieger requested a review from skef February 24, 2025 23:00
@skef
Copy link
Contributor

skef commented Feb 28, 2025

This API assumes that glyph groups will be (in effect) pre-segmented and provides no information about the priority of the segments (e.g. relative frequencies) or the relationships between segments (e.g. which set of locales each relates to). So, it preserves the qualities I've expressed concerns about elsewhere: It assumes that something else has divided up glyphs prior to calling it but what that thing has done is then expressed in terms of codepoints, washing away much of the dependency information it presumably took into account.

Maybe some other group would find a use for this but it doesn't address our needs.

What would help is modularizing encoding as a whole: "Draw a line" through ift::encoder separating out utility functions from "substantive" functions, with a corresponding ability to select among encoders in font2ift and configure them independently. Then the approaches could easily share the code for creating segments, adding conditions, and so forth.

@garretrieger
Copy link
Contributor Author

garretrieger commented Feb 28, 2025

This API assumes that glyph groups will be (in effect) pre-segmented and provides no information about the priority of the segments (e.g. relative frequencies) or the relationships between segments (e.g. which set of locales each relates to). So, it preserves the qualities I've expressed concerns about elsewhere: It assumes that something else has divided up glyphs prior to calling it but what that thing has done is then expressed in terms of codepoints, washing away much of the dependency information it presumably took into account.

The plan is to add both frequency and dependency information to the input, but on further thought since we haven't nailed down exactly what that looks like in the more general case it's probably premature to introduce an interface. I'll rework this to keep the closure based segmentation specific implementation separated out into it's own class but remove the interface for now. Down the line once it's needed we can reintroduce an interface in whatever form makes sense.

Maybe some other group would find a use for this but it doesn't address our needs.

What would help is modularizing encoding as a whole: "Draw a line" through ift::encoder separating out utility functions from "substantive" functions, with a corresponding ability to select among encoders in font2ift and configure them independently. Then the approaches could easily share the code for creating segments, adding conditions, and so forth.

The way I've got font2ift currently set up I believe matches what you're describing. Currently font2ift implements only the serialization part of the encoding process. As an input it takes an encoder config which among other things describes an already fully formed glyph segmentation. font2ift doesn't use any of functionality from closure_glyph_segmenter.*.

So at the present producing an IFT encoded font looks like this:

  1. Some utility/tool is used to generate the glyph keyed and table keyed segmentation plan and outputs that in the form of an encoder config. That could be done by "util/closure_glyph_keyed_segmenter_util", "util/iftb2config", or some other completely different implementation.
  2. The encoder config from step 1 is fed into font2ift which then generates the initial IFT font and all patches according to the plan in the config.

…hSegmentation.

- GlyphSegmentation contains common structures around representing a segmentation.
- The specific closure based implementation of glyph keyed segmentation is moved to it's own file and own util closure_glyph_segmenter/closure_glyph_keyed_segmenter_util.
@garretrieger garretrieger changed the title Add a GlyphSegementer interface and move the closure glyph segmentation code to be an implementation of it. Decouple the closure based glyph segmentation functionality from GlyphSegmentation. Feb 28, 2025
@skef
Copy link
Contributor

skef commented Feb 28, 2025

The way I've got font2ift currently set up I believe matches what you're describing.

I don't think it was before #99 was merged but at a quick glance it's looking closer to that now. I'll look in more detail.

Copy link
Contributor

@skef skef left a comment

Choose a reason for hiding this comment

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

OK, I understand this better now. (I first reviewed this PR relative to my understanding of the repo prior to the last big merge.)

@garretrieger garretrieger merged commit 65653d3 into w3c:main Mar 4, 2025
3 checks passed
@garretrieger garretrieger deleted the segmenter_interface branch March 5, 2025 19:46
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