Carry the composed box down the draw, rather than walking back up for it
Every widget that reads its box in pixels was making `Moves` compose its slot's chain again, a mean of 2.8 levels, about eight hundred times a frame. A draw already descends past every one of those entries on its way in, so `DrawInfo` carries what the slot composes to and `draw_at` steps it one box further -- which is a select where it was a walk. `Moves::size_of` and `compose` are left for `redraw`, which starts mid-tree with nothing above it in flight. Measured on the fixed-shape fixture, seed 1 depth 8, 500 frames of `many`, medians of 25 runs, twenty-five work counters identical throughout: | | instructions | cycles | | --- | ---: | ---: | | `d21a215`, before exact composition | 1,908M | 760M | | `45a7176`, composing on the fine grid | 1,880M | 755M | | this | **1,840M** | **735M** | So exact composition ends up 3.6% fewer instructions and 3.3% fewer cycles than the rounding-per-level walk it replaced, and the widening it needed was paid for twice over by not doing the walk. `Holds::through`'s allowance does not move: two half steps is where shrinker seed 220 pins it, not where the arithmetic does. `Painter` still composes a child's region into its own on the grid before asking for it in pixels, which is the last narrow step in that path; taking it out needs the child's region as its parent stated it, which `draw_inner` is not handed. Checked: fmt, clippy, 83 suite tests, 17 core unit tests, the release oracle at 100 seeds and at 1000 seeds of depth 6, all fifteen shrinker cases at 400 seeds of depth 5, and `tabs`, `view`, `minimal`, `text`, `random` and the tab replay byte-identical at 1920x1200 against `45a7176`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
45a717695b
commit
5f16617511
3 files changed
+92
-12
No files matched your search
@@ -4,7 +4,7 @@ use crate::ui::painter::{declared_box, declared_lens, placed_box};
|
||||
use crate::{
|
||||
ActiveData, Axis, DrawLayers, Holds, IdLike, LayoutLen, Len, MaskIdx, MoveIdx, Moves, Painter,
|
||||
PixelRegion, Px, PxVec2, RegionAlign, Rel, Size, StrongWidget, UiRegion, UiRsc, UiSpan, Weight,
|
||||
WidgetId, Widgets,
|
||||
WideRegion, WidgetId, Widgets,
|
||||
util::{HashMap, Vec2},
|
||||
};
|
||||
|
||||
@@ -24,6 +24,9 @@ pub(super) struct DrawInfo {
|
||||
/// that box in pixels.
|
||||
pub offer: UiRegion,
|
||||
pub offered_px: PxVec2,
|
||||
/// The box `parent_move` composes to, on the fine grid, so a widget's own
|
||||
/// box is one step further and not a walk back up the chain.
|
||||
pub slot_wide: WideRegion,
|
||||
/// A container's answer for where the widget sits. `None` uses the
|
||||
/// widget's own property.
|
||||
pub align: Option<RegionAlign>,
|
||||
@@ -106,6 +109,7 @@ impl UiRenderState {
|
||||
mask: MaskIdx::NONE,
|
||||
offer: UiRegion::FULL,
|
||||
offered_px: self.output_size,
|
||||
slot_wide: self.moves.compose(self.root_move, UiRegion::FULL),
|
||||
align: None,
|
||||
}
|
||||
}
|
||||
@@ -276,29 +280,37 @@ impl UiRenderState {
|
||||
old: Option<ActiveData>,
|
||||
rsc: &mut dyn UiRsc,
|
||||
) -> (Size, [Holds; 2]) {
|
||||
let (move_idx, local, retired_move) = match info.region_node {
|
||||
let (move_idx, local, retired_move, slot_wide) = match info.region_node {
|
||||
// Its box becomes its movable region, so it draws in that
|
||||
// region's coordinates and its box is one entry to rewrite.
|
||||
// region's coordinates and its box is one entry to rewrite --
|
||||
// and that box is what its contents compose through.
|
||||
true => (
|
||||
self.move_slot(id, info.parent_move, region),
|
||||
UiRegion::FULL,
|
||||
None,
|
||||
info.slot_wide.select(®ion),
|
||||
),
|
||||
// Keep the old entry alive until every descendant has migrated.
|
||||
// Reusing its index sooner could make an old parent look current.
|
||||
false => (info.parent_move, region, self.slots.remove(&id)),
|
||||
false => (
|
||||
info.parent_move,
|
||||
region,
|
||||
self.slots.remove(&id),
|
||||
info.slot_wide,
|
||||
),
|
||||
};
|
||||
let (old_children, old_answer) = match old {
|
||||
Some(old) => (old.children, Some(old.answer)),
|
||||
None => (Vec::new(), None),
|
||||
};
|
||||
rsc.widgets_mut().needs_redraw.remove(&id);
|
||||
let px = self.px_of(move_idx, local);
|
||||
let px = slot_wide.select_size(&local).to_px(self.output_size);
|
||||
let at_offer = same_px(px, info.offered_px);
|
||||
|
||||
let mut painter = Painter {
|
||||
state: self,
|
||||
region: local,
|
||||
slot_wide,
|
||||
mask: info.mask,
|
||||
layer: info.layer,
|
||||
own_layer: info.layer,
|
||||
@@ -332,6 +344,7 @@ impl UiRenderState {
|
||||
state: _,
|
||||
rsc: _,
|
||||
region: _,
|
||||
slot_wide: _,
|
||||
mask,
|
||||
textures,
|
||||
primitives,
|
||||
@@ -403,6 +416,7 @@ impl UiRenderState {
|
||||
mask,
|
||||
offer: UiRegion::FULL,
|
||||
offered_px: px,
|
||||
slot_wide,
|
||||
align: None,
|
||||
},
|
||||
rsc,
|
||||
@@ -515,7 +529,7 @@ impl UiRenderState {
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let px = self.px_of(info.parent_move, region);
|
||||
let px = info.slot_wide.select_size(®ion).to_px(self.output_size);
|
||||
let (size, holds) = active.answer;
|
||||
(holds[0].contains(px.x) && holds[1].contains(px.y)).then_some((size, holds))
|
||||
}
|
||||
@@ -616,7 +630,7 @@ impl UiRenderState {
|
||||
// In pixels, because `region` is a fraction of a slot's box and that
|
||||
// box may be what changed -- an unchanged fraction of a box half the
|
||||
// size is half the widget.
|
||||
if !active.holds_at(self.px_of(info.parent_move, region)) {
|
||||
if !active.holds_at(info.slot_wide.select_size(®ion).to_px(self.output_size)) {
|
||||
#[cfg(feature = "layout-diagnostics")]
|
||||
{
|
||||
diag::bump(Counter::ReuseOutside);
|
||||
@@ -985,6 +999,7 @@ impl UiRenderState {
|
||||
mask: active.mask,
|
||||
offer: active.offer,
|
||||
offered_px,
|
||||
slot_wide: self.moves.compose(active.parent_move, UiRegion::FULL),
|
||||
align: active.align_override.then_some(active.align),
|
||||
};
|
||||
let (was_answer, was) = (active.answer, (active.size, active.holds));
|
||||
|
||||
Reference in new issue
Block a user