Stop composer layout recursion on spaces
This commit is contained in:
1 parent
5ece49b8d9
commit
e5fee03da8
4 files changed
+86
-14
No files matched your search
@@ -112,6 +112,13 @@ where
|
||||
// the transcript: measured at 58px of stray text for a 475px message
|
||||
// in a 417px box.
|
||||
let content = field
|
||||
// A wrapping editor occupies the composer's width even when its
|
||||
// current text is short. Without this inner constraint the vertical
|
||||
// ScrollArea reports the text's narrow natural width to Pad; settling
|
||||
// then offers that width back to the editor, where a trailing space
|
||||
// wraps and doubles its height. The next parent pass restores the
|
||||
// wide one-line answer, so the two layouts have no fixed point.
|
||||
.width(rest(1))
|
||||
// `scrollable_to_end`: what is being typed is at the end, so a
|
||||
// message longer than the six lines shown holds that end.
|
||||
.scrollable(Axis::Y, Pin::End)
|
||||
|
||||
@@ -225,6 +225,40 @@ fn the_composer_sits_above_a_simulated_ime_inset() {
|
||||
);
|
||||
}
|
||||
|
||||
/// A trailing space is narrower than the composer's available width but can
|
||||
/// wrap when the field is measured again in its own reported width. The
|
||||
/// settling walk must not bounce forever between those one- and two-line
|
||||
/// answers. On Android that recursion exhausted the native UI thread's stack
|
||||
/// and ended in SIGSEGV, before Rust's panic hook could write anything.
|
||||
#[test]
|
||||
fn a_space_in_the_composer_finishes_layout() {
|
||||
let (mut h, screen) = opened();
|
||||
screen.composer.set_bottom_inset(&mut h.rsc, 1000.0);
|
||||
h.frame(PHONE_FRAME_MS * 2);
|
||||
h.state.set_focus(Some(screen.composer.field));
|
||||
screen.composer.field.edit(&mut h.rsc).set_cursor_byte(0);
|
||||
|
||||
for text in ["h", "i", " "] {
|
||||
screen.composer.field.edit(&mut h.rsc).insert(text);
|
||||
h.frame(PHONE_FRAME_MS);
|
||||
}
|
||||
|
||||
assert_eq!(screen.composer.field.edit(&mut h.rsc).text.text(), "hi ");
|
||||
let region = h
|
||||
.render
|
||||
.window_region(&screen.composer.field, &h.rsc)
|
||||
.expect("the composer field is drawn");
|
||||
let size = region.bot_right - region.top_left;
|
||||
assert!(
|
||||
size.x > phone_size().x / 2.0,
|
||||
"the field shrink-wrapped to the short message: {region:?}"
|
||||
);
|
||||
assert!(
|
||||
size.y < 30.0 * PHONE_SCALE,
|
||||
"the trailing space wrapped onto a second line: {region:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// A newline typed into the composer must leave the caret inside the
|
||||
/// bar's own padding, not flush against its bottom edge.
|
||||
///
|
||||
|
||||
+8
-1
@@ -289,7 +289,14 @@ those changed widgets from the outside in, after their parents have assigned
|
||||
the final boxes. Otherwise a newly appended child can retain the provisional
|
||||
(even inverted) region it was measured in until another update happens. The
|
||||
downward work is confined to the branch that changed; unchanged descendants
|
||||
still take `draw_inner`'s retained fast path.
|
||||
still take `draw_inner`'s retained fast path. Each downward visit is exactly
|
||||
one redraw, not another upward propagation: a wrapping child can have no fixed
|
||||
point when an ancestor shrink-wraps it (a trailing space alternated between one
|
||||
line in the offered width and two lines in its reported natural width). Feeding
|
||||
that answer back into the same branch recursively overflowed Android's native
|
||||
UI-thread stack before Rust could report a panic. A container that intends a
|
||||
wrapping child to occupy its width declares that constraint explicitly; the
|
||||
message composer does so on both sides of its vertical `ScrollArea`.
|
||||
|
||||
### 4. Wrapped text, and "needs child height before choosing width"
|
||||
|
||||
|
||||
@@ -1217,18 +1217,50 @@ impl UiRenderState {
|
||||
/// newly grown subtree can retain the provisional (even inverted) region
|
||||
/// it was measured in until an unrelated later update redraws it.
|
||||
fn redraw_and_settle(&mut self, id: WidgetId, rsc: &mut dyn UiRsc) {
|
||||
let Some((parent, changed)) = self.redraw_once(id, rsc) else {
|
||||
return;
|
||||
};
|
||||
if changed {
|
||||
if let Some(pid) = parent {
|
||||
self.redraw_and_settle(pid, rsc);
|
||||
}
|
||||
// The parent pass above has now placed this widget in its final
|
||||
// region. Draw it once more there; unchanged descendants still
|
||||
// take draw_inner's retained fast path. This is deliberately one
|
||||
// redraw rather than another settling pass: feeding its size
|
||||
// back into the same upward walk can alternate between the
|
||||
// provisional and final regions forever (a text edit first did
|
||||
// that when an Android IME committed a space), overflowing the
|
||||
// native thread's stack before Rust can report a panic.
|
||||
let settled_size = self.active.get(&id).map(|active| active.size);
|
||||
let _ = self.redraw_once(id, rsc);
|
||||
debug_assert_eq!(
|
||||
self.active.get(&id).map(|active| active.size),
|
||||
settled_size,
|
||||
"a widget changed size after its parent settled its final region"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Redraw `id` exactly once, returning its parent and whether the size it
|
||||
/// reports changed. [`Self::redraw_and_settle`] owns any propagation; in
|
||||
/// particular, its final downward redraw must not start another upward
|
||||
/// pass through the same branch.
|
||||
fn redraw_once(
|
||||
&mut self,
|
||||
id: WidgetId,
|
||||
rsc: &mut dyn UiRsc,
|
||||
) -> Option<(Option<WidgetId>, bool)> {
|
||||
rsc.widgets_mut().needs_redraw.remove(&id);
|
||||
// An ancestor is drawing this widget right now, and that draw is
|
||||
// about to write fresh primitives for it. Drawing it a second time
|
||||
// here would leave one of the two copies on screen with nothing
|
||||
// owning it -- see `draw_started`'s own doc.
|
||||
if self.draw_started.contains(&id) {
|
||||
return;
|
||||
return None;
|
||||
}
|
||||
|
||||
let Some(active) = self.remove(id, false, true, rsc) else {
|
||||
return;
|
||||
};
|
||||
let active = self.remove(id, false, true, rsc)?;
|
||||
let old_size = active.size;
|
||||
let parent = active.parent;
|
||||
// `old_move_slot` being `Some` below means the slot is reused in
|
||||
@@ -1259,15 +1291,7 @@ impl UiRenderState {
|
||||
// there is no query left that answers "what size would this be"
|
||||
// without actually drawing (LAYOUT.md section 5).
|
||||
let changed = self.active.get(&id).map(|a| a.size) != Some(old_size);
|
||||
if changed {
|
||||
if let Some(pid) = parent {
|
||||
self.redraw_and_settle(pid, rsc);
|
||||
}
|
||||
// The parent pass above has now placed this widget in its final
|
||||
// region. Draw it once more there; unchanged descendants still
|
||||
// take draw_inner's retained fast path.
|
||||
self.redraw_and_settle(id, rsc);
|
||||
}
|
||||
Some((parent, changed))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user