compiler: allow setting the row/col property from the outside

dfaure-kdab · ogoffart · commit 01a419306e5c · 2025-11-21T14:45:55.000+01:00
diff --git a/internal/compiler/layout.rs b/internal/compiler/layout.rs
@@ -275,19 +275,25 @@ impl LayoutConstraints {
     }
 }
 
+#[derive(Debug, Clone)]
+pub enum RowColExpr {
+    Named(NamedReference),
+    Literal(u16),
+}
+
 /// An element in a GridLayout
 #[derive(Debug, Clone)]
 pub struct GridLayoutElement {
     pub new_row: bool,
-    pub col_expr: Option<Expression>,
-    pub row_expr: Option<Expression>,
-    pub colspan_expr: Option<Expression>,
-    pub rowspan_expr: Option<Expression>,
+    pub col_expr: RowColExpr,
+    pub row_expr: RowColExpr,
+    pub colspan_expr: RowColExpr,
+    pub rowspan_expr: RowColExpr,
     pub item: LayoutItem,
 }
 
 impl GridLayoutElement {
-    pub fn span(&self, orientation: Orientation) -> &Option<Expression> {
+    pub fn span(&self, orientation: Orientation) -> &RowColExpr {
         match orientation {
             Orientation::Horizontal => &self.colspan_expr,
             Orientation::Vertical => &self.rowspan_expr,
@@ -426,7 +432,7 @@ fn find_binding<R>(
 }
 
 /// Return a named reference to a property if a binding is set on that property
-fn binding_reference(element: &ElementRc, name: &'static str) -> Option<NamedReference> {
+pub fn binding_reference(element: &ElementRc, name: &'static str) -> Option<NamedReference> {
     find_binding(element, name, |_, _, _| NamedReference::new(element, SmolStr::new_static(name)))
 }
 
@@ -468,6 +474,18 @@ impl GridLayout {
     pub fn visit_named_references(&mut self, visitor: &mut impl FnMut(&mut NamedReference)) {
         for cell in &mut self.elems {
             cell.item.constraints.visit_named_references(visitor);
+            if let RowColExpr::Named(ref mut e) = cell.col_expr {
+                visitor(e);
+            }
+            if let RowColExpr::Named(ref mut e) = cell.row_expr {
+                visitor(e);
+            }
+            if let RowColExpr::Named(ref mut e) = cell.colspan_expr {
+                visitor(e);
+            }
+            if let RowColExpr::Named(ref mut e) = cell.rowspan_expr {
+                visitor(e);
+            }
         }
         self.geometry.visit_named_references(visitor);
     }
diff --git a/internal/compiler/llr/lower_expression.rs b/internal/compiler/llr/lower_expression.rs
@@ -12,7 +12,7 @@ use super::lower_to_item_tree::{LoweredElement, LoweredSubComponentMapping, Lowe
 use super::{Animation, LocalMemberReference, MemberReference, PropertyIdx, RepeatedElementIdx};
 use crate::expression_tree::{BuiltinFunction, Callable, Expression as tree_Expression};
 use crate::langtype::{BuiltinPrivateStruct, EnumerationValue, Struct, StructName, Type};
-use crate::layout::Orientation;
+use crate::layout::{Orientation, RowColExpr};
 use crate::llr::Expression as llr_Expression;
 use crate::namedreference::NamedReference;
 use crate::object_tree::{Element, ElementRc, PropertyAnimation};
@@ -891,18 +891,16 @@ fn grid_layout_input_data(
             .elems
             .iter()
             .map(|c| {
-                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 rowspan_expr = lower_expr_or_default(&c.rowspan_expr, 1);
-                let colspan_expr = lower_expr_or_default(&c.colspan_expr, 1);
+                let propref_or_default = |named_ref: &RowColExpr| match named_ref {
+                    RowColExpr::Literal(n) => llr_Expression::NumberLiteral((*n).into()),
+                    RowColExpr::Named(e) => {
+                        llr_Expression::PropertyReference(ctx.map_property_reference(e))
+                    }
+                };
+                let row_expr = propref_or_default(&c.row_expr);
+                let col_expr = propref_or_default(&c.col_expr);
+                let rowspan_expr = propref_or_default(&c.rowspan_expr);
+                let colspan_expr = propref_or_default(&c.colspan_expr);
 
                 make_struct(
                     BuiltinPrivateStruct::GridLayoutInputData,
diff --git a/internal/compiler/passes/lower_layout.rs b/internal/compiler/passes/lower_layout.rs
@@ -340,21 +340,66 @@ impl GridLayout {
         numbering_type: &mut Option<RowColExpressionType>,
         diag: &mut BuildDiagnostics,
     ) {
-        let mut get_expr = |name: &str| {
-            let mut is_number_literal = false;
-            let expr = item_element.borrow_mut().bindings.get(name).map(|e| {
-                let expr = &e.borrow().expression;
-                is_number_literal =
-                    check_number_literal_is_positive_integer(expr, name, &*e.borrow(), diag);
-                expr.clone()
-            });
-            (expr, is_number_literal)
+        // Some compile-time checks
+        {
+            let mut check_expr = |name: &str| {
+                let mut is_number_literal = false;
+                let expr = item_element.borrow_mut().bindings.get(name).map(|e| {
+                    let expr = &e.borrow().expression;
+                    is_number_literal =
+                        check_number_literal_is_positive_integer(expr, name, &*e.borrow(), diag);
+                    expr.clone()
+                });
+                (expr, is_number_literal)
+            };
+
+            let (row_expr, row_is_number_literal) = check_expr("row");
+            let (col_expr, col_is_number_literal) = check_expr("col");
+            check_expr("rowspan");
+            check_expr("colspan");
+
+            let mut check_numbering_consistency =
+                |expr_type: RowColExpressionType, prop_name: &str| {
+                    if !matches!(expr_type, RowColExpressionType::Literal) {
+                        if let Some(current_numbering_type) = numbering_type {
+                            if *current_numbering_type != expr_type {
+                                let element_ref = item_element.borrow();
+                                let span: &dyn Spanned =
+                                    if let Some(binding) = element_ref.bindings.get(prop_name) {
+                                        &*binding.borrow()
+                                    } else {
+                                        &*element_ref
+                                    };
+                                diag.push_error(
+                                    format!("Cannot mix auto-numbering and runtime expressions for the '{prop_name}' property"),
+                                    span,
+                                );
+                            }
+                        } else {
+                            // Store the first auto or runtime expression case we see
+                            *numbering_type = Some(expr_type);
+                        }
+                    }
+                };
+
+            let row_expr_type =
+                RowColExpressionType::from_option_expr(&row_expr, row_is_number_literal);
+            check_numbering_consistency(row_expr_type, "row");
+
+            let col_expr_type =
+                RowColExpressionType::from_option_expr(&col_expr, col_is_number_literal);
+            check_numbering_consistency(col_expr_type, "col");
+        }
+
+        let propref_or_default = |name: &'static str| -> Option<RowColExpr> {
+            crate::layout::binding_reference(item_element, &name).map(|nr| RowColExpr::Named(nr))
         };
 
-        let (row_expr, row_is_number_literal) = get_expr("row");
-        let (col_expr, col_is_number_literal) = get_expr("col");
-        let (rowspan_expr, _) = get_expr("rowspan");
-        let (colspan_expr, _) = get_expr("colspan");
+        // MAX means "auto", see to_layout_data()
+        let row_expr = propref_or_default("row");
+        let col_expr = propref_or_default("col");
+        let rowspan_expr = propref_or_default("rowspan");
+        let colspan_expr = propref_or_default("colspan");
 
         self.add_element_with_coord_as_expr(
             item_element,
@@ -366,32 +411,6 @@ impl GridLayout {
             organized_data_prop,
             diag,
         );
-
-        let mut check_numbering_consistency = |expr_type: RowColExpressionType, prop_name: &str| {
-            if !matches!(expr_type, RowColExpressionType::Literal) {
-                if let Some(current_numbering_type) = numbering_type {
-                    if *current_numbering_type != expr_type {
-                        item_element.borrow_mut().bindings.get(prop_name).map(|binding| {
-                    diag.push_error(
-                    format!("Cannot mix auto-numbering and runtime expressions for the '{prop_name}' property"),
-                    &*binding.borrow(),
-                    );
-                });
-                    }
-                } else {
-                    // Store the first auto or runtime expression case we see
-                    *numbering_type = Some(expr_type);
-                }
-            }
-        };
-
-        let row_expr_type =
-            RowColExpressionType::from_option_expr(&row_expr, row_is_number_literal);
-        check_numbering_consistency(row_expr_type, "row");
-
-        let col_expr_type =
-            RowColExpressionType::from_option_expr(&col_expr, col_is_number_literal);
-        check_numbering_consistency(col_expr_type, "col");
     }
 
     fn add_element_with_coord(
@@ -407,14 +426,8 @@ impl GridLayout {
         self.add_element_with_coord_as_expr(
             item_element,
             false, // new_row
-            (
-                &Some(Expression::NumberLiteral(row as _, Unit::None)),
-                &Some(Expression::NumberLiteral(col as _, Unit::None)),
-            ),
-            (
-                &Some(Expression::NumberLiteral(rowspan as _, Unit::None)),
-                &Some(Expression::NumberLiteral(colspan as _, Unit::None)),
-            ),
+            (&Some(RowColExpr::Literal(row)), &Some(RowColExpr::Literal(col))),
+            (&Some(RowColExpr::Literal(rowspan)), &Some(RowColExpr::Literal(colspan))),
             layout_cache_prop_h,
             layout_cache_prop_v,
             organized_data_prop,
@@ -426,8 +439,8 @@ impl GridLayout {
         &mut self,
         item_element: &ElementRc,
         new_row: bool,
-        (row_expr, col_expr): (&Option<Expression>, &Option<Expression>),
-        (rowspan_expr, colspan_expr): (&Option<Expression>, &Option<Expression>),
+        (row_expr, col_expr): (&Option<RowColExpr>, &Option<RowColExpr>),
+        (rowspan_expr, colspan_expr): (&Option<RowColExpr>, &Option<RowColExpr>),
         layout_cache_prop_h: &NamedReference,
         layout_cache_prop_v: &NamedReference,
         organized_data_prop: &NamedReference,
@@ -463,12 +476,20 @@ impl GridLayout {
                 set_prop_from_cache(e, "row", organized_data_prop, org_index + 2, &None, diag);
             }
 
+            let expr_or_default = |expr: &Option<RowColExpr>, default: u16| -> RowColExpr {
+                match expr {
+                    Some(RowColExpr::Literal(v)) => RowColExpr::Literal(*v),
+                    Some(RowColExpr::Named(nr)) => RowColExpr::Named(nr.clone()),
+                    None => RowColExpr::Literal(default),
+                }
+            };
+
             self.elems.push(GridLayoutElement {
                 new_row,
-                col_expr: col_expr.clone(),
-                row_expr: row_expr.clone(),
-                colspan_expr: colspan_expr.clone(),
-                rowspan_expr: rowspan_expr.clone(),
+                col_expr: expr_or_default(col_expr, u16::MAX), // MAX means "auto"
+                row_expr: expr_or_default(row_expr, u16::MAX),
+                colspan_expr: expr_or_default(colspan_expr, 1),
+                rowspan_expr: expr_or_default(rowspan_expr, 1),
                 item: layout_item.item.clone(),
             });
         }
diff --git a/internal/compiler/tests/syntax/basic/layout2.slint b/internal/compiler/tests/syntax/basic/layout2.slint
@@ -30,7 +30,6 @@ export X := Rectangle {
 
                 init => {
                     self.colspan = 45;
-//                  ^error{The property must be known at compile time and cannot be changed at runtime}
                 }
             }
         }
@@ -49,25 +48,25 @@ export X := Rectangle {
     }
 
     lay2 := GridLayout {
-//          ^error{Cannot mix auto-numbering and runtime expressions for the 'col' property}
         property<int> negative: -5;
         property<int> last_row: 4;
         Text {
             row: lay2.last_row;
             col: -lay2.negative; // check that a unary minus isn't necessary an error
         }
         Text {
+//      ^error{Cannot mix auto-numbering and runtime expressions for the 'col' property}
             row: 0; // mixing runtime expr and literal is ok
         }
     }
 
     lay3 := GridLayout {
-//          ^error{Cannot mix auto-numbering and runtime expressions for the 'row' property}
         Text {
             row: lay2.last_row;
             col: lay2.last_row;
         }
         Text {
+//      ^error{Cannot mix auto-numbering and runtime expressions for the 'row' property}
             col: 1;
         }
     }
diff --git a/internal/compiler/typeregister.rs b/internal/compiler/typeregister.rs
@@ -247,7 +247,7 @@ pub fn reserved_properties() -> impl Iterator<Item = (&'static str, Type, Proper
         .chain(
             RESERVED_GRIDLAYOUT_PROPERTIES
                 .iter()
-                .map(|(k, v)| (*k, v.clone(), PropertyVisibility::Constexpr)),
+                .map(|(k, v)| (*k, v.clone(), PropertyVisibility::Input)),
         )
         .chain(IntoIterator::into_iter([
             ("absolute-position", logical_point_type().into(), PropertyVisibility::Output),
diff --git a/internal/interpreter/eval_layout.rs b/internal/interpreter/eval_layout.rs
@@ -2,12 +2,12 @@
 // SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
 
 use crate::dynamic_item_tree::InstanceRef;
-use crate::eval::{self, eval_expression, EvalLocalContext};
+use crate::eval::{self, EvalLocalContext};
 use crate::Value;
 use i_slint_compiler::expression_tree::Expression;
 use i_slint_compiler::langtype::Type;
 use i_slint_compiler::layout::{
-    GridLayout, Layout, LayoutConstraints, LayoutGeometry, Orientation,
+    GridLayout, Layout, LayoutConstraints, LayoutGeometry, Orientation, RowColExpr,
 };
 use i_slint_compiler::namedreference::NamedReference;
 use i_slint_compiler::object_tree::ElementRc;
@@ -92,7 +92,11 @@ pub(crate) fn organize_grid_layout(
     layout: &GridLayout,
     local_context: &mut EvalLocalContext,
 ) -> Value {
-    let cells = grid_layout_input_data(layout, local_context);
+    let component = local_context.component_instance;
+    let expr_eval = |nr: &NamedReference| -> f32 {
+        eval::load_property(component, &nr.element(), nr.name()).unwrap().try_into().unwrap()
+    };
+    let cells = grid_layout_input_data(layout, &expr_eval);
 
     if let Some(buttons_roles) = &layout.dialog_button_roles {
         let roles = buttons_roles
@@ -197,29 +201,30 @@ fn padding_and_spacing(
 
 fn grid_layout_input_data(
     grid_layout: &i_slint_compiler::layout::GridLayout,
-    local_context: &mut EvalLocalContext,
+    expr_eval: &impl Fn(&NamedReference) -> f32,
 ) -> Vec<core_layout::GridLayoutInputData> {
     grid_layout
         .elems
         .iter()
         .map(|cell| {
-            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 {
-                        Value::Number(n) if n >= 0.0 && n <= u16::MAX as f64 => n as u16,
-                        _ => panic!(
+            let eval_or_default = |expr: &RowColExpr| match expr {
+                RowColExpr::Literal(value) => *value,
+                RowColExpr::Named(e) => {
+                    let value = expr_eval(e);
+                    if value >= 0.0 && value <= u16::MAX as f32 {
+                        value as u16
+                    } else {
+                        panic!(
                             "Expected a positive integer, but got {:?} while evaluating {:?}",
                             value, e
-                        ),
+                        );
                     }
                 }
             };
-            let row = eval_or_default(&cell.row_expr, u16::MAX);
-            let col = eval_or_default(&cell.col_expr, u16::MAX);
-            let rowspan = eval_or_default(&cell.rowspan_expr, 1);
-            let colspan = eval_or_default(&cell.colspan_expr, 1);
+            let row = eval_or_default(&cell.row_expr);
+            let col = eval_or_default(&cell.col_expr);
+            let rowspan = eval_or_default(&cell.rowspan_expr);
+            let colspan = eval_or_default(&cell.colspan_expr);
             core_layout::GridLayoutInputData { new_row: cell.new_row, col, row, colspan, rowspan }
         })
         .collect()
diff --git a/tests/cases/layout/grid_variable_row_col.slint b/tests/cases/layout/grid_variable_row_col.slint

Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,6 @@ export X := Rectangle {`
`30`	`30`
`31`	`31`	`init => {`
`32`	`32`	`self.colspan = 45;`
`33`		`-// ^error{The property must be known at compile time and cannot be changed at runtime}`
`34`	`33`	`}`
`35`	`34`	`}`
`36`	`35`	`}`
`@@ -49,25 +48,25 @@ export X := Rectangle {`
`49`	`48`	`}`
`50`	`49`
`51`	`50`	`lay2 := GridLayout {`
`52`		`-// ^error{Cannot mix auto-numbering and runtime expressions for the 'col' property}`
`53`	`51`	`property<int> negative: -5;`
`54`	`52`	`property<int> last_row: 4;`
`55`	`53`	`Text {`
`56`	`54`	`row: lay2.last_row;`
`57`	`55`	`col: -lay2.negative; // check that a unary minus isn't necessary an error`
`58`	`56`	`}`
`59`	`57`	`Text {`
	`58`	`+// ^error{Cannot mix auto-numbering and runtime expressions for the 'col' property}`
`60`	`59`	`row: 0; // mixing runtime expr and literal is ok`
`61`	`60`	`}`
`62`	`61`	`}`
`63`	`62`
`64`	`63`	`lay3 := GridLayout {`
`65`		`-// ^error{Cannot mix auto-numbering and runtime expressions for the 'row' property}`
`66`	`64`	`Text {`
`67`	`65`	`row: lay2.last_row;`
`68`	`66`	`col: lay2.last_row;`
`69`	`67`	`}`
`70`	`68`	`Text {`
	`69`	`+// ^error{Cannot mix auto-numbering and runtime expressions for the 'row' property}`
`71`	`70`	`col: 1;`
`72`	`71`	`}`
`73`	`72`	`}`
Original file line number	Diff line number	Diff line change
`@@ -247,7 +247,7 @@ pub fn reserved_properties() -> impl Iterator<Item = (&'static str, Type, Proper`
`247`	`247`	`.chain(`
`248`	`248`	`RESERVED_GRIDLAYOUT_PROPERTIES`
`249`	`249`	`.iter()`
`250`		`- .map(\|(k, v)\| (*k, v.clone(), PropertyVisibility::Constexpr)),`
	`250`	`+ .map(\|(k, v)\| (*k, v.clone(), PropertyVisibility::Input)),`
`251`	`251`	`)`
`252`	`252`	`.chain(IntoIterator::into_iter([`
`253`	`253`	`("absolute-position", logical_point_type().into(), PropertyVisibility::Output),`