From dac34c0dfdf7f693c0fcdfad92ad564e80b6b16a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simon=20W=C3=BClker?= <simon.wuelker@arcor.de>
Date: Wed, 13 Aug 2025 20:55:28 +0200
Subject: [PATCH] fonts: Unify `letter-spacing`/`word-spacing` implementations
 for harfbuzz and fast path (#38657)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 components/fonts/font.rs   | 35 +++++++++++++++++++++++++++--------
 components/fonts/shaper.rs | 26 ++------------------------
 2 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/components/fonts/font.rs b/components/fonts/font.rs
index c98a40c7321..8723979814a 100644
--- a/components/fonts/font.rs
+++ b/components/fonts/font.rs
@@ -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
+}
diff --git a/components/fonts/shaper.rs b/components/fonts/shaper.rs
index fa9a919f84e..fa866849522 100644
--- a/components/fonts/shaper.rs
+++ b/components/fonts/shaper.rs
@@ -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)? };