Migrate layout pass

masonry/examples/calc_masonry.rs

@@ -217,7 +217,7 @@ impl Widget for CalcButton {
     }
 
     fn layout(&mut self, ctx: &mut LayoutCtx, bc: &BoxConstraints) -> Size {
-        let size = self.inner.layout(ctx, bc);
+        let size = ctx.run_layout(&mut self.inner, bc);
         ctx.place_child(&mut self.inner, Point::ORIGIN);
 
         size

masonry/src/contexts.rs

@@ -11,12 +11,15 @@ use tracing::{trace, warn};
 use vello::kurbo::Vec2;
 
 use crate::action::Action;
+use crate::passes::layout::run_layout_on;
 use crate::render_root::{MutateCallback, RenderRootSignal, RenderRootState};
 use crate::text::TextBrush;
 use crate::text_helpers::{ImeChangeSignal, TextFieldRegistration};
 use crate::tree_arena::ArenaMutChildren;
 use crate::widget::{WidgetMut, WidgetState};
-use crate::{AllowRawMut, CursorIcon, Insets, Point, Rect, Size, Widget, WidgetId, WidgetPod};
+use crate::{
+    AllowRawMut, BoxConstraints, CursorIcon, Insets, Point, Rect, Size, Widget, WidgetId, WidgetPod,
+};
 
 /// A macro for implementing methods on multiple contexts.
 ///
@@ -856,6 +859,16 @@ impl LayoutCtx<'_> {
         self.get_child_state(child).layout_rect().size()
     }
 
+    /// Compute layout of a child widget.
+    ///
+    /// Container widgets must call this on every child as part of
+    /// their [`layout`] method.
+    ///
+    /// [`layout`]: Widget::layout
+    pub fn run_layout<W: Widget>(&mut self, child: &mut WidgetPod<W>, bc: &BoxConstraints) -> Size {
+        run_layout_on(self, child, bc)
+    }
+
     /// Set the position of a child widget, in the parent's coordinate space. This
     /// will also implicitly change "hot" status and affect the parent's display rect.
     ///

masonry/src/passes/layout.rs

