alexheretic / ab-glyph

Rust API for loading, scaling, positioning and rasterizing OpenType font glyphs
Apache License 2.0
372 stars 24 forks source link

Outline with custom `OutlineBuilder` #81

Closed coastalwhite closed 1 week ago

coastalwhite commented 1 year ago

Would it be possible to utilize a custom owned_ttf_parser::OutlineBuilder to outline a glyph? The current implementation embeds the MoveTo and Close operations into the outline curves by adding extra points. This cannot be reliably extracted anymore, since f32s do not allow for reliable equalities. Therefore, it would be really handy to have a outline_with_builder method that provides similar behavior to outline however allows of use of a custom OutlineBuilder.

Example

use ab_glyph::{point, Font, FontRef, Glyph, OutlineBuilder};

enum OutlineCurve {
    Move(f32, f32),
    Line(f32, f32),
    Cubic(f32, f32, f32, f32, f32, f32),
    Quad(f32, f32, f32, f32),
    Close,
}

#[derive(Default)]
struct SvgOutlineBuilder(Vec<OutlineCurve>);

impl OutlineBuilder for SvgOutlineBuilder {
    fn move_to(&mut self, x: f32, y: f32) {
        self.0.push(OutlineCurve::Move(x, y))
    }

    fn line_to(&mut self, x: f32, y: f32) {
        self.0.push(OutlineCurve::Line(x, y))
    }

    fn curve_to(&mut self, x1: f32, y1: f32, x2: f32, y2: f32, x: f32, y: f32) {
        self.0.push(OutlineCurve::Cubic(x1, y1, x2, y2, x, y))
    }

    fn quad_to(&mut self, x1: f32, y1: f32, x: f32, y: f32) {
        self.0.push(OutlineCurve::Quad(x1, y1, x, y))
    }

    fn close(&mut self) {
        self.0.push(OutlineCurve::Close)
    }
}

let q_glyph: Glyph = font
    .glyph_id('q')
    .with_scale_and_position(24.0, point(100.0, 0.0));

let mut outline_builder = SvgOutlineBuilder::default();
let outline = font.outline_with_builder(q_glyph, &mut outline_builder);

This is not currently possible with the current API. This should probably also include reexporting the OutlineBuilder from owned_ttf_parser.

I apologize for filing yet another issue, but I feel like this could make the crate more useful in a wider range of use cases. If you feel like this is a welcome change, I would be happy to submit a PR for it.

alexheretic commented 1 year ago

Yes I can see how existing Font::outline isn't ideal for this case.

I don't re-export ttf-parser in this crate to isolate it from upstream breaking changes.

The following approaches come to mind:

  1. Add ttf-parser outline builder proxies to ab-glyph.

    This is closest to your suggested solution + extra code to isolate from upstream breaks.

  2. Add move & close variants to ab_glyph::OutlineCurve.

    This feels more correct as this is the crate's abstraction on ttf-parsers outline builder. It seems better to have just one way of getting outline data. The big downside is this would be a breaking change.

  3. Advise using ttf-parser directly for custom outlining.

    This is an approach that works now.

My initial thoughts are I'd like (2) but it might take a little longer to land as I'll want to bundle up some other potential breaking changes & I try to minimise these to minimise churn downstream.

So we could keep doing (3) until the next version is out with (2). Alternatively we add in (1) as a non-breaking addition and perhaps remove it in the next version. wdyt?

coastalwhite commented 1 year ago

Thank you. I will file a PR for (1) which we can use until a major release and see if I can also add something for (2), but it would probably require changes to the rasterizer code which you should check since I am not very familiar with this process.

alexheretic commented 1 year ago

Sounds good. I've raised an issue for (2) that I can group with other breakers later.

coastalwhite commented 1 year ago

(1) is actually quite a lot more complicated than I hoped. Since Font is used as a trait object, we cannot use generics in the trait interface. I will see if I can find a workaround :disappointed:

alexheretic commented 3 months ago

How about #108?

morn-0 commented 3 months ago

How about #108?

I think it's okay.

alexheretic commented 3 months ago

Thanks. And can you describe your use case briefly, why does it help having this here instead of using ttf-parser directly?

alexheretic commented 1 week ago

I don't have enough info to merge this now, can reopen if that changes.