fontations icon indicating copy to clipboard operation
fontations copied to clipboard

charmap().mappings() and Cmap::from_mappings have different types

Open simoncozens opened this issue 3 months ago • 6 comments

Boy this took me too long to work out: write_fonts::tables::Cmap::from_mappings(font.charmap().mappings().chain(...)) doesn't work, because:

pub fn from_mappings(
        mappings: impl IntoIterator<Item = (char, GlyphId)>,
    )

but

impl Iterator for Mappings<'_> {
    type Item = (u32, GlyphId);

They're different crates so there's no reason to expect that they should be the same, but the (lack of) parallelism is confusing.

simoncozens avatar Sep 29 '25 16:09 simoncozens

yea that is a bit annoying, and I'm not sure what makes the most sense.

  • on the write-fonts side, char is nice, since it makes it impossible for garbage u32s to be used
  • on the skrifa side, u32 makes sense, because we don't know that all the values are valid.

Of these two it feels more possible to change write-fonts, but I'm not sure if it's worth giving up the nice qualities of char?

cmyr avatar Oct 01 '25 22:10 cmyr

I think write-fonts shouldn't be in the business of policing what people put into the font file. If the format allows it, write-font should try to allow it IMO.

behdad avatar Oct 02 '25 19:10 behdad

Seems like thats two votes for u32 then?

rsheeter avatar Oct 02 '25 19:10 rsheeter

I think write-fonts shouldn't be in the business of policing what people put into the font file. If the format allows it, write-font should try to allow it IMO.

I don't entirely agree: if there are clear expectations about the values in a table (for instance that a count field should match the length of an array, or if a specific table version requires the presence of an additional field) then write-fonts should, where feasible, make it at least difficult to construct an invalid table.

I do think that there is a fine line here, and we don't want to be so assertive that it becomes annoying. That said, I can think of no reason why we would ever want to create font files that included invalid codepoints. (I'm not opposed to not using char, but I would only prefer u32 if there are clear ergonomics advantages, not because I think we should always leave validation up to the tool user.)

cmyr avatar Oct 03 '25 13:10 cmyr

I don't entirely agree: if there are clear expectations about the values in a table (for instance that a count field should match the length of an array, or if a specific table version requires the presence of an additional field) then write-fonts should, where feasible, make it at least difficult to construct an invalid table.

One sometimes needs to make invalid fonts (e.g. writing test cases). It is not a common use case by any stretch, but I think it is a valid one.

That said, I can think of no reason why we would ever want to create font files that included invalid codepoints.

Round-tripping one be one such reason. If someone is changing a hypothetical cmap table that has such a mapping, with such a restriction they can’t round-trip the existing mappings. In general, low-level libraries should try to get out of the way of its user. Such policing belongs to higher-level libraries or APIs.

khaledhosny avatar Oct 04 '25 05:10 khaledhosny

I don't entirely agree: if there are clear expectations about the values in a table (for instance that a count field should match the length of an array, or if a specific table version requires the presence of an additional field) then write-fonts should, where feasible, make it at least difficult to construct an invalid table.

One sometimes needs to make invalid fonts (e.g. writing test cases). It is not a common use case by any stretch, but I think it is a valid one.

Sure! For this we generally use the BeBuffer type to just let the user write bytes directly. If it ever arises that we have a user who needs to make more complicated invalid graphs of tables there are various ways we could support that.

That said, I can think of no reason why we would ever want to create font files that included invalid codepoints.

Round-tripping one be one such reason. If someone is changing a hypothetical cmap table that has such a mapping, with such a restriction they can’t round-trip the existing mappings. In general, low-level libraries should try to get out of the way of its user. Such policing belongs to higher-level libraries or APIs.

Round tripping is already supported; we will ready any data at all into a write_fonts table, and although we do by default run a validation pass before writing out table bytes, the user can skip this if they prefer. This conversation is specifically about a public constructor for creating a new table from scratch.

And to be clear I don't oppose using u32 here!

cmyr avatar Oct 04 '25 16:10 cmyr