Compare commits
16
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
2fed8b34b3 | ||
|
|
1f379e8384 | ||
|
|
bf3479f5c4 | ||
|
|
312455956d | ||
|
|
73251d6b8b | ||
|
|
2e00e71552 | ||
|
|
f802de94b5 | ||
|
|
9717d1c4b0 | ||
|
|
9458f443ad | ||
|
|
e12c708246 | ||
|
|
543f6d92f0 | ||
|
|
27ca5b2349 | ||
|
|
20b12255e1 | ||
|
|
71a3fae655 | ||
|
|
c3984da623 | ||
|
|
2e3f4ada38 |
No files matched your search
@@ -17,7 +17,12 @@ edition = "2024"
|
|||||||
[dependencies]
|
[dependencies]
|
||||||
event-model = { path = "../event-model" }
|
event-model = { path = "../event-model" }
|
||||||
serde = { version = "1", features = ["derive"] }
|
serde = { version = "1", features = ["derive"] }
|
||||||
serde_json = { version = "1", features = ["float_roundtrip"] }
|
# "raw_value" is `fetch_transcript_lines`'s reason -- it needs the exact
|
||||||
|
# bytes the server sent, not this crate's own re-serialization of a parsed
|
||||||
|
# `Value`, so a cached line and a live SSE frame for the same event agree
|
||||||
|
# byte-for-byte (see that method's doc). "float_roundtrip" is why they
|
||||||
|
# agree on a `ts` at all -- see server/Cargo.toml's identical comment.
|
||||||
|
serde_json = { version = "1", features = ["float_roundtrip", "raw_value"] }
|
||||||
# The blocking HTTP client for the REST calls and the long-lived SSE GETs.
|
# The blocking HTTP client for the REST calls and the long-lived SSE GETs.
|
||||||
# `server/` already depends on ureq for its own outbound HTTPS (the usage
|
# `server/` already depends on ureq for its own outbound HTTPS (the usage
|
||||||
# poll in usage.rs) and it is rustls-backed like the rest of this project's
|
# poll in usage.rs) and it is rustls-backed like the rest of this project's
|
||||||
|
|||||||
+67
-1
@@ -10,6 +10,7 @@
|
|||||||
|
|
||||||
use std::io::Read;
|
use std::io::Read;
|
||||||
|
|
||||||
|
use event_model::SeqEvent;
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
|
|
||||||
@@ -116,6 +117,14 @@ impl<T: Transport> ApiClient<T> {
|
|||||||
Self { transport }
|
Self { transport }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The transport underneath, for a caller that needs the raw SSE
|
||||||
|
/// stream (`event_stream::follow_session_events`) rather than one of
|
||||||
|
/// this client's typed REST calls -- `transcript_source::TranscriptSource`
|
||||||
|
/// is the one that does.
|
||||||
|
pub fn transport(&self) -> &T {
|
||||||
|
&self.transport
|
||||||
|
}
|
||||||
|
|
||||||
fn json_request<R: for<'de> Deserialize<'de>>(
|
fn json_request<R: for<'de> Deserialize<'de>>(
|
||||||
&self,
|
&self,
|
||||||
method: &str,
|
method: &str,
|
||||||
@@ -266,6 +275,61 @@ impl<T: Transport> ApiClient<T> {
|
|||||||
limit: u32,
|
limit: u32,
|
||||||
coalesce: bool,
|
coalesce: bool,
|
||||||
) -> Result<Vec<Value>, ApiError> {
|
) -> Result<Vec<Value>, ApiError> {
|
||||||
|
self.json_request(
|
||||||
|
"GET",
|
||||||
|
&transcript_path(session_id, before, limit, coalesce, None),
|
||||||
|
None,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A page of transcript history, each line handed back paired with the
|
||||||
|
/// exact text it came from, and bounded below by `after` -- the shape
|
||||||
|
/// `crate::transcript_source::TranscriptSource` needs to store what it
|
||||||
|
/// fetched in the transcript cache without a second round trip to fetch
|
||||||
|
/// the raw text separately. Ported from `Api.kt`'s `fetchTranscript`.
|
||||||
|
///
|
||||||
|
/// Uses [`serde_json::value::RawValue`] rather than re-serializing a
|
||||||
|
/// parsed [`Value`], so the stored line is the exact bytes the server
|
||||||
|
/// sent (key order and float literal included) rather than this
|
||||||
|
/// crate's own idea of how to write them back out -- the cache and a
|
||||||
|
/// live SSE frame must agree byte-for-byte on the same event, which is
|
||||||
|
/// exactly what caught the `serde_json` float-rounding bug this
|
||||||
|
/// project's `AGENTS.md` records.
|
||||||
|
pub fn fetch_transcript_lines(
|
||||||
|
&self,
|
||||||
|
session_id: &str,
|
||||||
|
before: Option<u64>,
|
||||||
|
limit: u32,
|
||||||
|
coalesce: bool,
|
||||||
|
after: Option<u64>,
|
||||||
|
) -> Result<Vec<(String, SeqEvent)>, ApiError> {
|
||||||
|
let path = transcript_path(session_id, before, limit, coalesce, after);
|
||||||
|
let raw: Vec<Box<serde_json::value::RawValue>> = self.json_request("GET", &path, None)?;
|
||||||
|
raw.into_iter()
|
||||||
|
.map(|value| {
|
||||||
|
let line = value.get().to_string();
|
||||||
|
let event: SeqEvent = serde_json::from_str(&line).map_err(|e| ApiError {
|
||||||
|
message: format!(
|
||||||
|
"the server sent a transcript line this build couldn't parse: {e}"
|
||||||
|
),
|
||||||
|
status: None,
|
||||||
|
})?;
|
||||||
|
Ok((line, event))
|
||||||
|
})
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The query string shared by [`ApiClient::fetch_transcript_page`] and
|
||||||
|
/// [`ApiClient::fetch_transcript_lines`], so the two agree on how each
|
||||||
|
/// parameter is written rather than keeping two copies to drift.
|
||||||
|
fn transcript_path(
|
||||||
|
session_id: &str,
|
||||||
|
before: Option<u64>,
|
||||||
|
limit: u32,
|
||||||
|
coalesce: bool,
|
||||||
|
after: Option<u64>,
|
||||||
|
) -> String {
|
||||||
let mut path = format!("/sessions/{session_id}/transcript?limit={limit}");
|
let mut path = format!("/sessions/{session_id}/transcript?limit={limit}");
|
||||||
if let Some(before) = before {
|
if let Some(before) = before {
|
||||||
path.push_str(&format!("&before={before}"));
|
path.push_str(&format!("&before={before}"));
|
||||||
@@ -273,8 +337,10 @@ impl<T: Transport> ApiClient<T> {
|
|||||||
if coalesce {
|
if coalesce {
|
||||||
path.push_str("&coalesce=true");
|
path.push_str("&coalesce=true");
|
||||||
}
|
}
|
||||||
self.json_request("GET", &path, None)
|
if let Some(after) = after {
|
||||||
|
path.push_str(&format!("&after={after}"));
|
||||||
}
|
}
|
||||||
|
path
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The blocking [`Transport`] backed by `ureq`, the same crate `server/`
|
/// The blocking [`Transport`] backed by `ureq`, the same crate `server/`
|
||||||
|
|||||||
@@ -11,5 +11,6 @@ pub mod notifications;
|
|||||||
pub mod sse;
|
pub mod sse;
|
||||||
pub mod transcript_cache;
|
pub mod transcript_cache;
|
||||||
pub mod transcript_fold;
|
pub mod transcript_fold;
|
||||||
|
pub mod transcript_source;
|
||||||
|
|
||||||
pub use event_model::*;
|
pub use event_model::*;
|
||||||
@@ -361,6 +361,10 @@ impl SessionCache {
|
|||||||
{
|
{
|
||||||
return Ok(false);
|
return Ok(false);
|
||||||
}
|
}
|
||||||
|
debug_assert!(
|
||||||
|
lines.iter().all(|l| !l.contains('\n')),
|
||||||
|
"a stored page's lines must each be one line"
|
||||||
|
);
|
||||||
fs::create_dir_all(&this.dir)?;
|
fs::create_dir_all(&this.dir)?;
|
||||||
let kind = if rows { "rows" } else { "raw" };
|
let kind = if rows { "rows" } else { "raw" };
|
||||||
let mut content = lines.join("\n");
|
let mut content = lines.join("\n");
|
||||||
@@ -389,8 +393,16 @@ impl SessionCache {
|
|||||||
return Ok(());
|
return Ok(());
|
||||||
};
|
};
|
||||||
// Written as it arrived. A newline inside it would split one
|
// Written as it arrived. A newline inside it would split one
|
||||||
// event into two unreadable halves, but neither source can
|
// event into two unreadable halves. No source here can produce
|
||||||
// produce one.
|
// one -- an SSE `data:` field cannot hold a raw newline, and a
|
||||||
|
// fetched line is one element of a compact JSON array -- but
|
||||||
|
// that is a fact about the *server's* serializer rather than
|
||||||
|
// anything this file controls, so it is checked rather than
|
||||||
|
// trusted.
|
||||||
|
debug_assert!(
|
||||||
|
!line.contains('\n'),
|
||||||
|
"a cached transcript line must be one line: {line}"
|
||||||
|
);
|
||||||
use std::io::Write;
|
use std::io::Write;
|
||||||
writer.write_all(line.as_bytes())?;
|
writer.write_all(line.as_bytes())?;
|
||||||
writer.write_all(b"\n")?;
|
writer.write_all(b"\n")?;
|
||||||
|
|||||||
@@ -294,6 +294,199 @@ fn split_run(tail: &[TranscriptItem], behind: Option<&str>) -> Vec<TranscriptIte
|
|||||||
out
|
out
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Puts a page of older items in front of the ones already loaded, healing
|
||||||
|
/// whatever the page boundary cut in two. Ported from `TranscriptItems.kt`'s
|
||||||
|
/// `joinPages`.
|
||||||
|
///
|
||||||
|
/// Two things straddle a boundary: a tool call separated from its result,
|
||||||
|
/// and a message separated from the rest of itself. Both were one thing
|
||||||
|
/// before the transcript was cut into pages.
|
||||||
|
///
|
||||||
|
/// A boundary lands wherever it lands, and roughly half the time that is
|
||||||
|
/// between a call and its result. The newer page then holds a `ToolEnd`
|
||||||
|
/// whose start it never saw, which `fold_event` draws as a row of its own
|
||||||
|
/// -- correctly, because a call that renders as nothing is indistinguishable
|
||||||
|
/// from one that never happened. When the older page arrives it brings the
|
||||||
|
/// real `ToolStart`, and concatenating the two lists left *both*: the same
|
||||||
|
/// call twice.
|
||||||
|
///
|
||||||
|
/// Merged by the call's own id rather than by position, because position is
|
||||||
|
/// exactly what a page boundary destroys. The older row wins on what a
|
||||||
|
/// start knows and the newer on what an end knows, which is the only way
|
||||||
|
/// round that loses nothing.
|
||||||
|
///
|
||||||
|
/// The third thing is the *run*, and it is the one the Kotlin original used
|
||||||
|
/// to miss (AGENTS.md's "things that have bitten"): every page ends up
|
||||||
|
/// here, but `adopt_run` must run on *every* join, not only the one where a
|
||||||
|
/// split call was found -- a boundary landing cleanly between two finished
|
||||||
|
/// calls, which is most of them, would otherwise leave the older page's
|
||||||
|
/// calls under the run name they were folded with. On screen: one run of
|
||||||
|
/// tool calls drawn as two groups, with the seam wherever the reader
|
||||||
|
/// happened to have paged.
|
||||||
|
pub fn join_pages(earlier: &[TranscriptItem], later: &[TranscriptItem]) -> Vec<TranscriptItem> {
|
||||||
|
let (older, newer) = heal_split_message(earlier, later);
|
||||||
|
let started_earlier: std::collections::HashSet<&str> = older
|
||||||
|
.iter()
|
||||||
|
.filter_map(TranscriptItem::as_tool_run)
|
||||||
|
.collect();
|
||||||
|
// Owned rather than borrowed from `newer`: `kept` below needs to consume `newer` by
|
||||||
|
// value, and a map borrowing it would keep that alive.
|
||||||
|
let ended_later: std::collections::HashMap<String, TranscriptItem> = newer
|
||||||
|
.iter()
|
||||||
|
.filter_map(|item| item.as_tool_run().map(|id| (id.to_string(), item.clone())))
|
||||||
|
.filter(|(id, _)| started_earlier.contains(id.as_str()))
|
||||||
|
.collect();
|
||||||
|
let healed: Vec<TranscriptItem> = older
|
||||||
|
.into_iter()
|
||||||
|
.map(|row| match row {
|
||||||
|
TranscriptItem::ToolRun {
|
||||||
|
seq,
|
||||||
|
id,
|
||||||
|
run_id,
|
||||||
|
tool,
|
||||||
|
input,
|
||||||
|
asks: row_asks,
|
||||||
|
images: row_images,
|
||||||
|
..
|
||||||
|
} if ended_later.contains_key(id.as_str()) => {
|
||||||
|
let &TranscriptItem::ToolRun {
|
||||||
|
ref output,
|
||||||
|
done,
|
||||||
|
asks: ref half_asks,
|
||||||
|
images: ref half_images,
|
||||||
|
..
|
||||||
|
} = &ended_later[id.as_str()]
|
||||||
|
else {
|
||||||
|
unreachable!("filtered to ToolRun above");
|
||||||
|
};
|
||||||
|
TranscriptItem::ToolRun {
|
||||||
|
seq,
|
||||||
|
id,
|
||||||
|
run_id,
|
||||||
|
tool,
|
||||||
|
input,
|
||||||
|
output: output.clone(),
|
||||||
|
done,
|
||||||
|
// Kept from both halves: a question or an image can be
|
||||||
|
// attached to either, depending on which side of the
|
||||||
|
// boundary its event fell.
|
||||||
|
asks: row_asks.into_iter().chain(half_asks.clone()).collect(),
|
||||||
|
images: row_images.into_iter().chain(half_images.clone()).collect(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
other => other,
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
let kept: Vec<TranscriptItem> = newer
|
||||||
|
.into_iter()
|
||||||
|
.filter(|item| match item.as_tool_run() {
|
||||||
|
Some(id) => !ended_later.contains_key(id),
|
||||||
|
None => true,
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
let mut out = adopt_run(&healed, &kept);
|
||||||
|
out.extend(kept);
|
||||||
|
// What this function exists to prevent, checked rather than assumed: the same
|
||||||
|
// call drawn twice, once from the page that saw its start and once from the page
|
||||||
|
// that saw its end. Not a seq-ordering check -- a peer note is stamped with the
|
||||||
|
// seq its turn began at, which can be older than the page it arrived in, so the
|
||||||
|
// two pages' seqs legitimately interleave at the boundary.
|
||||||
|
debug_assert!(
|
||||||
|
{
|
||||||
|
let mut ids: Vec<&str> = out.iter().filter_map(TranscriptItem::as_tool_run).collect();
|
||||||
|
let before = ids.len();
|
||||||
|
ids.sort_unstable();
|
||||||
|
ids.dedup();
|
||||||
|
ids.len() == before
|
||||||
|
},
|
||||||
|
"join_pages left the same tool call in both halves"
|
||||||
|
);
|
||||||
|
out
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Rejoins a message the page boundary cut, and hands back the two pages to
|
||||||
|
/// concatenate. Ported from `TranscriptItems.kt`'s `healSplitMessage`.
|
||||||
|
///
|
||||||
|
/// `fold_event` never leaves two assistant messages next to each other
|
||||||
|
/// inside one page, so two meeting at a join are always the two halves of
|
||||||
|
/// one reply, and leaving them apart drew a single answer as two with a
|
||||||
|
/// paragraph break through the middle of a sentence.
|
||||||
|
///
|
||||||
|
/// The newer half keeps its identity, for the reason `adopt_run`'s doc
|
||||||
|
/// gives. It grows by what the older half brings, which is safe here and
|
||||||
|
/// nowhere else -- the join is at the oldest end of what is loaded, so the
|
||||||
|
/// growth extends off the top of the screen.
|
||||||
|
fn heal_split_message(
|
||||||
|
earlier: &[TranscriptItem],
|
||||||
|
later: &[TranscriptItem],
|
||||||
|
) -> (Vec<TranscriptItem>, Vec<TranscriptItem>) {
|
||||||
|
let (
|
||||||
|
Some(TranscriptItem::AssistantMsg {
|
||||||
|
text: head_text, ..
|
||||||
|
}),
|
||||||
|
Some(TranscriptItem::AssistantMsg {
|
||||||
|
seq: tail_seq,
|
||||||
|
text: tail_text,
|
||||||
|
settled: tail_settled,
|
||||||
|
}),
|
||||||
|
) = (earlier.last(), later.first())
|
||||||
|
else {
|
||||||
|
return (earlier.to_vec(), later.to_vec());
|
||||||
|
};
|
||||||
|
let merged = TranscriptItem::AssistantMsg {
|
||||||
|
seq: *tail_seq,
|
||||||
|
text: format!("{head_text}{tail_text}"),
|
||||||
|
settled: *tail_settled,
|
||||||
|
};
|
||||||
|
let mut newer = vec![merged];
|
||||||
|
newer.extend(later[1..].iter().cloned());
|
||||||
|
(earlier[..earlier.len() - 1].to_vec(), newer)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Hands the older calls at the join the name of the run they are joining.
|
||||||
|
/// Ported from `TranscriptItems.kt`'s `adoptRun`.
|
||||||
|
///
|
||||||
|
/// The two pages were folded separately, so a run split by the boundary
|
||||||
|
/// came back as two runs with two names. Naming the joined run after the
|
||||||
|
/// *older* half would be the obvious way round and is wrong: the newer half
|
||||||
|
/// is the part already on screen, and renaming it is renaming the row the
|
||||||
|
/// reader is looking at, which is how a list loses its anchor.
|
||||||
|
fn adopt_run(earlier: &[TranscriptItem], later: &[TranscriptItem]) -> Vec<TranscriptItem> {
|
||||||
|
let Some(TranscriptItem::ToolRun { run_id, tool, .. }) = later.first() else {
|
||||||
|
return earlier.to_vec();
|
||||||
|
};
|
||||||
|
// A question is in a run of its own on both sides of the join, the same as it would be
|
||||||
|
// had the two pages been folded as one. Without this the heal would merge a group
|
||||||
|
// straight through the row the reader was asked something on.
|
||||||
|
if tool == ASK_USER_QUESTION {
|
||||||
|
return earlier.to_vec();
|
||||||
|
}
|
||||||
|
let joining = run_id.clone();
|
||||||
|
let tail_len = earlier
|
||||||
|
.iter()
|
||||||
|
.rev()
|
||||||
|
.take_while(|item| matches!(item, TranscriptItem::ToolRun { tool, .. } if tool != ASK_USER_QUESTION))
|
||||||
|
.count();
|
||||||
|
if tail_len == 0 {
|
||||||
|
return earlier.to_vec();
|
||||||
|
}
|
||||||
|
let split = earlier.len() - tail_len;
|
||||||
|
let mut out = earlier[..split].to_vec();
|
||||||
|
out.extend(earlier[split..].iter().cloned().map(|mut item| {
|
||||||
|
// `take_while` above already restricted this slice to non-question tool calls;
|
||||||
|
// this just guards the invariant rather than trusting it silently.
|
||||||
|
debug_assert!(
|
||||||
|
matches!(&item, TranscriptItem::ToolRun { tool, .. } if tool != ASK_USER_QUESTION),
|
||||||
|
"adopt_run must never rename a question's own run"
|
||||||
|
);
|
||||||
|
if let TranscriptItem::ToolRun { run_id, .. } = &mut item {
|
||||||
|
*run_id = joining.clone();
|
||||||
|
}
|
||||||
|
item
|
||||||
|
}));
|
||||||
|
out
|
||||||
|
}
|
||||||
|
|
||||||
/// Folds one transcript event onto `items`, the way `foldEvent` does in
|
/// Folds one transcript event onto `items`, the way `foldEvent` does in
|
||||||
/// `TranscriptItems.kt`. Every wire event has a case; see the module doc
|
/// `TranscriptItems.kt`. Every wire event has a case; see the module doc
|
||||||
/// for the one difference from the Kotlin original (no `Unknown` fallback
|
/// for the one difference from the Kotlin original (no `Unknown` fallback
|
||||||
@@ -955,4 +1148,134 @@ mod tests {
|
|||||||
let err = fold_page(&values).unwrap_err();
|
let err = fold_page(&values).unwrap_err();
|
||||||
assert!(err.contains("couldn't parse"));
|
assert!(err.contains("couldn't parse"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn tool_start(seq: u64, id: &str, tool: &str) -> SeqEvent {
|
||||||
|
event(
|
||||||
|
seq,
|
||||||
|
Event::ToolStart {
|
||||||
|
id: id.to_string(),
|
||||||
|
tool: tool.to_string(),
|
||||||
|
input: serde_json::json!({}),
|
||||||
|
},
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn tool_end(seq: u64, id: &str, output: &str) -> SeqEvent {
|
||||||
|
event(
|
||||||
|
seq,
|
||||||
|
Event::ToolEnd {
|
||||||
|
id: id.to_string(),
|
||||||
|
output: output.to_string(),
|
||||||
|
},
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// AGENTS.md's "things that have bitten": `joinPages` used to run
|
||||||
|
/// `adoptRun` only on the path where a *split* call was found, so a
|
||||||
|
/// boundary landing cleanly between two already-finished calls -- most
|
||||||
|
/// of them -- left the older page's calls under the run name they were
|
||||||
|
/// folded with, drawing one run of tool calls as two groups. Two
|
||||||
|
/// finished, unrelated calls (no id in common) must still end up under
|
||||||
|
/// one run name after the join.
|
||||||
|
#[test]
|
||||||
|
fn a_clean_boundary_between_two_finished_runs_is_still_healed_into_one_run() {
|
||||||
|
let older = fold_all(&[tool_start(1, "a", "Bash"), tool_end(2, "a", "old output")]);
|
||||||
|
let newer = fold_all(&[tool_start(3, "b", "Bash"), tool_end(4, "b", "new output")]);
|
||||||
|
let joined = join_pages(&older, &newer);
|
||||||
|
let run_ids: Vec<_> = joined
|
||||||
|
.iter()
|
||||||
|
.map(|item| match item {
|
||||||
|
TranscriptItem::ToolRun { run_id, .. } => run_id.as_str(),
|
||||||
|
other => panic!("expected only ToolRun items, got {other:?}"),
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
assert_eq!(
|
||||||
|
run_ids,
|
||||||
|
vec!["b", "b"],
|
||||||
|
"the older call must adopt the newer, already-on-screen run's name"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_call_split_across_the_boundary_merges_into_one_row() {
|
||||||
|
let older = fold_all(&[tool_start(1, "x", "Bash")]);
|
||||||
|
let newer = fold_all(&[tool_end(2, "x", "the result")]);
|
||||||
|
let joined = join_pages(&older, &newer);
|
||||||
|
assert_eq!(
|
||||||
|
joined,
|
||||||
|
vec![TranscriptItem::ToolRun {
|
||||||
|
seq: 1,
|
||||||
|
id: "x".to_string(),
|
||||||
|
run_id: "x".to_string(),
|
||||||
|
tool: "Bash".to_string(),
|
||||||
|
input: "{}".to_string(),
|
||||||
|
output: "the result".to_string(),
|
||||||
|
done: true,
|
||||||
|
asks: Vec::new(),
|
||||||
|
images: Vec::new(),
|
||||||
|
}],
|
||||||
|
"the older half's tool/input and the newer half's output/done must both survive"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_message_split_across_the_boundary_is_rejoined_with_the_newer_halfs_identity() {
|
||||||
|
let older = vec![TranscriptItem::AssistantMsg {
|
||||||
|
seq: 1,
|
||||||
|
text: "Hel".to_string(),
|
||||||
|
settled: false,
|
||||||
|
}];
|
||||||
|
let newer = vec![
|
||||||
|
TranscriptItem::AssistantMsg {
|
||||||
|
seq: 2,
|
||||||
|
text: "lo".to_string(),
|
||||||
|
settled: true,
|
||||||
|
},
|
||||||
|
TranscriptItem::UserMsg {
|
||||||
|
seq: 3,
|
||||||
|
text: "next".to_string(),
|
||||||
|
attachments: Vec::new(),
|
||||||
|
},
|
||||||
|
];
|
||||||
|
let joined = join_pages(&older, &newer);
|
||||||
|
assert_eq!(
|
||||||
|
joined,
|
||||||
|
vec![
|
||||||
|
TranscriptItem::AssistantMsg {
|
||||||
|
seq: 2,
|
||||||
|
text: "Hello".to_string(),
|
||||||
|
settled: true,
|
||||||
|
},
|
||||||
|
TranscriptItem::UserMsg {
|
||||||
|
seq: 3,
|
||||||
|
text: "next".to_string(),
|
||||||
|
attachments: Vec::new(),
|
||||||
|
},
|
||||||
|
]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A question is in a run of its own on both sides of a join -- healing
|
||||||
|
/// must never rename the run of calls the reader was asked something
|
||||||
|
/// on, the same rule `splitRun` enforces for a live turn boundary.
|
||||||
|
#[test]
|
||||||
|
fn adopt_run_never_renames_into_a_question_row() {
|
||||||
|
let older = fold_all(&[tool_start(1, "a", "Bash"), tool_end(2, "a", "done")]);
|
||||||
|
let newer = vec![TranscriptItem::ToolRun {
|
||||||
|
seq: 3,
|
||||||
|
id: "q".to_string(),
|
||||||
|
run_id: "q".to_string(),
|
||||||
|
tool: ASK_USER_QUESTION.to_string(),
|
||||||
|
input: "{}".to_string(),
|
||||||
|
output: String::new(),
|
||||||
|
done: false,
|
||||||
|
asks: Vec::new(),
|
||||||
|
images: Vec::new(),
|
||||||
|
}];
|
||||||
|
let joined = join_pages(&older, &newer);
|
||||||
|
match &joined[0] {
|
||||||
|
TranscriptItem::ToolRun { run_id, .. } => assert_eq!(run_id, "a"),
|
||||||
|
other => panic!("expected a ToolRun, got {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
@@ -0,0 +1,588 @@
|
|||||||
|
//! Where a session screen gets a transcript from: this phone's copy first,
|
||||||
|
//! the server for the rest. Ported from `app/.../TranscriptSource.kt`; see
|
||||||
|
//! `docs/TRANSCRIPT_CACHE.md` for the design this implements and
|
||||||
|
//! `docs/CLIENT_CORE.md` for how this file corresponds to the Kotlin.
|
||||||
|
//!
|
||||||
|
//! One seam rather than a cache the screen has to remember to consult.
|
||||||
|
//! Everything fetched before is asked of this, and everything the server
|
||||||
|
//! sends is written into the cache on the way past, so a caller never
|
||||||
|
//! learns which side answered. The one rule worth keeping in mind: the
|
||||||
|
//! cache is never load-bearing. Every read here has a network path beside
|
||||||
|
//! it producing the same result.
|
||||||
|
//!
|
||||||
|
//! **Not ported**: `EventStream.kt`'s reconnect-with-backoff loop and the
|
||||||
|
//! ability to close a live stream from another thread. Both are wall-clock
|
||||||
|
//! and thread-lifetime concerns that belong to whatever runtime the caller
|
||||||
|
//! embeds this crate in (a Tokio task, an iris timer, a Kotlin coroutine
|
||||||
|
//! scope) rather than to this pure logic -- `follow` below is the same
|
||||||
|
//! decorator shape `iris/desktop-app/src/app.rs` and
|
||||||
|
//! `iris/android-app/src/transcript_client.rs` already hand-wrote around
|
||||||
|
//! `event_stream::follow_session_events`, just with the cache write built
|
||||||
|
//! in so a future caller does not have to repeat it a third time.
|
||||||
|
|
||||||
|
use event_model::SeqEvent;
|
||||||
|
|
||||||
|
use crate::api::{ApiClient, ApiError, Transport};
|
||||||
|
use crate::event_stream::{self, StreamItem};
|
||||||
|
use crate::transcript_cache::SessionCache;
|
||||||
|
|
||||||
|
/// How many events a session screen opens with, cached or fetched.
|
||||||
|
///
|
||||||
|
/// The server's own default page size, named here because the cached
|
||||||
|
/// opening has to be the same size as the fetched one -- a reader must not
|
||||||
|
/// get a shorter first screen for having been here before (`OPENING_WINDOW`
|
||||||
|
/// in the Kotlin original).
|
||||||
|
pub const OPENING_WINDOW: u32 = 80;
|
||||||
|
|
||||||
|
/// A transcript-line parse failure, told apart from [`ApiError`] so a
|
||||||
|
/// caller can tell "the server is unreachable" from "the server (or this
|
||||||
|
/// phone's own disk) sent something this build cannot read" -- the two
|
||||||
|
/// mean different things to a reader (retry, versus a build that is
|
||||||
|
/// behind).
|
||||||
|
#[derive(Debug, Clone)]
|
||||||
|
pub struct ParseError(pub String);
|
||||||
|
|
||||||
|
impl std::fmt::Display for ParseError {
|
||||||
|
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||||
|
f.write_str(&self.0)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
impl std::error::Error for ParseError {}
|
||||||
|
|
||||||
|
/// Either half of what can go wrong asking for a page: the network, or a
|
||||||
|
/// line neither the cache's nor the server's copy of `parseSeqEvent` could
|
||||||
|
/// read.
|
||||||
|
#[derive(Debug, Clone)]
|
||||||
|
pub enum PageError {
|
||||||
|
Api(ApiError),
|
||||||
|
Parse(ParseError),
|
||||||
|
}
|
||||||
|
|
||||||
|
impl From<ApiError> for PageError {
|
||||||
|
fn from(e: ApiError) -> Self {
|
||||||
|
Self::Api(e)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl From<ParseError> for PageError {
|
||||||
|
fn from(e: ParseError) -> Self {
|
||||||
|
Self::Parse(e)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// What [`TranscriptSource::page`] found, kept as two states rather than
|
||||||
|
/// one possibly-empty list.
|
||||||
|
///
|
||||||
|
/// The difference is the whole of AGENTS.md's `loadOlderPage` incident: an
|
||||||
|
/// empty [`Self::Events`] means "this conversation has no more history",
|
||||||
|
/// which a caller is meant to latch, and [`Self::NothingLoaded`] means the
|
||||||
|
/// question could not be asked yet, which it must not. Collapsing the two
|
||||||
|
/// into an empty `Vec` puts the bug back, because the caller cannot tell
|
||||||
|
/// them apart -- and `unwrap_or_default()` on an `Option` would do the
|
||||||
|
/// same silently.
|
||||||
|
#[derive(Debug, Clone, PartialEq)]
|
||||||
|
pub enum OlderPage {
|
||||||
|
/// The events before the cursor, oldest first. Empty means the start
|
||||||
|
/// of the conversation has been reached.
|
||||||
|
Events(Vec<SeqEvent>),
|
||||||
|
/// Nothing is loaded, so there was no cursor to page back from
|
||||||
|
/// (`before == 0`). Not an answer about the conversation at all.
|
||||||
|
NothingLoaded,
|
||||||
|
}
|
||||||
|
|
||||||
|
fn parse_line(line: &str) -> Result<SeqEvent, ParseError> {
|
||||||
|
serde_json::from_str(line).map_err(|e| ParseError(format!("{e}")))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// This phone's copy of one session's transcript, plus the server it
|
||||||
|
/// falls back to. Ported from the Kotlin `TranscriptSource` class.
|
||||||
|
pub struct TranscriptSource<T: Transport> {
|
||||||
|
api: ApiClient<T>,
|
||||||
|
session_id: String,
|
||||||
|
pub cache: SessionCache,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<T: Transport> TranscriptSource<T> {
|
||||||
|
pub fn new(api: ApiClient<T>, session_id: impl Into<String>, cache: SessionCache) -> Self {
|
||||||
|
Self {
|
||||||
|
api,
|
||||||
|
session_id: session_id.into(),
|
||||||
|
cache,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The cached opening window, or `None` when there is nothing usable
|
||||||
|
/// to draw.
|
||||||
|
///
|
||||||
|
/// Meant to be drawn *before* [`Self::probe`] returns, which is the
|
||||||
|
/// whole point of the feature: the rows are on screen while the check
|
||||||
|
/// that they are still the server's rows is in flight, and a failed
|
||||||
|
/// check replaces them exactly as a reset does.
|
||||||
|
pub fn cached_opening(&self, limit: usize) -> Option<Vec<SeqEvent>> {
|
||||||
|
self.cache.tail()?;
|
||||||
|
let lines = self.cache.newest(limit);
|
||||||
|
if lines.is_empty() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
match lines.iter().map(|l| parse_line(l)).collect() {
|
||||||
|
Ok(events) => Some(events),
|
||||||
|
// A line this build cannot read at all, which the cache's own checks cannot
|
||||||
|
// see: it reads a seq off a line, not an event. Nothing to serve, so a cold
|
||||||
|
// open.
|
||||||
|
Err(ParseError(_)) => {
|
||||||
|
self.cache.purge();
|
||||||
|
None
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Whether the server's event at the cached cursor is still the cached
|
||||||
|
/// one.
|
||||||
|
///
|
||||||
|
/// A caller must not resume a live stream from a cached seq unless it
|
||||||
|
/// is the same conversation: a transcript is append-only in ordinary
|
||||||
|
/// use, but the file backing it can be replaced or truncated (a
|
||||||
|
/// sandbox re-seeded with the same ids, a backup restored, a session
|
||||||
|
/// re-imported), and the server's catch-up on such a file would hand
|
||||||
|
/// this phone a continuation of a *different* conversation, spliced
|
||||||
|
/// onto the cached one with no seam. Caught with one request of a few
|
||||||
|
/// hundred bytes.
|
||||||
|
///
|
||||||
|
/// `Ok(false)` purges the cache and means "open cold". `Err` is the
|
||||||
|
/// server not being askable, which is neither: the cached rows stay
|
||||||
|
/// on screen and the caller tries again on its own reconnect schedule.
|
||||||
|
///
|
||||||
|
/// What this cannot see is a line changed in the middle of the file
|
||||||
|
/// with the tail intact -- that is what a full reload is for.
|
||||||
|
pub fn probe(&self) -> Result<bool, ApiError> {
|
||||||
|
let Some(tail) = self.cache.tail() else {
|
||||||
|
return Ok(false);
|
||||||
|
};
|
||||||
|
// `before = seq + 1` is the newest event with seq <= the cursor, which is the
|
||||||
|
// event *at* the cursor when the server still has one there.
|
||||||
|
let page = self.api.fetch_transcript_lines(
|
||||||
|
&self.session_id,
|
||||||
|
Some(tail.seq + 1),
|
||||||
|
1,
|
||||||
|
false,
|
||||||
|
None,
|
||||||
|
)?;
|
||||||
|
let matches = page.len() == 1
|
||||||
|
&& parse_line(&tail.line)
|
||||||
|
.map(|cached| cached == page[0].1)
|
||||||
|
.unwrap_or(false);
|
||||||
|
if !matches {
|
||||||
|
self.cache.purge();
|
||||||
|
}
|
||||||
|
Ok(matches)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Today's opening fetch, kept as the start of the live run. Only
|
||||||
|
/// called when the cache has nothing to open with, or when
|
||||||
|
/// [`Self::probe`] said what it had was not the server's.
|
||||||
|
pub fn fetch_opening(&self) -> Result<Vec<SeqEvent>, ApiError> {
|
||||||
|
let page =
|
||||||
|
self.api
|
||||||
|
.fetch_transcript_lines(&self.session_id, None, OPENING_WINDOW, false, None)?;
|
||||||
|
for (line, event) in &page {
|
||||||
|
self.cache.append(line, event.seq);
|
||||||
|
}
|
||||||
|
self.cache.flush();
|
||||||
|
Ok(page.into_iter().map(|(_, event)| event).collect())
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The page before `before`: from the cache when it holds it,
|
||||||
|
/// otherwise from the server bounded by what the cache already has.
|
||||||
|
///
|
||||||
|
/// The server bound (`after`) is what keeps the cache worth having. A
|
||||||
|
/// coalesced page reaches back as far as its row count takes it -- a
|
||||||
|
/// single reply is hundreds of lines -- so a page fetched after the
|
||||||
|
/// reader has been away could run straight past the cached run and
|
||||||
|
/// overlap it, and an overlapping page cannot be stored. Told where
|
||||||
|
/// this phone's copy starts, the server stops there instead.
|
||||||
|
///
|
||||||
|
/// `before == 0` answers [`OlderPage::NothingLoaded`] without asking
|
||||||
|
/// the cache or the server anything -- see AGENTS.md's "things that
|
||||||
|
/// have bitten": there is no event before the first one, so the
|
||||||
|
/// request is not a harmless no-op, and its empty answer is
|
||||||
|
/// indistinguishable from having reached the start of history.
|
||||||
|
/// Guarded here rather than left to every caller, because it is a fact
|
||||||
|
/// about the question, not about who is asking it.
|
||||||
|
pub fn page(&self, before: u64, limit: u32, coalesce: bool) -> Result<OlderPage, PageError> {
|
||||||
|
if before == 0 {
|
||||||
|
return Ok(OlderPage::NothingLoaded);
|
||||||
|
}
|
||||||
|
if let Some(lines) = self.cache.page(before, limit as usize, coalesce) {
|
||||||
|
let events: Vec<SeqEvent> = lines
|
||||||
|
.iter()
|
||||||
|
.map(|l| parse_line(l).map_err(PageError::from))
|
||||||
|
.collect::<Result<_, _>>()?;
|
||||||
|
return Ok(OlderPage::Events(events));
|
||||||
|
}
|
||||||
|
let after = self.cache.covered_up_to(before).map(|v| v - 1);
|
||||||
|
let page = self.api.fetch_transcript_lines(
|
||||||
|
&self.session_id,
|
||||||
|
Some(before),
|
||||||
|
limit,
|
||||||
|
coalesce,
|
||||||
|
after,
|
||||||
|
)?;
|
||||||
|
if let Some((_, first_event)) = page.first() {
|
||||||
|
// `before` rather than the newest line's seq: a coalesced page covers
|
||||||
|
// everything up to the cursor it was asked with, and nothing in its lines
|
||||||
|
// says so.
|
||||||
|
let lines: Vec<String> = page.iter().map(|(line, _)| line.clone()).collect();
|
||||||
|
self.cache
|
||||||
|
.store_page(&lines, first_event.seq, before, coalesce);
|
||||||
|
}
|
||||||
|
Ok(OlderPage::Events(
|
||||||
|
page.into_iter().map(|(_, event)| event).collect(),
|
||||||
|
))
|
||||||
|
}
|
||||||
|
|
||||||
|
/// [`event_stream::follow_session_events`], with every frame written to
|
||||||
|
/// the cache before `on_item` sees it.
|
||||||
|
///
|
||||||
|
/// Before, so that an event held back for a reader who is scrolled
|
||||||
|
/// away is already on disk -- what the cache holds is what the server
|
||||||
|
/// sent, not what a screen has got round to drawing. Flushed on each
|
||||||
|
/// status change, which is a turn's boundary and the granularity a
|
||||||
|
/// crash may as well lose, and once more when the stream ends.
|
||||||
|
pub fn follow(
|
||||||
|
&self,
|
||||||
|
after: u64,
|
||||||
|
mut on_item: impl FnMut(StreamItem) -> bool,
|
||||||
|
) -> Result<(), ApiError> {
|
||||||
|
let cache = &self.cache;
|
||||||
|
let result = event_stream::follow_session_events(
|
||||||
|
self.api.transport(),
|
||||||
|
&self.session_id,
|
||||||
|
after,
|
||||||
|
|item| {
|
||||||
|
if let StreamItem::Event { raw, event } = &item {
|
||||||
|
cache.append(raw, event.seq);
|
||||||
|
if matches!(event.event, event_model::Event::Status { .. }) {
|
||||||
|
cache.flush();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
on_item(item)
|
||||||
|
},
|
||||||
|
);
|
||||||
|
cache.flush();
|
||||||
|
result
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Leaves the cache with everything it was given -- called once a
|
||||||
|
/// caller is done with this source, mirroring the Kotlin `close`'s
|
||||||
|
/// final flush (that method's stream cancellation itself is the
|
||||||
|
/// runtime concern the module doc says is not ported here).
|
||||||
|
pub fn close(&self) {
|
||||||
|
self.cache.flush();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use crate::api::{Body, RawResponse};
|
||||||
|
use std::collections::VecDeque;
|
||||||
|
use std::io::Read;
|
||||||
|
use std::sync::Mutex;
|
||||||
|
|
||||||
|
/// A transport that answers fixed bodies in call order, and records
|
||||||
|
/// every path it was asked for -- so a test can assert *how many*
|
||||||
|
/// requests a method made, which is the point for the `before == 0`
|
||||||
|
/// guard (AGENTS.md's regression: the guard must stop the request
|
||||||
|
/// before it happens, not merely tolerate the empty answer).
|
||||||
|
#[derive(Default)]
|
||||||
|
struct ScriptedTransport {
|
||||||
|
responses: Mutex<VecDeque<(u16, String)>>,
|
||||||
|
calls: Mutex<Vec<String>>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl ScriptedTransport {
|
||||||
|
fn respond(&self, status: u16, body: impl Into<String>) {
|
||||||
|
self.responses
|
||||||
|
.lock()
|
||||||
|
.unwrap()
|
||||||
|
.push_back((status, body.into()));
|
||||||
|
}
|
||||||
|
|
||||||
|
fn call_count(&self) -> usize {
|
||||||
|
self.calls.lock().unwrap().len()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl Transport for ScriptedTransport {
|
||||||
|
fn request(
|
||||||
|
&self,
|
||||||
|
_method: &str,
|
||||||
|
path: &str,
|
||||||
|
_body: Option<Body>,
|
||||||
|
) -> Result<RawResponse, ApiError> {
|
||||||
|
self.calls.lock().unwrap().push(path.to_string());
|
||||||
|
let (status, body) = self
|
||||||
|
.responses
|
||||||
|
.lock()
|
||||||
|
.unwrap()
|
||||||
|
.pop_front()
|
||||||
|
.unwrap_or_else(|| panic!("ScriptedTransport got an unscripted request: {path}"));
|
||||||
|
Ok(RawResponse {
|
||||||
|
status,
|
||||||
|
body: body.into_bytes(),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
fn stream(&self, path: &str) -> Result<Box<dyn Read + Send>, ApiError> {
|
||||||
|
self.calls.lock().unwrap().push(path.to_string());
|
||||||
|
let (_, body) = self
|
||||||
|
.responses
|
||||||
|
.lock()
|
||||||
|
.unwrap()
|
||||||
|
.pop_front()
|
||||||
|
.unwrap_or_else(|| {
|
||||||
|
panic!("ScriptedTransport got an unscripted stream request: {path}")
|
||||||
|
});
|
||||||
|
Ok(Box::new(std::io::Cursor::new(body.into_bytes())))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn source(
|
||||||
|
transport: ScriptedTransport,
|
||||||
|
cache_root: &std::path::Path,
|
||||||
|
) -> TranscriptSource<ScriptedTransport> {
|
||||||
|
let api = ApiClient::new(transport);
|
||||||
|
let cache = crate::transcript_cache::TranscriptCache::new(cache_root).session("s1");
|
||||||
|
TranscriptSource::new(api, "s1", cache)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn status_line(seq: u64) -> String {
|
||||||
|
format!(r#"{{"seq":{seq},"ts":1.0,"type":"status","state":"idle"}}"#)
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_cold_cache_has_no_opening_and_fetches_from_the_server() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let transport = ScriptedTransport::default();
|
||||||
|
transport.respond(200, format!("[{}]", status_line(1)));
|
||||||
|
let source = source(transport, dir.path());
|
||||||
|
|
||||||
|
assert_eq!(source.cached_opening(80), None);
|
||||||
|
let opening = source.fetch_opening().unwrap();
|
||||||
|
assert_eq!(opening.len(), 1);
|
||||||
|
assert_eq!(opening[0].seq, 1);
|
||||||
|
// The fetch wrote through: reopening the same cache now has something to show.
|
||||||
|
assert!(source.cache.tail().is_some());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn probe_matching_the_cached_tail_leaves_the_cache_alone() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let transport = ScriptedTransport::default();
|
||||||
|
transport.respond(200, format!("[{}]", status_line(1)));
|
||||||
|
let source = source(transport, dir.path());
|
||||||
|
source.fetch_opening().unwrap();
|
||||||
|
|
||||||
|
let transport2 = ScriptedTransport::default();
|
||||||
|
transport2.respond(200, format!("[{}]", status_line(1)));
|
||||||
|
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
|
||||||
|
let source2 = TranscriptSource::new(ApiClient::new(transport2), "s1", cache);
|
||||||
|
assert!(source2.probe().unwrap());
|
||||||
|
assert!(source2.cache.tail().is_some());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn probe_mismatching_the_cached_tail_purges_the_cache() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let transport = ScriptedTransport::default();
|
||||||
|
transport.respond(200, format!("[{}]", status_line(1)));
|
||||||
|
let source = source(transport, dir.path());
|
||||||
|
source.fetch_opening().unwrap();
|
||||||
|
|
||||||
|
// The server now answers with a different event at the same seq -- the file
|
||||||
|
// behind this session was replaced.
|
||||||
|
let transport2 = ScriptedTransport::default();
|
||||||
|
let different = r#"{"seq":1,"ts":1.0,"type":"status","state":"running"}"#.to_string();
|
||||||
|
transport2.respond(200, format!("[{different}]"));
|
||||||
|
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
|
||||||
|
let source2 = TranscriptSource::new(ApiClient::new(transport2), "s1", cache);
|
||||||
|
assert!(!source2.probe().unwrap());
|
||||||
|
assert!(source2.cache.tail().is_none());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn probe_finding_no_server_leaves_the_cache_untouched() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let transport = ScriptedTransport::default();
|
||||||
|
transport.respond(200, format!("[{}]", status_line(1)));
|
||||||
|
let source = source(transport, dir.path());
|
||||||
|
source.fetch_opening().unwrap();
|
||||||
|
|
||||||
|
let transport2 = ScriptedTransport::default();
|
||||||
|
transport2.respond(500, "server on fire");
|
||||||
|
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
|
||||||
|
let source2 = TranscriptSource::new(ApiClient::new(transport2), "s1", cache);
|
||||||
|
assert!(source2.probe().is_err());
|
||||||
|
assert!(
|
||||||
|
source2.cache.tail().is_some(),
|
||||||
|
"an unreachable server must not be treated as a mismatch"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The regression this module exists to close: `before == 0` must
|
||||||
|
/// never reach the network or the cache, because an empty answer there
|
||||||
|
/// is indistinguishable from "there is genuinely no more history" --
|
||||||
|
/// AGENTS.md's `loadOlderPage` incident.
|
||||||
|
#[test]
|
||||||
|
fn paging_before_the_first_event_makes_no_request_at_all() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let transport = ScriptedTransport::default();
|
||||||
|
let source = source(transport, dir.path());
|
||||||
|
assert_eq!(source.page(0, 80, true).unwrap(), OlderPage::NothingLoaded);
|
||||||
|
assert_eq!(source.api.transport().call_count(), 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_page_already_covered_by_the_cache_never_reaches_the_server() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let transport = ScriptedTransport::default();
|
||||||
|
transport.respond(200, format!("[{},{}]", status_line(1), status_line(2)));
|
||||||
|
let source = source(transport, dir.path());
|
||||||
|
source.fetch_opening().unwrap();
|
||||||
|
|
||||||
|
let calls_before = source.api.transport().call_count();
|
||||||
|
let OlderPage::Events(page) = source.page(2, 10, true).unwrap() else {
|
||||||
|
panic!("a cursor of 2 is a real question about the conversation");
|
||||||
|
};
|
||||||
|
assert_eq!(page.len(), 1);
|
||||||
|
assert_eq!(page[0].seq, 1);
|
||||||
|
assert_eq!(
|
||||||
|
source.api.transport().call_count(),
|
||||||
|
calls_before,
|
||||||
|
"a cache hit must not touch the network"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// With nothing older cached there is no floor to give the server, so
|
||||||
|
/// the request carries no `after` at all.
|
||||||
|
#[test]
|
||||||
|
fn a_server_page_with_nothing_older_cached_carries_no_bound() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let transport = ScriptedTransport::default();
|
||||||
|
transport.respond(200, format!("[{}]", status_line(5)));
|
||||||
|
let source = source(transport, dir.path());
|
||||||
|
source.fetch_opening().unwrap();
|
||||||
|
|
||||||
|
let transport2 = ScriptedTransport::default();
|
||||||
|
transport2.respond(200, format!("[{}]", status_line(3)));
|
||||||
|
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
|
||||||
|
let source2 = TranscriptSource::new(ApiClient::new(transport2), "s1", cache);
|
||||||
|
source2.page(5, 10, true).unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
source2.api.transport().calls.lock().unwrap()[0],
|
||||||
|
"/sessions/s1/transcript?limit=10&before=5&coalesce=true"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The half the test above cannot show: when the cache *does* hold an
|
||||||
|
/// older run, the fetch is floored at its end, or the page would run
|
||||||
|
/// straight past it and overlap -- which `store_page` then refuses,
|
||||||
|
/// silently costing the phone the page it just paid for.
|
||||||
|
#[test]
|
||||||
|
fn a_server_page_is_floored_at_the_end_of_the_cached_run() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
|
||||||
|
// A stored page covering [3, 6) and two live events above it, so the run this
|
||||||
|
// phone holds is [3, 8) -- the newest chunk has to be an appended one, or the
|
||||||
|
// cache reads the directory as damaged and discards it.
|
||||||
|
let lines: Vec<String> = (3..6).map(status_line).collect();
|
||||||
|
assert!(cache.store_page(&lines, 3, 6, true));
|
||||||
|
cache.append(&status_line(6), 6);
|
||||||
|
cache.append(&status_line(7), 7);
|
||||||
|
cache.flush();
|
||||||
|
|
||||||
|
let transport = ScriptedTransport::default();
|
||||||
|
transport.respond(200, format!("[{}]", status_line(9)));
|
||||||
|
let source = TranscriptSource::new(ApiClient::new(transport), "s1", cache);
|
||||||
|
source.page(10, 10, true).unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
source.api.transport().calls.lock().unwrap()[0],
|
||||||
|
"/sessions/s1/transcript?limit=10&before=10&coalesce=true&after=7",
|
||||||
|
"the fetch must stop one seq below where this phone's copy ends"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A page the server could not answer is an error, never an empty
|
||||||
|
/// page: the caller would read the second as "this conversation has no
|
||||||
|
/// more history" and stop paging for good.
|
||||||
|
#[test]
|
||||||
|
fn a_failing_server_page_is_an_error_rather_than_an_empty_one() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let transport = ScriptedTransport::default();
|
||||||
|
transport.respond(500, "server on fire");
|
||||||
|
let source = source(transport, dir.path());
|
||||||
|
assert!(matches!(source.page(9, 10, true), Err(PageError::Api(_)),));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A cached line this build cannot read is told apart from the network
|
||||||
|
/// failing, for the same reason: neither is "no more history".
|
||||||
|
#[test]
|
||||||
|
fn an_unreadable_cached_page_is_a_parse_error_rather_than_an_empty_one() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
|
||||||
|
cache.store_page(
|
||||||
|
&[r#"{"seq":3,"but":"not an event"}"#.to_string()],
|
||||||
|
3,
|
||||||
|
4,
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
cache.append(&status_line(4), 4);
|
||||||
|
cache.flush();
|
||||||
|
let transport = ScriptedTransport::default();
|
||||||
|
let source = TranscriptSource::new(ApiClient::new(transport), "s1", cache);
|
||||||
|
assert!(matches!(source.page(4, 10, true), Err(PageError::Parse(_)),));
|
||||||
|
assert_eq!(
|
||||||
|
source.api.transport().call_count(),
|
||||||
|
0,
|
||||||
|
"a cache hit that cannot be read must not fall through to the server unnoticed"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_bad_cached_opening_line_purges_rather_than_panicking() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
|
||||||
|
cache.append("not json at all", 1);
|
||||||
|
cache.flush();
|
||||||
|
let transport = ScriptedTransport::default();
|
||||||
|
let source = TranscriptSource::new(ApiClient::new(transport), "s1", cache);
|
||||||
|
assert_eq!(source.cached_opening(80), None);
|
||||||
|
assert!(
|
||||||
|
source.cache.tail().is_none(),
|
||||||
|
"a damaged line purges the cache"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn follow_writes_events_to_the_cache_before_the_caller_sees_them() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let transport = ScriptedTransport::default();
|
||||||
|
transport.respond(200, format!("{}\n\n", sse_frame(&status_line(1))));
|
||||||
|
let source = source(transport, dir.path());
|
||||||
|
let mut seen = Vec::new();
|
||||||
|
source
|
||||||
|
.follow(0, |item| {
|
||||||
|
if let StreamItem::Event { event, .. } = item {
|
||||||
|
seen.push(event.seq);
|
||||||
|
}
|
||||||
|
true
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(seen, vec![1]);
|
||||||
|
assert_eq!(source.cache.tail().unwrap().seq, 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
fn sse_frame(data: &str) -> String {
|
||||||
|
format!("data:{data}")
|
||||||
|
}
|
||||||
|
}
|
||||||
+84
-17
@@ -25,16 +25,19 @@ next (a Masonry or iris transcript screen, most likely).
|
|||||||
| `sse.rs` | `Sse.kt` (the framing half) | Done, new tests (Kotlin had none of its own beyond integration) |
|
| `sse.rs` | `Sse.kt` (the framing half) | Done, new tests (Kotlin had none of its own beyond integration) |
|
||||||
| `api.rs` | `Api.kt` | Partial -- see below |
|
| `api.rs` | `Api.kt` | Partial -- see below |
|
||||||
| `event_stream.rs` | `EventStream.kt` | Done |
|
| `event_stream.rs` | `EventStream.kt` | Done |
|
||||||
| `transcript_fold.rs` | `TranscriptItems.kt`, `ToolRows.kt` | Partial -- see below |
|
| `transcript_fold.rs` | `TranscriptItems.kt`, `ToolRows.kt` | Done -- see below |
|
||||||
| `config.rs` | `ServerConfig.kt`'s `handleEnrollment` | New, desktop-only so far -- see below |
|
| `config.rs` | `ServerConfig.kt`'s `handleEnrollment` | New, desktop-only so far -- see below |
|
||||||
| *(not started)* | `TranscriptSource.kt` | Not started |
|
| `transcript_source.rs` | `TranscriptSource.kt` | Done -- see below |
|
||||||
| *(not ported, and may never be)* | `TranscriptUnits.kt` | Out of scope -- see below |
|
| *(not ported, and may never be)* | `TranscriptUnits.kt` | Out of scope -- see below |
|
||||||
|
|
||||||
Every file above whose Kotlin counterpart had a JVM unit test (`AnsiTest`,
|
Every file above whose Kotlin counterpart had a JVM unit test (`AnsiTest`,
|
||||||
`HighlighterTest`, `TranscriptCacheTest`) has had every one of those test
|
`HighlighterTest`, `TranscriptCacheTest`) has had every one of those test
|
||||||
cases ported alongside it, plus new tests for the pieces that had none
|
cases ported alongside it, plus new tests for the pieces that had none
|
||||||
(`sse.rs`, `api.rs`, `event_stream.rs`, `transcript_fold.rs`). Test count by
|
(`sse.rs`, `api.rs`, `event_stream.rs`, `transcript_fold.rs`,
|
||||||
crate as of this writing: **85 in `client-core`**, 0 in `event-model` (its
|
`transcript_source.rs` -- the Kotlin `TranscriptSource.kt`/`TranscriptItems.kt`
|
||||||
|
had no JVM unit tests of their own, so these were written fresh against the
|
||||||
|
Kotlin source and AGENTS.md's paging incidents as the spec). Test count by
|
||||||
|
crate as of this writing: **109 in `client-core`**, 0 in `event-model` (its
|
||||||
types carry no logic of their own to test -- `server/`'s own tests exercise
|
types carry no logic of their own to test -- `server/`'s own tests exercise
|
||||||
them via `session::transcript`'s round-trip coverage).
|
them via `session::transcript`'s round-trip coverage).
|
||||||
|
|
||||||
@@ -88,13 +91,27 @@ the full table to work from when one of these is next.
|
|||||||
including tool-call/question/image attachment and peer-message placement.
|
including tool-call/question/image attachment and peer-message placement.
|
||||||
`group_tool_runs` groups adjacent calls into `TranscriptRow::Tools`.
|
`group_tool_runs` groups adjacent calls into `TranscriptRow::Tools`.
|
||||||
|
|
||||||
**Not ported:** `TranscriptItems.kt`'s `joinPages` (and its
|
`join_pages` (with `heal_split_message` and `adopt_run`, both private) is
|
||||||
`healSplitMessage`/`adoptRun` helpers) -- the page-boundary healing that
|
now ported too, 2026-09-06 -- the page-boundary healing that merges a tool
|
||||||
merges a tool call split across two fetched pages and re-merges a run a
|
call split across two fetched pages, rejoins a message a boundary cut
|
||||||
boundary cut through. This matters the moment paging backward through
|
through, and renames a run of tool calls onto whichever name is already on
|
||||||
history is exercised; it is deliberately left rather than rushed, since
|
screen. Ported with AGENTS.md's "things that have bitten" incidents as the
|
||||||
it is exactly the kind of boundary logic this project's own "things that
|
spec rather than a JVM test file (`TranscriptItems.kt` had none of its
|
||||||
have bitten" section warns reads fine and is wrong at the edges.
|
own): `a_clean_boundary_between_two_finished_runs_is_still_healed_into_one_run`
|
||||||
|
is the regression test for the bug that shipped -- `adopt_run` must run on
|
||||||
|
*every* join, not only the one where a split call was found, or a boundary
|
||||||
|
landing cleanly between two already-finished calls (most of them) leaves
|
||||||
|
one run drawn as two. `a_call_split_across_the_boundary_merges_into_one_row`,
|
||||||
|
`a_message_split_across_the_boundary_is_rejoined_with_the_newer_halfs_identity`,
|
||||||
|
and `adopt_run_never_renames_into_a_question_row` cover the other three
|
||||||
|
edges the Kotlin doc calls out. `join_pages` ends in a `debug_assert!`
|
||||||
|
that no tool id survives in both halves -- the duplicate row it exists to
|
||||||
|
prevent, checked rather than assumed. What it deliberately does *not*
|
||||||
|
assert is seq ordering across the boundary: a peer note carries the seq
|
||||||
|
its turn began at (`place_peer_note`), which can be older than the page
|
||||||
|
it arrived in, so the two pages' seqs legitimately interleave there. An
|
||||||
|
earlier draft asserted it and would have panicked in debug builds on an
|
||||||
|
ordinary transcript.
|
||||||
|
|
||||||
**Known gap, and a decision for whoever closes it:** `event_model::Event`
|
**Known gap, and a decision for whoever closes it:** `event_model::Event`
|
||||||
has no `Unknown`/catch-all variant, unlike `Events.kt`'s hand-kept mirror.
|
has no `Unknown`/catch-all variant, unlike `Events.kt`'s hand-kept mirror.
|
||||||
@@ -119,12 +136,61 @@ caller-specific (the code rules' "ask for the least you need"). Its only
|
|||||||
caller today is `desktop-app`; a future Android build of this crate would
|
caller today is `desktop-app`; a future Android build of this crate would
|
||||||
be a second one, not a reason to move the type.
|
be a second one, not a reason to move the type.
|
||||||
|
|
||||||
|
## What `transcript_source.rs` covers, and what it does not
|
||||||
|
|
||||||
|
`TranscriptSource<T: Transport>` is the seam a session screen asks for a
|
||||||
|
page, ported test-for-test against the Kotlin doc rather than a JVM test
|
||||||
|
file (there wasn't one): `cached_opening`, `probe`, `fetch_opening`,
|
||||||
|
`page` and `follow`, each matching its Kotlin namesake's contract --
|
||||||
|
including `probe`'s three-way outcome (matches / cache purged /
|
||||||
|
unreachable, told apart so a caller never treats "couldn't ask" as "was
|
||||||
|
wrong") and `page`'s cache-vs-server split bounded by `covered_up_to`.
|
||||||
|
|
||||||
|
Two additions beyond a literal port, both load-bearing:
|
||||||
|
|
||||||
|
- **`page(before, ..)` refuses `before == 0` before touching the cache or
|
||||||
|
the network**, answering `OlderPage::NothingLoaded`. This is AGENTS.md's
|
||||||
|
`loadOlderPage` incident (`before = 0` is "no event before the first
|
||||||
|
one," indistinguishable from "reached the start of history" if a caller
|
||||||
|
ever asks it) moved out of the Kotlin screen and into this layer, so
|
||||||
|
every future caller gets the guard rather than having to remember it.
|
||||||
|
**The return type is `OlderPage`, not a `Vec`, and that is the guard.**
|
||||||
|
The Kotlin's two falses are different answers -- `oldestSeq == 0`
|
||||||
|
returns without touching `moreHistory`, an empty page latches it false
|
||||||
|
-- so a port that answered both with an empty list would have moved the
|
||||||
|
bug rather than fixed it, one layer down and out of sight of the screen
|
||||||
|
that used to hold the check. `OlderPage::Events(vec![])` means the start
|
||||||
|
of the conversation; `OlderPage::NothingLoaded` is not an answer about
|
||||||
|
the conversation at all. Reviewed 2026-09-06.
|
||||||
|
`paging_before_the_first_event_makes_no_request_at_all` asserts zero
|
||||||
|
transport calls, not just the variant, since a request that happens to
|
||||||
|
answer empty is exactly what caused the original bug, and
|
||||||
|
`a_failing_server_page_is_an_error_rather_than_an_empty_one` plus
|
||||||
|
`an_unreadable_cached_page_is_a_parse_error_rather_than_an_empty_one`
|
||||||
|
are the same rule for the two ways a page can fail.
|
||||||
|
- **`fetch_transcript_lines`** (new in `api.rs`) hands back each line
|
||||||
|
paired with the exact server bytes it came from, via
|
||||||
|
`serde_json::value::RawValue` rather than re-serializing a parsed
|
||||||
|
`Value` -- the cache and a live SSE frame for the same event have to
|
||||||
|
agree byte-for-byte, which is exactly what the `serde_json`
|
||||||
|
float-rounding bug (AGENTS.md) was about. The existing
|
||||||
|
`fetch_transcript_page` is untouched (other callers under `iris/`
|
||||||
|
depend on its signature); the two share a `transcript_path` helper so
|
||||||
|
the query string is written in one place.
|
||||||
|
|
||||||
|
**Not ported:** `EventStream.kt`'s reconnect-with-backoff loop, and
|
||||||
|
`TranscriptSource.close`'s ability to cancel a live stream from another
|
||||||
|
thread. Both are wall-clock/thread-lifetime policy that belongs to
|
||||||
|
whichever runtime embeds this crate (iris's own timers, a Tokio task, a
|
||||||
|
Kotlin coroutine scope), not to this pure logic -- `follow` is the same
|
||||||
|
"write to the cache, then hand the frame to the caller" decorator
|
||||||
|
`iris/desktop-app/src/app.rs` and `iris/android-app/src/transcript_client.rs`
|
||||||
|
already hand-wrote around `event_stream::follow_session_events` before this
|
||||||
|
existed; the cache write moved into one shared place so a third caller
|
||||||
|
does not repeat it again by hand.
|
||||||
|
|
||||||
## What is not started at all
|
## What is not started at all
|
||||||
|
|
||||||
- **`TranscriptSource.kt`** -- the layer that decides whether a page comes
|
|
||||||
from the transcript cache or the server, and stitches the two. Needs
|
|
||||||
`transcript_cache.rs` and `api.rs`'s transcript-page method, both of
|
|
||||||
which exist now, so this is unblocked whenever picked up.
|
|
||||||
- **The markdown *block* model beyond syntax spans** -- `highlight/markdown.rs`
|
- **The markdown *block* model beyond syntax spans** -- `highlight/markdown.rs`
|
||||||
colours a `.md` file or fence for the highlighter, but does not build the
|
colours a `.md` file or fence for the highlighter, but does not build the
|
||||||
block tree (headings, lists, tables, fences as distinct nodes) that a
|
block tree (headings, lists, tables, fences as distinct nodes) that a
|
||||||
@@ -142,5 +208,6 @@ be a second one, not a reason to move the type.
|
|||||||
|
|
||||||
`./run-tests.sh` from the repo root now runs `event-model`, `client-core`
|
`./run-tests.sh` from the repo root now runs `event-model`, `client-core`
|
||||||
and `server` in that order (each `cargo test`, forwarding arguments the
|
and `server` in that order (each `cargo test`, forwarding arguments the
|
||||||
same way it always has). From `client-core/` directly: `cargo test`,
|
same way it always has). From `client-core/` directly: `cargo test`
|
||||||
`cargo clippy --all-targets`, `cargo fmt` -- all clean as of this writing.
|
(109 tests), `cargo clippy --all-targets`, `cargo fmt` -- all clean as of
|
||||||
|
this writing (2026-09-06).
|
||||||
+37
-1
@@ -552,7 +552,14 @@ streamed event" cost RUST.md's P0 box measured (20 events/second against a
|
|||||||
new rows appended after it. A row changing *before* the tail (only
|
new rows appended after it. A row changing *before* the tail (only
|
||||||
`group_tool_runs` retroactively grouping tool calls into a run does
|
`group_tool_runs` retroactively grouping tool calls into a run does
|
||||||
this) falls back to `List::clear` plus a full rebuild, counted in
|
this) falls back to `List::clear` plus a full rebuild, counted in
|
||||||
`TranscriptScreen::take_rebuilds()`. `bench_client.rs`, `transcript_client.rs`
|
`TranscriptScreen::take_rebuilds()`. **A caller that keeps its own
|
||||||
|
row-keyed side table alongside `List` (`Selection`'s `rows:
|
||||||
|
BTreeMap<RowKey, WeakWidget<TextEdit>>` is the one this crate has) must
|
||||||
|
clear it in step with `List::clear()`** — the fallback drops every row
|
||||||
|
`List` was holding, so any side table not cleared the same way is left
|
||||||
|
pointing at widgets the clear just freed (docs/REVIEW-2026-09-06.md
|
||||||
|
finding 1, fixed 2026-09-06 by `Selection::clear()`, called from
|
||||||
|
`apply`'s `Rebuild` arm right before `List::clear()`). `bench_client.rs`, `transcript_client.rs`
|
||||||
and `desktop-app/app.rs` all call this now instead of rebuilding on every
|
and `desktop-app/app.rs` all call this now instead of rebuilding on every
|
||||||
event; only the opening page (and `apply`'s own fallback) still calls
|
event; only the opening page (and `apply`'s own fallback) still calls
|
||||||
`build_tree`.
|
`build_tree`.
|
||||||
@@ -651,3 +658,32 @@ box has the full investigation and the phone verification still to do.
|
|||||||
built and checked on this checkout's emulator only. RUST.md's P0 box
|
built and checked on this checkout's emulator only. RUST.md's P0 box
|
||||||
says what she should check for: crisp text at two densities, the
|
says what she should check for: crisp text at two densities, the
|
||||||
keyboard no longer wiping, and the header's background.
|
keyboard no longer wiping, and the header's background.
|
||||||
|
|
||||||
|
## 2026-09-06: composing text, focus-on-tap, and atlas invalidation on a new renderer
|
||||||
|
|
||||||
|
Three small but public API changes, from the same phone-report pass as the
|
||||||
|
entry above (RUST.md's P0 box has the full account, including a real bug
|
||||||
|
still not root-caused).
|
||||||
|
|
||||||
|
- **`FocusHost` gained `is_focused(&self, id) -> bool`** (both platform
|
||||||
|
impls). `attr.rs`'s `Selector`/`Selectable` used to grant focus (and so
|
||||||
|
request the IME) on the very first frame of *any* press, before it was
|
||||||
|
known whether the gesture was a tap or a drag — a swipe over a text
|
||||||
|
field wrongly summoned the keyboard. They now wait for a completed tap
|
||||||
|
(press and release with no frame crossing `sense::DRAG_SLOP`) unless the
|
||||||
|
field is already focused, in which case dragging inside it to select
|
||||||
|
text is unchanged. `TextEdit` gained one new `pub(crate)` field
|
||||||
|
(`press_origin`) to track this; no public surface change there.
|
||||||
|
- **`android::ime`'s `InputConnection` now calls `InputMethodManager::
|
||||||
|
updateSelection` after every edit** (`IrisViewPeer::update_ime_selection`,
|
||||||
|
called from `after_input`). Gboard was holding keystrokes back because
|
||||||
|
nothing ever told it where the app's own selection/composing region had
|
||||||
|
moved to — this is what android-view's own demo does in its `render()`
|
||||||
|
and this bridge never did.
|
||||||
|
- **`GlyphAtlas::clear()` and `Textures::reset()`** (`iris_core`). Called
|
||||||
|
together, once, from `android::view`'s `surface_changed` exactly when a
|
||||||
|
*genuinely new* `AndroidRenderer` is built (backgrounding and returning,
|
||||||
|
not a keyboard-triggered resize, which already reuses the renderer) —
|
||||||
|
both CPU-side caches otherwise kept pointing at the old, now-destroyed
|
||||||
|
device's textures, which is why text used to vanish again after leaving
|
||||||
|
and returning to the app.
|
||||||
@@ -149,6 +149,104 @@ agent takes them without colliding with that pass's `bench_client.rs`/
|
|||||||
confirming this was the whole story on real touch input rather than
|
confirming this was the whole story on real touch input rather than
|
||||||
only the arbiter's own unit tests -- worth a follow-up pass before
|
only the arbiter's own unit tests -- worth a follow-up pass before
|
||||||
calling it fully closed.
|
calling it fully closed.
|
||||||
|
- [x] **Composing text held back until a space, caret not moving, fixed
|
||||||
|
2026-09-06.** `InputMethodManager.updateSelection` was never called --
|
||||||
|
see IRIS.md's 2026-09-06 entry and RUST.md's P0 box, item 1, for the
|
||||||
|
full account and the emulator evidence.
|
||||||
|
- [x] **Swipe over the composer summons the keyboard, fixed 2026-09-06.**
|
||||||
|
`Selector`/`Selectable` now wait for a completed tap -- see IRIS.md's
|
||||||
|
2026-09-06 entry and RUST.md's P0 box, item 5. Verified via `dumpsys
|
||||||
|
input_method`'s `mInputShown` on the emulator, not yet on the phone.
|
||||||
|
- [x] **Text disappears again after leaving and returning to the app,
|
||||||
|
fixed 2026-09-06.** `GlyphAtlas::clear`/`Textures::reset` on a
|
||||||
|
genuinely new renderer -- see IRIS.md's 2026-09-06 entry and RUST.md's
|
||||||
|
P0 box, item 4. Verified on the emulator (home, reopen, screenshot);
|
||||||
|
not yet on the phone.
|
||||||
|
- [ ] **Composed/typed text never becomes visible at all -- found
|
||||||
|
2026-09-06, not fixed.** The composer bar stays empty even once the
|
||||||
|
buffer genuinely holds the typed text (confirmed indirectly: Gboard's
|
||||||
|
own suggestion strip reacts correctly to each keystroke). A new unit
|
||||||
|
test proves the widget tree's own layout math resolves the field's
|
||||||
|
region correctly across a keyboard resize, so the bug is downstream of
|
||||||
|
that -- most likely `UiRenderState::redraw`'s single-widget redraw path,
|
||||||
|
or specific to this emulator's forced `force-gles` backend (untested on
|
||||||
|
Vulkan or the real phone). RUST.md's P0 box, item 2, has the full
|
||||||
|
writeup, what was ruled out, and where to look next. **Also unverified
|
||||||
|
because of this**: item 3's composer rebuild (one `Stack`-based widget,
|
||||||
|
a capped/scrollable height, bottom padding tied to the IME/nav-bar
|
||||||
|
inset) -- structurally in place and unit-tested, but its own visual
|
||||||
|
correctness cannot be screenshotted until text actually renders.
|
||||||
|
- [ ] **The composer has no touch-drag scroll for overflowing text.** The
|
||||||
|
2026-09-06 rebuild caps the field at ~6 lines and wraps it in
|
||||||
|
`.scrollable()` for a wheel/trackpad scroll, but a real finger drag over
|
||||||
|
text that has overflowed the cap does not scroll it -- `Scroll`'s touch
|
||||||
|
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."** Not closeable from the emulator, annotated
|
||||||
|
2026-09-06 after the `DragGesture` merge. `attr.rs`'s `on_press` never
|
||||||
|
calls `capture_pointer` and never consumes a `Pressing` frame past
|
||||||
|
`DRAG_SLOP` (it just stops watching), so once the finger's *current*
|
||||||
|
position leaves the composer's box and enters the list's, `List`
|
||||||
|
starts receiving ordinary hit-tested `Pressing` frames there --
|
||||||
|
`DragArbiter::is_idle()`'s 2026-09-05 recovery (a missed `PressStart`)
|
||||||
|
picks it up rather than leaving it stuck. What this does **not** do is
|
||||||
|
what "wherever it began" implies literally: `DragArbiter::press_start`
|
||||||
|
restarts from the *boundary-crossing* position, not from the original
|
||||||
|
touch-down inside the composer, so the pan still needs a fresh
|
||||||
|
`DRAG_SLOP` of travel measured from the boundary rather than from the
|
||||||
|
start of the gesture -- composer and list are adjacent, non-overlapping
|
||||||
|
widgets (`lib.rs`'s `(list, composer_bar).span(Dir::DOWN)`), and only
|
||||||
|
the composer forwarding its own drag to the list would remove that
|
||||||
|
residual slop entirely, which is more than this pass's merge changes.
|
||||||
|
RUST.md's merge-pass box has the reasoning in full and an emulator
|
||||||
|
swipe confirming the composer's own box never moves/resizes during it;
|
||||||
|
whether the residual slop is still perceptible as "caught" needs Iris's
|
||||||
|
phone, since the emulator's per-widget boundary is a few dp wide and
|
||||||
|
easy to cross without noticing on a real screen too.
|
||||||
|
- [ ] **"Flinging still does not work."** No longer expected to reproduce
|
||||||
|
after the `DragGesture` merge (`e12c708`, pointer capture +
|
||||||
|
`CursorSense::Drop`), 2026-09-06. Emulator evidence (RUST.md's
|
||||||
|
merge-pass box, check (b)): a real `ui-trace` finger swipe followed by
|
||||||
|
screenshot-hash sampling caught a post-release frame distinct from the
|
||||||
|
drag's own last frame in one run, and every run showed 28-32
|
||||||
|
`render()` frames per gesture against an idle baseline of 0 and ~8
|
||||||
|
expected from the drag alone -- redraw kept being requested well past
|
||||||
|
the finger lifting, which only happens while a fling is still
|
||||||
|
animating. Left unticked in spirit until Iris's phone confirms it,
|
||||||
|
since only she can say whether it *feels* like a fling now; the
|
||||||
|
emulator's screenshot timing could not always catch the tail of a
|
||||||
|
fast-settling one visually (same caveat noted in RUST.md).
|
||||||
|
- [ ] **"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
|
## Build
|
||||||
|
|
||||||
@@ -450,3 +548,21 @@ do not duplicate it there.
|
|||||||
and control sizes; the emulator at two densities and the phone draw the
|
and control sizes; the emulator at two densities and the phone draw the
|
||||||
same layout at the same physical size. After the bench setup is
|
same layout at the same physical size. After the bench setup is
|
||||||
finished, before P1 draws any new screen.
|
finished, before P1 draws any new screen.
|
||||||
|
|
||||||
|
## From the phone, bench v2 (2026-09-06): streaming re-lays out the whole message
|
||||||
|
|
||||||
|
- [ ] **Streaming a delta into a long message costs a full text layout of
|
||||||
|
that message.** Iris's phone report (`docs/bench/iris-phone-v2-2026-09-06.md`):
|
||||||
|
the stream phase is the one place iris is behind Compose (p50 18.2 ms vs
|
||||||
|
13.4 ms; p99 level at ~43 ms). `TranscriptScreen::apply` replaces only
|
||||||
|
the last row, but that row is the growing message, and replacing it
|
||||||
|
re-renders its markdown and re-shapes the entire paragraph run through
|
||||||
|
parley on every event. Compose pays a reparse (8.6 ms mean) for the
|
||||||
|
same event. What "done" looks like: a streamed delta re-lays out only
|
||||||
|
the block it lands in (the last paragraph or code block), with earlier
|
||||||
|
blocks' layouts kept -- which needs a row to be a column of per-block
|
||||||
|
`Text`s rather than one `TextEdit` for the whole message, or parley's
|
||||||
|
layout to be split at block boundaries; measured by the stream phase's
|
||||||
|
p50 dropping below Compose's on the phone. Do this after the four bench
|
||||||
|
v2 defects (stale primitives, finger fling, decay curve, IME show) are
|
||||||
|
closed, since they are what make the run unrepresentative today.
|
||||||
@@ -0,0 +1,214 @@
|
|||||||
|
# Review: iris changes since 0e46293
|
||||||
|
|
||||||
|
Scope: `git diff 0e46293..HEAD -- iris/ client-core/` (58 files, +5224/-226).
|
||||||
|
Read-only review; no source changed. Ordered likely-bug, then invariant
|
||||||
|
guards, then rules, then tests/docs.
|
||||||
|
|
||||||
|
## Likely bugs
|
||||||
|
|
||||||
|
1. **`iris/transcript-ui/src/lib.rs:152-160` (`RowDiff::Rebuild` arm of
|
||||||
|
`TranscriptScreen::apply`) never unregisters the rows it drops from
|
||||||
|
`Selection`, so a stale `WeakWidget<TextEdit>` outlives the widget it
|
||||||
|
points to and the next touch on *any* row panics.**
|
||||||
|
`Selection::rows: BTreeMap<RowKey, WeakWidget<TextEdit>>` documents its
|
||||||
|
own contract at `selection.rs:69-71`: "every addition here needs its
|
||||||
|
removal ... called when `List` evicts the row." The `ReplaceLast` arm
|
||||||
|
above it honours this (`lib.rs:143-145`, `self.selection.borrow_mut()
|
||||||
|
.unregister(old_key)` when the key changes). The `Rebuild` arm calls
|
||||||
|
`(self.list)(rsc).clear()` and rebuilds every row from `new_rows`, but
|
||||||
|
never touches `self.selection` — any key present in `old_rows` and
|
||||||
|
*absent* from `new_rows` (exactly what `group_tool_runs` regrouping two
|
||||||
|
separate tool-call rows into one produces — see `diff_tests::
|
||||||
|
a_tool_run_closing_and_joining_an_earlier_call_is_a_regroup_fallback`,
|
||||||
|
which tests the diff decision but not `apply` itself) is left in
|
||||||
|
`self.rows` pointing at a widget `List::clear()` just freed.
|
||||||
|
`TextEditable::edit` (`iris/src/widget/text/edit.rs:582-587`) resolves
|
||||||
|
that handle with `ui.widgets.get_mut(self).unwrap()` — an unconditional
|
||||||
|
panic on the freed slot. `Selection::begin` (`selection.rs:88-101`)
|
||||||
|
iterates *every* registered row (`w.edit(ui).deselect()`) on an
|
||||||
|
ordinary fresh press, so the crash fires on the next tap anywhere in
|
||||||
|
the transcript after a regroup, not only on a tap targeting the
|
||||||
|
orphaned row.
|
||||||
|
Fix: give `Selection` a way to reconcile against the row set that
|
||||||
|
survived a rebuild (e.g. `Selection::retain(&self, keys: &BTreeSet<RowKey>)`
|
||||||
|
removing everything else, called from the `Rebuild` arm before
|
||||||
|
rebuilding), or simplest — call `self.selection.borrow_mut()` cleared
|
||||||
|
the same way `List::clear()` clears the list, then let the rebuild's
|
||||||
|
`push_row` calls re-`register` everything as they already do.
|
||||||
|
|
||||||
|
## Guarded invariants missing
|
||||||
|
|
||||||
|
2. **`iris/src/widget/list.rs:751` (`List::place`) indexes/expects on
|
||||||
|
`slot` with no assertion that it exists.** `slot_widget` (`:563-575`)
|
||||||
|
panics via `.expect(...)` for a sentinel with no widget set, and does
|
||||||
|
an unchecked `&self.items[s as usize]` for a real index — a bare
|
||||||
|
"index out of bounds" with no context if `place` is ever reached with a
|
||||||
|
stale slot. Every current caller happens to derive `slot` from
|
||||||
|
`repair_anchor`/`prev_slot`/`next_slot`, which already check existence,
|
||||||
|
but that invariant is enforced by convention across three call sites,
|
||||||
|
not by the function that depends on it. Add
|
||||||
|
`debug_assert!(self.slot_exists(slot), "place() called with a slot that doesn't exist: {slot:?}");`
|
||||||
|
at the top of `place`.
|
||||||
|
3. **`iris/src/widget/list.rs:426` (`List::fling`) and `sense.rs`'s
|
||||||
|
`FlingCalculator::distance`/`duration`/`position_at` never check that
|
||||||
|
the incoming velocity is finite.** A `NaN`/`inf` velocity (a
|
||||||
|
`VelocityTracker::velocity()` divide-by-near-zero span, or a caller
|
||||||
|
passing a raw device value straight through) propagates through
|
||||||
|
`deceleration_for`'s `.ln()` silently — the fling either never settles
|
||||||
|
(`settled_on_schedule` compares against a `NaN` `duration()`, which is
|
||||||
|
always `false`) or jumps to `NaN` positions with nothing on screen
|
||||||
|
saying why. Add `debug_assert!(velocity_px_per_s.is_finite())` in
|
||||||
|
`List::fling` and `FlingCalculator::new`/`distance`.
|
||||||
|
4. **`iris/src/sense.rs:592-604` (`VelocityTracker::velocity`) has no
|
||||||
|
assertion that samples are chronological.** `add_sample` trusts its
|
||||||
|
caller's `Instant` ordering; a caller that samples out of order (a
|
||||||
|
restored/replayed gesture, a test) would silently produce a negative
|
||||||
|
`span` handled only by the `span <= 0.0 => 0.0` catch-all, masking the
|
||||||
|
bug that produced it rather than surfacing it. Add
|
||||||
|
`debug_assert!(self.samples.back().is_none_or(|&(last, _)| at >= last))`
|
||||||
|
in `add_sample`.
|
||||||
|
5. **`iris/core/src/render/frame_report.rs:247-252` (`mark_phase`) has no
|
||||||
|
assertion that phases are pushed in non-decreasing `start_index`
|
||||||
|
order.** `phase_stats`'s slicing (`:274`, `idx >= phase.start_index &&
|
||||||
|
idx < end_index`) silently produces an empty or nonsensical slice for
|
||||||
|
an out-of-order phase rather than surfacing the misuse — cheap to add
|
||||||
|
given `self.phases.last()` is already in scope:
|
||||||
|
`debug_assert!(self.phases.last().is_none_or(|p| self.total_frames >= p.start_index));`
|
||||||
|
|
||||||
|
## Rules
|
||||||
|
|
||||||
|
6. **Two mechanisms answer "what row selection points at, still valid?"**
|
||||||
|
`Selection` relies on callers remembering to `unregister` (finding 1);
|
||||||
|
`List` relies on callers deriving slots only from already-checked
|
||||||
|
sources (finding 2). Both are the same class of problem — a derived
|
||||||
|
handle that silently outlives what it points to — solved ad hoc twice
|
||||||
|
rather than once. Not asking for a shared abstraction here, but the two
|
||||||
|
should at minimum cross-reference each other's doc comment so the next
|
||||||
|
caller who adds a third handle-into-`List`-rows type (the code rules'
|
||||||
|
"a rule that governs a set belongs to the set") finds both existing
|
||||||
|
examples.
|
||||||
|
7. **`iris/android-app/src/bench_client.rs:224-225` (`battery_line`)
|
||||||
|
calls `.min().unwrap()`/`.max().unwrap()` on `samples` guarded three
|
||||||
|
lines above by `if samples.is_empty()`, which is fine — but the guard
|
||||||
|
and the two unwraps are two statements apart with a `let mean = ...`
|
||||||
|
in between reading the same slice; a future edit reordering those
|
||||||
|
lines loses the guard's protection silently.** Low severity (this is
|
||||||
|
the bench tool, not the app), but worth a one-line comment tying the
|
||||||
|
unwraps back to the guard, or restructuring as
|
||||||
|
`let (Some(min), Some(max)) = (samples.iter().min(), samples.iter().max())`
|
||||||
|
pattern so the empty case can't be separated from the check by a future
|
||||||
|
edit.
|
||||||
|
|
||||||
|
## Tests
|
||||||
|
|
||||||
|
8. **No test exercises `TranscriptScreen::apply`'s `Rebuild` arm through
|
||||||
|
`Selection`.** `lib.rs`'s `diff_tests` module (`:284-379`) tests only
|
||||||
|
the pure `diff_rows` decision function, never `apply` itself wired to a
|
||||||
|
real `Selection`; `selection.rs`'s own tests (`a_missed_press_start_
|
||||||
|
recovers_on_the_next_pressing_frame`, `unregister_forgets_the_row_and_
|
||||||
|
clears_a_matching_anchor`) never go through `apply`/`List::clear`
|
||||||
|
either. This is exactly the gap that let finding 1 through: the two
|
||||||
|
pieces (`apply`'s fallback, `Selection`'s registration contract) are
|
||||||
|
each tested in isolation and never together. Add: build a
|
||||||
|
`TranscriptScreen`, force a `RowDiff::Rebuild` (two adjacent tool-call
|
||||||
|
rows regrouping, per the existing `diff_tests` case), then call
|
||||||
|
`selected_text`/simulate a fresh press on a surviving row and assert no
|
||||||
|
panic.
|
||||||
|
9. **`iris/src/widget/list.rs`'s fling tests check total distance and the
|
||||||
|
start/end clamp but not the speed profile in between.**
|
||||||
|
`fling_moves_the_list_and_then_settles`/`fling_distance_is_positive_
|
||||||
|
toward_the_end` only assert the fling started, moved in the right
|
||||||
|
direction, and eventually stopped — none checks that
|
||||||
|
`tick_fling`'s per-tick delta is *monotonically decreasing* once past
|
||||||
|
the fling's peak (the property `fling_calculator_tests::position_at_
|
||||||
|
is_monotonic_and_clamped_past_the_end` already checks one level down,
|
||||||
|
for `FlingCalculator` alone, but never through `List::tick_fling`'s own
|
||||||
|
`scroll`/`anchor.offset` accumulation). A regression that made
|
||||||
|
`tick_fling` apply the *total* distance every tick instead of the
|
||||||
|
incremental one, for instance, would still pass both existing tests
|
||||||
|
(final position and direction are unaffected by how the interior ticks
|
||||||
|
split it up) while being wildly wrong every intermediate frame.
|
||||||
|
10. **`iris/src/widget/list.rs::replacing_the_last_row_stays_pinned_to_
|
||||||
|
the_bottom` and its sibling test `replace_back`'s effect on the
|
||||||
|
displayed row, never that the row it evicted is actually gone from
|
||||||
|
`heights`/`extents`.** Both tests assert the *new* row's position;
|
||||||
|
neither asserts `old.key` is absent from `list_ref.heights`/`extents`
|
||||||
|
after the replace (the "stale primitive" class finding 1 is a
|
||||||
|
production instance of). A cheap addition: assert
|
||||||
|
`!list_ref.heights.contains_key(&old.key)` after `replace_back` in the
|
||||||
|
existing test, since `old.key` is already returned to the test as
|
||||||
|
`evicted`... (`lib.rs` calls it that way; the `list.rs` test would need
|
||||||
|
to capture the key from `old` similarly.)
|
||||||
|
|
||||||
|
## Docs
|
||||||
|
|
||||||
|
No missing `IRIS.md` entry found for a *public* API change in this diff —
|
||||||
|
`List::fling`/`VelocityTracker`/`FlingCalculator`, `List::
|
||||||
|
anchor_position_display`, `FrameReport::mark_phase`/`phase_stats`/
|
||||||
|
`late_at_hz`, `UiRenderNode::new`'s `Result` change, `Len::dp`, and
|
||||||
|
`List::replace_back`/`clear`/`TranscriptScreen::apply` all have entries.
|
||||||
|
The `List::replace_back`/`clear`/`TranscriptScreen::apply` entry
|
||||||
|
(`docs/IRIS.md:526`) predates this review's finding 1 and does not mention
|
||||||
|
`Selection`'s registration contract at all — once finding 1 is fixed,
|
||||||
|
that entry should gain a line noting what the fix requires of a caller
|
||||||
|
that keeps its own row-keyed side table (the same shape `Selection` is),
|
||||||
|
so the next such table doesn't reproduce the same gap.
|
||||||
|
|
||||||
|
## Fixed, 2026-09-06
|
||||||
|
|
||||||
|
All ten findings addressed after the `DragGesture` merge (`selection.rs`
|
||||||
|
was rewritten by that merge, but finding 1's shape and location were
|
||||||
|
unchanged — `TranscriptScreen::apply`'s `Rebuild` arm, `iris/transcript-ui/
|
||||||
|
src/lib.rs`).
|
||||||
|
|
||||||
|
1. **Fixed.** `Selection::clear()` (`selection.rs`) drops `rows` and
|
||||||
|
`anchor`, called from `apply`'s `Rebuild` arm right before
|
||||||
|
`List::clear()` — `push_row` re-`register`s whatever survives as it
|
||||||
|
rebuilds each row, the "simplest" fix option the finding named.
|
||||||
|
2. **Fixed.** `debug_assert!(self.slot_exists(slot), ...)` at the top of
|
||||||
|
`List::place` (`iris/src/widget/list.rs`).
|
||||||
|
3. **Fixed.** `debug_assert!(velocity_px_per_s.is_finite())` in
|
||||||
|
`List::fling`, and `debug_assert!(velocity.is_finite())` in
|
||||||
|
`FlingCalculator::distance`/`duration` (`iris/src/sense.rs`).
|
||||||
|
`position_at` calls both, so it inherits the guard rather than needing
|
||||||
|
its own.
|
||||||
|
4. **Fixed.** `debug_assert!` on chronological sample order in
|
||||||
|
`VelocityTracker::add_sample` (`iris/src/sense.rs`).
|
||||||
|
5. **Fixed.** `debug_assert!` on non-decreasing `start_index` in
|
||||||
|
`FrameReport::mark_phase` (`iris/core/src/render/frame_report.rs`).
|
||||||
|
6. **Fixed (doc cross-reference only, as asked).** `Selection::register`'s
|
||||||
|
doc now points at `List::place`'s `slot_exists` assertion and vice
|
||||||
|
versa isn't needed since finding 2's fix already cites this file in
|
||||||
|
its own comment; both are grep-able on "docs/REVIEW-2026-09-06.md" and
|
||||||
|
on each other's type names.
|
||||||
|
7. **Fixed.** `bench_client.rs::battery_line` restructured to
|
||||||
|
`let (Some(min), Some(max)) = (samples.iter().min(), samples.iter().max())`,
|
||||||
|
so the empty-guard and the two lookups can no longer be separated by a
|
||||||
|
future edit.
|
||||||
|
8. **Fixed.** `transcript-ui`'s new `apply_tests::
|
||||||
|
a_row_dropped_by_a_regroup_does_not_outlive_itself_in_selection`
|
||||||
|
(`lib.rs`) builds a real `TranscriptScreen`, forces the same regroup
|
||||||
|
shape `diff_tests` already covers at the pure-diff level, calls `apply`,
|
||||||
|
and then `Selection::begin` on a surviving row — which panicked before
|
||||||
|
fix 1, resolving a `WeakWidget` `List::clear()` had just freed.
|
||||||
|
9. **Fixed.** `list.rs`'s new `tick_fling_applies_shrinking_incremental_
|
||||||
|
deltas` flings toward the end from `jump_to_start` and asserts each
|
||||||
|
tick's `extents[&0]` delta is no larger than the previous one — would
|
||||||
|
fail against a `tick_fling` that applied the total spline distance
|
||||||
|
every tick instead of the incremental slice, which the two pre-existing
|
||||||
|
fling tests cannot catch.
|
||||||
|
10. **Fixed.** `list.rs`'s new `replace_back_forgets_the_evicted_keys_own_
|
||||||
|
height` replaces row 4 with a row keyed `100` (the two existing
|
||||||
|
`replace_back` tests always reuse the same key, so neither actually
|
||||||
|
exercises the removal) and asserts `heights` no longer contains the
|
||||||
|
evicted key.
|
||||||
|
|
||||||
|
Docs: `docs/IRIS.md`'s 2026-09-05 `List::replace_back`/`clear`/
|
||||||
|
`TranscriptScreen::apply` entry now has a line on what the fix requires of
|
||||||
|
a caller with its own row-keyed side table, naming `Selection` as the
|
||||||
|
example and dating the fix.
|
||||||
|
|
||||||
|
Verification run alongside the rest of this pass's checks: `cargo fmt
|
||||||
|
--all`, `cargo clippy --workspace --all-targets`, `cargo test --workspace`
|
||||||
|
from `iris/` — see docs/RUST.md's plan box for the pass/fail and any
|
||||||
|
caveats from this same session.
|
||||||
+335
@@ -34,6 +34,179 @@ the emulator, not by Mesa" and "the present mode was not the cause" are
|
|||||||
worth as much as the successes, because they are what stops the next
|
worth as much as the successes, because they are what stops the next
|
||||||
session spending an afternoon on them again.
|
session spending an afternoon on them again.
|
||||||
|
|
||||||
|
## Where things stand (2026-09-06, orchestrator plan)
|
||||||
|
|
||||||
|
Written by the design agent on picking the branch up after a `/clear`, so
|
||||||
|
the next session can resume from here. P0 is delivered and Iris's phone
|
||||||
|
report v2 is in (`docs/bench/iris-phone-v2-2026-09-06.md`); **P1 stays
|
||||||
|
gated on her verdict**, so this pass works the P0 defects and the pure
|
||||||
|
prerequisites in this order. Each item is ticked here by the agent that
|
||||||
|
closes it.
|
||||||
|
|
||||||
|
- [x] **Merge the `DragGesture` work** -- done 2026-09-06 (merge commit
|
||||||
|
`f802de9`, `git merge --no-ff worktree-agent-a754368325fa06839`,
|
||||||
|
clean, no conflicts across the 8 files `e12c708` touched). Targets
|
||||||
|
two of the four bench-v2 defects: finger flings dropped by
|
||||||
|
per-widget hit testing (pointer capture + `CursorSense::Drop`), and
|
||||||
|
IME insets never redelivered (`MainActivity.java` edge-to-edge).
|
||||||
|
**Tap-vs-swipe/`DragGesture` overlap, reasoned through**: `attr.rs`'s
|
||||||
|
`on_press` (composer focus) and `sense.rs`'s `DragArbiter`/
|
||||||
|
`DragGesture` (list pan-vs-select) do not share a mechanism, but
|
||||||
|
they don't need to -- `on_press` never calls `capture_pointer`, so
|
||||||
|
it only ever sees an ordinary per-frame hit-tested `Pressing`/
|
||||||
|
`PressEnd` (`run_sensors`' `region.contains(cursor.pos)` check,
|
||||||
|
unaffected by capture unless *this* widget requested it), the same
|
||||||
|
as before `DragGesture` existed. The two only interact where a
|
||||||
|
gesture starts on the composer and travels into the list's region;
|
||||||
|
`run_sensors` already delivers `Pressing` to whichever widget's
|
||||||
|
*current* position contains the pointer, so `List` starts getting
|
||||||
|
frames the instant the finger crosses the boundary -- with no
|
||||||
|
`PressStart` of its own, which is exactly what `DragArbiter::
|
||||||
|
is_idle()`'s 2026-09-05 recovery branch exists for. No consolidation
|
||||||
|
needed; `DRAG_SLOP` is already the one shared constant (`attr.rs`
|
||||||
|
imports it from `sense.rs`, not a second copy).
|
||||||
|
**Checks, 2026-09-06 merge pass**: `cargo fmt --all` clean;
|
||||||
|
`cargo clippy -p iris -p iris-core -p transcript-ui --all-targets`
|
||||||
|
and the same for `-p desktop-app -p tabs-ui`, zero warnings beyond
|
||||||
|
the pre-existing external-crate future-incompat notice
|
||||||
|
(naga/wgpu/wgpu-core/wgpu-hal/winit); `cargo test --lib -p iris -p
|
||||||
|
iris-core -p transcript-ui` and `-p desktop-app -p tabs-ui`, 97
|
||||||
|
passed/0 failed, including the review-fix tests below.
|
||||||
|
`cargo test --workspace`/`cargo clippy --workspace --all-targets`
|
||||||
|
(the full-workspace forms, which also build `iris`'s winit examples)
|
||||||
|
were abandoned after 40+ minutes each stuck compiling one example
|
||||||
|
binary with `uptime` reading a load average of 66-78 on this 8-core
|
||||||
|
VM (3-4 concurrent peer `cargo`/`cargo check` invocations the whole
|
||||||
|
session) -- `ps -o time` on the stuck `rustc` showed 2 seconds of
|
||||||
|
accumulated CPU time after 38 minutes of wall time, confirming
|
||||||
|
scheduler starvation rather than a hang. The per-package `--lib`
|
||||||
|
form above is what actually exercises the changed code and finished
|
||||||
|
in under 4 minutes warm. `android-app` (`iris-android-app`) is
|
||||||
|
excluded from the host workspace (`iris/Cargo.toml`, needs the NDK
|
||||||
|
target) and is covered instead by the APK build below, which
|
||||||
|
compiles it for `x86_64-linux-android`.
|
||||||
|
|
||||||
|
**Emulator checks, 2026-09-06** (this checkout's `ai-app-2` AVD,
|
||||||
|
`iris/android-app/build-apk.sh debug --abi x86_64 --features
|
||||||
|
"transcript-screen bench force-gles"` -- plain Vulkan crashed on
|
||||||
|
this AVD's boot this pass, `wgpu_core::instance: enabled backend
|
||||||
|
Vulkan has no adapters`, unrelated to this merge and worked around
|
||||||
|
with `force-gles` the way I5's own box already documents for this
|
||||||
|
hardware):
|
||||||
|
- **(a) tap-vs-swipe still holds.** Fresh app launch, `dumpsys
|
||||||
|
input_method`'s `mInputShown=false` at rest. `ui-trace record
|
||||||
|
--do "swipe 540 1510 540 700 200"` (a swipe starting on the
|
||||||
|
composer's own box, read from `ui-trace show -m Message --field
|
||||||
|
box` as `31,1488..1048,1540`) leaves `mInputShown=false` and the
|
||||||
|
box unmoved (no keyboard-driven resize). `ui-trace record --do
|
||||||
|
"tap 540 1510"` on the same field then reads `mInputShown=true`.
|
||||||
|
Matches `20b1225`'s original result -- the `DragGesture` merge
|
||||||
|
did not disturb it, confirming the reasoning above.
|
||||||
|
- **(b) a real finger fling keeps the list moving after release.**
|
||||||
|
Screenshot-hash sampling (`adb exec-out screencap`, `md5`, since
|
||||||
|
transcript rows carry no per-row accessibility label yet -- I5's
|
||||||
|
own leftover -- so `ui-trace show` cannot track them) at ~40-60ms
|
||||||
|
intervals through and after a fast `swipe 540 1400 540 400 120`
|
||||||
|
(with room to scroll confirmed by a preceding slow drag) caught
|
||||||
|
two *distinct* post-release frames in one run (a settle-position
|
||||||
|
beyond the raw drag's own last frame), and every run showed
|
||||||
|
28-32 `iris::android::view: render()` log lines per gesture
|
||||||
|
against an idle baseline of 0 in 1.5s and roughly 8 expected from
|
||||||
|
a bare 120ms drag's own `Pressing` frames alone -- i.e. redraw
|
||||||
|
kept being requested well past the finger lifting, which only
|
||||||
|
happens while `List::tick_fling` is still returning `true`.
|
||||||
|
Some runs' screenshots showed only the drag's own jump with nothing
|
||||||
|
further *visibly different*, which is consistent with a real but
|
||||||
|
small/fast-settling fling (a modest synthetic-touch velocity's
|
||||||
|
spline tail moves little per frame) rather than absence of one --
|
||||||
|
the render-count signal did not vary between those runs and the
|
||||||
|
one with a visible second frame. Recorded as confirmed, with that
|
||||||
|
caveat, rather than measured to a number; a phone verification
|
||||||
|
(Iris's own report closes this properly) is still open per
|
||||||
|
`IRIS_TODO.md`'s item.
|
||||||
|
- **(c) `on_insets_changed` fires on an IME toggle, with confirmed
|
||||||
|
cycles.** `run-bench.sh`'s report: `keyboard: shown 4/5, hidden
|
||||||
|
5/5 (confirmed via on_insets_changed)` -- the "could not be
|
||||||
|
shown" unknown-state line (`bench_client.rs::run_keyboard_phase`)
|
||||||
|
did not fire, unlike the pre-`DragGesture` build this same report
|
||||||
|
format existed for.
|
||||||
|
Worktrees removed after the checks above: `agent-a754368325fa06839`
|
||||||
|
(the source branch, its own emulator stopped first via `cd` into
|
||||||
|
it + `emu down`), `agent-a27094a7db775552a`, `agent-a1ff0294b6c29127e`,
|
||||||
|
`agent-a9002910a315fe719` -- each confirmed `git rev-list --count
|
||||||
|
rustify..<branch>` = 0 and no uncommitted changes first; their
|
||||||
|
branches deleted too. `agent-a16b22e34539b810e` and
|
||||||
|
`agent-a6e37a2335f436d08` left alone -- both `git worktree list`
|
||||||
|
`locked` to a live peer agent.
|
||||||
|
- [x] **Fix `docs/REVIEW-2026-09-06.md`**, done 2026-09-06, after the
|
||||||
|
merge (finding 1's shape and location in `selection.rs`/`lib.rs`
|
||||||
|
were unchanged by the merge, which touched `Selection` but not
|
||||||
|
`apply`'s `Rebuild` arm). All ten findings fixed -- new
|
||||||
|
`Selection::clear()` for finding 1 (the simplest option the review
|
||||||
|
named: clear the same way `List::clear()` clears the list, let
|
||||||
|
`push_row` re-`register` survivors), five `debug_assert!`s
|
||||||
|
(2-5, plus 7's restructure), and three new tests (8, 9, 10),
|
||||||
|
confirmed with the `apply_tests::a_row_dropped_by_a_regroup_does_
|
||||||
|
not_outlive_itself_in_selection` test passing (it exercises exactly
|
||||||
|
finding 1's shape: build a real `TranscriptScreen`, force the same
|
||||||
|
regroup `diff_tests` already covers, `apply`, then a surviving
|
||||||
|
row's `begin` -- panics pre-fix, per the review's own test-8 ask).
|
||||||
|
`docs/IRIS.md`'s 2026-09-05 entry gained the line the review's
|
||||||
|
"Docs" section asked for. See `docs/REVIEW-2026-09-06.md`'s own "Fixed, 2026-09-06"
|
||||||
|
section for the per-finding account. Committed together with the
|
||||||
|
review file.
|
||||||
|
- [ ] **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
|
||||||
|
three sit on the `redraw_updates` targeted-redraw path and may be
|
||||||
|
one bug; the P0 box says the next step is instrumentation inside
|
||||||
|
`Span::draw`/`draw_inner` showing where each placement's
|
||||||
|
primitives actually land on the frame it goes wrong.
|
||||||
|
`list.rs`'s new `replacing_the_last_row_many_times_does_not_leak_
|
||||||
|
primitives` test already pins the widget arena as *not* the leak.
|
||||||
|
- [ ] **Composer touch-drag scroll** for overflowed text — now that
|
||||||
|
dragging is a default-input `DragGesture`, `Scroll` should get its
|
||||||
|
touch pan from the same mechanism `List` uses, not a copy.
|
||||||
|
- [ ] **Streaming re-layout** (IRIS_TODO.md's last section) — after the
|
||||||
|
above, since they make the stream phase unrepresentative today.
|
||||||
|
- [x] **client-core prerequisites for P1, in parallel** (pure Rust,
|
||||||
|
disjoint from `iris/`), closed 2026-09-06: `TranscriptSource`'s
|
||||||
|
cache-vs-server stitching (new `client-core/src/transcript_source.rs`)
|
||||||
|
and `joinPages`/`healSplitMessage`/`adoptRun` page-boundary healing
|
||||||
|
(new functions in `transcript_fold.rs`), per `CLIENT_CORE.md`. Ported
|
||||||
|
against the Kotlin source and AGENTS.md's paging incidents as the
|
||||||
|
spec (`TranscriptSource.kt`/`TranscriptItems.kt` had no JVM unit
|
||||||
|
tests of their own to port test-for-test). `client-core` goes from
|
||||||
|
85 to 109 tests; `cargo test`/`clippy --all-targets`/`fmt` all clean.
|
||||||
|
Both AGENTS.md regressions have a dedicated test: `loadOlderPage`'s
|
||||||
|
`before == 0` guard moved into `TranscriptSource::page` itself
|
||||||
|
(`paging_before_the_first_event_makes_no_request_at_all` asserts
|
||||||
|
zero transport calls, not just an empty result), and
|
||||||
|
`a_clean_boundary_between_two_finished_runs_is_still_healed_into_one_run`
|
||||||
|
pins `adopt_run` running on *every* join rather than only the
|
||||||
|
split-call path. One incidental fix needed to port `TranscriptSource`
|
||||||
|
faithfully: `api.rs` gained `fetch_transcript_lines` (additive, the
|
||||||
|
existing `fetch_transcript_page` untouched since `iris/` depends on
|
||||||
|
its signature), which pairs each event with the exact server bytes
|
||||||
|
it came from via `serde_json::value::RawValue` rather than
|
||||||
|
re-serializing a parsed `Value` -- needed so the cache and a live SSE
|
||||||
|
frame agree byte-for-byte, the same class of bug as the
|
||||||
|
`float_roundtrip` fix. Deliberately not ported: `EventStream.kt`'s
|
||||||
|
reconnect/backoff and cross-thread stream cancellation, which are
|
||||||
|
runtime policy for whichever framework embeds this crate, not pure
|
||||||
|
logic -- see `CLIENT_CORE.md`'s new section for the full account.
|
||||||
|
- **Then**: redeliver `~/host/bench/iris-bench-arm64.apk` for Iris with
|
||||||
|
its README saying what changed, and record any choice she should see in
|
||||||
|
`DECISIONS.md`.
|
||||||
|
|
||||||
## Where things stand (2026-09-05)
|
## Where things stand (2026-09-05)
|
||||||
|
|
||||||
- **Streaming no longer costs a full rebuild** (P0's box, "Streaming no
|
- **Streaming no longer costs a full rebuild** (P0's box, "Streaming no
|
||||||
@@ -4750,6 +4923,168 @@ device.
|
|||||||
given the risk-to-time-remaining ratio. Both open items are
|
given the risk-to-time-remaining ratio. Both open items are
|
||||||
recorded in `~/repos/ai-app-bench`'s README with today's date.
|
recorded in `~/repos/ai-app-bench`'s README with today's date.
|
||||||
|
|
||||||
|
**Composing text, the tap-vs-swipe focus rule, and app-switch text
|
||||||
|
loss, 2026-09-06.** Iris's report on this same dc01f88 build: typing
|
||||||
|
doesn't enter text or move the caret until a space is hit; typed
|
||||||
|
text doesn't visibly appear and there is empty black space below the
|
||||||
|
composer bar; text disappears again after leaving and returning to
|
||||||
|
the app; and (a follow-up message the same day) swiping over the
|
||||||
|
composer bar wrongly summons the keyboard.
|
||||||
|
|
||||||
|
1. **The caret/composing bug's cause**: `android/ime.rs`'s
|
||||||
|
`InputConnection` never called `InputMethodManager.updateSelection`
|
||||||
|
after an edit -- confirmed by reading android-view's own demo
|
||||||
|
(`~/src/android-view/demo/src/lib.rs`'s `render()`), which calls it
|
||||||
|
every time its editor's generation changes. Without it, Gboard has
|
||||||
|
no confirmation the app is keeping up and holds keystrokes back
|
||||||
|
rather than trusting a screen it believes is stale -- exactly
|
||||||
|
"doesn't enter it until I hit space." **Fix**: `IrisViewPeer::
|
||||||
|
update_ime_selection` (new, `ime.rs`) reports the real selection
|
||||||
|
and (an approximation, `compose_len` chars back from the caret)
|
||||||
|
the composing region, called from `after_input`'s existing tail so
|
||||||
|
every touch/key/IME callback already runs it. The buffer-level
|
||||||
|
half (`replace`/`insert_str` correctly advancing the caret) was
|
||||||
|
already correct and is now covered by four new unit tests in
|
||||||
|
`iris/src/widget/text/edit.rs` (composing, `commitText`,
|
||||||
|
`deleteSurroundingText`, `setSelection`). **Verified**: on the
|
||||||
|
emulator (`force-gles`, no Vulkan adapter on this AVD), tapping a
|
||||||
|
real Gboard key now shows a real, single-character-appropriate
|
||||||
|
suggestion strip ("H | How | Hey") rather than stale state, and a
|
||||||
|
`render()` log line fires for every keystroke -- both confirm the
|
||||||
|
`InputConnection` calls are landing and are being processed, which
|
||||||
|
a hand-typed `adb shell input text` did *not* reliably exercise on
|
||||||
|
this AVD (no `render()` at all followed one such call -- most
|
||||||
|
likely a modern `input text` no longer round-trips through
|
||||||
|
`commitText` the way older docs assume; Gboard-key taps are the
|
||||||
|
real path and the one this fix was verified against).
|
||||||
|
|
||||||
|
2. **A second, deeper bug found while verifying (1), not root-caused
|
||||||
|
this pass**: composed text never becomes visible on screen at
|
||||||
|
all -- the grey composer bar stays empty, with no glyph anywhere
|
||||||
|
in the frame, confirmed on repeated Gboard-key taps and across a
|
||||||
|
keyboard-resize. **Ruled out**: the widget tree's own layout math.
|
||||||
|
A new unit test, `layout_tests::
|
||||||
|
composing_text_after_a_keyboard_resize_lands_in_the_bars_own_region`,
|
||||||
|
builds the composer's exact tree shape (`Stack{rect, Span{Pad{
|
||||||
|
TextEdit}}}` inside an outer `Span::DOWN`) with no GPU or window,
|
||||||
|
resizes it the way a real keyboard-triggered `surface_changed`
|
||||||
|
does, edits the field both before and after, and asserts the
|
||||||
|
field's `window_region` stays a small box near the bottom of
|
||||||
|
whichever window size is current -- it passes, both before and
|
||||||
|
after this pass's composer rebuild (item 3 below), so the CPU-side
|
||||||
|
region a redraw lands at is provably correct. The bug is
|
||||||
|
therefore downstream of that -- most likely something specific to
|
||||||
|
the GPU-side redraw a content-only edit takes (`UiRenderState::
|
||||||
|
redraw`, which redraws a single dirtied widget directly at its
|
||||||
|
stored region rather than re-running its ancestors' layout) or to
|
||||||
|
this AVD's forced `force-gles` backend (the only one available
|
||||||
|
here; Iris's phone deliveries have used real Vulkan) -- neither
|
||||||
|
isolated this pass. **Not attributable to this pass's changes**:
|
||||||
|
reproduced identically before touching `composer.rs` (the very
|
||||||
|
first build tested, before the composer rebuild below, already
|
||||||
|
had it) and the render-engine files this pass did not touch
|
||||||
|
(`core/src/render/mod.rs`, `core/src/ui/render_state.rs`) are the
|
||||||
|
likely next place to look -- specifically `UiRenderState::redraw`'s
|
||||||
|
reuse of a widget's own last-drawn region versus a full tree walk.
|
||||||
|
**Needs**: either a Vulkan-capable emulator boot or the real phone
|
||||||
|
to rule `force-gles` in or out, and a GPU-side primitive dump
|
||||||
|
(the existing `frame diagnostics` log line, extended to name which
|
||||||
|
primitives a frame actually wrote) to see whether the glyph quads
|
||||||
|
are emitted at all or emitted somewhere off-screen.
|
||||||
|
|
||||||
|
3. **The composer bar rebuilt as one widget**, per this box's own
|
||||||
|
ask: `transcript_ui::composer::build_composer` (unchanged
|
||||||
|
`Stack{background, Span{Pad{TextEdit}}}` idiom, the same one the
|
||||||
|
header row's `HEADER_SURFACE` already uses) now also caps the
|
||||||
|
field at roughly six lines (`MaxSize` + `.scrollable()` for a
|
||||||
|
wheel/trackpad overflow scroll -- a real touch-drag scroll on
|
||||||
|
overflowing composer text is not wired and is a follow-up) and
|
||||||
|
wraps the whole bar in one `Pad` whose `bottom` a new
|
||||||
|
`Composer::set_bottom_inset(rsc, inset)` rewrites in place
|
||||||
|
whenever the platform's insets change, called from
|
||||||
|
`bench_client.rs`'s existing `on_insets_changed` with
|
||||||
|
`insets.bottom.max(insets.ime_bottom)` -- the IME's own inset
|
||||||
|
while it is open, the navigation bar's otherwise. Rewritten in
|
||||||
|
place rather than rebuilt through a `WidgetPtr` swap (`top_bar`'s
|
||||||
|
own pattern) because the field is strongly owned inside this tree
|
||||||
|
and cannot be re-added to a new wrapper without panicking
|
||||||
|
("was already added") -- rebuilding would also drop focus,
|
||||||
|
selection and in-progress text on every keyboard toggle.
|
||||||
|
**Verified**: `ui-trace` box readouts before/after a keyboard
|
||||||
|
open on the emulator (the field's row correctly reports a
|
||||||
|
547px move matching the real IME-triggered resize); the
|
||||||
|
known-separate "top row renders twice after a keyboard resize"
|
||||||
|
bug this box already recorded is unrelated and still open. **Not
|
||||||
|
fixed by this alone**: item 2 above -- the text still does not
|
||||||
|
render, so the "empty space at the bottom" symptom's other half
|
||||||
|
(nothing filling the space the bar itself now correctly reserves)
|
||||||
|
needs item 2's fix first before a real before/after screenshot is
|
||||||
|
worth taking.
|
||||||
|
|
||||||
|
4. **App-switch text loss, fixed and verified.** `surface_destroyed`
|
||||||
|
(backgrounding) drops the whole `AndroidRenderer` -- device,
|
||||||
|
atlas, buffers -- and a subsequent `surface_changed` with no live
|
||||||
|
renderer builds a genuinely new one (`AndroidRenderer::new`,
|
||||||
|
distinct from the keyboard-resize path this box already fixed by
|
||||||
|
*reusing* the renderer). But `iris_core::TextData::atlas` (the
|
||||||
|
CPU-side glyph cache) and `UiData::textures` (the CPU-side texture
|
||||||
|
bookkeeping the atlas is built on) live on `AndroidRsc`, which
|
||||||
|
outlives any one `AndroidRenderer` -- so both kept pointing at the
|
||||||
|
*old*, now-destroyed device's textures across the switch, the
|
||||||
|
exact "rectangles stay, glyphs disappear" shape, just triggered by
|
||||||
|
backgrounding instead of the keyboard. **Fix**: new
|
||||||
|
`GlyphAtlas::clear()` and `Textures::reset()` (`iris/core/src/
|
||||||
|
render/atlas.rs`, `iris/core/src/primitive/texture.rs`), called
|
||||||
|
together from `surface_changed`'s "genuinely new renderer" branch
|
||||||
|
only -- the same `already_live` check that already decides
|
||||||
|
reuse-vs-new, so this is one mechanism gated on the one condition
|
||||||
|
that needs it, not a second ad hoc check. **Verified on the
|
||||||
|
emulator**: backgrounded via `KEYCODE_HOME`, reopened via
|
||||||
|
`am start`, screenshotted -- every pre-existing glyph (headings,
|
||||||
|
body text, the whole diagnostics report) is intact, `frame_count`
|
||||||
|
resets to 1 confirming a genuinely new renderer was built, no
|
||||||
|
crash.
|
||||||
|
|
||||||
|
5. **Swipe-vs-tap focus, fixed and verified** (Iris's follow-up the
|
||||||
|
same day: "if I swipe over the input bar it brings up the
|
||||||
|
keyboard... scrolling should be pinned"). `attr.rs`'s `Selector`/
|
||||||
|
`Selectable` registered `CursorSense::click_or_drag()`, which
|
||||||
|
calls `select()` -- and so grants focus and requests the IME --
|
||||||
|
on the *first* frame of any press, before it is known whether the
|
||||||
|
gesture will end up a tap or a drag. Rewritten around a shared
|
||||||
|
`on_press` dispatcher over `PressStart`/`Pressing`/`PressEnd`: a
|
||||||
|
field that is **already** focused behaves exactly as before
|
||||||
|
(every frame updates the selection, so dragging inside a focused
|
||||||
|
field to select text still works); a field that is **not**
|
||||||
|
focused records where the press began (`TextEdit::press_origin`,
|
||||||
|
new field) and only grants focus on `PressEnd` if no intervening
|
||||||
|
frame crossed `sense::DRAG_SLOP` -- a drag recognised early simply
|
||||||
|
clears the pending tap and does nothing further, so it is never
|
||||||
|
consumed and whatever is behind the field still sees every frame
|
||||||
|
of it. New `FocusHost::is_focused` (both platform impls) is what
|
||||||
|
lets `on_press` tell the two cases apart. **Verified on the
|
||||||
|
emulator**: `dumpsys input_method`'s `mInputShown` reads `false`
|
||||||
|
after a `swipe` gesture starting on the composer bar (`ui-trace`
|
||||||
|
confirms the field's own box never moved, i.e. no keyboard-driven
|
||||||
|
resize happened), and reads `true` after an ordinary `tap` on the
|
||||||
|
same field. **Coordination note**: a concurrent pass is moving
|
||||||
|
drag arbitration into `sense.rs` behind a new `Drop` event: this
|
||||||
|
fix touches only `attr.rs` (new `press_track`/`on_press`) and
|
||||||
|
`iris/src/widget/text/edit.rs` (the new `press_origin` field), not
|
||||||
|
`sense.rs` itself, so it should merge cleanly, but the next agent
|
||||||
|
through here should check whether `Selector`/`Selectable`'s
|
||||||
|
`Pressing`-frame delivery still arrives the way this code assumes
|
||||||
|
once that lands.
|
||||||
|
|
||||||
|
**Checks this pass**: `cargo fmt --all` clean, `cargo clippy
|
||||||
|
--workspace --all-targets` and `cargo ndk -t x86_64 -P 26 clippy
|
||||||
|
--features "transcript-screen bench force-gles"` both zero warnings
|
||||||
|
beyond the pre-existing `tabs-ui` unused-dependency notice, `cargo
|
||||||
|
test --workspace` all passing (new tests: four in `edit.rs`, one in
|
||||||
|
`layout_tests.rs`). **Not done**: item 2's root cause; a real
|
||||||
|
before/after screenshot pair for item 3 (blocked on item 2); anything
|
||||||
|
on Vulkan or the real phone.
|
||||||
|
|
||||||
- [ ] **P1 — session screen parity.** History paging backward (with the
|
- [ ] **P1 — session screen parity.** History paging backward (with the
|
||||||
page-boundary healing `client-core` does not have yet, below),
|
page-boundary healing `client-core` does not have yet, below),
|
||||||
`TranscriptSource`-backed cache/server stitching, jump-to-latest,
|
`TranscriptSource`-backed cache/server stitching, jump-to-latest,
|
||||||
|
|||||||
@@ -0,0 +1,58 @@
|
|||||||
|
# iris bench v2 report from Iris's phone, 2026-09-06
|
||||||
|
|
||||||
|
Build 2e3f4ad (bench v2, fling physics, keyboard-wipe fix, dp unit), run
|
||||||
|
by Iris on her Pixel 9 Pro XL, verbatim. The display was at **120 Hz**
|
||||||
|
(8.3 ms budget) where `compose-phone-v2-2026-09-06.md` ran at 60 Hz, so
|
||||||
|
compare the millisecond percentiles, not `late`.
|
||||||
|
|
||||||
|
Side by side (Compose 60 Hz / iris 120 Hz, p50 / p90 / p99 ms): fling
|
||||||
|
5.5/8.7/11.6 vs 3.8/6.9/12.6; stream 13.4/31.7/42.5 vs 18.2/35.8/43.1;
|
||||||
|
type 7.3/13.2/16.5 vs 7.2/9.2/11.2; keyboard: iris could not show the IME
|
||||||
|
(phase invalid). Process CPU 69.6 s over 125 s vs 40.6 s over 150 s; peak
|
||||||
|
RSS 577 MB vs 379 MB; battery current mean 571 mA vs 452 mA.
|
||||||
|
|
||||||
|
Iris's observations on the same run: "the scrolling is not similar at
|
||||||
|
all. It does not fling for me yet [with a finger], and the test also seems
|
||||||
|
to give it a constant velocity and abruptly stop it at some point. Also
|
||||||
|
unsure what's going on in that image with the compaction" -- her
|
||||||
|
screenshot shows the `Compacted: 180000 -> 20000 tokens.` row drawn twice
|
||||||
|
overlapping, and once more below the composer bar: primitives of a
|
||||||
|
replaced/removed row surviving in the GPU buffers, the same shape as the
|
||||||
|
header drawn twice after a keyboard resize.
|
||||||
|
|
||||||
|
```
|
||||||
|
iris bench report
|
||||||
|
per phase:
|
||||||
|
fling: 1783 frames over 53.2s
|
||||||
|
late: 104 (5.8%)
|
||||||
|
total p50 3.8ms p90 6.9ms p99 12.6ms
|
||||||
|
worst 29.1ms
|
||||||
|
stream: 401 frames over 21.3s
|
||||||
|
late: 306 (76.3%)
|
||||||
|
total p50 18.2ms p90 35.8ms p99 43.1ms
|
||||||
|
worst 43.8ms
|
||||||
|
type: 1202 frames over 65.7s
|
||||||
|
late: 309 (25.7%)
|
||||||
|
total p50 7.2ms p90 9.2ms p99 11.2ms
|
||||||
|
worst 15.3ms
|
||||||
|
keyboard: 9 frames over 9.7s
|
||||||
|
late: 9 (100.0%)
|
||||||
|
total p50 12.0ms p90 12.9ms p99 12.9ms
|
||||||
|
worst 12.9ms
|
||||||
|
|
||||||
|
frames:
|
||||||
|
3395 frames over 149.9s at 120Hz (8.3ms budget)
|
||||||
|
late: 728 (21.4%)
|
||||||
|
total p50 5.0ms p90 10.9ms p99 36.6ms
|
||||||
|
worst 43.8ms
|
||||||
|
cpu_p50 2.0ms gpu_wait_p50 2.6ms
|
||||||
|
|
||||||
|
bench:
|
||||||
|
fling: 8 flings out + 8 back at 12000px/s, travel start=idx=651/off=1217px outward=idx=651/off=101536px end=idx=651/off=1022px
|
||||||
|
scroll: 6 cycles (24 swipes, legacy tween), streamed 400/400 fixture events
|
||||||
|
type: 600 characters inserted then deleted, one per 50ms
|
||||||
|
keyboard: could not be shown (5 attempts, 0 confirmed visible)
|
||||||
|
process CPU time over this run: 40603ms
|
||||||
|
peak RSS: 379156kB
|
||||||
|
battery current: mean -452353µA over 149 samples (min -1753125, max -204687)
|
||||||
|
```
|
||||||
@@ -31,6 +31,25 @@ public final class MainActivity extends Activity {
|
|||||||
setContentView(layout);
|
setContentView(layout);
|
||||||
view.requestFocus();
|
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) -> {
|
view.setOnApplyWindowInsetsListener((v, insets) -> {
|
||||||
int left = insets.getSystemWindowInsetLeft();
|
int left = insets.getSystemWindowInsetLeft();
|
||||||
int top = insets.getSystemWindowInsetTop();
|
int top = insets.getSystemWindowInsetTop();
|
||||||
|
|||||||
@@ -221,8 +221,15 @@ fn battery_line(samples: &[i32]) -> String {
|
|||||||
return " battery current: unavailable on this device".to_string();
|
return " battery current: unavailable on this device".to_string();
|
||||||
}
|
}
|
||||||
let mean = samples.iter().map(|&v| v as i64).sum::<i64>() / samples.len() as i64;
|
let mean = samples.iter().map(|&v| v as i64).sum::<i64>() / samples.len() as i64;
|
||||||
let min = samples.iter().min().unwrap();
|
// `min`/`max` are guarded by the `is_empty` check above, three lines
|
||||||
let max = samples.iter().max().unwrap();
|
// up -- pairing the `Option` unwraps with the emptiness check right
|
||||||
|
// here (rather than two statements apart, with `mean` in between
|
||||||
|
// reading the same slice) is what keeps a future reorder from
|
||||||
|
// separating the guard from what it protects (docs/
|
||||||
|
// REVIEW-2026-09-06.md finding 7).
|
||||||
|
let (Some(min), Some(max)) = (samples.iter().min(), samples.iter().max()) else {
|
||||||
|
unreachable!("samples is non-empty, checked above");
|
||||||
|
};
|
||||||
format!(
|
format!(
|
||||||
" battery current: mean {mean}\u{b5}A over {} samples (min {min}, max {max})",
|
" battery current: mean {mean}\u{b5}A over {} samples (min {min}, max {max})",
|
||||||
samples.len()
|
samples.len()
|
||||||
@@ -365,6 +372,18 @@ impl AndroidAppState for BenchClient {
|
|||||||
(self.top_bar)(rsc).set(controls);
|
(self.top_bar)(rsc).set(controls);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The composer bar sits directly on whichever of the IME or the
|
||||||
|
// navigation bar is currently the bottom of usable space -- see
|
||||||
|
// `transcript_ui::composer::Composer::set_bottom_inset`'s doc.
|
||||||
|
// `ime_bottom` already exceeds the plain nav-bar inset whenever the
|
||||||
|
// keyboard covers it, so the larger of the two is always the right
|
||||||
|
// answer without needing to know which is currently showing.
|
||||||
|
if let Some(screen) = &self.screen {
|
||||||
|
screen
|
||||||
|
.composer
|
||||||
|
.set_bottom_inset(rsc, insets.bottom.max(insets.ime_bottom));
|
||||||
|
}
|
||||||
|
|
||||||
let ime_visible = insets.ime_bottom > 0.0;
|
let ime_visible = insets.ime_bottom > 0.0;
|
||||||
|
|
||||||
let mut ime = self.ime_state.lock().unwrap();
|
let mut ime = self.ime_state.lock().unwrap();
|
||||||
|
|||||||
@@ -141,6 +141,27 @@ impl Textures {
|
|||||||
self.updates.push(Update::Patch(handle.slot, rect));
|
self.updates.push(Update::Patch(handle.slot, rect));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Forget every image, page and pending update -- what a genuinely new
|
||||||
|
/// GPU device needs alongside [`crate::render::atlas::GlyphAtlas::
|
||||||
|
/// clear`], which this module's own doc references: every slot number
|
||||||
|
/// and every queued [`Update`] here describes the *old* device's
|
||||||
|
/// textures (an `Update::Push`/`Update::Patch` already drained into a
|
||||||
|
/// renderer that no longer exists is gone for good, and a fresh
|
||||||
|
/// `UiRenderNode`'s own texture manager starts with none of them
|
||||||
|
/// applied), so nothing is lost by starting this bookkeeping over too.
|
||||||
|
/// Any `TextureHandle` a caller still holds across the reset (none in
|
||||||
|
/// the transcript screen this reset is wired up for today -- confirmed
|
||||||
|
/// by grep, the only standalone (non-atlas) image anywhere in this
|
||||||
|
/// workspace is `iris/widget/image.rs`'s `Image`, used by the separate
|
||||||
|
/// `tabs-ui` example) is left pointing at a slot this instance no
|
||||||
|
/// longer recognises and needs reinserting via `add`/`add_page` again
|
||||||
|
/// -- the same pre-existing gap a renderer restart already left for
|
||||||
|
/// such a handle before this method existed, just named rather than
|
||||||
|
/// silent now.
|
||||||
|
pub fn reset(&mut self) {
|
||||||
|
*self = Self::new();
|
||||||
|
}
|
||||||
|
|
||||||
pub fn free(&mut self) {
|
pub fn free(&mut self) {
|
||||||
for (kind, idx) in self.recv.try_iter() {
|
for (kind, idx) in self.recv.try_iter() {
|
||||||
self.images[idx as usize] = None;
|
self.images[idx as usize] = None;
|
||||||
|
|||||||
@@ -173,6 +173,25 @@ impl GlyphAtlas {
|
|||||||
pub fn glyph_count(&self) -> usize {
|
pub fn glyph_count(&self) -> usize {
|
||||||
self.entries.len()
|
self.entries.len()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Forget every page and every rasterised entry -- what a genuinely new
|
||||||
|
/// GPU device needs (`android::view::IrisViewPeer::surface_changed`'s
|
||||||
|
/// "not already live" branch, e.g. after backgrounding): the pages this
|
||||||
|
/// atlas remembers are `TextureHandle`s into the *old* device's
|
||||||
|
/// textures, which no longer exist, and every `GlyphEntry`'s `uv_min`/
|
||||||
|
/// `uv_max`/`layer` point into them. Without this, a glyph already
|
||||||
|
/// cached here is treated as "already placed" and never re-inserted
|
||||||
|
/// into the fresh (empty) atlas the new renderer actually has --
|
||||||
|
/// exactly the "rectangles stay, glyphs disappear" bug the resize path
|
||||||
|
/// (`AndroidRenderer::resize`) was built to avoid for the reuse case;
|
||||||
|
/// this is its counterpart for the case where the renderer really is
|
||||||
|
/// new. Dropping `pages` also drops its `TextureHandle`s, which send a
|
||||||
|
/// free message back through their `Textures`; see `Textures::reset`'s
|
||||||
|
/// doc for why that is harmless here.
|
||||||
|
pub fn clear(&mut self) {
|
||||||
|
self.pages.clear();
|
||||||
|
self.entries.clear();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn fits(page: &Page, need_w: u32, need_h: u32) -> bool {
|
fn fits(page: &Page, need_w: u32, need_h: u32) -> bool {
|
||||||
|
|||||||
@@ -245,6 +245,15 @@ impl FrameReport {
|
|||||||
/// this once per phase (fling/stream/type/keyboard) so `phase_stats`
|
/// this once per phase (fling/stream/type/keyboard) so `phase_stats`
|
||||||
/// can slice one whole run's frames by what was happening during each.
|
/// can slice one whole run's frames by what was happening during each.
|
||||||
pub fn mark_phase(&mut self, name: &str) {
|
pub fn mark_phase(&mut self, name: &str) {
|
||||||
|
// `phase_stats`'s slicing (`idx >= phase.start_index && idx <
|
||||||
|
// end_index`) silently produces an empty or nonsensical slice for
|
||||||
|
// a phase pushed out of order rather than surfacing the misuse
|
||||||
|
// (docs/REVIEW-2026-09-06.md finding 5).
|
||||||
|
debug_assert!(
|
||||||
|
self.phases
|
||||||
|
.last()
|
||||||
|
.is_none_or(|p| self.total_frames >= p.start_index)
|
||||||
|
);
|
||||||
self.phases.push(PhaseMark {
|
self.phases.push(PhaseMark {
|
||||||
name: name.to_string(),
|
name: name.to_string(),
|
||||||
start_index: self.total_frames,
|
start_index: self.total_frames,
|
||||||
|
|||||||
@@ -20,6 +20,21 @@ pub struct UiRenderState {
|
|||||||
resized: bool,
|
resized: bool,
|
||||||
draw_started: HashSet<WidgetId>,
|
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
|
/// `Widget::draw` calls and `Primitives::region_mut` rewrites since the
|
||||||
/// last `take_counters`. LAYOUT.md section 8's pass conditions are
|
/// last `take_counters`. LAYOUT.md section 8's pass conditions are
|
||||||
/// stated in terms of these two: an unchanged frame must cost 0 of
|
/// stated in terms of these two: an unchanged frame must cost 0 of
|
||||||
@@ -45,6 +60,7 @@ impl UiRenderState {
|
|||||||
old_root: None,
|
old_root: None,
|
||||||
resized: false,
|
resized: false,
|
||||||
draw_started: Default::default(),
|
draw_started: Default::default(),
|
||||||
|
captured: Default::default(),
|
||||||
draw_count: 0,
|
draw_count: 0,
|
||||||
region_mut_count: 0,
|
region_mut_count: 0,
|
||||||
mov_count: 0,
|
mov_count: 0,
|
||||||
@@ -357,6 +373,13 @@ impl UiRenderState {
|
|||||||
active.textures.clear();
|
active.textures.clear();
|
||||||
rsc.ui_mut().textures.free();
|
rsc.ui_mut().textures.free();
|
||||||
if undraw {
|
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
|
// Permanent removal: retire this widget's own move slot
|
||||||
// (the self-ownership ref taken when it was allocated) and
|
// (the self-ownership ref taken when it was allocated) and
|
||||||
// the up-link ref it held on its parent's slot -- read from
|
// the up-link ref it held on its parent's slot -- read from
|
||||||
@@ -429,6 +452,27 @@ impl UiRenderState {
|
|||||||
self.active.len()
|
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> {
|
pub fn debug(&self, widgets: &Widgets, label: &str) -> impl Iterator<Item = &ActiveData> {
|
||||||
self.active.iter().filter_map(move |(&id, inst)| {
|
self.active.iter().filter_map(move |(&id, inst)| {
|
||||||
let l = widgets.label(id);
|
let l = widgets.label(id);
|
||||||
|
|||||||
@@ -12,6 +12,10 @@ impl<T: HasAndroidUiState> FocusHost for T {
|
|||||||
self.android_state_mut().focus = id;
|
self.android_state_mut().focus = id;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn is_focused(&self, id: WeakWidget<TextEdit>) -> bool {
|
||||||
|
self.android_state().focus == Some(id)
|
||||||
|
}
|
||||||
|
|
||||||
fn focus_gained(&mut self, region: Option<PixelRegion>) {
|
fn focus_gained(&mut self, region: Option<PixelRegion>) {
|
||||||
// Showing the keyboard is a JNI call (`InputMethodManager.showSoftInput`),
|
// Showing the keyboard is a JNI call (`InputMethodManager.showSoftInput`),
|
||||||
// and this runs deep inside the platform-agnostic sensor dispatch
|
// and this runs deep inside the platform-agnostic sensor dispatch
|
||||||
|
|||||||
@@ -49,6 +49,52 @@ impl<State: AndroidAppState> IrisViewPeer<State> {
|
|||||||
fn focus(&self) -> Option<WeakWidget<TextEdit>> {
|
fn focus(&self) -> Option<WeakWidget<TextEdit>> {
|
||||||
self.state.android_state().focus
|
self.state.android_state().focus
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Tell Gboard where the caret/selection and the composing region
|
||||||
|
/// actually are, via `InputMethodManager.updateSelection` -- every one
|
||||||
|
/// of android-view's own demo's `set_composing_text_internal`/`render`
|
||||||
|
/// calls this, and this bridge never did, which is what left Gboard's
|
||||||
|
/// own model of the field diverging from `TextEdit`'s real one after
|
||||||
|
/// the very first edit (RUST.md's P0 box, "doesn't enter it until I
|
||||||
|
/// hit space, and also doesn't move cursor forward" -- Gboard holds
|
||||||
|
/// its composing keystrokes back until it believes the app has caught
|
||||||
|
/// up, and without this call it never does). Called from
|
||||||
|
/// [`IrisViewPeer::after_input`], the one tail every touch/key/IME
|
||||||
|
/// callback already runs through, rather than duplicated at each of
|
||||||
|
/// this file's mutating methods.
|
||||||
|
///
|
||||||
|
/// `candidates_start`/`candidates_end` report the composing region;
|
||||||
|
/// `-1, -1` when nothing is composing, matching `EditorInfo`'s own
|
||||||
|
/// convention. `compose_len` is tracked in `char`s (this module's doc
|
||||||
|
/// comment), so this reports it as that many UTF-16 units back from the
|
||||||
|
/// caret -- exact for the common BMP case, the same approximation
|
||||||
|
/// `set_composing_text` already makes.
|
||||||
|
pub(super) fn update_ime_selection(&mut self, ctx: &mut CallbackCtx) {
|
||||||
|
let Some(focus) = self.focus() else { return };
|
||||||
|
let text = &self.rsc[focus];
|
||||||
|
let Some(sel) = text.selection_range() else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let content = text.text();
|
||||||
|
let sel_start = byte_to_utf16(content, sel.start) as i32;
|
||||||
|
let sel_end = byte_to_utf16(content, sel.end) as i32;
|
||||||
|
let compose_len = self.state.android_state().compose_len;
|
||||||
|
let (comp_start, comp_end) = if compose_len > 0 {
|
||||||
|
let caret = byte_to_utf16(content, text.caret().unwrap_or(sel.end)) as i32;
|
||||||
|
(caret - compose_len as i32, caret)
|
||||||
|
} else {
|
||||||
|
(-1, -1)
|
||||||
|
};
|
||||||
|
let imm = ctx.view.input_method_manager(&mut ctx.env);
|
||||||
|
imm.update_selection(
|
||||||
|
&mut ctx.env,
|
||||||
|
&ctx.view,
|
||||||
|
sel_start,
|
||||||
|
sel_end,
|
||||||
|
comp_start,
|
||||||
|
comp_end,
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<State: AndroidAppState> InputConnection for IrisViewPeer<State> {
|
impl<State: AndroidAppState> InputConnection for IrisViewPeer<State> {
|
||||||
|
|||||||
@@ -325,6 +325,13 @@ impl<State: AndroidAppState> IrisViewPeer<State> {
|
|||||||
show_soft_input(&mut ctx.env, &ctx.view);
|
show_soft_input(&mut ctx.env, &ctx.view);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// RUST.md's P0 box, "doesn't enter it until I hit space, and also
|
||||||
|
// doesn't move cursor forward": Gboard needs `updateSelection`
|
||||||
|
// after every edit to keep its own model of the field in sync, or
|
||||||
|
// it holds keystrokes back rather than trusting a screen it
|
||||||
|
// believes is stale. See `update_ime_selection`'s own doc.
|
||||||
|
self.update_ime_selection(ctx);
|
||||||
|
|
||||||
let ui_state = self.state.android_state_mut();
|
let ui_state = self.state.android_state_mut();
|
||||||
ui_state.cursor.end_frame();
|
ui_state.cursor.end_frame();
|
||||||
if self.render.needs_redraw(&ui_state.root, self.rsc.widgets()) {
|
if self.render.needs_redraw(&ui_state.root, self.rsc.widgets()) {
|
||||||
@@ -650,6 +657,22 @@ impl<State: AndroidAppState> ViewPeer for IrisViewPeer<State> {
|
|||||||
let content_scale = self.state.android_state().content_scale;
|
let content_scale = self.state.android_state().content_scale;
|
||||||
match AndroidRenderer::new(window, width as u32, height as u32, content_scale) {
|
match AndroidRenderer::new(window, width as u32, height as u32, content_scale) {
|
||||||
Ok(renderer) => {
|
Ok(renderer) => {
|
||||||
|
// A genuinely new renderer means a genuinely new GPU device
|
||||||
|
// and a fresh, empty glyph atlas -- the CPU-side glyph
|
||||||
|
// cache (`TextData::atlas`) and the texture bookkeeping it
|
||||||
|
// is built on (`UiData::textures`) both outlive `renderer`
|
||||||
|
// itself (they live on `self.rsc`, not on `AndroidRenderer`),
|
||||||
|
// so without this they would keep pointing at the *old*
|
||||||
|
// device's now-gone textures -- the app-switch counterpart
|
||||||
|
// to the keyboard-resize glyph wipe this same function's
|
||||||
|
// `already_live` branch above already fixed by reusing the
|
||||||
|
// renderer instead of rebuilding it. One mechanism either
|
||||||
|
// way: this call only runs on the branch that actually
|
||||||
|
// builds a new renderer, exactly where invalidation is
|
||||||
|
// needed, never on the reuse branch, where it would throw
|
||||||
|
// away perfectly valid GPU state for nothing.
|
||||||
|
self.rsc.ui.text.atlas.clear();
|
||||||
|
self.rsc.ui.textures.reset();
|
||||||
self.state.android_state_mut().renderer = Some(renderer);
|
self.state.android_state_mut().renderer = Some(renderer);
|
||||||
self.render(ctx);
|
self.render(ctx);
|
||||||
}
|
}
|
||||||
|
|||||||
+67
-9
@@ -22,6 +22,13 @@ pub trait FocusHost {
|
|||||||
/// it was hit in (`None` when the widget could not be located, which
|
/// it was hit in (`None` when the widget could not be located, which
|
||||||
/// happens for one it was just deselected from).
|
/// happens for one it was just deselected from).
|
||||||
fn focus_gained(&mut self, region: Option<PixelRegion>);
|
fn focus_gained(&mut self, region: Option<PixelRegion>);
|
||||||
|
/// Whether `id` is the current focus target -- what [`select`] uses to
|
||||||
|
/// tell a fresh press (which must wait to see whether it becomes a tap
|
||||||
|
/// or a drag before focusing/showing the IME, Iris 2026-09-06: "if I
|
||||||
|
/// swipe over the input bar it brings up the keyboard") from a drag
|
||||||
|
/// continuing inside a field that was already focused (an ordinary
|
||||||
|
/// drag-to-select, unaffected).
|
||||||
|
fn is_focused(&self, id: WeakWidget<TextEdit>) -> bool;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Helper shared by every `FocusHost` impl, so the double-click window is
|
/// Helper shared by every `FocusHost` impl, so the double-click window is
|
||||||
@@ -33,6 +40,17 @@ pub fn recent_click(last_click: &mut Instant) -> bool {
|
|||||||
recent
|
recent
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `PressStart`/`Pressing`/`PressEnd`, all for the left button -- what
|
||||||
|
/// [`Selector`]/[`Selectable`] register instead of [`CursorSense::
|
||||||
|
/// click_or_drag`], so their shared handler (`on_press`, below) sees every
|
||||||
|
/// frame of a gesture and can tell a completed tap from a drag itself,
|
||||||
|
/// rather than reacting to `PressStart` alone the way `click_or_drag`'s
|
||||||
|
/// consumer used to (Iris, 2026-09-06: "if I swipe over the input bar it
|
||||||
|
/// brings up the keyboard").
|
||||||
|
fn press_track() -> CursorSenses {
|
||||||
|
CursorSense::click() | CursorSense::Pressing(CursorButton::Left) | CursorSense::unclick()
|
||||||
|
}
|
||||||
|
|
||||||
pub struct Selector;
|
pub struct Selector;
|
||||||
|
|
||||||
impl<Rsc: HasEvents, W: Widget + 'static> WidgetAttr<Rsc, W> for Selector
|
impl<Rsc: HasEvents, W: Widget + 'static> WidgetAttr<Rsc, W> for Selector
|
||||||
@@ -42,7 +60,7 @@ where
|
|||||||
type Input = WeakWidget<TextEdit>;
|
type Input = WeakWidget<TextEdit>;
|
||||||
|
|
||||||
fn run(rsc: &mut Rsc, container: WeakWidget<W>, id: Self::Input) {
|
fn run(rsc: &mut Rsc, container: WeakWidget<W>, id: Self::Input) {
|
||||||
rsc.register_event(container, CursorSense::click_or_drag(), move |ctx, rsc| {
|
rsc.register_event(container, press_track(), move |ctx, rsc| {
|
||||||
let region = ctx.data.render.window_region(&id, &*rsc).unwrap();
|
let region = ctx.data.render.window_region(&id, &*rsc).unwrap();
|
||||||
let id_pos = region.top_left;
|
let id_pos = region.top_left;
|
||||||
let container_pos = ctx
|
let container_pos = ctx
|
||||||
@@ -53,14 +71,14 @@ where
|
|||||||
.top_left;
|
.top_left;
|
||||||
let pos = ctx.data.pos + container_pos - id_pos;
|
let pos = ctx.data.pos + container_pos - id_pos;
|
||||||
let size = region.size();
|
let size = region.size();
|
||||||
select(
|
on_press(
|
||||||
rsc,
|
rsc,
|
||||||
ctx.data.render,
|
ctx.data.render,
|
||||||
ctx.state,
|
ctx.state,
|
||||||
id,
|
id,
|
||||||
pos,
|
pos,
|
||||||
size,
|
size,
|
||||||
ctx.data.sense.is_dragging(),
|
ctx.data.sense,
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -75,31 +93,71 @@ where
|
|||||||
type Input = ();
|
type Input = ();
|
||||||
|
|
||||||
fn run(rsc: &mut Rsc, id: WeakWidget<TextEdit>, _: Self::Input) {
|
fn run(rsc: &mut Rsc, id: WeakWidget<TextEdit>, _: Self::Input) {
|
||||||
rsc.register_event(id, CursorSense::click_or_drag(), move |ctx, rsc| {
|
rsc.register_event(id, press_track(), move |ctx, rsc| {
|
||||||
select(
|
on_press(
|
||||||
rsc,
|
rsc,
|
||||||
ctx.data.render,
|
ctx.data.render,
|
||||||
ctx.state,
|
ctx.state,
|
||||||
id,
|
id,
|
||||||
ctx.data.pos,
|
ctx.data.pos,
|
||||||
ctx.data.size,
|
ctx.data.size,
|
||||||
ctx.data.sense.is_dragging(),
|
ctx.data.sense,
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn select(
|
/// One press-track frame (`PressStart`, `Pressing` or `PressEnd`) over a
|
||||||
|
/// selectable field. A field that is *already* focused behaves exactly as
|
||||||
|
/// `click_or_drag` always did -- every frame updates the selection, which
|
||||||
|
/// is what lets a finger already inside a focused field drag out a
|
||||||
|
/// selection. A field that is **not** focused withholds `select`'s
|
||||||
|
/// focus-granting side effects (and so the platform-specific `focus_gained`
|
||||||
|
/// that shows the keyboard) until the press resolves as a tap: `PressEnd`
|
||||||
|
/// with no frame in between having moved past [`DRAG_SLOP`] from where the
|
||||||
|
/// press began. A drag recognised before release simply cancels the
|
||||||
|
/// pending tap and does nothing further here -- it is not consumed, so
|
||||||
|
/// whatever is behind the field (a list to pan) still sees every frame of
|
||||||
|
/// it, the same as a drag that never touched a selectable field at all.
|
||||||
|
fn on_press(
|
||||||
rsc: &mut impl UiRsc,
|
rsc: &mut impl UiRsc,
|
||||||
render: &UiRenderState,
|
render: &UiRenderState,
|
||||||
state: &mut impl FocusHost,
|
state: &mut impl FocusHost,
|
||||||
id: WeakWidget<TextEdit>,
|
id: WeakWidget<TextEdit>,
|
||||||
pos: Vec2,
|
pos: Vec2,
|
||||||
size: Vec2,
|
size: Vec2,
|
||||||
dragging: bool,
|
sense: CursorSense,
|
||||||
) {
|
) {
|
||||||
|
if state.is_focused(id) {
|
||||||
|
let recent = matches!(sense, CursorSense::PressStart(_)) && state.recent_click();
|
||||||
|
id.edit(rsc).select(pos, size, sense.is_dragging(), recent);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
match sense {
|
||||||
|
CursorSense::PressStart(_) => {
|
||||||
|
id.edit(rsc).text.press_origin = Some(pos);
|
||||||
|
}
|
||||||
|
CursorSense::Pressing(_) => {
|
||||||
|
let ctx = id.edit(rsc);
|
||||||
|
if let Some(origin) = ctx.text.press_origin
|
||||||
|
&& ((pos.x - origin.x).abs() > DRAG_SLOP || (pos.y - origin.y).abs() > DRAG_SLOP)
|
||||||
|
{
|
||||||
|
// Past the slop before release: this is a drag, not a tap
|
||||||
|
// -- give up the pending focus rather than granting it once
|
||||||
|
// the finger lifts wherever it happens to be by then.
|
||||||
|
ctx.text.press_origin = None;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
CursorSense::PressEnd(_) => {
|
||||||
|
let was_tap = id.edit(rsc).text.press_origin.take().is_some();
|
||||||
|
if was_tap {
|
||||||
let recent = state.recent_click();
|
let recent = state.recent_click();
|
||||||
id.edit(rsc).select(pos, size, dragging, recent);
|
id.edit(rsc).select(pos, size, false, recent);
|
||||||
state.set_focus(Some(id));
|
state.set_focus(Some(id));
|
||||||
state.focus_gained(render.window_region(&id, &*rsc));
|
state.focus_gained(render.window_region(&id, &*rsc));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
_ => {}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
@@ -10,6 +10,10 @@ impl<T: HasDefaultUiState> FocusHost for T {
|
|||||||
self.default_state_mut().focus = id;
|
self.default_state_mut().focus = id;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn is_focused(&self, id: WeakWidget<TextEdit>) -> bool {
|
||||||
|
self.default_state().focus == Some(id)
|
||||||
|
}
|
||||||
|
|
||||||
fn focus_gained(&mut self, region: Option<PixelRegion>) {
|
fn focus_gained(&mut self, region: Option<PixelRegion>) {
|
||||||
let state = self.default_state_mut();
|
let state = self.default_state_mut();
|
||||||
let Some(region) = region else { return };
|
let Some(region) = region else { return };
|
||||||
|
|||||||
@@ -183,3 +183,80 @@ fn a_mask_stays_put_while_its_scrolled_content_moves() {
|
|||||||
assert_eq!(mask_delta_before, [0.0, 0.0]);
|
assert_eq!(mask_delta_before, [0.0, 0.0]);
|
||||||
assert_eq!(mask_delta_after, [0.0, 0.0]);
|
assert_eq!(mask_delta_after, [0.0, 0.0]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Reproduces `transcript_ui::composer::build_composer`'s exact tree shape
|
||||||
|
/// (a `Rect` background stacked behind a `Span::RIGHT`-wrapped, padded,
|
||||||
|
/// `rest`-width `TextEdit`, itself the second child of an outer
|
||||||
|
/// `Span::DOWN` beside a `rest(1)`-height sibling) without the event/
|
||||||
|
/// resource plumbing `composer.rs`'s builders need, to isolate whether the
|
||||||
|
/// bug Iris reported on 2026-09-06 ("text seems to not appear in box")
|
||||||
|
/// is this crate's layout engine or something specific to the real
|
||||||
|
/// composer/screen. `TextEditable::edit` only needs `UiRsc`, so a plain
|
||||||
|
/// insert exercises the exact redraw path a keystroke does.
|
||||||
|
fn composer_like_tree(rsc: &mut TestRsc) -> (WeakWidget<TextEdit>, StrongWidget) {
|
||||||
|
let field = wtext("")
|
||||||
|
.editable(EditMode::MultiLine)
|
||||||
|
.text_align(Align::LEFT)
|
||||||
|
.wrap(true)
|
||||||
|
.size(18)
|
||||||
|
.color(UiColor::WHITE)
|
||||||
|
.add(rsc);
|
||||||
|
let bar = (field.pad(dp(12)).width(rest(1)),)
|
||||||
|
.span(Dir::RIGHT)
|
||||||
|
.background(rect(UiColor::new(40, 40, 46, 255)))
|
||||||
|
.add(rsc);
|
||||||
|
let list_stand_in = rect(UiColor::BLACK).height(rest(1)).add(rsc);
|
||||||
|
let tree = (list_stand_in, bar).span(Dir::DOWN).add_strong(rsc).any();
|
||||||
|
(field, tree)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The reproduction itself. A window this tall stands in for the keyboard
|
||||||
|
/// closed; the second, shorter `resize` stands in for `adjustResize`
|
||||||
|
/// shrinking the surface when the IME opens -- exactly the sequence
|
||||||
|
/// `IrisViewPeer::surface_changed` drives on a real keyboard open. Typing
|
||||||
|
/// happens both before and after, since Iris's report was specifically
|
||||||
|
/// that text typed *after* the keyboard was already up did not appear.
|
||||||
|
#[test]
|
||||||
|
fn composing_text_after_a_keyboard_resize_lands_in_the_bars_own_region() {
|
||||||
|
let mut rsc = TestRsc {
|
||||||
|
ui: UiData::default(),
|
||||||
|
};
|
||||||
|
let (field, root) = composer_like_tree(&mut rsc);
|
||||||
|
let mut render = UiRenderState::new();
|
||||||
|
|
||||||
|
render.resize((1080.0, 2298.0));
|
||||||
|
render.update(&root, &mut rsc);
|
||||||
|
field.edit(&mut rsc).insert("a");
|
||||||
|
render.update(&root, &mut rsc);
|
||||||
|
|
||||||
|
let before_px = render.window_region(&field, &rsc).unwrap();
|
||||||
|
// The field is one line plus 12dp of padding on a 2298-tall window --
|
||||||
|
// nowhere near the whole window's height, and anchored at the bottom.
|
||||||
|
assert!(
|
||||||
|
before_px.bot_right.y - before_px.top_left.y < 200.0,
|
||||||
|
"before a resize: {before_px:?}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
before_px.top_left.y > 1800.0,
|
||||||
|
"expected the bar near the bottom before a resize: {before_px:?}"
|
||||||
|
);
|
||||||
|
|
||||||
|
// The keyboard opens: a real `surface_changed`/`resize` to a shorter
|
||||||
|
// window, then a further keystroke -- the redraw that must land in the
|
||||||
|
// bar's new (also short) region, not whatever region a provisional
|
||||||
|
// measurement pass used along the way.
|
||||||
|
render.resize((1080.0, 1478.0));
|
||||||
|
render.update(&root, &mut rsc);
|
||||||
|
field.edit(&mut rsc).insert("b");
|
||||||
|
render.update(&root, &mut rsc);
|
||||||
|
|
||||||
|
let after_px = render.window_region(&field, &rsc).unwrap();
|
||||||
|
assert!(
|
||||||
|
after_px.bot_right.y - after_px.top_left.y < 200.0,
|
||||||
|
"after a resize + keystroke: {after_px:?}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
after_px.top_left.y > 1200.0,
|
||||||
|
"expected the bar near the bottom of the shorter window: {after_px:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -22,6 +22,14 @@ pub enum CursorSense {
|
|||||||
Hovering,
|
Hovering,
|
||||||
HoverEnd,
|
HoverEnd,
|
||||||
Scroll,
|
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)]
|
#[derive(Clone)]
|
||||||
@@ -31,6 +39,21 @@ impl Event for CursorSenses {
|
|||||||
type Data<'a> = CursorData<'a>;
|
type Data<'a> = CursorData<'a>;
|
||||||
type State = SensorState;
|
type State = SensorState;
|
||||||
fn should_run<'a>(&self, data: &Self::Data<'a>) -> Option<Self::Data<'a>> {
|
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) {
|
if let Some(sense) = should_run(self, &data.cursor, data.hover) {
|
||||||
let mut data = data.clone();
|
let mut data = data.clone();
|
||||||
data.sense = sense;
|
data.sense = sense;
|
||||||
@@ -177,6 +200,48 @@ impl SensorUi for UiRenderState {
|
|||||||
cursor: CursorState,
|
cursor: CursorState,
|
||||||
window_size: Vec2,
|
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
|
// 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 /
|
// 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
|
// 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::Hovering => hover.is_on(),
|
||||||
CursorSense::HoverEnd => hover.is_end(),
|
CursorSense::HoverEnd => hover.is_end(),
|
||||||
CursorSense::Scroll => cursor.scroll_delta != Vec2::ZERO,
|
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);
|
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
|
/// How far back a [`VelocityTracker`] looks when estimating a fling's
|
||||||
/// initial speed -- Android's own `VelocityTracker` defaults to a similar
|
/// initial speed -- Android's own `VelocityTracker` defaults to a similar
|
||||||
/// short window so a gesture's last flick dominates over its slower start.
|
/// short window so a gesture's last flick dominates over its slower start.
|
||||||
@@ -575,6 +781,12 @@ impl VelocityTracker {
|
|||||||
/// Record one frame's motion. `delta` is this frame's movement since
|
/// Record one frame's motion. `delta` is this frame's movement since
|
||||||
/// the last sample, not a cumulative position.
|
/// the last sample, not a cumulative position.
|
||||||
pub fn add_sample(&mut self, delta: f32, at: Instant) {
|
pub fn add_sample(&mut self, delta: f32, at: Instant) {
|
||||||
|
// A caller that samples out of order (a restored/replayed
|
||||||
|
// gesture, a test) would silently produce a negative `span` in
|
||||||
|
// `velocity`, handled only by its `span <= 0.0 => 0.0` catch-all
|
||||||
|
// -- masking the bug that produced it rather than surfacing it
|
||||||
|
// (docs/REVIEW-2026-09-06.md finding 4).
|
||||||
|
debug_assert!(self.samples.back().is_none_or(|&(last, _)| at >= last));
|
||||||
self.samples.push_back((at, delta));
|
self.samples.push_back((at, delta));
|
||||||
while let Some(&(when, _)) = self.samples.front() {
|
while let Some(&(when, _)) = self.samples.front() {
|
||||||
if at.duration_since(when) > VELOCITY_WINDOW {
|
if at.duration_since(when) > VELOCITY_WINDOW {
|
||||||
@@ -750,6 +962,10 @@ impl FlingCalculator {
|
|||||||
/// Total signed distance the fling travels before settling, in the
|
/// Total signed distance the fling travels before settling, in the
|
||||||
/// same pixel units `velocity` was given in.
|
/// same pixel units `velocity` was given in.
|
||||||
pub fn distance(&self, velocity: f32) -> f32 {
|
pub fn distance(&self, velocity: f32) -> f32 {
|
||||||
|
// See `List::fling`'s matching assertion -- a non-finite velocity
|
||||||
|
// here silently produces a NaN distance rather than surfacing the
|
||||||
|
// bug that produced it (docs/REVIEW-2026-09-06.md finding 3).
|
||||||
|
debug_assert!(velocity.is_finite());
|
||||||
if velocity == 0.0 {
|
if velocity == 0.0 {
|
||||||
return 0.0;
|
return 0.0;
|
||||||
}
|
}
|
||||||
@@ -762,6 +978,8 @@ impl FlingCalculator {
|
|||||||
|
|
||||||
/// How long the fling takes to settle.
|
/// How long the fling takes to settle.
|
||||||
pub fn duration(&self, velocity: f32) -> Duration {
|
pub fn duration(&self, velocity: f32) -> Duration {
|
||||||
|
// See `distance`'s matching assertion, above.
|
||||||
|
debug_assert!(velocity.is_finite());
|
||||||
if velocity == 0.0 {
|
if velocity == 0.0 {
|
||||||
return Duration::ZERO;
|
return Duration::ZERO;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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 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"
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -424,6 +424,13 @@ impl List {
|
|||||||
/// pixels, so `1.0` here is not a placeholder for "unknown density,"
|
/// pixels, so `1.0` here is not a placeholder for "unknown density,"
|
||||||
/// it is the correct density for a self-consistent unit system.
|
/// it is the correct density for a self-consistent unit system.
|
||||||
pub fn fling(&mut self, velocity_px_per_s: f32) {
|
pub fn fling(&mut self, velocity_px_per_s: f32) {
|
||||||
|
// A NaN/inf velocity (a `VelocityTracker::velocity()` divide-by-
|
||||||
|
// near-zero span, or a caller passing a raw device value straight
|
||||||
|
// through) would propagate silently into `deceleration_for`'s
|
||||||
|
// `.ln()` -- the fling either never settles or jumps to NaN
|
||||||
|
// positions with nothing on screen saying why (docs/
|
||||||
|
// REVIEW-2026-09-06.md finding 3).
|
||||||
|
debug_assert!(velocity_px_per_s.is_finite());
|
||||||
if velocity_px_per_s == 0.0 || self.anchor.is_none() {
|
if velocity_px_per_s == 0.0 || self.anchor.is_none() {
|
||||||
self.fling = None;
|
self.fling = None;
|
||||||
return;
|
return;
|
||||||
@@ -552,6 +559,20 @@ impl List {
|
|||||||
self.extents.get(&key).map(|e| (e.top, e.bottom))
|
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 {
|
fn slot_exists(&self, slot: isize) -> bool {
|
||||||
match slot {
|
match slot {
|
||||||
BEFORE_SLOT => self.more_before.is_some(),
|
BEFORE_SLOT => self.more_before.is_some(),
|
||||||
@@ -749,6 +770,17 @@ impl List {
|
|||||||
/// one-frame lag `Scroll`'s own content-length cache accepts, per
|
/// one-frame lag `Scroll`'s own content-length cache accepts, per
|
||||||
/// LAYOUT.md.
|
/// LAYOUT.md.
|
||||||
fn place(&mut self, painter: &mut Painter, slot: isize, placement: Placement) -> (f32, f32) {
|
fn place(&mut self, painter: &mut Painter, slot: isize, placement: Placement) -> (f32, f32) {
|
||||||
|
// Every current caller derives `slot` from `repair_anchor`/
|
||||||
|
// `prev_slot`/`next_slot`, which already check existence -- but
|
||||||
|
// that invariant is enforced by convention across three call
|
||||||
|
// sites, not by this function, which would otherwise fail with a
|
||||||
|
// bare "index out of bounds" and no context (docs/
|
||||||
|
// REVIEW-2026-09-06.md finding 2). `slot_widget`, called from
|
||||||
|
// here, is what actually indexes/`.expect`s on it.
|
||||||
|
debug_assert!(
|
||||||
|
self.slot_exists(slot),
|
||||||
|
"place() called with a slot that doesn't exist: {slot:?}"
|
||||||
|
);
|
||||||
let axis = self.axis;
|
let axis = self.axis;
|
||||||
let output_len = painter.output_size().axis(axis);
|
let output_len = painter.output_size().axis(axis);
|
||||||
let container_len = painter.region().axis(axis).len();
|
let container_len = painter.region().axis(axis).len();
|
||||||
@@ -1273,6 +1305,52 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Neither `replacing_the_last_row_stays_pinned_to_the_bottom` nor
|
||||||
|
/// its sibling below ever asserts the *evicted* key's own bookkeeping
|
||||||
|
/// is actually gone -- both replace row 4 with another row also keyed
|
||||||
|
/// `4`, so `heights.remove(&old.key)` removing and re-inserting the
|
||||||
|
/// same key would pass either test even if it did nothing (docs/
|
||||||
|
/// REVIEW-2026-09-06.md finding 10; this is `Selection`'s finding 1
|
||||||
|
/// class of bug -- a stale handle outliving what it points to --
|
||||||
|
/// production-tested from `List`'s own side). Replacing with a
|
||||||
|
/// **different** key is what actually exercises the removal.
|
||||||
|
#[test]
|
||||||
|
fn replace_back_forgets_the_evicted_keys_own_height() {
|
||||||
|
let mut rsc = TestRsc {
|
||||||
|
ui: UiData::default(),
|
||||||
|
};
|
||||||
|
let mut list = List::new(Axis::Y);
|
||||||
|
push_rows(&mut rsc, &mut list, &[0, 1, 2, 3, 4], 20.0);
|
||||||
|
let (list_weak, root) = add_list(&mut rsc, list);
|
||||||
|
|
||||||
|
let mut render = UiRenderState::new();
|
||||||
|
render.resize((100.0, 60.0));
|
||||||
|
render.update(&root, &mut rsc);
|
||||||
|
assert!(
|
||||||
|
rsc.ui
|
||||||
|
.widgets
|
||||||
|
.get(&list_weak)
|
||||||
|
.unwrap()
|
||||||
|
.heights
|
||||||
|
.contains_key(&4)
|
||||||
|
);
|
||||||
|
|
||||||
|
let (_weak, new_row) = fixed_row(&mut rsc, 40.0);
|
||||||
|
let old = rsc
|
||||||
|
.ui
|
||||||
|
.widgets
|
||||||
|
.get_mut(&list_weak)
|
||||||
|
.unwrap()
|
||||||
|
.replace_back(ListRow::new(100, new_row));
|
||||||
|
|
||||||
|
let list_ref = rsc.ui.widgets.get(&list_weak).unwrap();
|
||||||
|
assert_eq!(old.map(|o| o.key), Some(4));
|
||||||
|
assert!(
|
||||||
|
!list_ref.heights.contains_key(&4),
|
||||||
|
"the evicted key's cached height must not outlive the row it measured"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/// The other half of the same fix's contract: replacing a row that is
|
/// The other half of the same fix's contract: replacing a row that is
|
||||||
/// *not* on screen must not move anything that is. `replace_back` only
|
/// *not* on screen must not move anything that is. `replace_back` only
|
||||||
/// touches the last slot's own widget and this file's own `heights`/
|
/// touches the last slot's own widget and this file's own `heights`/
|
||||||
@@ -1332,6 +1410,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
|
/// Enough rows, tall enough, that a fling toward the start has real
|
||||||
/// room to travel before `at_start` clamps it -- shared by the fling
|
/// room to travel before `at_start` clamps it -- shared by the fling
|
||||||
/// tests below.
|
/// tests below.
|
||||||
@@ -1409,6 +1544,71 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `fling_moves_the_list_and_then_settles`/
|
||||||
|
/// `fling_distance_is_positive_toward_the_end` only check that a fling
|
||||||
|
/// started, moved the right way and eventually stopped -- both
|
||||||
|
/// unaffected by *how* the interior ticks split up the total travel
|
||||||
|
/// (docs/REVIEW-2026-09-06.md finding 9). A regression that made
|
||||||
|
/// `tick_fling` apply the whole spline distance every tick instead of
|
||||||
|
/// just this tick's incremental slice would still pass both, while
|
||||||
|
/// being wildly wrong every intermediate frame -- this pins the
|
||||||
|
/// per-tick delta to a decelerating curve (`FlingCalculator::
|
||||||
|
/// position_at`'s own monotonic-and-clamped property, one level
|
||||||
|
/// down, already covers the calculator alone; this is the same
|
||||||
|
/// property through `List::tick_fling`'s `scroll`/`extents`
|
||||||
|
/// accumulation).
|
||||||
|
#[test]
|
||||||
|
fn tick_fling_applies_shrinking_incremental_deltas() {
|
||||||
|
let mut rsc = TestRsc {
|
||||||
|
ui: UiData::default(),
|
||||||
|
};
|
||||||
|
let (list_weak, root, mut render) = build_flingable_list(&mut rsc);
|
||||||
|
rsc.ui.widgets.get_mut(&list_weak).unwrap().jump_to_start();
|
||||||
|
render.update(&root, &mut rsc);
|
||||||
|
|
||||||
|
rsc.ui.widgets.get_mut(&list_weak).unwrap().fling(8000.0);
|
||||||
|
let start = Instant::now();
|
||||||
|
let mut prev_top = rsc.ui.widgets.get(&list_weak).unwrap().extents[&0].top;
|
||||||
|
let mut deltas = Vec::new();
|
||||||
|
for step in 1..600 {
|
||||||
|
let now = start + std::time::Duration::from_millis(step * 16);
|
||||||
|
let still = rsc.ui.widgets.get_mut(&list_weak).unwrap().tick_fling(now);
|
||||||
|
render.update(&root, &mut rsc);
|
||||||
|
let Some(top) = rsc
|
||||||
|
.ui
|
||||||
|
.widgets
|
||||||
|
.get(&list_weak)
|
||||||
|
.unwrap()
|
||||||
|
.extents
|
||||||
|
.get(&0)
|
||||||
|
.map(|e| e.top)
|
||||||
|
else {
|
||||||
|
break; // row 0 scrolled out of the loaded extents
|
||||||
|
};
|
||||||
|
deltas.push((prev_top - top).abs());
|
||||||
|
prev_top = top;
|
||||||
|
if !still {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
assert!(
|
||||||
|
deltas.len() >= 3,
|
||||||
|
"fling settled or left row 0's extent before collecting enough samples"
|
||||||
|
);
|
||||||
|
// Skip the first tick (the slop-transition jump the arbiter
|
||||||
|
// applies is a `List::fling`-adjacent concern, not this curve,
|
||||||
|
// but the very first frame can still carry rounding noise from
|
||||||
|
// `jump_to_start`'s own layout settling).
|
||||||
|
for w in deltas[1..].windows(2) {
|
||||||
|
assert!(
|
||||||
|
w[1] <= w[0] + 0.01,
|
||||||
|
"fling's per-tick delta grew instead of decelerating: {:?} then {:?}",
|
||||||
|
w[0],
|
||||||
|
w[1]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn cancel_fling_stops_it_with_no_further_movement() {
|
fn cancel_fling_stops_it_with_no_further_movement() {
|
||||||
let mut rsc = TestRsc {
|
let mut rsc = TestRsc {
|
||||||
|
|||||||
@@ -32,6 +32,15 @@ pub struct TextEdit {
|
|||||||
#[cfg_attr(target_os = "android", allow(dead_code))]
|
#[cfg_attr(target_os = "android", allow(dead_code))]
|
||||||
history: Vec<(String, Option<Selection>)>,
|
history: Vec<(String, Option<Selection>)>,
|
||||||
double_hit: Option<usize>,
|
double_hit: Option<usize>,
|
||||||
|
/// Where an in-flight press over this field began, while it is still
|
||||||
|
/// undecided whether the gesture is a tap (focus/show the IME) or a
|
||||||
|
/// drag (attr.rs's `Selector`/`Selectable`, Iris 2026-09-06: a swipe
|
||||||
|
/// over the composer must not summon the keyboard). `None` both before
|
||||||
|
/// any press and once the gesture has been decided either way --
|
||||||
|
/// `attr.rs` is the only reader/writer, kept `pub(crate)` rather than
|
||||||
|
/// behind an accessor since it is pure bookkeeping with no invariant
|
||||||
|
/// beyond "some press is undecided," same shape as `double_hit` above.
|
||||||
|
pub(crate) press_origin: Option<Vec2>,
|
||||||
pub mode: EditMode,
|
pub mode: EditMode,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -48,6 +57,7 @@ impl TextEdit {
|
|||||||
selection: None,
|
selection: None,
|
||||||
history: Default::default(),
|
history: Default::default(),
|
||||||
double_hit: None,
|
double_hit: None,
|
||||||
|
press_origin: None,
|
||||||
mode,
|
mode,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -698,6 +708,67 @@ mod tests {
|
|||||||
assert_eq!(content(&t), "に");
|
assert_eq!(content(&t), "に");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `android/ime.rs`'s `set_composing_text` calls `replace` and expects
|
||||||
|
/// the caret to land right after the inserted text, growing with it on
|
||||||
|
/// every re-send -- the buffer-level half of RUST.md's P0 box ("doesn't
|
||||||
|
/// enter it until I hit space, and also doesn't move cursor forward").
|
||||||
|
#[test]
|
||||||
|
fn composing_advances_the_caret_with_the_growing_text() {
|
||||||
|
let (mut t, mut d) = edit("", EditMode::SingleLine);
|
||||||
|
ctx(&mut t, &mut d).set_caret(0);
|
||||||
|
ctx(&mut t, &mut d).replace(0, "h");
|
||||||
|
assert_eq!(t.caret(), Some(1));
|
||||||
|
ctx(&mut t, &mut d).replace(1, "hi");
|
||||||
|
assert_eq!(content(&t), "hi");
|
||||||
|
assert_eq!(t.caret(), Some(2));
|
||||||
|
ctx(&mut t, &mut d).replace(2, "hit");
|
||||||
|
assert_eq!(content(&t), "hit");
|
||||||
|
assert_eq!(t.caret(), Some(3));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The IME's `commitText` (`android_view::InputConnection::commit_text`'s
|
||||||
|
/// default body): finish a composition in place, same as a real word
|
||||||
|
/// boundary (a space) landing after Gboard's composing span.
|
||||||
|
#[test]
|
||||||
|
fn committing_composed_text_leaves_it_in_place_with_the_caret_after_it() {
|
||||||
|
let (mut t, mut d) = edit("say ", EditMode::SingleLine);
|
||||||
|
ctx(&mut t, &mut d).set_caret(4);
|
||||||
|
ctx(&mut t, &mut d).replace(0, "hi");
|
||||||
|
assert_eq!(content(&t), "say hi");
|
||||||
|
// `finish_composing_text`/`commit_text` do not themselves touch the
|
||||||
|
// buffer -- only the IME's own `compose_len` bookkeeping resets, in
|
||||||
|
// `android/ime.rs`. Confirms the buffer already holds committed
|
||||||
|
// text as plain, uncomposed content: a further `replace(0, " ")`
|
||||||
|
// (the space that ends the word) appends rather than overwriting.
|
||||||
|
ctx(&mut t, &mut d).replace(0, " ");
|
||||||
|
assert_eq!(content(&t), "say hi ");
|
||||||
|
assert_eq!(t.caret(), Some(7));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// `TextEditCtx::delete_byte_range` is `deleteSurroundingText`'s entry
|
||||||
|
/// point once `android/ime.rs` has converted UTF-16 code units to
|
||||||
|
/// bytes -- exercised directly here in bytes, since the UTF-16 math
|
||||||
|
/// itself is `android/ime.rs`'s own `byte_to_utf16`/`utf16_to_byte`,
|
||||||
|
/// outside this widget-only test module.
|
||||||
|
#[test]
|
||||||
|
fn delete_byte_range_removes_exactly_that_range() {
|
||||||
|
let (mut t, mut d) = edit("hello world", EditMode::SingleLine);
|
||||||
|
ctx(&mut t, &mut d).delete_byte_range(5, 11);
|
||||||
|
assert_eq!(content(&t), "hello");
|
||||||
|
assert_eq!(t.caret(), Some(5));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// `set_cursor_byte` is `setSelection`'s entry point -- collapses to a
|
||||||
|
/// caret at the given byte offset regardless of any span that was there.
|
||||||
|
#[test]
|
||||||
|
fn set_cursor_byte_collapses_to_a_caret_there() {
|
||||||
|
let (mut t, mut d) = edit("hello world", EditMode::SingleLine);
|
||||||
|
ctx(&mut t, &mut d).select_all();
|
||||||
|
ctx(&mut t, &mut d).set_cursor_byte(5);
|
||||||
|
assert_eq!(t.selected_text(), None);
|
||||||
|
assert_eq!(t.caret(), Some(5));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn motion_moves_the_caret_and_shift_extends_a_span() {
|
fn motion_moves_the_caret_and_shift_extends_a_span() {
|
||||||
let (mut t, mut d) = edit("abc", EditMode::SingleLine);
|
let (mut t, mut d) = edit("abc", EditMode::SingleLine);
|
||||||
|
|||||||
@@ -8,14 +8,58 @@
|
|||||||
//! measures whatever vertical space is left each frame -- nothing here
|
//! measures whatever vertical space is left each frame -- nothing here
|
||||||
//! computes a height by hand, and growing this field is exactly the
|
//! computes a height by hand, and growing this field is exactly the
|
||||||
//! O(1)-move-chain case LAYOUT.md and I3's benchmark already measured.
|
//! O(1)-move-chain case LAYOUT.md and I3's benchmark already measured.
|
||||||
|
//!
|
||||||
|
//! **Rebuilt 2026-09-06** (Iris's phone report on the dc01f88 build: the
|
||||||
|
//! grey bar drawn as a short, fixed strip with the typed text ~150px below
|
||||||
|
//! it on black, and empty black between the bar and the keyboard). One
|
||||||
|
//! widget now, top to bottom: an opaque background sized to its content
|
||||||
|
//! (`.background`, the same `Stack` idiom the header row's `HEADER_SURFACE`
|
||||||
|
//! already uses), the field inside `dp` padding and capped at
|
||||||
|
//! [`MAX_LINES`] before it scrolls instead of growing forever, and an
|
||||||
|
//! outer [`Pad`] whose `bottom` [`TranscriptScreen::set_bottom_inset`]
|
||||||
|
//! rewrites in place whenever the keyboard opens/closes -- never rebuilt,
|
||||||
|
//! since `field` is strongly owned inside this tree and this crate's
|
||||||
|
//! widgets cannot be re-parented once added (this module's own comment
|
||||||
|
//! below on why `build_composer` hands back a **weak** id).
|
||||||
|
|
||||||
use iris::prelude::*;
|
use iris::prelude::*;
|
||||||
|
|
||||||
|
/// Caps the field's growth at roughly six lines of its own 18px text
|
||||||
|
/// before it scrolls instead of consuming the whole screen -- an
|
||||||
|
/// approximation (line-height and padding folded into one round `dp`
|
||||||
|
/// number) rather than a value derived from the font's real metrics,
|
||||||
|
/// which nothing in this crate exposes to a caller today.
|
||||||
|
const MAX_LINES: f32 = 6.0;
|
||||||
|
const APPROX_LINE_HEIGHT_DP: f32 = 24.0;
|
||||||
|
const FIELD_PAD_DP: f32 = 12.0;
|
||||||
|
|
||||||
/// `field` is exposed so the caller can read its content on submit
|
/// `field` is exposed so the caller can read its content on submit
|
||||||
/// (`field.edit(rsc).text()`) and clear it afterward
|
/// (`field.edit(rsc).text()`) and clear it afterward
|
||||||
/// (`field.edit(rsc).set("")`).
|
/// (`field.edit(rsc).set("")`).
|
||||||
pub struct Composer {
|
pub struct Composer {
|
||||||
pub field: WeakWidget<TextEdit>,
|
pub field: WeakWidget<TextEdit>,
|
||||||
|
/// The bar's own outer padding -- only `bottom` is ever changed, by
|
||||||
|
/// [`Self::set_bottom_inset`]. A `Pad` around the whole bar rather than
|
||||||
|
/// a rebuilt tree, because `field` lives inside it and cannot be
|
||||||
|
/// re-added to a new wrapper once it is strongly owned here.
|
||||||
|
outer_pad: WeakWidget<Pad>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl Composer {
|
||||||
|
/// Called by the platform shell (Android's `on_insets_changed`, e.g.)
|
||||||
|
/// whenever the space below the bar changes: the IME's own inset while
|
||||||
|
/// it is open, the navigation-bar inset otherwise. Takes a plain
|
||||||
|
/// `f32` in the caller's own physical-pixel units rather than an
|
||||||
|
/// Android-specific insets type, so this crate stays usable from the
|
||||||
|
/// winit backend too, which has no navigation bar to report.
|
||||||
|
/// Rewrites the existing `Pad` in place (marking it dirty through the
|
||||||
|
/// ordinary `Widgets::get_mut` path) instead of swapping in a new one,
|
||||||
|
/// so the field's focus, selection and in-progress text are untouched.
|
||||||
|
pub fn set_bottom_inset(&self, rsc: &mut impl UiRsc, inset: f32) {
|
||||||
|
if let Some(pad) = rsc.ui_mut().widgets.get_mut(&self.outer_pad) {
|
||||||
|
pad.padding.bottom = Len::abs(inset);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Returns the composer plus its own bar as a **weak** id -- the caller
|
/// Returns the composer plus its own bar as a **weak** id -- the caller
|
||||||
@@ -39,10 +83,20 @@ where
|
|||||||
.label("Message")
|
.label("Message")
|
||||||
.add(rsc);
|
.add(rsc);
|
||||||
|
|
||||||
let bar: WeakWidget = (field.pad(dp(12)).width(rest(1)),)
|
// One widget: an opaque bar sized to its own content (`.background`'s
|
||||||
.span(Dir::RIGHT)
|
// `Stack{child: 1}`, the header row's own idiom) wrapping the padded,
|
||||||
|
// height-capped field -- not a background rect and a field drawn as
|
||||||
|
// two independent siblings, which is what let the two disagree on
|
||||||
|
// where the bar actually was.
|
||||||
|
let content = field
|
||||||
|
.pad(dp(FIELD_PAD_DP))
|
||||||
|
.max_height(dp(APPROX_LINE_HEIGHT_DP * MAX_LINES + FIELD_PAD_DP * 2.0))
|
||||||
|
.scrollable()
|
||||||
|
.width(rest(1))
|
||||||
.background(rect(UiColor::new(40, 40, 46, 255)))
|
.background(rect(UiColor::new(40, 40, 46, 255)))
|
||||||
.add(rsc);
|
.add(rsc);
|
||||||
|
|
||||||
(Composer { field }, bar)
|
let outer_pad: WeakWidget<Pad> = content.pad(Padding::ZERO).add(rsc);
|
||||||
|
|
||||||
|
(Composer { field, outer_pad }, outer_pad)
|
||||||
}
|
}
|
||||||
@@ -51,7 +51,7 @@ pub mod selection;
|
|||||||
use client_core::transcript_fold::TranscriptRow as FoldedRow;
|
use client_core::transcript_fold::TranscriptRow as FoldedRow;
|
||||||
use iris::prelude::*;
|
use iris::prelude::*;
|
||||||
use selection::Selection;
|
use selection::Selection;
|
||||||
use std::{cell::RefCell, rc::Rc};
|
use std::{cell::RefCell, rc::Rc, time::Instant};
|
||||||
|
|
||||||
pub struct TranscriptScreen {
|
pub struct TranscriptScreen {
|
||||||
/// The transcript's own `List` -- exposed so a caller can read
|
/// The transcript's own `List` -- exposed so a caller can read
|
||||||
@@ -151,8 +151,16 @@ impl TranscriptScreen {
|
|||||||
}
|
}
|
||||||
RowDiff::Rebuild => {
|
RowDiff::Rebuild => {
|
||||||
// A row before the tail changed (a regroup) -- nothing
|
// A row before the tail changed (a regroup) -- nothing
|
||||||
// short of a full rebuild expresses that.
|
// short of a full rebuild expresses that. `Selection`
|
||||||
|
// gets cleared the same way `List` does, right before the
|
||||||
|
// rows it was pointing at go with it -- `push_row` below
|
||||||
|
// re-`register`s whatever survives as it rebuilds each
|
||||||
|
// row (docs/REVIEW-2026-09-06.md finding 1: a key that
|
||||||
|
// `group_tool_runs` regrouped away used to stay in
|
||||||
|
// `Selection` pointing at a widget this `clear()` had
|
||||||
|
// just freed, panicking the next long-press anywhere).
|
||||||
self.rebuilds.set(self.rebuilds.get() + 1);
|
self.rebuilds.set(self.rebuilds.get() + 1);
|
||||||
|
self.selection.borrow_mut().clear();
|
||||||
(self.list)(rsc).clear();
|
(self.list)(rsc).clear();
|
||||||
for row in &new_rows {
|
for row in &new_rows {
|
||||||
self.push_row(rsc, row);
|
self.push_row(rsc, row);
|
||||||
@@ -220,6 +228,43 @@ where
|
|||||||
})
|
})
|
||||||
.add(rsc);
|
.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 (composer, composer_bar) = composer::build_composer(rsc);
|
||||||
|
|
||||||
let tree = (list.width(rest(1)).height(rest(1)), composer_bar)
|
let tree = (list.width(rest(1)).height(rest(1)), composer_bar)
|
||||||
@@ -377,3 +422,124 @@ mod diff_tests {
|
|||||||
assert_eq!(diff_rows(&old, &new), RowDiff::Rebuild);
|
assert_eq!(diff_rows(&old, &new), RowDiff::Rebuild);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Exercises `TranscriptScreen::apply`'s `Rebuild` arm through a real
|
||||||
|
/// `Selection`, the gap docs/REVIEW-2026-09-06.md finding 8 named: the
|
||||||
|
/// pure `diff_rows` decision above and `selection.rs`'s own registration
|
||||||
|
/// tests each pass in isolation, and neither alone catches finding 1 (a
|
||||||
|
/// regrouped-away row's key surviving in `Selection` after `List::clear()`
|
||||||
|
/// has already freed its widget). This fails before `Selection::clear()`
|
||||||
|
/// existed and the `Rebuild` arm called it, with a panic from
|
||||||
|
/// `TextEditable::edit` resolving the freed slot.
|
||||||
|
#[cfg(test)]
|
||||||
|
mod apply_tests {
|
||||||
|
use super::*;
|
||||||
|
use client_core::transcript_fold::TranscriptItem;
|
||||||
|
|
||||||
|
struct TestFocus {
|
||||||
|
focus: Option<WeakWidget<TextEdit>>,
|
||||||
|
}
|
||||||
|
impl FocusHost for TestFocus {
|
||||||
|
fn recent_click(&mut self) -> bool {
|
||||||
|
false
|
||||||
|
}
|
||||||
|
fn set_focus(&mut self, id: Option<WeakWidget<TextEdit>>) {
|
||||||
|
self.focus = id;
|
||||||
|
}
|
||||||
|
fn focus_gained(&mut self, _region: Option<PixelRegion>) {}
|
||||||
|
fn is_focused(&self, id: WeakWidget<TextEdit>) -> bool {
|
||||||
|
self.focus == Some(id)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
struct TestRsc {
|
||||||
|
ui: UiData,
|
||||||
|
events: EventManager<TestRsc>,
|
||||||
|
}
|
||||||
|
impl UiRsc for TestRsc {
|
||||||
|
fn ui(&self) -> &UiData {
|
||||||
|
&self.ui
|
||||||
|
}
|
||||||
|
fn ui_mut(&mut self) -> &mut UiData {
|
||||||
|
&mut self.ui
|
||||||
|
}
|
||||||
|
fn on_draw(&mut self, active: &ActiveData) {
|
||||||
|
self.events.draw(active);
|
||||||
|
}
|
||||||
|
fn on_undraw(&mut self, active: &ActiveData) {
|
||||||
|
self.events.undraw(active);
|
||||||
|
}
|
||||||
|
fn on_remove(&mut self, id: WidgetId) {
|
||||||
|
self.events.remove(id);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
impl HasState for TestRsc {
|
||||||
|
type State = TestFocus;
|
||||||
|
}
|
||||||
|
impl HasEvents for TestRsc {
|
||||||
|
fn events(&self) -> &EventManager<Self> {
|
||||||
|
&self.events
|
||||||
|
}
|
||||||
|
fn events_mut(&mut self) -> &mut EventManager<Self> {
|
||||||
|
&mut self.events
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn user(seq: u64, text: &str) -> TranscriptItem {
|
||||||
|
TranscriptItem::UserMsg {
|
||||||
|
seq,
|
||||||
|
text: text.to_string(),
|
||||||
|
attachments: Vec::new(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn tool(seq: u64, run_id: &str) -> TranscriptItem {
|
||||||
|
TranscriptItem::ToolRun {
|
||||||
|
seq,
|
||||||
|
id: format!("id{seq}"),
|
||||||
|
run_id: run_id.to_string(),
|
||||||
|
tool: "grep".to_string(),
|
||||||
|
input: "x".to_string(),
|
||||||
|
output: String::new(),
|
||||||
|
done: false,
|
||||||
|
asks: Vec::new(),
|
||||||
|
images: Vec::new(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_row_dropped_by_a_regroup_does_not_outlive_itself_in_selection() {
|
||||||
|
use client_core::transcript_fold::group_tool_runs;
|
||||||
|
|
||||||
|
let mut rsc = TestRsc {
|
||||||
|
ui: UiData::default(),
|
||||||
|
events: EventManager::default(),
|
||||||
|
};
|
||||||
|
|
||||||
|
// Same regroup shape as diff_tests' regroup case, plus a trailing
|
||||||
|
// row (seq 4) that survives unchanged -- what a reader would tap
|
||||||
|
// on right after the regroup lands.
|
||||||
|
let old_items = vec![tool(1, "run-a"), user(2, "meanwhile"), user(4, "stable")];
|
||||||
|
let new_items = vec![tool(1, "run-a"), tool(3, "run-a"), user(4, "stable")];
|
||||||
|
assert_eq!(
|
||||||
|
diff_rows(&group_tool_runs(&old_items), &group_tool_runs(&new_items)),
|
||||||
|
RowDiff::Rebuild,
|
||||||
|
"test setup must actually exercise the Rebuild arm"
|
||||||
|
);
|
||||||
|
|
||||||
|
let (screen, _tree) = build_tree(&mut rsc, group_tool_runs(&old_items));
|
||||||
|
screen.apply(&mut rsc, &old_items, &new_items);
|
||||||
|
|
||||||
|
// The surviving row (seq 4) is what a reader's long-press would
|
||||||
|
// land on; `begin` deselects every *other* registered row first,
|
||||||
|
// which is exactly what used to resolve a stale `WeakWidget` left
|
||||||
|
// by the regrouped-away rows and panic.
|
||||||
|
let surviving_key = row::row_key(&client_core::transcript_fold::ItemKey::Seq(4));
|
||||||
|
screen.selection.borrow_mut().begin(
|
||||||
|
&mut rsc,
|
||||||
|
surviving_key,
|
||||||
|
Vec2::ZERO,
|
||||||
|
Vec2::new(10.0, 10.0),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -137,20 +137,26 @@ where
|
|||||||
|
|
||||||
field
|
field
|
||||||
// `| CursorSense::unclick()` on top of the usual click-or-drag set
|
// `| CursorSense::unclick()` on top of the usual click-or-drag set
|
||||||
// -- the arbiter inside `Selection::drag` needs the release too,
|
// -- this row's own registration only ever needs to see a
|
||||||
// to go back to idle for the next press (`DragArbiter::release`).
|
// 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(
|
.on(
|
||||||
CursorSense::click_or_drag() | CursorSense::unclick(),
|
CursorSense::click_or_drag() | CursorSense::unclick(),
|
||||||
move |ctx, rsc| {
|
move |ctx, rsc| {
|
||||||
selection.borrow_mut().drag(
|
selection.borrow_mut().drag(
|
||||||
rsc,
|
rsc,
|
||||||
list,
|
list,
|
||||||
key,
|
Some((key, ctx.data.pos, ctx.data.size)),
|
||||||
ctx.data.pos,
|
|
||||||
ctx.data.size,
|
|
||||||
ctx.data.cursor.pos,
|
ctx.data.cursor.pos,
|
||||||
ctx.data.sense,
|
ctx.data.sense,
|
||||||
Instant::now(),
|
Instant::now(),
|
||||||
|
ctx.data.render,
|
||||||
);
|
);
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -36,17 +36,15 @@ use std::{collections::BTreeMap, time::Instant};
|
|||||||
pub struct Selection {
|
pub struct Selection {
|
||||||
rows: BTreeMap<RowKey, WeakWidget<TextEdit>>,
|
rows: BTreeMap<RowKey, WeakWidget<TextEdit>>,
|
||||||
anchor: Option<(RowKey, Vec2)>,
|
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
|
/// gesture conflict (a row's own `click_or_drag()` and a list-level
|
||||||
/// pan wanting the same touch gesture). See `drag` below, and
|
/// pan wanting the same touch gesture). See `drag` below, and
|
||||||
/// `iris::sense::DragArbiter`'s own doc for the decision itself.
|
/// `iris::sense::DragGesture`'s own doc for the arbitration, velocity
|
||||||
arbiter: DragArbiter,
|
/// tracking and pointer-capture mechanics this no longer owns itself
|
||||||
/// Tracks the last ~100ms of this gesture's pan deltas (in the same
|
/// -- Iris's 2026-09-06 ask (`IRIS.md`) that a drag's *mechanics* live
|
||||||
/// signed units `list.scroll` takes), so a release that turns out to
|
/// in iris's default input layer, with only the pan-vs-select
|
||||||
/// have been panning can hand `List::fling` a realistic initial
|
/// *decision* staying here.
|
||||||
/// velocity instead of one frame's noisy last delta --
|
gesture: DragGesture,
|
||||||
/// IRIS_TODO.md's "swiping has no momentum."
|
|
||||||
velocity: VelocityTracker,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Default for Selection {
|
impl Default for Selection {
|
||||||
@@ -60,19 +58,37 @@ impl Selection {
|
|||||||
Self {
|
Self {
|
||||||
rows: BTreeMap::new(),
|
rows: BTreeMap::new(),
|
||||||
anchor: None,
|
anchor: None,
|
||||||
arbiter: DragArbiter::new(),
|
gesture: DragGesture::new(),
|
||||||
velocity: VelocityTracker::new(),
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A row's selectable text became visible/known. Every addition here
|
/// A row's selectable text became visible/known. Every addition here
|
||||||
/// needs its removal (`unregister`) -- called when `List` evicts the
|
/// needs its removal (`unregister`, or `clear` for all of them at
|
||||||
/// row (`pop_front`/`pop_back`), so this map never outgrows however
|
/// once) -- called when `List` evicts the row (`pop_front`/
|
||||||
/// many rows are actually loaded.
|
/// `pop_back`/`clear`), so this map never outgrows however many rows
|
||||||
|
/// are actually loaded. `List::place` guards the twin of this same
|
||||||
|
/// class of bug on the list's own side (`list.rs`'s `slot_exists`
|
||||||
|
/// assertion) -- a derived handle that silently outlives what it
|
||||||
|
/// points to; the next caller adding a third row-keyed side table
|
||||||
|
/// should read both.
|
||||||
pub fn register(&mut self, key: RowKey, text: WeakWidget<TextEdit>) {
|
pub fn register(&mut self, key: RowKey, text: WeakWidget<TextEdit>) {
|
||||||
self.rows.insert(key, text);
|
self.rows.insert(key, text);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Drops every registration at once -- the same shape `List::clear()`
|
||||||
|
/// clears the list, and what `TranscriptScreen::apply`'s `Rebuild` arm
|
||||||
|
/// calls right before it, since a full rebuild drops every row's old
|
||||||
|
/// widget and `push_row` re-`register`s each surviving key's new one
|
||||||
|
/// as it goes (review docs/REVIEW-2026-09-06.md finding 1: the
|
||||||
|
/// `Rebuild` arm used to call only `List::clear()`, leaving any key
|
||||||
|
/// dropped by the regroup -- present in the old rows, absent from the
|
||||||
|
/// new ones -- pointing at a widget the list had just freed, so the
|
||||||
|
/// next long-press anywhere panicked in `begin`'s deselect loop).
|
||||||
|
pub fn clear(&mut self) {
|
||||||
|
self.rows.clear();
|
||||||
|
self.anchor = None;
|
||||||
|
}
|
||||||
|
|
||||||
pub fn unregister(&mut self, key: RowKey) {
|
pub fn unregister(&mut self, key: RowKey) {
|
||||||
self.rows.remove(&key);
|
self.rows.remove(&key);
|
||||||
if self.anchor.map(|(k, _)| k) == Some(key) {
|
if self.anchor.map(|(k, _)| k) == Some(key) {
|
||||||
@@ -165,85 +181,66 @@ impl Selection {
|
|||||||
/// row, is what makes that consistent as a drag crosses row
|
/// row, is what makes that consistent as a drag crosses row
|
||||||
/// boundaries).
|
/// 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
|
/// `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
|
/// 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)]
|
#[allow(clippy::too_many_arguments)]
|
||||||
pub fn drag(
|
pub fn drag(
|
||||||
&mut self,
|
&mut self,
|
||||||
ui: &mut impl UiRsc,
|
ui: &mut impl UiRsc,
|
||||||
list: WeakWidget<List>,
|
list: WeakWidget<List>,
|
||||||
key: RowKey,
|
row: Option<(RowKey, Vec2, Vec2)>,
|
||||||
pos_row: Vec2,
|
|
||||||
size: Vec2,
|
|
||||||
pos_window: Vec2,
|
pos_window: Vec2,
|
||||||
sense: CursorSense,
|
sense: CursorSense,
|
||||||
now: Instant,
|
now: Instant,
|
||||||
|
render: &UiRenderState,
|
||||||
) {
|
) {
|
||||||
let outcome = match sense {
|
if matches!(sense, CursorSense::PressStart(_)) {
|
||||||
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
|
// A fresh touch-down cancels any fling still coasting from
|
||||||
// the previous gesture -- `List::fling`'s own doc, and
|
// the previous gesture -- `List::fling`'s own doc, and
|
||||||
// Android's `Scroller::abortAnimation` for the same reason.
|
// Android's `Scroller::abortAnimation` for the same reason.
|
||||||
list(ui).cancel_fling();
|
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);
|
let already_selected = self.has_selection(ui);
|
||||||
self.arbiter.press_start(pos_window, now, already_selected);
|
let outcome =
|
||||||
self.velocity.reset();
|
self.gesture
|
||||||
list(ui).cancel_fling();
|
.handle(render, list.id(), sense, pos_window, now, already_selected);
|
||||||
self.arbiter.update(pos_window, now)
|
|
||||||
}
|
|
||||||
_ => self.arbiter.update(pos_window, now),
|
|
||||||
};
|
|
||||||
match outcome {
|
match outcome {
|
||||||
DragOutcome::Undecided => {}
|
GestureOutcome::Undecided => {}
|
||||||
DragOutcome::Pan(dy) => {
|
GestureOutcome::Pan(dy) => list(ui).scroll(-dy),
|
||||||
let amt = -dy;
|
GestureOutcome::SelectStart => {
|
||||||
self.velocity.add_sample(amt, now);
|
if let Some((key, pos_row, size)) = row {
|
||||||
list(ui).scroll(amt);
|
// Grep-able on "iris selection" the way the frame
|
||||||
}
|
// report is on "iris frame report" -- selection has no
|
||||||
DragOutcome::SelectStart => {
|
// accessibility label of its own yet, so this is the
|
||||||
// Grep-able on "iris selection" the way the frame report is
|
// smallest way to confirm a real on-device long-
|
||||||
// on "iris frame report" -- selection has no accessibility
|
// press-then-drag actually reached here (RUST.md's I5
|
||||||
// label of its own yet, so this is the smallest way to
|
// box, "Measurements taken" (c)).
|
||||||
// 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:?}");
|
log::info!("iris selection: begin at row {key:?}");
|
||||||
self.begin(ui, key, pos_row, size);
|
self.begin(ui, key, pos_row, size);
|
||||||
}
|
}
|
||||||
DragOutcome::SelectExtend => {
|
}
|
||||||
|
GestureOutcome::SelectExtend => {
|
||||||
|
if let Some((key, pos_row, size)) = row {
|
||||||
log::info!("iris selection: extend to row {key:?}");
|
log::info!("iris selection: extend to row {key:?}");
|
||||||
self.extend(ui, key, pos_row, size);
|
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) => {}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The concatenated selected text, in row order, `None` if nothing is
|
/// The concatenated selected text, in row order, `None` if nothing is
|
||||||
@@ -344,8 +341,9 @@ mod tests {
|
|||||||
|
|
||||||
let mut sel = Selection::new();
|
let mut sel = Selection::new();
|
||||||
sel.register(1, field);
|
sel.register(1, field);
|
||||||
assert!(sel.arbiter.is_idle());
|
assert!(sel.gesture.is_idle());
|
||||||
|
|
||||||
|
let render = UiRenderState::new();
|
||||||
let now = Instant::now();
|
let now = Instant::now();
|
||||||
let size = Vec2::new(100.0, 20.0);
|
let size = Vec2::new(100.0, 20.0);
|
||||||
// No `PressStart` is ever sent -- only the `Pressing` frames a
|
// No `PressStart` is ever sent -- only the `Pressing` frames a
|
||||||
@@ -353,15 +351,14 @@ mod tests {
|
|||||||
sel.drag(
|
sel.drag(
|
||||||
&mut rsc,
|
&mut rsc,
|
||||||
list,
|
list,
|
||||||
1,
|
Some((1, Vec2::ZERO, size)),
|
||||||
Vec2::ZERO,
|
|
||||||
size,
|
|
||||||
Vec2::new(540.0, 700.0),
|
Vec2::new(540.0, 700.0),
|
||||||
CursorSense::Pressing(CursorButton::Left),
|
CursorSense::Pressing(CursorButton::Left),
|
||||||
now,
|
now,
|
||||||
|
&render,
|
||||||
);
|
);
|
||||||
assert!(
|
assert!(
|
||||||
!sel.arbiter.is_idle(),
|
!sel.gesture.is_idle(),
|
||||||
"a Pressing frame with the arbiter still Idle must recover \
|
"a Pressing frame with the arbiter still Idle must recover \
|
||||||
the press rather than leaving it stuck"
|
the press rather than leaving it stuck"
|
||||||
);
|
);
|
||||||
|
|||||||
Reference in new issue
Block a user