Selection handles: capture base line at grab (fix jitter runaway)
The relative-line mapping resolved the anchor's base line from the anchor's CURRENT byte on every drag event. Once the anchor crossed a line, its own movement fed back into its target: a finger jittering near a line boundary added one more line per event and raced the anchor to the bottom of the file (reported: tiny vertical movements jump the anchor off-screen). Capture the anchor's visual line once at the grab (SelDragPressLine) and compute the target from that fixed base. A jitter test (TestSelDrag_EndHandle_JitterNearLineBoundary_Stable) pins the runaway: mutation-verified against the per-event re-resolution.
This commit is contained in:
parent
7239b1e7ae
commit
110c92e3fa
|
|
@ -87,12 +87,31 @@ on an idle window and every menu tap was swallowed.
|
||||||
|
|
||||||
## 2. Evidence (verified against gioui.org v0.10.0 sources, the live repo's version)
|
## 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
|
1. **`widget.Editor` is not virtualized** — `textView.layoutText` seeks to the
|
||||||
invalidation and keeps a ~200 B/rune in-memory `glyphIndex`. Measured on this
|
start and shapes the *entire* document on every invalidation (text edit,
|
||||||
box: 1 MB ≈ 17 ms/keystroke & ~0.32 GB; 3 MB ≈ 1.7 GB; 4 MB OOM-killed the host.
|
size/param change), with an infinite viewport; the shaper's `document.reset()`
|
||||||
Planning figure **~0.5 GB of RAM per 1 MB of text**. A phone cannot edit 10 MB
|
is `lines = lines[:0]`, so the backing array of the largest layout is retained
|
||||||
in `widget.Editor`. This is *why* the live repo went the chunked-buffer route,
|
forever. **Re-benchmarked 2026-08-18 against v0.10.0** (plain `text.Shaper`,
|
||||||
and it's the reason v1's "just use the widget" was wrong for large files.
|
whole-document shape, the widget's exact call pattern; x86_64 VM):
|
||||||
|
|
||||||
|
| doc | shape time (per edit/keystroke) | retained heap |
|
||||||
|
|---|---|---|
|
||||||
|
| 1 MB | ~120 ms | ~5 MB |
|
||||||
|
| 2 MB | ~0.4 s | ~7 MB |
|
||||||
|
| 5 MB | ~3.4 s | ~15 MB |
|
||||||
|
| 10 MB | ~14 s | ~29 MB |
|
||||||
|
|
||||||
|
The **CPU wall is the decision argument**: a full re-shape on every keystroke
|
||||||
|
is already sluggish at 1 MB (~8 fps of edits) and unusable above ~2 MB; a
|
||||||
|
phone CPU (3–10× slower than this VM) is worse. Memory grows only ~3× the
|
||||||
|
text size with the raw shaper — the earlier "~0.5 GB per MB" figures (which
|
||||||
|
likely included Android PSS overhead / an older version) do not reproduce, but
|
||||||
|
they are not needed: the linear per-edit shape cost alone rules out
|
||||||
|
`widget.Editor` for multi-MB files. This is *why* the live repo went the
|
||||||
|
chunked-buffer route, and it's the reason v1's "just use the widget" was
|
||||||
|
wrong for large files. (For small files — sub-MB notes — `widget.Editor`
|
||||||
|
would be adequate; the custom editor is only strictly required above ~1 MB,
|
||||||
|
but the app's 50 MB target is 50× that.)
|
||||||
2. **The Android IME works through the op layer, not the widget.** The
|
2. **The Android IME works through the op layer, not the widget.** The
|
||||||
`window`'s editor state (`app.Window` → `input.EditorState{Selection, Snippet}`)
|
`window`'s editor state (`app.Window` → `input.EditorState{Selection, Snippet}`)
|
||||||
is what `GioInputConnection` (Android's `InputConnection`) reads. It is fed by
|
is what `GioInputConnection` (Android's `InputConnection`) reads. It is fed by
|
||||||
|
|
@ -975,13 +994,18 @@ downward, dragging the start handle up extends it upward. The mapping is
|
||||||
relative (displacement from the grab, not the finger's absolute line)
|
relative (displacement from the grab, not the finger's absolute line)
|
||||||
precisely because the grab is usually a line or more below the anchor:
|
precisely because the grab is usually a line or more below the anchor:
|
||||||
tracking the finger's absolute line would first drag the anchor the wrong
|
tracking the finger's absolute line would first drag the anchor the wrong
|
||||||
way, through a collapse, before it ever reached the anchor's line.
|
way, through a collapse, before it ever reached the anchor's line. The
|
||||||
Regressions in `touch_selection_test.go`:
|
base line is captured **once at the grab** and never re-resolved from the
|
||||||
|
anchor's current byte: that would feed the anchor's own movement back
|
||||||
|
into its target line, and a finger jittering near a line boundary would
|
||||||
|
race the anchor off the screen (one extra line per event — reported as
|
||||||
|
"tiny vertical movements jump the anchor to the top/bottom of the
|
||||||
|
screen"). Regressions in `touch_selection_test.go`:
|
||||||
`TestSelDrag_StartHandle_PressOnLineBelow_KeepsSelection` (the original
|
`TestSelDrag_StartHandle_PressOnLineBelow_KeepsSelection` (the original
|
||||||
bug's geometry), `TestSelDrag_EndHandle_DragDownExtendsAcrossLines`,
|
bug's geometry), `TestSelDrag_EndHandle_DragDownExtendsAcrossLines`,
|
||||||
`TestSelDrag_StartHandle_DragUpExtendsToLineAbove` — all
|
`TestSelDrag_StartHandle_DragUpExtendsToLineAbove`,
|
||||||
mutation-verified against both the pre-fix mapping and the
|
`TestSelDrag_EndHandle_JitterNearLineBoundary_Stable` (the runaway) — all
|
||||||
absolute-finger-line variant.
|
mutation-verified against the pre-fix variants.
|
||||||
|
|
||||||
**Menu anchors to the stable end.** While a start-handle drag is in
|
**Menu anchors to the stable end.** While a start-handle drag is in
|
||||||
progress the selection start is the moving end, so the menu anchors to
|
progress the selection start is the moving end, so the menu anchors to
|
||||||
|
|
|
||||||
|
|
@ -108,9 +108,14 @@ type EditorState struct {
|
||||||
SelDragWhich int
|
SelDragWhich int
|
||||||
SelDragRel int
|
SelDragRel int
|
||||||
// SelDragPressY is the text-local Y of the first event of an in-progress
|
// SelDragPressY is the text-local Y of the first event of an in-progress
|
||||||
// handle/body drag (the grab). Start/end handles move relative to this
|
// handle/body drag (the grab), and SelDragPressLine the visual line of
|
||||||
// (see handleAnchorPos).
|
// the dragged anchor AT THE GRAB. Start/end handles move relative to
|
||||||
|
// these (see handleAnchorPos). The base line must be captured once at the
|
||||||
|
// grab: re-resolving it from the anchor's current byte on every event
|
||||||
|
// makes the anchor's own movement feed back into its target, and a
|
||||||
|
// jittering finger near a line boundary races the anchor off the screen.
|
||||||
SelDragPressY float64
|
SelDragPressY float64
|
||||||
|
SelDragPressLine int
|
||||||
Filename string
|
Filename string
|
||||||
// TooLarge is set when an opened file exceeds MaxEditableFileSize. The
|
// TooLarge is set when an opened file exceeds MaxEditableFileSize. The
|
||||||
// editor shows a "too large to edit" notice instead of content (the
|
// editor shows a "too large to edit" notice instead of content (the
|
||||||
|
|
@ -1109,6 +1114,16 @@ func selDragMove(which int, x, y ui.Dp) {
|
||||||
e.SelDragging = true
|
e.SelDragging = true
|
||||||
e.SelDragWhich = which
|
e.SelDragWhich = which
|
||||||
e.SelDragPressY = localY
|
e.SelDragPressY = localY
|
||||||
|
if which == 0 {
|
||||||
|
if l, ok2 := visualLineOfByte(e.SelectionStart - glyphBase()); ok2 {
|
||||||
|
e.SelDragPressLine = l
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if which == 1 {
|
||||||
|
if l, ok2 := visualLineOfByte(e.SelectionEnd - glyphBase()); ok2 {
|
||||||
|
e.SelDragPressLine = l
|
||||||
|
}
|
||||||
|
}
|
||||||
if which == 2 && ok {
|
if which == 2 && ok {
|
||||||
// Body drag: the grab fixes the finger's offset from the selection
|
// Body drag: the grab fixes the finger's offset from the selection
|
||||||
// start; the selection itself moves on later events.
|
// start; the selection itself moves on later events.
|
||||||
|
|
@ -1173,10 +1188,10 @@ func selDragMove(which int, x, y ui.Dp) {
|
||||||
// a deliberate vertical drag crosses lines only after the finger has moved
|
// a deliberate vertical drag crosses lines only after the finger has moved
|
||||||
// past half a line height from the grab.
|
// past half a line height from the grab.
|
||||||
func handleAnchorPos(e *EditorState, anchorByte int, localX, localY float64) (int, bool) {
|
func handleAnchorPos(e *EditorState, anchorByte int, localX, localY float64) (int, bool) {
|
||||||
line, ok := visualLineOfByte(anchorByte - glyphBase())
|
// Base line is the anchor's line at the GRAB (captured in selDragMove),
|
||||||
if !ok {
|
// never the anchor's current line: the anchor's own movement must not
|
||||||
return textPosFromLocalPoint(localX, localY)
|
// feed back into its target.
|
||||||
}
|
line := e.SelDragPressLine
|
||||||
lh := float64(EffectiveLineHeight())
|
lh := float64(EffectiveLineHeight())
|
||||||
if lh > 0 {
|
if lh > 0 {
|
||||||
target := line + int(math.Round((localY-e.SelDragPressY)/lh))
|
target := line + int(math.Round((localY-e.SelDragPressY)/lh))
|
||||||
|
|
|
||||||
|
|
@ -479,6 +479,63 @@ func checkSelection12_18(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// touchSelState3Lines is like touchSelState2Lines with a third line.
|
||||||
|
func touchSelState3Lines() {
|
||||||
|
content := "hello world\nsecond line\nthird line here\n"
|
||||||
|
TheState = NewState()
|
||||||
|
TheState.Editor.Buffer = content
|
||||||
|
TheState.Editor.CursorPosition = len(content)
|
||||||
|
TheState.EditorRegion = ui.Region{X: 16, Y: 100, W: 379, H: 700}
|
||||||
|
TheState.ScrollOffset = 0
|
||||||
|
TheState.Editor.IMEWindowStartByte = 0
|
||||||
|
TheState.Editor.IMEWindowText = content
|
||||||
|
lh := ui.Dp(16.8)
|
||||||
|
bo := []int{}
|
||||||
|
xs := []ui.Dp{}
|
||||||
|
s := []ui.Dp{}
|
||||||
|
ad := []ui.Dp{}
|
||||||
|
lines := []struct{ start, n int }{{0, 11}, {12, 11}, {24, 15}}
|
||||||
|
for li, l := range lines {
|
||||||
|
for j := 0; j < l.n; j++ {
|
||||||
|
bo = append(bo, l.start+j)
|
||||||
|
xs = append(xs, ui.Dp(10*j))
|
||||||
|
s = append(s, ui.Dp(float64(lh)*float64(li)))
|
||||||
|
ad = append(ad, 10)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
TheState.Editor.GlyphLayout = ui.GlyphLayout{
|
||||||
|
LineHeight: lh,
|
||||||
|
ByteOffsets: bo,
|
||||||
|
X: xs,
|
||||||
|
Y: s,
|
||||||
|
Advance: ad,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSelDrag_EndHandle_JitterNearLineBoundary_Stable is a regression test
|
||||||
|
// for a feedback runaway: the target line must be computed from the
|
||||||
|
// anchor's line AT THE GRAB. Re-resolving the base line from the anchor's
|
||||||
|
// current byte on every event makes the anchor's own movement feed into its
|
||||||
|
// target: once the anchor crosses a line, every further event adds another
|
||||||
|
// line, and a jittering finger near a boundary races the anchor to the
|
||||||
|
// bottom of the file.
|
||||||
|
func TestSelDrag_EndHandle_JitterNearLineBoundary_Stable(t *testing.T) {
|
||||||
|
touchSelState3Lines()
|
||||||
|
HandleLongPressAt(18, 108) // selects "hello" [0,5)
|
||||||
|
checkInitialSelection(t)
|
||||||
|
// End handle (which=1), anchor byte 5 on line 0. Grab at local (52, 25).
|
||||||
|
HandleSelDragEvt(ui.SelectionDragEvent{Which: 1, X: 16 + 52, Y: 100 + 25})
|
||||||
|
checkInitialSelection(t)
|
||||||
|
// The finger hovers just past the half-line boundary (dY = 10, 11, 10
|
||||||
|
// around the 8.4 threshold) and jitters. The anchor must settle one line
|
||||||
|
// down (byte 17, the 'n' of "second") and STAY there — not creep to
|
||||||
|
// line 2 (byte 29+) or further with each event.
|
||||||
|
for _, dy := range []int{35, 36, 35, 36, 35} {
|
||||||
|
HandleSelDragEvt(ui.SelectionDragEvent{Which: 1, X: 16 + 52, Y: ui.Dp(100 + dy)})
|
||||||
|
assertSelection(t, 0, 17)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func checkInitialSelection(t *testing.T) {
|
func checkInitialSelection(t *testing.T) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
s := TheState.Editor
|
s := TheState.Editor
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user