# Layout findings log What the sessions reviewing Iris's retained layout found, kept so that nothing here is rediscovered. Each entry says who found it and when. **Delete this file when #19 lands and its fixes are in**; what must outlive it (settled design, the measurement method) belongs in `docs/LAYOUT.md`, and the current plan is in `docs/HANDOFF.md`. ## Follow-up implementation review (2026-09-19) The ask/place split, window-unit frames, exact validity preimages, and bottom-up dirty settling implement the settled design. Keep this approach. It does not guarantee one body call per widget: an unhinted descendant that reports leftover weight still needs a room ask and a slot ask. The explicit measurement redesign remains deferred until an app screen justifies it. The fixes below are on `layout/one-ask` in `/home/bob/repos/iris`: - A collapsed share advances both span cursors. The regression covers one and two collapsed children in all four directions. - A masking widget owns a mask reference and reclaims its existing slot on redraw. Primitives retain their own references. Removing the mask, undrawing its owner, freeing the widget, and replacing the root release ownership; a changed inherited mask rejects drawing reuse. Tests check actual primitive mask indices, movement with and without a region node, child draw counts, clip removal/addition, and empty-mask slot reuse. - **A further handover defect:** `draw_inner` saved the old parent only after a redraw replaced `ActiveData`. The old parent therefore kept the child in its list and could undraw the subtree after its new parent drew it. Capture the old parent before replacing the record. A branch-switch regression reproduces disappearing content when its new parent owns a region node, and also checks the ordinary reuse path. The mask and handover tests fail on the reviewed code and pass with the fixes. The mask fix preserves child reuse rather than redrawing descendants on every mask repaint. No naming sweep or rounding-policy change is included. Validation: workspace tests with and without diagnostics, the release fast oracle, and all three prescribed seed scans pass. Comparing 34,488 cold boxes against `cadfba0` finds 650 changes; withholding just the collapsed-slot fix reproduces the baseline exactly. This is an expected geometry correction, not a cost-only change whose dump should remain identical. The tabs example and an exact 400 px collapsed-share fixture were rendered and inspected. Two test-harness savings leave the random stream and coverage unchanged: `generated` constructs one plan per seed for its sixteen scenarios, and the warm/cold comparison constructs its diagnostic ancestry lookup only after finding a mismatch. No overall speedup is claimed; no deep profile was run. **Integration is larger than an API rename.** The app's pinned `32f6ad8` has 45 commits not reachable from this review branch. In particular, the app's nested/shape masks and shared `Ui` ownership are absent here: #19's mask is still a single rectangle and `set_mask` rejects nested masks. Keep the app pin until those existing capabilities have been integrated. The "Masks" and "UI ownership" sections of `LAYOUT.md` describe the app-side implementation, not everything already present on the upstream review branch. ## Original review of #19 at `cadfba0` (2026-09-19) The original review read `cadfba0`, then the tip of `layout/one-ask`. `cargo fmt --all --check`, clippy `-D warnings` and the 123-test suite were clean there. These are the original failures, fixed by the follow-up above. ### A span misplaces the slot after a collapsed `leftover` child `src/widget/position/span.rs:101` keeps two cursors while it places: `fixed`, everything taken so far, and `start`, where the next slot begins. The branch that drops a share child with no room to divide advances `fixed` by the gap and not `start`: ```rust if len.leftover > Weight::ZERO && len.px == Px::ZERO && len.rel == Rel::ZERO && !shares { painter.undraw(child); fixed.px += self.gap; continue; // `start` still excludes this gap } let from = start; ``` In a 400 px row with `gap(10)` over children of 200 px, `leftover(1)` and 180 px -- exactly full, so nothing is left over and the share collapses -- the tail is placed at `(215, 0)..(395, 100)`. Its slot was 210..400, a gap too long and a gap too early, and the declared 180 was then centred in it. The row reports a total that ends at 400. A child drawn with `rel` lengths is stretched into the extra gap instead of being centred in it, because the slot is a `Place::Fill`. Recomputing the cursor in that branch fixes it, and the tail lands at `(220, 0)..(400, 100)`: ```rust start = shared(fixed, taken, total.leftover, room); ``` The 123-test suite passes with the line in. Nothing in it or in the generated oracle catches the defect: warm and cold layouts are wrong identically, so an oracle comparing the two cannot see it. This is the sharp form of the handoff's older "a vanished child leaves a double gap" item, which described the accounting and not the misplacement. ### A reused child keeps the mask its parent replaced `ActiveData::parent_mask` is recorded and documented as the inherited mask "the one a redraw of it must not be handed back", but `try_reuse` (`core/src/ui/render_state.rs:565`) never compares it with `info.mask`: it gates on dirtiness, layer, move parent and region-node status only. `Painter::set_mask` pushes a fresh `MaskIdx` on every draw, so a masking widget that redraws while its child is reused leaves that child's primitives naming a mask nobody updates again: ``` frame 1: masked mask=Id(0) inner mask=Id(0) frame 2 (the masked widget alone marked): masked mask=Id(1) inner mask=Id(0) frame 3 (the subtree moves up to y=10): mask 0: y starts at 50 <- what the inner's primitives are clipped by mask 1: y starts at 10 <- the live one, clipping nothing ``` The child's drawing is then clipped 40 px too high and its top is cut off. `main` guarded this with `active.mask == mask` in its reuse gate and re-marked the owners of a rebuilt mask in `remask_shape_users`; the rewrite dropped both. Returning `None` from `try_reuse` where `active.parent_mask != info.mask` fixes the repro and keeps the suite green, but it redraws the whole subtree whenever a masking ancestor redraws. The better fix is to give `set_mask` a per-widget mask slot kept across redraws, the way `UiRenderState::move_slot` already keeps a move entry, so the index is stable and `reposition` goes on updating the one the descendants name. ### Regression coverage Both paths now have regressions in `tests/cases/layout.rs` and `tests/cases/retained.rs`; the follow-up above records the additional cases. The descriptions above preserve the original failure at `cadfba0`. ### Clarity, in the order worth doing The first three -- the unswept `extent`, the two same-typed boxes on `ActiveData`, and `Painter`'s four holds accumulators -- were done in `5642f20`, and `Part::All` in `aeb60e5`. What is left: - `draw_inner` returns `(Size, LayoutHolds, LayoutHolds)`, two same-typed values ordered by convention. A named pair makes the swap unwriteable. - `DrawInfo::px` is computed at all four construction sites and read only by `diag::draw_request` and two `debug_assert!` messages. Compute it in the assert. - The `layout-diagnostics` block inside `try_reuse` is 23 lines of counters in the middle of the decision; one `diag::outside(...)` call would keep the branch readable. - `widget_at` does three linear scans per child (`children.contains`, `under.iter_mut().find`, `depend_on`), so a span of *n* children is O(n^2) per draw. Not a problem at today's sizes; it is worth knowing before a long transcript list lands on it.