A rule holds what a widget answers and a widget holds the box (Bryan, 2026-09-20). `layout/bounds` is green on all three scans; the hole left is a retained answer re-placed under a different rel base, which predates bounds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
57 KiB
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.
One ask, the root's included (2026-09-20)
0d03267. root_layout read the root's declared lengths against the window
while every other widget's box came of Painter::widget_at, where a rule of
the widget's own can take the box past what its parent offered. Bryan asked
for the two paths unified, and ruled out doing it by putting a widget above
the root: a root sized in pixels would make every fraction under it a fraction
of a px length, which pins the whole tree to those pixels and re-asks it on
every resize.
Placing::WINDOW is the unification -- the full output, fractions of the full
output, no move entry and no mask -- and Placing::ask is the one place a box
is decided, called by the painter, by a local redraw, and by the root's first
draw. What is left of the root's own path is the bookkeeping a widget with no
parent keeps.
Two things fell out. DrawInfo::asked is now the place the parent offered
rather than the place the ask came to, so a local redraw re-decides a rule
instead of re-reading its decision; the two were the same until a rule could
move the box. And the root's is_region_node is read, where the old path
passed false.
The comparison a rule makes is kept on the widget asked about rather than on
the asker. A window range means the same thing at either end of an ask and
in_parent passes one up unchanged, so the asker still ends up holding it
through the child's drawing -- and the root, which has no asker, needs nothing
of its own, since resize already checks its record.
Bounds are a rule that reads its box, and the oracle refuses one (2026-09-20)
On the branch layout/bounds (76aaf06), not in #19. SizeRule::{Min, Max, Clamp} is the capability MaxSize had on the app's pin and nothing on this
branch has. Every hand-written test passes, including a capped scroll taking
its viewport from the cap; the 400-seed depth-5 scan does not.
The finding is general and worth keeping whatever is decided. A bound is the
first rule whose effect depends on the box its parent gives it, and the
retained machinery hands a widget a box by paths that never ask it again
(place_in from a re-placing parent, reposition after a parent's box moved).
A decision made when the box was one length survives into a box of another, so
warm and cold disagree about a tree they agree on structurally. Four readings
measured over 400 seeds at depth 5:
- decided at every ask and kept: seeds 291, 1, 120, 178, 64 differ.
- re-decided at
place_inas well: seeds 1, 362, 188, 254, 156, because that path's box is the one the answer chose, not the one the widget was asked in. - skipping a place the parent decided outright, which is the rule the share follows: worse, since the same widget then gets two decisions by two paths.
- the bound as an answer rule only, leaving the box alone: seeds 4 and 196 -- much the closest.
The share is the one existing rule of this kind, and it is stable for two
reasons that do not generalise: place_at re-asks a child whose rel base it
narrows, and a share that binds is baked into the retained place as a Sized
length. A bound that binds is a length of the rel base, and Sized cannot say
"this slot, narrowed" for a Within place.
Neutering the bounds in the generator while leaving its random draws in place
puts the same shapes back to green, so the divergence is the bounds and not
the new trees. Two findings from the branch are worth having whatever happens
to it: a rule that is a fraction must pin its rel base, which Exact was
already missing for bounds-shaped reasons; and widget_trait! dropped
attributes, so no method it defines could carry a doc comment.
Settled (Bryan, 2026-09-20): a rule holds the answer, a widget holds the
box. de1eb7e. SizeRule::{Min, Max, Clamp} holds the length a widget
answers and leaves its box alone, which is stable because an answer is a
function of what the widget drew. MaxSize is the box version and is a widget
because a widget is drawn again whenever its own box changes, so the
comparison is made where the answer can be kept. .max_width/.max_height
are that widget; .min_width/.min_height stay a rule. All three seed scans
pass. The capped scroll -- 400 px of content under a 100 px cap, a 100 px
viewport and 300 to scroll -- is the app's MaxSize capability, and it comes
back through the widget.
One hole is left and is older than bounds: place_at hands a parent a
retained answer without checking that the answer still holds for the rel base
that place gives, so a fraction resolved against one rel base survives into
another. A fraction in a bound is the first thing to reach it (seeds 4 and 196
at depth 5), which is why the generated trees grow bounds in pixels only, said
where the tree is grown. An Exact rule that is a fraction can reach it too.
The fix is a holds check at the re-place site, not anything about bounds.
Images in the generated trees (2026-09-20)
Image is the only widget here whose size hint is a length in pixels, so it is
the only one that exercises a rule beside a hint -- which is what the two
findings above are about, and why neither was reachable from a generated tree.
Bryan asked for one in the trees, as a committed 64x64 checkerboard reusing one
texture handle. 2dba90b: Kind::Image is a fifth leaf drawn one time in five,
stepping to a plain rect under the shrinker; the picture is
src/assets/checkerboard.png, purple and black in 8 px cells, included rather
than opened; and a TextureHandle is a counted reference, so the first image in
a tree uploads and the rest clone it. Measured: seed 1 at depth 4 grows 13
images and holds 1 texture, and a_tree_of_images_uploads_one_texture asserts
it.
The cost is that a seed no longer names the tree it used to, so the corpus in
generated.rs is a corpus and not a set of regression cases -- 20 and 86 no
longer grow the trees whose defects they caught. Both live on as shrunk fixtures
in unsettled.rs, which are trees rather than numbers, and both files now say
so. New dump baseline: 34,571 boxes over the 400 depth-5 trees. All three
scans pass over the new trees (400 at depth 5 in 62.79s, 1,000 at depth 6 in
160.20s, 2,000 at depth 4 in 299.58s).
Ninth sweep: the widget vocabulary, and the eighth sweep's fix (2026-09-20)
Over the part no earlier round named -- src/widget/trait_fns.rs and
wrapper.rs, core/src/widget/widgets.rs, the util additions, examples/
and the two manifests -- and once more over 77ed7a2, which was the eighth
sweep's own commit. Eight findings, all in c2b8bf8. The cold dump over 400
depth-5 trees is byte-identical to 77ed7a2 across all 34,488 boxes, and
all three seed scans pass (400 at depth 5 in 63.27s, 1,000 at depth 6 in
160.45s, 2,000 at depth 4 in 302.52s).
A hint overrode a rule. Widgets::declared_lens asked
rules[axis].declared() and fell through to the widget's own size_hint
whenever that answered None -- which it does for a share, because a share is
not a declaration. So a widget carrying width(leftover(1)) and hinting a
pixel length of its own was given a box of the hint, against the rule and
against the comment inside the function ("a hint still narrows the box where no
rule does"). Painter::size_hint spells the same rule-else-hint step three
hundred lines up and gets it right, with the reason written on it; both read
Widgets::exact_len now, and declared_lens is the part of its answer that
needs nobody to divide it.
Image is the only widget in the repository whose hint is a declared length
(every other hints LEFTOVER, whose declared() is None either way), and
neither the tests nor the generator builds one, so nothing here could reach the
difference -- which is why the dump is unchanged, and why
a_share_rule_beats_the_widgets_own_pixel_size builds a widget of its own. It
records the box it was asked in: 400 with the rule and 50 without, and 50
either way at 77ed7a2. The alternative reading -- a hint narrows even under a
share rule -- would mean changing that comment instead, and is Bryan's to
prefer if he does.
What a leftover means where nothing divides it (Bryan, 2026-09-20, on the
finding above). A leftover under a parent that does not divide is still a
leftover, acting as a minimum: where the px and rel parts come to less than
the box it fills the rest, and where they come to more they overflow as normal.
The length is max(box, px + rel*box) -- which is the same max he gave for
Scroll's content length on 2026-09-18, generalised to every non-dividing
parent.
Measured at c2b8bf8, a probe recording the box it is asked in, in a 400 px
window, under .wrapper() against a one-child span:
rule nothing divides a span divides
leftover(1) 400 400
px(50) + leftover(1) 400 400
px(500) + leftover(1) 400 500
rel(0.5) + leftover(1) 400 400
px(500), no leftover 500 500
One row disagrees. A leftover whose fixed part is longer than the box loses the
overflow where nothing divides it. The span is right and says so in place --
"One that also asked for pixels or a fraction keeps those and overflows" -- and
only_a_pure_leftover_child_disappears_when_nothing_is_left asserts it at
100..120 of a 100 px row. The cause is that LayoutLen::declared refuses to
answer for anything carrying leftover weight, so the non-dividing path never
learns the fixed part and falls back to the offer. The same length without the
share does overflow (drawn -50..450, its alignment centring it), so what
swallows it is the share and not the overflow.
Fixed in b295c8b, after Bryan asked for it if it was cheap and pointed out
that the machinery is a minimum size of rel(1.0) and should share a path with
one. What made it cheap was saying it as the place the parent gives rather than
as a declaration: where the fixed part is the longer, widget_at hands the
child fixed.as_desc().fills(), which is what a declared length already comes
to, and active.placed keeps it -- so nothing about Declared, ActiveData or
the resize path had to change. A place that is already the child's placement is
skipped, because a parent that divides has given the share whatever it was owed;
without that guard a span's slot was re-placed and its child moved from 0..500
to -50..450.
The comparison is one operation now, Painter::longer_than, and both callers
share it: the span's room for the shares it divides, and a share past the box it
was given. It narrows the widget's window range where the span replaced it,
since a comparison the framework makes on an arbitrary parent's behalf is one
more reason its drawing holds rather than the only one. SizeRule::Min of
rel(1.0) is the same operation and belongs on the same path when it lands.
Two things were tried and dropped. An escalation for a changed rule
(shares_past_a_length) turned out to buy nothing: the reported size is the
rule resolved, so a changed rule changes the answer and the parent refuses its
own drawing -- the tests pass without it. And Holds::crossing, a named
constructor for the range, had one caller and read better spelled there.
The root still reads the old way. root_layout asks
Widgets::declared_lens with no painter, so a mixed share on the root is the
window rather than the longer of the two. It would mean the comparison in a
second place, and the root's box is the window and is relaid out on every
resize; nothing here puts a share on a root.
Marking a widget for redraw had no name. Twenty-one sites under tests/
said it as widgets_mut().get_dyn_mut(id); with the widget thrown away: five
with a let _ = in front, one with a comment explaining what the line was for
("taking mutable access is the ordinary content-change signal"), and one inside
a local function already called mark. The framework's own word for it is in
its comments -- "marked for redraw" -- and Widgets::mark_for_redraw is now
the method. revision_cost.rs keeps the long spelling and says why in place,
since it is deliberately in the API subset an old worktree also has.
assert_same_regions could not see the defect the eighth sweep had just
fixed. It zips the warm and cold id lists, so a list naming one widget twice
compares fewer boxes than it lists and reports nothing. It now rejects a
repeated id and two lists of different lengths, which is the same check applied
to the whole class rather than to the four fixtures that had it wrong -- and it
verifies that round's claim about the other nine: all eighteen cases pass.
Bare pairs where the framework has named ones. random.rs's Lens and
Aligns were [Option<LayoutLen>; 2] and [Option<AxisAlign>; 2], read as
[0]/[1] and zipped against a hand-written [Axis::X, Axis::Y] in two rigs.
They are SizeRules and Align, which is what the framework calls those
pairs; Align took the Index<Axis> every other per-axis pair on this branch
has, and RegionAlign::from does the "an axis left out is centred" step both
rigs were spelling per axis. The three sites that wrote the axis pair out say
Axis::BOTH, which the layout core already says ten times. The plan's
Debug is why Align gained one.
Forty-five lines nothing references. BothAxis<T>, AxisT, XAxis and
YAxis -- a const trait, two marker types and three accessors -- have no user
anywhere in the workspace, and are the mechanism impl_axis_index! replaced,
in the very file this branch took Vec2::axis/axis_mut out of. Deleted as a
drive-by in a block the branch was already rewriting, the way MASK_NONE was;
drop it if the scope matters more. They pre-date the PR, so a plain "is this
name used" scan over the diff does not surface them.
One word for two things. Wrapper (the widget) arrived on this branch
beside core's WidgetWrapper (a dynamic borrow guard), both in the prelude.
The alias had two uses in one file and DynBorrower<dyn Widget> is what they
are, so it is gone rather than renamed. Wrapper::new, Wrapper::empty and
its hand-written Default were three names for one value, two unused.
Smaller. Arena::get_mut was the only pub(crate) among pub siblings on
a public type. Selector rounded the pointer onto the pixel grid in order to
add two values already on it, losing the precision the platform gave it for
nothing -- the step between the two regions is taken on the grid instead, which
also makes it agree with Selectable, the other caller of select. And the
two debug profile settings now carry their reason where the next reader
looks: profile.dev's was added in a commit about renaming rest and
explained nowhere, and profile.test's only in the message of the commit that
made the tests one target.
Tripped a rule and left as it stands
examples/random.rsis a viewer for the fuzz generator rather than a demonstration of a feature, which is what an example is for here. Left:scripts/run-headless.shopens an example by name, so this is how a generated tree is put on a screen at all.- It also carries a fifth copy of the six-line
envhelper, on top of the four undertests/. Same answer as the eighth sweep gave: sharing it means a new file for six lines ofstd. IRIS_SEED/IRIS_DEPTH,IRIS_GENERATED_SEED(S)/IRIS_GENERATED_DEPTHandIRIS_DUMP_SEEDS/IRIS_DUMP_DEPTHare three names for two knobs. Left: the prefixes are what lets one shell set a rig's seed without changing another's, and the two rigs that grow the same tree from the same number do share the unprefixed pair.set_size_ruletakes aSizeRulewhileset_size_rulestakesimpl Into<SizeRule>per axis, so seven callers writeSizeRule::Exact(len)where two writeSome(len). Left: widening the single-axis one makesharness.rs'slen.into()ambiguous between two conversions. The doc that called it "for a caller holding a pair" -- which it is not, since it takes two values -- now just says both axes at once.Wrapper::setisreplacewith the result dropped, and both have a caller inexamples/tabs. Left: it isOption::insertbesideOption::replace, and the tab bar wants each.Masked's size comment creditsScrollwith the same reasoning rather than restating it. Left: the reason it gives is its own ("it clips what is inside to that box"), and the cross-reference is to a design parallel, not an API.- The nine
RegionAlign/CardinalAlignconstants include four nothing names (TOP_CENTER,CENTER_RIGHT,BOT_CENTER,V_CENTER). Left: they are one vocabulary of nine positions and six cardinals, and deleting the members nobody has needed yet is the rule written on one member of a set.
Eighth sweep: the tests, and the seventh sweep's fix (2026-09-20)
Over the part no earlier round named -- the 6,300 lines under tests/, which
is more than half of what #19 adds -- and once more over f8aa0c5, which was
the seventh sweep's own fix and so unreviewed. Seven findings, all in
77ed7a2. No library code changed: the cold dump over 400 depth-5 trees is
byte-identical to f8aa0c5 across all 34,488 boxes, and the seed scans
have nothing to find, since nothing that decides a layout moved.
That count is not the 34,492 the four rounds before this one recorded, nor
the 34,490 77ed7a2's own message gives. The dump prints eight lines that
are not a box -- cargo's two, libtest's four, and two blanks -- so a wc -l
of the run comes to 34,496 and any partial filter lands somewhere between.
The boxes are the lines matching ^[0-9]+ [0-9]+ , and there are 34,488 of
them at both f8aa0c5 and 77ed7a2. What every round actually established
is still true, since each diffed two dumps rather than trusting a count.
A shrunk fixture that names one widget three times. width, sized and
align set a rule on the widget they are handed and give its own id back;
only pad and wrapper make a new widget. So in
let wrapped = wtext("Wrapping shapes").size(16).wrap(true).add(&mut h.rsc);
let sized = wrapped.width(76).add(&mut h.rsc);
let aligned = sized;
all three names are one text, and all three went into the vector of ids the
case compares warm against cold. Measured: plant and plant_fixed list six
ids and hold four widgets, plant_pair lists four and holds three,
plant_scrolled lists eight and holds seven; the other nine fixtures are
honest. So four cases check fewer boxes than they say, and the doc comment of
each quotes the inflated count as the size of the tree the shrinker reduced
to -- which is the number a reader uses to judge whether a case is still the
minimal one. The names are gone and the counts are what the fixtures build.
trace_unsettled.rs carries a copy of two of these fixtures and had the same
aliases.
The check that mattered here is that a rebuilt fixture is the same tree: a
regression test whose fixture quietly changed still passes and no longer
covers its defect. Each was diffed against its old self -- same widget slots,
same regions, for both settings of swapped.
A helper at the top of the file, and seven copies of its body below it.
assert_same_regions collects every widget whose box moved and reports them
together. Six tests call it; seven more spell the eight lines out instead,
byte for byte. They call it now, and it took #[track_caller] so the panic
names the case rather than the helper.
The GPU rigs' adapter probe, written twice. draw_cost and chain_cost
each held a config (identical) and an adapter probe (identical but for the
feature it asks for and what it returns). The probe leaks its Instance on
purpose -- a Vulkan loader may unload the driver as a test thread exits --
and only draw_cost said so, with chain_cost referring the reader to it.
Both come from tests/gpu/mod.rs now, shared through #[path] the way
scenario/mod.rs already is, with the justification on the thing it is
about.
The mask a widget is clipped by, resolved three times, two of them a
byte-identical closure defined inside a loop. mask_bounds takes the
MaskIdx rather than the widget, because the third site reads the slot it
saved before the frame: that a redraw keeps the same slot is exactly what it
is checking, and a helper that looked the slot up again would have made that
assertion pass for the wrong reason.
A field nothing reads. Layered::_revision existed to be incremented, to
mark its widget dirty. Two tests in the same file already do that with
widgets_mut().get_dyn_mut(id), which is the idiom tests/scenario/mod.rs
uses as well. The underscore was hiding the dead-code warning that would have
said so.
A claim the test below it does not make. plan.rs said "Every
simplification is strictly smaller, so taking them in turn reaches a fixed
point instead of circling", and then asserted small.size() <= node.size().
Measured: 53 of one tree's 101 simplifications keep the widget count, because
Plan::size counts widgets and dropping an alignment or stepping Wrapped -> OneLine does not change it. The assertion is the right one and the claim was
not. What actually rules out circling is that those steps are one-way too --
a Some becomes a None, a kind steps down a ladder with no way back up --
and the comment now says that. The test is no_simplification_of_a_plan_is_larger_than_it.
Numbers and lines that had gone stale. generated.rs says "Eight that
have never failed" and "the ten the others check"; both said one fewer,
having been written when SEEDS had nine entries and not updated when
d8ae9c3 added seed 20. The should_panic scroll test ended in an
h.frame() that cannot run, because Harness::set_root lays the tree out
and is where the panic comes from -- verified by deleting it. Two
drop(tree) calls sat at the end of their own scope.
Tripped a rule and left as it stands
envis written four times undertests/(layout_dump,layout_diagnostics,revision_cost,scenario).revision_cost's is deliberate and documented: that file is kept in the API subset an old worktree also has, so it can be dropped in and measured there, and a#[path]module would break that. Sharing the other three means either a new file for six lines ofstdor pullingscenario's 494 lines into two more binaries.determinism.rs'sBranchesOnMeasurementisiris::random::Branchwith the same four fields and nearly the same body, andunsettled.rsbeside it uses the realBranch. The difference is load-bearing: the copy states no contract, so the framework must re-ask it at every width, which is the whole point of a test about whether a re-measure branches the same way.Branchalso has asize_hintthat changes how a span treats it.- Three tests call
h.frame()immediately afterh.set_root, which already frames. Unlike theshould_panicone, these are reachable and assert that a settled second frame adds no draws. Left; the redundancy reads as noise but removing it removes a check. primitive_boundsandprimitive_masksshare the walk from a widget's primitives to their instances and differ only in the field they take. Left: naming the intermediate means exporting the instance type into the test, which is more coupling than the four shared lines are worth.Harness::replayignores each sample'st_ms, and the parser rejects time running backwards with a comment about the wait between samples -- which isreplay-touch's behaviour, not the harness's. Left: the parser is shared by both replays and the rule is the real one, but a harness gesture has no timing, so nothing in-process can measure a fling velocity.layout_diagnostics.rs'sreporttakes_harnessand reads it, because it is only used underlayout-diagnostics. An underscore on a parameter the body uses is backwards, but the alternative is acfg_attred allow.
Seventh sweep: the rigs, Fixed, and the sixth sweep's fix (2026-09-20)
Over what no earlier round named -- src/random.rs and tests/scenario/,
core/src/fixed.rs, scripts/run-headless.sh -- and once more over
b7b8d09, which was itself unreviewed because it was the sixth sweep's own
fix. Six findings, all in f8aa0c5. The cold dump over 400 depth-5 trees is
byte-identical to b7b8d09 across all 34,492 boxes, and all three seed scans
pass (400 at depth 5 in 62.75s, 1,000 at depth 6 in 162.37s, 2,000 at depth 4
in 305.25s).
A scroll still asking a question it has already answered. The sixth
sweep found anchor != Px::ZERO in Scroll's content test and removed it;
the operand beside it is the same defect and survived. self.content_len is
answer_px.max(container_len), so content that fits has nothing to scroll
through, and update_amt on the line above clamps amt to
content_len - container_len, which is then zero. So in
let content = match self.amt != Px::ZERO || self.content_len != self.container_len {
the first disjunct can never decide the match: amt != ZERO implies
content_len != container_len, which the second already tests. It is now
match self.content_len > self.container_len, the exact complement of the
content_len <= container_len that guards the contract fifteen lines above.
Verified by asserting the implication in place and running the whole suite,
the scrolling tests included; it held. This is the lesson of "the fixes a
review produces are themselves unreviewed code" arriving on schedule -- one
round's fix left the same mistake in the expression it was editing.
Things nothing reads, in the new arithmetic. Fixed::to_scale converts a
value between two fixed-point grids, and shift_round exists only to serve
it. Both arrived on this branch; the only caller either has ever had is
a_coarser_grid_rounds_and_a_finer_one_does_not, the test written for them.
Every other Fixed method has real callers (checked one by one), and nothing
needs a grid conversion: ratio, mul and div already cross grids where
layout has to. All three deleted. to_scale was also the one operation in
the file that could overflow silently without documenting it -- going to a
finer grid is self.0 << (TO - SHIFT) and the test only ever went coarse,
fine, coarse with a value small enough to survive the trip.
Len arithmetic written a component at a time. Len::align built a
Len whose px is always Px::ZERO, then added to and subtracted from both
of its components by hand:
start: Len::from_parts(at.rel.sub(self.rel.mul(rel)), at.px.sub(self.px.mul(rel))),
end: Len::from_parts(at.rel.add(self.rel.mul(rest)), at.px.add(self.px.mul(rest))),
Len::scale is "both parts by the same fraction" and Len has Add and
Sub, so the whole rule is at - self.scale(rel) and
at + self.scale(Rel::ONE.sub(rel)) -- the point the alignment names, less
the part of the length before it. Identical arithmetic, which the dump
confirms. It is the only place in the codebase that expanded a Len
operation like this; LayoutLen::apply_leftover touches rel alone, which
is genuinely one component.
A question asked through a value, one line from its sibling. 445287c
moved every method that answers a question about a value to &self.
LayoutLen::without_leftover was missed, and its own doc comment calls
apply_leftover -- which takes &self -- "the opposite reading of the same
value". Now &self too. Every other by-value method in the workspace that
does not return Self was checked: the rest are conversions on numbers
(Fixed::raw, to_f32) or builders.
A rig that scales a gesture against a mode the output no longer has.
run-headless.sh --mode sets out_w/out_h beside the swaymsg output ... mode, because those two numbers are the extent replay-touch passes to
zwlr_virtual_pointer_v1::motion_absolute -- the recording's coordinates are
a fraction of them. --resize, added in b7b8d09's round, changed the mode
and left the extent alone, so --resize 800x600@60Hz --replay flick.touch
replayed every sample at x * 800 / 1920 and finished looking like a run
that worked. Both are one set_mode function now, so a third mode change
cannot get it wrong. Found by reading rather than by running: this VM has no
recorded gesture that also resizes, which is exactly why it went unnoticed.
A comment the plan/build split stranded. src/random.rs's "a row takes
the height it is given rather than its tallest child" sat above
let gap = self.rng.below(3) as i32 * 4, telling a reader that the line
consumes no randomness. It describes the set_size_rules that 98d4e98
moved into Build::kind, and the line it was left above is one of the two
draws in the function. Moved to the rule it is about, and the
seed-stability half dropped: building a plan consumes no randomness at all
now, so there is nothing left for that clause to say.
Tripped a rule and left as it stands
Align::tuple,UiVec2::partial_alignandVec2::partial_alignhave no callers anywhere. All three pre-date this PR and are outside its diff, so they belong to the pre-review-gate sweep rather than to this branch.Vec2::align/partial_alignareUiVec2's two functions again withUiVec2::from(*self)in front. Also pre-existing, and merging them means deciding whetherVec2should have them at all.impl_op!has four grammars for one macro, andcore/src/util/vec2.rsspells two of them one line apart --impl_op!(impl Add for Vec2: add x y)besideimpl_op!(Vec2 Sub sub; x y). Theimpl ... for ...:arm has that one caller. Pre-dates this PR; the two arms this PR added (same ...) are a real distinction, since a type mixing a fraction and an offset has no meaning for a baref32.AxisAlign,RegionAlignandAlignlostEqwhenAxisAlignbecame aRelwrapper, whichRelderives. Nothing needs it, so it was left rather than adding a derive with no reader.Holds::throughspecial-cases a fully unbounded range before inverting a fraction, which the general path would also answer correctly throughnarrow. Left: it is the one place aPx::MIN-to-Px::MAXinterval is shifted and divided, and the early return says that no fraction can narrow "every length".Scroll'scontent_len <= container_lencan only be equality, given themaxthat built it. Left:<=reads as "the content fits", which is what the branch means, and the mirror>now reads as "it overflows".scenario::Case::name,windowandShuffle::oftakeselfonCopyenums. They are test-rig code the&selfpass did not cover, and an enum with no fields is the one place taking a value costs a caller nothing.
Sixth sweep: the shader boundary and the position widgets (2026-09-20)
Over what the five earlier rounds did not name -- the WGSL prelude and how
it is assembled, the position widgets, orientation/, and the sensor walk.
Scoped against upstream/main at ca2b4b2, which is PR #19's real base;
the first half of this sweep used the local main and had to be redone, for
which see "The branch layout" in docs/HANDOFF.md. Two findings.
The cold dump is byte-identical to 1096c31 and all three seed scans pass
(400 at depth 5 in 69.07s, 1,000 at depth 6 in 169.29s, 2,000 at depth 4 in
300.75s).
A number both sides count in, written twice. module_source already
builds each shader's preamble from iris_core's own constants, with a
comment saying why: "a grid the two disagree about puts every coordinate
somewhere else". The move-chain work then added, to the very file it
prepends, a second copy of two of its own numbers -- const MOVE_NONE and
const CHAIN_LIMIT, under "Keep in step with iris_core::CHAIN_LIMIT" --
asking a reader by hand for what the mechanism beside it exists to do, and
duplicating the reasoning CHAIN_LIMIT's Rust declaration already carries.
Both are injected now and the shader declares neither.
every_shader_validates composes the real preamble so it covers the change;
what it could never have caught is the two numbers drifting apart, which is
now unrepresentable.
MASK_NONE went in beside them, replacing a bare 4294967295u in masked
-- where the CPU deliberately keeps MaskIdx and MoveIdx as separate
types so the two cannot be swapped. That literal pre-dates this PR; it
is a one-line drive-by in a block the PR was already rewriting, taken
because leaving it means a named sentinel for one index and a magic number
for its sibling four lines apart. Drop it if the scope matters more.
A scroll positioning content it does not position. Two halves, one
mistake, both new in this PR -- the base's Scroll has none of this
machinery. The author believed Scroll places its own content.
content_len is answer_px.max(container_len), so it is never less than
the box. Two lines on, slack was (container_len - content_len).max(ZERO)
and anchor was slack * align.rel() -- provably always zero, whatever the
alignment, with moved then testing anchor != ZERO for nothing.
Instrumented at 1096c31, a centred scroll over 50 px of content in a
200 px box prints slack=0 align=AxisAlign(0.5) anchor=0 and centres the
content anyway: the framework does it, by placing the inner's answer in the
whole box, which comes out as rel 0.5 - px 25 and resolves at any length.
The comment credited the arithmetic for behaviour it could not produce.
The same belief cost a redraw. The contract for content that fits was
guarded by align == AxisAlign::NEG, on the reasoning that at any other
alignment "it moves with every length the box takes and the drawing holds
for that length alone". It does not move -- the framework's placement is a
fraction of the box -- and the default alignment is the middle, so the
common case was the guarded one. Painter::px_len holds a drawing to the
length it read unless the widget says otherwise, so with no holds stated
the scroll redrew on every box change. Measured with distinct_widgets: 1
at CENTER, 0 at TOP_LEFT. Dropping the alignment test gives 0 at all
three, which is what a_fitting_scroll_holds_for_every_box_its_content_fits_in
asserts; it fails at 1096c31 on the CENTER case. align had no other
reader, so the widget no longer asks its own alignment at all -- which is
the tell that both halves were one mistake.
UiSpan::translated and UiRegion::translated arrived on this branch
and are reachable only from each other, which is why a plain "is this name
used anywhere else" scan does not see them: neither looks unused on its own.
The region one carried a performance argument for a function nobody calls.
Both deleted.
Kept although they pre-date this PR
GlyphAtlas::glyph_count, TextBuffer::new_empty and the let-chain in
TextEdit::apply_event that #10 had expanded into a nested if. All three
were found under the wrong base and are outside #19's diff; Bryan said to
keep them anyway (2026-09-20), since the two are dead either way and the
third is a straight restoration.
Tripped a rule and left as it stands
diag::untrace_widgetis new here with no caller, which is the test thetranslatedpair was deleted by. Left: it is the "removed" half of whattrace_widget's own doc promises ("until explicitly removed or cleared"), andclear_traced_widgetsis the "cleared" half and does have one. A rig drives this API from outside.UiSpan::flipswapsstart.relwithend.relandstart.pxwithend.px, whereLenhas exactly those two fields and one swap of the structs would do. Pre-dates this PR.UiVec2translates under three names --shift,offset, andUiRegion::offset-- besideLen::offset, which adds pixels to one end and is a different operation. Pre-existing, and one vocabulary is Bryan's call rather than a sweep's.- The nine
#[allow(unused_variables)]are all on trait methods with empty default bodies, where the parameter names are the signature's documentation and underscoring them would hide it from implementors. GpuPages::updategrows before it drains and both go through the one queue, so the copy is ordered before the writes. Correct as it stands.
Fifth sweep: the retained path, the renderer and the diagnostics (2026-09-20)
Over the parts the four earlier rounds did not read -- the renderer, the
text store, the input default, the harness -- and once more over redraw.
Six findings, d8d5122 through 1096c31, the last two from Bryan's reading
of the first four. The cold dump is byte-identical to 781199a and all three
seed scans pass (400 at depth 5 in 69.45s, 1,000 at depth 6 in 214.69s, 2,000
at depth 4 in 419.50s).
A contract was kept where it no longer held (d8d5122). The sibling of
713e3e7, in the same function. redraw keeps the narrower of the old and
the fresh contract so widening and narrowing back do not churn the parent.
The drawing's half asks was_holds.contains(window, rel_base, region) first;
the answer's half did not. A widget whose answer contract widened in a frame
that also resized the window therefore kept a range the new window is
outside, and the parent's next ask refused it and redrew the whole subtree --
throwing away the drawing that widget had just made. Cost, not geometry: the
size kept is the size just reported. It needs both a resize the root does not
absorb and a mark on a deeper widget in the same frame, which is what
a_contract_this_window_is_outside_is_not_kept builds; it draws the leaf
twice at 781199a and once with the guard in.
Configuring a surface under its own texture (02048ea). wgpu 30 says at
both Surface::configure and Surface::get_current_texture that configuring
while a texture the surface handed out is still alive panics. The
Suboptimal arm of UiRenderer::draw configured with the texture it was
about to draw with in hand, so the first suboptimal frame -- a resize or a
display change on some drivers -- takes the app down rather than rebuilding
the swapchain. The texture is good for that frame, so it is drawn with and
presented and the rebuild happens after present consumes it. Not
reproducible on demand here; the claim rests on wgpu's own documented panic.
This one is the reason for the sweep recorded under "The code written before the review gate" below: nothing about the arm was hard, and it was written that way anyway.
A counter named the wrong contract (9b4cc32). AxisHolds is four
contracts and diag::outside counted three: a refusal because this window
is outside the range the drawing was made for bumped "reuse outside: a rel
base". A window range is pixels and a rel base pin is a window-unit length an
unchanged window can still change, so the rig answered "why did that
redraw?" with the wrong one for every resize. Same class as 8088a1f.
Things nothing reads (7502176, 1096c31). Axis::pair,
RegionAlign::NEAR and Painter::text_data arrived on this branch with no
caller and never got one. The comment beside a span's cross-axis accumulator
said a scalable child "makes Children scalable too" -- Children names
nothing in this repository, and what it makes scalable is the span.
text_data was held back a round on the grounds that it is the only way a
widget inside draw can reach TextData, and the app's integration might
want it. That reasoning is wrong: nothing in iris is kept for the app's sake,
because the app is to be largely rewritten against this API rather than
ported call by call (Bryan, 2026-09-20).
A question asked through a value (445287c). 7502176 moved
Holds::contains from &self to self to match its five siblings, which
was the wrong way to reconcile them. A method taking self can only be
called on a value, so a caller holding a reference has to dereference to ask
-- Copy or not (Bryan, 2026-09-20). Every method that answers a question
about a value now takes &self: Holds, AxisHolds and LayoutHolds
throughout, LayoutLen::{is_px, is_only_leftover, declared, fills} and
Size::within_box. Builders that return a changed copy still take self.
Tripped a rule and left as it stands
TextData's spare store clones the whole string intoPlacedon every re-break. Bounded at 128 entries, but the clone is per re-break and proportional to the text; a transcript-sized text would pay it on every width change. Left because a cheaper key changes what "two texts of the same words share an answer" means, which is a design question.TextEdit's undo history pushes a whole copy of the text per changed keystroke and is never bounded, andapply_eventclones the text on every event including the arrow keys.apply_eventis not in this PR's diff at all: it was last touched by #10 and #16, both already onupstream/main. It keeps resurfacing in sweeps because they diffed against the localmain-- see "The branch layout" indocs/HANDOFF.md. Bryan wants the unbounded push and the clone dealt with as a change of their own (2026-09-20).ActivationState::updatewrites four arms where theStart/OnandEnd/Offpairs are identical, andis_offis!is_on. Also verbatim frommain.TextView::draw's empty-with-hint branch matches onself.hintagain afteris_some()guarded it, so itsNonearm is unreachable. The guard cannot become anif letbecauseself.render(painter)needs&mut selfbetween the two. Left rather than cloning the handle to satisfy the shape.CurrentSurfaceTexture::Lostis answered by reconfiguring, where wgpu says to recreate the surface. It will not panic, and recreating needs the window; worth doing with the next renderer change rather than this one.
Quality sweep of the whole branch (2026-09-20)
A fourth sweep, over the layout core, the arithmetic, the atlas, the sensor
walk and the fuzz rig rather than over naming. Four findings, all on
layout/one-ask past 1ebd4d3; the cold dump is byte-identical to it and
all three seed scans pass (400 at depth 5 in 65.09s, 1,000 at depth 6 in
161.27s, 2,000 at depth 4 in 301.90s).
A kept contract was judged against the wrong box (713e3e7). redraw
keeps the narrower guarantee a parent holds when the fresh drawing covers
it, so widening and narrowing back do not churn the parent. It asked
was_holds.contains(.., active.placement) -- where the answer put the
drawing -- when holds is about active.region, the box the drawing was
made in. The two differ on every axis a widget reported less than it was
offered, so such a widget marked its parent every time its contract
widened. Cost, not geometry: accepting is always safe, since region is
always inside the old range, so refusing only escalates. resize and
try_reuse both already ask about region.
widening_what_a_drawing_holds_for_does_not_relay_out_the_parent fails at
1ebd4d3 and passes with the line changed; the existing
widening_and_restoring_a_contract_does_not_invalidate_its_reader cannot
see it, because its leaf reports LEFTOVER, which fills its box.
Two things nothing read (aea0387). ActiveData::size_deps was written
on every draw and cleared on every undraw, and read nowhere -- a Vec per
active widget. The Painter's own copy is the live one, used in draw_at
to record whoever asked about a child it did not draw. SizeRule::apply
had no caller and would have been wrong with one: it answers the rule's own
length where draw_at resolves a fraction against the rel base first.
Three reuse rejections said nothing (8088a1f). Of the eight
rejections in try_reuse, a changed inherited mask counted and traced
nothing, an undrawn record traced without counting, and a changed
region-node choice counted without tracing. The mask one is what this
branch's repair was about, so the rig could not answer "why did that
redraw?" for it. Adding a counter meant editing a variant list and a name
list at the same index; they are one declaration now.
A fuzz case ran only in the long scan (69ba915). Case::SizeResize
was in ALL and in none of generated.rs's case! invocations, so the
size-then-resize order -- which the enum's own comment argues is not the
same test as the other order -- was never checked by cargo test. The
tests and the list of which cases have one come from one macro invocation,
and a case missing from it now fails a test.
Tripped a rule and left as it stands
Spanreads every child's cross length throughplace_at(..).len(!axis)even wherehas_exact_size(!axis)makes it moot. The read looks like an unwanted dependency, butdepend_ononly matters for a child that is not inchildren, which is howundrawkeeps a measured-then-dropped child reachable. For a placed child it does nothing.PixelRegion::containsis inclusive at both ends, so two adjacent widgets both claim the boundary step. Senses on one layer never block each other, so both receiving it is what the design says.CursorData::senseis meaningless untilshould_runfills it, which the code says in place and proposes a prepare stage for. A real unrepresentable-state finding, but it is the event API's shape rather than this branch's.Wrapperwith no child answersSize::default(), which isLEFTOVER. It reads as "nothing" but matchesimpl Widget for (), whose comment says a gap takes the default length so a span gives it a share.ALLintests/scenario/mod.rsis still a hand-kept list of everyCase;Case::name's match is the compiler-checked one. A variant left out ofALLis invisible to the shrinker's--caseselection too.
Naming and logic sweep (2026-09-19)
Settled with Bryan across one session, on the branch past 58ce74d. Nothing
here changed what layout computes: the cold dump is byte-identical to
58ce74d at every commit.
How a description is said. A PlaceDescAxis is built by chaining off the
value that says it -- UiSpan::within_desc/shifted_desc, Len::as_desc --
never by a constructor naming the type, because a constructor sends the
reader back to the start of the line. The _desc suffix is what says which
type comes out. PlaceDescAxis::on_axis(axis) lifts one axis into a pair
with the whole box across it; on alone was rejected as contentless and
reserved for events. from_axes is the constructor taking a function, beside
the from_axis taking one axis and two values.
Arithmetic that needed a comment became a name. UiSpan::place was the
aligned-placement rule written out three times; LayoutLen::without_leftover
was the sibling apply_leftover never had, at six sites; is_px,
is_only_leftover and declared name field comparisons the surrounding
comments had to translate; Holds::covers was interval containment by hand.
Seven module-level functions became methods on the value each took first.
Every pair is a struct of two per-axis values, read with [axis].
LayoutHolds was four two-element arrays, so none of its own operations
could be written once; it is AxisHolds on x and y, and and, covers
and contains lost their loops. impl_axis_index! gives every pair
Index<Axis>/IndexMut<Axis>, replacing eighteen axis/axis_mut
methods -- const_index keeps them usable in const context. The bare
[Option<LayoutLen>; 2] became Declared of Option<Len>, which makes
"a share is never a declaration" structural rather than two filters and a
comment.
Two findings in the logic, both one mistake. A value computed from other state was being stored as if it were state, and in both cases the visible symptom was something that looked like an off-by-one:
- A
Spancarriedstartas a third accumulator besidefixedandtaken, assigned at three points, when every assignment wasreached(fixed, taken). Both ends of a slot are now read where they are used; the variable and two of the three calls per child go, and the gap added after the last child derives nothing rather than needing to be subtracted. - The measuring loop's
cursoraddedpxandrelby hand where the placing loop below saidfixed += len.without_leftover()-- the same sum, one of them named.
One property that held but nothing guarded. A Scroll's draw writes
amt and snap_end, so a second draw at another viewport reads what the
first wrote. Warm matches cold only because re-clamping is idempotent and
monotone. The seed scans build Scrolls and never scroll one, so this was
untested; a_scrolled_view_resized_lands_where_a_cold_layout_puts_it scrolls
four distances, one past the end, then widens. It passes.
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_innersaved the old parent only after a redraw replacedActiveData. 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:
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 fills.
Recomputing the cursor in that branch fixes it, and the tail lands at
(220, 0)..(400, 100):
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
Everything about naming is done: the unswept extent, the two same-typed
boxes on ActiveData and Painter's four holds accumulators in 5642f20,
Part::All in aeb60e5, and Place/Part themselves in 58ce74d. The
settled vocabulary and the ask API are in docs/LAYOUT.md.
The clarity sweep of 3da1c71 and 7e2b4cd closed the first three items
that stood here. Both are cold-dump identical to 6c84b6f, so none of it
moved a box, and all three seed scans passed on 7e2b4cd (400 at depth 5 in
66.25s, 1,000 at depth 6 in 188.34s, 2,000 at depth 4 in 300.80s) because
reposition and redepth were restructured on the retained path:
Answer{size, holds} andDrawn{answer, drawing_holds} replace(Size, LayoutHolds)and the three-tuple with twoLayoutHoldsin it.try_reuseanswersboolrather thanOption<()>.Span::alongisSpan::slot;farisrow,sharesishas_roombeside a namedany_leftover, andreachedguards on the weight it divides by rather than on the numerator.DrawInfo::pxwas the rel base in pixels, and all three readers printed it as the box the widget drew in. Removed; each readsregion.to_px(window).Placing::windowexisted only to feed it.diag::outsideholds the 23 counter lines that were insidetry_reuse.ActiveData::is_region_nodereplaces four copies ofmove_idx != parent_move;Axis::BOTHreplacesAXESin three modules;Len::rel_min,rel_maxand the unusedselect_lenare gone.
1ebd4d3 then closed the PlaceSpan item. PlaceSpan and RelBase are
pub and ui/mod.rs re-exports place by name rather than by glob, the way
it already did for painter, so the six pub(crate) accessors
(stated_rel_base, narrows_rel_base, within_span, is_sized,
does_fill, with_rel_base) are gone and in_parent matches
(at.span, declared). !at.is_sized() was dead: PlaceSpan::Sized is built
only by Len::as_desc, which sets RelBase::Len(self) in the same literal,
and deleting with_rel_base removes the only writer that could have
separated them. Visibility here is plain pub plus a named re-export
wherever the path can be hidden (Bryan, 2026-09-19); pub(super) is for
inherent methods on types the crate exports, where it cannot.
What is left, none of it urgent:
widget_atdoes 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.DrawInfoandActiveDataboth carryplacedandasked, twoPlaceDescfields distinguished only by position in every literal. They are genuinely different and documented, but the names are past participles with no operand; a rename is Bryan's vocabulary call.