consider spacing and padding when using relative values for the layout

Author: Murmele

internal/core/layout.rs

@@ -16,7 +16,7 @@ use num_traits::Float;
 
 pub use crate::items::Orientation;
 
-/// The constraint that applies to an item
+/// The constraint that applies to a layout
 // Also, the field needs to be in alphabetical order because how the generated code sort fields for struct
 #[repr(C)]
 #[derive(Clone, Copy, Debug, PartialEq)]
@@ -1139,18 +1139,22 @@ pub struct LayoutItemInfo {
 /// Solve a BoxLayout
 pub fn solve_box_layout(data: &BoxLayoutData, repeater_indices: Slice<u32>) -> SharedVector<Coord> {
     let mut result = SharedVector::<Coord>::default();
+    // TODO: why *2?
     result.resize(data.cells.len() * 2 + repeater_indices.len(), 0 as _);
 
     if data.cells.is_empty() {
         return result;
     }
 
+    let size_without_padding = data.size - data.padding.begin - data.padding.end;
+    let num_spacings = (data.cells.len() - 1) as Coord;
+    let content_size = size_without_padding - num_spacings * data.spacing; // The width the cells can occupy without going outside of the layout
     let mut layout_data: Vec<_> = data
         .cells
         .iter()
         .map(|c| {
-            let min = c.constraint.min.max(c.constraint.min_percent * data.size / 100 as Coord);
-            let max = c.constraint.max.min(c.constraint.max_percent * data.size / 100 as Coord);
+            let min = c.constraint.min.max(c.constraint.min_percent * content_size / 100 as Coord);
+            let max = c.constraint.max.min(c.constraint.max_percent * content_size / 100 as Coord);
             grid_internal::LayoutData {
                 min,
                 max,
@@ -1161,9 +1165,7 @@ pub fn solve_box_layout(data: &BoxLayoutData, repeater_indices: Slice<u32>) -> S
         })
         .collect();
 
-    let size_without_padding = data.size - data.padding.begin - data.padding.end;
     let pref_size: Coord = layout_data.iter().map(|it| it.pref).sum();
-    let num_spacings = (layout_data.len() - 1) as Coord;
     let spacings = data.spacing * num_spacings;
 
     let align = match data.alignment {
@@ -1239,12 +1241,12 @@ pub fn box_layout_info(
         return info;
     };
     let extra_w = padding.begin + padding.end + spacing * (count - 1) as Coord;
-    let min = cells.iter().map(|c| c.constraint.min).sum::<Coord>() + extra_w;
+    let min = cells.iter().map(|c| c.constraint.min).sum::<Coord>() + extra_w; // Minimum width of the complete layout
     let max = if is_stretch {
         (cells.iter().map(|c| c.constraint.max).fold(extra_w, Saturating::add)).max(min)
     } else {
         Coord::MAX
-    };
+    }; // Maximum width of the complete layout
     let preferred = cells.iter().map(|c| c.constraint.preferred_bounded()).sum::<Coord>() + extra_w;
     let stretch = cells.iter().map(|c| c.constraint.stretch).sum::<f32>();
     LayoutInfo { min, max, min_percent: 0 as _, max_percent: 100 as _, preferred, stretch }

internal/interpreter/eval_layout.rs

@@ -60,6 +60,7 @@ pub(crate) fn compute_grid_layout_info(
     .into()
 }
 
+/// Determine layout info of a box layout
 pub(crate) fn compute_box_layout_info(
     box_layout: &BoxLayout,
     orientation: Orientation,
@@ -394,6 +395,7 @@ fn flexbox_layout_data(
     (cells_h, cells_v, repeated_indices)
 }
 
+/// Determine the evaluated padding and spacing values from the layout geometry
 fn padding_and_spacing(
     layout_geometry: &LayoutGeometry,
     orientation: Orientation,
@@ -614,6 +616,7 @@ fn grid_layout_constraints(
     constraints
 }
 
+/// Collect all elements in this layout and store the LayoutItemInfo of it for further calculation
 fn box_layout_data(
     box_layout: &i_slint_compiler::layout::BoxLayout,
     orientation: Orientation,
@@ -625,6 +628,7 @@ fn box_layout_data(
     let mut cells = Vec::with_capacity(box_layout.elems.len());
     for cell in &box_layout.elems {
         if cell.element.borrow().repeated.is_some() {
+            // Collect all repeated elements
             let component_vec = repeater_instances(component, &cell.element);
             if let Some(ri) = repeater_indices.as_mut() {
                 ri.push(cells.len() as _);
@@ -636,6 +640,7 @@ fn box_layout_data(
                     .map(|x| x.as_pin_ref().layout_item_info(to_runtime(orientation), None)),
             );
         } else {
+            // Collect non repeated elements
             let mut layout_info =
                 get_layout_info(&cell.element, component, &window_adapter, orientation);
             fill_layout_info_constraints(

tests/cases/layout/horizontal_layout_percentages.slint

@@ -0,0 +1,57 @@
+// Copyright © SixtyFPS GmbH <info@slint.dev>
+// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
+
+// For the rectangle width the spacing must be subtracted when setting the width to 100%
+
+import { HorizontalBox } from "std-widgets.slint";
+export component TestCase inherits Window {
+    preferred-width: 300px;
+    preferred-height: 300px;
+
+    box:= HorizontalLayout {
+        spacing: 6px;
+        padding: 10px;
+        rect:= Rectangle {
+            width: 100%;
+            background: red;
+        }
+    }
+
+    out property <bool> check_padding: box.padding > 0;
+    out property <bool> check_x: rect.x == box.x + box.padding;
+    out property <bool> check_y: rect.y == box.y + box.padding;
+    out property <bool> check_width: rect.width == root.width - 2 * box.padding;
+    out property <bool> check_height: rect.height == root.height - 2 * box.padding;
+}
+
+/*
+
+```cpp
+auto handle = TestCase::create();
+const TestCase &instance = *handle;
+assert(instance.get_check_padding());
+assert(instance.get_check_x());
+assert(instance.get_check_x());
+assert(instance.get_check_width());
+assert(instance.get_check_height());
+```
+
+```rust
+let instance = TestCase::new().unwrap();
+assert!(instance.get_check_padding());
+assert!(instance.get_check_x());
+assert!(instance.get_check_y());
+assert!(instance.get_check_width());
+assert!(instance.get_check_height());
+```
+
+
+```js
+var instance = new slint.TestCase();
+assert(instance.check_padding);
+assert(instance.check_x);
+assert(instance.check_y);
+assert(instance.check_width);
+assert(instance.check_height);
+```
+*/

tests/cases/layout/horizontal_layout_percentages_multi_element.slint

@@ -0,0 +1,58 @@
+// Copyright © SixtyFPS GmbH <info@slint.dev>
+// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
+
+// The spacing and the padding must be considered for the determination of the width of the rectangles
+
+import { HorizontalBox } from "std-widgets.slint";
+export component TestCase inherits Window {
+    preferred-width: 300px;
+    preferred-height: 300px;
+
+    layout:= HorizontalLayout {
+        spacing: 10px; // Between elements
+        padding: 7px;
+        rect:= Rectangle {
+            width: 50%;
+            background: red;
+        }
+
+        rect2:= Rectangle {
+            width: 50%;
+            background: blue;
+        }
+    }
+
+    out property <bool> check_spacing: layout.spacing > 0;
+    out property <bool> check_padding: layout.padding > 0;
+    out property <bool> check_x: rect.x == layout.x + layout.padding;
+    out property <bool> check_width: rect.width + rect2.width + 2* layout.padding + layout.spacing == root.width;
+}
+
+/*
+
+```cpp
+auto handle = TestCase::create();
+const TestCase &instance = *handle;
+assert(instance.get_check_spacing());
+assert(instance.get_check_padding());
+assert(instance.get_check_x());
+assert(instance.get_check_width());
+```
+
+```rust
+let instance = TestCase::new().unwrap();
+assert!(instance.get_check_spacing());
+assert!(instance.get_check_padding());
+assert!(instance.get_check_x());
+assert!(instance.get_check_width());
+```
+
+
+```js
+var instance = new slint.TestCase();
+assert(instance.check_spacing);
+assert(instance.check_padding);
+assert(instance.check_x);
+assert(instance.check_width);
+```
+*/

tests/cases/layout/vertical_layout_percentages.slint

@@ -0,0 +1,57 @@
+// Copyright © SixtyFPS GmbH <info@slint.dev>
+// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
+
+// For the rectangle height the spacing must be subtracted when setting the height to 100%
+
+import { HorizontalBox } from "std-widgets.slint";
+export component TestCase inherits Window {
+    preferred-width: 300px;
+    preferred-height: 300px;
+
+    box:= VerticalLayout {
+        spacing: 6px;
+        padding: 10px;
+        rect:= Rectangle {
+            height: 100%;
+            background: red;
+        }
+    }
+
+    out property <bool> check_padding: box.padding > 0;
+    out property <bool> check_x: rect.x == box.x + box.padding;
+    out property <bool> check_y: rect.y == box.y + box.padding;
+    out property <bool> check_width: rect.width == root.width - 2 * box.padding;
+    out property <bool> check_height: rect.height == root.height - 2 * box.padding;
+}
+
+/*
+
+```cpp
+auto handle = TestCase::create();
+const TestCase &instance = *handle;
+assert(instance.get_check_padding());
+assert(instance.get_check_x());
+assert(instance.get_check_y());
+assert(instance.get_check_width());
+assert(instance.get_check_height());
+```
+
+```rust
+let instance = TestCase::new().unwrap();
+assert!(instance.get_check_padding());
+assert!(instance.get_check_x());
+assert!(instance.get_check_y());
+assert!(instance.get_check_width());
+assert!(instance.get_check_height());
+```
+
+
+```js
+var instance = new slint.TestCase();
+assert(instance.check_padding);
+assert(instance.check_x);
+assert(instance.check_y);
+assert(instance.check_width);
+assert(instance.check_height);
+```
+*/

tests/cases/layout/vertical_layout_percentages_multi_element.slint

@@ -0,0 +1,66 @@
+// Copyright © SixtyFPS GmbH <info@slint.dev>
+// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
+
+// The spacing and the padding must be considered for the determination of the height of the elements
+
+import { HorizontalBox } from "std-widgets.slint";
+export component TestCase inherits Window {
+    preferred-width: 300px;
+    preferred-height: 300px;
+
+    layout:= VerticalLayout {
+        spacing: 10px; // Between elements
+        padding: 7px;
+        rect:= Rectangle {
+            height: 50%;
+            background: red;
+        }
+
+        rect2:= Rectangle {
+            height: 50%;
+            background: blue;
+        }
+    }
+
+    out property <bool> check_spacing: layout.spacing > 0;
+    out property <bool> check_padding: layout.padding > 0;
+    out property <bool> check_x: rect.x == layout.x + layout.padding;
+    out property <bool> check_y: rect.y == layout.y + layout.padding;
+    out property <bool> check_width: rect.width == root.width - 2 * layout.padding;
+    out property <bool> check_height: rect.height + rect2.height + 2* layout.padding + layout.spacing == root.height;
+}
+
+/*
+
+```cpp
+auto handle = TestCase::create();
+const TestCase &instance = *handle;
+assert(instance.get_check_spacing());
+assert(instance.get_check_padding());
+assert(instance.get_check_x());
+assert(instance.get_check_y());
+assert(instance.get_check_width());
+assert(instance.get_check_height());
+```
+
+```rust
+let instance = TestCase::new().unwrap();
+assert!(instance.get_check_spacing());
+assert!(instance.get_check_padding());
+assert!(instance.get_check_x());
+assert!(instance.get_check_y());
+assert!(instance.get_check_width());
+assert!(instance.get_check_height());
+```
+
+
+```js
+var instance = new slint.TestCase();
+assert(instance.check_spacing);
+assert(instance.check_padding);
+assert(instance.check_x);
+assert(instance.check_y);
+assert(instance.check_width);
+assert(instance.check_height);
+```
+*/