tawasuyu

tawasuyu/taffy

A high performance rust-powered UI layout library

HTML 5544Rust 88Markdown 11TOML 91926 Definiciones

Fix panic in FloatContext::subdivide_segment and enforce CSS2 float rule 5 (#988)

93b32fa75e2f62a143953f9276aa6306ae7e3b39 · por Nico Burns · (hace 3 meses) · padre c3d5a7964 · ver el árbol en este commit

Delta semántico · 3

src/compute/float.rs

tests/hand_written.rs

tests/hand_written/floats.rs

3 archivos · +76 −11

diff --git a/src/compute/float.rs b/src/compute/float.rs
index e565ac0e5..107670780 100644
--- a/src/compute/float.rs
+++ b/src/compute/float.rs
@@ -264,28 +264,26 @@ impl FloatContext {
         let slot = direction as usize;
 
         // Ensure that float:
-        //    - Starts at or after the last placed float that was floated in the same direction as it
+        //    - Starts at or after the last placed float in either direction (CSS2 float rule 5:
+        //      "The outer top of a floating box may not be higher than the outer top of any block
+        //      or floated box generated by an element earlier in the source document")
         //    - Respects "clear"
+        let float_start = self.last_placed_floats[0].start.max(self.last_placed_floats[1].start);
         let hwm = match clear {
             Clear::Left => {
-                let float_dir_start = self.last_placed_floats[slot].start;
                 let left_end = self.last_placed_floats[0].end;
-                float_dir_start.max(left_end + 1)
+                float_start.max(left_end + 1)
             }
             Clear::Right => {
-                let float_dir_start = self.last_placed_floats[slot].start;
                 let right_end = self.last_placed_floats[1].end;
-                float_dir_start.max(right_end + 1)
+                float_start.max(right_end + 1)
             }
             Clear::Both => {
                 let left_end = self.last_placed_floats[0].end;
                 let right_end = self.last_placed_floats[1].end;
                 left_end.max(right_end) + 1
             }
-            Clear::None => {
-                // float_dir_start
-                self.last_placed_floats[slot].start
-            }
+            Clear::None => float_start,
         };
 
         // Ensure that float is placed in a segment at or below "min_y"
@@ -428,9 +426,22 @@ impl FloatContext {
                 }
                 self.segments.len() - 1
             }
-            Some(end_idx) => {
+            Some(mut end_idx) => {
                 let end_y = start_y + floated_box.height;
-                if end_y != self.segments[end_idx].y.end {
+
+                // Due to floating point imprecision, `end_y` may coincide with (or fall before)
+                // the start of the end segment even though the sum of the candidate segment
+                // heights was computed as sufficient. In that case the float effectively ends at
+                // the boundary of the previous segment, so walk back to the segment that actually
+                // contains `end_y`.
+                while end_idx > start_idx && end_y <= self.segments[end_idx].y.start {
+                    end_idx -= 1;
+                }
+
+                // Only subdivide if `end_y` falls strictly within the segment. If it lands on
+                // (or, due to floating point imprecision, beyond) a segment boundary then no
+                // subdivision is needed.
+                if self.segments[end_idx].y.start < end_y && end_y < self.segments[end_idx].y.end {
                     self.subdivide_segment(end_idx, end_y);
                 }
 
diff --git a/tests/hand_written.rs b/tests/hand_written.rs
index 150f7845f..8f1b996ee 100644
--- a/tests/hand_written.rs
+++ b/tests/hand_written.rs
@@ -1,6 +1,7 @@
 mod hand_written {
     mod border_and_padding;
     mod caching;
+    mod floats;
     mod measure;
     mod min_max_overrides;
     mod relayout;
diff --git a/tests/hand_written/floats.rs b/tests/hand_written/floats.rs
new file mode 100644
index 000000000..0840dc8f1
--- /dev/null
+++ b/tests/hand_written/floats.rs
@@ -0,0 +1,53 @@
+#![cfg(feature = "float_layout")]
+use taffy::geometry::Point;
+use taffy::prelude::*;
+use taffy::style::Float;
+use taffy_test_helpers::new_test_tree;
+
+/// Regression test for <https://wpt.live/css/CSS2/floats-clear/floats-146.xht>
+///
+/// A right float placed after two left floats must not be placed higher than the
+/// outer top of the earlier floats (CSS2 float rule 5). This scenario previously
+/// panicked in `FloatContext::subdivide_segment` when the float's end coincided
+/// with an existing segment boundary due to floating point imprecision.
+#[test]
+fn float_no_higher_than_earlier_floats() {
+    let mut taffy = new_test_tree();
+
+    let block = |width: f32, height: f32, float: Float| Style {
+        display: Display::Block,
+        float,
+        size: Size { width: length(width), height: length(height) },
+        ..Default::default()
+    };
+
+    // Spacer block advancing the flow position to a fractional y offset (43.2)
+    let spacer = taffy.new_leaf(block(400.0, 43.2, Float::None)).unwrap();
+    let float_a = taffy.new_leaf(block(258.0, 42.0, Float::Left)).unwrap();
+    let float_b = taffy.new_leaf(block(298.0, 42.0, Float::Left)).unwrap();
+    let float_c = taffy.new_leaf(block(98.0, 42.0, Float::Right)).unwrap();
+
+    let root = taffy
+        .new_with_children(
+            Style {
+                display: Display::Block,
+                size: Size { width: length(400.0), height: auto() },
+                ..Default::default()
+            },
+            &[spacer, float_a, float_b, float_c],
+        )
+        .unwrap();
+
+    taffy.compute_layout(root, Size::MAX_CONTENT).unwrap();
+
+    let layout_a = taffy.layout(float_a).unwrap();
+    let layout_b = taffy.layout(float_b).unwrap();
+    let layout_c = taffy.layout(float_c).unwrap();
+
+    // A is placed below the spacer
+    assert_eq!(layout_a.location, Point { x: 0.0, y: 43.0 });
+    // B does not fit next to A, so is placed below it
+    assert_eq!(layout_b.location, Point { x: 0.0, y: 85.0 });
+    // C must not be placed higher than B (it fits next to B on the right)
+    assert_eq!(layout_c.location, Point { x: 302.0, y: 85.0 });
+}