Allow rowspan and colspan to be expressions

ChangeLog: the row, col, rowspan and colspan properties can now be
expressions rather than having to be constants. The layout adapts when
the value of the expression changes.

Author: dfaure-kdab

internal/compiler/layout.rs

@@ -281,16 +281,16 @@ pub struct GridLayoutElement {
     pub new_row: bool,
     pub col_expr: Option<Expression>,
     pub row_expr: Option<Expression>,
-    pub colspan: u16,
-    pub rowspan: u16,
+    pub colspan_expr: Option<Expression>,
+    pub rowspan_expr: Option<Expression>,
     pub item: LayoutItem,
 }
 
 impl GridLayoutElement {
-    pub fn span(&self, orientation: Orientation) -> u16 {
+    pub fn span(&self, orientation: Orientation) -> &Option<Expression> {
         match orientation {
-            Orientation::Horizontal => self.colspan,
-            Orientation::Vertical => self.rowspan,
+            Orientation::Horizontal => &self.colspan_expr,
+            Orientation::Vertical => &self.rowspan_expr,
         }
     }
 }

internal/compiler/llr/lower_expression.rs

@@ -857,18 +857,20 @@ fn grid_layout_cell_data(
             .elems
             .iter()
             .map(|c| {
-                let span = c.span(orientation);
                 let layout_info =
                     get_layout_info(&c.item.element, ctx, &c.item.constraints, orientation);
 
-                let mut lower_expr_or_auto = |expr: &Option<crate::expression_tree::Expression>| {
-                    expr.as_ref().map_or_else(
-                        || llr_Expression::NumberLiteral(u16::MAX.into()), // MAX means "auto", see to_layout_data()
-                        |e| lower_expression(e, ctx),
-                    )
-                };
-                let row_expr = lower_expr_or_auto(&c.row_expr);
-                let col_expr = lower_expr_or_auto(&c.col_expr);
+                let mut lower_expr_or_default =
+                    |expr: &Option<crate::expression_tree::Expression>, default: u16| {
+                        expr.as_ref().map_or_else(
+                            || llr_Expression::NumberLiteral(default.into()),
+                            |e| lower_expression(e, ctx),
+                        )
+                    };
+                // MAX means "auto", see to_layout_data()
+                let row_expr = lower_expr_or_default(&c.row_expr, u16::MAX);
+                let col_expr = lower_expr_or_default(&c.col_expr, u16::MAX);
+                let span_expr = lower_expr_or_default(&c.span(orientation), 1);
 
                 make_struct(
                     BuiltinPrivateStruct::GridLayoutCellData,
@@ -884,7 +886,7 @@ fn grid_layout_cell_data(
                             },
                         ),
                         ("row", Type::Int32, row_expr),
-                        ("span", Type::Int32, llr_Expression::NumberLiteral(span as _)),
+                        ("span", Type::Int32, span_expr),
                     ],
                 )
             })

internal/compiler/passes/lower_layout.rs

