iris/ai-app
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Redesign span layout around retained placement

Browse Source
This commit is contained in:
iris committed 2026-09-09 15:11:57 -04:00
1 parent ae0af8f5e3
commit 0aa03cf621
30 files changed
+501 -1321
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

docs/LAYOUT.md
+27 -196
View File
@@ -25,16 +25,10 @@ having been carried out.
```rust
pub trait Widget: Any {
/// Draw within `painter.region()` (the space the parent offered) and
/// report how much of it was actually used, per axis.
fn draw(&mut self, painter: &mut Painter) -> Size;
/// True if `draw`'s output (both the primitives it writes and the
/// `Size` it returns) is the same for any `painter.region()` of the
/// same *content* -- an icon, a fixed-size rect, an already-decoded
/// image at its natural size. Default `false` (redraw on any change to
/// the offered region) because assuming independence wrongly produces
/// a stale draw; a widget must opt in.
fn size_hint(&self, axis: Axis) -> Option<Len> { None }
fn is_size_independent(&self) -> bool {
false
}
@@ -55,17 +49,9 @@ own comment: "this literally copies draw so that the lengths are correctly
set in the context, which makes this slow and not cool." Folding sizing into
`draw` deletes that duplicate simulation, not just moves it.
**No single-draw alternative was found that does less work per frame.** The
two-method trait was checked against three properties a real screen needs —
a row placing children in sequence, a widget centering on its own content,
and wrapped text — and in every one, `draw` already has to visit the child
to get a size that is *this specific one's* answer, which today's
`desired_width`/`desired_height` re-derive by re-running (a shrunk copy of)
the same layout the draw pass will do again. So the two-method trait is not
"measure once, draw once" in the general case; it is "measure once per axis,
then draw once," i.e. up to three visits per widget per frame, against one
under the design here. The single-draw model is therefore adopted as
proposed, not merely accepted as a preference.
`size_hint` is not a second layout pass. It is an optional exact answer for
an axis the widget declares without painter context or child access. A
lying hint fails a debug assertion when the widget is drawn.
### 2. Move: O(1) per moved subtree, via a per-widget offset chain
@@ -304,35 +290,18 @@ new caching is needed here; the two now-redundant call sites
disappear, which is a second `render` avoided per frame per text widget
that is being measured by a parent.
**"Parent wants the child's height before deciding the width it will
offer"** — the genuinely circular case named in the brief, e.g. a column
that sizes its own width to its widest child, where that child is wrapped
text whose height (which the column's *own* height depends on) depends on
the width the column has not yet decided. This is not solvable in one pass
for the same reason it is not solvable in CSS shrink-to-fit with wrapped
content: the two axes' answers are mutually dependent. `Span::desired_ortho`
(`span.rs:98-136`) already hits exactly this today and already resolves it
by an explicit second, throwaway pass (its own comment: "this literally
copies draw ... which makes this slow and not cool"). The design keeps that
resolution, made explicit rather than accidental: `Painter` gets
`Span` has no size-only pass. It first reads exact, context-free
`Widget::size_hint(axis)` values. It then draws unknown fixed children
forward from the current cursor, retaining what they paint. Once every
length is known, flexible space is allocated and `Painter::place` moves
each retained child into its final box. A child is redrawn only when that
box changes the size it was drawn for.
```rust
/// Draw `child` at a provisional region to learn its size under one
/// axis's worth of assumption, discard everything it wrote, then draw it
/// again at the region that assumption produced. For the rare parent that
/// cannot pick an offered size without already knowing the answer.
/// Twice the cost of one `draw`; every other case in this file avoids it.
pub fn draw_twice(&mut self, child: &StrongWidget, first: UiRegion, second: impl FnOnce(Size) -> UiRegion) -> Size;
```
implemented as: draw at `first`, record `Size`, remove the widget and its
subtree the same way a resize-triggered redraw already does (`draw_inner`'s
"if not \[same region\], maintain resize and track old children," `:97-100`,
which already frees the old primitives before redrawing) — reusing that
path rather than adding a second one — draw again at `second(size)`, return
the final `Size`. It is opt-in and named for its cost, so a widget only
pays it if it is the one that needs it; `Span`'s cross-axis case is the one
call site converted to it, replacing the hand-rolled duplicate loop.
Hints are optional and affect cost, never correctness. `Sized` can report
its declared axis without inspecting its child, which covers the important
`.height(rest())` case. A debug assertion compares every hint with the
eventual `draw` result. Widgets whose answer depends on shaping or on a
child return `None`.
### 5. Caching and invalidation
@@ -426,7 +395,7 @@ impl Widget for Aligned {
(None, Some(y)) => used.y.apply_rest().align(y).within(&full),
(None, None) => full,
};
painter.reposition(&self.inner, region); // O(1): one offset write, no second draw
painter.place(&self.inner, region);
used
}
}
@@ -435,9 +404,9 @@ impl Widget for Aligned {
`Painter::widget_within`/`widget`/`widget_at` (`painter.rs:55-76`) change
return type from `()` to `Size`, carrying the child's `draw` result back —
the only signature change needed to let a parent see what its child used.
`Painter::reposition` is new, computing the delta between where a child
was actually drawn and where it belongs and calling the O(1) `mov` from
§2. `SizeCtx` and `Painter::size_ctx`/`size`/`len_axis` (`painter.rs:141-150,
`Painter::place` moves an already-drawn child when its used area fits the
target box, and redraws it when the target changes its size. `SizeCtx` and
`Painter::size_ctx`/`size`/`len_axis` (`painter.rs:141-150,
180-182`) are deleted — nothing calls `desired_len` any more, so there is
nothing left for `SizeCtx` to answer; `draw_text`/`label`/`px_size`/
`output_size` already exist redundantly on both `SizeCtx` and `Painter`
@@ -455,17 +424,10 @@ deletes the `SizeCtx` copies, keeping the `Painter` ones.
transcript scroll must not pay every frame) but kept for resize-shaped
changes (§3) where the content's own region field, not an ancestor
chain, is what has to change.
- **A second, size-only trait method kept alongside `draw`** (e.g.
`fn size_hint(&self) -> Option<Size>` as a fast path some widgets could
implement to skip a draw when a cheap answer exists) — considered and
rejected: it reintroduces exactly the "two names for one concept" split
this change removes, for a saving `is_size_independent` (§1, §3b)
already covers for the cases where it would actually help (fixed-size
leaves). A widget whose size is cheap to compute but whose *drawing* is
not (unlikely in this codebase's widget set, but conceivable) is better
served by that widget caching its own draw output internally — exactly
the pattern `TextView::render` already uses (§4) — than by a second
trait method every implementor has to reason about.
- **A general measurement API** — rejected because it walks the same nested
tree again. The narrow `size_hint(axis)` contract is exact,
context-free, and optional; it exists only for sizes a widget already
declares itself.
- **Passing `available` as an explicit parameter to `draw`** (mirroring
Masonry's `layout(&mut self, ctx, bc: &BoxConstraints) -> Size`, the
yardstick per AGENTS.md) — rejected as redundant with `Painter::region()`,
@@ -478,137 +440,6 @@ deletes the `SizeCtx` copies, keeping the `Painter` ones.
but still not O(1), and the shader-side chain costs nothing extra to get
the better bound.
## Deviations found during implementation (2026-09-04)
Five corrections this file's first draft did not anticipate, each found by
`iris/run-headless.sh tabs --shot` disagreeing with a pixel-identical
pre-change screenshot (pass condition 1) and traced with `eprintln!` in
`draw_inner`/`reposition` — not by reasoning about the design in the
abstract. Recorded here rather than silently fixed in place, per the code
rules' escape-hatch requirement.
1. **`Aligned`'s provisional draw must call `painter.widget`, not
`widget_within(&self.inner, painter.region())`.** §6's original text drew
the sample as the latter. `widget_within` composes its `region` argument
as *local*, `UiRegion::FULL`-relative coordinates against
`painter.region()` (exactly what `UiRegion::FULL.within(&self.region) ==
self.region` relies on); handing it `painter.region()` itself —
already-resolved, window-relative coordinates — composes that frame a
second time. For the root widget this is silently the identity (its
region already is `[0,1]`), which is why it can look correct in a
trivial case and only breaks once something is nested — i.e. always, in
practice. Symptom: a centered child rendered at a wildly wrong offset
nested more than one level deep. Fixed by using `painter.widget`, which
hands the child `self.region` unmodified, with no second composition.
2. **A widget that reports a size smaller than its offered region must
actually paint at that size, anchored top-left of what it was given —
not fill the full offered region while merely *reporting* a smaller
number.** `Sized` and `MaxSize` both had exactly this bug: their
`desired_width`/`desired_height` predecessors capped the *reported*
value but their `draw` bodies called `painter.widget(&self.inner)`
unconstrained, which was harmless under the old two-pass model (a parent
always queried the size *before* drawing, so by the time `draw` ran the
offered region already matched) but wrong under `Aligned`'s new
provisional-draw-then-reposition pattern, which offers the *whole*
region on the first, learning pass. Symptom: a `.sized((100, 100))` rect
rendered stretched to fill its whole row instead of a 100×100 square.
Fixed by having both widgets carve the declared sub-region (`UiSpan`
sized to the axis's `Len`, anchored at `AxisAlign::Neg`) out of whatever
they were offered before drawing the child in it. `Image` needed the
same treatment from the start (`texture_within` at its own natural size,
not `texture()` at the full offered region) and was written that way in
the first pass, once this was understood; `Rect`'s "fill whatever I'm
given" is the one case where painting the *whole* offered region really
is the declared behavior, so it needed no change.
3. **The move-offset chain's `parent` link cannot be found by looking up
the parent's `ActiveData` in `draw_inner`, because the parent's
`ActiveData` does not exist yet while its own `Widget::draw` is still
running.** `ActiveData` is inserted only after `draw` returns
(`render_state.rs`, end of `draw_inner`), so a child drawn partway
through its parent's `draw` body — the ordinary case, since every
composite widget draws its children from inside its own `draw` — would
always read "no parent" from `self.active`, silently orphaning it at the
root of the chain. Fixed by threading the parent's `move_slot` down
through `Painter` (it already carries `mask`/`layer` the same way) and
passing it explicitly into `draw_inner` as `parent_move_slot`, rather
than deriving it from `self.active.get(parent_id)`. `move_parent_of`
(the `self.active`-based lookup) is kept, but only for `redraw()`, whose
target's parent genuinely is already active at that call site — the
doc comment on it says which is which. Symptom: `reposition` computed
the right delta and wrote it to the right slot, but the shader never
saw it, because the primitive doing the actual painting chained to
`u32::MAX` one level too early.
4. **`Painter::reposition` cannot reuse `active.region` as "where the
widget currently is," because for a widget offered more room than it
used, `active.region` is the *offered* box, not the *painted* one.**
This only matters for `reposition` (used by `Aligned`); `mov` (used by
`draw_inner`'s own same-size-different-position dispatch, for `Scroll`
and `Offset`) has no such gap, because there the offered region *is*
the visual footprint — content is sized to fill exactly what it is
given. `reposition` instead reconstructs "from" as `active.size`
(already tracked, per §5) anchored at `AxisAlign::Neg` within
`active.region` — i.e. it assumes the child painted itself top-left of
whatever it was offered, per point 2's convention — and **overwrites**
the slot's delta rather than accumulating it the way `mov` does, since
"from" is recomputed fresh from stable inputs every call and repeating
the same `reposition` (an unrelated redraw elsewhere re-running this
widget's parent) must not drift further each time. The one shape this
does not cover: `Aligned` wrapping `Aligned`, where the inner one's own
`reposition` may have moved its content away from top-left already. No
widget or example in this codebase builds that today; if one needs to,
`reposition` would need the child to report *where* it painted, not
just how big, which is a larger change than this pass's scope.
5. **A widget's `move_offsets` slot is allocated once, on its first-ever
draw, and reused in place — never reallocated — for every later redraw
of the same id, with its delta reset to `[0, 0]` on each reuse.** Not
spelled out in §2's original text, which only said slots are assigned
"when the widget is first drawn." Reallocating a fresh slot on every
redraw would leave any *retained* (not-redrawn) descendant's `parent`
link pointing at a now-orphaned old slot — a permanent leak, and worse,
a descendant that silently stops tracking its ancestor's future moves.
Resetting the delta on reuse (rather than carrying it forward) is
required because a full redraw bakes the widget's correct absolute
position into the fresh `region` argument directly; a stale delta left
over from before the redraw would double-offset it.
Two further points worth recording because they were *design decisions*
made while implementing, not bugs — `LAYOUT.md`'s own text left them
unspecified rather than getting them wrong:
- **`Scroll` offers its content a region sized by the *previous* frame's
measured content length, not a fresh one.** A fresh measurement would
require drawing the content once to learn its size and — since that
provisional size essentially never matches the previously active one —
redrawing it a second time at the real size, on every single scroll
tick, which is exactly the cost §2 exists to remove. Using the stale
length means an ordinary scroll (position changes, content does not)
offers the same *size* as last frame, only shifted, which is what makes
`draw_inner` dispatch it as the O(1) move. The cost: a real content-size
change lags one frame before the container's scroll range reflects it,
self-correcting the frame after (the content length itself, read from
what was actually drawn, is never stale — only the offered *region* used
for placement is). No example in this repository builds a `Scroll` yet,
so this could not be checked against a pixel diff; it is covered instead
by `iris/src/layout_tests.rs`'s three `Scroll`-based unit tests, which
build a tree and drive `UiRenderState` directly with no GPU or window
needed.
- **`redraw()`'s parent-relayout check draws the widget first, then
compares the fresh `ActiveData.size` the draw produced against the size
from before removal** — the mirror image of the old code's "query size,
compare, decide whether to draw," which no longer has a size query to
do the comparison with before drawing (§5 deleted `Cache`/`SizeCtx`
along with `desired_width`/`desired_height`). This can occasionally draw
a widget once more than the old code would have (if the parent it
bubbles up to ends up redrawing the same widget again as part of its own
relayout) — `draw_inner`'s own skip/move dispatch absorbs most of that
redundancy for free, and this path was not one of the migration's measured
conditions, so the remaining slack was accepted rather than chased
further.
## Density: `Len::dp`, resolved at `apply_rest` time (2026-09-06)
Iris asked for a third length kind beside `abs` (physical pixels) and
@@ -632,7 +463,7 @@ desktop backend has no per-monitor density wired up yet and stays at
`1.0`. Every layout call site that used to call `.apply_rest()`/
`.to_uivec2()` now passes `painter.density()` (nine call sites — `Span`,
`Sized`, `MaxSize`, `Aligned`, `Scroll`, `LazySpan::place`, and
`UiRenderState::reposition` itself). This also meant the Android
`UiRenderState::place` itself). This also meant the Android
boundary's global logical-space stopgap could come out entirely: window
size, touch coordinates and insets are physical pixels again, matching
`AndroidRenderer`'s own swapchain resolution, with `dp` doing the
@@ -817,8 +648,8 @@ why it must), and `.background(rect(..))` is the ordinary way to style
anything — so a one-frame-stale box is a background drawn at the wrong
size while the text inside it is already right. On screen that is a tool
card that looks closed while its text is there and open while it is not.
A `reposition` is not the fix and cannot be: it writes an offset, never a
size.
A move alone cannot fix a changed size; `Painter::place` redraws in that
case.
The cost is bounded and worth stating, because it is what makes the rule
safe to apply everywhere: the second draw happens only on the frame a