Please implement `.into::<write_fonts::tables::whatever>()` for all `read_fonts::tables::Whatever`

[simoncozens]

We recently wanted to modify a name table entry in a font in fontspector, and this is what we had to do:

        let new_record = NameRecord::new(
            3,
            1,
            0x0409,
            NameId::LICENSE_DESCRIPTION,
            OFL_BODY_TEXT.to_string().into(),
        );
        let mut new_records: BTreeSet<NameRecord> = name
            .name_record()
            .iter()
            .filter(|record| record.name_id() != NameId::LICENSE_DESCRIPTION)
            .map(|r| {
                #[allow(clippy::unwrap_used)]
                NameRecord::new(
                    r.platform_id(),
                    r.encoding_id(),
                    r.language_id(),
                    r.name_id(),
                    r.string(name.string_data())
                        .unwrap()
                        .chars()
                        .collect::<String>()
                        .to_string()
                        .into(),
                )
            })
            .collect();
        new_records.insert(new_record);
        let new_nametable = Name::new(new_records);
        let new_bytes = FontBuilder::new()
            .add_table(&new_nametable)
            .unwrap()
            .copy_missing_tables(font)
            .build();

This would be much more fluent if we could easily turn a read_fonts::NameRecord into a write_fonts::NameRecord:

        let new_nametable = Name::new(name
            .name_record()
            .iter()
            .map(|r| {
                let mut record = r.into();
                if record.name_id() == NameId::LICENSE_DESCRIPTION { record.string =  OFL_BODY_TEXT.to_string().into() }
                record
            })
            .collect()
        );
        let new_bytes = FontBuilder::new()
            .add_table(&new_nametable)
            .unwrap()
            .copy_missing_tables(font)
            .build();

which is much better.

[cmyr]

So this does exist for at least most tables (but not records) but it doesn't use From and Into because in the most general case we need to pass in the data that will be used to resolve fields that are offsets; instead we have two pairs of traits, FromObjRef/ToOwnedObj (which require offset data) and FromTableRef/ToOwnedTable which don't (because a table has its own offset data).

So given a read_fonts Name table, you can do,

use write_fonts::from_obj::ToOwnedTable;
let write_name: write_fonts::tables::name::Name = read_name.to_owned_table();

which maybe is what you're looking for?

iirc there was some issue with impling From and Into directly, possibly related to the orphan rules? There's an old issue #723 about trying to improve the ergonomics here.

[simoncozens]

Thanks, I will give that a try. We don't really have many examples of modifying fonts in different ways, so I'm hoping that as we continue to port things, we will discover what the needs and the patterns are.

[simoncozens]

Well this is much tidier:

        let mut name: Name = f.font().name().unwrap().to_owned_table();
        // Can't mutate it directly because it's a BTreeSet, convert to Vec
        let mut records = name.name_record.into_iter().collect::<Vec<NameRecord>>();
        for record in records.iter_mut() {
            if let NameRecord {
                name_id: NameId::LICENSE_DESCRIPTION,
                platform_id: 3,
                encoding_id: 1,
                language_id: 0x409,
                ..
            } = record
            {
                record.string = OFL_BODY_TEXT.to_string().into();
            }
        }
        name.name_record = records.into_iter().collect();
        let new_bytes = FontBuilder::new()
            .add_table(&name)
            .unwrap()
            .copy_missing_tables(f.font())
            .build();

Now the only gnarly bit is that name_records is a BTreeSet which isn't an ideal data structure for mutation - and write_fonts is all about mutation...

[rsheeter]

This would be much more fluent if we could easily turn a read_fonts::NameRecord into a write_fonts::NameRecord

I wonder how we could make to owned more obvious/discoverable, whether there is somewhere we could add docs or a ref or w/e to make it easier.

For context, https://docs.rs/write-fonts/latest/write_fonts/#loading-and-modifying-fonts aspires to explain about to owned. Did you look at the docs? Were they unclear? Did you look somewhere else?

[simoncozens]

I did read that section. I personally found it unclear. (Mentioning traits without mentioning the methods they provide and why you might want them is a blind spot for me.) Of course now I've been told that the first sentence is the important bit, it all makes sense.

I prefer docs in the "Here is something you want to do. (Here are some problems you may face.) Here's how to do it. Here's a real life example." format. I'll send a doc PR.

[cmyr]

Now the only gnarly bit is that name_records is a BTreeSet which isn't an ideal data structure for mutation - and write_fonts is all about mutation...

Interesting, I hadn't really thought about how BTreeSet can't implement iter_mut.

The reason it's a BTreeSet is because it lets us enforce the invariant that NameRecords are always sorted. We could do this instead with some custom SortedVec type or similar, but maybe that's trading one annoyance for another...

[simoncozens]

Just sort them when you build them?

[cmyr]

We don't have mutable access to the objects when writing them. We could provide custom logic for writing that handled the sorting (there's an annotation for that) and given that we need a custom annotation anyway (to specify that we use BTreeSet, here) maybe that would be preferable? Shouldn't be too tricky...