fonts: Unify letter-spacing/word-spacing implementations for harfbuzz and fast path (#38657)

Servo two shapers: harfbuzz and a "fast path" for simple cases that
don't have kerning or substitution tables. Both currently implement
`letter-spacing` and `word-spacing` seperately. Theres also a slight
difference, harfbuzz will apply word spacing to preserved newlines and
the fast path won't.

This change unifies the two implementations.

Testing: Covered by existing tests

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
This commit is contained in:
Simon Wülker 2025-08-13 20:55:28 +02:00 committed by GitHub
parent c741e45b21
commit dac34c0dfd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 29 additions and 32 deletions

View file

@ -456,14 +456,11 @@ impl Font {
None => continue,
};
let mut advance = Au::from_f64_px(self.glyph_h_advance(glyph_id));
if character == ' ' {
// https://drafts.csswg.org/css-text-3/#word-spacing-property
advance += options.word_spacing;
}
if let Some(letter_spacing) = options.letter_spacing {
advance += letter_spacing;
}
let mut advance = advance_for_shaped_glyph(
Au::from_f64_px(self.glyph_h_advance(glyph_id)),
character,
options,
);
let offset = prev_glyph_id.map(|prev| {
let h_kerning = Au::from_f64_px(self.glyph_h_kerning(prev, glyph_id));
advance += h_kerning;
@ -935,3 +932,25 @@ pub(crate) fn map_platform_values_to_style_values(mapping: &[(f64, f64)], value:
mapping[mapping.len() - 1].1
}
/// Computes the total advance for a glyph, taking `letter-spacing` and `word-spacing` into account.
pub(super) fn advance_for_shaped_glyph(
mut advance: Au,
character: char,
options: &ShapingOptions,
) -> Au {
if let Some(letter_spacing) = options.letter_spacing {
advance += letter_spacing;
};
// CSS 2.1 § 16.4 states that "word spacing affects each space (U+0020) and non-breaking
// space (U+00A0) left in the text after the white space processing rules have been
// applied. The effect of the property on other word-separator characters is undefined."
// We elect to only space the two required code points.
if character == ' ' || character == '\u{a0}' {
// https://drafts.csswg.org/css-text-3/#word-spacing-property
advance += options.word_spacing;
}
advance
}

View file

@ -28,6 +28,7 @@ use harfbuzz_sys::{
use log::debug;
use num_traits::Zero;
use crate::font::advance_for_shaped_glyph;
use crate::platform::font::FontTable;
use crate::{
BASE, ByteIndex, Font, FontBaseline, FontTableMethods, FontTableTag, GlyphData, GlyphId,
@ -561,8 +562,7 @@ impl Shaper {
)
} else {
let shape = glyph_data.entry_for_glyph(glyph_span.start, &mut y_pos);
let advance =
self.advance_for_shaped_glyph(shape.advance, character, options);
let advance = advance_for_shaped_glyph(shape.advance, character, options);
(shape.codepoint, advance, shape.offset)
};
@ -597,28 +597,6 @@ impl Shaper {
glyphs.finalize_changes();
}
fn advance_for_shaped_glyph(
&self,
mut advance: Au,
character: char,
options: &ShapingOptions,
) -> Au {
if let Some(letter_spacing) = options.letter_spacing {
advance += letter_spacing;
};
// CSS 2.1 § 16.4 states that "word spacing affects each space (U+0020) and non-breaking
// space (U+00A0) left in the text after the white space processing rules have been
// applied. The effect of the property on other word-separator characters is undefined."
// We elect to only space the two required code points.
if character == ' ' || character == '\u{a0}' {
// https://drafts.csswg.org/css-text-3/#word-spacing-property
advance += options.word_spacing;
}
advance
}
pub fn baseline(&self) -> Option<FontBaseline> {
unsafe { (*self.font).table_for_tag(BASE)? };