@@ -0,0 +1,192 @@
+// Copyright 2024 the Xilem Authors
+// SPDX-License-Identifier: Apache-2.0
+
+use tracing::{info_span, trace, warn};
+use vello::kurbo::{Point, Rect, Size};
+
+use crate::render_root::RenderRoot;
+use crate::widget::WidgetState;
+use crate::{BoxConstraints, LayoutCtx, Widget, WidgetPod};
+
+// TODO - negative rects?
+/// Return `true` if all of `smaller` is within `larger`.
+fn rect_contains(larger: &Rect, smaller: &Rect) -> bool {
+    smaller.x0 >= larger.x0
+        && smaller.x1 <= larger.x1
+        && smaller.y0 >= larger.y0
+        && smaller.y1 <= larger.y1
+}
+
+pub(crate) fn run_layout_inner<W: Widget>(
+    parent_ctx: &mut LayoutCtx<'_>,
+    pod: &mut WidgetPod<W>,
+    bc: &BoxConstraints,
+) -> bool {
+    let id = pod.id().to_raw();
+    let widget_mut = parent_ctx
+        .widget_children
+        .get_child_mut(id)
+        .expect("WidgetPod: inner widget not found in widget tree");
+    let mut state_mut = parent_ctx
+        .widget_state_children
+        .get_child_mut(id)
+        .expect("WidgetPod: inner widget not found in widget tree");
+    let widget = widget_mut.item;
+    let state = state_mut.item;
+
+    if state.is_stashed {
+        debug_panic!(
+            "Error in '{}' #{}: trying to compute layout of stashed widget.",
+            widget.short_type_name(),
+            id,
+        );
+        state.size = Size::ZERO;
+        return false;
+    }
+
+    state.needs_compose = true;
+    state.is_expecting_place_child_call = true;
+    // TODO - Not everything that has been re-laid out needs to be repainted.
+    state.needs_paint = true;
+    state.request_accessibility_update = true;
+    state.needs_accessibility_update = true;
+
+    bc.debug_check(widget.short_type_name());
+    trace!("Computing layout with constraints {:?}", bc);
+
+    state.local_paint_rect = Rect::ZERO;
+
+    let new_size = {
+        let mut inner_ctx = LayoutCtx {
+            widget_state: state,
+            widget_state_children: state_mut.children.reborrow_mut(),
+            widget_children: widget_mut.children,
+            global_state: parent_ctx.global_state,
+            mouse_pos: parent_ctx.mouse_pos,
+        };
+
+        widget.layout(&mut inner_ctx, bc)
+    };
+
+    // TODO - One we add request_layout, check for that flag.
+    // If it's true, that's probably an error.
+    state.needs_layout = false;
+
+    state.local_paint_rect = state
+        .local_paint_rect
+        .union(new_size.to_rect() + state.paint_insets);
+
+    #[cfg(debug_assertions)]
+    {
+        for child_id in widget.children_ids() {
+            let child_id = child_id.to_raw();
+            let child_state_mut = state_mut
+                .children
+                .get_child_mut(child_id)
+                .unwrap_or_else(|| panic!("widget #{child_id} not found"));
+            let child_state = child_state_mut.item;
+            if child_state.is_expecting_place_child_call {
+                debug_panic!(
+                        "Error in '{}' #{}: missing call to place_child method for child widget '{}' #{}. During layout pass, if a widget calls WidgetPod::layout() on its child, it then needs to call LayoutCtx::place_child() on the same child.",
+                        widget.short_type_name(),
+                        id,
+                        child_state.widget_name,
+                        child_state.id.to_raw(),
+                    );
+            }
+
+            // TODO - This check might be redundant with the code updating local_paint_rect
+            let child_rect = child_state.paint_rect();
+            if !rect_contains(&state.local_paint_rect, &child_rect) && !state.is_portal {
+                debug_panic!(
+                        "Error in '{}' #{}: paint_rect {:?} doesn't contain paint_rect {:?} of child widget '{}' #{}",
+                        widget.short_type_name(),
+                        id,
+                        state.local_paint_rect,
+                        child_rect,
+                        child_state.widget_name,
+                        child_state.id.to_raw(),
+                    );
+            }
+        }
+    }
+
+    // TODO - Figure out how to deal with the overflow problem, eg:
+    // What happens if a widget returns a size larger than the allowed constraints?
+    // Some possibilities are:
+    // - Always clip: might be expensive
+    // - Display it anyway: might lead to graphical bugs
+    // - Panic: too harsh?
+    // Also, we need to avoid spurious crashes when we initialize the app and the
+    // size is (0,0)
+    // See https://github.com/linebender/xilem/issues/377
+
+    let state_mut = parent_ctx
+        .widget_state_children
+        .get_child_mut(id)
+        .expect("WidgetPod: inner widget not found in widget tree");
+    parent_ctx.widget_state.merge_up(state_mut.item);
+    state_mut.item.size = new_size;
+
+    log_layout_issues(widget.short_type_name(), new_size);
+
+    true
+}
+
+fn log_layout_issues(type_name: &str, size: Size) {
+    if size.width.is_infinite() {
+        warn!("Widget `{type_name}` has an infinite width.");
+    }
+    if size.height.is_infinite() {
+        warn!("Widget `{type_name}` has an infinite height.");
+    }
+}
+
+pub(crate) fn run_layout_on<W: Widget>(
+    parent_ctx: &mut LayoutCtx<'_>,
+    pod: &mut WidgetPod<W>,
+    bc: &BoxConstraints,
+) -> Size {
+    pod.call_widget_method_with_checks(
+        "layout",
+        parent_ctx,
+        |ctx| {
+            (
+                ctx.widget_state_children.reborrow(),
+                ctx.widget_children.reborrow(),
+            )
+        },
+        |child, ctx| run_layout_inner(ctx, child, bc),
+    );
+
+    let id = pod.id().to_raw();
+    let state_mut = parent_ctx
+        .widget_state_children
+        .get_child_mut(id)
+        .expect("run_layout_on: inner widget not found in widget tree");
+    state_mut.item.size
+}
+
+pub fn root_layout(
+    root: &mut RenderRoot,
+    synthetic_root_state: &mut WidgetState,
+    bc: &BoxConstraints,
+) -> Size {
+    let _span = info_span!("layout").entered();
+
+    let mouse_pos = root.last_mouse_pos.map(|pos| (pos.x, pos.y).into());
+    let root_state_token = root.widget_arena.widget_states.root_token_mut();
+    let root_widget_token = root.widget_arena.widgets.root_token_mut();
+    let mut ctx = LayoutCtx {
+        global_state: &mut root.state,
+        widget_state: synthetic_root_state,
+        widget_state_children: root_state_token,
+        widget_children: root_widget_token,
+        mouse_pos,
+    };
+
+    let size = run_layout_on(&mut ctx, &mut root.root, bc);
+    ctx.place_child(&mut root.root, Point::ORIGIN);
+
+    size
+}

