From 619583df2ff0458568c68c8df38230a62aa2c76b Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Wed, 26 Jul 2023 16:21:43 -0400 Subject: [PATCH] [write-fonts] Add 'PendingVariationIndex' table 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). --- font-codegen/README.md | 2 ++ font-codegen/src/parsing.rs | 7 ++++ font-codegen/src/table.rs | 40 +++++++++++++-------- resources/codegen_inputs/layout.rs | 20 +++++++++++ write-fonts/generated/generated_layout.rs | 42 +++++++++++++++++++++++ write-fonts/src/tables/layout.rs | 10 ++++++ 6 files changed, 107 insertions(+), 14 deletions(-) diff --git a/font-codegen/README.md b/font-codegen/README.md index a8cd6e59c..584a2d95e 100644 --- a/font-codegen/README.md +++ b/font-codegen/README.md @@ -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 diff --git a/font-codegen/src/parsing.rs b/font-codegen/src/parsing.rs index e6a0211fd..97443ede4 100644 --- a/font-codegen/src/parsing.rs +++ b/font-codegen/src/parsing.rs @@ -64,6 +64,7 @@ pub(crate) struct TableAttrs { pub(crate) read_args: Option>, pub(crate) generic_offset: Option>, pub(crate) tag: Option>, + pub(crate) write_only: Option, /// Custom validation behaviour, must be a fn(&self, &mut ValidationCtx) for the type pub(crate) validate: Option>, } @@ -130,6 +131,7 @@ pub(crate) struct GroupVariant { pub(crate) struct VariantAttrs { pub(crate) docs: Vec, pub(crate) match_stmt: Option>, + pub(crate) write_only: Option, } impl FormatVariant { @@ -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 { @@ -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(), @@ -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 { diff --git a/font-codegen/src/table.rs b/font-codegen/src/table.rs index 2a6473a73..1aaaa30d3 100644 --- a/font-codegen/src/table.rs +++ b/font-codegen/src/table.rs @@ -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 { + 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 = ())); @@ -595,9 +598,11 @@ fn generate_format_from_obj( parse_module: &syn::Path, ) -> syn::Result { 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! { @@ -624,15 +629,20 @@ fn generate_format_from_obj( pub(crate) fn generate_format_group(item: &TableFormat) -> syn::Result { 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; @@ -641,16 +651,18 @@ pub(crate) fn generate_format_group(item: &TableFormat) -> syn::Result { 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 diff --git a/resources/codegen_inputs/layout.rs b/resources/codegen_inputs/layout.rs index c081a50cc..f8c140235 100644 --- a/resources/codegen_inputs/layout.rs +++ b/resources/codegen_inputs/layout.rs @@ -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) diff --git a/write-fonts/generated/generated_layout.rs b/write-fonts/generated/generated_layout.rs index 8b6f79518..7219f7319 100644 --- a/write-fonts/generated/generated_layout.rs +++ b/write-fonts/generated/generated_layout.rs @@ -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 { @@ -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 { @@ -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(), } } } @@ -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), } } } @@ -2684,6 +2720,12 @@ impl From for DeviceOrVariationIndex { } } +impl From 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 { diff --git a/write-fonts/src/tables/layout.rs b/write-fonts/src/tables/layout.rs index b275963e9..cc31bb79a 100644 --- a/write-fonts/src/tables/layout.rs +++ b/write-fonts/src/tables/layout.rs @@ -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 " + ) + } +} + fn encode_delta(format: DeltaFormat, values: &[i8]) -> Vec { let (chunk_size, mask, bits) = match format { DeltaFormat::Local2BitDeltas => (8, 0b11, 2),