iris: the keyboard reopens, the IME's height reaches the layout, and a fling actually moves
Items 1-3 of Iris's 22:16 phone report, plus the two defects that were hiding behind item 1 and only became visible once the first one was fixed. Emulator evidence and the numbers are in docs/RUST.md. **Keyboard reopen.** `attr.rs`'s already-focused branch calls `focus_gained` on a tap that stays inside `DRAG_SLOP` -- what Android's own `EditText` does, `showSoftInput` being idempotent. Dismissing the IME leaves the field focused, so the only branch that requested it never ran again. Negative control run: without this one call the second tap leaves `mInputShown=false`. Swipes across and out of the focused field still summon nothing. **IME height.** `MainActivity` sends `getInsets(ime()).bottom` and `isVisible(ime())` as two values; the height used to be sent *as* the boolean, so nothing had a number to pad by. `Insets`/`WindowInsets` carry both, `bench_client` reads the boolean for its state machine and the height for `Composer::set_bottom_inset`, and the list follows because it is `rest(1)` in the same `Span`. **Fling.** Three defects, in the order they were found: 1. `on_touch_event` read only each `MotionEvent`'s final position, so a batched 120Hz flick fed the tracker one sample and `velocity()` answered 0.0. Historical samples are replayed through the sensor pass now, `CursorState::time` carries each sample's own time (so a replay loop's speed cannot become the measured velocity -- the winit backend sets it too), the press is a sample as AOSP's own tracker does, and `iris drag release:` logs the decision for the phone's logcat. 2. Nothing advanced a fling between input events: `tick_fling`'s only caller was the benchmark's own loop, so the bench flung and a finger never did. iris has one animation mechanism now -- `Widget::tick`, `UiData::animate`/`tick_animations`, called by both backends before the draw and re-requesting a frame while it answers true. 3. With flings finally animating, one lasted 45 seconds: `List::fling` hardcoded density 1.0 against physical-pixel velocities, and `FlingCalculator`'s coefficient used the scroll friction where AOSP uses its 0.84 tuning constant -- 56x, inside an exponential. Emulator: 1.62s for v=11064, against AOSP's own 1.586s. **Two pre-existing faults found on the way.** `MOVE_CHAIN_LIMIT` was 16 and the composer's chain is 17, so every debug build aborted on a tap of the composer and every release build silently drew and hit-tested that subtree short; it is 64 in both the CPU walk and shader.wgsl, and the assert prints the chain so a cycle and a deep tree can be told apart. And `minSdk` is 29, since `getEventTimeNanos` is API 29 and a missing JNI method is a crash rather than a degraded fling. Every new invariant carries its guard: sample times non-decreasing in `on_touch_event`, and tests confirmed to fail without their fix for the press-seeded velocity, the animation registration and the AOSP magnitudes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
ba2afbaedb
commit
ed04d4c735
23 files changed
+735
-87
No files matched your search
@@ -80,11 +80,17 @@ var<storage> masks: array<Mask>;
|
||||
@group(3) @binding(1)
|
||||
var<storage> move_offsets: array<MoveOffset>;
|
||||
|
||||
// A move chain more than this deep means something else is wrong (an
|
||||
// accidental cycle) -- kept in step with `MOVE_CHAIN_LIMIT` in
|
||||
// render_state.rs, which walks the identical bound on the CPU side for
|
||||
// hit-testing. Bounded so a malformed chain cannot hang the GPU.
|
||||
const MOVE_CHAIN_LIMIT: u32 = 16u;
|
||||
// The bound on the parent walk, kept in step with `MOVE_CHAIN_LIMIT` in
|
||||
// render_state.rs, which walks the identical chain on the CPU side for
|
||||
// hit-testing. Bounded so a malformed chain (a cyclic `parent`) cannot
|
||||
// hang the GPU -- not a claim about how deep a real tree gets. It was 16
|
||||
// and that was too small: the transcript screen's composer field sits 17
|
||||
// slots below the root, measured 2026-09-07 on this checkout's emulator
|
||||
// by tapping it (the CPU walk's own debug assert names the chain now).
|
||||
// Past the bound both walks simply stop summing, so the widget draws and
|
||||
// hit-tests short by whatever the outer slots held, with nothing on
|
||||
// screen to say so.
|
||||
const MOVE_CHAIN_LIMIT: u32 = 64u;
|
||||
|
||||
/// Sums the pixel delta along the parent chain starting at `idx`, shared by
|
||||
/// the vertex stage (a primitive's own corners) and the fragment stage (its
|
||||
|
||||
@@ -24,6 +24,46 @@ pub struct UiData {
|
||||
/// id (never reallocated), so a retained descendant's `parent` index
|
||||
/// never goes stale -- see LAYOUT.md section 2.
|
||||
pub move_offsets: TrackedArena<MoveOffset, u32>,
|
||||
/// Every widget whose [`crate::Widget::tick`] should run before the
|
||||
/// next frame -- today, a `List` coasting through a fling. Added by
|
||||
/// [`Self::animate`] when the animation starts and removed by
|
||||
/// [`Self::tick_animations`] the frame its `tick` answers `false`, so
|
||||
/// a stopped animation costs nothing and a dropped widget cannot be
|
||||
/// ticked (`get_dyn_mut` answers `None` and it is dropped the same
|
||||
/// way).
|
||||
animating: Vec<WidgetId>,
|
||||
}
|
||||
|
||||
impl UiData {
|
||||
/// Ask for `id`'s [`crate::Widget::tick`] to run every frame until it
|
||||
/// says it is done. Idempotent -- registering an already-animating
|
||||
/// widget is the ordinary case (a second fling before the first
|
||||
/// settled) and must not tick it twice per frame.
|
||||
pub fn animate(&mut self, id: WidgetId) {
|
||||
if !self.animating.contains(&id) {
|
||||
self.animating.push(id);
|
||||
}
|
||||
}
|
||||
|
||||
/// Tick every registered widget to `now`, drop the ones that finished,
|
||||
/// and say whether any is still going -- which is a backend's cue to
|
||||
/// ask for another frame. Called once per frame *before* the draw, so
|
||||
/// what the frame draws is this instant's position rather than the
|
||||
/// previous one's.
|
||||
pub fn tick_animations(&mut self, now: std::time::Instant) -> bool {
|
||||
// Taken out and put back rather than iterated in place: `tick`
|
||||
// needs `&mut` on the widget arena this list lives beside, and a
|
||||
// widget is free to register another one while ticking.
|
||||
let mut registered = std::mem::take(&mut self.animating);
|
||||
registered.retain(|&id| match self.widgets.get_dyn_mut(id) {
|
||||
Some(widget) => widget.tick(now),
|
||||
None => false,
|
||||
});
|
||||
for id in registered {
|
||||
self.animate(id);
|
||||
}
|
||||
!self.animating.is_empty()
|
||||
}
|
||||
}
|
||||
|
||||
pub trait UiRsc {
|
||||
|
||||
@@ -60,10 +60,17 @@ pub struct UiRenderState {
|
||||
pub(super) shape_count: u64,
|
||||
}
|
||||
|
||||
/// A move chain more than this deep would mean something else is wrong
|
||||
/// (an accidental cycle) -- see `resolve_move` in shader.wgsl, which walks
|
||||
/// the identical bound and must be kept in step with this constant.
|
||||
pub const MOVE_CHAIN_LIMIT: usize = 16;
|
||||
/// The bound on the parent walk -- see `resolve_move` in shader.wgsl,
|
||||
/// which walks the identical chain and must be kept in step with this
|
||||
/// constant. It exists so a cyclic `parent` link cannot hang either walk,
|
||||
/// not as a statement about how deep a real tree gets: it was 16, and the
|
||||
/// transcript screen's composer field turned out to sit **17** slots below
|
||||
/// the root (measured 2026-09-07 on this checkout's emulator, by tapping
|
||||
/// the composer in a debug build -- the assert in `resolve_move_chain`
|
||||
/// prints the chain). A chain past the bound is not reported anywhere at
|
||||
/// run time; both walks just stop summing, so the widget is drawn and hit
|
||||
/// tested short by whatever the outer slots held.
|
||||
pub const MOVE_CHAIN_LIMIT: usize = 64;
|
||||
|
||||
impl UiRenderState {
|
||||
pub fn new() -> Self {
|
||||
@@ -708,26 +715,56 @@ impl UiRenderState {
|
||||
/// pixel delta along the parent chain starting at `slot`. Both walks
|
||||
/// share `MOVE_CHAIN_LIMIT` as their bound so the two cannot disagree
|
||||
/// about where the chain ends.
|
||||
fn resolve_move_chain(&self, mut slot: MoveIdx, rsc: &dyn UiRsc) -> Vec2 {
|
||||
fn resolve_move_chain(&self, slot: MoveIdx, rsc: &dyn UiRsc) -> Vec2 {
|
||||
let offsets = &rsc.ui().move_offsets;
|
||||
let mut delta = Vec2::ZERO;
|
||||
let mut at = slot;
|
||||
for i in 0..MOVE_CHAIN_LIMIT {
|
||||
let entry = &offsets[slot.idx()];
|
||||
let entry = &offsets[at.idx()];
|
||||
delta.x += entry.delta[0];
|
||||
delta.y += entry.delta[1];
|
||||
if entry.parent == MoveOffset::NONE_PARENT {
|
||||
return delta;
|
||||
}
|
||||
slot = Id::preset(entry.parent);
|
||||
at = Id::preset(entry.parent);
|
||||
// The chain itself, not just the fact that it was too long: a
|
||||
// cycle and a tree genuinely nested deeper than the shader can
|
||||
// follow are different faults with different fixes, and the
|
||||
// slot numbers are the only thing that tells them apart.
|
||||
debug_assert!(
|
||||
i + 1 < MOVE_CHAIN_LIMIT,
|
||||
"move offset chain exceeded MOVE_CHAIN_LIMIT; a widget's `parent` link is \
|
||||
probably cyclic"
|
||||
"move offset chain exceeded MOVE_CHAIN_LIMIT ({MOVE_CHAIN_LIMIT}): {chain} -- a \
|
||||
repeated slot means a `parent` link is cyclic, all-distinct slots mean the tree \
|
||||
nests deeper than shader.wgsl's own walk of the same bound",
|
||||
chain = Self::move_chain_debug(slot, offsets)
|
||||
);
|
||||
}
|
||||
delta
|
||||
}
|
||||
|
||||
/// The parent chain from `slot`, as `slot(dx, dy) -> ...`, walked twice
|
||||
/// `MOVE_CHAIN_LIMIT` so a cycle shows up as a repeated slot number
|
||||
/// rather than as a chain that merely stops. Only ever called from the
|
||||
/// failed assertion above.
|
||||
fn move_chain_debug(slot: MoveIdx, offsets: &[MoveOffset]) -> String {
|
||||
let mut parts = Vec::new();
|
||||
let mut at = slot;
|
||||
for _ in 0..MOVE_CHAIN_LIMIT * 2 {
|
||||
let entry = &offsets[at.idx()];
|
||||
parts.push(format!(
|
||||
"{}({}, {})",
|
||||
at.idx(),
|
||||
entry.delta[0],
|
||||
entry.delta[1]
|
||||
));
|
||||
if entry.parent == MoveOffset::NONE_PARENT {
|
||||
break;
|
||||
}
|
||||
at = Id::preset(entry.parent);
|
||||
}
|
||||
parts.join(" -> ")
|
||||
}
|
||||
|
||||
pub fn window_region(&self, id: &impl IdLike, rsc: &dyn UiRsc) -> Option<PixelRegion> {
|
||||
let region = self.resolved_region(id, rsc)?;
|
||||
Some(region.to_px(self.output_size))
|
||||
|
||||
@@ -41,6 +41,25 @@ pub trait Widget: Any {
|
||||
fn access_role(&self) -> accesskit::Role {
|
||||
accesskit::Role::Unknown
|
||||
}
|
||||
|
||||
/// Advance whatever this widget is animating to `now`, and say whether
|
||||
/// it is still animating afterwards. Default: nothing is, so a widget
|
||||
/// opts in by overriding this *and* by something calling
|
||||
/// [`crate::UiData::animate`] with its id when the animation starts --
|
||||
/// which is that animation's path out, since the driver
|
||||
/// ([`crate::UiData::tick_animations`]) drops every id whose `tick`
|
||||
/// answers `false`.
|
||||
///
|
||||
/// Called once per frame, before the frame's draw, by whichever
|
||||
/// backend owns the surface; a `true` answer is what makes that
|
||||
/// backend ask for another frame. So this is the only thing in iris
|
||||
/// that moves without an input event, and a widget that animates
|
||||
/// without registering simply never moves -- which is exactly how a
|
||||
/// finger fling looked on Iris's phone before this existed.
|
||||
#[allow(unused_variables)]
|
||||
fn tick(&mut self, now: std::time::Instant) -> bool {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
impl Widget for () {
|
||||
|
||||
Reference in new issue
Block a user