Decide reuse on the box a widget drew against, in pixels
Two holes the random trees found, both of which kept a wrapping text shaped for a width it no longer had. **A region is a fraction of a slot's box, so an unchanged region is not an unchanged box.** `try_reuse` compared regions, and a child drawn at `UiRegion::FULL` of a slot whose box had just halved compared equal to itself and was reused without being descended into. `ActiveData` now keeps the pixel size of the box it drew against and the comparison is against that, which is the question that was being asked all along and is right through a slot change and an output resize alike. **A size the parent learnt by drawing the child is an answer for that box only.** The walk looking for what cannot survive a length change skipped a child whose own box was a fixed width -- correctly, its box does not change -- but that width was what the child reported when the span drew it in the span's box, and the span's box did change. So a child whose size the widget read is redrawn unless it declares an exact `size_hint` for the changed axis, which is the one case the parent did not have to draw it to know. The cost is that a size-reading container gives up its reuse when its box changes length, which is every span, so `OnResize::Scale` now earns its keep on moves and on subtrees whose sizes nobody read rather than on every stretch. Correct first; `replace_cost` still measures the case the chain was built for. `tests/generated.rs` is what found both and what says they are fixed: 90 of 90 warm trees now land where a cold build does, against 83 before this commit and 83 on `db1751f`. The ignored sweep agrees over 300 checks on 100 seeds. `a_fixed_length_child_is_not_redrawn_when_the_box_around_it_grows` became `a_declared_length_...`: the child now says its width, since a width the span measured is not one it may keep. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
86a7e8dfc3
commit
b0f9f046da
4 files changed
+63
-30
No files matched your search
@@ -1,4 +1,6 @@
|
|||||||
use crate::{LayerId, MaskIdx, MoveIdx, PrimitiveHandle, Size, TextureHandle, UiRegion, WidgetId};
|
use crate::{
|
||||||
|
LayerId, MaskIdx, MoveIdx, PrimitiveHandle, Size, TextureHandle, UiRegion, WidgetId, util::Vec2,
|
||||||
|
};
|
||||||
|
|
||||||
/// important non rendering data for retained drawing
|
/// important non rendering data for retained drawing
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
@@ -7,6 +9,10 @@ pub struct ActiveData {
|
|||||||
pub region: UiRegion,
|
pub region: UiRegion,
|
||||||
/// What the widget said it used of `region`, the last time it drew.
|
/// What the widget said it used of `region`, the last time it drew.
|
||||||
pub size: Size,
|
pub size: Size,
|
||||||
|
/// The pixel size of the box it drew against. `region` alone cannot say:
|
||||||
|
/// it is a fraction of a slot's box, and the same fraction of a box that
|
||||||
|
/// has since changed is a different number of pixels.
|
||||||
|
pub px: Vec2,
|
||||||
pub parent: Option<WidgetId>,
|
pub parent: Option<WidgetId>,
|
||||||
pub textures: Vec<TextureHandle>,
|
pub textures: Vec<TextureHandle>,
|
||||||
pub primitives: Vec<PrimitiveHandle>,
|
pub primitives: Vec<PrimitiveHandle>,
|
||||||
|
|||||||
+45
-19
@@ -132,6 +132,7 @@ impl UiRenderState {
|
|||||||
(parent_move, region)
|
(parent_move, region)
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
let px = self.px_of(move_idx, local);
|
||||||
rsc.widgets_mut().needs_redraw.remove(&id);
|
rsc.widgets_mut().needs_redraw.remove(&id);
|
||||||
self.draw_started.insert(id);
|
self.draw_started.insert(id);
|
||||||
|
|
||||||
@@ -180,6 +181,7 @@ impl UiRenderState {
|
|||||||
id,
|
id,
|
||||||
region,
|
region,
|
||||||
size,
|
size,
|
||||||
|
px,
|
||||||
parent,
|
parent,
|
||||||
textures,
|
textures,
|
||||||
primitives,
|
primitives,
|
||||||
@@ -227,6 +229,14 @@ impl UiRenderState {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The pixel size of a region held in `slot`'s coordinates.
|
||||||
|
fn px_of(&self, slot: MoveIdx, region: UiRegion) -> Vec2 {
|
||||||
|
self.moves
|
||||||
|
.resolve(slot, region)
|
||||||
|
.size()
|
||||||
|
.to_abs(self.output_size)
|
||||||
|
}
|
||||||
|
|
||||||
/// The drawing a widget already has, kept for a new box if the box has not
|
/// The drawing a widget already has, kept for a new box if the box has not
|
||||||
/// changed in a way it depends on.
|
/// changed in a way it depends on.
|
||||||
fn try_reuse(
|
fn try_reuse(
|
||||||
@@ -245,8 +255,16 @@ impl UiRenderState {
|
|||||||
if active.parent_move != parent_move {
|
if active.parent_move != parent_move {
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
let (size, old, slot) = (active.size, active.region, active.move_idx);
|
let (size, old, slot, was) = (active.size, active.region, active.move_idx, active.px);
|
||||||
if old == region {
|
// 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.
|
||||||
|
let px = self.px_of(parent_move, region);
|
||||||
|
let mut changed = [false; 2];
|
||||||
|
for (axis, c) in AXES.into_iter().zip(changed.iter_mut()) {
|
||||||
|
*c = px.axis(axis) != was.axis(axis);
|
||||||
|
}
|
||||||
|
if !changed.iter().any(|&c| c) && old == region {
|
||||||
return Some(size);
|
return Some(size);
|
||||||
}
|
}
|
||||||
// Only a placed widget can be given a different box without drawing
|
// Only a placed widget can be given a different box without drawing
|
||||||
@@ -255,28 +273,23 @@ impl UiRenderState {
|
|||||||
if slot == parent_move {
|
if slot == parent_move {
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
let mut changed = [false; 2];
|
|
||||||
for (axis, c) in AXES.into_iter().zip(changed.iter_mut()) {
|
|
||||||
*c = region.axis(axis).len() != old.axis(axis).len();
|
|
||||||
}
|
|
||||||
if changed.iter().any(|&c| c) {
|
if changed.iter().any(|&c| c) {
|
||||||
let widget = rsc.widgets().get_dyn(id)?;
|
let widget = rsc.widgets().get_dyn(id)?;
|
||||||
let redraws = AXES
|
let redraws = AXES
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.zip(changed)
|
.zip(changed)
|
||||||
.any(|(axis, c)| c && widget.on_resize(axis) != OnResize::Scale);
|
.any(|(axis, c)| c && widget.on_resize(axis) != OnResize::Scale);
|
||||||
if redraws {
|
|
||||||
return None;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
// Anything under it that has to be drawn again is drawn by drawing
|
// Anything under it that has to be drawn again is drawn by drawing
|
||||||
// this, because whatever reads that widget's size sits in between and
|
// this, because whatever reads that widget's size sits in between
|
||||||
// has to lay out around whatever it comes to.
|
// and has to lay out around what it comes to.
|
||||||
if changed.iter().any(|&c| c) && self.redraws_under(id, changed, rsc) {
|
if redraws || self.redraws_under(id, changed, rsc) {
|
||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
self.moves.set(slot, region);
|
self.moves.set(slot, region);
|
||||||
self.active.get_mut(&id).unwrap().region = region;
|
let active = self.active.get_mut(&id).unwrap();
|
||||||
|
active.region = region;
|
||||||
|
active.px = px;
|
||||||
Some(size)
|
Some(size)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -293,10 +306,26 @@ impl UiRenderState {
|
|||||||
let Some(active) = self.active.get(&id) else {
|
let Some(active) = self.active.get(&id) else {
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
|
let size_deps = &active.size_deps;
|
||||||
active.children.iter().any(|&child| {
|
active.children.iter().any(|&child| {
|
||||||
let Some(data) = self.active.get(&child) else {
|
let Some(data) = self.active.get(&child) else {
|
||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
|
let Some(widget) = rsc.widgets().get_dyn(child) else {
|
||||||
|
return true;
|
||||||
|
};
|
||||||
|
// What it drew to learn this child's size was the child in *this*
|
||||||
|
// box, so a different box is a different answer -- unless the
|
||||||
|
// child gave an exact one without being drawn at all.
|
||||||
|
if size_deps.contains(&child) {
|
||||||
|
let measured = AXES
|
||||||
|
.into_iter()
|
||||||
|
.zip(changed)
|
||||||
|
.any(|(axis, c)| c && widget.size_hint(axis).is_none());
|
||||||
|
if measured {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
let mut own = changed;
|
let mut own = changed;
|
||||||
for (axis, c) in AXES.into_iter().zip(own.iter_mut()) {
|
for (axis, c) in AXES.into_iter().zip(own.iter_mut()) {
|
||||||
*c &= data.region.axis(axis).len().rel != 0.0;
|
*c &= data.region.axis(axis).len().rel != 0.0;
|
||||||
@@ -304,13 +333,10 @@ impl UiRenderState {
|
|||||||
if !own.iter().any(|&c| c) {
|
if !own.iter().any(|&c| c) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
let redraws = match rsc.widgets().get_dyn(child) {
|
let redraws = AXES
|
||||||
Some(widget) => AXES
|
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.zip(own)
|
.zip(own)
|
||||||
.any(|(axis, c)| c && widget.on_resize(axis) != OnResize::Scale),
|
.any(|(axis, c)| c && widget.on_resize(axis) != OnResize::Scale);
|
||||||
None => true,
|
|
||||||
};
|
|
||||||
redraws || self.redraws_under(child, own, rsc)
|
redraws || self.redraws_under(child, own, rsc)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
+4
-6
@@ -17,9 +17,7 @@ use iris::prelude::*;
|
|||||||
use iris::random::{Lens, Rng, Tree, grow};
|
use iris::random::{Lens, Rng, Tree, grow};
|
||||||
|
|
||||||
const DEPTH: usize = 4;
|
const DEPTH: usize = 4;
|
||||||
/// Seeds whose trees agree. The ones left out are `a_wrapping_child_of_a_row`
|
const SEEDS: [u64; 6] = [1, 2, 3, 5, 8, 13];
|
||||||
/// below, which is a defect older than the chain.
|
|
||||||
const SEEDS: [u64; 6] = [2, 3, 4, 5, 8, 9];
|
|
||||||
|
|
||||||
fn plant(h: &mut Harness, seed: u64, edits: &HashMap<usize, Lens>) -> Tree {
|
fn plant(h: &mut Harness, seed: u64, edits: &HashMap<usize, Lens>) -> Tree {
|
||||||
let (root, tree) = grow(&mut h.rsc, seed, DEPTH, edits);
|
let (root, tree) = grow(&mut h.rsc, seed, DEPTH, edits);
|
||||||
@@ -161,9 +159,9 @@ fn a_size_change_after_a_resize_lands_the_same_way() {
|
|||||||
/// same defect, and it wants fixing where the two draws meet -- LAYOUT.md §4 --
|
/// same defect, and it wants fixing where the two draws meet -- LAYOUT.md §4 --
|
||||||
/// rather than anywhere in the chain.
|
/// rather than anywhere in the chain.
|
||||||
#[test]
|
#[test]
|
||||||
#[ignore = "known divergence, and the reproduction for fixing it"]
|
#[ignore = "a hundred seeds, rather than the six the others check"]
|
||||||
fn a_wrapping_child_of_a_row_settles_somewhere_else_each_time() {
|
fn a_long_run_of_seeds_agrees() {
|
||||||
for seed in 1..=30 {
|
for seed in 1..=100 {
|
||||||
changed_size(seed);
|
changed_size(seed);
|
||||||
resized(seed);
|
resized(seed);
|
||||||
resized_then_changed(seed);
|
resized_then_changed(seed);
|
||||||
|
|||||||
+7
-4
@@ -320,11 +320,14 @@ fn a_widened_row_redraws_what_reads_its_length_and_nothing_else() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn a_fixed_length_child_is_not_redrawn_when_the_box_around_it_grows() {
|
fn a_declared_length_child_is_not_redrawn_when_the_box_around_it_grows() {
|
||||||
let mut h = Harness::new((400, 200));
|
let mut h = Harness::new((400, 200));
|
||||||
// It would be drawn again for a width it does not have: its own box is
|
// Its box is a fixed 80 wherever the row's edges end up, so drawing it
|
||||||
// a fixed 80 wherever the row's edges end up.
|
// again would be for a width it does not have. The declared width is what
|
||||||
let (fixed, draws) = counted(&mut h, Size::from((80, 200)), OnResize::Redraw);
|
// lets the span say that without drawing it: a width the span learnt by
|
||||||
|
// drawing the child in its own box is only an answer for that box.
|
||||||
|
let (counter, draws) = counted(&mut h, Size::from((80, 200)), OnResize::Redraw);
|
||||||
|
let fixed = counter.width(80).add(&mut h.rsc);
|
||||||
let (rest, _) = counted(&mut h, Size::REST, OnResize::Scale);
|
let (rest, _) = counted(&mut h, Size::REST, OnResize::Scale);
|
||||||
let row = (fixed, rest).span(Dir::RIGHT).add(&mut h.rsc);
|
let row = (fixed, rest).span(Dir::RIGHT).add(&mut h.rsc);
|
||||||
let bar = rect(Color::RED).width(100).add(&mut h.rsc);
|
let bar = rect(Color::RED).width(100).add(&mut h.rsc);
|
||||||
|
|||||||
Reference in new issue
Block a user