yeslogic / allsorts

Font parser, shaping engine, and subsetter implemented in Rust
https://yeslogic.com/blog/allsorts-rust-font-shaping-engine/
Apache License 2.0
706 stars 23 forks source link

Lifetimes are extremely difficult (impossible?) to navigate #52

Open LoganDark opened 2 years ago

LoganDark commented 2 years ago

Rasterization and such is very complex in this library, so I wanted to abstract parts of it (i.e. getting an OutlineBuilder) to make it easier. But that's not possible, because of how this library uses lifetimes.

Basic stuff like this does not work:

    pub fn from_bytes(bytes: &'a [u8]) -> Result<Self, FromBytesError> {
        let font_data = ReadScope::new(bytes.as_ref()).read::<FontData<'a>>()?;
        let provider = font_data.table_provider(0)?;
        let font = match allsorts::Font::new(provider)? {
            Some(font) => font,
            None => return Err(FromBytesError::NoFont)
        };

        let head_table = match font.head_table()? {
            Some(head_table) => head_table,
            None => return Err(FromBytesError::NoHeadTable)
        };

        Ok(Self { font_data, font, head_table })
    }

due to the ownership rules required by allsorts. For example, font (well actually provider) depends on font_data's lifetime instead of bytes.

(I managed to get that function working, I think, using boxes, but the below case I haven't been able to solve at all)

it only goes downhill from here. Let's say I wanted to outsource the process of finding an OutlineBuilder, to a function. I want to have that function do all the stupid parsing of tables for me so I can just focus on rasterizing the outlines

enum GlyphOutlineProvider<'a> {
    CFF(CFF<'a>),
    GLYF(GlyfTable<'a>)
}

#[derive(thiserror::Error, Eq, PartialEq, Debug)]
enum GlyphOutlineProviderOutlineBuilderError {
    #[error("{0:?}")]
    CFF(#[from] CFFError),

    #[error("{0:?}")]
    GLYF(#[from] ParseError)
}

impl<'a> OutlineBuilder for GlyphOutlineProvider<'a> {
    type Error = GlyphOutlineProviderOutlineBuilderError;

    fn visit<S: OutlineSink>(&mut self, glyph_index: u16, sink: &mut S) -> Result<(), Self::Error> {
        match self {
            Self::CFF(cff) => cff.visit(glyph_index, sink).map_err(Self::Error::CFF),
            Self::GLYF(glyf) => glyf.visit(glyph_index, sink).map_err(Self::Error::GLYF)
        }
    }
}

impl<'a> Font<'a> {
    fn glyph_outline_provider_for(font: &mut allsorts::Font<DynamicFontTableProvider>, head_table: &HeadTable) -> Result<GlyphOutlineProvider<'a>, ParseError> {
        if font.glyph_table_flags.contains(GlyphTableFlags::CFF) && font.font_table_provider.sfnt_version() == OTTO {
            let cff_data = font.font_table_provider.read_table_data(CFF)?;
            let cff = ReadScope::new(&*cff_data).read::<CFF<'_>>()?;

            Ok(GlyphOutlineProvider::CFF(cff))
        } else if font.glyph_table_flags.contains(GlyphTableFlags::GLYF) {
            let loca_data = font.font_table_provider.read_table_data(LOCA)?;

            let loca = ReadScope::new(&*loca_data).read_dep::<LocaTable<'_>>((
                font.maxp_table.num_glyphs as usize,
                head_table.index_to_loc_format
            ))?;

            let glyf_data = font.font_table_provider.read_table_data(GLYF)?;
            let glyf = ReadScope::new(&*glyf_data).read_dep::<GlyfTable<'_>>(&loca)?;

            Ok(GlyphOutlineProvider::GLYF(glyf))
        } else {
            panic!("no CFF or GLYF tables in the font?");
        }
    }
}

Whoops, Rust complains. Looks like cff_data depends on the lifetime of font instead of the data that it points to. And cff then depends on cff_data instead of the data that font points to. You can probably see where I'm going with this. This feels like a mess and it's impossible for me to navigate.

I can't store the buffer as a box in GlyphOutlineProvider because that would break rust's ownership rules (the box's contents are immutably borrowed, but someone else would be able to mutate the box if it were returned, so rust complains that the box is borrowed when I try to put it into the enum). And that's the end of that, there's really no other way to do this. Storing it under the struct is a bust because that would borrow the struct indefinitely. Unsafe code is a bust because it's way too hard to come up with something that is sound and doesn't cause UB. So on so on...

I would really love for everything in this library to just use the same lifetime, the lifetime of the slice that Font loads its data from. Because right now, the situation is kind of unmanageable for me.

I talked with @wezm over Discord and they recommended I create this issue

LoganDark commented 2 years ago

It appears that this is also a sort-of-solved problem with crates like ouroboros or moveit. However, it's not obvious that those crates exist or where to find them, so I didn't know they existed when writing this issue. :/

wezm commented 2 years ago

Yes they can be useful. We already use ouroboros in Allsorts for holding table data along with the parsed table, sorry I didn't mention it earlier. https://github.com/yeslogic/allsorts/blob/551f7cb96e1bc02b7ac77395b3d731797c5e60ef/src/font.rs#L99

LoganDark commented 2 years ago

Yes they can be useful. We already use ouroboros in Allsorts for holding table data along with the parsed table, sorry I didn't mention it earlier. https://github.com/yeslogic/prince/blob/35b30c568d2e32000013570b353f71e94ac64420/src/fonts/allsorts/src/font.rs#L99

Doesn't look like that repository is public. Don't worry, I get it :)

wezm commented 2 years ago

Oh sorry, it is public, I copied from the wrong repo. I've updated the link.

LoganDark commented 2 years ago

I've updated the link.

In my comment xD. But it works!

DemiMarie commented 1 year ago

Is this a problem?

wezm commented 1 year ago

I can't say for sure whether it's a problem still—it will depend on your use-case. However, this change from last year may have made things better: https://github.com/yeslogic/allsorts/commit/3d42e51c14bd70743a173c46908a4c667dad5b19

DemiMarie commented 1 year ago

I can't say for sure whether it's a problem still—it will depend on your use-case. However, this change from last year may have made things better: 3d42e51

What about for toolkits like Slint?

wezm commented 1 year ago

I expect you might run into other limitations even if you didn't have this particular problem. E.g. to implement a text field I think #31 would need to be implemented.

fogzot commented 10 months ago

I have the same problem with the references. I'd like to cache the Font instance and be able to pass to different functions the cache (a simple Hash<String,Box<CachedFont>> where CachedFont keeps track of the initial [u8] buffer and everything else needed up to Font) but I am fighting with the fact that every intermediate object (the reader, the table provider and so on) depends on the initial &[u8]. Is there any kind of "official" way to do this? Some example code maybe?