From 7e2b4cd9db8b94f769b5504c8a532d329a1afbca Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Sat, 19 Sep 2026 23:28:36 -0400 Subject: [PATCH] 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. --- core/src/layout_diagnostics.rs | 41 ++++++++++++++++++++++++--- core/src/ui/painter.rs | 3 -- core/src/ui/render_state.rs | 52 ++++++++-------------------------- tests/trace_unsettled.rs | 4 +-- 4 files changed, 51 insertions(+), 49 deletions(-) diff --git a/core/src/layout_diagnostics.rs b/core/src/layout_diagnostics.rs index b5f4daf..71894c3 100644 --- a/core/src/layout_diagnostics.rs +++ b/core/src/layout_diagnostics.rs @@ -15,7 +15,7 @@ //! reuse, size, placement, and text events for one suspicious widget. The //! 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::{ cell::RefCell, collections::{HashMap, HashSet}, @@ -264,7 +264,7 @@ pub enum TraceEvent { id: WidgetId, parent: Option, region: UiRegion, - pixel_size: PxVec2, + region_px: PxVec2, region_node: bool, }, Reuse { @@ -360,7 +360,7 @@ pub(crate) fn draw_request( id: WidgetId, parent: Option, region: UiRegion, - pixel_size: PxVec2, + region_px: PxVec2, region_node: bool, ) { trace( @@ -369,7 +369,7 @@ pub(crate) fn draw_request( id, parent, region, - pixel_size, + region_px, region_node, }, ); @@ -379,6 +379,39 @@ pub(crate) fn reuse(id: WidgetId, outcome: ReuseOutcome) { 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) { trace(id, TraceEvent::SizeReported { id, size }); } diff --git a/core/src/ui/painter.rs b/core/src/ui/painter.rs index 39e8108..6cc23db 100644 --- a/core/src/ui/painter.rs +++ b/core/src/ui/painter.rs @@ -196,7 +196,6 @@ impl<'a> Painter<'a> { if !re_asked { self.children.push(id.id()); } - let px = rel_base.to_px(self.window); let drawn = self.state.draw_inner( id.id(), DrawInfo { @@ -211,7 +210,6 @@ impl<'a> Painter<'a> { placed: place, asked: place, re_asked, - px, }, None, self.rsc, @@ -280,7 +278,6 @@ impl<'a> Painter<'a> { id: self.id, region: self.region, rel_base: self.rel_base, - window: self.window, depth: self.depth, move_idx: self.move_idx, mask: self.mask, diff --git a/core/src/ui/render_state.rs b/core/src/ui/render_state.rs index a34f01f..d84a82d 100644 --- a/core/src/ui/render_state.rs +++ b/core/src/ui/render_state.rs @@ -30,8 +30,6 @@ pub(super) struct DrawInfo { pub asked: PlaceDesc, /// Whether the parent already asked about this widget in this draw. 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 @@ -48,7 +46,6 @@ pub(super) struct Placing { pub id: WidgetId, pub region: UiRegion, pub rel_base: UiVec2, - pub window: PxVec2, pub depth: usize, pub move_idx: MoveIdx, pub mask: MaskIdx, @@ -130,7 +127,6 @@ impl UiRenderState { /// 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. fn root_info(&self, rel_base: UiVec2, region: UiRegion) -> DrawInfo { - let px = rel_base.to_px(self.output_size); DrawInfo { layer: 0, parent: None, @@ -143,7 +139,6 @@ impl UiRenderState { placed: PlaceDesc::WHOLE, asked: PlaceDesc::WHOLE, re_asked: false, - px, } } @@ -222,7 +217,13 @@ impl UiRenderState { #[cfg(feature = "layout-diagnostics")] { 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 declared = rsc.widgets().declared_lens(id); @@ -312,8 +313,6 @@ impl UiRenderState { .and_then(|old| old.mask_region.map(|_| old.mask)); let old_children = old.map_or_else(Vec::new, |old| old.children); rsc.widgets_mut().needs_redraw.remove(&id); - let px = info.px; - let window = self.output_size; let mut painter = Painter { state: self, @@ -409,8 +408,9 @@ impl UiRenderState { self.output_size, axis )), - "'{}' ({id:?}) clips to {px:?} and reports {size}", + "'{}' ({id:?}) clips to {} and reports {size}", rsc.widgets().label(id), + region.to_px(window), ); for c in &old_children { if !children.contains(c) { @@ -440,8 +440,9 @@ impl UiRenderState { .fold(answer_holds, |holds, (_, child)| holds.and(child)); debug_assert!( 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), + region.to_px(window), ); // 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 @@ -462,7 +463,6 @@ impl UiRenderState { placed: PlaceDesc::WHOLE, asked: PlaceDesc::WHOLE, re_asked: false, - px, }, rsc, ); @@ -612,31 +612,7 @@ impl UiRenderState { .contains(self.output_size, info.rel_base, region) { #[cfg(feature = "layout-diagnostics")] - { - // 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); - } + diag::outside(id, active.holds, region, info.rel_base, self.output_size); return false; } self.relocate(id, placed, info, rsc); @@ -718,7 +694,6 @@ impl UiRenderState { placed: place, asked: active.asked, re_asked: active.re_asked, - px: rel_base.to_px(at.window), }; self.relocate(child, placed, info, rsc); } @@ -749,7 +724,6 @@ impl UiRenderState { id, region: placed, rel_base: info.rel_base, - window: self.output_size, depth: info.depth, move_idx: active.move_idx, mask: active.mask, @@ -1116,7 +1090,6 @@ impl UiRenderState { placed: active.asked, asked: active.asked, re_asked: false, - px: rel_base.to_px(self.output_size), }; #[cfg(feature = "layout-diagnostics")] diag::bump(Counter::LocalRedraws); @@ -1165,7 +1138,6 @@ impl UiRenderState { id, region, rel_base: active.rel_base, - window: self.output_size, depth: active.depth, move_idx: active.move_idx, mask: active.mask, diff --git a/tests/trace_unsettled.rs b/tests/trace_unsettled.rs index 2d441be..240f4ff 100644 --- a/tests/trace_unsettled.rs +++ b/tests/trace_unsettled.rs @@ -42,12 +42,12 @@ fn dump(label: &str, report: &diag::Report, text: WidgetId) { TraceEvent::DrawRequest { id, region, - pixel_size, + region_px, .. } if *id == text => { println!( " 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 => {