The cut/copy/paste icons were dead placeholders (no handlers); clipboard actions live in the floating selection menu, so the second icon row is removed. The bar goes from two rows at 52dp to one row at 32dp, giving the editor 20dp more height. - filename_test.go now finds the label by type (back icon precedes it) - menu tracking tests updated for the new editorY (reg.Y = 10+32) - scroll_cursor e2e simulation updated to match - On-device verified: back button works, floating menu places above with room and flips below on the first lines, handles unaffected.
949 lines
59 KiB
Markdown
949 lines
59 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).
|
||
|
||
## 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).
|