Skip to content

Commit

Permalink
[write-fonts] Add 'PendingVariationIndex' table
Browse files Browse the repository at this point in the history
This is intended as a sentinal, which can be assigned a unique
ID during compilation, and then remapped to the final VariationIndex
values once the ItemVariationStore has been compiled.

This type will panic if we attempt to serialize it, which will ensure
that we never accidentally write temporary values to our VariationIndex
tables.

Although this is not part of the spec I am generating this in codegen
since it's much easier than trying to do it manually; making this work
required adding a new attribute, `#[write_fonts_only]`, to communicate
to codegen that we should not generate any parsing code for this type
(since it is not a real type that should ever exist in an actual font
file).
  • Loading branch information
cmyr committed Aug 2, 2023
1 parent 7a91225 commit ab7b01b
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 14 deletions.
2 changes: 2 additions & 0 deletions font-codegen/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ The following annotations are supported on top-level objects:
common tables that contain offsets which point to different concrete types
depending on the containing table, such as the `Layout` subtable shared
between GPOS and GSUB.
- `#[write_fonts_only]` Indicate that this table should only be generated for
`write-fonts` (i.e. should be ignored in `read-fonts`).

#### field attributes
- `#[nullable]`: only allowed on offsets or arrays of offsets, and indicates
Expand Down
7 changes: 7 additions & 0 deletions font-codegen/src/parsing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub(crate) struct TableAttrs {
pub(crate) read_args: Option<Attr<TableReadArgs>>,
pub(crate) generic_offset: Option<Attr<syn::Ident>>,
pub(crate) tag: Option<Attr<syn::LitStr>>,
pub(crate) write_only: Option<syn::Path>,
/// Custom validation behaviour, must be a fn(&self, &mut ValidationCtx) for the type
pub(crate) validate: Option<Attr<syn::Ident>>,
}
Expand Down Expand Up @@ -130,6 +131,7 @@ pub(crate) struct GroupVariant {
pub(crate) struct VariantAttrs {
pub(crate) docs: Vec<syn::Attribute>,
pub(crate) match_stmt: Option<Attr<InlineExpr>>,
pub(crate) write_only: Option<syn::Path>,
}

impl FormatVariant {
Expand Down Expand Up @@ -922,6 +924,7 @@ impl Parse for FormatVariant {
}

static MATCH_IF: &str = "match_if";
static WRITE_FONTS_ONLY: &str = "write_fonts_only";

impl Parse for VariantAttrs {
fn parse(input: ParseStream) -> syn::Result<Self> {
Expand All @@ -940,6 +943,8 @@ impl Parse for VariantAttrs {
this.docs.push(attr);
} else if ident == MATCH_IF {
this.match_stmt = Some(Attr::new(ident.clone(), attr.parse_args()?));
} else if ident == WRITE_FONTS_ONLY {
this.write_only = Some(attr.path().clone());
} else {
return Err(logged_syn_error(
ident.span(),
Expand Down Expand Up @@ -1061,6 +1066,8 @@ impl Parse for TableAttrs {
this.skip_font_write = Some(attr.path().clone());
} else if ident == SKIP_CONSTRUCTOR {
this.skip_constructor = Some(attr.path().clone());
} else if ident == WRITE_FONTS_ONLY {
this.write_only = Some(attr.path().clone());
} else if ident == READ_ARGS {
this.read_args = Some(Attr::new(ident.clone(), attr.parse_args()?));
} else if ident == GENERIC_OFFSET {
Expand Down
40 changes: 26 additions & 14 deletions font-codegen/src/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ use crate::parsing::{Attr, GenericGroup, Item, Items, Phase};
use super::parsing::{Field, ReferencedFields, Table, TableFormat, TableReadArg, TableReadArgs};

pub(crate) fn generate(item: &Table) -> syn::Result<TokenStream> {
if item.attrs.write_only.is_some() {
return Ok(Default::default());
}
let docs = &item.attrs.docs;
let generic = item.attrs.generic_offset.as_ref();
let generic_with_default = generic.map(|t| quote!(#t = ()));
Expand Down Expand Up @@ -595,9 +598,11 @@ fn generate_format_from_obj(
parse_module: &syn::Path,
) -> syn::Result<TokenStream> {
let name = &item.name;
let to_owned_arms = item.variants.iter().map(|variant| {
let var_name = &variant.name;
quote!( ObjRefType::#var_name(item) => #name::#var_name(item.to_owned_table()), )
let to_owned_arms = item.variants.iter().filter_map(|variant| {
variant.attrs.write_only.is_none().then(|| {
let var_name = &variant.name;
quote!( ObjRefType::#var_name(item) => #name::#var_name(item.to_owned_table()), )
})
});

Ok(quote! {
Expand All @@ -624,15 +629,20 @@ fn generate_format_from_obj(
pub(crate) fn generate_format_group(item: &TableFormat) -> syn::Result<TokenStream> {
let name = &item.name;
let docs = &item.attrs.docs;
let variants = item.variants.iter().map(|variant| {
let name = &variant.name;
let typ = variant.type_name();
let docs = &variant.attrs.docs;
quote! ( #( #docs )* #name(#typ<'a>) )
let variants = item.variants.iter().filter_map(|variant| {
variant.attrs.write_only.is_none().then(|| {
let name = &variant.name;
let typ = variant.type_name();
let docs = &variant.attrs.docs;
quote! ( #( #docs )* #name(#typ<'a>) )
})
});

let format = &item.format;
let match_arms = item.variants.iter().map(|variant| {
let match_arms = item.variants.iter().filter_map(|variant| {
if variant.attrs.write_only.is_some() {
return None;
}
let name = &variant.name;
let lhs = if let Some(expr) = variant.attrs.match_stmt.as_deref() {
let expr = &expr.expr;
Expand All @@ -641,16 +651,18 @@ pub(crate) fn generate_format_group(item: &TableFormat) -> syn::Result<TokenStre
let typ = variant.marker_name();
quote!(#typ::FORMAT)
};
quote! {
Some(quote! {
#lhs => {
Ok(Self::#name(FontRead::read(data)?))
}
}
})
});

let traversal_arms = item.variants.iter().map(|variant| {
let name = &variant.name;
quote!(Self::#name(table) => table)
let traversal_arms = item.variants.iter().filter_map(|variant| {
variant.attrs.write_only.is_none().then(|| {
let name = &variant.name;
quote!(Self::#name(table) => table)
})
});

let format_offset = item
Expand Down
20 changes: 20 additions & 0 deletions resources/codegen_inputs/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,12 +551,32 @@ table VariationIndex {
delta_format: DeltaFormat,
}

/// A type representing a temporary identifier for a set of variation deltas.
///
/// The final indices used in the VariationIndex table are not known until
/// all deltas have been collected. This variant is used to assign a
/// temporary identifier during compilation.
///
/// This type is not part of the spec and will never appear in an actual font file.
/// It is intended to serve as a sentinel value, and will panic when written,
/// ensuring that all VariationIndex tables have been correctly mapped before
/// the font is compiled.
#[write_fonts_only]
#[skip_from_obj]
#[skip_font_write]
table PendingVariationIndex {
/// A unique identifier for a given set of deltas.
delta_set_id: u32,
}

/// Either a [Device] table (in a non-variable font) or a [VariationIndex] table (in a variable font)
format DeltaFormat@4 DeviceOrVariationIndex {
#[match_if($format != DeltaFormat::VariationIndex)]
Device(Device),
#[match_if($format == DeltaFormat::VariationIndex)]
VariationIndex(VariationIndex),
#[write_fonts_only]
PendingVariationIndex(PendingVariationIndex),
}

/// [FeatureVariations Table](https://docs.microsoft.com/en-us/typography/opentype/spec/chapter2#featurevariations-table)
Expand Down
42 changes: 42 additions & 0 deletions write-fonts/generated/generated_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2601,11 +2601,39 @@ impl<'a> FontRead<'a> for VariationIndex {
}
}

/// A type representing a temporary identifier for a set of variation deltas.
///
/// The final indices used in the VariationIndex table are not known until
/// all deltas have been collected. This variant is used to assign a
/// temporary identifier during compilation.
///
/// This type is not part of the spec and will never appear in an actual font file.
/// It is intended to serve as a sentinel value, and will panic when written,
/// ensuring that all VariationIndex tables have been correctly mapped before
/// the font is compiled.
#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct PendingVariationIndex {
/// A unique identifier for a given set of deltas.
pub delta_set_id: u32,
}

impl PendingVariationIndex {
/// Construct a new `PendingVariationIndex`
pub fn new(delta_set_id: u32) -> Self {
Self { delta_set_id }
}
}

impl Validate for PendingVariationIndex {
fn validate_impl(&self, _ctx: &mut ValidationCtx) {}
}

/// Either a [Device] table (in a non-variable font) or a [VariationIndex] table (in a variable font)
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum DeviceOrVariationIndex {
Device(Device),
VariationIndex(VariationIndex),
PendingVariationIndex(PendingVariationIndex),
}

impl DeviceOrVariationIndex {
Expand All @@ -2616,6 +2644,11 @@ impl DeviceOrVariationIndex {
delta_set_inner_index,
))
}

/// Construct a new `PendingVariationIndex` subtable
pub fn pending_variation_index(delta_set_id: u32) -> Self {
Self::PendingVariationIndex(PendingVariationIndex::new(delta_set_id))
}
}

impl Default for DeviceOrVariationIndex {
Expand All @@ -2629,12 +2662,14 @@ impl FontWrite for DeviceOrVariationIndex {
match self {
Self::Device(item) => item.write_into(writer),
Self::VariationIndex(item) => item.write_into(writer),
Self::PendingVariationIndex(item) => item.write_into(writer),
}
}
fn table_type(&self) -> TableType {
match self {
Self::Device(item) => item.table_type(),
Self::VariationIndex(item) => item.table_type(),
Self::PendingVariationIndex(item) => item.table_type(),
}
}
}
Expand All @@ -2644,6 +2679,7 @@ impl Validate for DeviceOrVariationIndex {
match self {
Self::Device(item) => item.validate_impl(ctx),
Self::VariationIndex(item) => item.validate_impl(ctx),
Self::PendingVariationIndex(item) => item.validate_impl(ctx),
}
}
}
Expand Down Expand Up @@ -2684,6 +2720,12 @@ impl From<VariationIndex> for DeviceOrVariationIndex {
}
}

impl From<PendingVariationIndex> for DeviceOrVariationIndex {
fn from(src: PendingVariationIndex) -> DeviceOrVariationIndex {
DeviceOrVariationIndex::PendingVariationIndex(src)
}
}

/// [FeatureVariations Table](https://docs.microsoft.com/en-us/typography/opentype/spec/chapter2#featurevariations-table)
#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct FeatureVariations {
Expand Down
10 changes: 10 additions & 0 deletions write-fonts/src/tables/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,16 @@ impl DeviceOrVariationIndex {
}
}

impl FontWrite for PendingVariationIndex {
fn write_into(&self, _writer: &mut TableWriter) {
panic!(
"Attempted to write PendingVariationIndex.\n\
VariationIndex tables should always be resolved before compilation.\n\
Please report this bug at <https://github.com/googlefonts/fontations/issues>"
)
}
}

fn encode_delta(format: DeltaFormat, values: &[i8]) -> Vec<u16> {
let (chunk_size, mask, bits) = match format {
DeltaFormat::Local2BitDeltas => (8, 0b11, 2),
Expand Down

0 comments on commit ab7b01b

Please sign in to comment.