FlexboxLayout: detect binding loops when components have height-for-width children (slint-ui#11381)

Author: dfaure-kdab

internal/compiler/passes/binding_analysis.rs

@@ -556,21 +556,37 @@ fn recurse_expression(
             // Visit layout geometry dependencies
             if matches!(expr, Expression::SolveFlexboxLayout(..)) {
                 // The solve needs the main-axis dimension (width for row,
-                // height for column). For column direction, it also needs
-                // the cross-axis width for height-for-width items, but that
-                // dependency is safe (set by parent before solve) and doesn't
-                // need to be declared here.
+                // height for column). On the cross axis, *builtin* items
+                // receive the perpendicular size through the item VTable's
+                // `cross_axis_constraint` parameter and never read `self.width`
+                // at runtime, so no edge is declared for them here. *Component*
+                // items don't have that shortcut: their compiled
+                // `layoutinfo-<cross>` binding is evaluated as an ordinary
+                // property and really does depend on the cross-axis dimension
+                // (typically via inner height-for-width items). That edge is
+                // added by `visit_layout_items_layoutinfo_cross_axis_dependencies`
+                // so the binding-loop pass can detect the cycle.
                 use crate::layout::FlexboxAxisRelation;
                 match layout.axis_relation(Orientation::Horizontal) {
                     FlexboxAxisRelation::MainAxis => {
                         if let Some(nr) = layout.geometry.rect.width_reference.as_ref() {
                             vis(&nr.clone().into(), P);
                         }
+                        visit_layout_items_layoutinfo_cross_axis_dependencies(
+                            layout.elems.iter().map(|fi| &fi.item),
+                            Orientation::Vertical,
+                            vis,
+                        );
                     }
                     FlexboxAxisRelation::CrossAxis => {
                         if let Some(nr) = layout.geometry.rect.height_reference.as_ref() {
                             vis(&nr.clone().into(), P);
                         }
+                        visit_layout_items_layoutinfo_cross_axis_dependencies(
+                            layout.elems.iter().map(|fi| &fi.item),
+                            Orientation::Horizontal,
+                            vis,
+                        );
                     }
                     FlexboxAxisRelation::Unknown => {
                         // Runtime direction: conservatively depend on both
@@ -580,6 +596,16 @@ fn recurse_expression(
                         if let Some(nr) = layout.geometry.rect.height_reference.as_ref() {
                             vis(&nr.clone().into(), P);
                         }
+                        visit_layout_items_layoutinfo_cross_axis_dependencies(
+                            layout.elems.iter().map(|fi| &fi.item),
+                            Orientation::Horizontal,
+                            vis,
+                        );
+                        visit_layout_items_layoutinfo_cross_axis_dependencies(
+                            layout.elems.iter().map(|fi| &fi.item),
+                            Orientation::Vertical,
+                            vis,
+                        );
                     }
                 }
             } else if let Expression::ComputeFlexboxLayoutInfo(_, orientation) = expr {
@@ -767,6 +793,39 @@ fn visit_layout_items_dependencies<'a>(
     }
 }
 
+/// Visit cross-axis `layoutinfo-<cross>` dependencies for child elements that
+/// have a compiled `layoutinfo-<cross>` binding (i.e. an inlined component
+/// root, or a nested layout).
+///
+/// Pure builtins (`Image`, `Text`, `Rectangle`, …) do not set `layout_info_prop`
+/// — their cross-axis size is computed through the item VTable, which accepts a
+/// `cross_axis_constraint` argument, so they never read `self.width` at
+/// runtime and the parent's `SolveFlexboxLayout` has no real dependency on
+/// them. Elements that *do* set `layout_info_prop` run an ordinary property
+/// binding that may transitively depend on the cross-axis dimension (e.g. an
+/// inner word-wrapping `Text` or aspect-ratio `Image`). Declaring that edge
+/// lets `binding_analysis` detect cycles through component boundaries instead
+/// of letting them surface as a runtime recursion panic.
+fn visit_layout_items_layoutinfo_cross_axis_dependencies<'a>(
+    items: impl Iterator<Item = &'a LayoutItem>,
+    cross_axis: Orientation,
+    vis: &mut impl FnMut(&PropertyPath, ReadType),
+) {
+    for it in items {
+        let element = it.element.clone();
+        if let Some(nr) = element.borrow().layout_info_prop(cross_axis) {
+            vis(&nr.clone().into(), ReadType::PropertyRead);
+        } else if let ElementType::Component(base) = &element.borrow().base_type
+            && let Some(nr) = base.root_element.borrow().layout_info_prop(cross_axis)
+        {
+            vis(
+                &PropertyPath { elements: vec![ByAddress(element.clone())], prop: nr.clone() },
+                ReadType::PropertyRead,
+            );
+        }
+    }
+}
+
 /// The builtin function can call native code, and we need to visit the properties that are accessed by it
 fn visit_implicit_layout_info_dependencies(
     orientation: crate::layout::Orientation,

internal/compiler/tests/syntax/analysis/binding_loop_flexbox_component.slint

@@ -0,0 +1,31 @@
+// 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
+
+// A component whose cross-axis `layoutinfo-v` binding transitively reads its
+// own width (here, through a word-wrapping `Text` inside an inner layout)
+// forms a cycle when placed inside a FlexboxLayout: `SolveFlexboxLayout` needs
+// the child's `layoutinfo-v`, but the child's compiled `layoutinfo-v` binding
+// reads widths that depend back on `SolveFlexboxLayout`. Builtins avoid this
+// because they receive the cross-axis constraint through the item VTable
+// (`cross_axis_constraint`), but components don't have that shortcut — so the
+// dependency must be reported by the binding-loop pass.
+
+component Inner {
+//              >error{The binding for the property 'layoutinfo-v' is part of a binding loop (layoutinfo-v -> layout-cache -> width -> width -> layout-cache -> width -> layoutinfo-v -> layoutinfo-v)}
+    HorizontalLayout {
+//  >               <error{The binding for the property 'width' is part of a binding loop (layoutinfo-v -> layout-cache -> width -> width -> layout-cache -> width -> layoutinfo-v -> layoutinfo-v)}
+//  >               <^error{The binding for the property 'layout-cache' is part of a binding loop (layoutinfo-v -> layout-cache -> width -> width -> layout-cache -> width -> layoutinfo-v -> layoutinfo-v)}
+//  >               <^^error{The binding for the property 'width' is part of a binding loop (layoutinfo-v -> layout-cache -> width -> width -> layout-cache -> width -> layoutinfo-v -> layoutinfo-v)}
+//  >               <^^^error{The binding for the property 'layoutinfo-v' is part of a binding loop (layoutinfo-v -> layout-cache -> width -> width -> layout-cache -> width -> layoutinfo-v -> layoutinfo-v)}
+        Text { text: "hello"; wrap: word-wrap; }
+    }
+}
+//<<<error{The binding for the property 'layoutinfo-v' is part of a binding loop (layoutinfo-v -> layout-cache -> width -> width -> layout-cache -> width -> layoutinfo-v -> layoutinfo-v)}
+
+export component Demo inherits Window {
+    FlexboxLayout {
+//  >            <error{The binding for the property 'layout-cache' is part of a binding loop (layoutinfo-v -> layout-cache -> width -> width -> layout-cache -> width -> layoutinfo-v -> layoutinfo-v)}
+//  >            <^error{The binding for the property 'width' is part of a binding loop (layoutinfo-v -> layout-cache -> width -> width -> layout-cache -> width -> layoutinfo-v -> layoutinfo-v)}
+        Inner {}
+    }
+}