Greg Pomerantz
180fa966c8
Two feature bodies accumulated in the working tree:
1. Pinch to change the app font size, continuously (no snapping):
- internal/ui/pinch_tracker.go: logic-free touch state machine.
Two-mover formation (the resting palm can land first or last;
movement is the only signal valid for both), pair = the mover
pair whose distance changed most, baseline = press distance
(formDist), lazy pending releases, survivor-scroll forwarding
after a pair break. Robust to ~1 fps frames: a whole pinch can
land in one drain (formDist/brokeFactor/lazy releases).
- render.go: pinch probe (raw pointer events) + grab lifecycle so
the pair is exclusive (scroll sees nothing of the pair) and the
survivor's finger keeps working as a scroll after the pinch.
- state.go/logic.go/session.go/frame.go: app-local float font
scale, content-point pin (buffer byte + offset from baseline,
not a layout point, so rewrap keeps the same character under
the center), restore/font pins, session persistence.
- pinch_test.go, pinch_font_test.go, tag_identity_test.go,
real_draw_probe_test.go: unit + real-Renderer/real-Router tests.
2. Soft keyboard must not shift content:
- Root cause: gioui.org/app calls Router.RevealFocus on any frame
the viewport shrinks (IME open under adjustResize) and
synthesizes a pointer.Scroll nudge aimed at the focused field's
stale pre-resize bounds; gesture.Scroll consumed it -> a 32 dp
content jump.
- Fix: main.go flags the shrink frame; render.go drains that one
synthetic scroll for the gesture's tag before Update (scroll-
range clamping cannot work: the router UNIONs ranges across
frames). Finger scroll (pointer.Drag) and the flinger are
untouched. reveal_focus_drain_test.go reproduces RevealFocus at
the router level and verifies the drain + zero delta.
Also: tools/touchinject (platform-signed emulator multi-touch
injection harness + e2e script, adb has no two-finger input),
docs (spec 2.2 + development_plan 18-20), .gitignore, gofmt.
1346 lines
83 KiB
Markdown
1346 lines
83 KiB
Markdown
# Development Plan: reach a lean, usable Android text editor
|
||
|
||
Status: v11, 2026-08-17 (Phases 0–3 + doc reorg + Phase 6 scroll-perf/clamping
|
||
verification + tap-to-position-cursor fix + selection + real-file e2e +
|
||
Android arrow/shift workaround + touch selection + scroll-offset tap proof &
|
||
float32 decomposition fix + font-scale (user font-size setting) support +
|
||
data-corruption test suite & write-concurrency fix).
|
||
Written
|
||
against the **live** repo `/home/gmp/pad`. v1 (the widget-rebuild plan) is
|
||
superseded — see §12 for why. Doc reorganization (2026-08-16): the over-detailed
|
||
docs (`*_implementation_plan.md`, `touch.md`, `element_model.md`,
|
||
`layout_rendering.md`, `virtual_scroll_render_optimization.md`,
|
||
`conflict_resolution.md`, `bugs.txt`) were deleted; `spec.md` and
|
||
`architecture.md` were rewritten to describe the actual app; `doc/README.md`
|
||
adds the doc policy + build/install recipe. See `doc/README.md`.
|
||
Phases 0–3 done: single-owner no-lock architecture, Android IME wiring, on-device
|
||
IME validation (passing), viewport-on-open fix, chunked-buffer drift fix, the
|
||
whole-file shaper memory-leak fix, a measured 50 MB size limit, and the IME
|
||
rapid-commit desync fix (snippet/selection dedup). Phase 6 (2026-08-16) added a
|
||
default-off in-app performance profiler and verified: scroll does not degrade at
|
||
large offsets (10 MB file), scroll clamping is exact across 2→130,955-line
|
||
files, and memory plateaus ~250 MB (no leak). Phase 7 (2026-08-16) verified
|
||
tap-to-position-cursor on-device and found + fixed two real bugs: (1) for large
|
||
files scrolled deep, `SetCursorFromPoint` clamped the cursor to the bottom of the
|
||
viewport (a content-space vs window-relative Y mismatch from the Phase 3 windowing
|
||
refactor); fixed with `tapLocalY`. (2) the `GlyphLayout` byte offsets are
|
||
window-relative but the four cursor functions (tap, Home, End, vertical move)
|
||
treated them as absolute, so the cursor snapped to the window top; fixed by adding
|
||
the `IMEWindowStartByte` window base at each cursor boundary (`glyphBase`). Both
|
||
have regression tests. Phase 10 (2026-08-17) proved the tap mapping
|
||
scroll-offset independent by property test (4000 random scroll/tap pairs) and
|
||
on-device (marker typed on a visually identified line landed on exactly that
|
||
line at two fractional scroll positions); the property test exposed a float32
|
||
`int(s/lh)` rounding bug that could shift the rendered window — and every
|
||
tapped line — by one in a sub-pixel band of scroll offsets, now fixed with a
|
||
single shared float64 floor decomposition (see §5, Phase 10).
|
||
Phase 8 (2026-08-16/17) added **text selection** (shift+arrow extend; insert/
|
||
backspace/delete replace the selection; IME unions its range with the active
|
||
selection) with rendering + IME wiring. It also added **real-file e2e tests**
|
||
(`internal/test/e2e/real_file_*_test.go`: open/edit/autosave on real on-disk
|
||
files, incl. chunk-boundary and multi-byte edits) and found + fixed two real
|
||
bugs: (1) `UpdateLineIndexAfterEdit` only shifted offsets — any edit involving
|
||
newlines left the line index permanently inconsistent; replaced with
|
||
newline-aware `UpdateLineIndexAfterInsert`/`UpdateLineIndexAfterDelete`.
|
||
(2) `HandleBackspace`/`HandleDelete` deleted **one byte**, corrupting
|
||
multi-byte UTF-8 characters; now rune-granular.
|
||
On-device validation (2026-08-17) exposed a **third, platform-level gap**:
|
||
Gio v0.10 on Android (a) drops modifier state in the JNI bridge, and (b)
|
||
wraps plain arrow-key *presses* in `input.SystemEvent` for focus navigation,
|
||
so arrow keys never reached the editor and shift+arrow was impossible.
|
||
Fixed in main.go: explicit named `key.Filter`s for the four arrows (delivers
|
||
the press and suppresses the focus jump) plus app-side shift tracking.
|
||
Verified end-to-end on the emulator: plain arrows move the caret,
|
||
shift+arrow shows a highlight, typing replaces the selection. Remaining:
|
||
real-device swipe/autocorrect sign-off (the emulator's AOSP/Gboard keyboard
|
||
is a proxy).
|
||
Phase 9 (2026-08-17) added **touch selection** — the Android-native selection
|
||
model: long-press selects the word under the finger (blank → caret + paste-only
|
||
menu), double-tap selects the word, drag handles resize the selection, drag the
|
||
highlighted body moves it, and a floating copy/cut/paste menu is hit-tested in
|
||
logic and drawn by the renderer. All flows verified on-device, including the
|
||
full copy→paste and cut cycles through the real Android clipboard. Debugging
|
||
this surfaced two renderer bugs worth knowing about: (1) a map of click-registry
|
||
**values** (not pointers) silently discarded per-frame mutations, so the
|
||
long-press never fired; (2) on Android a tap's press and release arrive in the
|
||
**same frame**, and `gesture.Click`/`gesture.Drag` return one event per Update
|
||
call, so without draining each gesture's queue every frame the release was lost
|
||
on an idle window and every menu tap was swallowed.
|
||
|
||
## 1. Decision summary (updated)
|
||
|
||
- **Primary path: B — finish the live custom path, do NOT rebuild on widgets.**
|
||
The live repo already has the two hardest, most-asset-heavy pieces that a
|
||
rebuild would throw away: a **real filesystem** backend (`internal/io/pool/real/`)
|
||
and a **working Android APK** (JNI + permissions, built + signed). It also has
|
||
a **chunked buffer** (`internal/editor/chunked_buffer.go`) that already implements
|
||
the spec's "file never fully in memory" — the thing v1 said would need fork-level
|
||
work on `widget.Editor`. So the only blocker to the headline feature (Android IME:
|
||
swipe typing, autocorrect) is a **small, precisely-defined wiring gap** (§4), not a
|
||
rewrite.
|
||
- **Concurrency:** keep the existing logic-goroutine + channels model for now. It's
|
||
proven to build and (after this plan) pass tests. The v1 "one main goroutine"
|
||
simplification is a *later* cleanup, not a prerequisite for usability.
|
||
- **File-size limit:** the chunked buffer targets large files directly; we validate
|
||
it against a real 10 MB file on-device and set the honest limit from that, rather
|
||
than inheriting `widget.Editor`'s ~0.5 GB/MB wall (§2).
|
||
|
||
## 2. Evidence (verified against gioui.org v0.10.0 sources, the live repo's version)
|
||
|
||
1. **`widget.Editor` is not virtualized** — `textView.layoutText` seeks to the
|
||
start and shapes the *entire* document on every invalidation (text edit,
|
||
size/param change), with an infinite viewport; the shaper's `document.reset()`
|
||
is `lines = lines[:0]`, so the backing array of the largest layout is retained
|
||
forever. **Re-benchmarked 2026-08-18 against v0.10.0** (plain `text.Shaper`,
|
||
whole-document shape, the widget's exact call pattern; x86_64 VM):
|
||
|
||
| doc | shape time (per edit/keystroke) | retained heap |
|
||
|---|---|---|
|
||
| 1 MB | ~120 ms | ~5 MB |
|
||
| 2 MB | ~0.4 s | ~7 MB |
|
||
| 5 MB | ~3.4 s | ~15 MB |
|
||
| 10 MB | ~14 s | ~29 MB |
|
||
|
||
The **CPU wall is the decision argument**: a full re-shape on every keystroke
|
||
is already sluggish at 1 MB (~8 fps of edits) and unusable above ~2 MB; a
|
||
phone CPU (3–10× slower than this VM) is worse. Memory grows only ~3× the
|
||
text size with the raw shaper — the earlier "~0.5 GB per MB" figures (which
|
||
likely included Android PSS overhead / an older version) do not reproduce, but
|
||
they are not needed: the linear per-edit shape cost alone rules out
|
||
`widget.Editor` for multi-MB files. This is *why* the live repo went the
|
||
chunked-buffer route, and it's the reason v1's "just use the widget" was
|
||
wrong for large files. (For small files — sub-MB notes — `widget.Editor`
|
||
would be adequate; the custom editor is only strictly required above ~1 MB,
|
||
but the app's 50 MB target is 50× that.)
|
||
2. **The Android IME works through the op layer, not the widget.** The
|
||
`window`'s editor state (`app.Window` → `input.EditorState{Selection, Snippet}`)
|
||
is what `GioInputConnection` (Android's `InputConnection`) reads. It is fed by
|
||
`key.SelectionCmd` and `key.SnippetCmd` ops tagged with a **focusable** handler.
|
||
A handler becomes focusable by emitting a `key.FocusFilter{Target: tag}` (that's
|
||
all `widget.Editor` does — it never touches `io/input` at all). The live code
|
||
already emits `key.FocusCmd{Tag}` + `key.FocusFilter{Target}` and consumes
|
||
`key.EditEvent`/`key.SnippetEvent`, so the *skeleton* is present; the stateful
|
||
ops are missing (§4).
|
||
3. **The IME snippet is selection-scoped** — so a chunked/virtualized editor can
|
||
still feed a correct snippet (only the visible/selected window), which the live
|
||
chunked-buffer design is compatible with.
|
||
4. **No headless test window** in v0.9 or v0.10 — widget-level Go tests are
|
||
impossible; on-device e2e is required (§9).
|
||
5. **The dev agent has no vision** (verified) — the emulator debug loop is
|
||
data-based (state dumps, logcat, in-app frame-delta logs, PIL/OCR), not
|
||
screenshot-judgment
|
||
(§9.3).
|
||
|
||
## 3. Current state of the live repo (post housekeeping, 2026-08-16)
|
||
|
||
- `go build ./...` green. **`go test ./...` and `go test -race ./...` green.**
|
||
- `03fb638` fixed the red build (harness + 5 call sites after `NewLogic` gained
|
||
a `path` and `openfunc` parameter); the e2e harness uses a no-op `openfunc`
|
||
(matching `impl_other.go`, whose real `OpenFile` is a no-op off-Android).
|
||
- `7240b62` committed the JNI/Termux open-file bridge + word-wrap-aware
|
||
viewport/scroll WIP as the working-tree baseline.
|
||
- `58725a5` made state single-owner and turned the race detector green
|
||
(see §13).
|
||
- The working tree is a clean, race-clean baseline. Ready for the IME work.
|
||
|
||
## 4. The IME wiring gap (verified, testable) — THE central work item
|
||
|
||
Android IME text (swipe, autocorrect) arrives as `key.EditEvent{Range, Text}`
|
||
routed to the focused tag. Four things are missing or wrong:
|
||
|
||
1. **No `key.SelectionCmd` emitted.** Without it the driver's
|
||
`EditorState.Selection.Caret` stays at origin, so the IME doesn't know where the
|
||
caret/selection is → suggestions anchor wrongly and the caret jumps after a
|
||
commit. **Fix:** in the editor's draw/update pass, emit
|
||
`gtx.Execute(key.SelectionCmd{Tag, Range:{cursor,cursor}, Caret:{Pos,Ascent,Descent}})`
|
||
whenever the caret moves. The caret's **pixel** position already exists in
|
||
`internal/ui/render.go:534-554` (the code that draws the caret) — it must be
|
||
plumbed into `app.State` (caret pos in px) and read back here. This is the one
|
||
item coupled to the cursor-positioning work, but it reuses the same geometry.
|
||
2. **No `key.SnippetCmd` emitted.** Without it the IME receives an empty snippet →
|
||
no autocorrect context and no in-place swipe replacement. **Fix:** emit
|
||
`gtx.Execute(key.SnippetCmd{Tag, Snippet:{Range: selection, Text: selectedText}})`;
|
||
with no selection, an empty snippet at the caret (exactly what `widget.Editor`
|
||
sends). Feeding it from the chunked buffer is O(selection size), not O(file).
|
||
3. **`HandleKeyDown` ignores `EditEvent.Range`.** `state.go:313` does
|
||
`HandleInsert(v.Text)` for any non-backspace text and never deletes
|
||
`v.Range.Start..End`. A *replacement* commit (the normal swipe/autocorrect case)
|
||
therefore inserts the new text **without removing the old** → duplication/corruption.
|
||
**Fix:** `Replace(v.Range.Start, v.Range.End, v.Text)` on the chunked buffer and
|
||
advance the cursor to `Start + len(Text)`. (~10 lines; `ChunkedBuffer` already
|
||
has `Insert`/`Delete`.)
|
||
4. **No `key.InputHintOp{Tag, Hint: key.HintText}`.** Cosmetic but correct to add —
|
||
hints the on-screen keyboard into text/autocorrect mode.
|
||
|
||
Acceptance (all verifiable headlessly, §9): after an IME commit, the buffer equals
|
||
the expected post-replacement text, the caret is at the commit end, and the next
|
||
frame's `SelectionCmd`/`SnippetCmd` reflect that state. On a real IME, swipe +
|
||
autocorrect produce clean, non-duplicated text.
|
||
|
||
## 5. Phases (Path B)
|
||
|
||
### Phase 0 — clean baseline (small) — DONE (`58725a5`)
|
||
1. Commit the uncommitted JNI/chunk-buffer work. — `7240b62`.
|
||
2. `go test -race ./...` green (add `-race`; the channel model may surface races —
|
||
fix any found). — `58725a5`. Details in §13.
|
||
|
||
### Phase 1 — complete the IME (§4) — DONE (`9b78219`, `72b3c3f`)
|
||
All four items implemented and tested:
|
||
- **Item 3** (`9b78219`): `HandleKeyDown` honors `key.EditEvent.Range` via
|
||
`HandleReplaceRange` (rune→byte via UTF-8 leading-byte scan, string + chunked
|
||
paths). Also fixed a multi-chunk `Delete` corruption and a `FullContent`
|
||
truncation bug found while testing it.
|
||
- **Items 1/2/4** (`72b3c3f`): `TextField.Draw` emits, when focused,
|
||
`key.InputHintOp{HintText}`, `key.SnippetCmd` (the **visible window** as the
|
||
snippet, `Range {0,len}` so the IME reports `EditEvent.Range` window-relative),
|
||
and `key.SelectionCmd` (caret, window-relative rune index). `HandleReplaceRange`
|
||
offsets the window-relative range by `IMEWindowStartByte` (new `EditorState`
|
||
fields set during layout) to address the buffer.
|
||
|
||
Tests: `ime_range_test.go` covers string + chunked + unicode + swapped-bounds +
|
||
windowed (scrolled) paths for item 3. Items 1/2/4 are renderer-side op emission
|
||
(not observable headlessly); they are verified by build + on-device (Phase 2).
|
||
|
||
Note: pushing the visible window (not the whole file) as the snippet keeps IME
|
||
traffic small for large files. An `EditEvent.Range` that falls outside the
|
||
window (the IME discarding and re-anchoring the snippet — a rare case) is
|
||
clamped to the window end by the byte-conversion, so it degrades rather than
|
||
corrupting; the common within-window path is exact.
|
||
|
||
### Phase 2 — emulator verification — DONE (IME commit path validated on-device)
|
||
Environment (this 16 GB VM): Android SDK + NDK 27 + platform-tools + emulator +
|
||
API 35 `google_apis/x86_64` system image installed to `~/android-sdk`; AVD
|
||
`pad_avd` (Pixel 6 profile) boots with KVM (`/dev/kvm` chmod 666). Build recipe:
|
||
`gogio -target android -targetsdk 35 -arch amd64` → inject
|
||
`MANAGE_EXTERNAL_STORAGE` via apktool → sign with the debug keystore →
|
||
`adb install -r` (scripted in `scripts/build_emu.sh`).
|
||
|
||
**On-device IME validation — PASSING** (Gboard, API 35; observed via the gated
|
||
`imeDebugLog` trace + logcat + autosaved file diff + screenshots):
|
||
- Browser lists `/storage/emulated/0/Notes`; a row tap opens the file in the
|
||
in-app editor (see the open-file fix below).
|
||
- Editor focus → `key.FocusCmd`/`FocusFilter` → Gboard shows (soft input up).
|
||
- **Single-char commit** (tap a Gboard key): one `EditEvent{Range:{c,c},Text}` →
|
||
`HandleReplaceRange` inserts at the caret; buffer + cursor + autosaved file all
|
||
correct.
|
||
- **Multi-char commit** (tap keys with human ~0.6 s gaps): cursor advances in sync
|
||
(c→c+1→c+2…), each char lands at the right byte offset, file correct.
|
||
- **Deletion** (Gboard backspace): routed through the IME as
|
||
`EditEvent{Range:{i,i+1},Text:""}` (i.e. `deleteSurroundingText`) →
|
||
`HandleReplaceRange` deletes the range correctly.
|
||
- **Unicode** content (`héllo wörld 日本語`) displays and the buffer stays
|
||
consistent through edits around it.
|
||
- Conclusion: §4 (the IME wiring gap) works end to end for a real IME's commit
|
||
path. Swipe-typing and autocorrect use the *same* commit path
|
||
(`commitText`/`setComposingText` → `EditEvent`), so they are covered by this;
|
||
a real-device final sign-off with a swipe/autocorrect IME is still worth doing.
|
||
|
||
**Bugs found + fixed on-device (Phase 2):**
|
||
1. **Open-file crash (fixed, `c9c0d47`):** browser-row taps were routed through
|
||
the Android Termux bridge (`ui.OpenFile = openfunc`), which built a `file://`
|
||
Intent URI and crashed on Android 7+ with `FileUriExposedException`, and it
|
||
bypassed the editor entirely. Fix: `ui.OpenFile` now calls the in-app
|
||
`editor.OpenFile(path)`; the Termux bridge is retained as a dormant
|
||
`TheState.open` hook.
|
||
2. **Viewport opens at EOF (open bug, Phase 3):** after a file opens, the
|
||
*opening* tap leaks into the now-visible editor and `SetCursorFromPoint`/scroll
|
||
moves the viewport to the tapped row (below a short file's content) → the
|
||
content area renders blank (`VisibleByteRange` lands on a past-EOF visual
|
||
line; `visibleContent (0)`). Workaround used during testing: swipe to top.
|
||
Root cause is the tap that switches `page=EditorPage` also being delivered as an
|
||
editor tap. Needs: ignore the opening tap in the editor (or clamp
|
||
`SetCursorFromPoint`/scroll so a short file that fits the viewport never
|
||
scrolls).
|
||
3. **Rapid synthetic IME commits desync (edge case) — FIXED:** `adb shell input
|
||
text "…"` fires per-char commits faster than a frame; the per-frame
|
||
`SnippetCmd`/`SelectionCmd` re-push reset the IME's cursor, so fast commits
|
||
interleaved/corrupted (observed `" IME123"` → `E123IM…`). Fixed by deduping the
|
||
IME ops in `TextField.Draw` — push the snippet/selection only when they change
|
||
(and force a fresh push on focus (re)gain), mirroring `widget.Editor`'s
|
||
`updateSnippet`/selection gating. State lives in the main-owned `Renderer`
|
||
(commit `46383c1`). On-device, rapid commits (0.08–0.1 s cadence) now land
|
||
cleanly.
|
||
|
||
Observation loop that worked (no state-dump flag needed in the end): logcat
|
||
(`imeDebugLog` + `HandleKeyDown` + `VisibleByteRange`), autosaved-file diff via
|
||
`adb shell cat`, and screenshots (vision, auxiliary).
|
||
|
||
### Phase 3 — large-file validation + honest size limit — DONE
|
||
1. **Open a real 10 MB file** via the chunked buffer: opens in ~121 ms
|
||
(stat→read→index), renders correctly, smooth scroll. ✓
|
||
2. **Fixed the whole-file shaper leak** (the real 1 GB memory bug): with word
|
||
wrap on (the default), `VisibleByteRange` used the previously-shaped
|
||
`GlyphLayout.VisualLineStarts` to bound the visible range. That layout only
|
||
covers the visible window (~50 lines), not the document, so once the
|
||
viewport's line count exceeded the window's visual-line count the range fell
|
||
back to `end=fileLen` and the shaper laid out the **entire file** every frame.
|
||
Gio's shaper `document.reset()` keeps the backing-array cap, so memory grew
|
||
to the largest layout ever shaped and never shrank (~1.1 GB PSS for 10 MB,
|
||
OOM-killed under scroll). Fix: always derive the range from the real-line
|
||
`LineIndex`; word wrap needs no separate path (each real line yields ≥1
|
||
visual line). Now ~150 MB PSS / ~230 MB RSS at steady state, flat under
|
||
scroll. (commit `c79c142`)
|
||
3. **Fixed the pre-existing LineIndex storage mismatch**: `EditorLayout` read
|
||
`TheState.Editor.LineIndex` (never set; `SetLineIndex` had zero callers) for
|
||
`maxScroll`/`scrollByteOffset`, always using the huge pre-index estimate. Now
|
||
reads `cb.LineIndex`; the dead field + setter are removed.
|
||
4. **Honest size limit = 50 MB** (`MaxEditableFileSize`), measured on-device
|
||
(10 MB → ~150 MB PSS; 50 MB extrapolates to a few hundred MB, fine on a
|
||
phone). Larger files get a "too large to edit" state; the browser still
|
||
lists them. ✓
|
||
5. **IME rapid-commit desync fix**: `TextField.Draw` now dedups the IME
|
||
`SnippetCmd`/`SelectionCmd` (push only on change, fresh push on focus
|
||
(re)gain; state in the main-owned `Renderer`), mirroring `widget.Editor`.
|
||
Rapid commits (0.08–0.1 s cadence) land cleanly on-device (commit
|
||
`46383c1`). ✓
|
||
6. **Remaining (not blocking usability):** real-device swipe/autocorrect
|
||
sign-off (the emulator's AOSP/Gboard keyboard is a proxy for real IMEs).
|
||
|
||
### Phase 4 — the v1 simplifications, now that it's usable (optional, later)
|
||
Only after the app is usable: replace globals (`TheState`, `ui.OpenFile`) with
|
||
explicit state, prune dead task types, and *then* consider collapsing the
|
||
logic-goroutine/channel model to a single owner. These reduce future bug surface
|
||
but are not needed for a usable v1.
|
||
|
||
### Phase 5 — hardening (mostly done)
|
||
- ✓ Rewrite `doc/architecture.md` to match reality (2026-08-16: now describes
|
||
the single-owner/no-lock model, channel topology, Frame contract, ownership
|
||
rules; over-detailed companion docs deleted — see `doc/README.md` policy).
|
||
- ✓ Amend `spec.md` per §7 (2026-08-16: rewritten; unbuilt features moved to an
|
||
explicit “deferred” table; see the corrections noted under §7 below).
|
||
- ✓ Build/install recipe (2026-08-16: in `doc/README.md`; self-contained
|
||
in-repo script `scripts/build_emu.sh`).
|
||
- ☐ In-repo device test checklist (the adb tap/swipe/IME sequences used in
|
||
Phases 2–3 and Phase 6 are in this plan's phase notes but not a standalone
|
||
checklist).
|
||
|
||
### Phase 6 — scroll performance & clamping verification — DONE (2026-08-16)
|
||
|
||
Goal: (a) confirm scroll does not degrade at large offsets in a large file,
|
||
(b) confirm scroll clamping is correct across file sizes. Enabled the default-off
|
||
in-app profiler (architecture.md §11) and drove deterministic `frac` scroll
|
||
jumps + real swipes on the emulator.
|
||
|
||
**Scroll performance at large offsets (10 MB file, 130,955 lines):** swept
|
||
`frac` 0.02→1.0 with swipes. Logic-frame cadence is **flat across all offsets**
|
||
(p50 ~38–44 ms, p90 ~51–75 ms, no trend up at 0.9/0.98/1.0) — **no large-offset
|
||
degradation**. The visible byte range stays **≤ 4,274 B (0.04% of the file)** at
|
||
every offset, confirming the shaper never lays out the whole file. `ScreenRecord`
|
||
and `gfxinfo` were ruled out as metrics (downsampled / View-layer-only, since Pad
|
||
renders into a `SurfaceView`); the in-app profiler is the instrument.
|
||
|
||
**Scroll clamping (6 sizes, 2→130,955 lines):** for each, `top` and `bottom`
|
||
commands set the offset to exactly 0 and to exactly `maxScroll`; **no frame ever
|
||
exceeded `maxScroll` or went negative**. Sub-viewport files (`tiny_1line` 2 lines,
|
||
`small_10` 11 lines) correctly have `maxScroll = 0` and cannot scroll; larger
|
||
files' `maxScroll` scales correctly with content. No blank viewport at any size
|
||
(content verified on-screen).
|
||
|
||
**Memory (bonus):** PSS **plateaus ~250 MB** for the 10 MB file under sustained
|
||
scroll (bounded high-water mark from the shaper glyph cache + Go heap; grows
|
||
~13 MB over the first ~30 scrolls then flat) — **no leak**, well under the 2.5 GB
|
||
OOM line. The profiler's own overhead is negligible (same plateau with it off).
|
||
|
||
**Test-harness gotcha:** on-screen tap coordinates are not the naive
|
||
Dp→px map — see `README.md` §Screen coordinates (screen px vs display px
|
||
vs app-local; the tap rule is "read off the screenshot, ×1.2"). Don't
|
||
memorize content positions (file rows etc.); measure them from the current
|
||
screenshot. Verify file opens from the title bar / file content, not
|
||
logcat (the open log line was later removed as noise).
|
||
|
||
### Phase 7 — tap-to-position-cursor verification + fix — DONE (2026-08-16)
|
||
|
||
Asked "does tapping in the editor reposition the cursor, and has it been
|
||
verified?" — it had **not** been verified (the old unit test only checked
|
||
in-bounds/no-panic, never the landed offset). On-device verification found a
|
||
real bug:
|
||
|
||
- **Symptom:** on a large file scrolled deep (e.g. `big10mb.txt` at 70% down),
|
||
tapping anywhere in the viewport placed the cursor on the **bottom** line of
|
||
the viewport, regardless of where you tapped. It worked on small files only
|
||
because their visible window happened to be wide enough to keep the line number
|
||
in range.
|
||
- **Root cause:** the tap handler computed the tap's text-local Y in
|
||
**content space** (`pt.Y - region.Y + full ScrollOffset`) and passed it to
|
||
`SetCursorFromPoint`, whose `visualLine = y/lineHeight` then produced a huge
|
||
content-line number (e.g. 91,640). But the `GlyphLayout` is **window-relative**
|
||
(`layout.Y==0` is the top of the *visible window*, not the file), so the line
|
||
number far exceeded the window's ~86 lines and clamped to the last group (the
|
||
bottom line). The Phase 3 windowing refactor introduced the windowed layout
|
||
but the tap handler was never updated to match it.
|
||
- **Fix:** `tapLocalY()` converts the tap Y to window-relative space by adding
|
||
only the sub-line remainder of the scroll (`ScrollOffset mod lineHeight`),
|
||
never the full scroll. Extracted as a named helper so it is unit-testable.
|
||
- **Verification:** on-device, taps now map linearly across the viewport (top tap
|
||
→ window line 3; y=430/750/1000/1200 → lines 3/11/16/21, cursor 132/464/677/924),
|
||
and a screenshot confirms the cursor bar lands on the tapped line, not the
|
||
bottom. Two regression tests added (`TestTapLocalY_WindowRelative`,
|
||
`TestTapToPosition_LargeFileScrolled`) — both fail on the pre-fix formula
|
||
(cursor clamps to byte 78 = bottom line) and pass on the fix.
|
||
- Also removed the per-tap `log.Printf` debug lines in `SetCursorFromPoint`
|
||
(per-tap, per-glyph logcat noise).
|
||
|
||
### Phase 8 — selection on-device + Gio Android key limitation — DONE (2026-08-17)
|
||
|
||
Verified shift+arrow selection on the emulator, then root-caused why plain
|
||
hardware arrows never reached the editor on Android (see the Phase 8 note in the
|
||
status header): Gio v0.10's Android JNI drops modifier state, and plain arrow
|
||
*presses* are wrapped in `input.SystemEvent` for focus navigation. Fix in
|
||
main.go: explicit named `key.Filter`s for the four arrows (delivers the press
|
||
and suppresses the focus jump) plus app-side shift tracking (`NameShift`
|
||
press/release, reset on focus loss); forwarded events carry
|
||
`Shift: Modifiers.Contain(ModShift) || shiftDown`. Verified on-device: `keyevent
|
||
22` moves the caret, `keycombination 59 22` extends the selection, typing
|
||
replaces it. Documented as a Gio-version-sensitive workaround (architecture.md
|
||
§2.1).
|
||
|
||
### Phase 9 — touch selection — DONE (2026-08-17)
|
||
|
||
Implemented the Android-native touch selection model (v1 scope: word selection
|
||
+ resize + move + floating menu; no handle-tap-to-caret, no marquee, no
|
||
auto-scroll-to-cursor). See spec.md §2.2 and architecture.md §6.3 for the
|
||
contracts; the highlights:
|
||
|
||
- **Renderer reports finger positions, logic owns all geometry.** Touch events
|
||
are delivered as `ui.Point` (tap), `ui.DoubleTapPoint`, `ui.LongPressPoint`
|
||
(app-local Dp); the logic goroutine converts them to text coordinates with
|
||
the stored `EditorRegion` + scroll offset, hit-tests menu items itself, and
|
||
owns the menu rect, highlight range, and handle positions. The renderer only
|
||
draws what the frame snapshot says.
|
||
- **Long press needs invalidation to elapse.** Gio renders on demand; a
|
||
stationary finger produces no events and no frames, so the 400 ms threshold
|
||
could never be checked. The main loop polls `Renderer.PendingLongPress()`
|
||
and keeps `w.Invalidate()`-ing while a press is held still on the editor.
|
||
A non-grabbing raw pointer probe (`event.Op` tag) watches the press for
|
||
movement so a scroll cancels the pending long press; scroll/drag gestures
|
||
grab the pointer (`pointer.GrabCmd`) and cancel it the same way.
|
||
- **One event per Update is a trap on Android.** A tap's down+up routinely
|
||
land in one frame; `gesture.Click`/`gesture.Drag` return at most one event
|
||
per `Update` call, so the release sat in the queue until the *next* redraw —
|
||
which on an idle window never comes. The renderer now drains each gesture's
|
||
queue to exhaustion every frame (scroll already drains internally).
|
||
- **Clipboard crosses the goroutine boundary via channels; the ops run on the
|
||
main/Gio frame path** (`clipboardSetChan`/`pasteReqChan` logic→main,
|
||
`pasteChan` main→logic, all buffered so the harness never blocks logic).
|
||
On Android, `clipboard.ReadCmd` is answered synchronously during op flush
|
||
with a queued `transfer.DataEvent` that schedules **no** frame of its own,
|
||
so main invalidates the window after each read to guarantee a follow-up
|
||
frame in which the DataEvent is consumed and forwarded to logic.
|
||
- **Bugs found while validating on-device:** `clickReg` stored by value in a
|
||
map (per-frame press bookkeeping lost → long press never fired); `Menu.Draw`
|
||
didn't offset items by the menu origin; the clippable `drawElement` branch
|
||
skipped `SelDrag` registration (handles never drew); the press-probe query
|
||
omitted `Kinds` (a `pointer.Filter` with zero kinds matches nothing).
|
||
- **Verification:** unit tests (`touch_selection_test.go`, ~18) + e2e
|
||
(`touch_selection_e2e_test.go`) green under `-race`; on-device: long-press
|
||
word/blank, double-tap, handle-drag resize, single-tap copy/cut/paste
|
||
(including copy→paste and cut cycles through the real Android clipboard),
|
||
menu close-on-tap, plain-tap caret placement, and scroll-drag not firing a
|
||
long press.
|
||
|
||
### Phase 10 — proving tap-to-position is scroll-offset independent — DONE (2026-08-17)
|
||
|
||
Question: does the screen→line tap mapping account for the scroll offset, and
|
||
can it be shown to map a screen tap to the right document line at *any* scroll
|
||
offset? The answer was "yes, except one razor-thin band" — and the exception is
|
||
now fixed and property-tested.
|
||
|
||
- **The mapping is structurally scroll-aware.** The visible glyph layout is
|
||
window-relative, so a tap only needs the *sub-line* remainder of the scroll
|
||
offset (`tapLocalY` adds r, never the full scroll), and the window start
|
||
line k = floor(s/lh) re-anchors window-relative bytes to absolute file bytes
|
||
(`IMEWindowStartByte`). With k and r consistent, a tap a dp below the region
|
||
top always lands on content line k + ⌊(a+r)/lh⌋ = ⌊(a+s)/lh⌋ — the line
|
||
under the finger — for every s ≥ 0, wrapped or not (wrap only changes which
|
||
real line a display line belongs to, never the drawn geometry).
|
||
- **Bug found by the property test:** `int(s/lh)` in the Dp float32 domain
|
||
rounds the quotient to nearest, and can round UP across an integer boundary
|
||
while the float64 remainder still reflects the line below. In a
|
||
sub-pixel-wide band of scroll offsets the window started one line too far
|
||
while the draw shift said one line back — the whole rendered window (and
|
||
every tapped line) was off by one. Fixed with a single shared float64 floor
|
||
decomposition (`scrollDecompose`) used by the window start
|
||
(`visibleByteRangePrecise`/`Estimate`), the renderer's sub-line shift
|
||
(`visibleScrollOffset`), the tap mapping (`tapLocalY`), and chunk
|
||
prefetching — the three consumers cannot disagree by construction.
|
||
- **Proof:** `tap_scroll_property_test.go` — 4000 random (scroll offset, tap
|
||
position) pairs (integer-line, near-boundary, and arbitrary fractional
|
||
offsets) assert the cursor lands on the line whose *independently computed*
|
||
drawn range contains the finger (ground truth ⌊(a+s)/lh⌋, not the tap code's
|
||
own math). Failed on the exact-boundary case before the fix; green after.
|
||
- **On-device cross-check:** at two scroll positions (s = 4246.9 dp, r = 13.3;
|
||
s = 4210.7 dp, r = 10.7, both with a fractional sub-line remainder read
|
||
from the profiler CSV), a tap on a visually identified line typed a marker
|
||
that landed on exactly that line in the file on disk (line 264 and line 258
|
||
of a 300-line file). Screenshot, profiler ScrollDP, formula, and disk all
|
||
agreed.
|
||
|
||
### Phase 11 — hi-DPI / font-scale audit and font-scale fix — DONE (2026-08-17)
|
||
|
||
Question: does the tap/scroll mapping hold under all allowed device display
|
||
configurations, or only on this AVD's density?
|
||
|
||
- **Density (pure hi-DPI): holds, provably.** All geometry bookkeeping is in
|
||
density-dp; the device scale enters only at the px↔dp boundary via a single
|
||
`r.scale`/`State.scale` (= `PxPerDp`), with sub-pixel float32 rounding that
|
||
cannot cross a line boundary. The Phase 10 invariant uses no device numbers,
|
||
so it is scale-free: any density.
|
||
- **Font scale (user font-size setting): was broken, now fixed.** On Android,
|
||
`PxPerSp = fontScale × PxPerDp` (the Settings → font-size knob), and the
|
||
shaper draws baselines in sp — so at, say, fontScale 1.3 the rendered line
|
||
pitch is 21.84 dp while every logic-side consumer used the raw 16.8 dp.
|
||
Taps would have been off by up to (fontScale−1) viewportfuls of lines and
|
||
scroll clamping would have stopped short of the file bottom. Fixed by
|
||
tracking `fontScale` in `State` (`ScaleEvent.FontScale`, closed loop via
|
||
`Frame.FontScale`) and routing every line-height consumer through
|
||
`EffectiveLineHeight()` (window start, sub-line remainder, tap mapping,
|
||
scroll clamp, page size, cursor vertical move, menu position), with the
|
||
renderer using the same scaled pitch for `GlyphLayout.LineHeight`, the
|
||
caret, selection handles, and highlight (ascent/line-height in
|
||
`drawWrappedText` now `× fontScale` from `gtx.Metric`).
|
||
- **Proof:** `font_scale_test.go` — `TestTapPosition_FontScale_Property`
|
||
(2000 random scroll/tap pairs at fontScale 1.3 with the glyph layout
|
||
fabricated at the scaled line pitch, same independent ground truth as
|
||
Phase 10) plus an `EffectiveLineHeight` unit test. Existing tests are
|
||
unaffected (unknown fontScale ⇒ 1.0).
|
||
- **On-device cross-check:** with `settings put system font_scale 1.3` and
|
||
`0.8`, at multiple scroll positions (including a settled one where the
|
||
topmost visible line was verified to be exactly k = floor(s/lh_eff)), a tap
|
||
on a visually identified line typed a marker that landed on exactly that
|
||
line in the file on disk (fsline060, fsline080, fsline081 at 1.3×; fsline039
|
||
at 0.8×). Rendered line pitch measured 57 px at 1.3× and 35 px at 0.8× vs
|
||
44 px at 1.0× — matching 16.8×fontScale×2.625.
|
||
- **Verification-method note:** the profiler CSV flushes to disk at most every
|
||
2 s, so `tail` reads can be up to 2 s stale (a settling fling reads as
|
||
"settled" if two reads land in the same flush window). Settle checks compare
|
||
rows ≥ 2.5 s apart after the last input; the tap test (screenshot → tap →
|
||
marker → read the file off the device) is the ground truth and needs no
|
||
profiler.
|
||
|
||
### Phase 12 — data-corruption test suite + write-concurrency fix — DONE (2026-08-17)
|
||
|
||
Two questions: (a) how do we *prove* the edit→persist path cannot corrupt
|
||
file data; (b) fix the write-concurrency race the review identified (user
|
||
deprioritized it, then asked for it to be implemented after the test suite).
|
||
|
||
**Test suite (5 layers, all differential/oracle-based):**
|
||
- `chunked_buffer_fuzz_test.go` — every random Insert/Delete/replace mirrored
|
||
on a plain `[]byte` shadow model (chunk sizes 1…64 KB, 500–2000 ops each);
|
||
after every op: FileLen, FullContent, Content probes, chunk-size invariant.
|
||
Rune-aligned variant adds UTF-8 validity + an independent RuneIndexToByte
|
||
oracle.
|
||
- `line_index_fuzz_test.go` — incremental LineIndex updates vs a
|
||
full-recomputation oracle (mixed / no-newline / single-line / CRLF /
|
||
trailing-newline shapes), plus a trailing-empty-line structural test.
|
||
- `state_api_fuzz_test.go` — the production edit entry points
|
||
(HandleInsert/Backspace/Delete/ReplaceRange incl. selection variants);
|
||
after every op: content, UTF-8, chunk invariant, line index, exact cursor.
|
||
- `real_file_fuzz_test.go` (e2e) — random edit sequences incl. window-relative
|
||
IME replaces against the real FS; per-batch IME-window-consistency checks,
|
||
forced-flush disk byte-compare every batch, then a second Logic instance
|
||
(restart simulation) must reload byte-identical content; no stray temps.
|
||
- `filesystem_test.go` (real) — the atomicity contract: exact round trips, a
|
||
concurrent reader never sees a torn file across 150 alternating 2 MB
|
||
writes, a failed write leaves the original byte-identical, stale temps are
|
||
consumed.
|
||
|
||
**What the testing found (fixed in `5205906`):** `Insert` halved an oversized
|
||
spliced result once, so a large paste into a non-empty buffer left chunks up
|
||
to ~P/2 (8× the documented 2× bound at 1 MB/64 KB). `Insert` now re-chunks
|
||
the oversized result into pieces ≤ chunkSize, so "no chunk exceeds 2× target
|
||
after any edit" holds universally. Mutation-tested: dropping one byte in
|
||
Insert and one entry in UpdateLineIndexAfterInsert are both caught in ms.
|
||
|
||
**Write-concurrency fix (this round):**
|
||
- **Protocol (owner-side, per file):** at most one write in flight per file
|
||
(`writeInFlight[f]` = snapshot version); a save requested during a write is
|
||
deferred (`savePending`) and re-issued by the result handler; on success the
|
||
*snapshot's* version is recorded as written, so edits that landed during the
|
||
write trigger the re-issue — "last rename wins" coincides with "newest
|
||
snapshot wins". `FlushAll` (GoToBrowser/Shutdown) obeys the same protocol
|
||
instead of racing a worker write on the same temp file.
|
||
- **Mechanism (real FS):** `WriteFileAtomic` now uses a unique per-call temp
|
||
("`.<name>.tmp.<pid>.<seq>`"), so same-file interleaving is structurally
|
||
impossible even if the serialization regressed; each successful write also
|
||
best-effort removes stale temps of the same file (crash leftovers + the
|
||
legacy deterministic name).
|
||
- **Shutdown drain:** on `done`, the owner waits (bounded 5 s) for in-flight
|
||
writes and armed retries to settle before exiting, so the post-exit
|
||
synchronous `FlushAll` and `workerPool.Stop` cannot race a straggling
|
||
worker write. The retry timer now sends a non-blocking token (no timer-
|
||
goroutine stall on a full channel), and `emitFrame` no longer blocks on a
|
||
slow/gone main (dropped frames are snapshots; the next emission wins).
|
||
- **Proof:** `write_serialization_test.go` — a counting FS wrapper proves the
|
||
peak concurrent same-file saves is 1 across two deliberately overlapping
|
||
autosaves (2 s saves, edit inside the first save's window), and that a
|
||
Flush during an in-flight save neither adds a concurrent writer nor lets a
|
||
stale snapshot win. The test was mutation-verified: disabling the deferral
|
||
makes it fail with peak = 2. (Debugging note: the pool's `WriteFileTask`
|
||
calls `FS.WriteFile`, not `WriteFileAtomic` — the real FS is atomic only
|
||
because `WriteFile` delegates to `WriteFileAtomic`; the wrapper mirrors
|
||
that delegation or the timing window doesn't exist.)
|
||
- **Residuals (documented, out of scope):** no `fsync` before rename (power-
|
||
loss window only); external-change detection absent; a drain-deadline exit
|
||
with a straggling write can only lose freshness (unique temps keep every
|
||
rename a complete snapshot).
|
||
|
||
### Phase 13 — word-wrap scroll jump — DONE (2026-08-17)
|
||
|
||
**Bug (user-reported, content-dependent, not a performance problem):**
|
||
scrolling a wrapped file, the viewport jumped past the wrapped remainder of a
|
||
logical line the moment its bottom crossed the viewport top, instead of moving
|
||
pixel-by-pixel with the finger. Jump magnitude = (count−1)·lh, where count is
|
||
the line's visual-line count. Root cause: every scroll↔content mapping site
|
||
assumed 1 logical line = 1 visual line (V(k)=k): the window start line
|
||
(`visibleByteRangePrecise`), the renderer sub-line shift, the tap mapping
|
||
(`tapLocalY`), the max-scroll clamp, and `bytePosToScreenXY` (which also
|
||
ignored the sub-line shift entirely — the selection menu/handles were off by
|
||
up to a full line, a second latent bug fixed in the same change).
|
||
|
||
**Fix:** a `WrapIndex` (Fenwick tree of per-logical-line visual-line counts,
|
||
parallel to the LineIndex) routes every mapping site through one conversion:
|
||
the scroll offset lives in visual-line space, k = LineForVisual(⌊s/lh⌋),
|
||
r = s − V(k)·lh. Counts are corrected per frame from the renderer's
|
||
`VisualLineStarts` (the layout feedback now carries the exact window text the
|
||
layout was shaped for, its first logical line, and the content-edit counter;
|
||
corrections apply only when the edit counter matches). The edit hooks
|
||
(UpdateLineIndexAfter{Insert,Delete}) bookkeep the index in the same pass as
|
||
the LineIndex under the never-under-stale rule. Invariant (see
|
||
architecture.md §6.2): the viewport top is always exactly s into the
|
||
document's visual space (V(k)·lh + r = s); an all-ones index reduces to the
|
||
legacy 1:1 mapping, so pre-shaping and non-wrapped behavior are unchanged by
|
||
construction.
|
||
|
||
**Tests (all mutation-verified where practical):**
|
||
- `wrap_index_test.go`: Fenwick ops (Set/SetRange/Insert/Delete/LineForVisual/
|
||
prefixes) vs a naive model, 3000 random ops; all-ones-is-identity pin.
|
||
- `wrap_bookkeeping_test.go`: the edit hooks vs a shadow-string oracle (400
|
||
random insert/delete ops; changed lines must be re-stamped, survivors keep
|
||
counts). Caught a real under-marking: an insertion with no newline left the
|
||
containing line's stale count (m=0 skip).
|
||
- `wrap_mapping_test.go`: the jump regression — V(k)·lh + r == s over a 4000-
|
||
step sweep + 2000 random offsets on a fabricated wrapped file; the legacy
|
||
identity pin (all-ones and no-index reduce to the old mapping); a
|
||
boundary sweep across every wrapped-line boundary (no skip, no repeat,
|
||
fixed lines move exactly finger-speed).
|
||
- `wrap_apply_test.go`: VisualLineStarts→logical-line grouping (multi-wrap
|
||
line, empty line, trailing-newline edge, guard early-outs). The first
|
||
version of the test exposed a production bug: the correlation guard
|
||
(`WindowStartByte > len(windowText)`) was always true at non-zero scroll,
|
||
so corrections could never apply after scrolling; and grouping over the
|
||
CURRENT window text (instead of the shaped one) mis-attributes counts
|
||
whenever a scroll moved the window between shaping and delivery. Both
|
||
fixed by carrying the shaped window text in the frame/feedback.
|
||
- Also fixed a pre-existing e2e failure (`TestRealFile_ShiftSelectionInsert`):
|
||
`emitFrame` dropped a frame when the handoff buffer was full and, since
|
||
emission is event-driven, a dropped FINAL frame was never re-emitted — the
|
||
consumer could sit one state behind forever. `emitFrame` now replaces the
|
||
unread frame with the newer snapshot (latest frame wins) instead of
|
||
dropping; still non-blocking.
|
||
|
||
**On-device (emulator, wraptest.txt: 60 logical lines × 4 visual lines):**
|
||
dp sweep via the debug cmd poller: dp 0/17/50/67/134/340 land on
|
||
LINE000-vl0 / LINE000-vl1 / LINE000-vl3 / LINE001-vl0 / LINE002-vl0 /
|
||
LINE005-vl0 — pixel-exact 1:1 finger↔content, no jump at the wrapped
|
||
boundaries (dp 134 is where the old code jumped to LINE008); bottom clamp
|
||
lands exactly on the file end via the wrap-aware TotalVisuals().
|
||
|
||
### Phase 14 — scroll-anchored selection + keyboard/bottom-bar — DONE (2026-08-17)
|
||
|
||
Two user-reported bugs in the post-wrap build (`83f7aff`):
|
||
|
||
**Bug 1 — selection "jumps" when scrolling.** Root cause was a Go variable
|
||
shadowing regression introduced by the wrap fix: in `frameOf` the chunked
|
||
branch declared `start, end, winLine := cb.VisibleByteRange(...)` inside an
|
||
inner scope, shadowing the function's outer `start, end` — so
|
||
`IMEWindowStartByte` stayed 0 for chunked (multi-chunk) files even while
|
||
scrolled. The selection highlight (window-relative coords, mapped against
|
||
`IMEWindowStartByte`) and IME commits while scrolled were both mis-mapped
|
||
to near the file top. Fixed by declaring `winLine` in the outer scope and
|
||
assigning (not re-declaring). Regression tests:
|
||
`internal/test/e2e/real_file_scroll_selection_test.go` (window selection at
|
||
scroll: `IMEWindowStartByte` and window-relative selection asserted; IME
|
||
commit at scroll lands on the right line) — both mutation-verified against
|
||
the shadowing.
|
||
|
||
**Bug 2 — bottom bar hidden behind the keyboard.** Root cause chain:
|
||
`GioView extends SurfaceView` (GioView.java) → SurfaceView unconditionally
|
||
calls `requestTransparentRegion` → the window is marked
|
||
`FORMAT_TRANSLUCENT` (ViewRootImpl) → translucent windows are never resized
|
||
by the IME → `windowSoftInputMode=adjustResize` is dead. No theme or
|
||
background change can override this (verified: both patches attempted,
|
||
format stayed TRANSLUCENT). Fix (emulator + phone build scripts): a smali
|
||
patch adds `PadInsetsListener` (a `View$OnApplyWindowInsetsListener` on the
|
||
GioView) and, in `GioActivity.onCreate` (API 30+ only),
|
||
`window.setDecorFitsSystemWindows(false)` so IME insets are dispatched to
|
||
the view; the listener shrinks the GioView by the IME inset bottom
|
||
(`insets.getInsets(Type.ime()).bottom`), so the Go side sees a smaller
|
||
surface and lays the bottom bar above the keyboard. On API < 30 there are
|
||
no IME insets; the keyboard overlaps there (accepted limitation).
|
||
|
||
**Bug 3 (found while fixing 2) — keyboard impossible to dismiss.**
|
||
`TextField.Draw` issued `key.SoftKeyboardCmd{Show: true}` every frame while
|
||
focused. With the insets patch, each step of the keyboard's HIDE animation
|
||
dispatches insets → requestLayout → surface resize → Go frame → Draw →
|
||
show → the keyboard re-shows mid-animation. Fix: a `ShowIMESeq` pulse from
|
||
the logic layer (bumped on file open, tap, double-tap) — the renderer
|
||
issues show only when the pulse changes (and re-arms on focus loss),
|
||
matching `widget.Editor`, which shows on focus gain/click only, never per
|
||
frame.
|
||
|
||
**On-device (emulator):** keyboard dismisses with BACK and stays down;
|
||
tapping the editor re-shows it; typing still works; bottom bar visible
|
||
above the keyboard; double-tap word selection stays anchored to the same
|
||
word across a flick scroll. `go test -race ./...` green. Phone APK rebuilt
|
||
with the same smali patch.
|
||
|
||
## 6. File-size decision (re-framed)
|
||
|
||
v1 framed this as "accept a limit vs build a windowed editor." The live repo
|
||
already chose the better option — a **chunked buffer** (only viewport ± window
|
||
chunks resident, async prefetch, dirty-chunk protection, async line index). So the
|
||
decision is now: **validate the existing chunked buffer against a real 10 MB file
|
||
(Phase 3) and set the limit from the measurement.** No fork of Gio is on the
|
||
critical path. The chunked buffer proved out on a real 10 MB file (Phase 3, §5):
|
||
10 MB+ editing works with ~linear, bounded memory, without the 0.5 GB/MB wall
|
||
that kills `widget.Editor`.
|
||
|
||
## 7. Spec deltas (WRITTEN into spec.md, 2026-08-16 rewrite)
|
||
|
||
1. ✓ "No practical limits / file never fully in memory" → replaced by the
|
||
measured 50 MB hard limit + `TooLarge` state (spec.md §2.3).
|
||
2. ✓ Correction: undo is **not implemented at all** (not "in-session only") —
|
||
there is no undo stack in the code; listed as deferred (spec.md §7).
|
||
3. ✓ IME/swipe/autocorrect support is explicit (delivered Phase 1; spec.md
|
||
§2.2). Real-device swipe sign-off remains the one open validation item.
|
||
4. ✓ Correction: external-change detection is **not implemented** (no mtime
|
||
compare on open/resume, no watcher) — listed as deferred (spec.md §7), not
|
||
as a current requirement.
|
||
|
||
## 8. Non-goals (v1)
|
||
|
||
- File-system watcher / live browser refresh (re-scan on return to browser).
|
||
- Tabs, split view, syntax highlighting, diff/merge. (Search-in-file was
|
||
later implemented; see spec §2.2.)
|
||
- Desktop/other platforms (Android-first; window `390×844` dp).
|
||
- Rewriting on `widget.Editor` (v1 plan) — kept only as fallback (§12).
|
||
|
||
## 9. Testing strategy
|
||
|
||
The **existing** e2e harness is **kept** (it builds and passes now) and extended —
|
||
v1's "delete the harness" is wrong for Path B. E2E regression works in three layers:
|
||
|
||
### Layer 1 — Go unit tests on pure logic (the bulk)
|
||
- Port/keep `browser` sort/search/scan tests; keep `editor` chunked-buffer,
|
||
cursor, autosave tests.
|
||
- **New:** range-replace on `ChunkedBuffer` (§4.3) — pure, directly testable; the
|
||
exact corruption bug becomes a regression test.
|
||
- **New:** IME state transitions — after an `EditEvent{Range,Text}`, assert
|
||
buffer + caret; assert the emitted `SelectionCmd`/`SnippetCmd` reflect state.
|
||
- **New:** restore JSON round-trip; offset-vs-mtime validation; external-change
|
||
decision table; file-size guard.
|
||
- CI gate: `go build && go vet && go test -race ./...`.
|
||
|
||
### Layer 2 — UI thin by construction
|
||
Draw functions are wirings of the existing element model over `app.State`;
|
||
positioning math stays in one place (`render.go`). The cursor/caret geometry that
|
||
§4.1 reuses is the same code path as drawing, so IME correctness and visual
|
||
correctness share a foundation.
|
||
|
||
### Layer 3 — scripted on-device e2e (emulator now available)
|
||
The in-app selftest drives the **real state machine through the real draw path**
|
||
(browser entries → open file → edit → autosave deadline → reload-prompt decision),
|
||
logging PASS/FAIL to a file the host `adb pull`s and asserts.
|
||
|
||
### 9.3 Emulator observation layer (this 16 GB VM, KVM verified)
|
||
Gio renders into one GL surface, so Android's view hierarchy exposes nothing about
|
||
our UI. The debug loop is data-based — *more* precise than pixels for this codebase.
|
||
Vision is now enabled and verified in-session (screenshots can be read), but it is
|
||
only an **auxiliary** check: state dumps, logcat, in-app frame-delta logs, and
|
||
file diffs remain the authoritative correctness signals.
|
||
1. **State-dump debug flag** (add in Phase 1): debug builds write `app.State`
|
||
(browser entries, editor text length, cursor px, scroll, dirty, row geometry)
|
||
as JSON to the app data dir on a magic tap or 2 s interval; host `adb pull`s
|
||
and asserts. Primary "eyes."
|
||
2. **logcat** — Go panics + our logs.
|
||
3. **Frame timing for the 60 fps claims — `dumpsys gfxinfo` DOES NOT WORK
|
||
here (verified 2026-08-16): it reported 0 frames while the app was visibly
|
||
rendering, because Pad draws into a `SurfaceView` and gfxinfo only measures
|
||
the View-layer pipeline. Use temporary in-app instrumentation instead:
|
||
timestamp each frame in the main loop, log deltas, report p50/p90/p99/max.
|
||
`screenrecord` is a human artifact, not an agent metric (it captures ~22
|
||
fps with encoder-confounded timestamps).
|
||
4. **Screenshot → PIL color histogram / tesseract OCR** — coarse on-screen checks.
|
||
5. **File diffs** via `adb pull` — autosave/restore correctness.
|
||
6. **Driving** — `input tap/swipe/keyevent/text`, `am start/force-stop`,
|
||
`ime`/`settings` to pick the AOSP keyboard; swipe-typing exercised via
|
||
`input swipe` over the keyboard area and verified **behaviorally** (did the
|
||
commit land, non-duplicated, cursor correct?), not visually.
|
||
|
||
## 10. Definition of done (whole effort)
|
||
|
||
On an emulator (and a real phone for final IME sign-off): launch → previous file
|
||
+ caret restored → browse a large directory smoothly → open a real file →
|
||
**swipe-type with autocorrect, clean and non-duplicated** → close and reopen, caret
|
||
where you left it → external modification → reload prompt → autosave lands on disk
|
||
within ~1 s. Repo: `go test -race ./...` green.
|
||
|
||
## 11. Risks / watch-items
|
||
- **§4.1 caret plumbing** is coupled to the cursor-positioning geometry; if that
|
||
geometry is itself buggy, the IME will inherit it. Mitigation: the state-dump
|
||
reports caret px, so we can assert it against expected values in tests.
|
||
- **Channel model + `-race`** (Phase 0) — **resolved** in `58725a5` (§13);
|
||
`-race` is green and is now a standing regression gate.
|
||
- **Chunked buffer at 10 MB** (Phase 3) — **resolved**: opens a real 10 MB file
|
||
in ~121 ms, renders and scrolls smoothly, ~150 MB PSS at steady state. The 50 MB
|
||
limit is set from this measurement (§5 Phase 3, commit `c79c142`).
|
||
- **Chunked buffer fixed-slot drift** (Phase 3) — **resolved**: the buffer was
|
||
rewritten off the fixed `[i*chunkSize, (i+1)*chunkSize)` slot model. It now
|
||
holds an ordered `[][]byte` chunk slice with **prefix-sum byte offsets** and
|
||
full-loads in-range files on open (no lazy loading, no eviction), so byte↔chunk
|
||
mapping stays correct after length-changing edits. A rope is no longer needed
|
||
for the 50 MB target.
|
||
- **Whole-file shaper memory leak** (Phase 3) — **resolved**: `VisibleByteRange`
|
||
no longer falls back to `end=fileLen` via the window-only `VisualLineStarts`;
|
||
the range is always bounded by the real-line `LineIndex`. Memory is now stable
|
||
and scales ~linearly with file size (commit `c79c142`).
|
||
- **AOSP keyboard** is a proxy for real IMEs (Gboard, etc.); final swipe/autocorrect
|
||
sign-off needs a real device with a real IME.
|
||
- **Viewport opens at EOF** (Phase 2) — **resolved**: the opening tap/scroll is
|
||
swallowed for ~300 ms via a `justOpenedAt` window and `ScrollOffset` is
|
||
re-clamped in `EditorLayout` as a safety net (commit `3460ef3`).
|
||
|
||
## 12. Why v1 (widget rebuild) is now a fallback, not the plan
|
||
v1 was written against a stale snapshot (May 31) and concluded "delete
|
||
`internal/ui`, build on `widget.Editor`." Two later-verified facts changed that:
|
||
(a) the live repo already solved the two hardest parts a rebuild would discard
|
||
(real FS, APK, and the chunked buffer that `widget.Editor` cannot do), and (b) the
|
||
Android IME is reachable from the op layer with only the §4 gap — so the "free IME"
|
||
that motivated the rebuild is actually ~4 small wirings away on the existing code.
|
||
**Path A (widget rebuild) remains the fallback if the Phase 1 IME wiring proves
|
||
fragile on-device** (e.g., the caret/snippet plumbing drags in a long tail of
|
||
cursor-positioning bugs that are cheaper to not own). The emulator (Phase 2) is the
|
||
instrument that makes that call.
|
||
|
||
## 13. Single-owner state refactor (`58725a5`)
|
||
|
||
`go test -race ./...` was red. The races were architectural, not incidental —
|
||
several violated `doc/architecture.md §1` (logic goroutine = sole owner of
|
||
mutable state). Fixes, in order of impact:
|
||
|
||
1. **Frame is the only cross-goroutine state carrier.** `editor.Frame` now
|
||
carries `Elems`, `Scale`, `FocusedElementID`, and `Query`. The main goroutine
|
||
reads *only* the frame-receiver-stored snapshot (under its mutex); it no
|
||
longer calls `logic.State()` for scale/focus/search. `Renderer.Draw` takes
|
||
the scale as a parameter; the `ScaleProvider` indirection is gone.
|
||
2. **Gio-mutable widgets are main-owned.** `browser.BrowserState.SearchEditor`
|
||
(a `widget.Editor`) was removed. Main owns the search `widget.Editor` and
|
||
forwards its text via `SearchQueryChan`; the logic stores the result in
|
||
`Browser.Query`. More generally, `Renderer` now owns all `widget.Editor`
|
||
instances (registered by element ID) because Gio mutates them during draw.
|
||
3. **Autosave is owner-mediated.** The 1 s debounce timer goroutine no longer
|
||
reads editor state; it sends a `struct{}` token on `autosaveChan` and the
|
||
owner reconstructs content and dispatches the write.
|
||
4. **Shutdown is ordered and waitable.** `Shutdown()` = `Done()` →
|
||
`WaitForExit()` → `FlushAll()` → `workerPool.Stop()`, so `FlushAll` (lock-free)
|
||
only ever runs after the owner has exited or from the owner itself.
|
||
5. **Tests inspect via the owner.** `Harness.Inspect/WithState/FileLoaded/
|
||
FullContent/CursorPosition` (and the in-package `withState`/`l.Inspect`
|
||
helpers) execute callbacks on the logic goroutine. `Harness.Run` now panics
|
||
if called twice — `TestTypeAtStartOfBuffer` and
|
||
`TestEditorClickToMoveCursorWithScroll` had a *double `Run()`* (two logic
|
||
loops on one state) that was the largest race source.
|
||
6. **Test-only flakes fixed:** `TestWorkerPool_PriorityPreemption` was rewritten
|
||
deterministically (gate task holds the worker while both priorities queue —
|
||
the old version raced the worker's task pickup; Go's `select` is random when
|
||
both channels are ready, so the old ordering guarantee was unimplementable as
|
||
written). `TestLazyLoadingLargeDirectory` timeout raised 2 s → 15 s (a 10k
|
||
entry index build under `-race` exceeds 2 s).
|
||
|
||
Residual (accepted) invariants:
|
||
- `editor.TheState` is a global used by package-level mutators (`GoToEditor`,
|
||
`OpenFile`, `HandleInsert`, …); it is only safe to call them from the owner
|
||
goroutine (input-handler closures run on the owner via `SendInput`).
|
||
- `Logic.FlushAll` is lock-free by design; it must not run concurrently with
|
||
`Run` (guaranteed by the `Shutdown` order).
|
||
|
||
## 14. Android window geometry + selection menu tracking (2026-08-17)
|
||
|
||
Three user-visible bugs, all on the Android target: (1) the word-selection
|
||
menu did not follow the selected text when scrolling; (2) a large empty gap
|
||
between the keyboard top and the app's bottom bar when the IME was shown;
|
||
(3) the top bar rendered under the status bar and could not be clicked.
|
||
|
||
**Root cause of (2) and (3): the SurfaceView window cannot be resized by
|
||
Android.** Gio's `GioView` is a `SurfaceView`; the moment it attaches, the
|
||
window becomes translucent (the surface has a transparent region), and
|
||
`windowSoftInputMode=adjustResize` no longer resizes it — the keyboard
|
||
simply overlaps the window's bottom. No theme, background, or flag change
|
||
can make the window opaque again. The fix is to consume the insets *in
|
||
app*: a smali-injected `PadInsetsListener` (see `scripts/build_emu.sh`,
|
||
identical block in `scripts/build_phone.sh`) sets
|
||
`setOnApplyWindowInsetsListener` on the GioView in `GioActivity.onCreate`
|
||
(API 21+) and `setDecorFitsSystemWindows(false)` (API 30+, which delivers
|
||
IME insets at targetSdk 34). On every insets dispatch the listener shrinks
|
||
the view:
|
||
|
||
- `displayH = displayMetrics.heightPixels + statusTop + navBottom` — the
|
||
metrics report the *content* height, while IME insets are measured from
|
||
the absolute display bottom; the two frames must be reconciled before
|
||
subtracting.
|
||
- `bottomLimit = displayH − imeInset` (IME shown) or `displayH − navBottom`
|
||
(IME hidden); `height = bottomLimit − statusTop`, clamped ≥ 0.
|
||
- `topMargin = statusTop`, `bottomMargin = navBottom`, so the view spans
|
||
exactly `statusBarBottom..keyboardTop` (or `..navBarTop`).
|
||
|
||
On API < 30 there are no per-type insets; the listener degrades to no-op
|
||
and the keyboard overlaps (accepted limitation — both build scripts target
|
||
minSdk 16).
|
||
|
||
**Layout-params invariant (the subtle bug found while verifying):** the
|
||
height, topMargin, and bottomMargin change-detection checks must *all* run
|
||
on every dispatch, with a single `requestLayout` if any of the three
|
||
changed. The first version `goto`-skipped the margin checks whenever the
|
||
height changed — which is exactly the transition (keyboard show/hide), so
|
||
the margins were silently never applied during IME state changes and only
|
||
accidentally picked up on a later duplicate dispatch. The listener now
|
||
tracks a changed flag across all three checks.
|
||
|
||
**Selection menu tracking (1):** the menu anchor is recomputed every frame
|
||
in the logic goroutine (`positionSelectionMenu`): while a selection is
|
||
visible, its screen position is derived from the current layout (byte
|
||
offset → glyph → window-relative Dp) and the menu re-anchors to it; if the
|
||
selected word scrolls fully out of the viewport the menu hides instead of
|
||
stranding. Regression: `internal/editor/selection_menu_track_test.go`.
|
||
|
||
**Build-script invariant:** the Python patch heredoc in `build_emu.sh` and
|
||
`build_phone.sh` must stay byte-identical (both inject the same
|
||
`PadInsetsListener` class and the same `GioActivity` wiring); a drift would
|
||
produce an emulator APK that behaves differently from the phone APK. The
|
||
listener uses only int arithmetic and registers v0–v5 (the smali 22c/11x
|
||
formats accept only 4-bit register operands; apktool 3.0.3's smali also
|
||
lacks `cmpg-f`).
|
||
|
||
On-device verified (Pixel 6 profile, API 35, 1080×2400): top bar below the
|
||
status bar and clickable; bottom bar flush with the keyboard top (view
|
||
spans 128..1517 with the IME shown); BACK dismisses the keyboard and
|
||
re-tap re-raises it without a re-show loop; typing saves; the selection
|
||
menu tracks small scrolls and hides when the word leaves the viewport.
|
||
|
||
## 15. Selection handles: teardrop shape, grab box, and menu placement (2026-08-18)
|
||
|
||
**Handles.** The selection/caret handles are teardrops (a stem from the
|
||
caret point to a filled circle, like the native Android selector). The
|
||
circle is 20dp normal and 28dp while its drag is active. The GRAB region
|
||
is deliberately much larger than the visual: a 48dp box centred on the
|
||
circle centre. The visual size and the touch target are independent —
|
||
small visuals with 48dp targets is the native behaviour, and the earlier
|
||
16dp target was not grabbable by finger.
|
||
|
||
**Menu placement contract: above first, below on overflow.** The
|
||
copy/cut/paste menu is placed ABOVE the selected line (like the native
|
||
Android selection toolbar), flipping below only when there is no room
|
||
above the window, and clamped to the window edges. This is not a matter
|
||
of taste: the handles hang off the line's BOTTOM edge, and the menu is
|
||
drawn last, so it sits on top of the z-order — Gio routes a touch to the
|
||
topmost op whose clip contains it. A menu placed below the line therefore
|
||
covers the handles' grab boxes and silently steals every touch meant for
|
||
a handle drag (a drag can only grab once it has received the PRESS; a
|
||
press captured by the menu's click never reaches the drag, no matter how
|
||
far the finger then moves). Verified on device: with the menu up, drags
|
||
from the previously-dead lower grab region work once the menu is above;
|
||
in the first-line case (menu flipped below) the handles are still
|
||
grabbable from the uncovered top strip of their boxes.
|
||
|
||
Regressions: `internal/editor/selection_menu_track_test.go` (tracking +
|
||
above-placement arithmetic + first-line flip).
|
||
|
||
## 16. Top bar: single row (2026-08-18)
|
||
|
||
The dead cut/copy/paste icon row was removed from the editor top bar
|
||
(clipboard actions live exclusively in the floating selection menu, the
|
||
native pattern). The bar is now one 32dp row — back icon + filename —
|
||
down from two rows at 52dp, giving the editor 20dp more height. The
|
||
menu's "above first" placement (section 15) is unchanged; with the
|
||
shorter bar, selections on the first lines flip below because there is
|
||
no room above, and the menu may overlap the top bar when clamped high —
|
||
both are the native toolbar's behaviour (transient, dismissed by
|
||
tapping elsewhere).
|
||
|
||
## 17. Handle drags, menu anchoring, and the left-edge gesture (2026-08-18)
|
||
|
||
**Relative line mapping for handle drags.** A selection handle's 48dp grab
|
||
box is centred BELOW its line (the teardrop hangs off the line's bottom
|
||
edge), so a press usually lands on the neighbouring line. Mapping the
|
||
finger to its own line was fatal for a start-handle drag: the
|
||
neighbouring-line byte is usually past the other handle, the clamp
|
||
collapses the selection to zero length, the selection is cleared — and a
|
||
cleared selection un-registers the drag op, so Gio's router silently
|
||
stops delivering drag events to it (an inactive handler is deleted at the
|
||
next frame boundary without a cancel). Every downstream symptom — the
|
||
"stream cutoff", a tap landing on release, the system back gesture
|
||
firing on subsequent edge swipes — was a consequence of that single
|
||
collapse, not a system-side touch filter. The first attempt at a fix
|
||
locked the anchor to its own line entirely, which removed the bug but
|
||
also removed the native ability to drag a handle across lines.
|
||
|
||
Contract: a handle anchor moves RELATIVE to its own visual line. The
|
||
finger's vertical displacement from the GRAB position, in whole visual
|
||
line heights, selects the target line (less than half a line: the
|
||
anchor's own line); the finger's X is projected onto that line. A
|
||
stationary or horizontal drag therefore never moves the anchor off its
|
||
line (the grab is safe), while a deliberate vertical drag walks the
|
||
anchor across lines — dragging the end handle down extends the selection
|
||
downward, dragging the start handle up extends it upward. The mapping is
|
||
relative (displacement from the grab, not the finger's absolute line)
|
||
precisely because the grab is usually a line or more below the anchor:
|
||
tracking the finger's absolute line would first drag the anchor the wrong
|
||
way, through a collapse, before it ever reached the anchor's line. The
|
||
base line is captured **once at the grab** and never re-resolved from the
|
||
anchor's current byte: that would feed the anchor's own movement back
|
||
into its target line, and a finger jittering near a line boundary would
|
||
race the anchor off the screen (one extra line per event — reported as
|
||
"tiny vertical movements jump the anchor to the top/bottom of the
|
||
screen"). Regressions in `touch_selection_test.go`:
|
||
`TestSelDrag_StartHandle_PressOnLineBelow_KeepsSelection` (the original
|
||
bug's geometry), `TestSelDrag_EndHandle_DragDownExtendsAcrossLines`,
|
||
`TestSelDrag_StartHandle_DragUpExtendsToLineAbove`,
|
||
`TestSelDrag_EndHandle_JitterNearLineBoundary_Stable` (the runaway) — all
|
||
mutation-verified against the pre-fix variants.
|
||
|
||
**Menu anchors to the stable end.** While a start-handle drag is in
|
||
progress the selection start is the moving end, so the menu anchors to
|
||
the selection END (and vice-versa the default anchor is the start). The
|
||
menu therefore never chases the finger: dragging the end handle away
|
||
leaves the menu parked by the selection start, clear of the finger's
|
||
path and (combined with the above/below contract of section 15) clear of
|
||
the selected text. Verified on device: a long vertical end-handle drag
|
||
with the menu up leaves the menu, the highlight and both handles
|
||
undisturbed.
|
||
|
||
**The menu is never sticky-hidden while any part of the selection is
|
||
visible.** If both selection ends fall outside the shaped window (e.g.
|
||
the viewport scrolled away) but the selection still intersects the
|
||
window, the menu keeps its previous position, clamped inside the window;
|
||
it is hidden only when the whole selection is off-window, and re-anchoring
|
||
(the next selection change) re-shows it. This replaces the earlier
|
||
behaviour where an off-window anchor coordinate hid the menu
|
||
permanently. Regression:
|
||
`TestSelectionMenu_KeptVisibleWhileSelectionPartiallyOffScreen`.
|
||
|
||
**Left-edge exclusion for the start handle.** On gesture navigation
|
||
(API 30+/35) a horizontal rightward swipe starting within ~23dp of the
|
||
left screen edge starts the system BACK gesture; the gesture previews,
|
||
cancels the in-app touch stream (ACTION_CANCEL), hides the IME and
|
||
navigates. A start handle at the beginning of a line sits at screen
|
||
x≈26px — inside that zone — so grabbing it with a horizontal flick
|
||
navigated away instead of dragging. The app cannot disable the system
|
||
back gesture, but Android lets a view opt specific rects out via
|
||
`View.setSystemGestureExclusionRects` (API 29+). The editor reports the
|
||
two selection-handle grab rects (screen px) every frame; the app
|
||
forwards them to the view. Two invariants: the call must run on the
|
||
Android UI thread (the Go frame loop is not it — it is marshalled via a
|
||
tiny `PadExcl` Runnable posted through `View.post`, generated by the
|
||
build scripts' smali patch, which must stay byte-identical between
|
||
`build_emu.sh` and `build_phone.sh`), and the rects are in the view's
|
||
coordinate system (== window-local px for the full-screen GioView).
|
||
Verified on device: with the exclusion active, horizontal drags from the
|
||
left-edge start handle no longer trigger `startBackNavigation`
|
||
(`dumpsys window` shows the exclusion region; logcat shows zero
|
||
back-gesture previews).
|
||
|
||
## 18. Pinch-to-change-font-size, continuous (2026-08-22)
|
||
|
||
A two-finger pinch in the editor now changes the app's font size smoothly,
|
||
without snapping to whole points. The app-local scale is a float32
|
||
(default 1.0, clamped 0.5–3.0) layered on top of the system user font
|
||
setting; it is never rounded anywhere in the pipeline.
|
||
|
||
**Renderer (`internal/ui`).** Gio v0.10 has no two-finger pinch primitive,
|
||
so the Renderer owns a probe event tag clipped to the editor text region
|
||
(next to the long-press probe). It tracks the active pointers across frames
|
||
(window-px positions keyed by pointer ID) and emits one relative factor per
|
||
frame — `pinchDist(cur)/pinchDist(prev)` — as `ui.FontPinchEvent` to the
|
||
editor's new `ui.Pinch` interaction handler. `pinchDist` (two lowest-ID
|
||
pointers) is a pure function, unit-tested. While a pinch is active, scroll
|
||
emission is suppressed so the first finger does not drag the text.
|
||
`drawWrappedText` multiplies the editor's sp size by the frame's
|
||
`AppFontScale` (main goroutine feeds it via `SetAppFontScale` before
|
||
Draw); ascent/line-height/highlight/caret/handles all follow because they
|
||
derive from the same size.
|
||
|
||
**Logic (`internal/editor`).** `State.appFontScale` + `HandleFontPinch`
|
||
(multiply by the per-frame factor and clamp). The anchor is the pinch
|
||
CENTER, not the viewport top — and it is a **content point**, not a layout
|
||
point: `State.captureContentPin` names the glyph under the midpoint (the
|
||
ABSOLUTE buffer byte — the layout's `ByteOffsets` are window-relative, so
|
||
the capture adds `IMEWindowStartByte` — plus the point's offset from that
|
||
glyph's baseline), captured under the pre-change layout. A rewrap moves
|
||
the *text* of a visual line (the same fragment index holds different bytes
|
||
after the rewrap), so pinning (line, fragment) would leave a different
|
||
character at the center; naming the byte does not. The offset is applied
|
||
in two phases: (1) immediately, `rescaleScrollAnchored` rescales `S + m`
|
||
about the center (continuous, valid until rewrap lands); (2) on **every
|
||
newly shaped layout at the current scale** — the re-shape after the font
|
||
change and the rewrap corrections that follow it — `refineContentPin`
|
||
recomputes the offset from the pinned byte's fresh baseline:
|
||
`S' = vk·lh + Y + Dy − m`, where `vk = VisualsBefore(WindowStartLine)` is
|
||
the window's FIRST visual line (the window top sits at content `vk·lh`,
|
||
NOT `floor(S/lh)·lh` — a different line whenever the viewport top lands
|
||
mid-way through a wrapped logical line) and `Y` is the byte's baseline in
|
||
the fresh layout (located by its absolute byte, window start from
|
||
`LayoutFeedback.WindowStartByte`). That lands the byte exactly on the
|
||
center and is a fixed point when the layout already agrees (no drift, no
|
||
oscillation). Two stale-data traps had to be closed: the scale change
|
||
**invalidates the last shaped layout** (`invalidateShapedLayout`) —
|
||
otherwise the next frame computes its window start with the OLD line
|
||
height and the NEW rescaled offset, a window ~10k lines off — and
|
||
`refreshFontPin` skips feedback shaped at a different scale (during a
|
||
pinch every frame changes the scale, so all but the latest feedback are
|
||
stale). While armed (2 s, refreshed by feedback) the pin rides every
|
||
frame; a (line, fragment, sub-line) anchor (`captureFontPin`/`applyFontPin`)
|
||
stands in for points off any glyph; an edit (EditSeq mismatch), a scroll,
|
||
or the timeout disarms it. `SetAppFontScale` (the `fontsize` debug
|
||
command) keeps the top-anchored behavior (no fingers to center on) and
|
||
invalidates the stale layout too. `EffectiveLineHeight()` is now system ×
|
||
app; every geometry consumer (window start, tap mapping, scroll clamp,
|
||
restore) was already routed through it. `Frame.AppFontScale` carries the
|
||
value to the renderer; `LayoutFeedback.ScrollOffset` carries the shaped
|
||
scroll back (used by the fallback path and diagnostics).
|
||
|
||
**Persistence.** The session snapshot gains `AppFontScale`, and
|
||
`ScrollSub` is now stored as a *fraction* of the line height (font-
|
||
independent; legacy Dp values > 1 are converted on restore).
|
||
|
||
**Testing.** `pinch_font_test.go` (continuous product of small factors,
|
||
clamp at both ends, center-anchor invariance, glyph hit-testing, content-
|
||
pin capture, the refine keeping the pinned BYTE on the center across a
|
||
rewrap that moves it to another fragment, the fixed-point property, the
|
||
(line, fragment) fallback, fragment clamp on pinch-out, bad-data no-op),
|
||
`pinch_test.go` (pinchDist/pinchMid geometry,
|
||
factor-series telescoping). On the emulator
|
||
(`scripts/emu.sh cmd pinch <F>` / `fontsize <F>` — one-shot commands that
|
||
drive the same `HandleFontPinch` path a real pinch delivers, since adb has
|
||
no two-finger input; `pinch` anchors at the editor-region center): five
|
||
×1.05 steps produced line pitches 44→46→49→51→54→56 px (autocorrelation-
|
||
measured) — continuous, no whole-point snapping; on a 40k-line wrapped
|
||
file scrolled to the middle, a pinch in/out cycle (×1.5 → ×0.75 →
|
||
×1.125) exercised the rewrap in BOTH directions (1→2 and 2→1 fragments
|
||
per line) and the pin converged to a fixed point within 2–3 layout-
|
||
feedback frames at every step, keeping the captured BYTE's line on the
|
||
region center (verified against the app's own geometry, not the pixels);
|
||
the ground-truth tap test (tap a line, type a marker, read the file)
|
||
passed at a 1.125× scale after two rewrapping pinches; `fontsize` keeps
|
||
the top anchor across a 1→2 rewrap; font scale, file, cursor and scroll
|
||
all survive a full restart; 0.5/3.0 clamps hold; one-finger scroll is
|
||
unaffected (and takes over the viewport, disarming the pin).
|
||
|
||
**Bugs found by the emulator round (both would have passed the unit
|
||
suite).** (1) `GlyphLayout.ByteOffsets` are window-relative, not absolute:
|
||
capturing the pin's byte without adding the window start made the pin
|
||
chase a moving offset and never converge. (2) The scale change left the
|
||
old `GlyphLayout` in place; the next frame computed its window start with
|
||
the OLD line height and the NEW rescaled offset — a window ~10k lines
|
||
from the viewport (visible as a ~1000-line jump). Fixed by
|
||
`invalidateShapedLayout()` on every scale change.
|
||
|
||
**Bug found by the on-device round (the emulator round could never have
|
||
found it — adb has no two-finger input, and the debug `pinch` command
|
||
bypasses the probe entirely).** On the phone a real two-finger pinch did
|
||
nothing. On-device logcat (probe event logs + per-frame event counts)
|
||
showed the scroll gesture receiving every finger move while both probe
|
||
tags received zero events. Root cause: the probes were declared as
|
||
`struct{}` fields of the Renderer. The unnamed fieldless `struct{}` is a
|
||
SINGLE canonical Go type, so `pressProbe`, `pinchProbe` (and a
|
||
diagnostic third) were the SAME tag value. Gio's router keys handlers by
|
||
tag value, so all three `event.Op` registrations collapsed into one
|
||
handler; the press probe's drain (which runs first in `CheckGestures`)
|
||
consumed every event for that tag and the pinch probe was structurally
|
||
starved. Fixed by giving each probe its own named type
|
||
(`pressProbeTag`, `pinchProbeTag`), with a regression test
|
||
(`TestProbeTagIdentity`) asserting the tags remain distinct map keys, and
|
||
`TestRealDrawOpsProbeHit`, which runs the real `Renderer.Draw` op stream
|
||
through a real `input.Router` and asserts the probe tags receive the
|
||
pointer press. Verified on the Pixel 9 Pro: 927 probe events across a
|
||
multi-pinch session, 137 per-frame factors emitted and applied (net
|
||
scale 1.70×, font visibly enlarged), scroll suppressed mid-pinch,
|
||
anchor held.
|
||
|
||
## 19. Pinch tracker: explicit pair, two-mover formation, slow frames (2026-08-23)
|
||
|
||
The §18 probe design ("two lowest-ID pointers") was replaced by an explicit
|
||
pair state machine (`internal/ui/pinch_tracker.go`, pure and unit-tested;
|
||
the renderer's `consumePinchProbe` is now a thin adapter that feeds events,
|
||
executes the tracker's grabs, and emits its factor). Four on-device failure
|
||
modes drove the rewrite, plus a whole class of slow-frame bugs only visible
|
||
on the ~1 fps emulator:
|
||
|
||
1. **Single-finger scroll changed the font** — the pair was re-derived from
|
||
whatever pointers happened to be present, so a scroll finger got paired
|
||
with a stale pointer and its drags became "pinch".
|
||
2. **Scroll-down enlarged the font** — same root: the scroll finger's
|
||
distance to a stale second pointer grows as it moves.
|
||
3. **Two fingers produced a sudden zoom before the pinch** — the baseline
|
||
(`prevDist`) survived from the previous pinch, so a new pinch 2.5× wider
|
||
emitted 2.5× on its first frame.
|
||
4. **Pinch-out stopped and became a scroll** — the pair was not explicit;
|
||
once a finger moved past the scroll slop the router handed the pointer
|
||
to the scroll gesture and the "pair" silently switched composition.
|
||
|
||
**Explicit, stable pair + grabs.** When the pair forms, the adapter issues
|
||
`pointer.GrabCmd` for BOTH fingers (exclusive delivery to the probe:
|
||
releases arrive even off-clip, and scroll/click are dropped with a Cancel —
|
||
so the pair can never be stolen mid-gesture, failure mode 4). The pair's
|
||
composition and its baseline are never re-derived from ambient pointers.
|
||
`factor()` = current pair distance / previous frame's distance, one factor
|
||
per frame (the Android driver replays historical samples — several drags per
|
||
frame — so the font sees one factor per frame, not per sample). A sanity
|
||
clamp drops factors outside 0.1–10 and advances the baseline, so a
|
||
teleporting pointer (ID-reuse noise) cannot jump the font. When a pair
|
||
finger lifts, the other becomes the **survivor**: it stays grabbed (Gio
|
||
v0.10 has no release-grab) and its drags are forwarded as a plain scroll
|
||
delta (`survivorScroll`), so the finger is not dead. A second finger that
|
||
returns re-forms the pair with a fresh baseline.
|
||
|
||
**Formation requires TWO MOVING fingers — and nothing else.** The dominant
|
||
real-world case is a palm edge already down when the two pinch fingers
|
||
land; a static rule about "which finger is the palm" (the oldest? the
|
||
newest? the still one?) cannot survive both palm-first and palm-last hand
|
||
lands. Movement is the only signal that works for both: while 2–3 fresh
|
||
fingers are down the tracker is *pending*; the pair forms — at `factor()`
|
||
time, after the whole frame's events, never per-event (per-event locking
|
||
in the first mover pair seen would pair a finger with a drifting palm) —
|
||
when two pending fingers have each moved more than `pinchMoveEps` (10 px)
|
||
from where they pressed, and it is the mover pair whose **distance**
|
||
changed most (a drifting palm's distance to a finger changes little; a
|
||
pinch's does). A lone mover is a scroll, never a pair; a unison movement
|
||
(a two-finger slide) leaves the distance unchanged and forms nothing.
|
||
Consequences verified: a resting (pruned, >300 ms) or still palm can never
|
||
enter the distance; the three fresh-fingers case pairs the pinch fingers;
|
||
the re-form candidate (a finger landing on a survivor) must also move
|
||
before it becomes the pair.
|
||
|
||
**Baseline = the PRESS distance.** Any spread that happened before the pair
|
||
starts is owed, not lost: the formation frame emits `d_current /
|
||
d_press`, and a pinch that breaks before its first `factor()` settles the
|
||
same owed factor at the break.
|
||
|
||
**Slow frames (the ~1 fps emulator batches a whole gesture into one
|
||
drain).** (a) *Born-and-dead in one frame*: presses, drags and BOTH
|
||
releases in one drain — pending releases are held **lazy** (`released`
|
||
map) until `factor()`: the pair forms at the fingers' final positions,
|
||
then breaks there (no survivor when both released), settling the owed
|
||
factor. (b) *Pair broke mid-frame*: `brokeFactor`/`brokeMid` are settled
|
||
at the break (against `prevDist`, or the press distance when fresh) and
|
||
emitted by the subsequent `factor()` call, which would otherwise see
|
||
`on==false` and drop the frame's movement.
|
||
|
||
**Emulator multi-touch injection.** adb has no two-finger input, so the
|
||
failure modes could not be tested end-to-end until `tools/touchinject`: a
|
||
platform-signed (AOSP test key, `INJECT_EVENTS` granted) toy app whose
|
||
broadcast receiver injects a scripted `MotionEvent` stream
|
||
(`down/move/up/wait`, display px) through
|
||
`InputManager.injectInputEvent` — `/dev/uhid` is a dead end (no kernel
|
||
module in the image). Two known flakes, both documented in the harness:
|
||
the receiver process is "cached" and the 1.5 GB emulator OOM-kills it
|
||
mid-script occasionally (the harness verifies `=== done` in logcat and
|
||
re-runs; service routing is blocked by Android 12+ background-start
|
||
restrictions, and the AVD's locked bootloader blocks the system-app
|
||
escalation); and burst drags (all moves within one frame's drain) do not
|
||
scroll — the app is on-demand-rendering at ~1 fps, so scroll tests space
|
||
the moves ~80 ms apart, which also matches what a real finger produces
|
||
over several frames.
|
||
|
||
**End-to-end results (real injected MotionEvents, big wrapped file).**
|
||
Single-finger drag: zero factors, scale unchanged (font), content scrolls.
|
||
Two-finger pinch-out 300→596 px: exactly one factor 596/300 = 1.98667,
|
||
font ~2×. Palm-first three fingers (palm resting and still, pinch fingers
|
||
landing 80/120 ms later): pair is the two pinch fingers — the factor
|
||
tracks the pinch, the palm never enters the distance. Lift one finger
|
||
mid-pinch: the factor stream stops at the lift (font frozen), the
|
||
survivor's 600 px drag scrolls the content 600/3.5 = 171.4 dp. The
|
||
one-frame leak at formation (the pair's own drags of the formation frame
|
||
still reach scroll, since the grabs commit next frame) is bounded by the
|
||
scroll slop — the deliberate cost of not grabbing on press, which would
|
||
kill single-finger scrolls.
|
||
|
||
**Testing.** `pinch_test.go` now covers: factor-series telescoping
|
||
(baseline = press distance), single-finger never scales, resting/stale
|
||
palm excluded, extra finger during an active pinch ignored, fresh baseline
|
||
per pinch, sanity clamp, the three slow-frame shapes (full pinch in one
|
||
frame, stationary born-dead, born-and-dead), palm-first three fingers,
|
||
and survivor scroll + re-form (candidate must move).
|
||
`real_draw_probe_test.go` runs the real `Renderer.Draw` op stream through
|
||
a real `input.Router`: press frame (pending, nothing), formation frame
|
||
(grabs + owed factor, one-frame scroll leak), post-formation frames (scroll
|
||
sees nothing of the pair), off-clip survival, release via the grab,
|
||
survivor scroll forwarding, re-form.
|
||
|
||
## 20. IME open: content must not shift (2026-08-23)
|
||
|
||
**Bug.** With the soft keyboard open (adjustResize), the editor content
|
||
jumped up by exactly 32 dp (112 px) every time the keyboard appeared.
|
||
Top-anchored layout keeps the window start line put when only the
|
||
viewport height changes, so the shift was not our layout: KBW
|
||
instrumentation of every `ScrollOffset` writer showed
|
||
`HandleScroll` receiving a single +112 px delta at the resize frame.
|
||
|
||
**Root cause (Gio, not the app).** `gioui.org/app` `window.go`, on every
|
||
frame whose viewport *shrank*, calls `Router.RevealFocus(viewport)` —
|
||
"scroll the focused widget into view". For a text editor the focused
|
||
field's registered bounds (stale — from the pre-resize, taller frame)
|
||
extend below the new viewport, so RevealFocus synthesizes a
|
||
`pointer.Scroll` event (`Source: Touch`, position (0,0), Y = the nudge)
|
||
delivered to the focused field's scroll handler. `gesture.Scroll`
|
||
consumes it like any wheel scroll → `HandleScroll` → the 32 dp jump.
|
||
The event is invisible to the app: it never enters the pointer queue
|
||
(no `MotionEvent` on the Android side), it is manufactured by the router
|
||
during `processEvent(frameEvent)`, before the app's frame handler runs.
|
||
Reproduced at the router level: `RevealFocus` on a shrunken viewport
|
||
queues exactly one scroll event for the gesture's tag.
|
||
|
||
**Why not the obvious fixes.**
|
||
- Zeroing the scroll *range* on the shrink frame does nothing: the router
|
||
UNIONs scroll ranges into the handler's filter across frames
|
||
(`pointerFilter.Add`/`Merge`), so the historical max can never shrink
|
||
back to zero — the clamp stays at ±∞ forever.
|
||
- Patching `app/window.go` to drop the shrink→RevealFocus call would mean
|
||
shipping a forked gioui (the build constraint is clean v0.10.0).
|
||
- `adjustNothing` removes the resize but hides the cursor line under the
|
||
keyboard.
|
||
|
||
**Fix (app-side, two files).**
|
||
- `cmd/pad/main.go`: on each `FrameEvent`, detect a shrink
|
||
(`e.Size` smaller than the previous frame's) and set
|
||
`renderer.ZeroWheelScroll` for that one frame.
|
||
- `internal/ui/render.go` (`CheckGestures`): when flagged, drain
|
||
`pointer.Scroll` events for the editor scroll gesture's tag
|
||
(`q.Event(pointer.Filter{Target: reg.scroll, Kinds: pointer.Scroll})`)
|
||
before `gesture.Scroll.Update` consumes anything. Only the synthesized
|
||
nudge matches: finger scroll is `pointer.Drag`, inertia is the
|
||
flinger, and on a phone there is no trackpad wheel. Normal frames are
|
||
untouched.
|
||
|
||
**Verification.**
|
||
- `reveal_focus_drain_test.go`: real `input.Router` + real
|
||
`Renderer.Draw` ops; the focused field (KeyDown interaction required —
|
||
it records the `event.Op` tag reference, and a per-frame
|
||
`key.FocusFilter` consumer marks the handler focusable, else the key
|
||
queue clears the focus each frame), shrunken viewport, `RevealFocus` →
|
||
exactly one synthetic scroll queued for the gesture tag; the drain
|
||
terminates and `gesture.Scroll.Update` then returns 0.
|
||
- Emulator E2E (real injected taps, keyboard really opens, window
|
||
2560→1527 px): pre-fix the scroll gesture emitted delta=112 on the
|
||
shrink frame and a screenshot cross-correlation showed a 112 px content
|
||
shift; post-fix the drain consumes the event, the gesture delta is 0,
|
||
and the cross-correlation shift is 0 (corr 0.987). The perf-CSV
|
||
(ScrollDP per logic frame) shows no 32 dp step when the keyboard opens
|
||
on the final build.
|
||
|
||
**Notes.** The `aosp_atd` emulator later started ANR-ing Pad on first
|
||
frame — the ANR trace shows the main thread in
|
||
`GioView.onFrameCallback` → `glDeleteBuffers` → gfxstream guest →
|
||
`madvise` (91 s system time): the emulated GPU's buffer-free path,
|
||
unrelated to input handling. Final verification therefore used a small
|
||
file (fast first frame) plus the router-level test.
|