Rework pixel snapping, take two.

masonry/ARCHITECTURE.md

@@ -166,7 +166,7 @@ The current passes are:
 - **on_xxx_event:** Handles UX-related events, e.g. clicks, text entered, IME updates and accessibility input. Widgets can declare these events as "handled" which has a bunch of semantic implications.
 - **anim:** Do updates related to an animation frame.
 - **update:** Handles internal changes to some widgets, e.g. when the widget is marked as "disabled" or Masonry detects that a widget is hovered by a pointer.
-- **layout:** Container widgets measure their children with `LayoutCtx::compute_size` and then lay them out with `LayoutCtx::run_layout`, finally giving them a position with `LayoutCtx::place_child`.
+- **layout:** Container widgets measure their children with `LayoutCtx::compute_size` and then lay them out with `LayoutCtx::layout_child`, choosing an origin and size for each child.
 - **compose:** Computes the global transform/origin for every widget.
 - **paint** Paint every widget.
 - **accessibility:** Compute every widget's node in the accessibility tree.

masonry/examples/layers.rs

@@ -133,8 +133,7 @@ impl Widget for OverlayBox {
 
     fn layout(&mut self, ctx: &mut LayoutCtx<'_>, _props: &PropertiesRef<'_>, size: Size) {
         let child_size = ctx.compute_size(&mut self.child, SizeDef::fit(size), size.into());
-        ctx.run_layout(&mut self.child, child_size);
-        ctx.place_child(&mut self.child, Point::ORIGIN);
+        ctx.layout_child(&mut self.child, Point::ORIGIN, child_size);
     }
 
     fn paint(

masonry/src/doc/implementing_container_widget.md

@@ -72,11 +72,11 @@ Like with a leaf widget, the `measure` method must compute and return the length
 Before that, it must call [`MeasureCtx::compute_length`] for each of its own children.
 For a vertical stack, we want to sum these on the vertical axis and take the largest on the horizontal axis.
 
-Then later in `layout`, it must call [`LayoutCtx::run_layout`] then [`LayoutCtx::place_child`] for each of its own children:
+Then later in `layout`, it must call [`LayoutCtx::layout_child`] for each of its own children:
 
-- `LayoutCtx::run_layout` recursively calls `Widget::layout` on the child.
-  It takes a [`Size`] argument, which is the chosen size of the child.
-- `LayoutCtx::place_child` sets the child's position relative to the container.
+- `LayoutCtx::layout_child` recursively calls `Widget::layout` on the child.
+  It takes both a [`Point`] and [`Size`] argument, which are the chosen origin and size of the child.
+  The child's origin is in relation to the container.
 
 The `layout` method *must* iterate over all its children.
 Not doing so is a logical bug.
@@ -144,8 +144,7 @@ impl Widget for VerticalStack {
         let mut y_offset = 0.0;
         for child in &mut self.children {
             let child_size = ctx.compute_size(child, auto_size, context_size);
-            ctx.run_layout(child, child_size);
-            ctx.place_child(child, Point::new(0.0, y_offset));
+            ctx.layout_child(child, Point::new(0.0, y_offset), child_size);
 
             y_offset += child_size.height + self.gap;
         }
@@ -322,11 +321,11 @@ So for instance, if `VerticalStack::children_ids()` returns a list of three chil
 Pass methods in container widgets should only implement the logic that is specific to the container itself.
 For instance, a container widget with a background color should implement `paint` to draw the background.
 
+[`Point`]: crate::kurbo::Point
 [`Size`]: crate::kurbo::Size
 [`Widget`]: crate::core::Widget
 [`WidgetPod`]: crate::core::WidgetPod
 [`WidgetMut`]: crate::core::WidgetMut
 [`MeasureCtx::compute_length`]: crate::core::MeasureCtx::compute_length
-[`LayoutCtx::place_child`]: crate::core::LayoutCtx::place_child
-[`LayoutCtx::run_layout`]: crate::core::LayoutCtx::run_layout
+[`LayoutCtx::layout_child`]: crate::core::LayoutCtx::layout_child
 [`RegisterCtx::register_child`]: crate::core::RegisterCtx::register_child

masonry/src/doc/vertical_stack.rs

@@ -140,8 +140,7 @@ impl Widget for VerticalStack {
         let mut y_offset = 0.0;
         for child in &mut self.children {
             let child_size = ctx.compute_size(child, auto_size, context_size);
-            ctx.run_layout(child, child_size);
-            ctx.place_child(child, Point::new(0.0, y_offset));
+            ctx.layout_child(child, Point::new(0.0, y_offset), child_size);
 
             y_offset += child_size.height + self.gap;
         }

masonry/src/layers/selector_menu.rs

@@ -287,20 +287,19 @@ impl Widget for SelectorMenu {
         let mut y_offset = 0.0;
         for child in &mut self.children {
             let child_size = ctx.compute_size(child, auto_size, context_size);
-            ctx.run_layout(child, child_size);
-            ctx.place_child(child, Point::new(0.0, y_offset));
+            ctx.layout_child(child, Point::new(0.0, y_offset), child_size);
 
             y_offset += child_size.height + gap_length;
         }
 
         if !self.children.is_empty() {
             let first_child = self.children.first().unwrap();
-            let (first_baseline, _) = ctx.child_aligned_baselines(first_child);
+            let (first_baseline, _) = ctx.child_baselines(first_child);
             let first_child_origin = ctx.child_origin(first_child);
             let first_baseline = first_child_origin.y + first_baseline;
 
             let last_child = self.children.last().unwrap();
-            let (_, last_baseline) = ctx.child_aligned_baselines(last_child);
+            let (_, last_baseline) = ctx.child_baselines(last_child);
             let last_child_origin = ctx.child_origin(last_child);
             let last_baseline = last_child_origin.y + last_baseline;
 
@@ -363,7 +362,7 @@ impl Layer for SelectorMenu {
             PointerEvent::Down(PointerButtonEvent { state, .. }) => {
                 let local_pos = ctx.local_position(state.position);
 
-                !ctx.border_box_size().to_rect().contains(local_pos)
+                !ctx.border_box().contains(local_pos)
             }
             PointerEvent::Cancel(..) => true,
             _ => false,

masonry/src/layers/tooltip.rs

@@ -110,10 +110,9 @@ impl Widget for Tooltip {
 
     fn layout(&mut self, ctx: &mut LayoutCtx<'_>, _props: &PropertiesRef<'_>, size: Size) {
         let child_size = ctx.compute_size(&mut self.child, SizeDef::fit(size), size.into());
-        ctx.run_layout(&mut self.child, child_size);
 
         let child_origin = ((size - child_size).to_vec2() * 0.5).to_point();
-        ctx.place_child(&mut self.child, child_origin);
+        ctx.layout_child(&mut self.child, child_origin, child_size);
 
         ctx.derive_baselines(&self.child);
     }

masonry/src/tests/action.rs

@@ -66,8 +66,7 @@ fn action_source_removed() {
         })
         .layout_fn(move |child, ctx, _props, size| {
             if let Some(child) = child {
-                ctx.run_layout(child, size);
-                ctx.place_child(child, Point::ZERO);
+                ctx.layout_child(child, Point::ZERO, size);
             }
         })
         .children_fn(|child| {

masonry/src/tests/compose.rs

@@ -3,10 +3,11 @@
 
 use assert_matches::assert_matches;
 
-use crate::core::{ChildrenIds, NewWidget, Widget, WidgetPod, WidgetTag};
-use crate::kurbo::{Affine, Point, Vec2};
-use crate::layout::{Length, SizeDef};
+use crate::core::{ChildrenIds, NewWidget, Update, Widget, WidgetPod, WidgetTag};
+use crate::kurbo::{Affine, Point, Rect, Size, Vec2};
+use crate::layout::{AsUnit, Length, SizeDef};
 use crate::testing::{ModularWidget, Record, TestHarness, TestWidgetExt};
+use crate::tests::{assert_point_approx_eq, assert_rect_approx_eq};
 use crate::theme::test_property_set;
 use crate::widgets::SizedBox;
 
@@ -32,8 +33,7 @@ fn request_compose() {
         .measure_fn(|_state, _ctx, _props, _axis, _len_req, _cross_length| Length::ZERO)
         .layout_fn(|state, ctx, _props, size| {
             let child_size = ctx.compute_size(&mut state.child, SizeDef::fit(size), size.into());
-            ctx.run_layout(&mut state.child, child_size);
-            ctx.place_child(&mut state.child, state.pos);
+            ctx.layout_child(&mut state.child, state.pos, child_size);
         })
         .compose_fn(|state, ctx| {
             ctx.set_child_scroll_translation(&mut state.child, state.offset);
@@ -80,9 +80,108 @@ fn request_compose() {
     );
 }
 
+#[test]
+fn scroll_translation_updates_composed_geometry_without_layout() {
+    struct ChildAndOffset {
+        child: WidgetPod<dyn Widget>,
+        offset: Vec2,
+    }
+
+    let child_tag = WidgetTag::unique();
+    let parent_tag = WidgetTag::unique();
+    let child = NewWidget::new(
+        ModularWidget::new(())
+            .measure_fn(|_, _, _, _, _, _| 10.3.px())
+            .record(),
+    )
+    .with_tag(child_tag);
+
+    let parent = ModularWidget::new(ChildAndOffset {
+        child: child.erased().to_pod(),
+        offset: Vec2::ZERO,
+    })
+    .layout_fn(|state, ctx, _, size| {
+        let child_size = ctx.compute_size(&mut state.child, SizeDef::fit(size), size.into());
+        ctx.layout_child(&mut state.child, Point::new(5.1, 5.3), child_size);
+    })
+    .compose_fn(|state, ctx| {
+        ctx.set_child_scroll_translation(&mut state.child, state.offset);
+    })
+    .register_children_fn(|state, ctx| {
+        ctx.register_child(&mut state.child);
+    })
+    .children_fn(|state| ChildrenIds::from_slice(&[state.child.id()]))
+    .prepare()
+    .with_tag(parent_tag);
+
+    let mut harness = TestHarness::create(test_property_set(), parent);
+    harness.flush_records_of(child_tag);
+
+    let hit_after_scroll = Point::new(16., 16.);
+    harness.mouse_move(hit_after_scroll);
+    let records = harness.take_records_of(child_tag);
+    assert!(
+        !records.iter().any(|record| matches!(
+            record,
+            Record::PointerEvent(_) | Record::Update(Update::HoveredChanged(true))
+        )),
+        "pointer should not reach the child before scroll translation"
+    );
+    assert!(!harness.get_widget(child_tag).ctx().is_hovered());
+
+    harness.edit_widget(parent_tag, |mut parent| {
+        parent.widget.state.offset = Vec2::new(2.2, 0.8);
+        parent.ctx.request_compose();
+    });
+
+    let records = harness.take_records_of(child_tag);
+    assert!(
+        !records
+            .iter()
+            .any(|record| matches!(record, Record::Layout(_))),
+        "scroll translation should not rerun child layout"
+    );
+    assert!(
+        records.iter().any(|record| matches!(
+            record,
+            Record::PointerEvent(_) | Record::Update(Update::HoveredChanged(true))
+        )),
+        "stationary pointer should reach the child after scroll translation"
+    );
+    assert!(harness.get_widget(child_tag).ctx().is_hovered());
+
+    let child = harness.get_widget(child_tag);
+    let child_id = child.id();
+    let ctx = child.ctx();
+    assert_eq!(ctx.border_box().size(), Size::new(10., 11.));
+    assert_rect_approx_eq(
+        "window border box",
+        ctx.window_transform().transform_rect_bbox(ctx.border_box()),
+        Rect::new(7., 6., 17., 17.),
+    );
+
+    let _ = harness.redraw();
+    let access_bounds = harness
+        .access_node(child_id)
+        .unwrap()
+        .bounding_box()
+        .unwrap();
+    assert_rect_approx_eq(
+        "access bounds",
+        Rect::new(
+            access_bounds.x0,
+            access_bounds.y0,
+            access_bounds.x1,
+            access_bounds.y1,
+        ),
+        Rect::new(7., 6., 17., 17.),
+    );
+}
+
 #[test]
 fn scroll_pixel_snap() {
-    let child_tag = WidgetTag::named("child");
+    let parent_tag = WidgetTag::unique();
+    let child_tag = WidgetTag::unique();
     let child = NewWidget::new(SizedBox::empty()).with_tag(child_tag);
 
     let parent = ModularWidget::new_parent(child)
@@ -91,13 +190,23 @@ fn scroll_pixel_snap() {
 
             ctx.set_child_scroll_translation(state, offset);
         })
-        .prepare();
+        .prepare()
+        .with_tag(parent_tag);
 
-    let harness = TestHarness::create(test_property_set(), parent);
+    let mut harness = TestHarness::create(test_property_set(), parent);
 
     // Origin should be rounded to (0., 1.) by pixel-snapping.
     let child = harness.get_widget(child_tag);
     let ctx = child.ctx();
     let origin = ctx.to_window(ctx.border_box().origin());
     assert_eq!(origin, Point::new(0., 1.));
+
+    harness.edit_widget(parent_tag, |mut parent| {
+        parent.set_snap_disabled(true);
+    });
+
+    let child = harness.get_widget(child_tag);
+    let ctx = child.ctx();
+    let origin = ctx.to_window(ctx.border_box().origin());
+    assert_point_approx_eq("origin", origin, Point::new(0.1, 0.9));
 }