Compare commits

...
4 Commits
Author SHA1 Message Date
iris-aiandClaude Opus 5 69ba91588a Run every generated case without the long seed scan
`Case::SizeResize` was in `ALL` and in none of the `case!` invocations, so
a size change followed by a resize -- the order the enum's own comment
argues is not the same test as the other one -- was only ever checked by
the ignored long run. The tests and the list of which cases have one come
from one macro invocation now, and a case missing from it fails a test
rather than going quiet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-20 00:20:23 -04:00
iris-aiandClaude Opus 5 8088a1fa59 Say why a reuse was refused, on every path that refuses one
Three of the eight rejections in `try_reuse` were invisible or half-visible
to the diagnostics: a changed inherited mask counted nothing and traced
nothing, an undrawn record traced without counting, and a changed
region-node choice counted without tracing. The mask one is the rejection
this branch's repair was about, so "why did that redraw?" was exactly the
question the rig could not answer.

Adding a counter meant editing a variant list and a name list at the same
index, which renames every total after a slip and says nothing. The two
lists are one declaration now.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-20 00:20:23 -04:00
iris-aiandClaude Opus 5 713e3e747b Judge a kept contract against the box it was asked in
A local redraw keeps the narrower guarantee its parent holds when the new
drawing covers it, so widening and narrowing back do not churn the parent.
It checked that guarantee against `placement`, where the answer put the
drawing, rather than `region`, the box the drawing was made in and the box
both contracts are about. The two differ on every axis a widget reported
less than it was offered, so any such widget escalated to its parent every
time its contract widened -- which is the churn the retention exists to
avoid. `resize` and `try_reuse` both already ask about `region`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-20 00:18:18 -04:00
iris-aiandClaude Opus 5 aea0387567 Stop keeping what nothing reads back
`ActiveData::size_deps` was written on every draw, cleared on every undraw,
and read nowhere: a `Vec<WidgetId>` per active widget for a list only the
`Painter`'s own copy is used from, in `draw_at`, before the record is built.
What it looked like it was for -- reaching a widget whose size was read --
is already done there, by recording whoever asked about a child it did not
draw.

`SizeRule::apply` had no caller and would have been wrong if it found one:
it answers with the rule's own length, where `draw_at` resolves a fraction
against the rel base first. One rule, applied in one place.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-20 00:18:18 -04:00
6 changed files with 178 additions and 153 deletions

No files matched your search

