Print the box a widget drew in, not the rel base labelled as it

`DrawInfo::px` was the child's rel base in pixels, resolved at all four
construction sites on every draw and read only by three diagnostics --
each of which called it the box: `diag::draw_request`'s `pixel_size`,
printed by `trace_unsettled` as "draw in"; and two `debug_assert`
messages saying "clips to" and "drew in". A rel base and a box differ
wherever a parent hands down part of its own, which is every child of a
span, so all three said something that was not true.

The field is gone and each site reads `region.to_px(window)`, which is
the box it claimed to be printing and costs nothing outside a failing
assert. The trace field is `region_px`. `Placing::window` existed only
to resolve that value and follows it out.

The 23-line counter block inside `try_reuse`'s "outside its range"
branch is `diag::outside`, beside the other diagnostics, so the decision
reads as its six checks.

fmt, workspace clippy under `-D warnings` with and without
`layout-diagnostics`, and the workspace tests under both are clean. The
cold dump over 400 depth-5 trees is byte-identical: 34,492 boxes. The
trace now prints "draw in 189.00x176.00" beside a region 189 by 176.
This commit is contained in:
iris-ai committed 2026-09-19 23:28:36 -04:00
1 parent 3da1c71870
commit 7e2b4cd9db
4 files changed
+51 -49

No files matched your search