masonry/src/passes/mod.rs

@@ -6,6 +6,7 @@ use crate::WidgetId;
 
 pub mod compose;
 pub mod event;
+pub mod layout;
 pub mod mutate;
 pub mod update;
 

masonry/src/render_root.rs

@@ -7,7 +7,7 @@ use accesskit::{ActionRequest, NodeBuilder, Tree, TreeUpdate};
 use parley::fontique::{self, Collection, CollectionOptions};
 use parley::{FontContext, LayoutContext};
 use tracing::{debug, info_span, warn};
-use vello::kurbo::{self, Affine, Point};
+use vello::kurbo::{self, Affine};
 use vello::peniko::{Color, Fill};
 use vello::Scene;
 
@@ -16,12 +16,13 @@ use std::time::Instant;
 #[cfg(target_arch = "wasm32")]
 use web_time::Instant;
 
-use crate::contexts::{LayoutCtx, LifeCycleCtx, PaintCtx};
+use crate::contexts::{LifeCycleCtx, PaintCtx};
 use crate::debug_logger::DebugLogger;
 use crate::dpi::{LogicalPosition, LogicalSize, PhysicalSize};
 use crate::event::{PointerEvent, TextEvent, WindowEvent};
 use crate::passes::compose::root_compose;
 use crate::passes::event::{root_on_access_event, root_on_pointer_event, root_on_text_event};
+use crate::passes::layout::root_layout;
 use crate::passes::mutate::{mutate_widget, run_mutate_pass};
 use crate::passes::update::run_update_pointer_pass;
 use crate::text::TextBrush;
