From c16f7bff6d986ac40cb78dada36b348af4c0cf9f Mon Sep 17 00:00:00 2001 From: Nico Burns Date: Thu, 28 Nov 2024 13:43:07 +1300 Subject: [PATCH 1/8] Remove skrifa from vello's public API Signed-off-by: Nico Burns --- Cargo.lock | 1 + examples/scenes/Cargo.toml | 1 + examples/scenes/src/simple_text.rs | 14 +++++++++++--- vello/src/lib.rs | 1 - vello/src/scene.rs | 4 ++-- 5 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8c78eda1..2b764228 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1992,6 +1992,7 @@ dependencies = [ "image", "rand", "roxmltree", + "skrifa", "vello", "web-time", ] diff --git a/examples/scenes/Cargo.toml b/examples/scenes/Cargo.toml index 28ca944a..36c6688f 100644 --- a/examples/scenes/Cargo.toml +++ b/examples/scenes/Cargo.toml @@ -11,6 +11,7 @@ workspace = true [dependencies] vello = { workspace = true } +skrifa = { workspace = true } anyhow = { workspace = true } clap = { workspace = true, features = ["derive"] } image = { workspace = true, features = ["jpeg"] } diff --git a/examples/scenes/src/simple_text.rs b/examples/scenes/src/simple_text.rs index 09587143..e755b15a 100644 --- a/examples/scenes/src/simple_text.rs +++ b/examples/scenes/src/simple_text.rs @@ -3,6 +3,8 @@ use std::sync::Arc; +use skrifa::prelude::NormalizedCoord; +use skrifa::{raw::FontRef, MetadataProvider}; use vello::kurbo::Affine; use vello::peniko::{color::palette, Blob, Brush, BrushRef, Font, StyleRef}; use vello::skrifa::{raw::FontRef, MetadataProvider}; @@ -144,7 +146,7 @@ impl SimpleText { let brush = brush.into(); let style = style.into(); let axes = font_ref.axes(); - let font_size = vello::skrifa::instance::Size::new(size); + let font_size = skrifa::instance::Size::new(size); let var_loc = axes.location(variations.iter().copied()); let charmap = font_ref.charmap(); let metrics = font_ref.metrics(font_size, &var_loc); @@ -157,7 +159,13 @@ impl SimpleText { .font_size(size) .transform(transform) .glyph_transform(glyph_transform) - .normalized_coords(var_loc.coords()) + .normalized_coords( + var_loc + .coords() + .iter() + .copied() + .map(NormalizedCoord::to_bits), + ) .brush(brush) .hint(false) .draw( @@ -206,7 +214,7 @@ impl SimpleText { } fn to_font_ref(font: &Font) -> Option> { - use vello::skrifa::raw::FileRef; + use skrifa::raw::FileRef; let file_ref = FileRef::new(font.data.as_ref()).ok()?; match file_ref { FileRef::Font(font) => Some(font), diff --git a/vello/src/lib.rs b/vello/src/lib.rs index 0e135380..0c199dd2 100644 --- a/vello/src/lib.rs +++ b/vello/src/lib.rs @@ -147,7 +147,6 @@ pub mod low_level { pub use peniko; /// 2D geometry, with a focus on curves. pub use peniko::kurbo; -pub use skrifa; #[cfg(feature = "wgpu")] pub use wgpu; diff --git a/vello/src/scene.rs b/vello/src/scene.rs index 51f507f5..5e75e67f 100644 --- a/vello/src/scene.rs +++ b/vello/src/scene.rs @@ -406,7 +406,7 @@ impl<'a> DrawGlyphs<'a> { /// Sets the normalized design space coordinates for a variable font instance. #[must_use] - pub fn normalized_coords(mut self, coords: &[NormalizedCoord]) -> Self { + pub fn normalized_coords(mut self, coords: impl Iterator) -> Self { self.scene .encoding .resources @@ -416,7 +416,7 @@ impl<'a> DrawGlyphs<'a> { .encoding .resources .normalized_coords - .extend_from_slice(coords); + .extend(coords.map(NormalizedCoord::from_bits)); self.run.normalized_coords.end = self.scene.encoding.resources.normalized_coords.len(); self } From c0f2d4ab2aa6526ebe41f346c1850602b63c09c0 Mon Sep 17 00:00:00 2001 From: Nico Burns Date: Tue, 10 Dec 2024 21:17:26 +1300 Subject: [PATCH 2/8] Move imports to top of file --- examples/scenes/src/simple_text.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/examples/scenes/src/simple_text.rs b/examples/scenes/src/simple_text.rs index e755b15a..91e3dd04 100644 --- a/examples/scenes/src/simple_text.rs +++ b/examples/scenes/src/simple_text.rs @@ -4,10 +4,9 @@ use std::sync::Arc; use skrifa::prelude::NormalizedCoord; -use skrifa::{raw::FontRef, MetadataProvider}; +use skrifa::{raw::{FontRef, FileRef}, MetadataProvider}; use vello::kurbo::Affine; -use vello::peniko::{color::palette, Blob, Brush, BrushRef, Font, StyleRef}; -use vello::skrifa::{raw::FontRef, MetadataProvider}; +use vello::peniko::{color::palette, Blob, Brush, BrushRef, Font, StyleRef, Fill}; use vello::{Glyph, Scene}; // This is very much a hack to get things working. @@ -198,7 +197,6 @@ impl SimpleText { transform: Affine, text: &str, ) { - use vello::peniko::Fill; let brush = brush.unwrap_or(&Brush::Solid(palette::css::WHITE)); self.add_run( scene, @@ -214,7 +212,6 @@ impl SimpleText { } fn to_font_ref(font: &Font) -> Option> { - use skrifa::raw::FileRef; let file_ref = FileRef::new(font.data.as_ref()).ok()?; match file_ref { FileRef::Font(font) => Some(font), From b1c783750852643a86f7ed45fcc12c5d1f9f30ca Mon Sep 17 00:00:00 2001 From: Nico Burns Date: Tue, 10 Dec 2024 21:22:30 +1300 Subject: [PATCH 3/8] Add changelog entry Signed-off-by: Nico Burns --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74d7a3dd..1e81c990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ This is the first step towards providing richer color functionality, better hand - Breaking: The `full` feature is no longer present as the full pipeline is now always built ([#754][] by [@waywardmonkeys]) - The `r8` permutation of the shaders is no longer available ([#756][] by [@waywardmonkeys]) - Breaking: The `buffer_labels` feature is no longer present as the labels are always configured ([#757][] by [@waywardmonkeys]) +- Use `i16` rather than `skrifa::NormalizedCoord` in the public API ([#747][] by [@nicoburns]) ### Fixed From 16cfa9f6d69e23389896c837b4355bd7cc2653e5 Mon Sep 17 00:00:00 2001 From: Nico Burns Date: Tue, 10 Dec 2024 21:29:20 +1300 Subject: [PATCH 4/8] cargo fmt Signed-off-by: Nico Burns --- examples/scenes/src/simple_text.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/examples/scenes/src/simple_text.rs b/examples/scenes/src/simple_text.rs index 91e3dd04..a6ce59a0 100644 --- a/examples/scenes/src/simple_text.rs +++ b/examples/scenes/src/simple_text.rs @@ -4,9 +4,12 @@ use std::sync::Arc; use skrifa::prelude::NormalizedCoord; -use skrifa::{raw::{FontRef, FileRef}, MetadataProvider}; +use skrifa::{ + raw::{FileRef, FontRef}, + MetadataProvider, +}; use vello::kurbo::Affine; -use vello::peniko::{color::palette, Blob, Brush, BrushRef, Font, StyleRef, Fill}; +use vello::peniko::{color::palette, Blob, Brush, BrushRef, Fill, Font, StyleRef}; use vello::{Glyph, Scene}; // This is very much a hack to get things working. From 68f546cecca79b366d5a9dbf852ceeaa2b398a9c Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Wed, 15 Jan 2025 14:24:44 +0000 Subject: [PATCH 5/8] Address remaining issues --- Cargo.lock | 1 + examples/scenes/Cargo.toml | 1 + examples/scenes/src/simple_text.rs | 9 +-------- vello/src/lib.rs | 2 +- vello/src/scene.rs | 15 ++++++++------- vello_encoding/src/encoding.rs | 4 ++-- vello_encoding/src/lib.rs | 18 ++++++++++++++++++ vello_encoding/src/resolve.rs | 11 +++++++---- 8 files changed, 39 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2b764228..e5472d43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1987,6 +1987,7 @@ name = "scenes" version = "0.0.0" dependencies = [ "anyhow", + "bytemuck", "clap", "getrandom", "image", diff --git a/examples/scenes/Cargo.toml b/examples/scenes/Cargo.toml index 36c6688f..d9976ea2 100644 --- a/examples/scenes/Cargo.toml +++ b/examples/scenes/Cargo.toml @@ -19,6 +19,7 @@ rand = "0.8.5" # for pico_svg roxmltree = "0.20.0" +bytemuck.workspace = true [target.'cfg(target_arch = "wasm32")'.dependencies] web-time = { workspace = true } diff --git a/examples/scenes/src/simple_text.rs b/examples/scenes/src/simple_text.rs index a6ce59a0..2c062f6f 100644 --- a/examples/scenes/src/simple_text.rs +++ b/examples/scenes/src/simple_text.rs @@ -3,7 +3,6 @@ use std::sync::Arc; -use skrifa::prelude::NormalizedCoord; use skrifa::{ raw::{FileRef, FontRef}, MetadataProvider, @@ -161,13 +160,7 @@ impl SimpleText { .font_size(size) .transform(transform) .glyph_transform(glyph_transform) - .normalized_coords( - var_loc - .coords() - .iter() - .copied() - .map(NormalizedCoord::to_bits), - ) + .normalized_coords(bytemuck::cast_slice(var_loc.coords())) .brush(brush) .hint(false) .draw( diff --git a/vello/src/lib.rs b/vello/src/lib.rs index 0c199dd2..71d36537 100644 --- a/vello/src/lib.rs +++ b/vello/src/lib.rs @@ -152,7 +152,7 @@ pub use peniko::kurbo; pub use wgpu; pub use scene::{DrawGlyphs, Scene}; -pub use vello_encoding::Glyph; +pub use vello_encoding::{Glyph, NormalizedCoord}; use low_level::ShaderId; #[cfg(feature = "wgpu")] diff --git a/vello/src/scene.rs b/vello/src/scene.rs index 5e75e67f..2a144528 100644 --- a/vello/src/scene.rs +++ b/vello/src/scene.rs @@ -14,7 +14,7 @@ use peniko::{ use png::{BitDepth, ColorType, Transformations}; use skrifa::{ color::{ColorGlyph, ColorPainter}, - instance::{LocationRef, NormalizedCoord}, + instance::LocationRef, outline::{DrawSettings, OutlinePen}, prelude::Size, raw::{tables::cpal::Cpal, TableProvider}, @@ -22,7 +22,7 @@ use skrifa::{ }; #[cfg(feature = "bump_estimate")] use vello_encoding::BumpAllocatorMemory; -use vello_encoding::{Encoding, Glyph, GlyphRun, Patch, Transform}; +use vello_encoding::{Encoding, Glyph, GlyphRun, NormalizedCoord, Patch, Transform}; // TODO - Document invariants and edge cases (#470) // - What happens when we pass a transform matrix with NaN values to the Scene? @@ -406,7 +406,7 @@ impl<'a> DrawGlyphs<'a> { /// Sets the normalized design space coordinates for a variable font instance. #[must_use] - pub fn normalized_coords(mut self, coords: impl Iterator) -> Self { + pub fn normalized_coords(mut self, coords: &[NormalizedCoord]) -> Self { self.scene .encoding .resources @@ -416,7 +416,7 @@ impl<'a> DrawGlyphs<'a> { .encoding .resources .normalized_coords - .extend(coords.map(NormalizedCoord::from_bits)); + .extend(coords); self.run.normalized_coords.end = self.scene.encoding.resources.normalized_coords.len(); self } @@ -508,10 +508,11 @@ impl<'a> DrawGlyphs<'a> { let mut final_glyph = None; let mut outline_count = 0; // We copy out of the variable font coords here because we need to call an exclusive self method - let coords = &self.scene.encoding.resources.normalized_coords - [self.run.normalized_coords.clone()] + let coords = bytemuck::cast_slice::<_, skrifa::instance::NormalizedCoord>( + &self.scene.encoding.resources.normalized_coords[self.run.normalized_coords.clone()], + ) .to_vec(); - let location = LocationRef::new(coords); + let location = LocationRef::new(&coords); loop { let ppem = self.run.font_size; let outline_glyphs = (&mut glyphs).take_while(|glyph| { diff --git a/vello_encoding/src/encoding.rs b/vello_encoding/src/encoding.rs index d581acd4..4e7f8029 100644 --- a/vello_encoding/src/encoding.rs +++ b/vello_encoding/src/encoding.rs @@ -3,13 +3,13 @@ use super::{ DrawBlurRoundedRect, DrawColor, DrawImage, DrawLinearGradient, DrawRadialGradient, - DrawSweepGradient, DrawTag, Glyph, GlyphRun, Patch, PathEncoder, PathTag, Style, Transform, + DrawSweepGradient, DrawTag, Glyph, GlyphRun, NormalizedCoord, Patch, PathEncoder, PathTag, + Style, Transform, }; use peniko::color::{palette, DynamicColor}; use peniko::kurbo::{Shape, Stroke}; use peniko::{BlendMode, BrushRef, ColorStop, Extend, Fill, GradientKind, Image}; -use skrifa::instance::NormalizedCoord; /// Encoded data streams for a scene. /// diff --git a/vello_encoding/src/lib.rs b/vello_encoding/src/lib.rs index 0c63b1f5..cb17c8d2 100644 --- a/vello_encoding/src/lib.rs +++ b/vello_encoding/src/lib.rs @@ -76,3 +76,21 @@ pub use resolve::{resolve_solid_paths_only, Layout, Patch, Resolver}; #[cfg(feature = "bump_estimate")] pub use estimate::BumpEstimator; + +/// A normalized variation coordinate (for variable fonts) in 2.14 fixed point format. +/// +/// In most cases, this can be [cast](bytemuck::cast_slice) from the +/// normalised coords provided by your text layout library. +/// +/// Equivalent to [`skrifa::instance::NormalizedCoord`], but defined +/// in Vello so that Skrifa is not part of Vello's public API. +/// This allows Vello to update its Skrifa in a patch release, and limits +/// the need for updates only to align Skrifa versions. +pub type NormalizedCoord = i16; + +#[cfg(test)] +mod tests { + const _NORMALISED_COORD_SIZE_MATCHES: () = assert!( + size_of::() == size_of::() + ); +} diff --git a/vello_encoding/src/resolve.rs b/vello_encoding/src/resolve.rs index 44c03e3f..78249355 100644 --- a/vello_encoding/src/resolve.rs +++ b/vello_encoding/src/resolve.rs @@ -422,10 +422,13 @@ impl Resolver { hint = false; } } - let Some(mut session) = self - .glyph_cache - .session(&run.font, coords, font_size, hint, &run.style) - else { + let Some(mut session) = self.glyph_cache.session( + &run.font, + bytemuck::cast_slice(coords), + font_size, + hint, + &run.style, + ) else { continue; }; let glyph_start = self.glyphs.len(); From a9534cb19a8e56a1a8d49eb5d58b8486337d7371 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Wed, 15 Jan 2025 14:28:28 +0000 Subject: [PATCH 6/8] Update Changelog to reflect new reality Also sorts missed alphabetical names in changelog links --- CHANGELOG.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e81c990..a65f1abf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ This is the first step towards providing richer color functionality, better hand - Breaking: The `full` feature is no longer present as the full pipeline is now always built ([#754][] by [@waywardmonkeys]) - The `r8` permutation of the shaders is no longer available ([#756][] by [@waywardmonkeys]) - Breaking: The `buffer_labels` feature is no longer present as the labels are always configured ([#757][] by [@waywardmonkeys]) -- Use `i16` rather than `skrifa::NormalizedCoord` in the public API ([#747][] by [@nicoburns]) +- Breaking: Use a type alias for `i16` rather than `skrifa::NormalizedCoord` in the public API ([#747][] by [@nicoburns][]) ### Fixed @@ -143,15 +143,16 @@ This release has an [MSRV][] of 1.75. [@dfrg]: https://github.com/drfg [@DJMcNab]: https://github.com/DJMcNab [@kmoon2437]: https://github.com/kmoon2437 +[@LaurenzV]: https://github.com/LaurenzV [@msiglreith]: https://github.com/msiglreith +[@nicoburns]: https://github.com/nicoburns +[@ratmice]: https://github.com/ratmice [@simbleau]: https://github.com/simbleau [@TheNachoBIT]: https://github.com/TheNachoBIT [@timtom-dev]: https://github.com/timtom-dev [@TrueDoctor]: https://github.com/TrueDoctor [@waywardmonkeys]: https://github.com/waywardmonkeys [@yutannihilation]: https://github.com/yutannihilation -[@LaurenzV]: https://github.com/LaurenzV -[@ratmice]: https://github.com/ratmice [#416]: https://github.com/linebender/vello/pull/416 [#435]: https://github.com/linebender/vello/pull/435 @@ -214,6 +215,7 @@ This release has an [MSRV][] of 1.75. [#740]: https://github.com/linebender/vello/pull/740 [#742]: https://github.com/linebender/vello/pull/742 [#743]: https://github.com/linebender/vello/pull/743 +[#747]: https://github.com/linebender/vello/pull/747 [#754]: https://github.com/linebender/vello/pull/754 [#756]: https://github.com/linebender/vello/pull/756 From 8aae0a878d0923de391a10a8e956dbeefaf61680 Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Wed, 15 Jan 2025 14:31:03 +0000 Subject: [PATCH 7/8] Remove inferable turbofish --- vello/src/scene.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vello/src/scene.rs b/vello/src/scene.rs index 2a144528..ce409009 100644 --- a/vello/src/scene.rs +++ b/vello/src/scene.rs @@ -508,7 +508,7 @@ impl<'a> DrawGlyphs<'a> { let mut final_glyph = None; let mut outline_count = 0; // We copy out of the variable font coords here because we need to call an exclusive self method - let coords = bytemuck::cast_slice::<_, skrifa::instance::NormalizedCoord>( + let coords = bytemuck::cast_slice( &self.scene.encoding.resources.normalized_coords[self.run.normalized_coords.clone()], ) .to_vec(); From d81582dbc0fbc2e1e584e91a5679be66d28eea1b Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Wed, 15 Jan 2025 14:33:37 +0000 Subject: [PATCH 8/8] Fix unneeded change --- vello/src/scene.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vello/src/scene.rs b/vello/src/scene.rs index ce409009..7ef928dd 100644 --- a/vello/src/scene.rs +++ b/vello/src/scene.rs @@ -416,7 +416,7 @@ impl<'a> DrawGlyphs<'a> { .encoding .resources .normalized_coords - .extend(coords); + .extend_from_slice(coords); self.run.normalized_coords.end = self.scene.encoding.resources.normalized_coords.len(); self }