+37 -4
View File
@@ -15,7 +15,7 @@
//! reuse, size, placement, and text events for one suspicious widget. The //! reuse, size, placement, and text events for one suspicious widget. The
//! selection is a set and survives [`take`] until cleared. //! selection is a set and survives [`take`] until cleared.
use crate::{Axis, LayoutLen, PxVec2, Size, UiRegion, WidgetId}; use crate::{Axis, LayoutHolds, LayoutLen, PxVec2, Size, UiRegion, UiVec2, WidgetId};
use std::{ use std::{
cell::RefCell, cell::RefCell,
collections::{HashMap, HashSet}, collections::{HashMap, HashSet},
@@ -264,7 +264,7 @@ pub enum TraceEvent {
id: WidgetId, id: WidgetId,
parent: Option<WidgetId>, parent: Option<WidgetId>,
region: UiRegion, region: UiRegion,
pixel_size: PxVec2, region_px: PxVec2,
region_node: bool, region_node: bool,
}, },
Reuse { Reuse {
@@ -360,7 +360,7 @@ pub(crate) fn draw_request(
id: WidgetId, id: WidgetId,
parent: Option<WidgetId>, parent: Option<WidgetId>,
region: UiRegion, region: UiRegion,
pixel_size: PxVec2, region_px: PxVec2,
region_node: bool, region_node: bool,
) { ) {
trace( trace(
@@ -369,7 +369,7 @@ pub(crate) fn draw_request(
id, id,
parent, parent,
region, region,
pixel_size, region_px,
region_node, region_node,
}, },
); );
@@ -379,6 +379,39 @@ pub(crate) fn reuse(id: WidgetId, outcome: ReuseOutcome) {
trace(id, TraceEvent::Reuse { id, outcome }); trace(id, TraceEvent::Reuse { id, outcome });
} }
/// A drawing that cannot be reused because the box on offer is outside what
/// it holds for, and which of the three contracts said so. They overlap: a
/// drawing can be outside two of them at once, and counting each is what
/// says where a rel base redrawing more than it should is coming from.
pub(crate) fn outside(
id: WidgetId,
holds: LayoutHolds,
region: UiRegion,
rel_base: UiVec2,
window: PxVec2,
) {
for axis in Axis::BOTH {
let holds = holds[axis];
let len = region[axis].len();
let window = window[axis];
if holds.region_len.is_some_and(|pinned| pinned != len) {
bump(Counter::OutsidePinnedLen);
}
if !holds.window.contains(window)
|| holds
.rel_base
.is_some_and(|pinned| pinned != rel_base[axis])
{
bump(Counter::OutsideRelBase);
}
if !holds.region.contains(len.to_px(window)) {
bump(Counter::OutsideRegion);
}
}
bump(Counter::ReuseOutside);
reuse(id, ReuseOutcome::Outside);
}
pub(crate) fn size_reported(id: WidgetId, size: Size) { pub(crate) fn size_reported(id: WidgetId, size: Size) {
trace(id, TraceEvent::SizeReported { id, size }); trace(id, TraceEvent::SizeReported { id, size });
} }
-3
View File
@@ -196,7 +196,6 @@ impl<'a> Painter<'a> {
if !re_asked { if !re_asked {
self.children.push(id.id()); self.children.push(id.id());
} }
let px = rel_base.to_px(self.window);
let drawn = self.state.draw_inner( let drawn = self.state.draw_inner(
id.id(), id.id(),
DrawInfo { DrawInfo {
@@ -211,7 +210,6 @@ impl<'a> Painter<'a> {
placed: place, placed: place,
asked: place, asked: place,
re_asked, re_asked,
px,
}, },
None, None,
self.rsc, self.rsc,
@@ -280,7 +278,6 @@ impl<'a> Painter<'a> {
id: self.id, id: self.id,
region: self.region, region: self.region,
rel_base: self.rel_base, rel_base: self.rel_base,
window: self.window,
depth: self.depth, depth: self.depth,
move_idx: self.move_idx, move_idx: self.move_idx,
mask: self.mask, mask: self.mask,
+12 -40
View File
@@ -30,8 +30,6 @@ pub(super) struct DrawInfo {
pub asked: PlaceDesc, pub asked: PlaceDesc,
/// Whether the parent already asked about this widget in this draw. /// Whether the parent already asked about this widget in this draw.
pub re_asked: bool, pub re_asked: bool,
/// The rel base in pixels, resolved once against the window.
pub px: PxVec2,
} }
/// What one draw of a widget came to: the answer it gave, and the boxes and /// What one draw of a widget came to: the answer it gave, and the boxes and
@@ -48,7 +46,6 @@ pub(super) struct Placing {
pub id: WidgetId, pub id: WidgetId,
pub region: UiRegion, pub region: UiRegion,
pub rel_base: UiVec2, pub rel_base: UiVec2,
pub window: PxVec2,
pub depth: usize, pub depth: usize,
pub move_idx: MoveIdx, pub move_idx: MoveIdx,
pub mask: MaskIdx, pub mask: MaskIdx,
@@ -130,7 +127,6 @@ impl UiRenderState {
/// The root is asked about in the output. Its own rules narrow both its /// The root is asked about in the output. Its own rules narrow both its
/// rel base and box; nothing above it chose a different one. /// rel base and box; nothing above it chose a different one.
fn root_info(&self, rel_base: UiVec2, region: UiRegion) -> DrawInfo { fn root_info(&self, rel_base: UiVec2, region: UiRegion) -> DrawInfo {
let px = rel_base.to_px(self.output_size);
DrawInfo { DrawInfo {
layer: 0, layer: 0,
parent: None, parent: None,
@@ -143,7 +139,6 @@ impl UiRenderState {
placed: PlaceDesc::WHOLE, placed: PlaceDesc::WHOLE,
asked: PlaceDesc::WHOLE, asked: PlaceDesc::WHOLE,
re_asked: false, re_asked: false,
px,
} }
} }
@@ -222,7 +217,13 @@ impl UiRenderState {
#[cfg(feature = "layout-diagnostics")] #[cfg(feature = "layout-diagnostics")]
{ {
diag::bump(Counter::DrawRequests); diag::bump(Counter::DrawRequests);
diag::draw_request(id, info.parent, region, info.px, info.region_node); diag::draw_request(
id,
info.parent,
region,
region.to_px(self.output_size).size(),
info.region_node,
);
} }
let align = rsc.widgets().alignment(id); let align = rsc.widgets().alignment(id);
let declared = rsc.widgets().declared_lens(id); let declared = rsc.widgets().declared_lens(id);
@@ -312,8 +313,6 @@ impl UiRenderState {
.and_then(|old| old.mask_region.map(|_| old.mask)); .and_then(|old| old.mask_region.map(|_| old.mask));
let old_children = old.map_or_else(Vec::new, |old| old.children); let old_children = old.map_or_else(Vec::new, |old| old.children);
rsc.widgets_mut().needs_redraw.remove(&id); rsc.widgets_mut().needs_redraw.remove(&id);
let px = info.px;
let window = self.output_size; let window = self.output_size;
let mut painter = Painter { let mut painter = Painter {
state: self, state: self,
@@ -409,8 +408,9 @@ impl UiRenderState {
self.output_size, self.output_size,
axis axis
)), )),
"'{}' ({id:?}) clips to {px:?} and reports {size}", "'{}' ({id:?}) clips to {} and reports {size}",
rsc.widgets().label(id), rsc.widgets().label(id),
region.to_px(window),
); );
for c in &old_children { for c in &old_children {
if !children.contains(c) { if !children.contains(c) {
@@ -440,8 +440,9 @@ impl UiRenderState {
.fold(answer_holds, |holds, (_, child)| holds.and(child)); .fold(answer_holds, |holds, (_, child)| holds.and(child));
debug_assert!( debug_assert!(
holds.contains(self.output_size, info.rel_base, region), holds.contains(self.output_size, info.rel_base, region),
"'{}' ({id:?}) drew in {px:?}, outside the ranges it reported: {holds:?}", "'{}' ({id:?}) drew in {}, outside the ranges it reported: {holds:?}",
rsc.widgets().label(id), rsc.widgets().label(id),
region.to_px(window),
); );
// What it asked about and did not draw is still something it asked, // What it asked about and did not draw is still something it asked,
// and a change there has to reach it. Asking answered whatever mark // and a change there has to reach it. Asking answered whatever mark
@@ -462,7 +463,6 @@ impl UiRenderState {
placed: PlaceDesc::WHOLE, placed: PlaceDesc::WHOLE,
asked: PlaceDesc::WHOLE, asked: PlaceDesc::WHOLE,
re_asked: false, re_asked: false,
px,
}, },
rsc, rsc,
); );
@@ -612,31 +612,7 @@ impl UiRenderState {
.contains(self.output_size, info.rel_base, region) .contains(self.output_size, info.rel_base, region)
{ {
#[cfg(feature = "layout-diagnostics")] #[cfg(feature = "layout-diagnostics")]
{ diag::outside(id, active.holds, region, info.rel_base, self.output_size);
// Which of the three said no, so a rel base that redraws more
// than it should says where to look. They overlap: a drawing
// can be outside two of them at once.
for axis in Axis::BOTH {
let holds = active.holds[axis];
let len = region[axis].len();
let window = self.output_size[axis];
if holds.region_len.is_some_and(|pinned| pinned != len) {
diag::bump(Counter::OutsidePinnedLen);
}
if !holds.window.contains(window)
|| holds
.rel_base
.is_some_and(|pinned| pinned != info.rel_base[axis])
{
diag::bump(Counter::OutsideRelBase);
}
if !holds.region.contains(len.to_px(window)) {
diag::bump(Counter::OutsideRegion);
}
}
diag::bump(Counter::ReuseOutside);
diag::reuse(id, ReuseOutcome::Outside);
}
return false; return false;
} }
self.relocate(id, placed, info, rsc); self.relocate(id, placed, info, rsc);
@@ -718,7 +694,6 @@ impl UiRenderState {
placed: place, placed: place,
asked: active.asked, asked: active.asked,
re_asked: active.re_asked, re_asked: active.re_asked,
px: rel_base.to_px(at.window),
}; };
self.relocate(child, placed, info, rsc); self.relocate(child, placed, info, rsc);
} }
@@ -749,7 +724,6 @@ impl UiRenderState {
id, id,
region: placed, region: placed,
rel_base: info.rel_base, rel_base: info.rel_base,
window: self.output_size,
depth: info.depth, depth: info.depth,
move_idx: active.move_idx, move_idx: active.move_idx,
mask: active.mask, mask: active.mask,
@@ -1116,7 +1090,6 @@ impl UiRenderState {
placed: active.asked, placed: active.asked,
asked: active.asked, asked: active.asked,
re_asked: false, re_asked: false,
px: rel_base.to_px(self.output_size),
}; };
#[cfg(feature = "layout-diagnostics")] #[cfg(feature = "layout-diagnostics")]
diag::bump(Counter::LocalRedraws); diag::bump(Counter::LocalRedraws);
@@ -1165,7 +1138,6 @@ impl UiRenderState {
id, id,
region, region,
rel_base: active.rel_base, rel_base: active.rel_base,
window: self.output_size,
depth: active.depth, depth: active.depth,
move_idx: active.move_idx, move_idx: active.move_idx,
mask: active.mask, mask: active.mask,
+2 -2
View File
@@ -42,12 +42,12 @@ fn dump(label: &str, report: &diag::Report, text: WidgetId) {
TraceEvent::DrawRequest { TraceEvent::DrawRequest {
id, id,
region, region,
pixel_size, region_px,
.. ..
} if *id == text => { } if *id == text => {
println!( println!(
" draw in {:.2}x{:.2} region {region:?}", " draw in {:.2}x{:.2} region {region:?}",
pixel_size.x, pixel_size.y region_px.x, region_px.y
) )
} }
TraceEvent::SizeReported { id, size } if *id == text => { TraceEvent::SizeReported { id, size } if *id == text => {