googlefonts / fontations

Reading and writing font files
Apache License 2.0
395 stars 24 forks source link

It's not obvious which FontData to give #681

Open rsheeter opened 1 year ago

rsheeter commented 1 year ago

Suppose you want to loop over your base anchors to see their coordinates. It's a bit more indirect than you'd hoped but np. And then you hit this:

read_fonts::tables::gpos::BaseRecord
pub fn base_anchors(&self, data: FontData<'a>) -> ArrayOfNullableOffsets<'a, AnchorTable<'a>, Offset16>

Great, which FontData does it want? Pity it's not apparent from type but that's OK, the comment will help right?

    /// A dynamically resolving wrapper for [`base_anchor_offsets`][Self::base_anchor_offsets].
    pub fn base_anchors(
        &self,
        data: FontData<'a>,
    ) -> ArrayOfNullableOffsets<'a, AnchorTable<'a>, Offset16> {
        let offsets = self.base_anchor_offsets();
        ArrayOfNullableOffsets::new(offsets, data, ())
    }

To be fair you can tell the answer from the spec but it's a bit tiresome. It would be nice to make it easier. At the very least add comments to these methods that say where to get the correct FontData. Bonus points for having types so you can't pass the wrong one without making an effort.

For context see https://github.com/googlefonts/fontc/pull/492 GPOS tests such as compile_basic_gpos_mark_base.

rsheeter commented 1 year ago

(you also rapidly run into the VSCode doesn't navigate generated files very well issue)

cmyr commented 1 year ago

One note is that the docs for base_anchor_offsets (referenced here) do actually include the info we want:

https://docs.rs/read-fonts/latest/read_fonts/tables/gpos/struct.BaseRecord.html#method.base_anchor_offsets

the methods like base_anchors were added later, to try and simplify things, and they get their own docs that just point to the method they are wrapping. It should be easy enough to update those to provide a bit more elucidation.

rsheeter commented 1 year ago

base_anchors does simplify things, it just needs a tiny bit of help to make it obvious what to pass it (if we have to pass it something, as a user I would prefer it didn't but I appreciate that's fiddly)