iris/android: the composer's launch position was the bench report pane, plus surface/insets lifecycle logging
The empty benchmark-report TextEdit held .height(rest(1)) beside content.height(rest(2)), so it reserved a third of the window at every launch and pushed the composer two thirds down -- Iris's 11:39 phone report. It is sized to its content now, capped and scrollable, and sits above the transcript rather than under the composer. New log::info! lines for one insets change, one surface_changed, one renderer build and one surface_destroyed, each with the glyph/atlas counts, so a phone's adb logcat can answer the app-switch text loss the emulator cannot reproduce.
This commit is contained in:
1 parent
2fed8b34b3
commit
d9872989fa
3 files changed
+82
-3
No files matched your search
+21
-1
@@ -189,8 +189,28 @@ 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
|
||||
- [x] **"The app definitely does not start with keyboard spacing
|
||||
correct. This is how it looks without me doing anything initially."**
|
||||
**Not an inset bug at all -- fixed 2026-09-06.** The black third is the
|
||||
bench shell's own empty *benchmark report* pane: `bench_client.rs`'s
|
||||
root tree gave it `.height(rest(1))` beside `content.height(rest(2))`,
|
||||
so an empty `TextEdit` reserved a third of the window at every launch
|
||||
and pushed the composer up by exactly that. Measured on this checkout's
|
||||
emulator at the phone's own size (1080x2424, density 420, gesture nav),
|
||||
which reproduced Iris's screenshot exactly: new `iris insets:` log line
|
||||
reported `bottom=63 ime_bottom=0` at launch (a nav bar, no keyboard --
|
||||
so the inset the composer was fed was never large), while `ui-trace
|
||||
show -m Message --field box` put the field at `31,1488..1048,1540` on a
|
||||
2282px-tall surface, 789px clear of the bottom -- that pane's third.
|
||||
**Unit mixing checked explicitly and cleared**: `set_bottom_inset` takes
|
||||
physical px and stores `Len::abs`, `MainActivity.java`'s `1`/`0`
|
||||
`ime_bottom` only ever reaches `insets.bottom.max(ime_bottom)` and
|
||||
`> 0.0`, and every `dp` in the composer resolves at layout time. Fix:
|
||||
the report pane is sized to its content (`.max_height(dp(260))
|
||||
.scrollable()`), and moved above the transcript so it cannot eat the
|
||||
composer's nav-bar clearance. After: field box `31,2277..1048,2329`,
|
||||
grey bar ending at device y2361 with the 63px nav strip below it
|
||||
(`/tmp/fix1.png` this pass).
|
||||
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
|
||||
|
||||
@@ -92,6 +92,12 @@ const ANIM_STEP_MS: u64 = 16;
|
||||
|
||||
const FIXTURE_JSONL: &str = include_str!("../../../app/bench-fixture/assets/transcript.jsonl");
|
||||
|
||||
/// How much of the screen a *filled* benchmark report may take before it
|
||||
/// scrolls instead of growing -- roughly a third of a phone screen, the
|
||||
/// share the pane used to reserve unconditionally. An empty report takes
|
||||
/// nothing at all; see `new`'s comment at the tree it is used in.
|
||||
const REPORT_MAX_HEIGHT_DP: f32 = 260.0;
|
||||
|
||||
pub struct BenchClient {
|
||||
ui_state: AndroidUiState,
|
||||
content: WeakWidget<WidgetPtr>,
|
||||
@@ -255,10 +261,29 @@ impl AndroidAppState for BenchClient {
|
||||
let top_bar = WidgetPtr::new().add(rsc);
|
||||
let controls = bench_controls(rsc, 0.0);
|
||||
top_bar(rsc).set(controls);
|
||||
// The report pane is sized to whatever report it is holding, not
|
||||
// to a share of the window: `rest(1)` here reserved a third of
|
||||
// the screen for an *empty* `TextEdit` at every launch, which is
|
||||
// what Iris's 2026-09-06 11:39 phone report described as "the app
|
||||
// does not start with keyboard spacing correct" -- the composer
|
||||
// two thirds down with black below it, nothing to do with the IME
|
||||
// inset (measured: `iris insets:` reports bottom=63 ime_bottom=0
|
||||
// at launch, while the `Message` field's own box sat 789px above
|
||||
// the bottom of a 2282px surface -- exactly this pane's third).
|
||||
// Capped and scrollable so a long report cannot take the screen
|
||||
// back over, the same idiom `composer.rs` uses for the field.
|
||||
// Above the transcript, not below it: the report is what the
|
||||
// header's own "Run benchmark" button produces (UI_RULES.md --
|
||||
// results appear where the action was started), and a pane under
|
||||
// the composer would eat the navigation-bar clearance
|
||||
// `set_bottom_inset` gives it.
|
||||
let tree = (
|
||||
top_bar,
|
||||
content.height(rest(2)),
|
||||
report_display.height(rest(1)).pad(dp(8)),
|
||||
report_display
|
||||
.pad(dp(8))
|
||||
.max_height(dp(REPORT_MAX_HEIGHT_DP))
|
||||
.scrollable(),
|
||||
content.height(rest(1)),
|
||||
)
|
||||
.span(Dir::DOWN)
|
||||
.add_strong(rsc)
|
||||
|
||||
@@ -368,6 +368,21 @@ impl<State: AndroidAppState> IrisViewPeer<State> {
|
||||
let current_insets = ui_state.insets();
|
||||
if current_insets != ui_state.last_insets {
|
||||
let physical = WindowInsets::from_physical(current_insets);
|
||||
// One line per real insets change. Iris's phone is the only
|
||||
// place several of these bugs reproduce and `adb logcat` is
|
||||
// the only instrument there (this-machine-android: system
|
||||
// tracing is broken on that device), so the numbers a layout
|
||||
// is actually fed have to reach the log -- "the composer
|
||||
// floats at launch" is unanswerable from a screenshot alone.
|
||||
log::info!(
|
||||
"iris insets: left={} top={} right={} bottom={} ime_bottom={} window={:?}",
|
||||
physical.left,
|
||||
physical.top,
|
||||
physical.right,
|
||||
physical.bottom,
|
||||
physical.ime_bottom,
|
||||
self.window_size(),
|
||||
);
|
||||
self.state.android_state_mut().last_insets = current_insets;
|
||||
self.state.on_insets_changed(&mut self.rsc, physical);
|
||||
}
|
||||
@@ -627,6 +642,12 @@ impl<State: AndroidAppState> ViewPeer for IrisViewPeer<State> {
|
||||
// backgrounding) still goes through `AndroidRenderer::new` below,
|
||||
// since `renderer` is `None` in that case.
|
||||
let already_live = self.state.android_state().renderer.is_some();
|
||||
log::info!(
|
||||
"iris surface: surface_changed {width}x{height} already_live={already_live} \
|
||||
glyphs_cached={} atlas_pages={}",
|
||||
self.rsc.ui.text.atlas.glyph_count(),
|
||||
self.rsc.ui.text.atlas.page_count(),
|
||||
);
|
||||
if already_live {
|
||||
let ui_state = self.state.android_state_mut();
|
||||
ui_state
|
||||
@@ -671,6 +692,13 @@ impl<State: AndroidAppState> ViewPeer for IrisViewPeer<State> {
|
||||
// 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.
|
||||
log::info!(
|
||||
"iris surface: new renderer built ({:?}), clearing glyph atlas: \
|
||||
glyphs={} pages={}",
|
||||
renderer.adapter_backend,
|
||||
self.rsc.ui.text.atlas.glyph_count(),
|
||||
self.rsc.ui.text.atlas.page_count(),
|
||||
);
|
||||
self.rsc.ui.text.atlas.clear();
|
||||
self.rsc.ui.textures.reset();
|
||||
self.state.android_state_mut().renderer = Some(renderer);
|
||||
@@ -707,6 +735,12 @@ impl<State: AndroidAppState> ViewPeer for IrisViewPeer<State> {
|
||||
_ctx: &mut CallbackCtx<'local>,
|
||||
_holder: &android_view::SurfaceHolder<'local>,
|
||||
) {
|
||||
log::info!(
|
||||
"iris surface: surface_destroyed, tearing the renderer down \
|
||||
(glyphs_cached={} atlas_pages={})",
|
||||
self.rsc.ui.text.atlas.glyph_count(),
|
||||
self.rsc.ui.text.atlas.page_count(),
|
||||
);
|
||||
self.state.android_state_mut().renderer = None;
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user