Migrate layout pass (#529)

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.
 ///
@@ -738,6 +741,19 @@ impl LayoutCtx<'_> {
         }
     }
 
+    // TODO - Reorder methods so that methods necessary for layout
+    // appear higher in documentation.
+
+    /// 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 explicit paint [`Insets`] for this widget.
     ///
     /// You are not required to set explicit paint bounds unless you need

masonry/src/passes/layout.rs

@@ -0,0 +1,217 @@
+// Copyright 2024 the Xilem Authors
+// SPDX-License-Identifier: Apache-2.0
+
+//! The layout pass, which provides the size and position of each widget
+//! before any translations applied in [`compose`](crate::passes::compose).
+//! Most of the logic for this pass happens in [`Widget::layout`] implementations.
+
+use tracing::{info_span, trace};
+use vello::kurbo::{Point, Rect, Size};
+
+use crate::render_root::RenderRoot;
+use crate::widget::WidgetState;
+use crate::{BoxConstraints, LayoutCtx, Widget, WidgetPod};
+
+// TODO - Replace with contains_rect once new Kurbo version is released.
+// See https://github.com/linebender/kurbo/pull/347
+/// 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
+}
+
+// Returns "true" if the Widget's layout method was called, in which case debug checks
+// need to be run. (See 'called_widget' in WidgetPod::call_widget_method_with_checks)
+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;
+    }
+
+    // TODO - Not everything that has been re-laid out needs to be repainted.
+    state.needs_paint = true;
+    state.needs_compose = true;
+    state.needs_accessibility = true;
+    state.request_paint = true;
+    state.request_compose = true;
+    state.request_accessibility = 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,
+        };
+
+        // TODO - If constraints are the same and request_layout isn't set,
+        // skip calling layout
+        inner_ctx.widget_state.request_layout = false;
+        widget.layout(&mut inner_ctx, bc)
+    };
+    if state.request_layout {
+        debug_panic!(
+            "Error in '{}' #{}: layout request flag was set during layout pass",
+            widget.short_type_name(),
+            id,
+        );
+    }
+
+    state.needs_layout = false;
+    state.is_expecting_place_child_call = true;
+
+    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;
+
+    {
+        if new_size.width.is_infinite() {
+            debug_panic!(
+                "Error in '{}' #{}: width is infinite",
+                widget.short_type_name(),
+                id,
+            );
+        }
+        if new_size.height.is_infinite() {
+            debug_panic!(
+                "Error in '{}' #{}: height is infinite",
+                widget.short_type_name(),
+                id,
+            );
+        }
+    }
+
+    true
+}
+
+/// Run [`Widget::layout`] method on the widget contained in `pod`.
+/// This will be called by [`LayoutCtx::run_layout`], which is itself called in the parent widget's `layout`.
+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(crate) 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

@@ -8,6 +8,7 @@ use crate::{Widget, WidgetId, WidgetState};
 pub mod accessibility;
 pub mod compose;
 pub mod event;
+pub mod layout;
 pub mod mutate;
 pub mod paint;
 pub mod update;

masonry/src/render_root.rs

@@ -7,21 +7,22 @@ use accesskit::{ActionRequest, Tree, TreeUpdate};
 use parley::fontique::{self, Collection, CollectionOptions};
 use parley::{FontContext, LayoutContext};
 use tracing::{info_span, warn};
-use vello::kurbo::{self, Point, Rect};
+use vello::kurbo::{self, Rect};
 use vello::Scene;
 
 #[cfg(not(target_arch = "wasm32"))]
 use std::time::Instant;
 #[cfg(target_arch = "wasm32")]
 use web_time::Instant;
 
-use crate::contexts::{LayoutCtx, LifeCycleCtx};
+use crate::contexts::LifeCycleCtx;
 use crate::debug_logger::DebugLogger;
 use crate::dpi::{LogicalPosition, LogicalSize, PhysicalSize};
 use crate::event::{PointerEvent, TextEvent, WindowEvent};
 use crate::passes::accessibility::root_accessibility;
 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::paint::root_paint;
 use crate::passes::update::{
@@ -466,41 +467,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/testing/helper_widgets.rs

@@ -331,7 +331,7 @@ impl Widget for ReplaceChild {
     }
 
     fn layout(&mut self, ctx: &mut LayoutCtx, bc: &BoxConstraints) -> Size {
-        self.child.layout(ctx, bc)
+        ctx.run_layout(&mut self.child, bc)
     }
 
     fn compose(&mut self, _ctx: &mut ComposeCtx) {}

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

@@ -115,7 +115,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(