Merge remote-tracking branch 'origin/rustify' into worktree-agent-a6e37a2335f436d08
This commit is contained in:
commit
312455956d
10 files changed
+621
-86
No files matched your search
@@ -183,6 +183,48 @@ agent takes them without colliding with that pass's `bench_client.rs`/
|
||||
handling is a follow-up, the same shape `List`'s own touch-drag pan
|
||||
needed before I3/I5.
|
||||
|
||||
## From the phone, 2026-09-06, 11:39 (build delivered 02:07, commit 543f6d9)
|
||||
|
||||
Iris's report on the build with the composing-text, tap-vs-swipe and
|
||||
atlas-reset fixes, with a screenshot, verbatim. Each is open until an
|
||||
agent ticks it here with the evidence.
|
||||
|
||||
- [ ] **"The app definitely does not start with keyboard spacing
|
||||
correct. This is how it looks without me doing anything initially."**
|
||||
The screenshot shows the composer bar (the grey band) sitting about
|
||||
two thirds of the way down a 704x1568 screen, with black below it to
|
||||
the bottom, and the transcript ending at "Claude / Results" just above
|
||||
it -- at launch, no keyboard. So the composer's bottom padding, which
|
||||
the 2026-09-06 rebuild tied to the IME/nav-bar inset, is being fed a
|
||||
large value at start on the phone. Suspects, in order: the initial
|
||||
inset delivery on the phone (GrapheneOS, gesture navigation) versus
|
||||
the emulator; `ime_bottom` now carrying a `1`/`0` boolean through a
|
||||
field the composer may still read as pixels or dp; a stale value from
|
||||
before the first `on_insets_changed`. Reproduce with the phone's
|
||||
screen size and density on the emulator before guessing.
|
||||
- [ ] **"Swiping still gets caught by the grey bar but keeps working
|
||||
after I go past it."** A pan that starts on the composer is held by
|
||||
the composer until the finger leaves its region, then the list takes
|
||||
over. The tap-vs-swipe fix in `attr.rs` stops the *focus*, but the
|
||||
press frames are still being handled by the field rather than passed
|
||||
to the list from the first slop-crossing frame. The `DragGesture`
|
||||
merge (RUST.md's plan box) should make this one mechanism: once a
|
||||
gesture commits to a pan, the list captures it wherever it began.
|
||||
- [ ] **"Flinging still does not work."** Expected on this build: finger
|
||||
flings are dropped by per-widget hit testing, which `DragGesture`'s
|
||||
pointer capture (commit `e12c708`, not yet merged at 02:07) targets.
|
||||
Stays open until verified on her phone, not the emulator.
|
||||
- [ ] **"Text still disappears if I leave and come back to the app."**
|
||||
The `GlyphAtlas::clear`/`Textures::reset` fix was verified on the
|
||||
emulator under `force-gles` only; the phone runs Vulkan. So either the
|
||||
reset is not reached on the phone's path (a different surface-
|
||||
lifecycle sequence -- `surface_destroyed`/`surface_created` ordering,
|
||||
or the renderer not being rebuilt but its textures lost), or the CPU
|
||||
glyph cache and the GPU atlas still disagree after it. Needs logging
|
||||
of the renderer lifecycle on the phone build, readable from `adb
|
||||
logcat` when Iris next runs it, since no emulator here has a Vulkan
|
||||
adapter under host GPU.
|
||||
|
||||
## Build
|
||||
|
||||
- [x] **Benchmarks**, not unit tests, run on demand (2026-09-05; a
|
||||
|
||||
@@ -63,6 +63,14 @@ closes it.
|
||||
next tap anywhere panics. Findings 2–5 are `debug_assert!`s on
|
||||
invariants, 8 and 10 are the missing tests. Commit the review file
|
||||
with the fixes.
|
||||
- [ ] **Iris's 11:39 phone report on the 02:07 build** (four items,
|
||||
verbatim in `IRIS_TODO.md`'s "From the phone, 2026-09-06, 11:39"):
|
||||
composer floating two thirds down the screen at launch with black
|
||||
below it; a swipe starting on the composer held until the finger
|
||||
leaves it; no fling (expected, `DragGesture` unmerged); text still
|
||||
lost on app-switch on the phone despite the emulator-verified
|
||||
atlas reset. The first and last are the same class as the next
|
||||
box and go to that agent; the middle two are the merge box's.
|
||||
- [ ] **Stale primitives and invisible composer text** — the header drawn
|
||||
twice after a keyboard resize, the `Compacted:` row drawn twice on
|
||||
Iris's phone, and typed text never appearing (P0 box item 2). All
|
||||
|
||||
@@ -31,6 +31,25 @@ public final class MainActivity extends Activity {
|
||||
setContentView(layout);
|
||||
view.requestFocus();
|
||||
|
||||
// RUST.md's P0 box, defect 4 ("keyboard: could not be shown"):
|
||||
// `logcat` showed the platform's own IME open/resize happening
|
||||
// while `setOnApplyWindowInsetsListener` fired only once, at
|
||||
// attach, and never again for a pure keyboard toggle -- a plain
|
||||
// (non-edge-to-edge) window is only guaranteed that one initial
|
||||
// dispatch; `adjustResize` handling the IME entirely by resizing
|
||||
// the window is not itself a trigger for a fresh one. Opting into
|
||||
// edge-to-edge (a platform call, API 30+, no new dependency) is
|
||||
// what makes the system redeliver insets on every change,
|
||||
// including the ones this activity actually cares about --
|
||||
// `getSystemWindowInset*` below is unaffected by this (it has
|
||||
// always reported the raw system-bar/IME overlap regardless of
|
||||
// who consumes it), so the on-screen bars and the padding Rust
|
||||
// already derives from those four numbers are unchanged; only the
|
||||
// callback's firing became reliable.
|
||||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
|
||||
getWindow().setDecorFitsSystemWindows(false);
|
||||
}
|
||||
|
||||
view.setOnApplyWindowInsetsListener((v, insets) -> {
|
||||
int left = insets.getSystemWindowInsetLeft();
|
||||
int top = insets.getSystemWindowInsetTop();
|
||||
|
||||
@@ -20,6 +20,21 @@ pub struct UiRenderState {
|
||||
resized: bool,
|
||||
draw_started: HashSet<WidgetId>,
|
||||
|
||||
/// The widget currently holding exclusive pointer input, if any --
|
||||
/// `iris::sense::SensorUi::run_sensors` reads and clears this every
|
||||
/// call. Interior mutability (a `Mutex`, not a bare `Cell`, since a
|
||||
/// `CursorData` reaching this through an async `task_on` handler needs
|
||||
/// `Send`/`Sync`) because `run_sensors` takes `&self` (widgets are
|
||||
/// dispatched to, not owned, at that layer) and this render state is
|
||||
/// the one structure both backends (winit, android-view) already hold
|
||||
/// across frames, the same way `old_root`/`resized` are -- see
|
||||
/// `iris::sense`'s pointer-capture doc for why a drag needs this: once
|
||||
/// a gesture has committed to panning or selecting, every later sample
|
||||
/// of it must reach the same widget even if the finger has moved off
|
||||
/// whatever hit region first noticed the press. Never held across an
|
||||
/// await or another lock -- every access here is a single get/set.
|
||||
captured: std::sync::Mutex<Option<WidgetId>>,
|
||||
|
||||
/// `Widget::draw` calls and `Primitives::region_mut` rewrites since the
|
||||
/// last `take_counters`. LAYOUT.md section 8's pass conditions are
|
||||
/// stated in terms of these two: an unchanged frame must cost 0 of
|
||||
@@ -45,6 +60,7 @@ impl UiRenderState {
|
||||
old_root: None,
|
||||
resized: false,
|
||||
draw_started: Default::default(),
|
||||
captured: Default::default(),
|
||||
draw_count: 0,
|
||||
region_mut_count: 0,
|
||||
mov_count: 0,
|
||||
@@ -357,6 +373,13 @@ impl UiRenderState {
|
||||
active.textures.clear();
|
||||
rsc.ui_mut().textures.free();
|
||||
if undraw {
|
||||
// A captured widget that goes away mid-gesture (List's
|
||||
// virtualisation retiring a row, a rebuild) must not leave
|
||||
// the pointer permanently captured by an id nothing will
|
||||
// ever draw again -- `captured`'s own path out.
|
||||
if *self.captured.lock().unwrap() == Some(id) {
|
||||
*self.captured.lock().unwrap() = None;
|
||||
}
|
||||
// Permanent removal: retire this widget's own move slot
|
||||
// (the self-ownership ref taken when it was allocated) and
|
||||
// the up-link ref it held on its parent's slot -- read from
|
||||
@@ -429,6 +452,27 @@ impl UiRenderState {
|
||||
self.active.len()
|
||||
}
|
||||
|
||||
/// Give `id` exclusive pointer input from the next `run_sensors` call
|
||||
/// on -- see `captured`'s field doc. Overwrites any previous capture
|
||||
/// (a gesture that starts a new one has already decided the old one
|
||||
/// is over).
|
||||
pub fn capture_pointer(&self, id: WidgetId) {
|
||||
*self.captured.lock().unwrap() = Some(id);
|
||||
}
|
||||
|
||||
/// Release exclusive pointer input, if any is held -- called once
|
||||
/// `run_sensors` has delivered the terminal `Drop` to the capturing
|
||||
/// widget, or by that widget itself if it decides the gesture is over
|
||||
/// some other way.
|
||||
pub fn release_pointer(&self) {
|
||||
*self.captured.lock().unwrap() = None;
|
||||
}
|
||||
|
||||
/// The widget currently holding exclusive pointer input, if any.
|
||||
pub fn captured_pointer(&self) -> Option<WidgetId> {
|
||||
*self.captured.lock().unwrap()
|
||||
}
|
||||
|
||||
pub fn debug(&self, widgets: &Widgets, label: &str) -> impl Iterator<Item = &ActiveData> {
|
||||
self.active.iter().filter_map(move |(&id, inst)| {
|
||||
let l = widgets.label(id);
|
||||
|
||||
@@ -22,6 +22,14 @@ pub enum CursorSense {
|
||||
Hovering,
|
||||
HoverEnd,
|
||||
Scroll,
|
||||
/// Delivered exactly once, in place of `PressEnd`, to whichever widget
|
||||
/// currently holds pointer capture (`UiRenderState::capture_pointer`)
|
||||
/// when the button lifts -- see `iris::sense`'s pointer-capture doc
|
||||
/// and `DragGesture`. A widget must register this explicitly (it is
|
||||
/// never bundled into `click_or_drag`/`unclick`, since most widgets
|
||||
/// never call `capture_pointer` and have no use for it) to receive it
|
||||
/// at all; ordinary hit-tested widgets keep seeing `PressEnd`.
|
||||
Drop,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
@@ -31,6 +39,21 @@ impl Event for CursorSenses {
|
||||
type Data<'a> = CursorData<'a>;
|
||||
type State = SensorState;
|
||||
fn should_run<'a>(&self, data: &Self::Data<'a>) -> Option<Self::Data<'a>> {
|
||||
// `Drop` is never derived from raw cursor/hover state below (the
|
||||
// free `should_run`'s own arm for it is only ever asked here,
|
||||
// never independently true or false against the button) -- it is
|
||||
// set exclusively by `run_sensors`' pointer-capture branch, which
|
||||
// has already decided this exact frame is the captured widget's
|
||||
// terminal event. Matching it by identity, ahead of the general
|
||||
// derivation, matters because a captured widget's registration
|
||||
// list very likely also carries `PressEnd` (`unclick()`, for the
|
||||
// ordinary un-captured case) -- the same button-lift condition
|
||||
// `PressEnd` matches on, so falling through to the loop below
|
||||
// would let whichever of the two happens to be registered first
|
||||
// win, silently swallowing the `Drop` a caller relied on.
|
||||
if data.sense == CursorSense::Drop {
|
||||
return self.contains(&CursorSense::Drop).then(|| data.clone());
|
||||
}
|
||||
if let Some(sense) = should_run(self, &data.cursor, data.hover) {
|
||||
let mut data = data.clone();
|
||||
data.sense = sense;
|
||||
@@ -177,6 +200,48 @@ impl SensorUi for UiRenderState {
|
||||
cursor: CursorState,
|
||||
window_size: Vec2,
|
||||
) {
|
||||
// Exclusive pointer capture (`UiRenderState::capture_pointer`,
|
||||
// `DragGesture`): once some widget has committed to a drag, every
|
||||
// other widget sees nothing from this pointer at all -- no hover,
|
||||
// no click, no press -- until it releases. This is what lets a
|
||||
// fast pan or a selection keep going once the finger has moved
|
||||
// off whatever hit region first noticed the press (including
|
||||
// right off the end of the gesture, at `PressEnd`/`Cancel`): a
|
||||
// per-widget hit test would otherwise silently stop delivering to
|
||||
// *anyone* the moment the pointer left every registered region,
|
||||
// which is exactly what used to leave a fling never started (no
|
||||
// widget ever saw the release). The captured widget keeps getting
|
||||
// ordinary `Pressing` frames while the button is down and gets
|
||||
// exactly one `Drop` -- not `PressEnd` -- the frame it lifts,
|
||||
// which also releases the capture.
|
||||
if let Some(id) = self.captured_pointer() {
|
||||
let Some(shape) = self.resolved_region(&id, rsc) else {
|
||||
self.release_pointer();
|
||||
return;
|
||||
};
|
||||
let region = shape.to_px(window_size);
|
||||
let button_down = cursor.buttons.select(&CursorButton::Left).is_on();
|
||||
let sense = if button_down {
|
||||
CursorSense::Pressing(CursorButton::Left)
|
||||
} else {
|
||||
CursorSense::Drop
|
||||
};
|
||||
let data = CursorData {
|
||||
pos: cursor.pos - region.top_left,
|
||||
size: region.bot_right - region.top_left,
|
||||
scroll_delta: cursor.scroll_delta,
|
||||
hover: ActivationState::On,
|
||||
cursor: cursor.clone(),
|
||||
sense,
|
||||
render: self,
|
||||
};
|
||||
rsc.run_event::<CursorSense>(id, data, state);
|
||||
if !button_down {
|
||||
self.release_pointer();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
// in order to remove this take, need to store active list in UiRenderState somehow
|
||||
// this would probably be done through a generic parameter that adds yet another rsc /
|
||||
// state like thing, but local to render state, and is passed to UiRsc events so you can
|
||||
@@ -266,6 +331,15 @@ pub fn should_run(
|
||||
CursorSense::Hovering => hover.is_on(),
|
||||
CursorSense::HoverEnd => hover.is_end(),
|
||||
CursorSense::Scroll => cursor.scroll_delta != Vec2::ZERO,
|
||||
// Never derived here -- `Drop` only ever fires through
|
||||
// `CursorSenses::should_run`'s own special case, ahead of this
|
||||
// loop, for the one widget `run_sensors`' capture branch is
|
||||
// delivering it to this frame. If this arm answered from raw
|
||||
// button state instead, an ordinary hit-tested widget that
|
||||
// happened to register `Drop` (with no capture involved at
|
||||
// all) would see it fire on every plain button-up under the
|
||||
// cursor.
|
||||
CursorSense::Drop => false,
|
||||
} {
|
||||
return Some(*sense);
|
||||
}
|
||||
@@ -541,6 +615,138 @@ impl DragArbiter {
|
||||
}
|
||||
}
|
||||
|
||||
/// What a [`DragGesture`] decided this frame -- [`DragOutcome`] plus the
|
||||
/// one further state a shared gesture needs: the drag ending, with the
|
||||
/// released velocity if (and only if) it had committed to panning.
|
||||
#[derive(Debug, Clone, Copy, PartialEq)]
|
||||
pub enum GestureOutcome {
|
||||
Undecided,
|
||||
/// Same units and sign as [`DragOutcome::Pan`] -- the caller's own
|
||||
/// convention (`List::scroll`'s, for a transcript) to apply.
|
||||
Pan(f32),
|
||||
SelectStart,
|
||||
SelectExtend,
|
||||
/// The drag ended -- `PressEnd` or the capture's own terminal `Drop`.
|
||||
/// `Some(velocity)` only if the gesture had committed to panning
|
||||
/// (never a tap, a long-press selection, or one still `Undecided`);
|
||||
/// same units as `Pan`, so a caller hands it to `List::fling` with
|
||||
/// whatever sign flip it already applies to `Pan`.
|
||||
Released(Option<f32>),
|
||||
}
|
||||
|
||||
/// Bundles a [`DragArbiter`] and a [`VelocityTracker`] into the one thing
|
||||
/// most drag-driven widgets need: arbitrate pan-vs-hold, track the pan's
|
||||
/// velocity, and take pointer capture (`UiRenderState::capture_pointer`)
|
||||
/// the moment the gesture commits so the rest of it -- including the
|
||||
/// terminal release -- keeps reaching the same widget even after the
|
||||
/// finger has moved off whatever hit region first noticed the press. Iris
|
||||
/// asked for this to live here rather than in `transcript-ui::Selection`
|
||||
/// (2026-09-06, recorded in `IRIS.md`): "dragging should be part of the
|
||||
/// default input system ... anything that provides good performance and
|
||||
/// can be generalized well is part of iris rather than the app." A caller
|
||||
/// still decides what a committed pan or a completed selection *means*
|
||||
/// (transcript-ui's pan-vs-select is one call site; a slider or a plain
|
||||
/// scroll area is another) -- this only owns the *mechanics* every one of
|
||||
/// them would otherwise duplicate.
|
||||
pub struct DragGesture {
|
||||
arbiter: DragArbiter,
|
||||
velocity: VelocityTracker,
|
||||
}
|
||||
|
||||
impl Default for DragGesture {
|
||||
fn default() -> Self {
|
||||
Self::new()
|
||||
}
|
||||
}
|
||||
|
||||
impl DragGesture {
|
||||
pub fn new() -> Self {
|
||||
Self {
|
||||
arbiter: DragArbiter::new(),
|
||||
velocity: VelocityTracker::new(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether this gesture has no press in flight -- a thin passthrough
|
||||
/// to the underlying `DragArbiter::is_idle`, for a caller (a test, a
|
||||
/// diagnostic) that wants to observe the recovery behaviour `handle`'s
|
||||
/// idle-recovery branch documents without reaching into a private
|
||||
/// field.
|
||||
pub fn is_idle(&self) -> bool {
|
||||
self.arbiter.is_idle()
|
||||
}
|
||||
|
||||
/// Feed one frame of a gesture through. `id` is the widget iris should
|
||||
/// give exclusive pointer input to once this gesture commits to
|
||||
/// panning or selecting -- a stable widget that outlives the gesture
|
||||
/// (a `List`'s own id, not one of its virtualised rows, which can be
|
||||
/// retired mid-drag as content scrolls). `render` is `CursorData`'s
|
||||
/// own field, already in hand at every call site. `already_selected`
|
||||
/// only matters for the first frame of a gesture (`PressStart`, or the
|
||||
/// recovery branch below) -- see `DragArbiter::press_start`'s doc.
|
||||
pub fn handle(
|
||||
&mut self,
|
||||
render: &UiRenderState,
|
||||
id: WidgetId,
|
||||
sense: CursorSense,
|
||||
pos_window: Vec2,
|
||||
now: Instant,
|
||||
already_selected: bool,
|
||||
) -> GestureOutcome {
|
||||
match sense {
|
||||
CursorSense::PressStart(_) => {
|
||||
self.velocity.reset();
|
||||
self.arbiter.press_start(pos_window, now, already_selected);
|
||||
self.dispatch(render, id, pos_window, now)
|
||||
}
|
||||
CursorSense::Drop | CursorSense::PressEnd(_) => {
|
||||
let released = if self.arbiter.is_panning() {
|
||||
Some(self.velocity.velocity())
|
||||
} else {
|
||||
None
|
||||
};
|
||||
self.arbiter.release();
|
||||
render.release_pointer();
|
||||
GestureOutcome::Released(released)
|
||||
}
|
||||
// See `DragArbiter::update`'s own doc: a `Pressing` frame can
|
||||
// arrive with no matching `PressStart` if the touch-down
|
||||
// landed outside whichever hit region first noticed it.
|
||||
_ if self.arbiter.is_idle() => {
|
||||
self.velocity.reset();
|
||||
self.arbiter.press_start(pos_window, now, already_selected);
|
||||
self.dispatch(render, id, pos_window, now)
|
||||
}
|
||||
_ => self.dispatch(render, id, pos_window, now),
|
||||
}
|
||||
}
|
||||
|
||||
fn dispatch(
|
||||
&mut self,
|
||||
render: &UiRenderState,
|
||||
id: WidgetId,
|
||||
pos: Vec2,
|
||||
now: Instant,
|
||||
) -> GestureOutcome {
|
||||
match self.arbiter.update(pos, now) {
|
||||
DragOutcome::Undecided => GestureOutcome::Undecided,
|
||||
DragOutcome::Pan(dy) => {
|
||||
render.capture_pointer(id);
|
||||
self.velocity.add_sample(dy, now);
|
||||
GestureOutcome::Pan(dy)
|
||||
}
|
||||
DragOutcome::SelectStart => {
|
||||
render.capture_pointer(id);
|
||||
GestureOutcome::SelectStart
|
||||
}
|
||||
DragOutcome::SelectExtend => {
|
||||
render.capture_pointer(id);
|
||||
GestureOutcome::SelectExtend
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// How far back a [`VelocityTracker`] looks when estimating a fling's
|
||||
/// initial speed -- Android's own `VelocityTracker` defaults to a similar
|
||||
/// short window so a gesture's last flick dominates over its slower start.
|
||||
|
||||
@@ -122,3 +122,127 @@ fn a_button_over_a_list_scrolls_the_list_and_still_clicks() {
|
||||
"the button on top must still receive an actual click"
|
||||
);
|
||||
}
|
||||
|
||||
/// The bug behind "finger flings do nothing" (RUST.md's P0 phone report,
|
||||
/// defect 2): a fast gesture's `PressEnd` can land at a screen position
|
||||
/// nothing is registered at -- past the edge of whatever widget noticed
|
||||
/// the press, in a gap, or off the loaded content entirely. Before pointer
|
||||
/// capture, `run_sensors`' hit test simply delivered nothing that frame,
|
||||
/// so a widget mid-drag never saw its release and never got a chance to
|
||||
/// start a fling. `UiRenderState::capture_pointer`/`DragGesture` fix this
|
||||
/// by giving the drag's widget every frame regardless of where the
|
||||
/// pointer is, including the terminal `Drop` in place of `PressEnd`.
|
||||
#[test]
|
||||
fn a_release_outside_every_hit_region_still_reaches_the_captured_widget() {
|
||||
let mut rsc = SenseRsc {
|
||||
ui: UiData::default(),
|
||||
events: EventManager::default(),
|
||||
};
|
||||
|
||||
// A small draggable widget in the corner -- the release below lands
|
||||
// far outside it, exactly the "moved off the hit region" case.
|
||||
let draggable = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE)).any();
|
||||
let draggable_weak = draggable.weak();
|
||||
|
||||
let dropped = Rc::new(Cell::new(false));
|
||||
{
|
||||
let dropped = dropped.clone();
|
||||
rsc.register_event(
|
||||
draggable_weak,
|
||||
CursorSense::click_or_drag() | CursorSense::unclick() | CursorSense::Drop,
|
||||
move |ctx, rsc| match ctx.data.sense {
|
||||
CursorSense::PressStart(_) | CursorSense::Pressing(_) => {
|
||||
// Any committed drag takes capture -- a real caller
|
||||
// would gate this on a `DragArbiter`/`DragGesture`
|
||||
// decision, but this test only needs to exercise the
|
||||
// capture-and-release mechanics themselves.
|
||||
ctx.data.render.capture_pointer(draggable_weak.id());
|
||||
let _ = rsc;
|
||||
}
|
||||
CursorSense::Drop => dropped.set(true),
|
||||
_ => {}
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
let mut render = UiRenderState::new();
|
||||
render.resize((100.0, 100.0));
|
||||
render.update(&draggable, &mut rsc);
|
||||
|
||||
let mut state = ();
|
||||
let mut press = cursor_at((5.0, 5.0).into());
|
||||
press.buttons.left = ActivationState::Start;
|
||||
render.run_sensors(&mut rsc, &mut state, press, (100.0, 100.0).into());
|
||||
assert_eq!(
|
||||
render.captured_pointer(),
|
||||
Some(draggable.id()),
|
||||
"the press should have taken capture"
|
||||
);
|
||||
|
||||
// The release lands nowhere near the widget's own region -- the exact
|
||||
// shape of a fast fling's `ACTION_UP`.
|
||||
let mut release = cursor_at((95.0, 95.0).into());
|
||||
release.buttons.left = ActivationState::End;
|
||||
render.run_sensors(&mut rsc, &mut state, release, (100.0, 100.0).into());
|
||||
|
||||
assert!(
|
||||
dropped.get(),
|
||||
"a release outside every widget's hit region must still reach \
|
||||
the widget holding pointer capture"
|
||||
);
|
||||
assert_eq!(
|
||||
render.captured_pointer(),
|
||||
None,
|
||||
"Drop must release the capture"
|
||||
);
|
||||
}
|
||||
|
||||
/// A widget that never registers `CursorSense::Drop` at all must not be
|
||||
/// affected by someone else's capture -- capture is per-gesture, not
|
||||
/// global suppression of the whole input system for widgets that were
|
||||
/// never party to it. (Practically this matters because a captured
|
||||
/// widget's registration list still has to include `Drop` for `should_run`
|
||||
/// to ever match it; this pins that half of the contract.)
|
||||
#[test]
|
||||
fn capturing_one_widget_starves_every_other_widget_of_events() {
|
||||
let mut rsc = SenseRsc {
|
||||
ui: UiData::default(),
|
||||
events: EventManager::default(),
|
||||
};
|
||||
|
||||
let a = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE));
|
||||
let a_weak = a.weak();
|
||||
let b = rsc.ui.widgets.add_strong(Rect::new(UiColor::RED));
|
||||
let b_weak = b.weak();
|
||||
|
||||
let b_hovered = Rc::new(Cell::new(false));
|
||||
{
|
||||
let b_hovered = b_hovered.clone();
|
||||
rsc.register_event(b_weak, CursorSense::Hovering, move |_ctx, _rsc| {
|
||||
b_hovered.set(true);
|
||||
});
|
||||
}
|
||||
|
||||
let root = rsc
|
||||
.ui
|
||||
.widgets
|
||||
.add_strong(Stack {
|
||||
children: vec![a.any(), b.any()],
|
||||
size: StackSize::default(),
|
||||
})
|
||||
.any();
|
||||
|
||||
let mut render = UiRenderState::new();
|
||||
render.resize((100.0, 100.0));
|
||||
render.update(&root, &mut rsc);
|
||||
render.capture_pointer(a_weak.id());
|
||||
|
||||
let mut state = ();
|
||||
let cursor = cursor_at((50.0, 50.0).into());
|
||||
render.run_sensors(&mut rsc, &mut state, cursor, (100.0, 100.0).into());
|
||||
|
||||
assert!(
|
||||
!b_hovered.get(),
|
||||
"while a's drag holds capture, b must see no hover at all"
|
||||
);
|
||||
}
|
||||
@@ -552,6 +552,20 @@ impl List {
|
||||
self.extents.get(&key).map(|e| (e.top, e.bottom))
|
||||
}
|
||||
|
||||
/// The row whose on-screen box (as of the last layout) contains
|
||||
/// `viewport_pos`, or `None` if it falls outside every row currently
|
||||
/// drawn (a gap, a header, or off the loaded content entirely). O
|
||||
/// (visible rows), same as `reanchor_at_tap`. What a caller resolves a
|
||||
/// pointer-captured gesture's row-under-the-finger against once the
|
||||
/// gesture is no longer being delivered through any one row's own hit
|
||||
/// region -- see `iris::sense`'s pointer-capture doc.
|
||||
pub fn key_at(&self, viewport_pos: f32) -> Option<RowKey> {
|
||||
self.extents
|
||||
.iter()
|
||||
.find(|(_, ext)| viewport_pos >= ext.top && viewport_pos <= ext.bottom)
|
||||
.map(|(&key, _)| key)
|
||||
}
|
||||
|
||||
fn slot_exists(&self, slot: isize) -> bool {
|
||||
match slot {
|
||||
BEFORE_SLOT => self.more_before.is_some(),
|
||||
@@ -1332,6 +1346,63 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// RUST.md's P0 phone report (Iris's screenshot, 2026-09-06): a
|
||||
/// replaced row's primitives drawn a second time, overlapping the
|
||||
/// replacement. Reproduces the exact path `TranscriptScreen::apply`'s
|
||||
/// `ReplaceLast` case drives up to 400 times during a streamed reply
|
||||
/// (`bench_client.rs`'s stream phase): the last slot's widget is
|
||||
/// swapped for a brand-new one, same key, and (since a fresh widget
|
||||
/// has no cached height) placed via `place`'s `draw_twice` path every
|
||||
/// time -- the provisional-then-real two-draw sequence LAYOUT.md
|
||||
/// documents as the one place in this crate that deliberately draws a
|
||||
/// widget twice. If `draw_inner`'s old-children diffing or
|
||||
/// `UiRenderState::remove`'s primitive freeing ever failed to retire
|
||||
/// the evicted widget (or the provisional draw's own primitives), it
|
||||
/// would show up here as `active_widgets` growing without bound.
|
||||
/// **Passes as written** -- this pins the widget-arena layer as
|
||||
/// correct in isolation; see the P0 box for where the duplicate was
|
||||
/// actually chased to instead (`Span`'s two-phase draw and the
|
||||
/// `redraw_all`-vs-`redraw_updates` split, still open).
|
||||
#[test]
|
||||
fn replacing_the_last_row_many_times_does_not_leak_primitives() {
|
||||
let mut rsc = TestRsc {
|
||||
ui: UiData::default(),
|
||||
};
|
||||
let mut list = List::new(Axis::Y);
|
||||
for key in 0..5u64 {
|
||||
let (_bg_id, row) = background_styled_row(&mut rsc, 20.0);
|
||||
list.push_back(ListRow::new(key, row));
|
||||
}
|
||||
let (list_weak, root) = add_list(&mut rsc, list);
|
||||
|
||||
let mut render = UiRenderState::new();
|
||||
render.resize((100.0, 100.0));
|
||||
render.update(&root, &mut rsc);
|
||||
|
||||
let before = render.active_widgets();
|
||||
for i in 0..400u32 {
|
||||
// A varying height keeps every replace on the `draw_twice`
|
||||
// (cache-miss) path rather than settling into the O(1)
|
||||
// same-size `mov` fast path once the height happens to repeat.
|
||||
let (_bg_id, new_row) = background_styled_row(&mut rsc, 20.0 + (i % 3) as f32);
|
||||
rsc.ui
|
||||
.widgets
|
||||
.get_mut(&list_weak)
|
||||
.unwrap()
|
||||
.replace_back(ListRow::new(4, new_row));
|
||||
render.update(&root, &mut rsc);
|
||||
}
|
||||
let after = render.active_widgets();
|
||||
|
||||
assert_eq!(
|
||||
before, after,
|
||||
"400 replaces of the last row must leave exactly the same \
|
||||
number of active widgets as before a leaked id (and the \
|
||||
primitives that live as long as its ActiveData does) would \
|
||||
show up here as growth"
|
||||
);
|
||||
}
|
||||
|
||||
/// Enough rows, tall enough, that a fling toward the start has real
|
||||
/// room to travel before `at_start` clamps it -- shared by the fling
|
||||
/// tests below.
|
||||
|
||||
@@ -51,7 +51,7 @@ pub mod selection;
|
||||
use client_core::transcript_fold::TranscriptRow as FoldedRow;
|
||||
use iris::prelude::*;
|
||||
use selection::Selection;
|
||||
use std::{cell::RefCell, rc::Rc};
|
||||
use std::{cell::RefCell, rc::Rc, time::Instant};
|
||||
|
||||
pub struct TranscriptScreen {
|
||||
/// The transcript's own `List` -- exposed so a caller can read
|
||||
@@ -220,6 +220,43 @@ where
|
||||
})
|
||||
.add(rsc);
|
||||
|
||||
// The continuation of a row-started drag once it has committed and
|
||||
// taken pointer capture on `list`'s own id (`row.rs`'s registration is
|
||||
// only ever the gesture's first frame) -- registered once here, not
|
||||
// once per row, since `DragGesture`'s single shared instance must see
|
||||
// each frame of one gesture exactly once. `ctx.data.pos`/`size` are
|
||||
// already relative to `list`'s own on-screen box (this is what it was
|
||||
// registered against), which is exactly the viewport-pixel space
|
||||
// `List::key_at`/`extent` work in, so the row-under-the-pointer is
|
||||
// resolved from those instead of a per-row hit test.
|
||||
{
|
||||
let selection = selection.clone();
|
||||
list.on(
|
||||
CursorSense::Pressing(CursorButton::Left) | CursorSense::Drop,
|
||||
move |ctx, rsc| {
|
||||
let pos = ctx.data.pos;
|
||||
let row = list(rsc).key_at(pos.y).and_then(|key| {
|
||||
let (top, bottom) = list(rsc).extent(key)?;
|
||||
Some((
|
||||
key,
|
||||
Vec2::new(pos.x, pos.y - top),
|
||||
Vec2::new(ctx.data.size.x, bottom - top),
|
||||
))
|
||||
});
|
||||
selection.borrow_mut().drag(
|
||||
rsc,
|
||||
list,
|
||||
row,
|
||||
ctx.data.cursor.pos,
|
||||
ctx.data.sense,
|
||||
Instant::now(),
|
||||
ctx.data.render,
|
||||
);
|
||||
},
|
||||
)
|
||||
.add(rsc);
|
||||
}
|
||||
|
||||
let (composer, composer_bar) = composer::build_composer(rsc);
|
||||
|
||||
let tree = (list.width(rest(1)).height(rest(1)), composer_bar)
|
||||
|
||||
@@ -137,20 +137,26 @@ where
|
||||
|
||||
field
|
||||
// `| CursorSense::unclick()` on top of the usual click-or-drag set
|
||||
// -- the arbiter inside `Selection::drag` needs the release too,
|
||||
// to go back to idle for the next press (`DragArbiter::release`).
|
||||
// -- this row's own registration only ever needs to see a
|
||||
// gesture's *first* frame (`PressStart`, or a `Pressing` that
|
||||
// missed it -- `DragGesture::handle`'s idle-recovery branch); once
|
||||
// it commits, `DragGesture` takes pointer capture on `list`'s own
|
||||
// id and every further frame, including the terminal `Drop`,
|
||||
// reaches `lib.rs`'s list-level registration instead -- see
|
||||
// `iris::sense`'s pointer-capture doc for why that has to be a
|
||||
// stable id rather than this row's, which `List` can retire mid-
|
||||
// drag as content scrolls.
|
||||
.on(
|
||||
CursorSense::click_or_drag() | CursorSense::unclick(),
|
||||
move |ctx, rsc| {
|
||||
selection.borrow_mut().drag(
|
||||
rsc,
|
||||
list,
|
||||
key,
|
||||
ctx.data.pos,
|
||||
ctx.data.size,
|
||||
Some((key, ctx.data.pos, ctx.data.size)),
|
||||
ctx.data.cursor.pos,
|
||||
ctx.data.sense,
|
||||
Instant::now(),
|
||||
ctx.data.render,
|
||||
);
|
||||
},
|
||||
)
|
||||
|
||||
@@ -36,17 +36,15 @@ use std::{collections::BTreeMap, time::Instant};
|
||||
pub struct Selection {
|
||||
rows: BTreeMap<RowKey, WeakWidget<TextEdit>>,
|
||||
anchor: Option<(RowKey, Vec2)>,
|
||||
/// One arbiter shared by every row's drag handler -- RUST.md's I5
|
||||
/// One gesture shared by every row's drag handler -- RUST.md's I5
|
||||
/// gesture conflict (a row's own `click_or_drag()` and a list-level
|
||||
/// pan wanting the same touch gesture). See `drag` below, and
|
||||
/// `iris::sense::DragArbiter`'s own doc for the decision itself.
|
||||
arbiter: DragArbiter,
|
||||
/// Tracks the last ~100ms of this gesture's pan deltas (in the same
|
||||
/// signed units `list.scroll` takes), so a release that turns out to
|
||||
/// have been panning can hand `List::fling` a realistic initial
|
||||
/// velocity instead of one frame's noisy last delta --
|
||||
/// IRIS_TODO.md's "swiping has no momentum."
|
||||
velocity: VelocityTracker,
|
||||
/// `iris::sense::DragGesture`'s own doc for the arbitration, velocity
|
||||
/// tracking and pointer-capture mechanics this no longer owns itself
|
||||
/// -- Iris's 2026-09-06 ask (`IRIS.md`) that a drag's *mechanics* live
|
||||
/// in iris's default input layer, with only the pan-vs-select
|
||||
/// *decision* staying here.
|
||||
gesture: DragGesture,
|
||||
}
|
||||
|
||||
impl Default for Selection {
|
||||
@@ -60,8 +58,7 @@ impl Selection {
|
||||
Self {
|
||||
rows: BTreeMap::new(),
|
||||
anchor: None,
|
||||
arbiter: DragArbiter::new(),
|
||||
velocity: VelocityTracker::new(),
|
||||
gesture: DragGesture::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -165,84 +162,65 @@ impl Selection {
|
||||
/// row, is what makes that consistent as a drag crosses row
|
||||
/// boundaries).
|
||||
///
|
||||
/// `pos_row`/`size` are row-local, as `begin`/`extend` want;
|
||||
/// `row`, if given, is `(key, pos_row, size)` for whichever row the
|
||||
/// pointer is currently over -- row-local, as `begin`/`extend` want.
|
||||
/// `None` once the gesture is pointer-captured (`iris::sense`'s
|
||||
/// pointer-capture doc) and the current position falls outside every
|
||||
/// row `List` has loaded (a gap, or off the end of the content); a
|
||||
/// `Pan` outcome never needs it, so this only actually matters mid-
|
||||
/// selection, where it is rare and the frame is simply dropped.
|
||||
/// `pos_window` is in window space, since a pan's delta has to stay
|
||||
/// meaningful even when this frame's event landed on a different row
|
||||
/// than the last one.
|
||||
/// than the last one. `render` is `CursorData`'s own field -- what
|
||||
/// `DragGesture` needs to take pointer capture.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn drag(
|
||||
&mut self,
|
||||
ui: &mut impl UiRsc,
|
||||
list: WeakWidget<List>,
|
||||
key: RowKey,
|
||||
pos_row: Vec2,
|
||||
size: Vec2,
|
||||
row: Option<(RowKey, Vec2, Vec2)>,
|
||||
pos_window: Vec2,
|
||||
sense: CursorSense,
|
||||
now: Instant,
|
||||
render: &UiRenderState,
|
||||
) {
|
||||
let outcome = match sense {
|
||||
CursorSense::PressStart(_) => {
|
||||
let already_selected = self.has_selection(ui);
|
||||
self.arbiter.press_start(pos_window, now, already_selected);
|
||||
self.velocity.reset();
|
||||
// A fresh touch-down cancels any fling still coasting from
|
||||
// the previous gesture -- `List::fling`'s own doc, and
|
||||
// Android's `Scroller::abortAnimation` for the same reason.
|
||||
list(ui).cancel_fling();
|
||||
self.arbiter.update(pos_window, now)
|
||||
}
|
||||
CursorSense::PressEnd(_) => {
|
||||
// A fling only ever follows a pan -- never a selection
|
||||
// that happened to end with the finger still moving, and
|
||||
// never a tap/long-press that never left `Undecided`.
|
||||
if self.arbiter.is_panning() {
|
||||
let v = self.velocity.velocity();
|
||||
list(ui).fling(v);
|
||||
}
|
||||
self.arbiter.release();
|
||||
return;
|
||||
}
|
||||
// A `Pressing` frame with the arbiter still `Idle` means this
|
||||
// gesture's `ACTION_DOWN` landed somewhere no row's sensor
|
||||
// covers (a row's own padding/gap, or a header with no
|
||||
// selection handler) and this row is only now getting the
|
||||
// touch as it moves across it -- the touch is definitely still
|
||||
// down (that's what `Pressing` means), so without this the
|
||||
// arbiter would sit in `Idle` answering `Undecided` for the
|
||||
// rest of the gesture (`DragArbiter::update`'s own doc).
|
||||
// Recovered by starting the press here instead of where it
|
||||
// was missed -- RUST.md's I5 intermittent-touch-scroll-dropout
|
||||
// finding, 2026-09-05.
|
||||
_ if self.arbiter.is_idle() => {
|
||||
let already_selected = self.has_selection(ui);
|
||||
self.arbiter.press_start(pos_window, now, already_selected);
|
||||
self.velocity.reset();
|
||||
list(ui).cancel_fling();
|
||||
self.arbiter.update(pos_window, now)
|
||||
}
|
||||
_ => self.arbiter.update(pos_window, now),
|
||||
};
|
||||
if matches!(sense, CursorSense::PressStart(_)) {
|
||||
// A fresh touch-down cancels any fling still coasting from
|
||||
// the previous gesture -- `List::fling`'s own doc, and
|
||||
// Android's `Scroller::abortAnimation` for the same reason.
|
||||
list(ui).cancel_fling();
|
||||
}
|
||||
let already_selected = self.has_selection(ui);
|
||||
let outcome =
|
||||
self.gesture
|
||||
.handle(render, list.id(), sense, pos_window, now, already_selected);
|
||||
match outcome {
|
||||
DragOutcome::Undecided => {}
|
||||
DragOutcome::Pan(dy) => {
|
||||
let amt = -dy;
|
||||
self.velocity.add_sample(amt, now);
|
||||
list(ui).scroll(amt);
|
||||
GestureOutcome::Undecided => {}
|
||||
GestureOutcome::Pan(dy) => list(ui).scroll(-dy),
|
||||
GestureOutcome::SelectStart => {
|
||||
if let Some((key, pos_row, size)) = row {
|
||||
// Grep-able on "iris selection" the way the frame
|
||||
// report is on "iris frame report" -- selection has no
|
||||
// accessibility label of its own yet, so this is the
|
||||
// smallest way to confirm a real on-device long-
|
||||
// press-then-drag actually reached here (RUST.md's I5
|
||||
// box, "Measurements taken" (c)).
|
||||
log::info!("iris selection: begin at row {key:?}");
|
||||
self.begin(ui, key, pos_row, size);
|
||||
}
|
||||
}
|
||||
DragOutcome::SelectStart => {
|
||||
// Grep-able on "iris selection" the way the frame report is
|
||||
// on "iris frame report" -- selection has no accessibility
|
||||
// label of its own yet, so this is the smallest way to
|
||||
// confirm a real on-device long-press-then-drag actually
|
||||
// reached here (RUST.md's I5 box, "Measurements taken" (c)).
|
||||
log::info!("iris selection: begin at row {key:?}");
|
||||
self.begin(ui, key, pos_row, size);
|
||||
}
|
||||
DragOutcome::SelectExtend => {
|
||||
log::info!("iris selection: extend to row {key:?}");
|
||||
self.extend(ui, key, pos_row, size);
|
||||
GestureOutcome::SelectExtend => {
|
||||
if let Some((key, pos_row, size)) = row {
|
||||
log::info!("iris selection: extend to row {key:?}");
|
||||
self.extend(ui, key, pos_row, size);
|
||||
}
|
||||
}
|
||||
// A fling only ever follows a pan -- never a selection that
|
||||
// happened to end with the finger still moving, and never a
|
||||
// tap/long-press that never left `Undecided` -- exactly what
|
||||
// `DragGesture`'s `Some(v)` already encodes.
|
||||
GestureOutcome::Released(Some(v)) => list(ui).fling(-v),
|
||||
GestureOutcome::Released(None) => {}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -344,8 +322,9 @@ mod tests {
|
||||
|
||||
let mut sel = Selection::new();
|
||||
sel.register(1, field);
|
||||
assert!(sel.arbiter.is_idle());
|
||||
assert!(sel.gesture.is_idle());
|
||||
|
||||
let render = UiRenderState::new();
|
||||
let now = Instant::now();
|
||||
let size = Vec2::new(100.0, 20.0);
|
||||
// No `PressStart` is ever sent -- only the `Pressing` frames a
|
||||
@@ -353,15 +332,14 @@ mod tests {
|
||||
sel.drag(
|
||||
&mut rsc,
|
||||
list,
|
||||
1,
|
||||
Vec2::ZERO,
|
||||
size,
|
||||
Some((1, Vec2::ZERO, size)),
|
||||
Vec2::new(540.0, 700.0),
|
||||
CursorSense::Pressing(CursorButton::Left),
|
||||
now,
|
||||
&render,
|
||||
);
|
||||
assert!(
|
||||
!sel.arbiter.is_idle(),
|
||||
!sel.gesture.is_idle(),
|
||||
"a Pressing frame with the arbiter still Idle must recover \
|
||||
the press rather than leaving it stuck"
|
||||
);
|
||||
|
||||
Reference in new issue
Block a user