Make container_width a required alignment parameter

Some prior discussion here: #278 (comment). > Aligning to the layout's calculated width is not necessarily a sensible default, as that width changes depending on how exactly lines were broken given the container width (max_advance) used for line breaking.
linebender · Feb 18, 2025 · a5c74e9 · a5c74e9
1 parent 5325b2c
commit a5c74e9
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 32 deletions.
diff --git a/examples/swash_render/src/main.rs b/examples/swash_render/src/main.rs
@@ -45,7 +45,7 @@ fn main() {
     let display_scale = 1.0;
 
     // The width for line wrapping
-    let max_advance = Some(200.0 * display_scale);
+    let max_advance = 200.0 * display_scale;
 
     // Colours for rendering
     let text_color = Rgba([0, 0, 0, 255]);
@@ -167,7 +167,7 @@ fn main() {
     };
 
     // Perform layout (including bidi resolution and shaping) with start alignment
-    layout.break_all_lines(max_advance);
+    layout.break_all_lines(Some(max_advance));
     layout.align(max_advance, Alignment::Start, AlignmentOptions::default());
 
     // Create image to render into

diff --git a/examples/tiny_skia_render/src/main.rs b/examples/tiny_skia_render/src/main.rs
@@ -48,7 +48,7 @@ fn main() {
     let display_scale = 1.0;
 
     // The width for line wrapping
-    let max_advance = Some(200.0 * display_scale);
+    let max_advance = 200.0 * display_scale;
 
     // Colours for rendering
     let foreground_color = Color::BLACK;
@@ -98,7 +98,7 @@ fn main() {
     let mut layout: Layout<ColorBrush> = builder.build(&text);
 
     // Perform layout (including bidi resolution and shaping) with start alignment
-    layout.break_all_lines(max_advance);
+    layout.break_all_lines(Some(max_advance));
     layout.align(max_advance, Alignment::Start, AlignmentOptions::default());
     let width = layout.width().ceil() as u32;
     let height = layout.height().ceil() as u32;

diff --git a/parley/src/layout/alignment.rs b/parley/src/layout/alignment.rs
@@ -34,11 +34,11 @@ impl Default for AlignmentOptions {
 /// Prior to re-line-breaking or re-aligning, [`unjustify`] has to be called.
 pub(crate) fn align<B: Brush>(
     layout: &mut LayoutData<B>,
-    alignment_width: Option<f32>,
+    alignment_width: f32,
     alignment: Alignment,
     options: AlignmentOptions,
 ) {
-    layout.alignment_width = alignment_width.unwrap_or(layout.width);
+    layout.alignment_width = alignment_width;
     layout.is_aligned_justified = alignment == Alignment::Justified;
 
     align_impl::<_, false>(layout, alignment, options);

diff --git a/parley/src/layout/cluster.rs b/parley/src/layout/cluster.rs
@@ -494,7 +494,7 @@ mod tests {
     fn cluster_from_position_with_alignment(alignment: Alignment) {
         let mut layout = create_unaligned_layout();
         let width = layout.full_width();
-        layout.align(Some(width + 100.), alignment, AlignmentOptions::default());
+        layout.align(width + 100., alignment, AlignmentOptions::default());
         assert_eq!(
             layout.len(),
             1,

diff --git a/parley/src/layout/editor.rs b/parley/src/layout/editor.rs
@@ -1038,8 +1038,11 @@ where
         }
         self.layout = builder.build(&self.buffer);
         self.layout.break_all_lines(self.width);
-        self.layout
-            .align(self.width, self.alignment, AlignmentOptions::default());
+        self.layout.align(
+            self.width.unwrap_or(self.layout.width()),
+            self.alignment,
+            AlignmentOptions::default(),
+        );
         self.selection = self.selection.refresh(&self.layout);
         self.layout_dirty = false;
         self.generation.nudge();

diff --git a/parley/src/layout/mod.rs b/parley/src/layout/mod.rs
@@ -184,7 +184,7 @@ impl<B: Brush> Layout<B> {
     /// [`Layout::width`] is used.
     pub fn align(
         &mut self,
-        container_width: Option<f32>,
+        container_width: f32,
         alignment: Alignment,
         options: AlignmentOptions,
     ) {

diff --git a/parley/src/lib.rs b/parley/src/lib.rs
@@ -51,8 +51,8 @@
 //! let mut layout: Layout<()> = builder.build(&TEXT);
 //!
 //! // Run line-breaking and alignment on the Layout
-//! const MAX_WIDTH : Option<f32> = Some(100.0);
-//! layout.break_all_lines(MAX_WIDTH);
+//! const MAX_WIDTH: f32 = 100.0;
+//! layout.break_all_lines(Some(MAX_WIDTH));
 //! layout.align(MAX_WIDTH, Alignment::Start, AlignmentOptions::default());
 //!
 //! // Inspect computed layout (see examples for more details)

diff --git a/parley/src/tests/test_basic.rs b/parley/src/tests/test_basic.rs
@@ -13,7 +13,11 @@ fn plain_multiline_text() {
     let mut builder = env.ranged_builder(text);
     let mut layout = builder.build(text);
     layout.break_all_lines(None);
-    layout.align(None, Alignment::Start, AlignmentOptions::default());
+    layout.align(
+        layout.width(),
+        Alignment::Start,
+        AlignmentOptions::default(),
+    );
 
     env.check_layout_snapshot(&layout);
 }
@@ -38,7 +42,11 @@ fn placing_inboxes() {
         });
         let mut layout = builder.build(text);
         layout.break_all_lines(None);
-        layout.align(None, Alignment::Start, AlignmentOptions::default());
+        layout.align(
+            layout.width(),
+            Alignment::Start,
+            AlignmentOptions::default(),
+        );
         env.with_name(test_case_name).check_layout_snapshot(&layout);
     }
 }
@@ -59,7 +67,11 @@ fn only_inboxes_wrap() {
     }
     let mut layout = builder.build(text);
     layout.break_all_lines(Some(40.0));
-    layout.align(None, Alignment::Middle, AlignmentOptions::default());
+    layout.align(
+        layout.width(),
+        Alignment::Middle,
+        AlignmentOptions::default(),
+    );
 
     env.check_layout_snapshot(&layout);
 }
@@ -91,7 +103,11 @@ fn full_width_inbox() {
         });
         let mut layout = builder.build(text);
         layout.break_all_lines(Some(100.));
-        layout.align(None, Alignment::Start, AlignmentOptions::default());
+        layout.align(
+            layout.width(),
+            Alignment::Start,
+            AlignmentOptions::default(),
+        );
         env.with_name(test_case_name).check_layout_snapshot(&layout);
     }
 }
@@ -104,7 +120,11 @@ fn trailing_whitespace() {
     let mut builder = env.ranged_builder(text);
     let mut layout = builder.build(text);
     layout.break_all_lines(Some(45.));
-    layout.align(None, Alignment::Start, AlignmentOptions::default());
+    layout.align(
+        layout.width(),
+        Alignment::Start,
+        AlignmentOptions::default(),
+    );
 
     assert!(
         layout.width() < layout.full_width(),
@@ -133,7 +153,11 @@ fn leading_whitespace() {
         builder.push_text("  Line 2");
         let (mut layout, _) = builder.build();
         layout.break_all_lines(None);
-        layout.align(None, Alignment::Start, AlignmentOptions::default());
+        layout.align(
+            layout.width(),
+            Alignment::Start,
+            AlignmentOptions::default(),
+        );
         env.with_name(test_case_name).check_layout_snapshot(&layout);
     }
 }
@@ -152,7 +176,7 @@ fn base_level_alignment_ltr() {
         let mut builder = env.ranged_builder(text);
         let mut layout = builder.build(text);
         layout.break_all_lines(Some(150.0));
-        layout.align(Some(150.0), alignment, AlignmentOptions::default());
+        layout.align(150.0, alignment, AlignmentOptions::default());
         env.with_name(test_case_name).check_layout_snapshot(&layout);
     }
 }
@@ -171,7 +195,7 @@ fn base_level_alignment_rtl() {
         let mut builder = env.ranged_builder(text);
         let mut layout = builder.build(text);
         layout.break_all_lines(Some(150.0));
-        layout.align(None, alignment, AlignmentOptions::default());
+        layout.align(layout.width(), alignment, AlignmentOptions::default());
         env.with_name(test_case_name).check_layout_snapshot(&layout);
     }
 }
@@ -186,7 +210,7 @@ fn overflow_alignment_rtl() {
     let mut builder = env.ranged_builder(text);
     let mut layout = builder.build(text);
     layout.break_all_lines(Some(1000.0));
-    layout.align(Some(10.), Alignment::Middle, AlignmentOptions::default());
+    layout.align(10., Alignment::Middle, AlignmentOptions::default());
     env.rendering_config().size = Some(Size::new(10., layout.height().into()));
     env.check_layout_snapshot(&layout);
 }
@@ -201,11 +225,19 @@ fn content_widths() {
     let mut layout = builder.build(text);
 
     layout.break_all_lines(Some(layout.min_content_width()));
-    layout.align(None, Alignment::Start, AlignmentOptions::default());
+    layout.align(
+        layout.width(),
+        Alignment::Start,
+        AlignmentOptions::default(),
+    );
     env.with_name("min").check_layout_snapshot(&layout);
 
     layout.break_all_lines(Some(layout.max_content_width()));
-    layout.align(None, Alignment::Start, AlignmentOptions::default());
+    layout.align(
+        layout.width(),
+        Alignment::Start,
+        AlignmentOptions::default(),
+    );
     env.with_name("max").check_layout_snapshot(&layout);
 }
 
@@ -219,11 +251,19 @@ fn content_widths_rtl() {
     let mut layout = builder.build(text);
 
     layout.break_all_lines(Some(layout.min_content_width()));
-    layout.align(None, Alignment::Start, AlignmentOptions::default());
+    layout.align(
+        layout.width(),
+        Alignment::Start,
+        AlignmentOptions::default(),
+    );
     env.with_name("min").check_layout_snapshot(&layout);
 
     layout.break_all_lines(Some(layout.max_content_width()));
-    layout.align(None, Alignment::Start, AlignmentOptions::default());
+    layout.align(
+        layout.width(),
+        Alignment::Start,
+        AlignmentOptions::default(),
+    );
     assert!(
         layout.width() <= layout.max_content_width(),
         "Layout should never be wider than the max content width"
@@ -246,7 +286,11 @@ fn inbox_content_width() {
         });
         let mut layout = builder.build(text);
         layout.break_all_lines(Some(layout.min_content_width()));
-        layout.align(None, Alignment::Start, AlignmentOptions::default());
+        layout.align(
+            layout.width(),
+            Alignment::Start,
+            AlignmentOptions::default(),
+        );
 
         env.with_name("full_width").check_layout_snapshot(&layout);
     }
@@ -262,7 +306,11 @@ fn inbox_content_width() {
         });
         let mut layout = builder.build(text);
         layout.break_all_lines(Some(layout.max_content_width()));
-        layout.align(None, Alignment::Start, AlignmentOptions::default());
+        layout.align(
+            layout.width(),
+            Alignment::Start,
+            AlignmentOptions::default(),
+        );
 
         assert!(
             layout.width() <= layout.max_content_width(),
@@ -287,11 +335,7 @@ fn realign() {
         if [2, 3, 4].contains(&idx) {
             layout.break_all_lines(Some(150.0));
         }
-        layout.align(
-            Some(150.),
-            Alignment::Justified,
-            AlignmentOptions::default(),
-        );
+        layout.align(150., Alignment::Justified, AlignmentOptions::default());
     }
     env.check_layout_snapshot(&layout);
 }