API design questions about fontdue

[mooman219]

Thanks for the input!

Rasterization API
Rasterization is fairly straight forward in that it will always produce a result as specified by the font. If the font doesn't include a glyph, the glyph in the 0 index is rasterized. What's happening in your case is the font does specify a bitmap for ' ', it's just nothing, and it doesn't specify a bitmap for '\n'. Right now the only way to figure this out is to run lookup_glyph_index and check if it's 0. This probably should be info on the metrics.

Layout API
Okay being honest I threw together layout and it's definitely a sketchy prototype right now. I added a notice on the readme.md that layout is definitely immature in response to help set expectations currently. I believe I mishandle control characters right now, and I agree that the user_data issue is a problem in the current API that needs to be solved, and it wasn't initially included because I've been the only user of fontdue for a long time and didn't need it.

I'm in the middle of a layout rewrite right now that fixes a couple of the bugs opened in the last 7 days. The rewrite so far leaves the API looking like:

pub fn main() {
    let font = include_bytes!("../resources/fonts/Roboto-Regular.ttf") as &[u8];
    let roboto_regular = fontdue::Font::from_bytes(font, fontdue::FontSettings::default()).unwrap();
    let mut layout = Layout::new(CoordinateSystem::PositiveYUp);
    layout.reset(&LayoutSettings {
        ..LayoutSettings::default()
    });
    let fonts = &[roboto_regular];
    layout.append(fonts, &TextStyle::new("Hello ", 35.0, 0));
    layout.append(fonts, &TextStyle::new("world!", 40.0, 0));
    for glyph in layout.glyphs() {
        println!("{:?}", glyph);
    }
}

This design lets me expose more intermediate data like line count / height / etc, lets new text be more optimally laid out, and gives me more opportunities to add functions like inserting padding. Unfortunately, this API would make using a style index annoying for everyone involved, so it looks like stashing some metadata in there is the best option. I saw your PR and just learned that you can provide a default concrete type for a generic which is amazing. I'd be like to merge just the user data bit right now, but also I'll probably have the rewrite in a better state than current layout by this weekend and would rather wait for that.

The gist
Rasterization should include a flag on metrics if the glyph is in the font, layout should handle control characters better and include userdata on output glyphs. The user data probably needs to be copy for this to work reasonably. Do these changes seem reasonable?

[bschwind]

What's happening in your case is the font does specify a bitmap for ' ', it's just nothing, and it doesn't specify a bitmap for '\n'. Right now the only way to figure this out is to run lookup_glyph_index and check if it's 0. This probably should be info on the metrics.

Makes sense! I think that's enough info for me to work around, combined with something like char.is_whitespace()

Okay being honest I threw together layout and it's definitely a sketchy prototype right now

It's still quite nice and useful though, I appreciate that it exists!

I'd be like to merge just the user data bit right now, but also I'll probably have the rewrite in a better state than current layout by this weekend and would rather wait for that.

Cool, I'll remove the TextStyle index code from the PR but you're also welcome to close it or use whatever code from it in your new layout system. I'm not sure yet if it will interact well with your new API but we can find out!

The user data probably needs to be copy for this to work reasonably. Do these changes seem reasonable?

In my PR the user data actually ends up being borrowed and it all works out due to the lifetimes on the input TextStyle structs. So if that's the case the user data doesn't need to be Copy and ideally we can avoid doing any extra allocations for it by just having a reference to it. Once you have the new layout code I can take another crack at implementing it in that way.

Also as a disclaimer, I haven't made a text/rasterization API before so definitely compare my suggestions with existing work and ideally get other fontdue users' opinions. I'd feel terrible if an API suggestion that benefits me gets implemented and causes pain for everyone else 😅

[mooman219]

Mostly fixed with the last commit + your PR. There's a Copy + Clone restriction right now that I'll deal with later. Feel free to open another issue with comments/concerns on the new API and

[bschwind]

There's a Copy + Clone restriction right now that'll deal with later.

I see why you did that now, with separate calls to layout() being allowed now. It would definitely be nice to have a zero-copy version which uses references + lifetimes to metadata to avoid that. Let me know if you want to bounce ideas around on that, because although it's more efficient, the API is less ergonomic.

[mooman219]

Given the first rewrite is on HEAD now, if you have any suggestions or want to make another PR, I'm very open to suggestions on making it more ergonomic. I really appreciate your first PR's design of not having the user deal with generics if they don't want to use the user data.

[bschwind]

I'll have to think about it some more, maybe some benchmarking is needed too. Another approach could be to use Rc<UserData>, but that could possibly end up being more expensive than just copying a small value around.

I see you have some benchmarks so I'll play around with those and see what I come up with :)

I added a version based on the current HEAD which uses lifetimes and references but like we discovered, it makes usage of the layout API less ergonomic, especially if you want to pass references to temporary variables in a call to layout.append()

[bschwind]

Hey again! I haven't done any benchmarks yet but I just integrated the latest version of fontdue into an open-source game I'm working on. It uses wgpu, you can check it out here:

bschwind/sus#3

The newer API is much nicer for knowing how to style the glyphs. The only snag I ran into was knowing to call layout.reset(&layout_settings); when rendering a block of text in a particular location, but that wasn't a big deal.

I need to structure my text drawing API similarly to fontdue so I can queue up a bunch of differently placed text blocks without incurring a GPU draw call for each one.

Anyway, great work on the library so far, seems to be working great!

[mooman219]

This is great! The latest commit is on crates.io under 0.4 right now, so no need to use a revision if you don't want to.

The only snag I ran into was knowing to call layout.reset(&layout_settings)

My soft intention for Layout is that game logic owns it and passes it into the renderer/engine. This way you can actually leverage the extra perf from append (and eventually deleting) vs redoing layout for the whole text section.

[bschwind]

My soft intention for Layout is that game logic owns it and passes it into the renderer/engine. This way you can actually leverage the extra perf from append (and eventually deleting) vs redoing layout for the whole text section.

I see. Does that mean for every independent block of text (for example, a right-justified block of test and some centered text) I should maintain a separate Layout struct instead of calling reset() on the singular Layout struct I currently have?

Thanks for publishing 0.4 by the way, I didn't even notice!

[mooman219]

Up to you really to see how it works out in your use-case. Recalculating layout will be more expensive than reallocating heap.

Reusing Layout and just resetting state means you're not reallocating heap for state.
Reusing Layout and appending on the new characters means you're both not reallocating heap and not recalculating positioning.