@@ -280,17 +280,6 @@ impl GridLayout {
         layout_cache_prop_v: &NamedReference,
         diag: &mut BuildDiagnostics,
     ) {
-        let mut get_const_value = |name: &str| {
-            item_element
-                .borrow_mut()
-                .bindings
-                .get(name)
-                .and_then(|e| eval_const_expr(&e.borrow().expression, name, &*e.borrow(), diag))
-        };
-        let colspan = get_const_value("colspan").unwrap_or(1);
-        let rowspan = get_const_value("rowspan").unwrap_or(1);
-
-        // TODO: use get_expr for colspan/rowspan too
         let mut get_expr = |name: &str| {
             item_element.borrow_mut().bindings.get(name).map(|e| {
                 let expr = &e.borrow().expression;
@@ -301,12 +290,14 @@ impl GridLayout {
 
         let row_expr = get_expr("row");
         let col_expr = get_expr("col");
+        let rowspan_expr = get_expr("rowspan");
+        let colspan_expr = get_expr("colspan");
 
         self.add_element_with_coord_as_expr(
             item_element,
             new_row,
             (&row_expr, &col_expr),
-            (rowspan, colspan),
+            (&rowspan_expr, &colspan_expr),
             layout_cache_prop_h,
             layout_cache_prop_v,
             diag,
@@ -329,7 +320,10 @@ impl GridLayout {
                 &Some(Expression::NumberLiteral(row as _, Unit::None)),
                 &Some(Expression::NumberLiteral(col as _, Unit::None)),
             ),
-            (rowspan, colspan),
+            (
+                &Some(Expression::NumberLiteral(rowspan as _, Unit::None)),
+                &Some(Expression::NumberLiteral(colspan as _, Unit::None)),
+            ),
             layout_cache_prop_h,
             layout_cache_prop_v,
             diag,
@@ -341,7 +335,7 @@ impl GridLayout {
         item_element: &ElementRc,
         new_row: bool,
         (row_expr, col_expr): (&Option<Expression>, &Option<Expression>),
-        (rowspan, colspan): (u16, u16),
+        (rowspan_expr, colspan_expr): (&Option<Expression>, &Option<Expression>),
         layout_cache_prop_h: &NamedReference,
         layout_cache_prop_v: &NamedReference,
         diag: &mut BuildDiagnostics,
@@ -379,8 +373,8 @@ impl GridLayout {
                 new_row,
                 col_expr: col_expr.clone(),
                 row_expr: row_expr.clone(),
-                colspan,
-                rowspan,
+                colspan_expr: colspan_expr.clone(),
+                rowspan_expr: rowspan_expr.clone(),
                 item: layout_item.item.clone(),
             });
         }
@@ -845,29 +839,6 @@ fn set_prop_from_cache(
     }
 }
 
-fn eval_const_expr(
-    expression: &Expression,
-    name: &str,
-    span: &dyn crate::diagnostics::Spanned,
-    diag: &mut BuildDiagnostics,
-) -> Option<u16> {
-    match super::ignore_debug_hooks(expression) {
-        Expression::NumberLiteral(v, Unit::None) => {
-            if *v < 0. || *v > u16::MAX as f64 || !v.trunc().approx_eq(v) {
-                diag.push_error(format!("'{name}' must be a positive integer"), span);
-                None
-            } else {
-                Some(*v as u16)
-            }
-        }
-        Expression::Cast { from, .. } => eval_const_expr(from, name, span, diag),
-        _ => {
-            diag.push_error(format!("'{name}' must be an integer literal"), span);
-            None
-        }
-    }
-}
-
 // If it's a number literal, it must be a positive integer
 // But also allow any other kind of expression
 fn check_number_literal_is_positive_integer(

internal/compiler/tests/syntax/basic/layout2.slint

@@ -13,7 +13,6 @@ export X := Rectangle {
                 text: lay.foo + parent.width;
 //                                     ^error{Element 'Row' does not have a property 'width'}
                 colspan: 1 + 1;
-//                       ^error{'colspan' must be an integer literal}
                 rowspan: 2;
             }
             Text {
@@ -23,6 +22,8 @@ export X := Rectangle {
 //                   ^error{'col' must be a positive integer}
                 rowspan: 2.2;
 //                       ^error{'rowspan' must be a positive integer}
+                colspan: -1;
+//                       ^error{'colspan' must be a positive integer}
                 y: 0;
 //                 ^error{The property 'y' cannot be set for elements placed in this layout, because the layout is already setting it}
 

internal/interpreter/eval_layout.rs

@@ -180,8 +180,8 @@ fn grid_layout_data(
                 &expr_eval,
             );
             let span = cell.span(orientation);
-            let mut eval_or_auto = |expr: &Option<Expression>| match expr {
-                None => u16::MAX, // auto
+            let mut eval_or_default = |expr: &Option<Expression>, default: u16| match expr {
+                None => default,
                 Some(e) => {
                     let value = eval_expression(e, local_context);
                     match value {
@@ -193,8 +193,9 @@ fn grid_layout_data(
                     }
                 }
             };
-            let row = eval_or_auto(&cell.row_expr);
-            let col = eval_or_auto(&cell.col_expr);
+            let row = eval_or_default(&cell.row_expr, u16::MAX);
+            let col = eval_or_default(&cell.col_expr, u16::MAX);
+            let span = eval_or_default(&span, 1);
             core_layout::GridLayoutCellData {
                 new_row: cell.new_row,
                 col_or_row: match orientation {

tests/cases/layout/grid_variable_row_col.slint

@@ -3,6 +3,7 @@
 
 export component TestCase inherits Window {
     property <int> blue_row: 1;
+    property <int> blue_colspan: 2;
     property <int> green_col: 1;
     property <int> red_row: 0;
     property <int> red_col: 0;
@@ -17,8 +18,7 @@ export component TestCase inherits Window {
             background: blue;
             row: blue_row;
             col: 0;
-            colspan: 1;
-            width: 40phx;
+            colspan: blue_colspan;
             height: 40phx;
         }
         rg := Rectangle {
@@ -39,23 +39,24 @@ export component TestCase inherits Window {
         }
     }
     out property <bool> initial_grid_ok: {
-        // red is at (0, 0), green is at (row=0, col=1), blue is at (row=1, col=0)
+        // red is at (0, 0), green is at (row=0, col=1), blue is at (row=1, col=0) with colspan 2
         rr.x == 50phx  && rr.y == 50phx  && rr.colspan == 1 &&
         rg.x == 100phx && rg.y == 50phx  && rg.colspan == 1 &&
-        rb.x == 50phx  && rb.y == 100phx && rb.colspan == 1
+        rb.x == 50phx  && rb.y == 100phx && rb.width == 90phx && rb.colspan == 2
     }
 
     public function change_grid() {
         green_col = 0;
         red_row = 1;
         blue_row = 2;
+        blue_colspan = 1;
     }
 
     out property <bool> final_grid_ok: {
         // now they are all at col 0, and in the order green, red, blue
-        rr.x == 50phx && rr.y == 100phx && rr.colspan == 1 &&
-        rg.x == 50phx && rg.y == 50phx  && rg.colspan == 1 &&
-        rb.x == 50phx && rb.y == 150phx && rb.colspan == 1
+        rr.x == 50phx && rr.y == 100phx && rr.width == 40phx && rr.colspan == 1 &&
+        rg.x == 50phx && rg.y == 50phx  && rg.width == 40phx && rg.colspan == 1 &&
+        rb.x == 50phx && rb.y == 150phx && rb.width == 40phx && rb.colspan == 1
     }
 
     init => {