Skip to content

Commit

Permalink
More improvements to offset getter docs
Browse files Browse the repository at this point in the history 
- manually update codegen inputs to name parent field where it was
  missing
- simplify docs in case where manual data is required
  • Loading branch information
@cmyr
cmyr committed Nov 1, 2023
1 parent 1c9a234 commit ece8f27
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 178 deletions.
45 changes: 26 additions & 19 deletions font-codegen/src/fields.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,29 @@ impl Field {
quote!( self.shape.#shape_range_fn_name() #try_op )
}

fn typed_offset_getter_docs(&self, has_data_arg: bool) -> TokenStream {
let raw_name = &self.name;
// If there's no arguments than we just link to the raw offset method
if !has_data_arg {
let docs = if self.is_array() {
format!(" A dynamically resolving wrapper for [`{raw_name}`][Self::{raw_name}].")
} else {
format!(" Attempt to resolve [`{raw_name}`][Self::{raw_name}].")
};
return quote!(#[doc = #docs]);
}

// if there is a data argument than we want to be more explicit
let original_docs = &self.attrs.docs;

quote! {
#(#original_docs)*
#[doc = ""]
#[doc = " The `data` argument should be retrieved from the parent table"]
#[doc = " By calling its `offset_data` method."]
}
}

fn typed_offset_field_getter(
&self,
generic: Option<&syn::Ident>,
Expand Down Expand Up @@ -811,19 +834,8 @@ impl Field {
let args = args.to_tokens_for_table_getter();
quote!(let args = #args;)
});
let docs = self.typed_offset_getter_docs(record.is_some());

let original_docs = &self.attrs.docs;

let extra_docs_if_input_data = input_data_if_needed.is_some().then(|| {
quote! {
#[doc = ""]
#[doc = " The `data` argument should generally be retrieved via the `offset_data`"]
#[doc = " method of the parent table, but you should consult the docs of the"]
#[doc = " referenced method (reproduced below)"]
#[doc = ""]
#(#original_docs)*
}
});
if self.is_array() {
let OffsetTarget::Table(target_ident) = target else {
panic!("I don't think arrays of offsets to arrays are in the spec?");
Expand All @@ -850,12 +862,9 @@ impl Field {
}

let bind_offsets = quote!( let offsets = self.#raw_name(); );
let docs =
format!(" A dynamically resolving wrapper for [`{raw_name}`][Self::{raw_name}].");

Some(quote! {
#[doc = #docs]
#extra_docs_if_input_data
#docs
pub fn #getter_name (&self #input_data_if_needed) -> #return_type #where_read_clause {
#data_alias_if_needed
#bind_offsets
Expand All @@ -872,7 +881,6 @@ impl Field {
None => quote!(resolve(data)),
Some(_) => quote!(resolve_with_args(data, &args)),
};
let docs = format!(" Attempt to resolve [`{raw_name}`][Self::{raw_name}].");
let getter_impl = if self.is_version_dependent() {
// if this is nullable *and* version dependent we add a `?`
// to avoid returning Option<Option<_>>
Expand All @@ -882,8 +890,7 @@ impl Field {
quote!( self. #raw_name () .#resolve )
};
Some(quote! {
#[doc = #docs]
#extra_docs_if_input_data
#docs
pub fn #getter_name #decl_lifetime_if_needed (&self #input_data_if_needed) -> #return_type #where_read_clause {
#data_alias_if_needed
#args_if_needed
Expand Down
36 changes: 12 additions & 24 deletions read-fonts/generated/generated_base.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,13 +389,10 @@ impl BaseScriptRecord {
self.base_script_offset.get()
}

/// Attempt to resolve [`base_script_offset`][Self::base_script_offset].
///
/// The `data` argument should generally be retrieved via the `offset_data`
/// method of the parent table, but you should consult the docs of the
/// referenced method (reproduced below)
///
/// Offset to BaseScript table, from beginning of BaseScriptList
///
/// The `data` argument should be retrieved from the parent table
/// By calling its `offset_data` method.
pub fn base_script<'a>(&self, data: FontData<'a>) -> Result<BaseScript<'a>, ReadError> {
self.base_script_offset().resolve(data)
}
Expand Down Expand Up @@ -574,13 +571,10 @@ impl BaseLangSysRecord {
self.min_max_offset.get()
}

/// Attempt to resolve [`min_max_offset`][Self::min_max_offset].
///
/// The `data` argument should generally be retrieved via the `offset_data`
/// method of the parent table, but you should consult the docs of the
/// referenced method (reproduced below)
///
/// Offset to MinMax table, from beginning of BaseScript table
///
/// The `data` argument should be retrieved from the parent table
/// By calling its `offset_data` method.
pub fn min_max<'a>(&self, data: FontData<'a>) -> Result<MinMax<'a>, ReadError> {
self.min_max_offset().resolve(data)
}
Expand Down Expand Up @@ -873,14 +867,11 @@ impl FeatMinMaxRecord {
self.min_coord_offset.get()
}

/// Attempt to resolve [`min_coord_offset`][Self::min_coord_offset].
///
/// The `data` argument should generally be retrieved via the `offset_data`
/// method of the parent table, but you should consult the docs of the
/// referenced method (reproduced below)
///
/// Offset to BaseCoord table that defines the minimum extent
/// value, from beginning of MinMax table (may be NULL)
///
/// The `data` argument should be retrieved from the parent table
/// By calling its `offset_data` method.
pub fn min_coord<'a>(&self, data: FontData<'a>) -> Option<Result<MinMax<'a>, ReadError>> {
self.min_coord_offset().resolve(data)
}
Expand All @@ -891,14 +882,11 @@ impl FeatMinMaxRecord {
self.max_coord_offset.get()
}

/// Attempt to resolve [`max_coord_offset`][Self::max_coord_offset].
///
/// The `data` argument should generally be retrieved via the `offset_data`
/// method of the parent table, but you should consult the docs of the
/// referenced method (reproduced below)
///
/// Offset to BaseCoord table that defines the maximum extent
/// value, from beginning of MinMax table (may be NULL)
///
/// The `data` argument should be retrieved from the parent table
/// By calling its `offset_data` method.
pub fn max_coord<'a>(&self, data: FontData<'a>) -> Option<Result<MinMax<'a>, ReadError>> {
self.max_coord_offset().resolve(data)
}
Expand Down
53 changes: 22 additions & 31 deletions read-fonts/generated/generated_cmap.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub struct EncodingRecord {
pub platform_id: BigEndian<PlatformId>,
/// Platform-specific encoding ID.
pub encoding_id: BigEndian<u16>,
/// Byte offset from beginning of table to the subtable for this
/// Byte offset from beginning of the [`Cmap`] table to the subtable for this
/// encoding.
pub subtable_offset: BigEndian<Offset32>,
}
Expand All @@ -121,20 +121,17 @@ impl EncodingRecord {
self.encoding_id.get()
}

/// Byte offset from beginning of table to the subtable for this
/// Byte offset from beginning of the [`Cmap`] table to the subtable for this
/// encoding.
pub fn subtable_offset(&self) -> Offset32 {
self.subtable_offset.get()
}

/// Attempt to resolve [`subtable_offset`][Self::subtable_offset].
///
/// The `data` argument should generally be retrieved via the `offset_data`
/// method of the parent table, but you should consult the docs of the
/// referenced method (reproduced below)
///
/// Byte offset from beginning of table to the subtable for this
/// Byte offset from beginning of the [`Cmap`] table to the subtable for this
/// encoding.
///
/// The `data` argument should be retrieved from the parent table
/// By calling its `offset_data` method.
pub fn subtable<'a>(&self, data: FontData<'a>) -> Result<CmapSubtable<'a>, ReadError> {
self.subtable_offset().resolve(data)
}
Expand Down Expand Up @@ -1621,11 +1618,11 @@ impl<'a> std::fmt::Debug for Cmap14<'a> {
pub struct VariationSelector {
/// Variation selector
pub var_selector: BigEndian<Uint24>,
/// Offset from the start of the format 14 subtable to Default UVS
/// Table. May be 0.
/// Offset from the start of the [`Cmap14`] subtable to Default UVS
/// Table. May be NULL.
pub default_uvs_offset: BigEndian<Nullable<Offset32>>,
/// Offset from the start of the format 14 subtable to Non-Default
/// UVS Table. May be 0.
/// Offset from the start of the [`Cmap14`] subtable to Non-Default
/// UVS Table. May be NULL.
pub non_default_uvs_offset: BigEndian<Nullable<Offset32>>,
}

Expand All @@ -1635,38 +1632,32 @@ impl VariationSelector {
self.var_selector.get()
}

/// Offset from the start of the format 14 subtable to Default UVS
/// Table. May be 0.
/// Offset from the start of the [`Cmap14`] subtable to Default UVS
/// Table. May be NULL.
pub fn default_uvs_offset(&self) -> Nullable<Offset32> {
self.default_uvs_offset.get()
}

/// Attempt to resolve [`default_uvs_offset`][Self::default_uvs_offset].
/// Offset from the start of the [`Cmap14`] subtable to Default UVS
/// Table. May be NULL.
///
/// The `data` argument should generally be retrieved via the `offset_data`
/// method of the parent table, but you should consult the docs of the
/// referenced method (reproduced below)
///
/// Offset from the start of the format 14 subtable to Default UVS
/// Table. May be 0.
/// The `data` argument should be retrieved from the parent table
/// By calling its `offset_data` method.
pub fn default_uvs<'a>(&self, data: FontData<'a>) -> Option<Result<DefaultUvs<'a>, ReadError>> {
self.default_uvs_offset().resolve(data)
}

/// Offset from the start of the format 14 subtable to Non-Default
/// UVS Table. May be 0.
/// Offset from the start of the [`Cmap14`] subtable to Non-Default
/// UVS Table. May be NULL.
pub fn non_default_uvs_offset(&self) -> Nullable<Offset32> {
self.non_default_uvs_offset.get()
}

/// Attempt to resolve [`non_default_uvs_offset`][Self::non_default_uvs_offset].
///
/// The `data` argument should generally be retrieved via the `offset_data`
/// method of the parent table, but you should consult the docs of the
/// referenced method (reproduced below)
/// Offset from the start of the [`Cmap14`] subtable to Non-Default
/// UVS Table. May be NULL.
///
/// Offset from the start of the format 14 subtable to Non-Default
/// UVS Table. May be 0.
/// The `data` argument should be retrieved from the parent table
/// By calling its `offset_data` method.
pub fn non_default_uvs<'a>(
&self,
data: FontData<'a>,
Expand Down
26 changes: 10 additions & 16 deletions read-fonts/generated/generated_colr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ impl<'a> std::fmt::Debug for BaseGlyphList<'a> {
pub struct BaseGlyphPaint {
/// Glyph ID of the base glyph.
pub glyph_id: BigEndian<GlyphId>,
/// Offset to a Paint table.
/// Offset to a Paint table, from the beginning of the [`BaseGlyphList`] table.
pub paint_offset: BigEndian<Offset32>,
}

Expand All @@ -494,18 +494,15 @@ impl BaseGlyphPaint {
self.glyph_id.get()
}

/// Offset to a Paint table.
/// Offset to a Paint table, from the beginning of the [`BaseGlyphList`] table.
pub fn paint_offset(&self) -> Offset32 {
self.paint_offset.get()
}

/// Attempt to resolve [`paint_offset`][Self::paint_offset].
/// Offset to a Paint table, from the beginning of the [`BaseGlyphList`] table.
///
/// The `data` argument should generally be retrieved via the `offset_data`
/// method of the parent table, but you should consult the docs of the
/// referenced method (reproduced below)
///
/// Offset to a Paint table.
/// The `data` argument should be retrieved from the parent table
/// By calling its `offset_data` method.
pub fn paint<'a>(&self, data: FontData<'a>) -> Result<Paint<'a>, ReadError> {
self.paint_offset().resolve(data)
}
Expand Down Expand Up @@ -721,7 +718,7 @@ pub struct Clip {
pub start_glyph_id: BigEndian<GlyphId>,
/// Last glyph ID in the range.
pub end_glyph_id: BigEndian<GlyphId>,
/// Offset to a ClipBox table.
/// Offset to a ClipBox table, from the beginning of the [`ClipList`] table.
pub clip_box_offset: BigEndian<Offset24>,
}

Expand All @@ -736,18 +733,15 @@ impl Clip {
self.end_glyph_id.get()
}

/// Offset to a ClipBox table.
/// Offset to a ClipBox table, from the beginning of the [`ClipList`] table.
pub fn clip_box_offset(&self) -> Offset24 {
self.clip_box_offset.get()
}

/// Attempt to resolve [`clip_box_offset`][Self::clip_box_offset].
///
/// The `data` argument should generally be retrieved via the `offset_data`
/// method of the parent table, but you should consult the docs of the
/// referenced method (reproduced below)
/// Offset to a ClipBox table, from the beginning of the [`ClipList`] table.
///
/// Offset to a ClipBox table.
/// The `data` argument should be retrieved from the parent table
/// By calling its `offset_data` method.
pub fn clip_box<'a>(&self, data: FontData<'a>) -> Result<ClipBox<'a>, ReadError> {
self.clip_box_offset().resolve(data)
}
Expand Down
Loading

0 comments on commit ece8f27

Please sign in to comment.