diff --git a/core/src/primitive/text.rs b/core/src/primitive/text.rs index e3274a5..d0034a4 100644 --- a/core/src/primitive/text.rs +++ b/core/src/primitive/text.rs @@ -176,6 +176,23 @@ impl TextBuffer { self.layout_key.as_ref()?.max_width } + /// Widths covered by the current line breaks, including a wider shaping + /// retained when a later draw requested a narrower box. + pub fn width_holds(&self) -> crate::Holds { + let Some(width) = self.wrap_width() else { + return crate::Holds::ANY; + }; + let width = Px::from_f32(width); + let soft_wrapped = self.layout.lines().any(|line| { + matches!( + line.break_reason(), + parley::layout::BreakReason::Regular | parley::layout::BreakReason::Emergency + ) + }); + let upper = if soft_wrapped { width } else { Px::MAX }; + crate::Holds::from(Px::ceil_from_f32(self.layout.width()).min(width)..=upper) + } + pub fn size(&self) -> Vec2 { Vec2::new(self.layout.width(), self.layout.height()) } diff --git a/core/src/ui/active.rs b/core/src/ui/active.rs index 73c4a3b..10de432 100644 --- a/core/src/ui/active.rs +++ b/core/src/ui/active.rs @@ -15,10 +15,9 @@ pub struct ActiveData { pub region: UiRegion, /// Where its drawing sits inside that box, in the box's own coordinates. pub placement: UiRegion, - /// The same box as lengths of its parent's box, which is the one route - /// to a box in pixels: a draw threads these down a level at a time, and - /// [`crate::UiRenderState::redraw`] takes the same steps back up. - pub given_len: UiVec2, + /// The original frame in its parent widget's coordinates. Recomposition + /// and pixel-length evaluation both follow this chain. + pub given_region: UiRegion, /// The lengths of the box its parent first asked about it in, as /// lengths of the box the parent was itself offered. Any later box it /// was given was decided knowing its answer, so this is the question diff --git a/core/src/ui/painter.rs b/core/src/ui/painter.rs index 96b0bb2..b5ea8ce 100644 --- a/core/src/ui/painter.rs +++ b/core/src/ui/painter.rs @@ -258,7 +258,7 @@ impl<'a> Painter<'a> { parent_move: self.move_idx, region_node, mask: self.mask, - given_len, + given_region: local, offer_len, offer_placement, px, diff --git a/core/src/ui/render_state.rs b/core/src/ui/render_state.rs index f465f90..1f509cc 100644 --- a/core/src/ui/render_state.rs +++ b/core/src/ui/render_state.rs @@ -2,9 +2,9 @@ use crate::layout_diagnostics::{self as diag, Counter, ReuseOutcome, TimerKind}; use crate::ui::painter::{ask_box, declared_lens, placed_box, placed_lens}; use crate::{ - ActiveData, Axis, DrawLayers, Holds, IdLike, LayoutHolds, LayoutLen, Len, MaskIdx, MoveIdx, - Moves, Painter, PixelRegion, Px, PxVec2, Rel, Size, StrongWidget, UiRegion, UiRsc, UiSpan, - UiVec2, Weight, WidgetId, Widgets, + ActiveData, Axis, DrawLayers, Holds, IdLike, LayoutHolds, LayoutLen, MaskIdx, MoveIdx, Moves, + Painter, PixelRegion, PxVec2, Size, StrongWidget, UiRegion, UiRsc, UiSpan, UiVec2, Weight, + WidgetId, Widgets, util::{HashMap, Vec2}, }; @@ -20,11 +20,9 @@ pub(super) struct DrawInfo { pub parent_move: MoveIdx, pub region_node: bool, pub mask: MaskIdx, - /// The box its parent gave it, as lengths of the parent's own box, and - /// the lengths of the box it was first asked about in the same form. - /// Both describe the box the *parent* stated, so the second, placing ask - /// carries them unchanged while its own region is the placement inside. - pub given_len: UiVec2, + /// The frame in the parent widget's coordinates, before composition. + pub given_region: UiRegion, + /// The original offer's lengths relative to the parent's own offer. pub offer_len: UiVec2, pub offer_placement: [Option; 2], /// This ask's box in pixels, and the offer's: one multiply from the @@ -121,7 +119,7 @@ impl UiRenderState { let stands = self .active .get(&root) - .is_some_and(|active| active.answers_at(active.given_len.to_px(size))); + .is_some_and(|active| active.answers_at(active.given_region.size().to_px(size))); if !stands { widgets.needs_redraw.insert(root); } @@ -141,7 +139,7 @@ impl UiRenderState { parent_move: MoveIdx::NONE, region_node: false, mask: MaskIdx::NONE, - given_len: region.size(), + given_region: region, offer_len: UiVec2::FULL_SIZE, offer_placement: [None; 2], px, @@ -278,7 +276,7 @@ impl UiRenderState { // what of that box the answer then took. A local redraw asks the // same question again from these. active.region = region; - active.given_len = info.given_len; + active.given_region = info.given_region; active.offer_len = info.offer_len; if info.placement == info.offer_placement && info.px == info.offered_px { active.answer = Some(settled); @@ -483,7 +481,7 @@ impl UiRenderState { parent_move: move_idx, region_node: false, mask, - given_len: UiVec2::FULL_SIZE, + given_region: UiRegion::FULL, offer_len: UiVec2::FULL_SIZE, offer_placement: [None; 2], px, @@ -500,7 +498,7 @@ impl UiRenderState { id, region, placement, - given_len: info.given_len, + given_region: info.given_region, offer_len: info.offer_len, offer_placement: info.offer_placement, // Whoever asked writes the answer, if this was the asking. @@ -616,7 +614,7 @@ impl UiRenderState { Some(parent) => self.asked_px(parent.id), None => (self.output_size, self.output_size), }; - let px = active.given_len.to_px(parent_px); + let px = active.given_region.size().to_px(parent_px); let mut offered = active.offer_len.to_px(parent_offer); for axis in AXES { // A declared length is resolved by whoever drew the widget, in @@ -697,14 +695,12 @@ impl UiRenderState { } let extent_moved = active.placement != placement; let moved = active.region != region; - let (answer, old_region, slot) = - ((active.size, active.holds), active.region, active.move_idx); + let (answer, slot) = ((active.size, active.holds), active.move_idx); if moved { if has_region_node { self.moves.set(slot, region); } else { - let remap = RegionRemap::new(old_region, region)?; - self.remap_subtree(id, &remap, info.parent_move, rsc); + self.recompose_subtree(id, region, info.parent_move, rsc); } } if extent_moved { @@ -713,7 +709,7 @@ impl UiRenderState { self.redepth(id, info.depth); let active = self.active.get_mut(&id).unwrap(); active.region = region; - active.given_len = info.given_len; + active.given_region = info.given_region; active.offer_len = info.offer_len; #[cfg(feature = "layout-diagnostics")] { @@ -785,7 +781,7 @@ impl UiRenderState { parent_move, region_node: active.move_idx != active.parent_move, mask, - given_len: child_local.size(), + given_region: child_local, offer_len: active.offer_len, offer_placement: active.offer_placement, px: child_local.size().to_px(info.px), @@ -820,44 +816,35 @@ impl UiRenderState { } } - /// Re-expresses an ordinary retained subtree in a new parent region. - /// An independently movable descendant needs only its own region changed; - /// its contents stay in that region's coordinate space. - fn remap_subtree( + /// Replays the original local compositions, including their rounding order. + /// A region node terminates the walk because its contents name its slot. + fn recompose_subtree( &mut self, id: WidgetId, - remap: &RegionRemap, + region: UiRegion, parent_move: MoveIdx, rsc: &mut dyn UiRsc, ) { let active = self.active.get_mut(&id).unwrap(); + active.region = region; if active.move_idx != parent_move { - let region = remap.apply(active.region); - active.region = region; self.moves.set(active.move_idx, region); return; } - active.region = remap.apply(active.region); for primitive in &active.primitives { let handle = &primitive.handle; *self.layers[handle.layer].region_mut(handle) = - primitive.region.resolve(active.region, active.placement); + primitive.region.resolve(region, active.placement); + } + if let Some(local) = active.mask_region { + rsc.ui_mut().masks.get_mut(active.mask).region = + local.resolve(region, active.placement); } - let own_mask = (active.mask != active.parent_mask).then_some(active.mask); let children = active.children.len(); - // A mask the widget set itself moves with it; one it inherited - // belongs to the widget that set it, and moves there or not at all. - if let Some(idx) = own_mask { - let mask = rsc.ui_mut().masks.get_mut(idx); - debug_assert_eq!(mask.move_idx, parent_move); - mask.region = active - .mask_region - .unwrap() - .resolve(active.region, active.placement); - } for index in 0..children { let child = self.active[&id].children[index]; - self.remap_subtree(child, remap, parent_move, rsc); + let local = self.active[&child].given_region; + self.recompose_subtree(child, local.within(®ion), parent_move, rsc); } } @@ -933,7 +920,7 @@ impl UiRenderState { id, region: UiRegion::FULL, placement: UiRegion::FULL, - given_len: UiVec2::FULL_SIZE, + given_region: UiRegion::FULL, offer_len: UiVec2::FULL_SIZE, offer_placement: [None; 2], answer: None, @@ -1171,7 +1158,7 @@ impl UiRenderState { parent_move: active.parent_move, region_node: rsc.widgets().is_region_node(id), mask: active.parent_mask, - given_len: active.given_len, + given_region: active.given_region, offer_len: active.offer_len, offer_placement: active.offer_placement, px: given_px, @@ -1219,118 +1206,6 @@ fn within_box(size: Size, px: PxVec2, axis: Axis) -> bool { len.leftover != Weight::ZERO || box_len.mul(len.rel) + len.px <= box_len } -/// A retained region rewritten from one parent box into another. A fixed -/// source extent can be translated but cannot recover fractions for a resize. -#[derive(Clone, Copy)] -struct RegionRemap { - axes: [AxisRemap; 2], -} - -/// Moving one axis of a box into another, worked out once for the whole -/// subtree that moves with it. Every part of that subtree is divided by the -/// same extent and placed between the same two ends, so the ends and the -/// divisor belong here rather than in each part's arithmetic. -#[derive(Clone, Copy)] -enum AxisRemap { - /// A box that kept its length carries its parts by moving them, which is - /// exact. Dividing to find the fraction each sits at and multiplying to - /// place it again are two roundings, and they land a step from where - /// growing the tree that way does. - Translate(Len), - /// A box that changed length has to re-express each part as a fraction of - /// the new one, which is what a part of a box means. - Scale(AxisScale), -} - -#[derive(Clone, Copy)] -struct AxisScale { - /// What the fraction is measured from, and what divides it. `whole` is - /// the common case of a box spanning the whole of its parent's, where - /// dividing by one is the expensive way to write a subtraction. - start_rel: Rel, - extent: Rel, - whole: bool, - /// `lerp` is `a + (b - a) * fraction`, and both ends are the same for - /// every part, so each is kept as its near end and its span. - from_px: Px, - from_px_span: Px, - to_rel: Rel, - to_rel_span: Rel, - to_px: Px, - to_px_span: Px, -} - -impl RegionRemap { - fn new(from: UiRegion, to: UiRegion) -> Option { - Some(Self { - axes: [AxisRemap::new(from.x, to.x)?, AxisRemap::new(from.y, to.y)?], - }) - } - - fn apply(&self, region: UiRegion) -> UiRegion { - // A box that only moved carries every part of itself by the same two - // amounts, and that is the common move. Asking it once for the whole - // region is what lets it be eight adds in a row rather than four - // sequences with a branch each -- measured, it is where the time in a - // move goes. - if let [AxisRemap::Translate(x), AxisRemap::Translate(y)] = self.axes { - return region.translated(x, y); - } - UiRegion { - x: self.axes[0].apply_span(region.x), - y: self.axes[1].apply_span(region.y), - } - } -} - -impl AxisRemap { - fn new(from: UiSpan, to: UiSpan) -> Option { - if from.len() == to.len() { - return Some(Self::Translate(to.start - from.start)); - } - let extent = from.end.rel - from.start.rel; - // Without a relative extent there is no fraction to re-express: a box - // of fixed length cannot say where its parts sit in a different one. - if extent == Rel::ZERO { - return None; - } - Some(Self::Scale(AxisScale { - start_rel: from.start.rel, - extent, - whole: extent == Rel::ONE, - from_px: from.start.px, - from_px_span: from.end.px - from.start.px, - to_rel: to.start.rel, - to_rel_span: to.end.rel - to.start.rel, - to_px: to.start.px, - to_px_span: to.end.px - to.start.px, - })) - } - - fn apply_span(&self, span: UiSpan) -> UiSpan { - UiSpan { - start: self.apply_scalar(span.start), - end: self.apply_scalar(span.end), - } - } - - fn apply_scalar(&self, scalar: Len) -> Len { - let scale = match self { - Self::Translate(by) => return scalar + *by, - Self::Scale(scale) => scale, - }; - let offset = scalar.rel - scale.start_rel; - let fraction = match scale.whole { - true => offset, - false => offset / scale.extent, - }; - let from_px = scale.from_px + scale.from_px_span.mul(fraction); - let to_rel = scale.to_rel + scale.to_rel_span.mul(fraction); - let to_px = scale.to_px + scale.to_px_span.mul(fraction); - Len::from_parts(to_rel, scalar.px - from_px + to_px) - } -} - impl Default for UiRenderState { fn default() -> Self { Self::new() diff --git a/src/widget/text/mod.rs b/src/widget/text/mod.rs index e0dd0b4..3ee1c61 100644 --- a/src/widget/text/mod.rs +++ b/src/widget/text/mod.rs @@ -50,20 +50,11 @@ impl TextView { let width = self.attrs.wrap.then(|| painter.px_len(Axis::X)); // The shaper measures in floats, which is where a glyph advance comes // from; what it answers goes back on the grid. - let text = painter.render_text(&mut self.buf, &self.attrs, width.map(Px::to_f32)); - // A greedy break is the same break at every width from its longest - // line up to the one it was made at: each line still fits, and none - // could take a word that did not fit in the wider box. A line too - // long to fit at all says nothing about narrower boxes. - // - // The step at or above that longest line rather than the nearest - // one, since the shaper measures in floats: the nearest step is - // under the line half the time, and a range starting there admits a - // box the line does not fit in, where the break is not this one. - if let Some(width) = width { - painter.holds(Axis::X, Px::ceil_from_f32(text.size.x).min(width)..=width); + painter.render_text(&mut self.buf, &self.attrs, width.map(Px::to_f32)); + if width.is_some() { + painter.holds(Axis::X, self.buf.width_holds()); } - text + self.buf.rendered().expect("render_text placed the glyphs") } pub fn tex(&self) -> Option<&RenderedText> { diff --git a/tests/cases/retained.rs b/tests/cases/retained.rs index 932b3ab..6676152 100644 --- a/tests/cases/retained.rs +++ b/tests/cases/retained.rs @@ -808,3 +808,109 @@ fn changing_an_inherited_extent_keeps_the_original_measurement_offer() { primitive_bounds(&cold, other.id()) ); } + +#[test] +fn widening_text_without_soft_breaks_reuses_its_drawing() { + struct CountedText { + text: Text, + draws: Rc>, + } + impl Widget for CountedText { + fn draw(&mut self, painter: &mut Painter) -> Size { + self.draws.set(self.draws.get() + 1); + self.text.draw(painter) + } + } + for content in ["Short text", "Two hard\nline breaks\nhere", ""] { + let plant = |h: &mut Harness| { + let mut text = Text::new(content); + text.wrap = true; + let draws = Rc::new(Cell::new(0)); + let root = CountedText { + text, + draws: draws.clone(), + } + .add(&mut h.rsc); + h.set_root(root); + (root, draws) + }; + let mut warm = Harness::new((300, 200)); + let (root, draws) = plant(&mut warm); + let before = draws.get(); + warm.resize((500, 200)); + warm.frame(); + assert_eq!(draws.get(), before, "{content:?}"); + let mut cold = Harness::new((500, 200)); + let (other, _) = plant(&mut cold); + assert_eq!(warm.region(&root), cold.region(&other)); + assert_eq!( + primitive_bounds(&warm, root.id()), + primitive_bounds(&cold, other.id()) + ); + } +} + +#[test] +fn resizing_a_fixed_frame_recomposes_its_contents_without_drawing_them() { + struct Frame { + child: StrongWidget, + region: UiRegion, + } + impl Widget for Frame { + fn draw(&mut self, painter: &mut Painter) -> Size { + painter.widget_within(&self.child, self.region); + Size::LEFTOVER + } + } + struct Painted(Rc>); + impl Widget for Painted { + fn draw(&mut self, painter: &mut Painter) -> Size { + self.0.set(self.0.get() + 1); + painter.set_mask(DrawRegion::Extent(UiRegion::FULL)); + painter.primitive(RectPrimitive::color(Color::BLUE)); + Size::LEFTOVER + } + } + let fixed = |start, end| UiRegion::new(UiSpan::new(Len::px(start), Len::px(end)), UiSpan::FULL); + for node in [false, true] { + let plant = |h: &mut Harness, region| { + let draws = Rc::new(Cell::new(0)); + let leaf = Painted(draws.clone()).add(&mut h.rsc); + h.rsc.widgets_mut().set_region_node(leaf, node); + let inner = Frame { + child: leaf.add_strong(&mut h.rsc), + region: UiRegion::new(UiSpan::new(Len::rel(0.23), Len::rel(0.83)), UiSpan::FULL), + } + .add_strong(&mut h.rsc); + let root = Frame { + child: inner, + region, + } + .add(&mut h.rsc); + h.set_root(root); + (root, leaf, draws) + }; + let mut warm = Harness::new((400, 200)); + let (root, leaf, draws) = plant(&mut warm, fixed(7.0, 104.0)); + let before = draws.get(); + warm.rsc[root].region = fixed(19.0, 180.0); + warm.frame(); + assert_eq!(draws.get(), before); + let mut cold = Harness::new((400, 200)); + let (_, other, _) = plant(&mut cold, fixed(19.0, 180.0)); + assert_eq!(warm.region(&leaf), cold.region(&other)); + assert_eq!( + primitive_bounds(&warm, leaf.id()), + primitive_bounds(&cold, other.id()) + ); + let mask = |h: &Harness, id: WidgetId| { + let active = &h.render.active[&id]; + let mask = &h.rsc.ui().masks[active.mask.idx()]; + h.render + .moves + .resolve(mask.move_idx, mask.region) + .to_px(h.render.output_size()) + }; + assert_eq!(mask(&warm, leaf.id()), mask(&cold, other.id())); + } +} diff --git a/tests/scenario/mod.rs b/tests/scenario/mod.rs index 746308b..c0106a0 100644 --- a/tests/scenario/mod.rs +++ b/tests/scenario/mod.rs @@ -42,21 +42,6 @@ const OUTER: (f32, f32) = (1920.0, 1200.0); const INNER: (f32, f32) = (640.0, 900.0); const STILL: (f32, f32) = (900.0, 1200.0); -/// The same box, to two steps of the grid between the two ways of reaching -/// it. A move, a repaint, a row of shares and every length in pixels land on -/// the same number. What needs the slack is a position: a box centred in a -/// fraction of its parent against the same box centred in its own pixels, -/// and a box re-expressed as a fraction of a parent that changed length. -/// A step is a thousandth of a pixel, where this was a twentieth of one -/// before any of it was on a grid. -/// -/// **One step is not enough**, tried 2026-09-17 once a length in pixels -/// stopped being composed: it passes the 100-seed oracle and fails the -/// 400-seed shrinker on `resize-size`, seeds 384 and 162, by 0.002 px. So -/// what is left here is the resize path's own rounding rather than a length -/// reached two ways. -const AGREE_STEPS: i32 = 2; - /// A way of changing what a span holds. Each is a shape worth its own case: /// taking a child out of the middle is not the same as emptying a span, and /// adding one is not the same as adding three. @@ -399,20 +384,6 @@ fn describe_widget(id: WidgetId, h: &Harness) -> String { label } -fn same_region(got: Option, want: Option) -> bool { - match (got, want) { - (Some(got), Some(want)) => { - let same = |a: Px, b: Px| (a - b).abs() <= Px::STEP.mul_int(AGREE_STEPS); - same(got.top_left.x, want.top_left.x) - && same(got.top_left.y, want.top_left.y) - && same(got.bot_right.x, want.bot_right.x) - && same(got.bot_right.y, want.bot_right.y) - } - (None, None) => true, - _ => false, - } -} - /// Runs `case` on the tree `plan` describes, warm and cold, and says where /// the two disagree. `seed` chooses only the values a case picks at random, /// so one plan under one case is one comparison however it was reached. @@ -439,7 +410,7 @@ pub fn diverges(plan: &Plan, case: Case, seed: u64) -> Option { for (i, (&w, &c)) in tree.ids.iter().zip(&cold_tree.ids).enumerate() { let (got, want) = (warm.region(&w), cold.region(&c)); drawn += got.is_some() as usize; - if same_region(got, want) { + if got == want { continue; } // Where two trees disagree is rarely where the cause is, so the