Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
191 changes: 50 additions & 141 deletions sparse_strips/vello_common/src/glyph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ impl<'a, T: GlyphRenderer + 'a> GlyphRunBuilder<'a, T> {
font_size: 16.0,
transform,
glyph_transform: None,
horizontal_skew: None,
hint: true,
normalized_coords: &[],
},
Expand All @@ -71,19 +70,17 @@ impl<'a, T: GlyphRenderer + 'a> GlyphRunBuilder<'a, T> {
self
}

/// Set the per-glyph transform. Use `horizontal_skew` to simulate italic text.
/// Set the per-glyph transform. Use `Affine::skew` with a horizontal-only skew to simulate
/// italic text.
pub fn glyph_transform(mut self, transform: Affine) -> Self {
self.run.glyph_transform = Some(transform);
self
}

/// Set the horizontal skew angle in radians to simulate italic/oblique text.
pub fn horizontal_skew(mut self, angle: f32) -> Self {
self.run.horizontal_skew = Some(angle);
self
}

/// Set whether font hinting is enabled.
///
/// This performs vertical hinting only. Hinting is performed only if the combined `transform`
/// and `glyph_transform` have a uniform scale and no vertical skew or rotation.
pub fn hint(mut self, hint: bool) -> Self {
self.run.hint = hint;
self
Expand Down Expand Up @@ -112,10 +109,7 @@ impl<'a, T: GlyphRenderer + 'a> GlyphRunBuilder<'a, T> {

let PreparedGlyphRun {
transform,
glyph_transform,
size,
scale,
horizontal_skew,
normalized_coords,
hinting_instance,
} = prepare_glyph_run(&self.run, &outlines);
Expand All @@ -140,18 +134,21 @@ impl<'a, T: GlyphRenderer + 'a> GlyphRunBuilder<'a, T> {
continue;
}

let mut local_transform =
Affine::translate(Vec2::new(glyph.x as f64 * scale, glyph.y as f64 * scale));
if let Some(skew) = horizontal_skew {
local_transform *= Affine::skew(skew.tan() as f64, 0.0);
}
if let Some(glyph_transform) = glyph_transform {
local_transform *= glyph_transform;
}
// Calculate the global glyph translation based on the glyph's local position within
// the run and the run's global transform.
//
// This is a partial affine matrix multiplication, calculating only the translation
// component that we need. It is added below to calculate the total transform of this
// glyph.
let [a, b, c, d, _, _] = self.run.transform.as_coeffs();
let translation = Vec2::new(
a * glyph.x as f64 + c * glyph.y as f64,
b * glyph.x as f64 + d * glyph.y as f64,
);

// When hinting, ensure the y-offset is integer. The x-offset doesn't matter, as we
// perform vertical-only hinting.
let mut total_transform = (transform * local_transform).as_coeffs();
let mut total_transform = transform.then_translate(translation).as_coeffs();
Copy link
Member

Choose a reason for hiding this comment

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

This feels suspicious to me. In theory, we want to skew, then translate locally, then transform globally, right?
But this ordering seems to me to be translate locally, then skew, then transform globally.

In any case, we need a test which has a long-ish text run skewed using glyph_transform.

Copy link
Member Author

Choose a reason for hiding this comment

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

For posterity: we apply glyph_transform, then translate with the glyph position from shaping/layouting, then apply the global transform.

Mathematically that's transform * glyph_translation * glyph_transform, where all of these are 3x3 affine matrices.

glyph_translation only adds a translation. That means we can equivalently calculate transform * glyph_transform, and then add the (transformed) translation component from glyph_translation to the total transform. This order is slightly cheaper to calculate.

if hinting_instance.is_some() {
total_transform[5] = total_transform[5].round();
}
Expand Down Expand Up @@ -181,22 +178,21 @@ struct GlyphRun<'a> {
font_size: f32,
/// Global transform.
transform: Affine,
/// Per-glyph transform. Use `horizontal_skew` to simulate italic text.
/// Per-glyph transform. Use [`Affine::skew`] with horizontal-skew only to simulate italic
/// text.
glyph_transform: Option<Affine>,
/// Horizontal skew angle in radians for simulating italic/oblique text.
horizontal_skew: Option<f32>,
/// Normalized variation coordinates for variable fonts.
normalized_coords: &'a [skrifa::instance::NormalizedCoord],
/// Controls whether font hinting is enabled.
hint: bool,
}

