From de95014c562cf8289132b1d9af65e00ea3ee8f62 Mon Sep 17 00:00:00 2001 From: Kostya Farber Date: Thu, 2 Jul 2026 07:37:47 +0100 Subject: [PATCH 1/5] fix(backends): preserve plist dates, uids, and unsigned integers in libs UFO lib values of type date and uid were silently replaced with empty strings, and unsigned integers above i64::MAX were zeroed. LibValue now carries Date (RFC 3339 string, plist XML format), Uid (u64 payload), and UnsignedInteger variants, with tagged self-describing serde, and the UFO reader/writer, .shift package schema, and SQLite store round-trip them. --- crates/shift-backends/src/ufo/reader.rs | 10 +++++++++- crates/shift-backends/src/ufo/writer.rs | 5 +++++ crates/shift-font/src/ir/lib_data.rs | 10 +++++++++- crates/shift-font/src/test_support.rs | 4 ++++ crates/shift-source/src/package.rs | 9 +++++++++ crates/shift-store/src/change_set.rs | 5 +++++ crates/shift-store/src/font_state.rs | 12 ++++++++++++ 7 files changed, 53 insertions(+), 2 deletions(-) diff --git a/crates/shift-backends/src/ufo/reader.rs b/crates/shift-backends/src/ufo/reader.rs index c1707983..f056a3bc 100644 --- a/crates/shift-backends/src/ufo/reader.rs +++ b/crates/shift-backends/src/ufo/reader.rs @@ -82,7 +82,13 @@ impl UfoReader { fn convert_plist_to_lib_value(plist: &plist::Value) -> LibValue { match plist { plist::Value::String(s) => LibValue::String(s.clone()), - plist::Value::Integer(i) => LibValue::Integer(i.as_signed().unwrap_or(0)), + plist::Value::Integer(i) => match i.as_signed() { + Some(value) => LibValue::Integer(value), + None => LibValue::UnsignedInteger( + i.as_unsigned() + .expect("plist integer is signed or unsigned"), + ), + }, plist::Value::Real(f) => LibValue::Float(*f), plist::Value::Boolean(b) => LibValue::Boolean(*b), plist::Value::Array(arr) => { @@ -96,6 +102,8 @@ impl UfoReader { LibValue::Dict(map) } plist::Value::Data(d) => LibValue::Data(d.clone()), + plist::Value::Date(d) => LibValue::Date(d.to_xml_format()), + plist::Value::Uid(u) => LibValue::Uid(u.get()), _ => LibValue::String(String::new()), } } diff --git a/crates/shift-backends/src/ufo/writer.rs b/crates/shift-backends/src/ufo/writer.rs index f3178a8c..6ab0bd6f 100644 --- a/crates/shift-backends/src/ufo/writer.rs +++ b/crates/shift-backends/src/ufo/writer.rs @@ -221,6 +221,7 @@ impl UfoWriter { match value { LibValue::String(s) => plist::Value::String(s.clone()), LibValue::Integer(i) => plist::Value::Integer((*i).into()), + LibValue::UnsignedInteger(u) => plist::Value::Integer((*u).into()), LibValue::Float(f) => plist::Value::Real(*f), LibValue::Boolean(b) => plist::Value::Boolean(*b), LibValue::Array(arr) => { @@ -234,6 +235,10 @@ impl UfoWriter { plist::Value::Dictionary(plist_dict) } LibValue::Data(d) => plist::Value::Data(d.clone()), + LibValue::Date(d) => plist::Date::from_xml_format(d) + .map(plist::Value::Date) + .unwrap_or_else(|_| plist::Value::String(d.clone())), + LibValue::Uid(u) => plist::Value::Uid(plist::Uid::new(*u)), } } diff --git a/crates/shift-font/src/ir/lib_data.rs b/crates/shift-font/src/ir/lib_data.rs index 91484d8a..a47f5541 100644 --- a/crates/shift-font/src/ir/lib_data.rs +++ b/crates/shift-font/src/ir/lib_data.rs @@ -7,15 +7,23 @@ pub struct LibData { } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -#[serde(untagged)] +#[serde(tag = "type", content = "value", rename_all = "camelCase")] pub enum LibValue { String(String), Integer(i64), + /// Unsigned integers above `i64::MAX`; anything smaller uses `Integer`. + UnsignedInteger(u64), Float(f64), Boolean(bool), Array(Vec), Dict(HashMap), Data(Vec), + /// A plist date, carried as the RFC 3339 / ISO 8601 string used by XML + /// plists (`plist::Date::to_xml_format`), e.g. `2024-02-02T02:02:02Z`. + Date(String), + /// A plist archive UID payload. XML plists cannot represent UIDs, so this + /// only occurs in binary plists; it is carried verbatim. + Uid(u64), } impl LibData { diff --git a/crates/shift-font/src/test_support.rs b/crates/shift-font/src/test_support.rs index 1647a38b..b2e8f07b 100644 --- a/crates/shift-font/src/test_support.rs +++ b/crates/shift-font/src/test_support.rs @@ -76,10 +76,14 @@ pub fn sample_font() -> Font { LibValue::Array(vec![ LibValue::String("array string".to_string()), LibValue::Integer(12), + LibValue::Integer(-12), + LibValue::UnsignedInteger(u64::MAX), LibValue::Float(1.5), LibValue::Boolean(false), LibValue::Dict(nested), LibValue::Data(vec![0, 1, 2, 255]), + LibValue::Date("2024-02-02T02:02:02Z".to_string()), + LibValue::Uid(9), ]), ); diff --git a/crates/shift-source/src/package.rs b/crates/shift-source/src/package.rs index 9d36a4ca..f25c49cc 100644 --- a/crates/shift-source/src/package.rs +++ b/crates/shift-source/src/package.rs @@ -409,11 +409,14 @@ struct LayerLibDoc { enum LibValueDoc { String(String), Integer(i64), + UnsignedInteger(u64), Float(f64), Boolean(bool), Array(Vec), Dict(BTreeMap), Data(Vec), + Date(String), + Uid(u64), } pub fn font_to_tree(font: &Font) -> Result { @@ -1121,6 +1124,7 @@ impl From<&LibValue> for LibValueDoc { match value { LibValue::String(value) => Self::String(value.clone()), LibValue::Integer(value) => Self::Integer(*value), + LibValue::UnsignedInteger(value) => Self::UnsignedInteger(*value), LibValue::Float(value) => Self::Float(*value), LibValue::Boolean(value) => Self::Boolean(*value), LibValue::Array(values) => Self::Array(values.iter().map(Self::from).collect()), @@ -1131,6 +1135,8 @@ impl From<&LibValue> for LibValueDoc { .collect(), ), LibValue::Data(value) => Self::Data(value.clone()), + LibValue::Date(value) => Self::Date(value.clone()), + LibValue::Uid(value) => Self::Uid(*value), } } } @@ -1142,6 +1148,7 @@ impl TryFrom for LibValue { Ok(match doc { LibValueDoc::String(value) => Self::String(value), LibValueDoc::Integer(value) => Self::Integer(value), + LibValueDoc::UnsignedInteger(value) => Self::UnsignedInteger(value), LibValueDoc::Float(value) => Self::Float(value), LibValueDoc::Boolean(value) => Self::Boolean(value), LibValueDoc::Array(values) => Self::Array( @@ -1157,6 +1164,8 @@ impl TryFrom for LibValue { .collect::, _>>()?, ), LibValueDoc::Data(value) => Self::Data(value), + LibValueDoc::Date(value) => Self::Date(value), + LibValueDoc::Uid(value) => Self::Uid(value), }) } } diff --git a/crates/shift-store/src/change_set.rs b/crates/shift-store/src/change_set.rs index b0cfd64d..862ece77 100644 --- a/crates/shift-store/src/change_set.rs +++ b/crates/shift-store/src/change_set.rs @@ -816,6 +816,9 @@ fn lib_value_to_json(value: &font::LibValue) -> serde_json::Value { typed_json("string", serde_json::Value::String(value.clone())) } font::LibValue::Integer(value) => typed_json("integer", serde_json::json!(value)), + font::LibValue::UnsignedInteger(value) => { + typed_json("unsignedInteger", serde_json::json!(value)) + } font::LibValue::Float(value) => typed_json("float", serde_json::json!(value)), font::LibValue::Boolean(value) => typed_json("boolean", serde_json::json!(value)), font::LibValue::Array(values) => typed_json( @@ -832,6 +835,8 @@ fn lib_value_to_json(value: &font::LibValue) -> serde_json::Value { ), ), font::LibValue::Data(values) => typed_json("data", serde_json::json!(values)), + font::LibValue::Date(value) => typed_json("date", serde_json::Value::String(value.clone())), + font::LibValue::Uid(value) => typed_json("uid", serde_json::json!(value)), } } diff --git a/crates/shift-store/src/font_state.rs b/crates/shift-store/src/font_state.rs index f0a0d8fa..2b4b28db 100644 --- a/crates/shift-store/src/font_state.rs +++ b/crates/shift-store/src/font_state.rs @@ -618,6 +618,10 @@ fn lib_value_from_value(value: serde_json::Value) -> Result value + .as_u64() + .map(font::LibValue::UnsignedInteger) + .ok_or_else(|| StoreError::InvalidLibValue("expected unsigned integer".to_string())), "float" => value .as_f64() .map(font::LibValue::Float) @@ -657,6 +661,14 @@ fn lib_value_from_value(value: serde_json::Value) -> Result Err(StoreError::InvalidLibValue("expected data".to_string())), }, + "date" => match value { + serde_json::Value::String(value) => Ok(font::LibValue::Date(value)), + _ => Err(StoreError::InvalidLibValue("expected date".to_string())), + }, + "uid" => value + .as_u64() + .map(font::LibValue::Uid) + .ok_or_else(|| StoreError::InvalidLibValue("expected uid".to_string())), _ => Err(StoreError::InvalidLibValue(format!("unknown type {kind}"))), } } From 5c0f2be3bac4467df43967d5673ef4ba25596e05 Mon Sep 17 00:00:00 2001 From: Kostya Farber Date: Thu, 2 Jul 2026 07:44:46 +0100 Subject: [PATCH 2/5] feat: carry UFO data/ and images/ directories through IR and .shift --- crates/shift-backends/src/traits.rs | 14 ++++- crates/shift-backends/src/ufo/reader.rs | 16 +++++ crates/shift-backends/src/ufo/writer.rs | 20 ++++++- crates/shift-bridge/src/bridge.rs | 8 +++ crates/shift-font/src/ir/binary_data.rs | 73 +++++++++++++++++++++++ crates/shift-font/src/ir/font.rs | 29 +++++++++ crates/shift-font/src/ir/mod.rs | 2 + crates/shift-font/src/test_support.rs | 11 ++++ crates/shift-source/src/lib.rs | 7 ++- crates/shift-source/src/package.rs | 35 +++++++++++ crates/shift-source/tests/package_test.rs | 11 ++-- crates/shift-store/src/change_set.rs | 20 +++++++ crates/shift-store/src/font_state.rs | 24 ++++++++ crates/shift-store/src/schema.rs | 7 +++ 14 files changed, 267 insertions(+), 10 deletions(-) create mode 100644 crates/shift-font/src/ir/binary_data.rs diff --git a/crates/shift-backends/src/traits.rs b/crates/shift-backends/src/traits.rs index 63be0f13..43be1d50 100644 --- a/crates/shift-backends/src/traits.rs +++ b/crates/shift-backends/src/traits.rs @@ -1,7 +1,7 @@ use crate::errors::FormatBackendResult; use shift_font::{ - Axis, FeatureData, Font, FontMetadata, FontMetrics, Glyph, GlyphName, Guideline, KerningData, - LibData, Source, SourceId, + Axis, BinaryData, FeatureData, Font, FontMetadata, FontMetrics, Glyph, GlyphName, Guideline, + KerningData, LibData, Source, SourceId, }; pub trait FontView { @@ -16,6 +16,8 @@ pub trait FontView { fn features(&self) -> &FeatureData; fn guidelines(&self) -> &[Guideline]; fn lib(&self) -> &LibData; + fn data_files(&self) -> &BinaryData; + fn images(&self) -> &BinaryData; } impl FontView for Font { @@ -62,6 +64,14 @@ impl FontView for Font { fn lib(&self) -> &LibData { self.lib() } + + fn data_files(&self) -> &BinaryData { + self.data_files() + } + + fn images(&self) -> &BinaryData { + self.images() + } } pub trait FontReader: Send + Sync { diff --git a/crates/shift-backends/src/ufo/reader.rs b/crates/shift-backends/src/ufo/reader.rs index f056a3bc..80985859 100644 --- a/crates/shift-backends/src/ufo/reader.rs +++ b/crates/shift-backends/src/ufo/reader.rs @@ -329,6 +329,22 @@ impl FontReader for UfoReader { *font.lib_mut() = Self::convert_lib(&norad_font.lib); } + for (data_path, contents) in norad_font.data.iter() { + let bytes = contents.map_err(|e| { + FormatBackendError::Ufo(format!("failed to read data file {data_path:?}: {e}")) + })?; + font.data_files_mut() + .insert(data_path.to_string_lossy().into_owned(), bytes.to_vec()); + } + + for (image_path, contents) in norad_font.images.iter() { + let bytes = contents.map_err(|e| { + FormatBackendError::Ufo(format!("failed to read image file {image_path:?}: {e}")) + })?; + font.images_mut() + .insert(image_path.to_string_lossy().into_owned(), bytes.to_vec()); + } + Ok(font) } } diff --git a/crates/shift-backends/src/ufo/writer.rs b/crates/shift-backends/src/ufo/writer.rs index 6ab0bd6f..dbf8e395 100644 --- a/crates/shift-backends/src/ufo/writer.rs +++ b/crates/shift-backends/src/ufo/writer.rs @@ -4,7 +4,7 @@ use norad::{Font as NoradFont, Glyph as NoradGlyph, Line, Name}; use shift_font::{ Contour, Font, Glyph, GlyphLayer, Guideline, KerningSide, LibData, LibValue, Point, PointType, }; -use std::path::Path; +use std::path::{Path, PathBuf}; pub struct UfoWriter; @@ -410,6 +410,24 @@ impl UfoWriter { norad_font.features = fea_source.to_string(); } + for (data_path, bytes) in font.data_files().iter() { + norad_font + .data + .insert(PathBuf::from(data_path), bytes.clone()) + .map_err(|e| { + FormatBackendError::Ufo(format!("invalid data file {data_path:?}: {e}")) + })?; + } + + for (image_path, bytes) in font.images().iter() { + norad_font + .images + .insert(PathBuf::from(image_path), bytes.clone()) + .map_err(|e| { + FormatBackendError::Ufo(format!("invalid image file {image_path:?}: {e}")) + })?; + } + Ok(norad_font) } diff --git a/crates/shift-bridge/src/bridge.rs b/crates/shift-bridge/src/bridge.rs index 046ef11b..fe584c59 100644 --- a/crates/shift-bridge/src/bridge.rs +++ b/crates/shift-bridge/src/bridge.rs @@ -197,6 +197,14 @@ impl FontView for FontSaveSnapshot { fn lib(&self) -> &shift_font::LibData { self.font.lib() } + + fn data_files(&self) -> &shift_font::BinaryData { + self.font.data_files() + } + + fn images(&self) -> &shift_font::BinaryData { + self.font.images() + } } pub struct ExportFontTask { diff --git a/crates/shift-font/src/ir/binary_data.rs b/crates/shift-font/src/ir/binary_data.rs new file mode 100644 index 00000000..8de11a96 --- /dev/null +++ b/crates/shift-font/src/ir/binary_data.rs @@ -0,0 +1,73 @@ +use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; + +/// Opaque binary files carried with the font, keyed by relative path. +/// +/// Mirrors the UFO `data/` and `images/` directories: the font model never +/// interprets these bytes, it only preserves them across load and save. +#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] +pub struct BinaryData { + files: BTreeMap>, +} + +impl BinaryData { + pub fn new() -> Self { + Self { + files: BTreeMap::new(), + } + } + + pub fn is_empty(&self) -> bool { + self.files.is_empty() + } + + pub fn len(&self) -> usize { + self.files.len() + } + + pub fn get(&self, path: &str) -> Option<&[u8]> { + self.files.get(path).map(Vec::as_slice) + } + + pub fn insert(&mut self, path: String, bytes: Vec) { + self.files.insert(path, bytes); + } + + pub fn remove(&mut self, path: &str) -> Option> { + self.files.remove(path) + } + + pub fn paths(&self) -> impl Iterator { + self.files.keys() + } + + pub fn iter(&self) -> impl Iterator)> { + self.files.iter() + } + + pub fn from_map(files: BTreeMap>) -> Self { + Self { files } + } + + pub fn into_inner(self) -> BTreeMap> { + self.files + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn binary_data_operations() { + let mut data = BinaryData::new(); + assert!(data.is_empty()); + + data.insert("nested/blob.bin".to_string(), vec![0, 1, 2]); + assert_eq!(data.len(), 1); + assert_eq!(data.get("nested/blob.bin"), Some([0, 1, 2].as_slice())); + + data.remove("nested/blob.bin"); + assert!(data.is_empty()); + } +} diff --git a/crates/shift-font/src/ir/font.rs b/crates/shift-font/src/ir/font.rs index 7bc8acbc..521ed05d 100644 --- a/crates/shift-font/src/ir/font.rs +++ b/crates/shift-font/src/ir/font.rs @@ -1,4 +1,5 @@ use crate::axis::{Axis, Location}; +use crate::binary_data::BinaryData; use crate::entity::{AxisId, GlyphId, LayerId, SourceId}; use crate::error::{CoreError, CoreResult}; use crate::features::FeatureData; @@ -101,6 +102,10 @@ struct FontData { features: FeatureData, guidelines: Vec, lib: LibData, + #[serde(default)] + data_files: BinaryData, + #[serde(default)] + images: BinaryData, } #[derive(Clone, Debug, Default)] @@ -293,6 +298,8 @@ impl Default for Font { features: FeatureData::new(), guidelines: Vec::new(), lib: LibData::new(), + data_files: BinaryData::new(), + images: BinaryData::new(), }, index: FontIndex::default(), }), @@ -319,6 +326,8 @@ impl Font { features: FeatureData::new(), guidelines: Vec::new(), lib: LibData::new(), + data_files: BinaryData::new(), + images: BinaryData::new(), }, index: FontIndex::default(), }), @@ -649,6 +658,26 @@ impl Font { pub fn lib_mut(&mut self) -> &mut LibData { &mut self.data_mut().lib } + + /// Opaque files from the source format's `data/` directory, preserved + /// verbatim across load and save. + pub fn data_files(&self) -> &BinaryData { + &self.data().data_files + } + + pub fn data_files_mut(&mut self) -> &mut BinaryData { + &mut self.data_mut().data_files + } + + /// Opaque files from the source format's `images/` directory, preserved + /// verbatim across load and save. + pub fn images(&self) -> &BinaryData { + &self.data().images + } + + pub fn images_mut(&mut self) -> &mut BinaryData { + &mut self.data_mut().images + } } #[cfg(test)] diff --git a/crates/shift-font/src/ir/mod.rs b/crates/shift-font/src/ir/mod.rs index 70469f15..cc66be80 100644 --- a/crates/shift-font/src/ir/mod.rs +++ b/crates/shift-font/src/ir/mod.rs @@ -1,5 +1,6 @@ pub mod anchor; pub mod axis; +pub mod binary_data; pub mod boolean; pub mod component; pub mod contour; @@ -19,6 +20,7 @@ pub mod variation; pub use anchor::Anchor; pub use axis::{Axis, Location}; +pub use binary_data::BinaryData; pub use boolean::{boolean, BooleanOp}; pub use component::{Component, DecomposedTransform, Transform}; pub use contour::{Contour, Contours}; diff --git a/crates/shift-font/src/test_support.rs b/crates/shift-font/src/test_support.rs index b2e8f07b..975a884c 100644 --- a/crates/shift-font/src/test_support.rs +++ b/crates/shift-font/src/test_support.rs @@ -87,6 +87,17 @@ pub fn sample_font() -> Font { ]), ); + font.data_files_mut().insert( + "com.shift.testdata/nested/blob.bin".to_string(), + vec![0x00, 0xFF, 0x10, 0x20], + ); + font.data_files_mut() + .insert("notes.txt".to_string(), b"dogfood data".to_vec()); + font.images_mut().insert( + "swatch.png".to_string(), + vec![0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0x01], + ); + let weight_id = AxisId::from_raw("weight"); let mut weight = Axis::with_id( weight_id.clone(), diff --git a/crates/shift-source/src/lib.rs b/crates/shift-source/src/lib.rs index 0a2b12e1..4ed56964 100644 --- a/crates/shift-source/src/lib.rs +++ b/crates/shift-source/src/lib.rs @@ -1,7 +1,8 @@ mod package; pub use package::{ - AXES_FILE, FEATURES_FILE, FONT_FILE, FORMAT_ID, GLYPHS_DIR, KERNING_FILE, LIB_MODULE_FILE, - MANIFEST_FILE, MODULES_DIR, PackageTree, SCHEMA_VERSION, SOURCES_FILE, ShiftSourcePackage, - SourcePackageError, font_to_tree, read_tree, tree_to_font, write_tree_atomic, + AXES_FILE, DATA_DIR, FEATURES_FILE, FONT_FILE, FORMAT_ID, GLYPHS_DIR, IMAGES_DIR, KERNING_FILE, + LIB_MODULE_FILE, MANIFEST_FILE, MODULES_DIR, PackageTree, SCHEMA_VERSION, SOURCES_FILE, + ShiftSourcePackage, SourcePackageError, font_to_tree, read_tree, tree_to_font, + write_tree_atomic, }; diff --git a/crates/shift-source/src/package.rs b/crates/shift-source/src/package.rs index f25c49cc..99f99780 100644 --- a/crates/shift-source/src/package.rs +++ b/crates/shift-source/src/package.rs @@ -22,6 +22,8 @@ pub const SOURCES_FILE: &str = "sources.json"; pub const FEATURES_FILE: &str = "features.fea"; pub const KERNING_FILE: &str = "kerning.json"; pub const GLYPHS_DIR: &str = "glyphs"; +pub const DATA_DIR: &str = "data"; +pub const IMAGES_DIR: &str = "images"; pub const MODULES_DIR: &str = "modules"; pub const LIB_MODULE_FILE: &str = "modules/shift.libData.json"; @@ -482,6 +484,14 @@ pub fn font_to_tree(font: &Font) -> Result { glyph_entries.sort_by(|(left, _), (right, _)| left.cmp(right)); tree.extend(glyph_entries); + for (path, bytes) in font.data_files().iter() { + tree.push((format!("{DATA_DIR}/{path}"), bytes.clone())); + } + + for (path, bytes) in font.images().iter() { + tree.push((format!("{IMAGES_DIR}/{path}"), bytes.clone())); + } + Ok(tree) } @@ -543,6 +553,10 @@ pub fn tree_to_font(tree: PackageTree) -> Result { for (path, bytes) in entries { if path.starts_with(&format!("{GLYPHS_DIR}/glyph_")) { glyph_entries.push((path, bytes)); + } else if let Some(rel_path) = binary_entry_rel_path(&path, DATA_DIR)? { + font.data_files_mut().insert(rel_path, bytes); + } else if let Some(rel_path) = binary_entry_rel_path(&path, IMAGES_DIR)? { + font.images_mut().insert(rel_path, bytes); } else { return Err(SourcePackageError::UnexpectedEntry(path)); } @@ -691,6 +705,27 @@ fn validate_manifest(manifest: &ManifestDoc) -> Result<(), SourcePackageError> { Ok(()) } +/// Extracts the path relative to `dir` for binary package entries, rejecting +/// entries that could escape the directory when written back to disk. +fn binary_entry_rel_path(path: &str, dir: &str) -> Result, SourcePackageError> { + let Some(rel_path) = path + .strip_prefix(dir) + .and_then(|rest| rest.strip_prefix('/')) + else { + return Ok(None); + }; + + let is_safe = !rel_path.is_empty() + && Path::new(rel_path) + .components() + .all(|component| matches!(component, std::path::Component::Normal(_))); + if is_safe { + Ok(Some(rel_path.to_string())) + } else { + Err(SourcePackageError::UnexpectedEntry(path.to_string())) + } +} + fn validate_glyph_path_id(path: &str, id: &str) -> Result<(), SourcePackageError> { let expected_path = format!("{GLYPHS_DIR}/{id}.json"); if path == expected_path { diff --git a/crates/shift-source/tests/package_test.rs b/crates/shift-source/tests/package_test.rs index 6a076bc5..b7e22d60 100644 --- a/crates/shift-source/tests/package_test.rs +++ b/crates/shift-source/tests/package_test.rs @@ -4,9 +4,9 @@ use shift_font::{ Axis, AxisId, Font, KerningPair, Location, Source, SourceId, test_support::sample_font, }; use shift_source::{ - AXES_FILE, FEATURES_FILE, FONT_FILE, GLYPHS_DIR, KERNING_FILE, LIB_MODULE_FILE, MANIFEST_FILE, - PackageTree, SOURCES_FILE, ShiftSourcePackage, SourcePackageError, font_to_tree, tree_to_font, - write_tree_atomic, + AXES_FILE, DATA_DIR, FEATURES_FILE, FONT_FILE, GLYPHS_DIR, IMAGES_DIR, KERNING_FILE, + LIB_MODULE_FILE, MANIFEST_FILE, PackageTree, SOURCES_FILE, ShiftSourcePackage, + SourcePackageError, font_to_tree, tree_to_font, write_tree_atomic, }; use zip::{CompressionMethod, ZipArchive}; @@ -81,7 +81,10 @@ fn serializes_same_font_to_byte_identical_tree() { KERNING_FILE, LIB_MODULE_FILE, &format!("{GLYPHS_DIR}/glyph_A.json"), - &format!("{GLYPHS_DIR}/glyph_acute.json") + &format!("{GLYPHS_DIR}/glyph_acute.json"), + &format!("{DATA_DIR}/com.shift.testdata/nested/blob.bin"), + &format!("{DATA_DIR}/notes.txt"), + &format!("{IMAGES_DIR}/swatch.png") ] ); } diff --git a/crates/shift-store/src/change_set.rs b/crates/shift-store/src/change_set.rs index 862ece77..78b0faf9 100644 --- a/crates/shift-store/src/change_set.rs +++ b/crates/shift-store/src/change_set.rs @@ -22,6 +22,7 @@ impl ShiftStore { tx.execute("DELETE FROM glyph_layer_lib", [])?; tx.execute("DELETE FROM glyph_lib", [])?; tx.execute("DELETE FROM font_lib", [])?; + tx.execute("DELETE FROM font_binaries", [])?; tx.execute("DELETE FROM kerning_pairs", [])?; tx.execute("DELETE FROM kerning_group_members", [])?; tx.execute("DELETE FROM kerning_groups", [])?; @@ -43,6 +44,8 @@ impl ShiftStore { replace_feature_text(&tx, font.features().fea_source())?; replace_font_guidelines(&tx, font.guidelines())?; replace_lib_data(&tx, "font_lib", "key", None, font.lib())?; + replace_font_binaries(&tx, "data", font.data_files())?; + replace_font_binaries(&tx, "image", font.images())?; replace_kerning(&tx, font.kerning())?; for (order_index, axis) in font.axes().iter().enumerate() { @@ -806,6 +809,23 @@ fn replace_lib_data( Ok(()) } +fn replace_font_binaries( + tx: &Transaction<'_>, + kind: &str, + binaries: &font::BinaryData, +) -> Result<(), StoreError> { + for (path, bytes) in binaries.iter() { + tx.execute( + " + INSERT INTO font_binaries (kind, path, bytes) + VALUES (?1, ?2, ?3) + ", + params![kind, path, bytes], + )?; + } + Ok(()) +} + fn lib_value_json(value: &font::LibValue) -> Result { Ok(serde_json::to_string(&lib_value_to_json(value))?) } diff --git a/crates/shift-store/src/font_state.rs b/crates/shift-store/src/font_state.rs index 2b4b28db..89c5ada1 100644 --- a/crates/shift-store/src/font_state.rs +++ b/crates/shift-store/src/font_state.rs @@ -16,6 +16,8 @@ impl ShiftStore { *font.features_mut() = load_feature_data(&self.conn)?; *font.lib_mut() = load_lib_data(&self.conn, "font_lib", None)?; + *font.data_files_mut() = load_font_binaries(&self.conn, "data")?; + *font.images_mut() = load_font_binaries(&self.conn, "image")?; for guideline in load_font_guidelines(&self.conn)? { font.add_guideline(guideline); @@ -593,6 +595,28 @@ fn load_lib_data( Ok(font::LibData::from_map(values)) } +fn load_font_binaries( + conn: &rusqlite::Connection, + kind: &str, +) -> Result { + let mut binaries = font::BinaryData::new(); + let mut stmt = conn.prepare( + " + SELECT path, bytes + FROM font_binaries + WHERE kind = ?1 + ", + )?; + let rows = stmt.query_map([kind], |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, Vec>(1)?)) + })?; + for row in rows { + let (path, bytes) = row?; + binaries.insert(path, bytes); + } + Ok(binaries) +} + fn lib_value_from_json(value_json: &str) -> Result { let value: serde_json::Value = serde_json::from_str(value_json)?; lib_value_from_value(value) diff --git a/crates/shift-store/src/schema.rs b/crates/shift-store/src/schema.rs index 9ea90eec..404a9c38 100644 --- a/crates/shift-store/src/schema.rs +++ b/crates/shift-store/src/schema.rs @@ -250,6 +250,13 @@ CREATE TABLE IF NOT EXISTS glyph_layer_lib ( FOREIGN KEY (layer_id) REFERENCES glyph_layers(id) ON DELETE CASCADE ); +CREATE TABLE IF NOT EXISTS font_binaries ( + kind TEXT NOT NULL CHECK (kind IN ('data', 'image')), + path TEXT NOT NULL, + bytes BLOB NOT NULL, + PRIMARY KEY (kind, path) +); + CREATE TABLE IF NOT EXISTS workspace_state ( id INTEGER PRIMARY KEY CHECK (id = 1), document_id TEXT, From 76570876624a632d70cf4663f4afa64e676c8f73 Mon Sep 17 00:00:00 2001 From: Kostya Farber Date: Thu, 2 Jul 2026 07:49:21 +0100 Subject: [PATCH 3/5] feat: carry UFO layerinfo color and lib on sources --- crates/shift-backends/src/ufo/reader.rs | 7 +++ crates/shift-backends/src/ufo/writer.rs | 31 ++++++++++ crates/shift-font/src/ir/font.rs | 7 +++ crates/shift-font/src/ir/source.rs | 31 ++++++++++ crates/shift-font/src/test_support.rs | 10 ++- crates/shift-source/src/package.rs | 81 ++++++++++++++++++++++++- crates/shift-store/src/change_set.rs | 19 ++++-- crates/shift-store/src/font_state.rs | 23 +++++-- crates/shift-store/src/schema.rs | 9 +++ 9 files changed, 204 insertions(+), 14 deletions(-) diff --git a/crates/shift-backends/src/ufo/reader.rs b/crates/shift-backends/src/ufo/reader.rs index 80985859..96b9e818 100644 --- a/crates/shift-backends/src/ufo/reader.rs +++ b/crates/shift-backends/src/ufo/reader.rs @@ -302,6 +302,13 @@ impl FontReader for UfoReader { font.add_source(Source::new(layer.name().to_string(), Location::new())) }; + if let Some(source) = font.source_mut(source_id.clone()) { + source.set_color(layer.color.as_ref().map(|color| color.to_rgba_string())); + if !layer.lib.is_empty() { + *source.lib_mut() = Self::convert_lib(&layer.lib); + } + } + for norad_glyph in layer.iter() { let (glyph, glyph_components) = Self::convert_glyph_layer(norad_glyph, LayerId::new(), source_id.clone()); diff --git a/crates/shift-backends/src/ufo/writer.rs b/crates/shift-backends/src/ufo/writer.rs index dbf8e395..3e686c54 100644 --- a/crates/shift-backends/src/ufo/writer.rs +++ b/crates/shift-backends/src/ufo/writer.rs @@ -302,6 +302,28 @@ impl Default for UfoWriter { } impl UfoWriter { + /// Writes a source's carried `layerinfo.plist` metadata (color and layer + /// lib) onto the norad layer it maps to. + fn apply_layer_metadata( + source: &shift_font::Source, + layer: &mut norad::Layer, + ) -> FormatBackendResult<()> { + layer.color = source + .color() + .map(|color| { + color + .parse::() + .map_err(|_| FormatBackendError::Ufo(format!("invalid layer color {color:?}"))) + }) + .transpose()?; + + if !source.lib().is_empty() { + layer.lib = Self::convert_lib(source.lib()); + } + + Ok(()) + } + pub fn save_view(&self, font: &impl FontView, path: &str) -> FormatBackendResult<()> { let norad_font = Self::build_norad_font(font)?; Self::write_atomic(&norad_font, Path::new(path)) @@ -388,6 +410,14 @@ impl UfoWriter { } } + if let Some(default_source) = font + .sources() + .iter() + .find(|source| Some(source.id()) == default_source_id) + { + Self::apply_layer_metadata(default_source, default_layer)?; + } + for source in font.sources() { if Some(source.id()) == default_source_id { continue; @@ -397,6 +427,7 @@ impl UfoWriter { .layers .new_layer(source.name()) .map_err(|e| FormatBackendError::Ufo(e.to_string()))?; + Self::apply_layer_metadata(source, norad_layer)?; for glyph in font.glyphs() { if let Some(layer_data) = glyph.layer_for_source(source.id()) { diff --git a/crates/shift-font/src/ir/font.rs b/crates/shift-font/src/ir/font.rs index 521ed05d..d1105037 100644 --- a/crates/shift-font/src/ir/font.rs +++ b/crates/shift-font/src/ir/font.rs @@ -400,6 +400,13 @@ impl Font { &self.data().sources } + pub fn source_mut(&mut self, source_id: SourceId) -> Option<&mut Source> { + self.data_mut() + .sources + .iter_mut() + .find(|source| source.id() == source_id) + } + pub fn add_source(&mut self, source: Source) -> SourceId { let source_id = source.id(); let data = self.data_mut(); diff --git a/crates/shift-font/src/ir/source.rs b/crates/shift-font/src/ir/source.rs index 5c88e544..3d0df4ae 100644 --- a/crates/shift-font/src/ir/source.rs +++ b/crates/shift-font/src/ir/source.rs @@ -1,5 +1,6 @@ use crate::axis::Location; use crate::entity::SourceId; +use crate::lib_data::LibData; use serde::{Deserialize, Serialize}; #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] @@ -9,6 +10,10 @@ pub struct Source { name: String, location: Location, filename: Option, + #[serde(default)] + color: Option, + #[serde(default)] + lib: LibData, } impl Source { @@ -18,6 +23,8 @@ impl Source { name, location, filename: None, + color: None, + lib: LibData::new(), } } @@ -27,6 +34,8 @@ impl Source { name, location, filename: Some(filename), + color: None, + lib: LibData::new(), } } @@ -41,6 +50,8 @@ impl Source { name, location, filename, + color: None, + lib: LibData::new(), } } @@ -60,6 +71,26 @@ impl Source { self.filename.as_deref() } + /// The source's display color from the format's layer metadata + /// (e.g. UFO `layerinfo.plist`), as an `r,g,b,a` string. + pub fn color(&self) -> Option<&str> { + self.color.as_deref() + } + + pub fn set_color(&mut self, color: Option) { + self.color = color; + } + + /// Layer-level lib data from the format's layer metadata + /// (e.g. UFO `layerinfo.plist`). + pub fn lib(&self) -> &LibData { + &self.lib + } + + pub fn lib_mut(&mut self) -> &mut LibData { + &mut self.lib + } + pub fn set_name(&mut self, name: String) { self.name = name; } diff --git a/crates/shift-font/src/test_support.rs b/crates/shift-font/src/test_support.rs index 975a884c..9188c326 100644 --- a/crates/shift-font/src/test_support.rs +++ b/crates/shift-font/src/test_support.rs @@ -134,12 +134,18 @@ pub fn sample_font() -> Font { regular_location, Some("Regular.ufo".to_string()), )); - font.add_source(Source::with_id( + let mut bold_source = Source::with_id( bold_id.clone(), "Bold".to_string(), bold_location, Some("Bold.ufo".to_string()), - )); + ); + bold_source.set_color(Some("1,0.75,0,0.7".to_string())); + bold_source.lib_mut().set( + "com.shift.sourceNote".to_string(), + LibValue::String("bold layer note".to_string()), + ); + font.add_source(bold_source); font.set_default_source_id(regular_id.clone()); let acute_id = GlyphId::from_raw("acute"); diff --git a/crates/shift-source/src/package.rs b/crates/shift-source/src/package.rs index 99f99780..2f296d48 100644 --- a/crates/shift-source/src/package.rs +++ b/crates/shift-source/src/package.rs @@ -264,6 +264,8 @@ struct SourceDoc { name: String, location: BTreeMap, filename: Option, + #[serde(default)] + color: Option, } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] @@ -386,10 +388,19 @@ struct LibModuleDoc { module: String, schema_version: u32, font: BTreeMap, + #[serde(default)] + sources: BTreeMap, glyphs: BTreeMap, layers: BTreeMap, } +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct SourceLibDoc { + name: String, + lib: BTreeMap, +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] struct GlyphLibDoc { @@ -537,6 +548,10 @@ pub fn tree_to_font(tree: PackageTree) -> Result { font.add_source(Source::try_from(source_doc)?); } + if let Some(module_doc) = &mut lib_module_doc { + apply_lib_module_to_sources(&mut font, module_doc)?; + } + let source_ids = font .sources() .iter() @@ -1002,6 +1017,40 @@ fn validate_lib_module(module_doc: &LibModuleDoc) -> Result<(), SourcePackageErr Ok(()) } +fn apply_lib_module_to_sources( + font: &mut Font, + module_doc: &mut LibModuleDoc, +) -> Result<(), SourcePackageError> { + let source_ids = font.sources().iter().map(Source::id).collect::>(); + for source_id in source_ids { + let Some(source_doc) = module_doc.sources.remove(&source_id.to_string()) else { + continue; + }; + + let source = font.source_mut(source_id.clone()).ok_or_else(|| { + SourcePackageError::DanglingReference { + field: "lib source", + id: source_id.to_string(), + } + })?; + + if source_doc.name != source.name() { + return Err(SourcePackageError::InvalidModule { + path: LIB_MODULE_FILE.to_string(), + message: format!( + "source lib name cache {:?} does not match source {:?}", + source_doc.name, + source.name() + ), + }); + } + + *source.lib_mut() = lib_from_doc(source_doc.lib)?; + } + + Ok(()) +} + fn apply_lib_module_to_glyph( glyph: &mut Glyph, module_doc: &mut LibModuleDoc, @@ -1074,6 +1123,13 @@ fn apply_lib_module_to_glyph( } fn ensure_lib_module_consumed(module_doc: LibModuleDoc) -> Result<(), SourcePackageError> { + if let Some(source_id) = module_doc.sources.keys().next() { + return Err(SourcePackageError::DanglingReference { + field: "lib source", + id: source_id.clone(), + }); + } + if let Some(glyph_id) = module_doc.glyphs.keys().next() { return Err(SourcePackageError::DanglingReference { field: "lib glyph", @@ -1108,9 +1164,24 @@ fn lib_from_doc(doc: BTreeMap) -> Result Option { let font_lib = lib_to_doc(font.lib()); + let mut sources = BTreeMap::new(); let mut glyphs = BTreeMap::new(); let mut layers = BTreeMap::new(); + for source in font.sources() { + if source.lib().is_empty() { + continue; + } + + sources.insert( + source.id().to_string(), + SourceLibDoc { + name: source.name().to_string(), + lib: lib_to_doc(source.lib()), + }, + ); + } + for glyph in font.glyphs() { if !glyph.lib().is_empty() { glyphs.insert( @@ -1139,7 +1210,7 @@ impl LibModuleDoc { } } - if font_lib.is_empty() && glyphs.is_empty() && layers.is_empty() { + if font_lib.is_empty() && sources.is_empty() && glyphs.is_empty() && layers.is_empty() { return None; } @@ -1148,6 +1219,7 @@ impl LibModuleDoc { module: LIB_MODULE_NAME.to_string(), schema_version: LIB_MODULE_SCHEMA_VERSION, font: font_lib, + sources, glyphs, layers, }) @@ -1570,6 +1642,7 @@ impl TryFrom<&Source> for SourceDoc { name: source.name().to_string(), location, filename: source.filename().map(str::to_string), + color: source.color().map(str::to_string), }) } } @@ -1587,12 +1660,14 @@ impl TryFrom for Source { ); } - Ok(Self::with_id( + let mut source = Self::with_id( parse_id("source", &doc.id)?, doc.name, Location::from_map(location), doc.filename, - )) + ); + source.set_color(doc.color); + Ok(source) } } diff --git a/crates/shift-store/src/change_set.rs b/crates/shift-store/src/change_set.rs index 78b0faf9..f8631693 100644 --- a/crates/shift-store/src/change_set.rs +++ b/crates/shift-store/src/change_set.rs @@ -37,6 +37,7 @@ impl ShiftStore { tx.execute("DELETE FROM glyph_unicodes", [])?; tx.execute("DELETE FROM glyphs", [])?; tx.execute("DELETE FROM source_locations", [])?; + tx.execute("DELETE FROM source_lib", [])?; tx.execute("DELETE FROM sources", [])?; tx.execute("DELETE FROM axes", [])?; @@ -58,8 +59,16 @@ impl ShiftStore { &source.id(), Some(source.name()), source.filename(), + source.color(), order_index as i64, )?; + replace_lib_data( + &tx, + "source_lib", + "source_id", + Some(&source.id().to_string()), + source.lib(), + )?; for (axis_id, value) in source.location().iter() { // Location entries on undefined axes have no row to reference. @@ -112,7 +121,7 @@ fn apply_change(tx: &Transaction<'_>, change: &font::FontChange) -> Result<(), S Ok(()) } font::FontChange::SourceCreated(change) => { - upsert_source(tx, &change.source_id, Some(&change.name), None, 0)?; + upsert_source(tx, &change.source_id, Some(&change.name), None, None, 0)?; for axis_value in &change.location { upsert_source_location( @@ -301,18 +310,20 @@ fn upsert_source( source_id: &font::SourceId, name: Option<&str>, filename: Option<&str>, + color: Option<&str>, order_index: i64, ) -> Result<(), StoreError> { tx.execute( " - INSERT INTO sources (id, name, filename, kind, order_index) - VALUES (?1, ?2, ?3, 'master', ?4) + INSERT INTO sources (id, name, filename, color, kind, order_index) + VALUES (?1, ?2, ?3, ?4, 'master', ?5) ON CONFLICT(id) DO UPDATE SET name = COALESCE(excluded.name, sources.name), filename = excluded.filename, + color = excluded.color, order_index = excluded.order_index ", - params![source_id.to_string(), name, filename, order_index], + params![source_id.to_string(), name, filename, color, order_index], )?; Ok(()) } diff --git a/crates/shift-store/src/font_state.rs b/crates/shift-store/src/font_state.rs index 89c5ada1..cd6afb61 100644 --- a/crates/shift-store/src/font_state.rs +++ b/crates/shift-store/src/font_state.rs @@ -140,7 +140,7 @@ fn load_axes(conn: &rusqlite::Connection) -> Result, StoreError> fn load_sources(conn: &rusqlite::Connection) -> Result, StoreError> { let mut stmt = conn.prepare( " - SELECT id, name, filename + SELECT id, name, filename, color FROM sources ORDER BY order_index, id ", @@ -149,16 +149,29 @@ fn load_sources(conn: &rusqlite::Connection) -> Result, StoreE let rows = stmt.query_map([], |row| { let source_id = font::SourceId::from_raw(row.get::<_, String>(0)?); let location = load_source_location(conn, &source_id)?; - Ok(font::Source::with_id( + let mut source = font::Source::with_id( source_id, row.get::<_, Option>(1)?.unwrap_or_default(), location, row.get(2)?, - )) + ); + source.set_color(row.get(3)?); + Ok(source) })?; - rows.collect::, _>>() - .map_err(StoreError::from) + let mut sources = rows + .collect::, _>>() + .map_err(StoreError::from)?; + + for source in &mut sources { + *source.lib_mut() = load_lib_data( + conn, + "source_lib", + Some(("source_id", &source.id().to_string())), + )?; + } + + Ok(sources) } fn load_source_location( diff --git a/crates/shift-store/src/schema.rs b/crates/shift-store/src/schema.rs index 404a9c38..6a95cfaa 100644 --- a/crates/shift-store/src/schema.rs +++ b/crates/shift-store/src/schema.rs @@ -51,6 +51,7 @@ CREATE TABLE IF NOT EXISTS sources ( family_name TEXT, style_name TEXT, filename TEXT, + color TEXT, kind TEXT NOT NULL, order_index INTEGER NOT NULL DEFAULT 0 ); @@ -234,6 +235,14 @@ CREATE TABLE IF NOT EXISTS font_lib ( value_json TEXT NOT NULL ); +CREATE TABLE IF NOT EXISTS source_lib ( + source_id TEXT NOT NULL, + key TEXT NOT NULL, + value_json TEXT NOT NULL, + PRIMARY KEY (source_id, key), + FOREIGN KEY (source_id) REFERENCES sources(id) ON DELETE CASCADE +); + CREATE TABLE IF NOT EXISTS glyph_lib ( glyph_id TEXT NOT NULL, key TEXT NOT NULL, From 9b8af5bd38ed5072627fed8852d0daa895d6b412 Mon Sep 17 00:00:00 2001 From: Kostya Farber Date: Thu, 2 Jul 2026 07:57:20 +0100 Subject: [PATCH 4/5] feat: preserve unmapped fontinfo fields across UFO round-trips Shift models only a subset of fontinfo.plist; everything else (postscript hinting, OS/2, WOFF metadata, name records, ...) was dropped on save. The reader now captures unmapped keys as a plist-shaped remainder on Font, and the writer rebuilds fontinfo from it before overwriting every modeled key from the IR, so the IR always owns modeled fields. Font-level guideline names and colors are now carried too. The remainder persists through the .shift package (modules/shift.fontInfo.json) and the SQLite store. An empty styleMapFamilyName is deliberately not carried: the reference Python toolchain treats it as unset, and compilers that consume it verbatim export an empty family name. --- crates/shift-backends/src/traits.rs | 5 ++ crates/shift-backends/src/ufo/reader.rs | 72 ++++++++++++++++++++++- crates/shift-backends/src/ufo/writer.rs | 70 ++++++++++++++-------- crates/shift-backends/tests/export.rs | 16 ++++- crates/shift-bridge/src/bridge.rs | 4 ++ crates/shift-font/src/ir/font.rs | 16 +++++ crates/shift-font/src/test_support.rs | 16 +++++ crates/shift-source/src/lib.rs | 8 +-- crates/shift-source/src/package.rs | 67 +++++++++++++++++++++ crates/shift-source/tests/package_test.rs | 5 +- crates/shift-store/src/change_set.rs | 21 ++++--- crates/shift-store/src/font_state.rs | 1 + crates/shift-store/src/schema.rs | 5 ++ 13 files changed, 264 insertions(+), 42 deletions(-) diff --git a/crates/shift-backends/src/traits.rs b/crates/shift-backends/src/traits.rs index 43be1d50..d64e1b00 100644 --- a/crates/shift-backends/src/traits.rs +++ b/crates/shift-backends/src/traits.rs @@ -16,6 +16,7 @@ pub trait FontView { fn features(&self) -> &FeatureData; fn guidelines(&self) -> &[Guideline]; fn lib(&self) -> &LibData; + fn fontinfo_remainder(&self) -> &LibData; fn data_files(&self) -> &BinaryData; fn images(&self) -> &BinaryData; } @@ -65,6 +66,10 @@ impl FontView for Font { self.lib() } + fn fontinfo_remainder(&self) -> &LibData { + self.fontinfo_remainder() + } + fn data_files(&self) -> &BinaryData { self.data_files() } diff --git a/crates/shift-backends/src/ufo/reader.rs b/crates/shift-backends/src/ufo/reader.rs index 96b9e818..af34474c 100644 --- a/crates/shift-backends/src/ufo/reader.rs +++ b/crates/shift-backends/src/ufo/reader.rs @@ -11,6 +11,34 @@ use std::path::Path; pub struct UfoReader; +/// The `fontinfo.plist` keys Shift models on [`shift_font::FontMetadata`], +/// [`shift_font::FontMetrics`], and font guidelines. These keys are owned by +/// the IR: they never enter the fontinfo remainder, and on save they are +/// written from the IR fields. +pub(crate) const MAPPED_FONTINFO_KEYS: &[&str] = &[ + "familyName", + "styleName", + "versionMajor", + "versionMinor", + "copyright", + "trademark", + "openTypeNameDesigner", + "openTypeNameDesignerURL", + "openTypeNameManufacturer", + "openTypeNameManufacturerURL", + "openTypeNameLicense", + "openTypeNameLicenseURL", + "openTypeNameDescription", + "note", + "unitsPerEm", + "ascender", + "descender", + "capHeight", + "xHeight", + "italicAngle", + "guidelines", +]; + struct PendingComponent { layer_id: LayerId, base_glyph_name: String, @@ -72,11 +100,14 @@ impl UfoReader { } fn convert_guideline(guideline: &norad::Guideline) -> Guideline { - match guideline.line { + let mut converted = match guideline.line { Line::Horizontal(y) => Guideline::horizontal(y), Line::Vertical(x) => Guideline::vertical(x), Line::Angle { x, y, degrees } => Guideline::angled(x, y, degrees), - } + }; + converted.set_name(guideline.name.as_ref().map(|name| name.to_string())); + converted.set_color(guideline.color.as_ref().map(|color| color.to_rgba_string())); + converted } fn convert_plist_to_lib_value(plist: &plist::Value) -> LibValue { @@ -108,6 +139,39 @@ impl UfoReader { } } + /// Captures every `fontinfo.plist` field Shift does not model as a + /// plist-shaped map, so unknown-to-Shift fields survive a round-trip. + fn convert_fontinfo_remainder( + font_info: &norad::FontInfo, + ) -> FormatBackendResult> { + let value = plist::to_value(font_info) + .map_err(|e| FormatBackendError::Ufo(format!("failed to capture fontinfo: {e}")))?; + let plist::Value::Dictionary(mut dict) = value else { + return Ok(None); + }; + + for key in MAPPED_FONTINFO_KEYS { + dict.remove(key); + } + + // An empty style-map family is "unset" to the reference Python + // toolchain but poisons compilers that consume it verbatim (it + // becomes the exported family name), so it is not carried. + if dict + .get("styleMapFamilyName") + .and_then(plist::Value::as_string) + == Some("") + { + dict.remove("styleMapFamilyName"); + } + + if dict.is_empty() { + Ok(None) + } else { + Ok(Some(Self::convert_lib(&dict))) + } + } + fn convert_lib(lib: &plist::Dictionary) -> LibData { let mut data = HashMap::new(); for (k, v) in lib.iter() { @@ -293,6 +357,10 @@ impl FontReader for UfoReader { font.metrics_mut().x_height = norad_font.font_info.x_height; font.metrics_mut().italic_angle = norad_font.font_info.italic_angle; + if let Some(remainder) = Self::convert_fontinfo_remainder(&norad_font.font_info)? { + *font.fontinfo_remainder_mut() = remainder; + } + let norad_default_layer_name = norad_font.layers.default_layer().name().clone(); let mut pending_components = Vec::new(); for layer in norad_font.layers.iter() { diff --git a/crates/shift-backends/src/ufo/writer.rs b/crates/shift-backends/src/ufo/writer.rs index 3e686c54..17af8658 100644 --- a/crates/shift-backends/src/ufo/writer.rs +++ b/crates/shift-backends/src/ufo/writer.rs @@ -214,7 +214,12 @@ impl UfoWriter { .name() .map(|name| ufo_name("guideline", name)) .transpose()?; - Ok(norad::Guideline::new(line, name, None, None)) + // Colors that are not valid UFO color strings (possible for + // app-authored guidelines) cannot be written and are dropped. + let color = guideline + .color() + .and_then(|color| color.parse::().ok()); + Ok(norad::Guideline::new(line, name, color, None)) } fn convert_lib_value_to_plist(value: &LibValue) -> plist::Value { @@ -329,31 +334,48 @@ impl UfoWriter { Self::write_atomic(&norad_font, Path::new(path)) } + /// Rebuilds `fontinfo.plist` from the carried remainder, then writes every + /// Shift-modeled field from the IR so the IR always wins for those keys. + fn build_font_info(font: &impl FontView) -> FormatBackendResult { + let remainder = font.fontinfo_remainder(); + let mut font_info = if remainder.is_empty() { + norad::FontInfo::default() + } else { + let dict = Self::convert_lib(remainder); + plist::from_value(&plist::Value::Dictionary(dict)).map_err(|e| { + FormatBackendError::Ufo(format!("invalid preserved fontinfo data: {e}")) + })? + }; + + font_info.family_name = font.metadata().family_name.clone(); + font_info.style_name = font.metadata().style_name.clone(); + font_info.version_major = font.metadata().version_major; + font_info.version_minor = font.metadata().version_minor.map(|v| v as u32); + font_info.copyright = font.metadata().copyright.clone(); + font_info.trademark = font.metadata().trademark.clone(); + font_info.open_type_name_designer = font.metadata().designer.clone(); + font_info.open_type_name_designer_url = font.metadata().designer_url.clone(); + font_info.open_type_name_manufacturer = font.metadata().manufacturer.clone(); + font_info.open_type_name_manufacturer_url = font.metadata().manufacturer_url.clone(); + font_info.open_type_name_license = font.metadata().license.clone(); + font_info.open_type_name_license_url = font.metadata().license_url.clone(); + font_info.open_type_name_description = font.metadata().description.clone(); + font_info.note = font.metadata().note.clone(); + + font_info.units_per_em = Some((font.metrics().units_per_em as u32).into()); + font_info.ascender = Some(font.metrics().ascender); + font_info.descender = Some(font.metrics().descender); + font_info.cap_height = font.metrics().cap_height; + font_info.x_height = font.metrics().x_height; + font_info.italic_angle = font.metrics().italic_angle; + font_info.guidelines = None; + + Ok(font_info) + } + fn build_norad_font(font: &impl FontView) -> FormatBackendResult { let mut norad_font = NoradFont::new(); - - norad_font.font_info.family_name = font.metadata().family_name.clone(); - norad_font.font_info.style_name = font.metadata().style_name.clone(); - norad_font.font_info.version_major = font.metadata().version_major; - norad_font.font_info.version_minor = font.metadata().version_minor.map(|v| v as u32); - norad_font.font_info.copyright = font.metadata().copyright.clone(); - norad_font.font_info.trademark = font.metadata().trademark.clone(); - norad_font.font_info.open_type_name_designer = font.metadata().designer.clone(); - norad_font.font_info.open_type_name_designer_url = font.metadata().designer_url.clone(); - norad_font.font_info.open_type_name_manufacturer = font.metadata().manufacturer.clone(); - norad_font.font_info.open_type_name_manufacturer_url = - font.metadata().manufacturer_url.clone(); - norad_font.font_info.open_type_name_license = font.metadata().license.clone(); - norad_font.font_info.open_type_name_license_url = font.metadata().license_url.clone(); - norad_font.font_info.open_type_name_description = font.metadata().description.clone(); - norad_font.font_info.note = font.metadata().note.clone(); - - norad_font.font_info.units_per_em = Some((font.metrics().units_per_em as u32).into()); - norad_font.font_info.ascender = Some(font.metrics().ascender); - norad_font.font_info.descender = Some(font.metrics().descender); - norad_font.font_info.cap_height = font.metrics().cap_height; - norad_font.font_info.x_height = font.metrics().x_height; - norad_font.font_info.italic_angle = font.metrics().italic_angle; + norad_font.font_info = Self::build_font_info(font)?; let groups = font .kerning() diff --git a/crates/shift-backends/tests/export.rs b/crates/shift-backends/tests/export.rs index e29764f4..0f9a0ac8 100644 --- a/crates/shift-backends/tests/export.rs +++ b/crates/shift-backends/tests/export.rs @@ -70,9 +70,23 @@ fn exports_mutatorsans_ufo_to_readable_ttf() { exported_family.contains(source_family), "exported family name should include source family: {exported_family}" ); + // A source-declared style map wins the exported subfamily name, so the + // style name is either the source style or its declared style-map style. + let source_style_map = match source.fontinfo_remainder().get("styleMapStyleName") { + Some(shift_font::LibValue::String(style_map)) => Some(style_map.as_str()), + _ => None, + }; assert!( exported_family.contains(source_style) - || exported.metadata().style_name.as_deref() == Some(source_style), + || exported.metadata().style_name.as_deref() == Some(source_style) + || exported + .metadata() + .style_name + .as_deref() + .zip(source_style_map) + .is_some_and(|(exported_style, style_map)| { + exported_style.eq_ignore_ascii_case(style_map) + }), "exported name data should include source style: family={exported_family}, style={:?}", exported.metadata().style_name ); diff --git a/crates/shift-bridge/src/bridge.rs b/crates/shift-bridge/src/bridge.rs index fe584c59..41df7317 100644 --- a/crates/shift-bridge/src/bridge.rs +++ b/crates/shift-bridge/src/bridge.rs @@ -198,6 +198,10 @@ impl FontView for FontSaveSnapshot { self.font.lib() } + fn fontinfo_remainder(&self) -> &shift_font::LibData { + self.font.fontinfo_remainder() + } + fn data_files(&self) -> &shift_font::BinaryData { self.font.data_files() } diff --git a/crates/shift-font/src/ir/font.rs b/crates/shift-font/src/ir/font.rs index d1105037..614ce074 100644 --- a/crates/shift-font/src/ir/font.rs +++ b/crates/shift-font/src/ir/font.rs @@ -103,6 +103,8 @@ struct FontData { guidelines: Vec, lib: LibData, #[serde(default)] + fontinfo_remainder: LibData, + #[serde(default)] data_files: BinaryData, #[serde(default)] images: BinaryData, @@ -298,6 +300,7 @@ impl Default for Font { features: FeatureData::new(), guidelines: Vec::new(), lib: LibData::new(), + fontinfo_remainder: LibData::new(), data_files: BinaryData::new(), images: BinaryData::new(), }, @@ -326,6 +329,7 @@ impl Font { features: FeatureData::new(), guidelines: Vec::new(), lib: LibData::new(), + fontinfo_remainder: LibData::new(), data_files: BinaryData::new(), images: BinaryData::new(), }, @@ -666,6 +670,18 @@ impl Font { &mut self.data_mut().lib } + /// Source-format font-info fields that Shift does not model, preserved + /// as a plist-shaped map keyed by the format's field names (e.g. UFO + /// `fontinfo.plist` keys). Modeled fields never appear here; they live + /// on [`FontMetadata`] and [`FontMetrics`] and win on save. + pub fn fontinfo_remainder(&self) -> &LibData { + &self.data().fontinfo_remainder + } + + pub fn fontinfo_remainder_mut(&mut self) -> &mut LibData { + &mut self.data_mut().fontinfo_remainder + } + /// Opaque files from the source format's `data/` directory, preserved /// verbatim across load and save. pub fn data_files(&self) -> &BinaryData { diff --git a/crates/shift-font/src/test_support.rs b/crates/shift-font/src/test_support.rs index 9188c326..0b1b697b 100644 --- a/crates/shift-font/src/test_support.rs +++ b/crates/shift-font/src/test_support.rs @@ -87,6 +87,22 @@ pub fn sample_font() -> Font { ]), ); + font.fontinfo_remainder_mut().set( + "postscriptBlueValues".to_string(), + LibValue::Array(vec![LibValue::Integer(-16), LibValue::Integer(0)]), + ); + font.fontinfo_remainder_mut() + .set("openTypeOS2WeightClass".to_string(), LibValue::Integer(700)); + let mut woff_metadata = HashMap::new(); + woff_metadata.insert( + "id".to_string(), + LibValue::String("dogfood-unique-id".to_string()), + ); + font.fontinfo_remainder_mut().set( + "woffMetadataUniqueID".to_string(), + LibValue::Dict(woff_metadata), + ); + font.data_files_mut().insert( "com.shift.testdata/nested/blob.bin".to_string(), vec![0x00, 0xFF, 0x10, 0x20], diff --git a/crates/shift-source/src/lib.rs b/crates/shift-source/src/lib.rs index 4ed56964..01a4a361 100644 --- a/crates/shift-source/src/lib.rs +++ b/crates/shift-source/src/lib.rs @@ -1,8 +1,8 @@ mod package; pub use package::{ - AXES_FILE, DATA_DIR, FEATURES_FILE, FONT_FILE, FORMAT_ID, GLYPHS_DIR, IMAGES_DIR, KERNING_FILE, - LIB_MODULE_FILE, MANIFEST_FILE, MODULES_DIR, PackageTree, SCHEMA_VERSION, SOURCES_FILE, - ShiftSourcePackage, SourcePackageError, font_to_tree, read_tree, tree_to_font, - write_tree_atomic, + AXES_FILE, DATA_DIR, FEATURES_FILE, FONT_FILE, FONTINFO_MODULE_FILE, FORMAT_ID, GLYPHS_DIR, + IMAGES_DIR, KERNING_FILE, LIB_MODULE_FILE, MANIFEST_FILE, MODULES_DIR, PackageTree, + SCHEMA_VERSION, SOURCES_FILE, ShiftSourcePackage, SourcePackageError, font_to_tree, read_tree, + tree_to_font, write_tree_atomic, }; diff --git a/crates/shift-source/src/package.rs b/crates/shift-source/src/package.rs index 2f296d48..b0ad15a7 100644 --- a/crates/shift-source/src/package.rs +++ b/crates/shift-source/src/package.rs @@ -26,6 +26,7 @@ pub const DATA_DIR: &str = "data"; pub const IMAGES_DIR: &str = "images"; pub const MODULES_DIR: &str = "modules"; pub const LIB_MODULE_FILE: &str = "modules/shift.libData.json"; +pub const FONTINFO_MODULE_FILE: &str = "modules/shift.fontInfo.json"; pub const FORMAT_ID: &str = "shift-source"; pub const SCHEMA_VERSION: u32 = 1; @@ -33,6 +34,8 @@ const KERNING_SCHEMA_VERSION: u32 = 1; const LIB_MODULE_OWNER: &str = "shift"; const LIB_MODULE_NAME: &str = "libData"; const LIB_MODULE_SCHEMA_VERSION: u32 = 1; +const FONTINFO_MODULE_NAME: &str = "fontInfo"; +const FONTINFO_MODULE_SCHEMA_VERSION: u32 = 1; pub type PackageTree = Vec<(String, Vec)>; @@ -401,6 +404,32 @@ struct SourceLibDoc { lib: BTreeMap, } +/// Source-format font-info fields Shift does not model, preserved verbatim +/// so a UFO round-trip through a `.shift` package stays lossless. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct FontInfoModuleDoc { + owner: String, + module: String, + schema_version: u32, + fields: BTreeMap, +} + +impl FontInfoModuleDoc { + fn from_font(font: &Font) -> Option { + if font.fontinfo_remainder().is_empty() { + return None; + } + + Some(Self { + owner: LIB_MODULE_OWNER.to_string(), + module: FONTINFO_MODULE_NAME.to_string(), + schema_version: FONTINFO_MODULE_SCHEMA_VERSION, + fields: lib_to_doc(font.fontinfo_remainder()), + }) + } +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] struct GlyphLibDoc { @@ -484,6 +513,10 @@ pub fn font_to_tree(font: &Font) -> Result { tree.push(json_entry(LIB_MODULE_FILE, &lib_module_doc)?); } + if let Some(fontinfo_module_doc) = FontInfoModuleDoc::from_font(font) { + tree.push(json_entry(FONTINFO_MODULE_FILE, &fontinfo_module_doc)?); + } + let mut glyph_entries = font .glyphs() .map(|glyph| { @@ -522,6 +555,11 @@ pub fn tree_to_font(tree: PackageTree) -> Result { if let Some(module_doc) = &lib_module_doc { validate_lib_module(module_doc)?; } + let fontinfo_module_doc: Option = + take_optional_json(&mut entries, FONTINFO_MODULE_FILE)?; + if let Some(module_doc) = &fontinfo_module_doc { + validate_fontinfo_module(module_doc)?; + } let mut font = Font::empty(); *font.metadata_mut() = FontMetadata::from(font_doc.metadata); @@ -538,6 +576,10 @@ pub fn tree_to_font(tree: PackageTree) -> Result { *font.lib_mut() = lib_from_doc(std::mem::take(&mut module_doc.font))?; } + if let Some(module_doc) = fontinfo_module_doc { + *font.fontinfo_remainder_mut() = lib_from_doc(module_doc.fields)?; + } + for axis_doc in axes_doc.axes { font.add_axis(Axis::try_from(axis_doc)?); } @@ -1017,6 +1059,31 @@ fn validate_lib_module(module_doc: &LibModuleDoc) -> Result<(), SourcePackageErr Ok(()) } +fn validate_fontinfo_module(module_doc: &FontInfoModuleDoc) -> Result<(), SourcePackageError> { + if module_doc.owner != LIB_MODULE_OWNER { + return Err(SourcePackageError::InvalidModule { + path: FONTINFO_MODULE_FILE.to_string(), + message: format!("expected owner {LIB_MODULE_OWNER:?}"), + }); + } + + if module_doc.module != FONTINFO_MODULE_NAME { + return Err(SourcePackageError::InvalidModule { + path: FONTINFO_MODULE_FILE.to_string(), + message: format!("expected module {FONTINFO_MODULE_NAME:?}"), + }); + } + + if module_doc.schema_version != FONTINFO_MODULE_SCHEMA_VERSION { + return Err(SourcePackageError::InvalidModule { + path: FONTINFO_MODULE_FILE.to_string(), + message: format!("unsupported schema version {}", module_doc.schema_version), + }); + } + + Ok(()) +} + fn apply_lib_module_to_sources( font: &mut Font, module_doc: &mut LibModuleDoc, diff --git a/crates/shift-source/tests/package_test.rs b/crates/shift-source/tests/package_test.rs index b7e22d60..e7250c12 100644 --- a/crates/shift-source/tests/package_test.rs +++ b/crates/shift-source/tests/package_test.rs @@ -4,8 +4,8 @@ use shift_font::{ Axis, AxisId, Font, KerningPair, Location, Source, SourceId, test_support::sample_font, }; use shift_source::{ - AXES_FILE, DATA_DIR, FEATURES_FILE, FONT_FILE, GLYPHS_DIR, IMAGES_DIR, KERNING_FILE, - LIB_MODULE_FILE, MANIFEST_FILE, PackageTree, SOURCES_FILE, ShiftSourcePackage, + AXES_FILE, DATA_DIR, FEATURES_FILE, FONT_FILE, FONTINFO_MODULE_FILE, GLYPHS_DIR, IMAGES_DIR, + KERNING_FILE, LIB_MODULE_FILE, MANIFEST_FILE, PackageTree, SOURCES_FILE, ShiftSourcePackage, SourcePackageError, font_to_tree, tree_to_font, write_tree_atomic, }; use zip::{CompressionMethod, ZipArchive}; @@ -80,6 +80,7 @@ fn serializes_same_font_to_byte_identical_tree() { FEATURES_FILE, KERNING_FILE, LIB_MODULE_FILE, + FONTINFO_MODULE_FILE, &format!("{GLYPHS_DIR}/glyph_A.json"), &format!("{GLYPHS_DIR}/glyph_acute.json"), &format!("{DATA_DIR}/com.shift.testdata/nested/blob.bin"), diff --git a/crates/shift-store/src/change_set.rs b/crates/shift-store/src/change_set.rs index f8631693..810d3d90 100644 --- a/crates/shift-store/src/change_set.rs +++ b/crates/shift-store/src/change_set.rs @@ -22,6 +22,7 @@ impl ShiftStore { tx.execute("DELETE FROM glyph_layer_lib", [])?; tx.execute("DELETE FROM glyph_lib", [])?; tx.execute("DELETE FROM font_lib", [])?; + tx.execute("DELETE FROM fontinfo_remainder", [])?; tx.execute("DELETE FROM font_binaries", [])?; tx.execute("DELETE FROM kerning_pairs", [])?; tx.execute("DELETE FROM kerning_group_members", [])?; @@ -45,6 +46,13 @@ impl ShiftStore { replace_feature_text(&tx, font.features().fea_source())?; replace_font_guidelines(&tx, font.guidelines())?; replace_lib_data(&tx, "font_lib", "key", None, font.lib())?; + replace_lib_data( + &tx, + "fontinfo_remainder", + "key", + None, + font.fontinfo_remainder(), + )?; replace_font_binaries(&tx, "data", font.data_files())?; replace_font_binaries(&tx, "image", font.images())?; replace_kerning(&tx, font.kerning())?; @@ -804,16 +812,11 @@ fn replace_lib_data( } } None => { - debug_assert_eq!(table, "font_lib"); - tx.execute("DELETE FROM font_lib", [])?; + let delete_sql = format!("DELETE FROM {table}"); + tx.execute(&delete_sql, [])?; + let insert_sql = format!("INSERT INTO {table} (key, value_json) VALUES (?1, ?2)"); for (key, value) in lib.iter() { - tx.execute( - " - INSERT INTO font_lib (key, value_json) - VALUES (?1, ?2) - ", - params![key, lib_value_json(value)?], - )?; + tx.execute(&insert_sql, params![key, lib_value_json(value)?])?; } } } diff --git a/crates/shift-store/src/font_state.rs b/crates/shift-store/src/font_state.rs index cd6afb61..f10f0a60 100644 --- a/crates/shift-store/src/font_state.rs +++ b/crates/shift-store/src/font_state.rs @@ -16,6 +16,7 @@ impl ShiftStore { *font.features_mut() = load_feature_data(&self.conn)?; *font.lib_mut() = load_lib_data(&self.conn, "font_lib", None)?; + *font.fontinfo_remainder_mut() = load_lib_data(&self.conn, "fontinfo_remainder", None)?; *font.data_files_mut() = load_font_binaries(&self.conn, "data")?; *font.images_mut() = load_font_binaries(&self.conn, "image")?; diff --git a/crates/shift-store/src/schema.rs b/crates/shift-store/src/schema.rs index 6a95cfaa..63f724bd 100644 --- a/crates/shift-store/src/schema.rs +++ b/crates/shift-store/src/schema.rs @@ -235,6 +235,11 @@ CREATE TABLE IF NOT EXISTS font_lib ( value_json TEXT NOT NULL ); +CREATE TABLE IF NOT EXISTS fontinfo_remainder ( + key TEXT PRIMARY KEY, + value_json TEXT NOT NULL +); + CREATE TABLE IF NOT EXISTS source_lib ( source_id TEXT NOT NULL, key TEXT NOT NULL, From c537b736938571791a12cb033c92ec2b49b3f3f0 Mon Sep 17 00:00:00 2001 From: Kostya Farber Date: Thu, 2 Jul 2026 08:00:24 +0100 Subject: [PATCH 5/5] test: UFO and .shift round-trips of libs, binaries, layerinfo, fontinfo --- crates/shift-backends/tests/round_trip/ufo.rs | 309 +++++++++++++++++- crates/shift-source/tests/package_test.rs | 45 ++- 2 files changed, 352 insertions(+), 2 deletions(-) diff --git a/crates/shift-backends/tests/round_trip/ufo.rs b/crates/shift-backends/tests/round_trip/ufo.rs index 09d2c115..4856032d 100644 --- a/crates/shift-backends/tests/round_trip/ufo.rs +++ b/crates/shift-backends/tests/round_trip/ufo.rs @@ -1,7 +1,7 @@ use std::path::{Path, PathBuf}; use shift_backends::font_loader::FontLoader; -use shift_font::{Anchor, Font, Glyph, GlyphLayer}; +use shift_font::{Anchor, Font, Glyph, GlyphLayer, LibValue}; fn fixtures_path() -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")) @@ -89,6 +89,313 @@ fn assert_layer_geometry_matches(original: &GlyphLayer, reloaded: &GlyphLayer) { } } +const PNG_BYTES: &[u8] = &[0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0x2A]; + +/// Builds a UFO exercising everything Shift must preserve without modeling: +/// every plist type in libs, data/ and images/ files, layerinfo color and +/// lib, and fontinfo fields outside the mapped set. +fn preservation_fixture_ufo(dir: &Path) -> PathBuf { + use norad::fontinfo::{Os2Panose, Os2WidthClass, WoffMetadataUniqueId}; + + let mut norad_font = norad::Font::new(); + + let info = &mut norad_font.font_info; + info.family_name = Some("Preservation Sans".to_string()); + info.style_name = Some("Regular".to_string()); + info.units_per_em = Some(1000_u32.into()); + info.ascender = Some(800.0); + info.descender = Some(-200.0); + info.postscript_blue_values = Some(vec![-16.0, 0.0, 500.0, 516.0]); + info.postscript_stem_snap_h = Some(vec![80.0, 88.0]); + info.postscript_stem_snap_v = Some(vec![92.0, 100.0]); + info.open_type_os2_weight_class = Some(700); + info.open_type_os2_width_class = Some(Os2WidthClass::Condensed); + info.open_type_os2_type = Some(vec![2]); + info.open_type_os2_unicode_ranges = Some(vec![0, 1, 38]); + info.open_type_os2_panose = Some(Os2Panose { + family_type: 2, + serif_style: 11, + weight: 8, + proportion: 3, + contrast: 5, + stroke_variation: 2, + arm_style: 2, + letterform: 2, + midline: 2, + x_height: 4, + }); + info.woff_metadata_unique_id = Some(WoffMetadataUniqueId { + id: "preservation-woff-id".to_string(), + }); + + norad_font.guidelines_mut().push(norad::Guideline::new( + norad::Line::Horizontal(520.0), + Some(norad::Name::new("x-guide").unwrap()), + Some("0,0.5,1,1".parse().unwrap()), + None, + )); + + let date = plist::Date::from_xml_format("2024-02-02T02:02:02Z").unwrap(); + let mut nested = plist::Dictionary::new(); + nested.insert("nestedString".into(), plist::Value::String("deep".into())); + nested.insert("nestedInt".into(), plist::Value::Integer(7.into())); + let lib = &mut norad_font.lib; + lib.insert( + "com.shift.string".into(), + plist::Value::String("lib string".into()), + ); + lib.insert("com.shift.int".into(), plist::Value::Integer(42.into())); + lib.insert( + "com.shift.negativeInt".into(), + plist::Value::Integer((-42_i64).into()), + ); + lib.insert( + "com.shift.hugeUnsigned".into(), + plist::Value::Integer(u64::MAX.into()), + ); + lib.insert("com.shift.float".into(), plist::Value::Real(1.25)); + lib.insert("com.shift.bool".into(), plist::Value::Boolean(true)); + lib.insert( + "com.shift.array".into(), + plist::Value::Array(vec![ + plist::Value::String("first".into()), + plist::Value::Integer(2.into()), + plist::Value::Boolean(false), + ]), + ); + lib.insert("com.shift.dict".into(), plist::Value::Dictionary(nested)); + lib.insert( + "com.shift.data".into(), + plist::Value::Data(vec![0, 1, 254, 255]), + ); + lib.insert("com.shift.date".into(), plist::Value::Date(date)); + + let mut glyph = norad::Glyph::new("A"); + glyph.width = 600.0; + glyph.codepoints.insert('A'); + glyph.lib.insert( + "com.shift.glyphNote".into(), + plist::Value::String("glyph lib".into()), + ); + glyph.lib.insert( + "com.shift.glyphDate".into(), + plist::Value::Date(plist::Date::from_xml_format("2025-12-31T23:59:59Z").unwrap()), + ); + norad_font.layers.default_layer_mut().insert_glyph(glyph); + + let background = norad_font.layers.new_layer("background").unwrap(); + background.color = Some("1,0,0,0.5".parse().unwrap()); + background.lib.insert( + "com.shift.layerRole".into(), + plist::Value::String("sketch".into()), + ); + let mut background_glyph = norad::Glyph::new("A"); + background_glyph.width = 600.0; + background.insert_glyph(background_glyph); + + norad_font + .data + .insert( + PathBuf::from("com.shift.test/nested/payload.bin"), + vec![0x00, 0x01, 0xFE, 0xFF], + ) + .unwrap(); + norad_font + .data + .insert(PathBuf::from("readme.txt"), b"data dir".to_vec()) + .unwrap(); + norad_font + .images + .insert(PathBuf::from("glyph-photo.png"), PNG_BYTES.to_vec()) + .unwrap(); + + let ufo_path = dir.join("Preservation.ufo"); + norad_font.save(&ufo_path).expect("fixture UFO should save"); + ufo_path +} + +fn lib_string(value: Option<&LibValue>) -> &str { + match value { + Some(LibValue::String(value)) => value, + other => panic!("expected string lib value, got {other:?}"), + } +} + +#[test] +fn preserves_libs_binaries_layerinfo_and_fontinfo_through_round_trip() { + let temp_dir = tempfile::tempdir().expect("tempdir should be created"); + let fixture = preservation_fixture_ufo(temp_dir.path()); + + let original = load_font(&fixture); + let reloaded = round_trip(&original); + + assert_eq!( + original.lib().get("com.shift.hugeUnsigned"), + Some(&LibValue::UnsignedInteger(u64::MAX)) + ); + assert_eq!( + original.lib().get("com.shift.negativeInt"), + Some(&LibValue::Integer(-42)) + ); + assert_eq!( + original.lib().get("com.shift.date"), + Some(&LibValue::Date("2024-02-02T02:02:02Z".to_string())) + ); + assert_eq!( + original.lib().get("com.shift.data"), + Some(&LibValue::Data(vec![0, 1, 254, 255])) + ); + assert!(!original.lib().is_empty()); + assert_eq!(reloaded.lib(), original.lib()); + + let original_glyph = original.glyph_by_name("A").expect("A should exist"); + let reloaded_glyph = reloaded.glyph_by_name("A").expect("A should survive"); + assert_eq!( + lib_string(original_glyph.lib().get("com.shift.glyphNote")), + "glyph lib" + ); + assert_eq!( + original_glyph.lib().get("com.shift.glyphDate"), + Some(&LibValue::Date("2025-12-31T23:59:59Z".to_string())) + ); + assert_eq!(reloaded_glyph.lib(), original_glyph.lib()); + + assert_eq!( + original + .data_files() + .get("com.shift.test/nested/payload.bin"), + Some([0x00, 0x01, 0xFE, 0xFF].as_slice()) + ); + assert_eq!( + original.data_files().get("readme.txt"), + Some(b"data dir".as_slice()) + ); + assert_eq!(original.images().get("glyph-photo.png"), Some(PNG_BYTES)); + assert_eq!(reloaded.data_files(), original.data_files()); + assert_eq!(reloaded.images(), original.images()); + + let original_background = original + .sources() + .iter() + .find(|source| source.name() == "background") + .expect("background source should exist"); + let reloaded_background = reloaded + .sources() + .iter() + .find(|source| source.name() == "background") + .expect("background source should survive"); + assert_eq!(original_background.color(), Some("1,0,0,0.5")); + assert_eq!(reloaded_background.color(), original_background.color()); + assert_eq!( + lib_string(original_background.lib().get("com.shift.layerRole")), + "sketch" + ); + assert_eq!(reloaded_background.lib(), original_background.lib()); + + let expected_remainder = [ + ( + "postscriptBlueValues", + LibValue::Array(vec![ + LibValue::Integer(-16), + LibValue::Integer(0), + LibValue::Integer(500), + LibValue::Integer(516), + ]), + ), + ( + "postscriptStemSnapH", + LibValue::Array(vec![LibValue::Integer(80), LibValue::Integer(88)]), + ), + ( + "postscriptStemSnapV", + LibValue::Array(vec![LibValue::Integer(92), LibValue::Integer(100)]), + ), + ("openTypeOS2WeightClass", LibValue::Integer(700)), + ("openTypeOS2WidthClass", LibValue::Integer(3)), + ( + "openTypeOS2Type", + LibValue::Array(vec![LibValue::Integer(2)]), + ), + ( + "openTypeOS2UnicodeRanges", + LibValue::Array(vec![ + LibValue::Integer(0), + LibValue::Integer(1), + LibValue::Integer(38), + ]), + ), + ( + "openTypeOS2Panose", + LibValue::Array(vec![ + LibValue::Integer(2), + LibValue::Integer(11), + LibValue::Integer(8), + LibValue::Integer(3), + LibValue::Integer(5), + LibValue::Integer(2), + LibValue::Integer(2), + LibValue::Integer(2), + LibValue::Integer(2), + LibValue::Integer(4), + ]), + ), + ]; + for (key, expected) in expected_remainder { + assert_eq!( + original.fontinfo_remainder().get(key), + Some(&expected), + "fontinfo remainder should carry {key}" + ); + } + match original.fontinfo_remainder().get("woffMetadataUniqueID") { + Some(LibValue::Dict(woff)) => { + assert_eq!(lib_string(woff.get("id")), "preservation-woff-id") + } + other => panic!("expected WOFF metadata dict, got {other:?}"), + } + assert!( + original.fontinfo_remainder().get("familyName").is_none(), + "modeled fields must not leak into the fontinfo remainder" + ); + assert_eq!(reloaded.fontinfo_remainder(), original.fontinfo_remainder()); + + let original_guideline = original + .guidelines() + .iter() + .find(|guideline| guideline.name() == Some("x-guide")) + .expect("font guideline should exist"); + let reloaded_guideline = reloaded + .guidelines() + .iter() + .find(|guideline| guideline.name() == Some("x-guide")) + .expect("font guideline should survive"); + assert_eq!(original_guideline.y(), Some(520.0)); + assert_eq!(original_guideline.color(), Some("0,0.5,1,1")); + assert_eq!(reloaded_guideline.y(), original_guideline.y()); + assert_eq!(reloaded_guideline.color(), original_guideline.color()); +} + +#[test] +fn edited_ir_fields_win_over_preserved_fontinfo_on_save() { + let temp_dir = tempfile::tempdir().expect("tempdir should be created"); + let fixture = preservation_fixture_ufo(temp_dir.path()); + + let mut font = load_font(&fixture); + font.metadata_mut().family_name = Some("Renamed Sans".to_string()); + + let reloaded = round_trip(&font); + + assert_eq!( + reloaded.metadata().family_name.as_deref(), + Some("Renamed Sans"), + "edited IR metadata must win on save" + ); + assert!( + reloaded.fontinfo_remainder().get("familyName").is_none(), + "no stale familyName may resurrect from the remainder" + ); +} + #[test] fn preserves_metadata_metrics_and_glyph_count() { let original = load_font(&mutatorsans_ufo_path()); diff --git a/crates/shift-source/tests/package_test.rs b/crates/shift-source/tests/package_test.rs index e7250c12..f93ad1b1 100644 --- a/crates/shift-source/tests/package_test.rs +++ b/crates/shift-source/tests/package_test.rs @@ -1,7 +1,8 @@ use std::fs::File; use shift_font::{ - Axis, AxisId, Font, KerningPair, Location, Source, SourceId, test_support::sample_font, + Axis, AxisId, Font, KerningPair, LibValue, Location, Source, SourceId, + test_support::sample_font, }; use shift_source::{ AXES_FILE, DATA_DIR, FEATURES_FILE, FONT_FILE, FONTINFO_MODULE_FILE, GLYPHS_DIR, IMAGES_DIR, @@ -50,6 +51,48 @@ fn shift_round_trip_preserves_whole_font() { assert_eq!(loaded, original); } +#[test] +fn shift_round_trip_preserves_exotic_lib_values_binaries_and_remainders() { + let temp = tempfile::tempdir().unwrap(); + let package_path = temp.path().join("Dogfood.shift"); + + ShiftSourcePackage::save_font(&package_path, &sample_font()).unwrap(); + let loaded = ShiftSourcePackage::load_font(&package_path).unwrap(); + + let Some(LibValue::Array(variants)) = loaded.lib().get("com.shift.allLibVariants") else { + panic!("font lib should carry the variant corpus"); + }; + assert!(variants.contains(&LibValue::Integer(-12))); + assert!(variants.contains(&LibValue::UnsignedInteger(u64::MAX))); + assert!(variants.contains(&LibValue::Date("2024-02-02T02:02:02Z".to_string()))); + assert!(variants.contains(&LibValue::Uid(9))); + assert!(variants.contains(&LibValue::Data(vec![0, 1, 2, 255]))); + + assert_eq!( + loaded + .data_files() + .get("com.shift.testdata/nested/blob.bin"), + Some([0x00, 0xFF, 0x10, 0x20].as_slice()) + ); + assert!(loaded.images().get("swatch.png").is_some()); + + let bold = loaded + .sources() + .iter() + .find(|source| source.name() == "Bold") + .expect("bold source should survive"); + assert_eq!(bold.color(), Some("1,0.75,0,0.7")); + assert_eq!( + bold.lib().get("com.shift.sourceNote"), + Some(&LibValue::String("bold layer note".to_string())) + ); + + assert_eq!( + loaded.fontinfo_remainder().get("openTypeOS2WeightClass"), + Some(&LibValue::Integer(700)) + ); +} + #[test] fn source_tree_round_trip_preserves_whole_font() { let original = sample_font();