write_fonts::FontBuilder not very ergonomic for font modification

[simoncozens]

Consider this (real) situation: a library has a number of functions - one which fixes problems with the name table; one which fixes problems with the fvar table instances; one which fixes problems with the STAT table. In Python, these would have signatures like:

fix_name(font: TTFont) # Modifies TTFont
fix_fvar(font: TTFont) 
fix_stat(font: TTFont)

How would we port this to Rust? Sounds easy enough, but these tables have dependencies on each other - altering the fvar table may require adding new name table entries or deleting some, and similarly for STAT. If we had a "writeable FontRef", this would be doable:

fix_name(font: &mut write_fonts::FontRef);
fix_fvar(font: &mut write_fonts::FontRef);
fix_stat(font: &mut write_fonts::FontRef);

The closest we have to that is a FontBuilder, but if we have a FontBuilder object, we can't get at the name table any more; there's no way to .get() a table from inside it. So instead right now, we are forced to do this:

fix_name(font: FontRef) -> Result<Vec<u8>, Error> { 
    let mut new_font = FontBuilder::new();
    let name: Name = font.name()?.to_owned_table();
    do_stuff(&mut name);
    new_font.add_table(&name_table)?;
    Ok(new_font.copy_missing_tables(font).build())
}

And now, if I want to fix the name and the fear and the STAT, I have to decompile, modify, copy, compile several times over. Urgh. The alternative is to return modified Name, Stat, etc. structs, but that doesn't really help - if you want to modify multiple things at once, you have to also take these modified structs as (individual!) inputs to your function instead of a FontRef, and that's also nasty for the user. What we want is a bag of write_fonts::table structs that we can compile when we've done modifying them - that would give us a writable FontRef, the write_fonts equivalent of a read_fonts::FontRef.

The reason we can't have this right now is that add_table compiles the table, and build just builds the offset table and the table directory. If we could instead stick write_fonts::table::Name, write_fonts::table::Fvar, etc. structs into the FontBuilder - and if build() did all the compilation at the end - then it would be much easier to get those table structs out again and re-modify them.

[cmyr]

agreed that sounds bad. There are sort of competing needs at play, here; on the one hand we want to be efficient and let the user give us unmodified bytes if those already exist, but on the other hand we want mutability.

Maybe there's a sort of 'copy on write' model we could use, where you can set a table using either raw bytes or a write_fonts table object, but then there are methods with signatures like,

impl FancierTableBuilder {
fn name_mut(&mut self) -> Option<&mut write_fonts::tables::name::Name>;
}

and then behind the scenes, if we only have the raw bytes we will go and deserialize them into the table, transparently?

we could also of course do what you're suggesting, and just always require the user to roundtrip through write_fonts types? It's mostly a question of whether or not we ever expect to be using this in some context (like subsetting) where we really expect the performance to matter.

[simoncozens]

Well, maybe this doesn't need to be part of FontBuilder - so long as write_fonts provides the functionality somewhere. There's a lot of fontTools code out there which needs porting. :-) What I mean is, there could be a separate write_fonts::Font structure which is in the "bag of mutable tables" format that fontTools users expect, for use by people who are not building a font from scratch.

It's probably easier to show you some code than explain it:

pub struct Font(BTreeMap<Tag, MutableTableEnum>);

enum MutableTableEnum {
    Avar(write_fonts::tables::avar),
    // ...
    CompiledTable(Vec<u8>), // Either we got handed some bytes, or we have a table we can't decompile
}

trait ToOwnedFont {
    fn to_owned_font(&self) -> Font;
}

impl ToOwnedFont for read_fonts::FontRef {}

impl Font {
    fn serialize(&self) -> Result<Vec<u8>, Error> {
        let mut builder = FontBuilder::new();
        for (tag, table) in self.0.iter() {
            // compile table if needed
            builder.add_table(&compiled_table)?;
        }
        builder.build()
    }
    // get, set, etc.
}

Of course this could be a separate project, but it seems like a use case which is a nice thing to support.

[rsheeter]

+1 to include a way easily mutate tables in an existing font at cost of likely not hitting subsetter level performance. Gradually replacing the sorts of things we do with fonttools is a core goal for write-fonts. I like @cmyr idea of providng an interface that could minimize work done to unmodified tables, e.g. you start with a full font and explicitly ask for mutable tables from it, meaning if you never asked for a given table we might avoid work.

I might guess it's very reasonable to modify several at once so being able to take several in a manner that avoids forgetting to give them back seems important?

[cmyr]

Yea, there might be some challenges in designing an API that allows for mutable access to multiple tables at once.

I definitely think we'd like to have something like this, at some point? @simoncozens I'd suggest you just play around "out of tree" on whatever API you think is best suited to your particular use-case, and if you come up with something that feels good we can consider merging it, and if I'm not satisfied that might prod me into sketching out what I think this could look like.

[simoncozens]

Yep, I'd come to that conclusion. I'll come up with an interface inside of Fontspector that does what I need, and we'll see if it makes sense to upstream.