struct PreparedGlyphRun<'a> {
/// The total transform (`global_transform * glyph_transform`), not accounting for glyph
/// translation.
transform: Affine,
glyph_transform: Option<Affine>,
/// The font size to generate glyph outlines for.
size: Size,
scale: f64,
horizontal_skew: Option<f32>,
normalized_coords: &'a [skrifa::instance::NormalizedCoord],
hinting_instance: Option<HintingInstance>,
}
Expand All @@ -209,66 +205,50 @@ fn prepare_glyph_run<'a>(
run: &GlyphRun<'a>,
outlines: &OutlineGlyphCollection<'_>,
) -> PreparedGlyphRun<'a> {
// TODO: Consider extracting the scale from the glyph transform and applying it to the font size.
if !run.hint || run.glyph_transform.is_some() {
if !run.hint {
return PreparedGlyphRun {
transform: run.transform,
glyph_transform: run.glyph_transform,
transform: run.transform * run.glyph_transform.unwrap_or(Affine::IDENTITY),
size: Size::new(run.font_size),
scale: 1.0,
horizontal_skew: run.horizontal_skew,
normalized_coords: run.normalized_coords,
hinting_instance: None,
};
}

// Hinting doesn't make sense if we later scale the glyphs via some transform. So, if
// this glyph can be scaled uniformly, we extract the scale from its global and glyph
// transform and apply it to font size for hinting. Note that this extracted scale
// should be later applied to the glyph's position.
// We perform vertical-only hinting.
//
// Hinting doesn't make sense if we later scale the glyphs via some transform. So we extract
// the scale from the global transform and glyph transform and apply it to the font size for
// hinting. We do require the scaling to be uniform: simply using the vertical scale as font
// size and then transforming by the relative horizontal scale can cause, e.g., overlapping
// glyphs. Note that this extracted scale should be later applied to the glyph's position.
//
// If the glyph is rotated or skewed, hinting is not applicable.
// As the hinting is vertical-only, we can handle horizontal skew, but not vertical skew or
// rotations.

// Attempt to extract uniform scale from the run's transform.
if let Some((scale, transform)) = take_uniform_scale(run.transform) {
let font_size = run.font_size * scale as f32;
let total_transform = run.transform * run.glyph_transform.unwrap_or(Affine::IDENTITY);
let [t_a, t_b, t_c, t_d, t_e, t_f] = total_transform.as_coeffs();

let size = Size::new(font_size);
let uniform_scale = t_a == t_d;
let vertically_uniform = t_b == 0.;

if uniform_scale && vertically_uniform {
let vertical_font_size = run.font_size * t_d as f32;
let size = Size::new(vertical_font_size);
let hinting_instance =
HintingInstance::new(outlines, size, run.normalized_coords, HINTING_OPTIONS).ok();

return PreparedGlyphRun {
transform,
glyph_transform: run.glyph_transform,
PreparedGlyphRun {
transform: Affine::new([1., 0., t_c, 1., t_e, t_f]),
size,
scale,
horizontal_skew: run.horizontal_skew,
normalized_coords: run.normalized_coords,
hinting_instance,
};
}

PreparedGlyphRun {
transform: run.transform,
glyph_transform: run.glyph_transform,
size: Size::new(run.font_size),
scale: 1.0,
horizontal_skew: run.horizontal_skew,
normalized_coords: run.normalized_coords,
hinting_instance: None,
}
}

/// If `transform` has a uniform scale without rotation or skew, return the scale factor and the
/// transform with the scale factored out. Translation is unchanged.
fn take_uniform_scale(transform: Affine) -> Option<(f64, Affine)> {
let [a, b, c, d, e, f] = transform.as_coeffs();
if a == d && b == 0.0 && c == 0.0 {
let extracted_scale = a;
let transform_without_scale = Affine::new([1.0, 0.0, 0.0, 1.0, e, f]);
Some((extracted_scale, transform_without_scale))
}
} else {
None
PreparedGlyphRun {
transform: run.transform * run.glyph_transform.unwrap_or(Affine::IDENTITY),
size: Size::new(run.font_size),
normalized_coords: run.normalized_coords,
hinting_instance: None,
}
}
}

Expand Down Expand Up @@ -330,75 +310,4 @@ mod tests {

const _NORMALISED_COORD_SIZE_MATCHES: () =
assert!(size_of::<skrifa::instance::NormalizedCoord>() == size_of::<NormalizedCoord>());

mod take_uniform_scale {
use super::*;

#[test]
fn identity_transform() {
let identity = Affine::IDENTITY;
let result = take_uniform_scale(identity);
assert!(result.is_some());
let (scale, transform) = result.unwrap();
assert!((scale - 1.0).abs() < 1e-10);
assert_eq!(transform, Affine::IDENTITY);
}

#[test]
fn pure_uniform_scale() {
let scale_transform = Affine::scale(2.5);
let result = take_uniform_scale(scale_transform);
assert!(result.is_some());
let (scale, transform) = result.unwrap();
assert!((scale - 2.5).abs() < 1e-10);
assert_eq!(transform, Affine::IDENTITY);
}

#[test]
fn scale_with_translation() {
let scale_translate = Affine::scale(3.0).then_translate(Vec2::new(10.0, 20.0));
let result = take_uniform_scale(scale_translate);
assert!(result.is_some());
let (scale, transform) = result.unwrap();
assert!((scale - 3.0).abs() < 1e-10);
// The translation should be adjusted by the scale factor
assert_eq!(transform, Affine::translate(Vec2::new(10.0, 20.0)));
}

#[test]
fn pure_translation() {
let translation = Affine::translate(Vec2::new(5.0, 7.0));
let result = take_uniform_scale(translation);
assert!(result.is_some());
let (scale, transform) = result.unwrap();
assert!((scale - 1.0).abs() < 1e-10);
assert_eq!(transform, translation);
}

#[test]
fn non_uniform_scale() {
let non_uniform = Affine::scale_non_uniform(2.0, 3.0);
assert!(take_uniform_scale(non_uniform).is_none());
}

#[test]
fn rotation_transform() {
let rotation = Affine::rotate(std::f64::consts::PI / 4.0);
assert!(take_uniform_scale(rotation).is_none());
}

#[test]
fn skew_transform() {
let skew = Affine::skew(0.5, 0.0);
assert!(take_uniform_scale(skew).is_none());
}

#[test]
fn complex_transform() {
let complex = Affine::translate(Vec2::new(10.0, 20.0))
.then_rotate(std::f64::consts::PI / 6.0)
.then_scale(2.0);
assert!(take_uniform_scale(complex).is_none());
}
}
}
4 changes: 2 additions & 2 deletions sparse_strips/vello_cpu/snapshots/glyphs_glyph_transform.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions sparse_strips/vello_cpu/snapshots/glyphs_skewed_long.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions sparse_strips/vello_cpu/snapshots/glyphs_skewed_unskewed.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
84 changes: 82 additions & 2 deletions sparse_strips/vello_cpu/tests/glyph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ fn glyphs_skewed() {
ctx.set_paint(REBECCA_PURPLE.with_alpha(0.5).into());
ctx.glyph_run(&font)
.font_size(font_size)
.horizontal_skew(-20_f32.to_radians())
.glyph_transform(Affine::skew(-20_f64.to_radians().tan(), 0.))
.hint(true)
.fill_glyphs(glyphs.into_iter());

Expand All @@ -98,13 +98,93 @@ fn glyphs_skewed_unhinted() {
ctx.set_paint(REBECCA_PURPLE.with_alpha(0.5).into());
ctx.glyph_run(&font)
.font_size(font_size)
.horizontal_skew(-20_f32.to_radians())
.glyph_transform(Affine::skew(-20_f64.to_radians().tan(), 0.))
.hint(false)
.fill_glyphs(glyphs.into_iter());

check_ref(&ctx, "glyphs_skewed_unhinted");
}

#[test]
fn glyphs_skewed_long() {
let mut ctx = get_ctx(250, 75, false);
let font_size: f32 = 20_f32;
let (font, glyphs) = layout_glyphs(
"Lorem ipsum dolor sit amet,\nconsectetur adipiscing elit.\nSed ornare arcu lectus.",
font_size,
);

ctx.set_transform(Affine::translate((0., f64::from(font_size))));
ctx.set_paint(REBECCA_PURPLE.into());
ctx.glyph_run(&font)
.font_size(font_size)
.glyph_transform(Affine::skew(-10_f64.to_radians().tan(), 0.))
.hint(true)
.fill_glyphs(glyphs.into_iter());

check_ref(&ctx, "glyphs_skewed_long");
}

#[test]
fn glyphs_skewed_long_unhinted() {
let mut ctx = get_ctx(250, 75, false);
let font_size: f32 = 20_f32;
let (font, glyphs) = layout_glyphs(
"Lorem ipsum dolor sit amet,\nconsectetur adipiscing elit.\nSed ornare arcu lectus.",
font_size,
);

ctx.set_transform(Affine::translate((0., f64::from(font_size))));
ctx.set_paint(REBECCA_PURPLE.into());
ctx.glyph_run(&font)
.font_size(font_size)
.glyph_transform(Affine::skew(-10_f64.to_radians().tan(), 0.))
.hint(false)
.fill_glyphs(glyphs.into_iter());

check_ref(&ctx, "glyphs_skewed_long_unhinted");
}

#[test]
fn glyphs_skewed_unskewed() {
let mut ctx = get_ctx(150, 125, false);
let font_size: f32 = 50_f32;
let (font, glyphs) = layout_glyphs("Hello,\nworld!", font_size);

ctx.set_transform(
Affine::translate((0., f64::from(font_size)))
* Affine::skew(-20_f64.to_radians().tan(), 0.),
);
ctx.set_paint(REBECCA_PURPLE.into());
ctx.glyph_run(&font)
.font_size(font_size)
.glyph_transform(Affine::skew(20_f64.to_radians().tan(), 0.))
.hint(true)
.fill_glyphs(glyphs.into_iter());

check_ref(&ctx, "glyphs_skewed_unskewed");
Copy link
Member

@DJMcNab DJMcNab Apr 7, 2025

Choose a reason for hiding this comment

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

Edit: This rendering does now in fact look right to me - I missed that world is transposed to the left.
I still don't fully understand how this comes together


This rendering doesn't look right to me. If we're skewing each glyph individually, that should be a different operation to skewing the global box.
That is, this playground shows what I expect the rendering of this snapshot to be.
https://developer.mozilla.org/en-US/play?id=%2BlgWWSeKM7878QsZJVVId%2Bz0GMEv%2FqgYywTMjfjDnIW9K6nNfCtN5UPeH7%2FhgPFEQloDnLVzXIaKA7fG

(this is the same issue as my other comment)

}

#[test]
fn glyphs_skewed_unskewed_unhinted() {
let mut ctx = get_ctx(150, 125, false);
let font_size: f32 = 50_f32;
let (font, glyphs) = layout_glyphs("Hello,\nworld!", font_size);

ctx.set_transform(
Affine::translate((0., f64::from(font_size)))
* Affine::skew(-20_f64.to_radians().tan(), 0.),
);
ctx.set_paint(REBECCA_PURPLE.into());
ctx.glyph_run(&font)
.font_size(font_size)
.glyph_transform(Affine::skew(20_f64.to_radians().tan(), 0.))
.hint(false)
.fill_glyphs(glyphs.into_iter());

check_ref(&ctx, "glyphs_skewed_unskewed_unhinted");
}

#[test]
fn glyphs_scaled() {
let mut ctx = get_ctx(150, 125, false);
Expand Down