Return the size from draw, and fold placing back into drawing

Review response.

`Painter::set_size` is gone: `Widget::draw` returns the `Size` instead, so
a widget that does not say what it used cannot compile rather than
panicking at the widget that forgot. That also settles setting it twice --
a branch that learns something late just returns a different value.

`Painter::place` and `UiRenderState::place` are gone too. `draw_inner`
already tried to reuse an active widget's drawing before redrawing it, so
`place` was `widget_within` with its own bookkeeping bolted on; drawing a
child a second time now *is* how a parent puts it where it belongs, and a
child is deduplicated in `children` because listing one twice would move
it twice. The unification also drops `place`'s use of `ActiveData::layer`,
which is the layer a widget's own `child_layer()` left the painter on
rather than the layer it was drawn into.

What `place` did unconditionally and `widget_within` did not is record the
size dependency, so `Painter::size_hint` now records one: reading a
child's length to lay out around it is reading its size, whether it came
from a draw or from a hint. `tests/retained.rs` has a parent that only
ever reads the hint, which is the case no existing widget exercises.

`()` sizes itself `Size::default()` -- rest -- rather than zero, so it is
a gap that takes an even share of a span; `WidgetPtr` with nothing in it
does the same, since it is the same situation. `Widget::on_resize`'s
default body said `Translate` while the enum's `#[default]` said `Redraw`;
it now defers to the enum.

`was` is `old` throughout, the `OnResize` variant comments are gone, and
so are two empty `impl` blocks.

Checked: fmt, clippy and 27 tests across the workspace; `tabs` on each of
its five tabs, and `tabs` with a replay that adds two images, all
byte-identical to `upstream/main`; `view` and `minimal` likewise; and the
`text` example rendered at 1920x1200 and 900x1200 to see the paragraph
reflow and its container follow.

core/src/ui/painter.rs

@@ -20,7 +20,6 @@ pub struct Painter<'a> {
     pub(super) children: Vec<WidgetId>,
     /// The children whose size this widget read while drawing.
     pub(super) size_deps: Vec<WidgetId>,
-    pub(super) size: Option<Size>,
     pub layer: usize,
     pub(super) id: WidgetId,
 }
@@ -70,24 +69,13 @@ impl<'a> Painter<'a> {
         self.mask = self.rsc.ui_mut().masks.push(Mask { region });
     }
 
-    /// States what this widget uses of the box it was handed, once per draw.
-    /// This is the size a parent reads.
-    pub fn set_size(&mut self, size: impl Into<Size>) {
-        debug_assert!(
-            self.size.is_none(),
-            "{} set its size twice in one draw",
-            self.label(),
-        );
-        self.size = Some(size.into());
-    }
-
     /// Draws a widget within this widget's region.
     pub fn widget<W: ?Sized>(&mut self, id: &StrongWidget<W>) -> DrawResult<'_, 'a> {
         self.widget_at(id, self.region)
     }
 
