Resolve a declared length where the widget is drawn, not inside it
SetSize took its declared length out of UiRegion::FULL, which is the box it was already given, so a span that had sized that box from the same hint had the fraction taken twice: .width(rel(0.5)) in a 400-wide span drew its child 100 wide. It held under a Pad or the root, which do not honour a hint, and hid under px, where 200 of a 200-wide box is all of it. The text example was 111,923 pixels from upstream/main because of it. Whoever draws a widget now takes its declared length, in its own box, which is what a fraction of one means, and is the identity for a caller that already reserved the space. rest is not taken: a share of what is left over is only a length to the widget dividing one, so it passes up in the size as it does out of a span. SetSize keeps only what it declares. A declared length is then part of the box its parent decided, so changing one has to redraw the parent; the lengths resolved into a box are kept beside it and compared. Assuming instead that any dirty widget which declares a length needs its parent costs 17% of a frame that dirties 130 of 260 widgets, and buys nothing. All five reference renders, the resize render and the image replay are byte-identical to upstream/main, the 100-seed sweep passes, and the resize fixture is 1.286 ms against 1.289 before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
169db7f16f
commit
de9ddc0ad4
4 files changed
+83
-22
No files matched your search
@@ -1,5 +1,6 @@
|
|||||||
use crate::{
|
use crate::{
|
||||||
LayerId, MaskIdx, MoveIdx, PrimitiveHandle, Size, TextureHandle, UiRegion, WidgetId, util::Vec2,
|
LayerId, Len, MaskIdx, MoveIdx, PrimitiveHandle, Size, TextureHandle, UiRegion, WidgetId,
|
||||||
|
util::Vec2,
|
||||||
};
|
};
|
||||||
|
|
||||||
/// important non rendering data for retained drawing
|
/// important non rendering data for retained drawing
|
||||||
@@ -38,6 +39,10 @@ pub struct ActiveData {
|
|||||||
/// The slot its primitives are positioned through: its own if its parent
|
/// The slot its primitives are positioned through: its own if its parent
|
||||||
/// placed it, otherwise the nearest ancestor that has one.
|
/// placed it, otherwise the nearest ancestor that has one.
|
||||||
pub move_idx: MoveIdx,
|
pub move_idx: MoveIdx,
|
||||||
|
/// The declared lengths whoever drew this widget resolved into its box.
|
||||||
|
/// A change to one moves a box this widget cannot fix by drawing again,
|
||||||
|
/// and comparing them is what says so.
|
||||||
|
pub declared: [Option<Len>; 2],
|
||||||
/// The slot `region` is given in, which is whatever its parent drew in.
|
/// The slot `region` is given in, which is whatever its parent drew in.
|
||||||
pub parent_move: MoveIdx,
|
pub parent_move: MoveIdx,
|
||||||
pub mask: MaskIdx,
|
pub mask: MaskIdx,
|
||||||
|
|||||||
+51
-6
@@ -2,7 +2,7 @@
|
|||||||
use crate::layout_diagnostics::{self as diag, Counter};
|
use crate::layout_diagnostics::{self as diag, Counter};
|
||||||
use crate::{
|
use crate::{
|
||||||
Axis, Len, RenderedText, Size, StrongWidget, TextAttrs, TextBuffer, TextData, TextureHandle,
|
Axis, Len, RenderedText, Size, StrongWidget, TextAttrs, TextBuffer, TextData, TextureHandle,
|
||||||
UiRegion, UiRenderState, UiRsc, UiScalar, UiVec2, WidgetId,
|
UiRegion, UiRenderState, UiRsc, UiScalar, UiVec2, Widget, WidgetId,
|
||||||
render::{
|
render::{
|
||||||
GlyphPrimitive, Mask, MaskIdx, MoveIdx, Primitive, PrimitiveHandle, PrimitiveInst,
|
GlyphPrimitive, Mask, MaskIdx, MoveIdx, Primitive, PrimitiveHandle, PrimitiveInst,
|
||||||
PrimitiveKind, TexturePrimitive,
|
PrimitiveKind, TexturePrimitive,
|
||||||
@@ -90,7 +90,14 @@ impl<'a> Painter<'a> {
|
|||||||
|
|
||||||
/// Draws a widget within this widget's region.
|
/// Draws a widget within this widget's region.
|
||||||
pub fn widget<'s, W: ?Sized>(&'s mut self, id: &'s StrongWidget<W>) -> DrawResult<'s, 'a, W> {
|
pub fn widget<'s, W: ?Sized>(&'s mut self, id: &'s StrongWidget<W>) -> DrawResult<'s, 'a, W> {
|
||||||
self.widget_at(id, self.region, false)
|
let declared = self.declared_lens(id);
|
||||||
|
// Composing `FULL` through a box is not quite the identity in f32,
|
||||||
|
// so a child with nothing declared keeps the box it would have had.
|
||||||
|
let region = match declared.iter().any(Option::is_some) {
|
||||||
|
true => declared_box(UiRegion::FULL, declared).within(&self.region),
|
||||||
|
false => self.region,
|
||||||
|
};
|
||||||
|
self.widget_at(id, region, false, declared)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Draws a widget somewhere within this one.
|
/// Draws a widget somewhere within this one.
|
||||||
@@ -99,8 +106,21 @@ impl<'a> Painter<'a> {
|
|||||||
id: &'s StrongWidget<W>,
|
id: &'s StrongWidget<W>,
|
||||||
region: UiRegion,
|
region: UiRegion,
|
||||||
) -> DrawResult<'s, 'a, W> {
|
) -> DrawResult<'s, 'a, W> {
|
||||||
let region = region.within(&self.region);
|
let declared = self.declared_lens(id);
|
||||||
self.widget_at(id, region, false)
|
let region = declared_box(region, declared).within(&self.region);
|
||||||
|
self.widget_at(id, region, false, declared)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// What a widget declares its lengths to be, which whoever draws it
|
||||||
|
/// resolves into its box. `rest` is not among them: a share of what is
|
||||||
|
/// left over is only a length to the widget dividing one, so it passes
|
||||||
|
/// up in the size instead. Reading it depends on nothing -- the box that
|
||||||
|
/// comes of it is kept on the child, and `redraw` compares it there.
|
||||||
|
fn declared_lens<W: ?Sized>(&self, id: &StrongWidget<W>) -> [Option<Len>; 2] {
|
||||||
|
let Some(widget) = self.rsc.widgets().get_dyn(id.id()) else {
|
||||||
|
return [None; 2];
|
||||||
|
};
|
||||||
|
[Axis::X, Axis::Y].map(|axis| declared_len(widget, axis))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Draws a child this widget decides the box of, and may decide again
|
/// Draws a child this widget decides the box of, and may decide again
|
||||||
@@ -116,10 +136,11 @@ impl<'a> Painter<'a> {
|
|||||||
) -> DrawResult<'s, 'a, W> {
|
) -> DrawResult<'s, 'a, W> {
|
||||||
#[cfg(feature = "layout-diagnostics")]
|
#[cfg(feature = "layout-diagnostics")]
|
||||||
diag::bump(Counter::PlaceCalls);
|
diag::bump(Counter::PlaceCalls);
|
||||||
let region = region.within(&self.region);
|
let declared = self.declared_lens(id);
|
||||||
|
let region = declared_box(region, declared).within(&self.region);
|
||||||
#[cfg(feature = "layout-diagnostics")]
|
#[cfg(feature = "layout-diagnostics")]
|
||||||
diag::placed(id.id(), self.id, region);
|
diag::placed(id.id(), self.id, region);
|
||||||
self.widget_at(id, region, true)
|
self.widget_at(id, region, true, declared)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn widget_at<'s, W: ?Sized>(
|
fn widget_at<'s, W: ?Sized>(
|
||||||
@@ -127,6 +148,7 @@ impl<'a> Painter<'a> {
|
|||||||
id: &'s StrongWidget<W>,
|
id: &'s StrongWidget<W>,
|
||||||
region: UiRegion,
|
region: UiRegion,
|
||||||
slotted: bool,
|
slotted: bool,
|
||||||
|
declared: [Option<Len>; 2],
|
||||||
) -> DrawResult<'s, 'a, W> {
|
) -> DrawResult<'s, 'a, W> {
|
||||||
// A child listed twice would be moved twice.
|
// A child listed twice would be moved twice.
|
||||||
if !self.children.contains(&id.id()) {
|
if !self.children.contains(&id.id()) {
|
||||||
@@ -145,6 +167,9 @@ impl<'a> Painter<'a> {
|
|||||||
self.rsc,
|
self.rsc,
|
||||||
);
|
);
|
||||||
self.offer(id.id(), region);
|
self.offer(id.id(), region);
|
||||||
|
if let Some(active) = self.state.active.get_mut(&id.id()) {
|
||||||
|
active.declared = declared;
|
||||||
|
}
|
||||||
DrawResult {
|
DrawResult {
|
||||||
child: id,
|
child: id,
|
||||||
painter: self,
|
painter: self,
|
||||||
@@ -413,3 +438,23 @@ impl PrimitiveLike for &TextureHandle {
|
|||||||
self.into()
|
self.into()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// What a widget declares a length of its box to be. `rest` is not one: a
|
||||||
|
/// share of what is left over is only a length to the widget dividing one,
|
||||||
|
/// so it passes up in the size instead.
|
||||||
|
pub(crate) fn declared_len(widget: &dyn Widget, axis: Axis) -> Option<Len> {
|
||||||
|
widget.size_hint(axis).filter(|len| len.rest == 0.0)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Takes a widget's declared lengths in the box `region` is given in, since a
|
||||||
|
/// fraction of a length means a fraction of that one. A caller that already
|
||||||
|
/// reserved the space hands back the same length, so this is the identity
|
||||||
|
/// for it.
|
||||||
|
fn declared_box(mut region: UiRegion, declared: [Option<Len>; 2]) -> UiRegion {
|
||||||
|
for (axis, len) in [Axis::X, Axis::Y].into_iter().zip(declared) {
|
||||||
|
let Some(len) = len else { continue };
|
||||||
|
let span = region.axis_mut(axis);
|
||||||
|
span.end = span.start + UiScalar::new(len.rel, len.px);
|
||||||
|
}
|
||||||
|
region
|
||||||
|
}
|
||||||
@@ -1,5 +1,6 @@
|
|||||||
#[cfg(feature = "layout-diagnostics")]
|
#[cfg(feature = "layout-diagnostics")]
|
||||||
use crate::layout_diagnostics::{self as diag, Counter, ReuseOutcome, TimerKind};
|
use crate::layout_diagnostics::{self as diag, Counter, ReuseOutcome, TimerKind};
|
||||||
|
use crate::ui::painter::declared_len;
|
||||||
use crate::{
|
use crate::{
|
||||||
ActiveData, Axis, DrawLayers, IdLike, MaskIdx, MoveIdx, Moves, OnResize, Painter, PixelRegion,
|
ActiveData, Axis, DrawLayers, IdLike, MaskIdx, MoveIdx, Moves, OnResize, Painter, PixelRegion,
|
||||||
Size, StrongWidget, UiRegion, UiRsc, UiScalar, UiSpan, WidgetId, Widgets,
|
Size, StrongWidget, UiRegion, UiRsc, UiScalar, UiSpan, WidgetId, Widgets,
|
||||||
@@ -307,6 +308,8 @@ impl UiRenderState {
|
|||||||
size_deps,
|
size_deps,
|
||||||
size_box_inputs,
|
size_box_inputs,
|
||||||
size_output_inputs,
|
size_output_inputs,
|
||||||
|
// Written by whoever draws it, which is what resolves them.
|
||||||
|
declared: [None; 2],
|
||||||
output_px: self.output_size,
|
output_px: self.output_size,
|
||||||
move_idx,
|
move_idx,
|
||||||
parent_move,
|
parent_move,
|
||||||
@@ -753,11 +756,28 @@ impl UiRenderState {
|
|||||||
AXES.into_iter()
|
AXES.into_iter()
|
||||||
.any(|axis| pixel_len_changed(active.px.axis(axis), px.axis(axis)))
|
.any(|axis| pixel_len_changed(active.px.axis(axis), px.axis(axis)))
|
||||||
});
|
});
|
||||||
|
// A declared length is resolved into this widget's box by whoever
|
||||||
|
// drew it, so a change to one moves a box this widget cannot fix by
|
||||||
|
// drawing again, however its own size comes out. Compared rather
|
||||||
|
// than assumed: a widget dirtied for any other reason declares what
|
||||||
|
// it declared before, and redrawing its parent for that costs 17%.
|
||||||
|
let declared_changed = self.active.get(&id).is_some_and(|active| {
|
||||||
|
rsc.widgets().get_dyn(id).is_some_and(|widget| {
|
||||||
|
AXES.into_iter()
|
||||||
|
.zip(active.declared)
|
||||||
|
.any(|(axis, was)| declared_len(widget, axis) != was)
|
||||||
|
})
|
||||||
|
});
|
||||||
let top = match box_changed {
|
let top = match box_changed {
|
||||||
true => self.top_reader(id),
|
true => self.top_reader(id),
|
||||||
false => None,
|
false => None,
|
||||||
}
|
}
|
||||||
.or_else(|| self.derived_box_reader(id));
|
.or_else(|| self.derived_box_reader(id))
|
||||||
|
.or_else(|| {
|
||||||
|
declared_changed
|
||||||
|
.then(|| self.active.get(&id).and_then(|active| active.parent))
|
||||||
|
.flatten()
|
||||||
|
});
|
||||||
if let Some(top) = top {
|
if let Some(top) = top {
|
||||||
#[cfg(feature = "layout-diagnostics")]
|
#[cfg(feature = "layout-diagnostics")]
|
||||||
diag::bump(Counter::EagerReaderRedraws);
|
diag::bump(Counter::EagerReaderRedraws);
|
||||||
|
|||||||
@@ -8,20 +8,11 @@ pub struct SetSize {
|
|||||||
|
|
||||||
impl Widget for SetSize {
|
impl Widget for SetSize {
|
||||||
fn draw(&mut self, painter: &mut Painter) -> Size {
|
fn draw(&mut self, painter: &mut Painter) -> Size {
|
||||||
// A declared length is what the child gets, whatever box this widget
|
// Nothing to apply: a declared length is taken where this widget is
|
||||||
// was offered before its parent knew that. Measuring it anywhere else
|
// drawn, so the box it has already is that length, and `rest` is a
|
||||||
// asks about a box it will not have, and the answer on the other axis
|
// share only whoever divides a length can work out. Both reach them
|
||||||
// is taken under that: a wrapping text measured in the whole width
|
// through `size_hint`.
|
||||||
// reports one line, and nothing revisits it once the real width
|
let child = painter.widget(&self.inner).size();
|
||||||
// arrives.
|
|
||||||
let mut region = UiRegion::FULL;
|
|
||||||
for (axis, len) in [(Axis::X, self.x), (Axis::Y, self.y)] {
|
|
||||||
if let Some(len) = len {
|
|
||||||
let span = region.axis_mut(axis);
|
|
||||||
span.end = span.start + len.apply_rest();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
let child = painter.widget_within(&self.inner, region).size();
|
|
||||||
Size {
|
Size {
|
||||||
x: self.x.unwrap_or(child.x),
|
x: self.x.unwrap_or(child.x),
|
||||||
y: self.y.unwrap_or(child.y),
|
y: self.y.unwrap_or(child.y),
|
||||||
|
|||||||
Reference in new issue
Block a user