Two user-reported bugs in the post-wrap build:
1. Selection 'jumped' when scrolling: frameOf's chunked branch shadowed the
outer start/end with 'start, end, winLine := cb.VisibleByteRange(...)',
leaving IMEWindowStartByte at 0 for chunked files while scrolled, so the
window-relative selection highlight (and IME commits) mis-mapped near the
file top. Fix: declare winLine outside, assign. Mutation-verified
regression tests: real_file_scroll_selection_test.go.
2. Bottom bar hidden behind the keyboard: GioView extends SurfaceView, which
unconditionally marks the window FORMAT_TRANSLUCENT; translucent windows
are never IME-resized, so adjustResize is dead. Fix (build scripts):
smali-patch a PadInsetsListener OnApplyWindowInsetsListener onto the
GioView (shrinks it by the IME inset bottom) + setDecorFitsSystemWindows
(false) on API 30+ so insets are dispatched. targetSdk kept at 34.
3. (Found while fixing 2) Keyboard could not be dismissed: TextField.Draw
issued SoftKeyboardCmd{Show:true} every frame; with insets-driven
resizes, the hide animation triggered redraws that re-showed the keyboard
mid-animation. Fix: ShowIMESeq pulse from the logic layer (open/tap/
double-tap); renderer shows only on pulse change, re-arming on focus
loss — matching widget.Editor.
On-device (emulator): BACK dismisses and stays down; tap re-shows; typing
works; bottom bar above keyboard; word selection anchored across flick
scroll. go test -race ./... green. Phone APK rebuilt with the same patch.
846 lines
53 KiB
Markdown
846 lines
53 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** — it shapes the *entire* document per
|
||
invalidation and keeps a ~200 B/rune in-memory `glyphIndex`. Measured on this
|
||
box: 1 MB ≈ 17 ms/keystroke & ~0.32 GB; 3 MB ≈ 1.7 GB; 4 MB OOM-killed the host.
|
||
Planning figure **~0.5 GB of RAM per 1 MB of text**. A phone cannot edit 10 MB
|
||
in `widget.Editor`. 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.
|
||
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, search-in-file, syntax highlighting, diff/merge.
|
||
- 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).
|