-    /// Draws a widget somewhere within this one.
-    /// Useful for drawing child widgets in select areas.
+    /// Draws a widget somewhere within this one. Drawing one a second time
+    /// gives it a new box, keeping the drawing it already has where it can.
     pub fn widget_within<W: ?Sized>(
         &mut self,
         id: &StrongWidget<W>,
@@ -102,10 +90,14 @@ impl<'a> Painter<'a> {
         id: &StrongWidget<W>,
         region: UiRegion,
     ) -> DrawResult<'_, 'a> {
-        self.children.push(id.id());
+        let child = id.id();
+        // A child listed twice would be moved twice.
+        if !self.children.contains(&child) {
+            self.children.push(child);
+        }
         let size = self.state.draw_inner(
             self.layer,
-            id.id(),
+            child,
             region,
             Some(self.id),
             self.mask,
@@ -113,43 +105,24 @@ impl<'a> Painter<'a> {
             self.rsc,
         );
         DrawResult {
-            child: id.id(),
+            child,
             painter: self,
             size,
         }
     }
 
     /// What a child says its length is without being drawn, if it can say.
-    pub fn size_hint<W: ?Sized>(&self, id: &StrongWidget<W>, axis: Axis) -> Option<Len> {
-        self.rsc.widgets().get_dyn(id.id())?.size_hint(axis)
+    /// Asking counts as reading its size.
+    pub fn size_hint<W: ?Sized>(&mut self, id: &StrongWidget<W>, axis: Axis) -> Option<Len> {
+        let hint = self.rsc.widgets().get_dyn(id.id())?.size_hint(axis)?;
+        self.depend_on_size(id.id());
+        Some(hint)
     }
 
-    /// Gives a child its final box once this widget knows what that is,
-    /// keeping the drawing it already has where it can.
-    pub fn place<W: ?Sized>(&mut self, id: &StrongWidget<W>, region: UiRegion) -> Size {
-        let region = region.within(&self.region);
-        let id = id.id();
-        // Choosing a child's box depends on its size, from a draw or a hint.
-        // Claiming it is separate: one left out of `children` is removed.
-        if !self.size_deps.contains(&id) {
-            self.size_deps.push(id);
+    fn depend_on_size(&mut self, child: WidgetId) {
+        if !self.size_deps.contains(&child) {
+            self.size_deps.push(child);
         }
-        if !self.children.contains(&id) {
-            self.children.push(id);
-        }
-        if self.state.active.contains_key(&id) {
-            return self.state.place(id, region, self.rsc);
-        }
-        // Not drawn yet: its length came from a hint, so this is its one draw.
-        self.state.draw_inner(
-            self.layer,
-            id,
-            region,
-            Some(self.id),
-            self.mask,
-            None,
-            self.rsc,
-        )
     }
 
     pub fn render_text(
@@ -230,7 +203,7 @@ pub struct DrawResult<'p, 'a> {
 
 impl DrawResult<'_, '_> {
     pub fn size(self) -> Size {
-        self.painter.size_deps.push(self.child);
+        self.painter.depend_on_size(self.child);
         self.size
     }
 

core/src/ui/render_state.rs

@@ -106,12 +106,11 @@ impl UiRenderState {
             primitives: Vec::new(),
             children: Vec::new(),
             size_deps: Vec::new(),
-            size: None,
             rsc,
         };
 
         let mut widget = painter.rsc.widgets().get_dyn_dynamic(id);
-        widget.draw(&mut painter);
+        let size = widget.draw(&mut painter);
         drop(widget);
 
         let Painter {
@@ -123,17 +122,10 @@ impl UiRenderState {
             primitives,
             children,
             size_deps,
-            size,
             layer,
             id,
         } = painter;
 
-        let size = size.unwrap_or_else(|| {
-            panic!(
-                "'{}' ({id:?}) drew without a size; every widget calls Painter::set_size",
-                rsc.widgets().label(id)
-            )
-        });
         debug_assert!(
             Self::hints_agree(id, size, rsc),
             "'{}' ({id:?}) drew a size its size_hint disagrees with",
@@ -166,26 +158,6 @@ impl UiRenderState {
         size
     }
 
-    /// Gives an already drawn widget a new box, keeping its drawing if it can
-    /// and drawing it again if it cannot.
-    pub(super) fn place(&mut self, id: WidgetId, region: UiRegion, rsc: &mut dyn UiRsc) -> Size {
-        if let Some(size) = self.try_reuse(id, region, rsc) {
-            return size;
-        }
-        let Some(active) = self.remove(id, false, rsc) else {
-            return Size::ZERO;
-        };
-        self.draw_inner(
-            active.layer,
-            id,
-            region,
-            active.parent,
-            active.mask,
-            Some(active.children),
-            rsc,
-        )
-    }
-
     /// The drawing a widget already has, kept for a new box if the box has not
     /// changed in a way it depends on.
     fn try_reuse(&mut self, id: WidgetId, region: UiRegion, rsc: &dyn UiRsc) -> Option<Size> {
@@ -193,18 +165,18 @@ impl UiRenderState {
             return None;
         }
         let active = self.active.get(&id)?;
-        let (size, was) = (active.size, active.region);
-        if was == region {
+        let (size, old) = (active.size, active.region);
+        if old == region {
             return Some(size);
         }
         // TODO: epsilon?
-        if was.size() == region.size() {
-            self.mov(id, was, region);
+        if old.size() == region.size() {
+            self.mov(id, old, region);
             return Some(size);
         }
         if self.reusable(id, region, rsc) {
             // Its drawing stands; the new box is remapped into the primitives.
-            self.mov(id, was, region);
+            self.mov(id, old, region);
             return Some(size);
         }
         None
@@ -220,10 +192,10 @@ impl UiRenderState {
         let Some(widget) = rsc.widgets().get_dyn(id) else {
             return false;
         };
-        let mut was = active.region;
+        let mut old = active.region;
         [Axis::X, Axis::Y].into_iter().all(|axis| {
             let offered = region.axis_mut(axis).len();
-            let had = was.axis_mut(axis).len();
+            let had = old.axis_mut(axis).len();
             match widget.on_resize(axis) {
                 OnResize::Scale => true,
                 // `Translate` is not acted on yet, and cannot be until a

core/src/widget/mod.rs

@@ -20,25 +20,15 @@ pub use widgets::*;
 /// text reads the width it is offered and not the height.
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
 pub enum OnResize {
-    /// Stretched to the new box, which is all its drawing ever was.
     Scale,
-    /// Carried to the new box at the size it drew, which it keeps.
     Translate,
-    /// Nothing doing: it draws again.
-    ///
-    /// The default, because it is the only answer that is right without
-    /// knowing anything about the widget. The other two are claims that the
-    /// drawing does not change when the box does, and a widget that inherited
-    /// such a claim by accident would be quietly wrong -- `SetSize` reports
-    /// one size and hands its child the whole box, so its pixels very much do
-    /// change.
     #[default]
     Redraw,
 }
 
 pub trait Widget: Any {
-    /// Draws the widget, and states what it used with `Painter::set_size`.
-    fn draw(&mut self, painter: &mut Painter);
+    /// Draws the widget, and returns what it used of the box it was given.
+    fn draw(&mut self, painter: &mut Painter) -> Size;
 
     /// An exact length the widget can give without a painter or its children.
     /// Optional, and saves a draw rather than changing one: a hint that
@@ -48,17 +38,18 @@ pub trait Widget: Any {
     }
 
     fn on_resize(&self, _axis: Axis) -> OnResize {
-        OnResize::Translate
+        OnResize::default()
     }
 }
 
 impl Widget for () {
-    fn draw(&mut self, painter: &mut Painter) {
-        painter.set_size(Size::ZERO);
+    /// A gap: nothing drawn, at the default length, so a span gives it a share.
+    fn draw(&mut self, _: &mut Painter) -> Size {
+        Size::default()
     }
 
     fn size_hint(&self, _axis: Axis) -> Option<Len> {
-        Some(Len::ZERO)
+        Some(Len::default())
     }
 
     fn on_resize(&self, _axis: Axis) -> OnResize {

src/widget/image.rs

@@ -6,9 +6,9 @@ pub struct Image {
 }
 
 impl Widget for Image {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         painter.primitive(&self.handle);
-        painter.set_size(Size::abs(self.handle.size()));
+        Size::abs(self.handle.size())
     }
 
     fn size_hint(&self, axis: Axis) -> Option<Len> {

src/widget/mask.rs

@@ -5,10 +5,9 @@ pub struct Masked {
 }
 
 impl Widget for Masked {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         painter.set_mask(painter.region());
-        let size = painter.widget(&self.inner).size();
-        painter.set_size(size);
+        painter.widget(&self.inner).size()
     }
 
     /// It clips to the box it was given, not to the part its child used.

src/widget/position/align.rs

@@ -6,7 +6,7 @@ pub struct Aligned {
 }
 
 impl Widget for Aligned {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         // Drawn where it may be too big, then given its aligned box once its
         // size is known.
         let size = painter.widget(&self.inner).size();
@@ -16,7 +16,7 @@ impl Widget for Aligned {
             (None, Some(y)) => UiRegion::new(UiSpan::FULL, size.y.apply_rest().align(y)),
             (None, None) => UiRegion::FULL,
         };
-        painter.place(&self.inner, region);
-        painter.set_size(size);
+        painter.widget_within(&self.inner, region);
+        size
     }
 }

src/widget/position/layer.rs

@@ -6,11 +6,10 @@ pub struct LayerOffset {
 }
 
 impl Widget for LayerOffset {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         for _ in 0..self.offset {
             painter.next_layer();
         }
-        let size = painter.widget(&self.inner).size();
-        painter.set_size(size);
+        painter.widget(&self.inner).size()
     }
 }

src/widget/position/max_size.rs

@@ -6,16 +6,14 @@ pub struct MaxSize {
     pub y: Option<Len>,
 }
 
-impl MaxSize {}
-
 impl Widget for MaxSize {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         let child = painter.widget(&self.inner).size();
         let output = painter.output_size();
-        painter.set_size(Size {
+        Size {
             x: capped(child.x, self.x, output.x),
             y: capped(child.y, self.y, output.y),
-        });
+        }
     }
 }
 

src/widget/position/offset.rs

@@ -6,9 +6,8 @@ pub struct Offset {
 }
 
 impl Widget for Offset {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         let region = UiRegion::FULL.offset(self.amt);
-        let size = painter.widget_within(&self.inner, region).size();
-        painter.set_size(size);
+        painter.widget_within(&self.inner, region).size()
     }
 }

src/widget/position/pad.rs

@@ -6,11 +6,11 @@ pub struct Pad {
 }
 
 impl Widget for Pad {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         let inner = painter
             .widget_within(&self.inner, self.padding.region())
             .size();
-        painter.set_size(Size {
+        Size {
             x: Len {
                 abs: inner.x.abs + self.padding.left + self.padding.right,
                 ..inner.x
@@ -19,7 +19,7 @@ impl Widget for Pad {
                 abs: inner.y.abs + self.padding.top + self.padding.bottom,
                 ..inner.y
             },
-        });
+        }
     }
 }
 

src/widget/position/scroll.rs

@@ -10,7 +10,7 @@ pub struct Scroll {
 }
 
 impl Widget for Scroll {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         let output_len = painter.output_size().axis(self.axis);
         let container_len = painter.region().axis(self.axis).len();
         // Drawn in the whole container to learn its length, then placed at
@@ -31,8 +31,8 @@ impl Widget for Scroll {
 
         let mut region = UiRegion::FULL.offset(Vec2::from_axis(self.axis, -self.amt, 0.0));
         region.axis_mut(self.axis).end = region.axis(self.axis).start.offset(self.content_len);
-        painter.place(&self.inner, region);
-        painter.set_size(child);
+        painter.widget_within(&self.inner, region);
+        child
     }
 }
 

src/widget/position/set_size.rs

@@ -6,15 +6,13 @@ pub struct SetSize {
     pub y: Option<Len>,
 }
 
-impl SetSize {}
-
 impl Widget for SetSize {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         let child = painter.widget(&self.inner).size();
-        painter.set_size(Size {
+        Size {
             x: self.x.unwrap_or(child.x),
             y: self.y.unwrap_or(child.y),
-        });
+        }
     }
 
     /// A declared axis is known without looking at the child, which is what

src/widget/position/span.rs

@@ -8,7 +8,7 @@ pub struct Span {
 }
 
 impl Widget for Span {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         let axis = self.dir.axis;
         // A length for every child before any is placed: from its own hint
         // where it has one, and from drawing it where it does not.
@@ -42,7 +42,7 @@ impl Widget for Span {
             if self.dir.sign == Sign::Neg {
                 region.flip(axis);
             }
-            let used = painter.place(child, region).axis(!axis);
+            let used = painter.widget_within(child, region).size().axis(!axis);
             // TODO: rel shouldn't do this, but no easy way before actually calculating pixels
             if used.rel > 0.0 || used.rest > 0.0 {
                 ortho = Len::REST;
@@ -56,7 +56,7 @@ impl Widget for Span {
             true => total,
             false => Len::default(),
         };
-        painter.set_size(Size::from_axis(axis, along, ortho));
+        Size::from_axis(axis, along, ortho)
     }
 }
 

src/widget/position/stack.rs

@@ -8,7 +8,7 @@ pub struct Stack {
 }
 
 impl Widget for Stack {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         let sizing = match self.size {
             StackSize::Default => None,
             StackSize::Child(i) => Some(i),
@@ -26,7 +26,7 @@ impl Widget for Stack {
                 size = drawn.size();
             }
         }
-        painter.set_size(size);
+        size
     }
 }
 

src/widget/ptr.rs

@@ -6,12 +6,11 @@ pub struct WidgetPtr {
 }
 
 impl Widget for WidgetPtr {
-    fn draw(&mut self, painter: &mut Painter) {
-        let size = match &self.inner {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
+        match &self.inner {
             Some(id) => painter.widget(id).size(),
-            None => Size::ZERO,
-        };
-        painter.set_size(size);
+            None => Size::default(),
+        }
     }
 }
 

src/widget/rect.rs

@@ -28,14 +28,14 @@ impl Rect {
 }
 
 impl Widget for Rect {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         painter.primitive(RectPrimitive {
             color: self.color,
             radius: self.radius,
             thickness: self.thickness,
             inner_radius: self.inner_radius,
         });
-        painter.set_size(Size::REST);
+        Size::REST
     }
 
     fn size_hint(&self, _: Axis) -> Option<Len> {

src/widget/text/edit.rs

@@ -55,37 +55,43 @@ impl TextEdit {
 }
 
 impl Widget for TextEdit {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         let base = painter.layer;
         painter.child_layer();
         let (_, size) = self.view.draw(painter);
-        painter.set_size(size);
         painter.layer = base;
         let region = self.region();
 
         let Some(selection) = self.selection else {
-            return;
+            return size;
         };
         let layout = self.view.buf.layout();
 
         // parley reports selection as boxes in layout space, so bidi and
         // wrapped lines come out right without this code knowing about either.
         for (rect, _) in selection.geometry(layout) {
-            let size = vec2(rect.width() as f32, rect.height() as f32);
+            let rect_size = vec2(rect.width() as f32, rect.height() as f32);
             let top_left = vec2(rect.x0 as f32, rect.y0 as f32);
             painter.primitive_within(
                 RectPrimitive::color(Color::SKY),
-                size.align(Align::TOP_LEFT).offset(top_left).within(&region),
+                rect_size
+                    .align(Align::TOP_LEFT)
+                    .offset(top_left)
+                    .within(&region),
             );
         }
 
         let caret = selection.focus().geometry(layout, CARET_WIDTH);
-        let size = vec2(caret.width() as f32, caret.height() as f32);
+        let caret_size = vec2(caret.width() as f32, caret.height() as f32);
         let top_left = vec2(caret.x0 as f32, caret.y0 as f32);
         painter.primitive_within(
             RectPrimitive::color(Color::WHITE),
-            size.align(Align::TOP_LEFT).offset(top_left).within(&region),
+            caret_size
+                .align(Align::TOP_LEFT)
+                .offset(top_left)
+                .within(&region),
         );
+        size
     }
 
     fn on_resize(&self, axis: Axis) -> OnResize {

src/widget/text/mod.rs

@@ -127,10 +127,9 @@ impl Text {
 }
 
 impl Widget for Text {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
         self.update_buf();
-        let (_, size) = self.view.draw(painter);
-        painter.set_size(size);
+        self.view.draw(painter).1
     }
 
     fn on_resize(&self, axis: Axis) -> OnResize {

tests/layout.rs

@@ -32,3 +32,30 @@ fn resizing_relays_out_against_the_new_output() {
     assert_corners!(h, left, (0, 0), (100, 100));
     assert_corners!(h, right, (100, 0), (800, 100));
 }
+
+#[test]
+fn an_empty_widget_takes_a_share_of_a_span() {
+    let mut h = Harness::new((400, 200));
+    let gap = ().add(&mut h.rsc);
+    let right = rect(Color::BLUE).width(100).add(&mut h.rsc);
+    h.set_root((gap, right).span(Dir::RIGHT));
+
+    assert_corners!(h, gap, (0, 0), (300, 200));
+    assert_corners!(h, right, (300, 0), (400, 200));
+}
+
+#[test]
+fn a_child_drawn_twice_moves_once() {
+    let mut h = Harness::new((400, 200));
+    // `Aligned` draws its child twice; listing it twice would move it twice.
+    let inner = rect(Color::BLUE).add(&mut h.rsc);
+    let centered = inner.center().width(200).add(&mut h.rsc);
+    let left = rect(Color::RED).width(100).add(&mut h.rsc);
+    h.set_root((left, centered).span(Dir::RIGHT));
+    assert_corners!(h, inner, (100, 0), (300, 200));
+
+    h.rsc[left].x = Some(Len::abs(150));
+    h.frame();
+
+    assert_corners!(h, inner, (150, 0), (350, 200));
+}

tests/retained.rs

@@ -14,9 +14,9 @@ struct Counted {
 }
 
 impl Widget for Counted {
-    fn draw(&mut self, painter: &mut Painter) {
+    fn draw(&mut self, _: &mut Painter) -> Size {
         self.draws.set(self.draws.get() + 1);
-        painter.set_size(self.size);
+        self.size
     }
 
     fn on_resize(&self, _: Axis) -> OnResize {
@@ -131,3 +131,35 @@ fn a_placed_child_survives_the_next_frame() {
     assert_corners!(h, top, (0, 0), (400, 80));
     assert_corners!(h, bottom, (0, 80), (400, 200));
 }
+
+/// Lays its child out from the hint alone, never reading what it drew.
+struct FromHint {
+    inner: StrongWidget,
+}
+
+impl Widget for FromHint {
+    fn draw(&mut self, painter: &mut Painter) -> Size {
+        let len = painter.size_hint(&self.inner, Axis::Y).unwrap();
+        let mut region = UiRegion::FULL;
+        region.y.end = region.y.start.offset(len.abs);
+        painter.widget_within(&self.inner, region);
+        Size::REST
+    }
+}
+
+#[test]
+fn a_parent_that_only_read_a_hint_relays_out_when_the_hint_changes() {
+    let mut h = Harness::new((400, 200));
+    let inner = rect(Color::RED).height(80).add(&mut h.rsc);
+    let parent = FromHint {
+        inner: inner.add_strong(&mut h.rsc),
+    }
+    .add(&mut h.rsc);
+    h.set_root(parent);
+    assert_corners!(h, inner, (0, 0), (400, 80));
+
+    h.rsc[inner].y = Some(Len::abs(120));
+    h.frame();
+
+    assert_corners!(h, inner, (0, 0), (400, 120));
+}