+62 -90
View File
@@ -23,100 +23,70 @@ use std::{
time::Instant,
};
#[derive(Clone, Copy)]
pub(crate) enum Counter {
Updates,
DrawRequests,
WidgetDraws,
RegionNodeDraws,
SizeReads,
HintHits,
HintMisses,
ReuseAttempts,
ReuseExact,
ReuseMoved,
ReuseDirty,
ReuseWrongParent,
ReuseRemapped,
ReuseOutside,
ReuseWrongLayer,
ReuseWrongNode,
QueuePops,
DepthReads,
LocalRedraws,
SizeChanges,
ReaderEdges,
PrimitiveWrites,
TextRenders,
TextShapeHits,
TextShapes,
TextBreaks,
GlyphPlacements,
OutsidePinnedLen,
OutsideRelBase,
OutsideRegion,
/// Declares a counter or timer kind beside the name its report prints. Two
/// lists in the same order was one list too many: a variant inserted without
/// its label moving with it renames every total after it, and nothing says
/// so.
macro_rules! labelled {
($(#[$meta:meta])* $vis:vis enum $Name:ident { $($variant:ident = $label:literal,)* }) => {
$(#[$meta])*
#[derive(Clone, Copy)]
$vis enum $Name { $($variant,)* }
impl $Name {
const COUNT: usize = [$($label,)*].len();
const NAMES: [&'static str; Self::COUNT] = [$($label,)*];
}
};
}
impl Counter {
const COUNT: usize = Self::OutsideRegion as usize + 1;
const NAMES: [&'static str; Self::COUNT] = [
"updates",
"draw requests",
"widget draws",
"region-node draws",
"draw-result size reads",
"hint hits",
"hint misses",
"reuse attempts",
"reuse exact",
"reuse moved",
"reuse: dirty",
"reuse: wrong parent",
"reuse remapped",
"reuse: outside what it holds for",
"reuse: another layer",
"reuse: region-node choice changed",
"redraw queue pops",
"depth reads",
"local redraws",
"size changes",
"reader edges",
"primitive writes",
"text renders",
"text shape hits",
"text shapes",
"text line breaks",
"glyph placements",
"reuse outside: the length it was pinned to",
"reuse outside: a rel base",
"reuse outside: a region length",
];
labelled! {
pub(crate) enum Counter {
Updates = "updates",
DrawRequests = "draw requests",
WidgetDraws = "widget draws",
RegionNodeDraws = "region-node draws",
SizeReads = "draw-result size reads",
HintHits = "hint hits",
HintMisses = "hint misses",
ReuseAttempts = "reuse attempts",
ReuseExact = "reuse exact",
ReuseMoved = "reuse moved",
ReuseDirty = "reuse: dirty",
ReuseUndrawn = "reuse: nothing drawn to keep",
ReuseWrongParent = "reuse: wrong parent",
ReuseRemapped = "reuse remapped",
ReuseOutside = "reuse: outside what it holds for",
ReuseWrongLayer = "reuse: another layer",
ReuseWrongNode = "reuse: region-node choice changed",
ReuseWrongMask = "reuse: a different inherited mask",
QueuePops = "redraw queue pops",
DepthReads = "depth reads",
LocalRedraws = "local redraws",
SizeChanges = "size changes",
ReaderEdges = "reader edges",
PrimitiveWrites = "primitive writes",
TextRenders = "text renders",
TextShapeHits = "text shape hits",
TextShapes = "text shapes",
TextBreaks = "text line breaks",
GlyphPlacements = "glyph placements",
OutsidePinnedLen = "reuse outside: the length it was pinned to",
OutsideRelBase = "reuse outside: a rel base",
OutsideRegion = "reuse outside: a region length",
}
}
#[derive(Clone, Copy)]
pub(crate) enum TimerKind {
Update,
FullLayout,
IncrementalLayout,
TextRender,
TextShape,
TextBreak,
GlyphPlacement,
}
impl TimerKind {
const COUNT: usize = Self::GlyphPlacement as usize + 1;
const NAMES: [&'static str; Self::COUNT] = [
"update total",
"full layout",
"incremental layout",
"text render",
"text shape",
"text line break",
"glyph placement",
];
labelled! {
pub(crate) enum TimerKind {
Update = "update total",
FullLayout = "full layout",
IncrementalLayout = "incremental layout",
TextRender = "text render",
TextShape = "text shape",
TextBreak = "text line break",
GlyphPlacement = "glyph placement",
}
}
#[derive(Clone)]
@@ -251,6 +221,8 @@ pub enum ReuseOutcome {
Dirty,
WrongParent,
WrongLayer,
WrongMask,
WrongNode,
Remapped,
Outside,
Undrawn,
-2
View File
@@ -51,8 +51,6 @@ pub struct ActiveData {
/// An owned mask holds one reference independently of its primitives.
pub mask_region: Option<UiRegion>,
pub children: Vec<WidgetId>,
/// The children whose size this widget read while drawing.
pub size_deps: Vec<WidgetId>,
/// The movable region its primitives are positioned through: its own when
/// opted in, otherwise the nearest ancestor's.
pub move_idx: MoveIdx,
+20 -6
View File
@@ -489,7 +489,6 @@ impl UiRenderState {
primitives,
mask_region,
children,
size_deps,
declared: rsc.widgets().declared_lens(id),
own_align: rsc.widgets().alignment(id),
move_idx,
@@ -570,12 +569,18 @@ impl UiRenderState {
};
if !active.drawn {
#[cfg(feature = "layout-diagnostics")]
diag::reuse(id, ReuseOutcome::Undrawn);
{
diag::bump(Counter::ReuseUndrawn);
diag::reuse(id, ReuseOutcome::Undrawn);
}
return false;
}
if active.is_region_node() != info.region_node {
#[cfg(feature = "layout-diagnostics")]
diag::bump(Counter::ReuseWrongNode);
{
diag::bump(Counter::ReuseWrongNode);
diag::reuse(id, ReuseOutcome::WrongNode);
}
return false;
}
// Drawn on another layer: the drawing sits in that layer's list and
@@ -591,7 +596,15 @@ impl UiRenderState {
}
return false;
}
// Its primitives name the mask it inherited, and a masking parent
// that redrew pushed another: keeping them would clip them by one
// nothing updates again.
if active.parent_mask != info.mask {
#[cfg(feature = "layout-diagnostics")]
{
diag::bump(Counter::ReuseWrongMask);
diag::reuse(id, ReuseOutcome::WrongMask);
}
return false;
}
// Drawn somewhere else in the tree: its box is in coordinates it no
@@ -800,7 +813,6 @@ impl UiRenderState {
}
// After the descendants, whose slots name this one as their parent.
self.drop_slot(id);
active.size_deps.clear();
active.drawn = false;
self.active.insert(id, active);
}
@@ -844,7 +856,6 @@ impl UiRenderState {
primitives: Vec::new(),
mask_region: None,
children: Vec::new(),
size_deps: Vec::new(),
move_idx: info.parent_move,
declared: Declared::NONE,
own_align: rsc.widgets().alignment(id),
@@ -1105,8 +1116,11 @@ impl UiRenderState {
{
active.answer = was_answer;
}
// Against the box it was asked in, which is what both contracts are
// about. Where the answer put the drawing is shorter than that
// wherever the widget reported less than it was offered.
if active.holds.covers(was_holds)
&& was_holds.contains(self.output_size, active.rel_base, active.placement)
&& was_holds.contains(self.output_size, active.rel_base, active.region)
{
active.holds = was_holds;
}
-8
View File
@@ -36,14 +36,6 @@ impl SizeRule {
Self::Exact(len) => Some(*len),
}
}
/// The length a widget reporting `reported` ends up with.
pub fn apply(&self, reported: LayoutLen) -> LayoutLen {
match self {
Self::Free => reported,
Self::Exact(len) => *len,
}
}
}
impl From<LayoutLen> for SizeRule {
+56
View File
@@ -1477,3 +1477,59 @@ fn a_redrawn_subtree_is_not_undrawn_by_the_parent_it_left() {
assert_eq!(h.region(&leaf), before);
}
}
/// A leaf that reports less than the box it is given and states which lengths
/// of that box its drawing holds for, so a test can widen the contract
/// without changing the answer.
struct Contracted {
holds: std::ops::RangeInclusive<Px>,
size: Size,
}
impl Widget for Contracted {
fn draw(&mut self, painter: &mut Painter) -> Size {
painter.holds(Axis::X, self.holds.clone());
self.size
}
}
struct CountedParent {
inner: StrongWidget,
draws: Rc<Cell<usize>>,
}
impl Widget for CountedParent {
fn draw(&mut self, painter: &mut Painter) -> Size {
self.draws.set(self.draws.get() + 1);
painter.widget(&self.inner).size()
}
}
#[test]
fn widening_what_a_drawing_holds_for_does_not_relay_out_the_parent() {
let mut h = Harness::new((400, 200));
let child = Contracted {
holds: Px::from_int(300)..=Px::from_int(500),
size: Size::from((100, 200)),
}
.add(&mut h.rsc);
let draws = Rc::new(Cell::new(0));
let root = CountedParent {
inner: child.upgrade(&mut h.rsc),
draws: draws.clone(),
}
.add(&mut h.rsc);
h.set_root(root);
let settled = draws.get();
// The same answer, good for more boxes than before, so the guarantee the
// parent kept still holds.
h.rsc[child].holds = Px::from_int(200)..=Px::from_int(600);
h.frame();
assert_eq!(
draws.get(),
settled,
"a wider contract for the same answer is not a change to lay out"
);
}
+40 -47
View File
@@ -47,57 +47,50 @@ fn check_plan(grown: &Plan, seed: u64, depth: usize, case: Case) {
}
}
macro_rules! case {
($name:ident, $case:expr) => {
#[test]
fn $name() {
for seed in SEEDS {
check(seed, depth(), $case);
/// A test per case, and the list of which cases have one, from the same
/// place. A case the ordinary suite leaves out runs only in the long scan,
/// which nobody runs by hand.
macro_rules! cases {
($($name:ident = $case:expr,)*) => {
$(
#[test]
fn $name() {
for seed in SEEDS {
check(seed, depth(), $case);
}
}
}
)*
const NAMED: [Case; [$($case,)*].len()] = [$($case,)*];
};
}
case!(
many_widgets_redrawing_at_once_leaves_every_box_where_it_was,
Case::RepaintSome
);
case!(
everything_redrawing_at_once_leaves_every_box_where_it_was,
Case::Repaint
);
case!(
a_resize_lands_where_starting_at_that_size_would,
Case::Resize
);
case!(
a_resize_and_a_repaint_land_where_starting_that_way_would,
Case::ResizeRepaint
);
case!(
a_size_change_after_a_resize_lands_the_same_way,
Case::ResizeSize
);
case!(
a_size_change_lands_where_growing_it_that_way_would,
Case::Size
);
case!(
every_size_changing_at_once_lands_where_growing_it_that_way_would,
Case::EverySize
);
case!(
an_alignment_change_lands_where_growing_it_that_way_would,
Case::Align
);
case!(
giving_and_taking_a_movable_region_rebuilds_what_resolves_it,
Case::RegionNode
);
case!(
reordering_a_span_lands_where_growing_it_that_way_would,
Case::Reorder
);
cases! {
many_widgets_redrawing_at_once_leaves_every_box_where_it_was = Case::RepaintSome,
everything_redrawing_at_once_leaves_every_box_where_it_was = Case::Repaint,
a_resize_lands_where_starting_at_that_size_would = Case::Resize,
a_resize_and_a_repaint_land_where_starting_that_way_would = Case::ResizeRepaint,
a_size_change_after_a_resize_lands_the_same_way = Case::ResizeSize,
a_resize_after_a_size_change_lands_the_same_way = Case::SizeResize,
a_size_change_lands_where_growing_it_that_way_would = Case::Size,
every_size_changing_at_once_lands_where_growing_it_that_way_would = Case::EverySize,
an_alignment_change_lands_where_growing_it_that_way_would = Case::Align,
giving_and_taking_a_movable_region_rebuilds_what_resolves_it = Case::RegionNode,
reordering_a_span_lands_where_growing_it_that_way_would = Case::Reorder,
}
/// The shuffles are one test between them, so they are the only cases `ALL`
/// may hold without a test of their own.
#[test]
fn every_case_runs_without_the_long_scan() {
for case in ALL {
assert!(
NAMED.contains(&case) || matches!(case, Case::Shuffle(_)),
"{} runs only in the long seed scan; give it a case here",
case.name()
);
}
}
#[test]
fn adding_and_removing_span_children_lands_where_growing_it_that_way_would() {