@@ -458,41 +459,20 @@ impl RenderRoot {
 
     // --- MARK: LAYOUT ---
     pub(crate) fn root_layout(&mut self) {
-        let mut dummy_state = WidgetState::synthetic(self.root.id(), self.get_kurbo_size());
-        let size = self.get_kurbo_size();
-        let mouse_pos = self.last_mouse_pos.map(|pos| (pos.x, pos.y).into());
-        let root_state_token = self.widget_arena.widget_states.root_token_mut();
-        let root_widget_token = self.widget_arena.widgets.root_token_mut();
-        let mut layout_ctx = LayoutCtx {
-            global_state: &mut self.state,
-            widget_state: &mut dummy_state,
-            widget_state_children: root_state_token,
-            widget_children: root_widget_token,
-            mouse_pos,
-        };
-
+        let window_size = self.get_kurbo_size();
         let bc = match self.size_policy {
-            WindowSizePolicy::User => BoxConstraints::tight(size),
+            WindowSizePolicy::User => BoxConstraints::tight(window_size),
             WindowSizePolicy::Content => BoxConstraints::UNBOUNDED,
         };
 
-        let size = {
-            layout_ctx
-                .global_state
-                .debug_logger
-                .push_important_span("LAYOUT");
-            let _span = info_span!("layout").entered();
-            self.root.layout(&mut layout_ctx, &bc)
-        };
-        layout_ctx.place_child(&mut self.root, Point::ORIGIN);
-        layout_ctx.global_state.debug_logger.pop_span();
+        let mut dummy_state = WidgetState::synthetic(self.root.id(), self.get_kurbo_size());
+        let size = root_layout(self, &mut dummy_state, &bc);
 
         if let WindowSizePolicy::Content = self.size_policy {
             let new_size = LogicalSize::new(size.width, size.height).to_physical(self.scale_factor);
             if self.size != new_size {
                 self.size = new_size;
-                layout_ctx
-                    .global_state
+                self.state
                     .signal_queue
                     .push_back(RenderRootSignal::SetSize(new_size));
             }

masonry/src/widget/align.rs

@@ -98,7 +98,7 @@ impl Widget for Align {
     fn on_status_change(&mut self, _ctx: &mut LifeCycleCtx, _event: &StatusChange) {}
 
     fn layout(&mut self, ctx: &mut LayoutCtx, bc: &BoxConstraints) -> Size {
-        let size = self.child.layout(ctx, &bc.loosen());
+        let size = ctx.run_layout(&mut self.child, &bc.loosen());
 
         log_size_warnings(size);
 

masonry/src/widget/button.rs

@@ -129,7 +129,7 @@ impl Widget for Button {
         let padding = Size::new(LABEL_INSETS.x_value(), LABEL_INSETS.y_value());
         let label_bc = bc.shrink(padding).loosen();
 
-        let label_size = self.label.layout(ctx, &label_bc);
+        let label_size = ctx.run_layout(&mut self.label, &label_bc);
 
         let baseline = ctx.child_baseline_offset(&self.label);
         ctx.set_baseline_offset(baseline + LABEL_INSETS.y1);

masonry/src/widget/checkbox.rs

@@ -112,7 +112,7 @@ impl Widget for Checkbox {
         let x_padding = theme::WIDGET_CONTROL_COMPONENT_PADDING;
         let check_size = theme::BASIC_WIDGET_HEIGHT;
 
-        let label_size = self.label.layout(ctx, bc);
+        let label_size = ctx.run_layout(&mut self.label, bc);
         ctx.place_child(&mut self.label, (check_size + x_padding, 0.0).into());
 
         let desired_size = Size::new(

masonry/src/widget/flex.rs

@@ -672,7 +672,7 @@ impl Widget for Flex {
 
                         let old_size = ctx.widget_state.layout_rect().size();
                         let child_bc = self.direction.constraints(&loosened_bc, 0.0, 9000.0); // TODO Infinity does lead to NaN and then crashes the app, if this is too big, it leads to floating point issues
-                        let child_size = widget.layout(ctx, &child_bc);
+                        let child_size = ctx.run_layout(widget, &child_bc);
 
                         if child_size.width.is_infinite() {
                             tracing::warn!("A non-Flex child has an infinite width.");
@@ -738,7 +738,7 @@ impl Widget for Flex {
 
                         let old_size = ctx.widget_state.layout_rect().size();
                         let child_bc = self.direction.constraints(&loosened_bc, 0.0, actual_major);
-                        let child_size = widget.layout(ctx, &child_bc);
+                        let child_size = ctx.run_layout(widget, &child_bc);
 
                         if old_size != child_size {
                             any_changed = true;
@@ -819,7 +819,7 @@ impl Widget for Flex {
                                 //TODO: this is the second call of layout on the same child, which
                                 // is bad, because it can lead to exponential increase in layout calls
                                 // when used multiple times in the widget hierarchy.
-                                widget.layout(ctx, &child_bc);
+                                ctx.run_layout(widget, &child_bc);
                             }
                             0.0
                         }

masonry/src/widget/portal.rs

@@ -330,7 +330,7 @@ impl<W: Widget> Widget for Portal<W> {
 
         let child_bc = BoxConstraints::new(min_child_size, max_child_size);
 
-        let content_size = self.child.layout(ctx, &child_bc);
+        let content_size = ctx.run_layout(&mut self.child, &child_bc);
         let portal_size = bc.constrain(content_size);
 
         // TODO - document better
@@ -352,7 +352,7 @@ impl<W: Widget> Widget for Portal<W> {
             // TODO - request paint for scrollbar?
             std::mem::drop(scrollbar);
 
-            let scrollbar_size = self.scrollbar_horizontal.layout(ctx, bc);
+            let scrollbar_size = ctx.run_layout(&mut self.scrollbar_horizontal, bc);
             ctx.place_child(
                 &mut self.scrollbar_horizontal,
                 Point::new(0.0, portal_size.height - scrollbar_size.height),
@@ -367,7 +367,7 @@ impl<W: Widget> Widget for Portal<W> {
             // TODO - request paint for scrollbar?
             std::mem::drop(scrollbar);
 
-            let scrollbar_size = self.scrollbar_vertical.layout(ctx, bc);
+            let scrollbar_size = ctx.run_layout(&mut self.scrollbar_vertical, bc);
             ctx.place_child(
                 &mut self.scrollbar_vertical,
                 Point::new(portal_size.width - scrollbar_size.width, 0.0),