Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove skrifa from vello's public API #747

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+19 −9
Open

Remove skrifa from vello's public API #747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6275e29
Remove skrifa from vello's public API
nicoburns Nov 28, 2024
aa5ee66
Move imports to top of file
nicoburns Dec 10, 2024
e847f5f
Add changelog entry
nicoburns Dec 10, 2024
e152003
cargo fmt
nicoburns Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ This is the first step towards providing richer color functionality, better hand
- Breaking: Updated to new `peniko` and `color` is now used for all colors ([#742][] by [@waywardmonkeys])
- 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])
- Use `i16` rather than `skrifa::NormalizedCoord` in the public API ([#747][] by [@nicoburns])

### Fixed

Expand Down
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions examples/scenes/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
20 changes: 14 additions & 6 deletions examples/scenes/src/simple_text.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@

use std::sync::Arc;

use skrifa::prelude::NormalizedCoord;
use skrifa::{
raw::{FileRef, FontRef},
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, Fill, Font, StyleRef};
use vello::{Glyph, Scene};

// This is very much a hack to get things working.
Expand Down Expand Up @@ -144,7 +148,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);
Expand All @@ -157,7 +161,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),
DJMcNab marked this conversation as resolved.
Show resolved Hide resolved
)
.brush(brush)
.hint(false)
.draw(
Expand Down Expand Up @@ -190,7 +200,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,
Expand All @@ -206,7 +215,6 @@ impl SimpleText {
}

fn to_font_ref(font: &Font) -> Option<FontRef<'_>> {
use vello::skrifa::raw::FileRef;
let file_ref = FileRef::new(font.data.as_ref()).ok()?;
match file_ref {
FileRef::Font(font) => Some(font),
Expand Down
1 change: 0 additions & 1 deletion vello/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,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;
Expand Down
4 changes: 2 additions & 2 deletions vello/src/scene.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ impl<'a> DrawGlyphs<'a> {
}

/// Sets the normalized design space coordinates for a variable font instance.
pub fn normalized_coords(mut self, coords: &[NormalizedCoord]) -> Self {
pub fn normalized_coords(mut self, coords: impl Iterator<Item = i16>) -> Self {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could require ExactSizeIterator here if we want to.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think it would hurt to have a pub type NormalizedCoord = i16 in vello_encoding

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does ExactSizeIterator win us?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we definitely should document this slightly better, even if just to say "this will be provided by your text layout implementation". The current format is quite unactionable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does ExactSizeIterator win us?

Potentially slightly cheaper allocation. But I suspect size_hint will work even without ExactSizeIterator, and that even if it doesn't the cost will be small.

self.scene
.encoding
.resources
Expand All @@ -411,7 +411,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
}
Expand Down
Loading