IME: guard distant commits; arm the resync on the first anomaly
The boundary trace (PADIME at EditorStateChanged) showed why a desynchronized IME never heals itself: gioui's EditorReplace advances its stored selection state when the commit arrives, so the post-commit selection push is always deduplicated away and updateSelection is never sent after a commit. A desynced IME is therefore healed only by a snippet restart — and the resync was armed only after THREE consecutive anomalies, while the phone incident's first anomaly (a 5-rune autocorrect 19,000 runes from the caret, width > 2) sailed through the old <=2-rune guard and clobbered distant text before any resync could fire. Now: - the guard also snaps commits that are far from the caret AND outside the live selection (a legitimate commit is always local: at the caret, inside the selection, or a nearby correction); a distant commit lands at the caret instead of clobbering text; - the resync is armed on the FIRST anomalous commit (snapped or distant), not after a streak; the streak counter is gone; - imeSelectionRuneRange() lets the guard allow selection replacements anywhere. TestRealFile_IMEForceResync rewritten for the new semantics and extended with the distant-replacement case (must land at the caret and arm the resync).
This commit is contained in:
parent
51344b9a5b
commit
a83cc1a72f
2
go.mod
2
go.mod
|
|
@ -15,3 +15,5 @@ require (
|
||||||
golang.org/x/net v0.48.0 // indirect
|
golang.org/x/net v0.48.0 // indirect
|
||||||
golang.org/x/text v0.32.0 // indirect
|
golang.org/x/text v0.32.0 // indirect
|
||||||
)
|
)
|
||||||
|
|
||||||
|
replace gioui.org => /home/gmp/gioui
|
||||||
|
|
|
||||||
2
go.sum
2
go.sum
|
|
@ -1,7 +1,5 @@
|
||||||
eliasnaur.com/font v0.0.0-20230308162249-dd43949cb42d h1:ARo7NCVvN2NdhLlJE9xAbKweuI9L6UgfTbYb0YwPacY=
|
eliasnaur.com/font v0.0.0-20230308162249-dd43949cb42d h1:ARo7NCVvN2NdhLlJE9xAbKweuI9L6UgfTbYb0YwPacY=
|
||||||
eliasnaur.com/font v0.0.0-20230308162249-dd43949cb42d/go.mod h1:OYVuxibdk9OSLX8vAqydtRPP87PyTFcT9uH3MlEGBQA=
|
eliasnaur.com/font v0.0.0-20230308162249-dd43949cb42d/go.mod h1:OYVuxibdk9OSLX8vAqydtRPP87PyTFcT9uH3MlEGBQA=
|
||||||
gioui.org v0.10.2 h1:bZU5CORROwc51sNha0zYdE2qWVaDncOp5EjV5nrZQZ8=
|
|
||||||
gioui.org v0.10.2/go.mod h1:iKILKNq6+LHMWhP/HjGDW/wDidUzRnb7B6c7ZD9y1Mg=
|
|
||||||
gioui.org/cpu v0.0.0-20210808092351-bfe733dd3334/go.mod h1:A8M0Cn5o+vY5LTMlnRoK3O5kG+rH0kWfJjeKd9QpBmQ=
|
gioui.org/cpu v0.0.0-20210808092351-bfe733dd3334/go.mod h1:A8M0Cn5o+vY5LTMlnRoK3O5kG+rH0kWfJjeKd9QpBmQ=
|
||||||
gioui.org/shader v1.0.9 h1:XxnqIfmClWpN49kizxH2W0JcCFrrEP4q3jZmNYaltbs=
|
gioui.org/shader v1.0.9 h1:XxnqIfmClWpN49kizxH2W0JcCFrrEP4q3jZmNYaltbs=
|
||||||
gioui.org/shader v1.0.9/go.mod h1:mWdiME581d/kV7/iEhLmUgUK5iZ09XR5XpduXzbePVM=
|
gioui.org/shader v1.0.9/go.mod h1:mWdiME581d/kV7/iEhLmUgUK5iZ09XR5XpduXzbePVM=
|
||||||
|
|
|
||||||
|
|
@ -127,15 +127,15 @@ type EditorState struct {
|
||||||
IMESelStartRune int
|
IMESelStartRune int
|
||||||
IMESelEndRune int
|
IMESelEndRune int
|
||||||
// IMEForceResync asks the next frame to re-push the snippet trimmed by
|
// IMEForceResync asks the next frame to re-push the snippet trimmed by
|
||||||
// one rune (a forced restartInput): after several consecutive
|
// one rune (a forced restartInput): an anomalous commit (drift-snapped
|
||||||
// anomalous commits (drift-snapped or empty) the IME's local text is
|
// or distant — see HandleIMECommit) means the IME's local text is
|
||||||
// desynchronized from ours and it keeps re-sending the same fix (an
|
// desynchronized from ours, and it may keep re-sending the same fix in
|
||||||
// endless empty-commit loop). A forced restart makes it re-fetch the
|
// a loop (observed on the phone: an endless stream of empty commits).
|
||||||
// real text and selection around the caret, which heals the model.
|
// A selection push cannot heal this (it is deduplicated away after
|
||||||
// Set by HandleIMECommit, consumed by the next frame (EditorLayout).
|
// commits); only a restart makes the IME re-fetch the real text and
|
||||||
|
// selection around the caret. Set by HandleIMECommit on the first
|
||||||
|
// anomaly, consumed by the next frame (EditorLayout).
|
||||||
IMEForceResync bool
|
IMEForceResync bool
|
||||||
// imeAnomStreak counts consecutive anomalous IME commits (see above).
|
|
||||||
imeAnomStreak int
|
|
||||||
// EditSeq counts content edits (incremented by markDirty). The shaped
|
// EditSeq counts content edits (incremented by markDirty). The shaped
|
||||||
// glyph layout arriving via layoutChan is only applied to the WrapIndex
|
// glyph layout arriving via layoutChan is only applied to the WrapIndex
|
||||||
// when its EditSeq matches, so a layout shaped before an edit can never
|
// when its EditSeq matches, so a layout shaped before an edit can never
|
||||||
|
|
@ -2220,6 +2220,19 @@ func (s *State) computeIMESnippetWindow() {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// imeSelectionRuneRange returns the live selection in absolute file runes,
|
||||||
|
// or (-1, -1) when there is none. Used by the anomalous-commit guard to
|
||||||
|
// allow commits inside a user selection anywhere.
|
||||||
|
func imeSelectionRuneRange() (int, int) {
|
||||||
|
if !selActive() {
|
||||||
|
return -1, -1
|
||||||
|
}
|
||||||
|
e := &TheState.Editor
|
||||||
|
r0 := TheState.imeRuneOffsetAt(e.SelectionStart)
|
||||||
|
r1 := r0 + utf8.RuneCountInString(TheState.fileContent(e.SelectionStart, e.SelectionEnd))
|
||||||
|
return r0, r1
|
||||||
|
}
|
||||||
|
|
||||||
// fileLenNow returns the current content length (chunked or string buffer).
|
// fileLenNow returns the current content length (chunked or string buffer).
|
||||||
func (s *State) fileLenNow() int64 {
|
func (s *State) fileLenNow() int64 {
|
||||||
if cb := s.Editor.ChunkedBuffer; cb != nil {
|
if cb := s.Editor.ChunkedBuffer; cb != nil {
|
||||||
|
|
@ -2362,28 +2375,43 @@ func HandleIMECommit(data any) {
|
||||||
if startRune > endRune {
|
if startRune > endRune {
|
||||||
startRune, endRune = endRune, startRune
|
startRune, endRune = endRune, startRune
|
||||||
}
|
}
|
||||||
snapped := false
|
// Anomalous-commit guard. A legitimate IME commit is always local: at
|
||||||
if endRune-startRune <= 2 {
|
// the caret (a typed char, a composing word), inside the live
|
||||||
caretRune := TheState.imeRuneOffsetAt(TheState.Editor.CursorPosition)
|
// selection, or a correction of the word just typed (a few runes left
|
||||||
if endRune != caretRune {
|
// of the caret). Anything else means the IME's local text has
|
||||||
log.Printf("IME DRIFT-SNAP range=[%d,%d) caret=%d -> snap to caret", startRune, endRune, caretRune)
|
// desynchronized from ours — its reported range addresses text that is
|
||||||
startRune, endRune = caretRune, caretRune
|
// not where we think it is. Applying it verbatim clobbers distant text
|
||||||
snapped = true
|
// (observed on the phone: a 5-rune autocorrect 19,000 runes from the
|
||||||
|
// caret); the only safe action is to apply it at the caret and force a
|
||||||
|
// re-syncing snippet re-push on the next frame (see IMEForceResync): a
|
||||||
|
// desynced IME is healed only by a restart (a selection push is
|
||||||
|
// deduplicated away after commits — see Renderer.FlushIME), so the
|
||||||
|
// resync is armed on the FIRST anomaly, not after a streak.
|
||||||
|
const maxCommitDistance = 1024
|
||||||
|
caretRune := TheState.imeRuneOffsetAt(TheState.Editor.CursorPosition)
|
||||||
|
selStartRune, selEndRune := imeSelectionRuneRange()
|
||||||
|
anomalous := endRune-startRune <= 2 && endRune != caretRune
|
||||||
|
if !anomalous && c.Text != "" {
|
||||||
|
// The commit must be near the caret, or inside the live selection
|
||||||
|
// (a replacement of a user selection, wherever it is).
|
||||||
|
lo := caretRune - maxCommitDistance
|
||||||
|
hi := caretRune + maxCommitDistance
|
||||||
|
if selStartRune >= 0 {
|
||||||
|
if selStartRune < lo {
|
||||||
|
lo = selStartRune
|
||||||
|
}
|
||||||
|
if selEndRune > hi {
|
||||||
|
hi = selEndRune
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if startRune > hi || endRune < lo {
|
||||||
|
anomalous = true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// Anomalous commits (snapped, or empty: an IME fix-up that changes
|
if anomalous {
|
||||||
// nothing) mean the IME's local text has desynchronized from ours.
|
log.Printf("IME DRIFT-SNAP range=[%d,%d) text=%q caret=%d -> snap to caret + resync", startRune, endRune, c.Text, caretRune)
|
||||||
// After a few in a row, force a re-syncing snippet re-push on the next
|
startRune, endRune = caretRune, caretRune
|
||||||
// frame (see IMEForceResync) so the IME re-fetches the real text and
|
TheState.Editor.IMEForceResync = true
|
||||||
// the loop ends; a normal commit resets the streak.
|
|
||||||
if snapped || c.Text == "" {
|
|
||||||
TheState.Editor.imeAnomStreak++
|
|
||||||
if TheState.Editor.imeAnomStreak >= 3 {
|
|
||||||
TheState.Editor.imeAnomStreak = 0
|
|
||||||
TheState.Editor.IMEForceResync = true
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
TheState.Editor.imeAnomStreak = 0
|
|
||||||
}
|
}
|
||||||
absStart, absEnd := runeToByteWhole(startRune), runeToByteWhole(endRune)
|
absStart, absEnd := runeToByteWhole(startRune), runeToByteWhole(endRune)
|
||||||
log.Printf("IME COMMIT range=[%d,%d) text=%q -> [%d,%d)", startRune, endRune, c.Text, absStart, absEnd)
|
log.Printf("IME COMMIT range=[%d,%d) text=%q -> [%d,%d)", startRune, endRune, c.Text, absStart, absEnd)
|
||||||
|
|
|
||||||
|
|
@ -211,12 +211,12 @@ func firstDiff(a, b string) int {
|
||||||
return n
|
return n
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestRealFile_IMEForceResync pins the desync self-heal: after several
|
// TestRealFile_IMEForceResync pins the desync recovery: the first
|
||||||
// consecutive anomalous IME commits (drift-snapped or empty — the pattern
|
// anomalous IME commit (a drift-snapped small commit, a distant commit —
|
||||||
// Gboard produces when its local text has desynchronized from the app and it
|
// the phone autocorrect that landed 19,000 runes from the caret — or an
|
||||||
// keeps re-sending the same fix), the next frame arms a forced snippet
|
// empty fix-up) arms a forced re-syncing snippet re-push on the very next
|
||||||
// re-push (a restartInput that makes the IME re-fetch the real text). A
|
// frame (a desynced IME is healed only by a restart; a selection push is
|
||||||
// normal commit resets the streak, so isolated anomalies never resync.
|
// deduplicated away after commits). A normal commit never arms it.
|
||||||
func TestRealFile_IMEForceResync(t *testing.T) {
|
func TestRealFile_IMEForceResync(t *testing.T) {
|
||||||
model := largeFileContent(4000)
|
model := largeFileContent(4000)
|
||||||
h, _ := realFileHarness(t, "resync.txt", model)
|
h, _ := realFileHarness(t, "resync.txt", model)
|
||||||
|
|
@ -227,7 +227,6 @@ func TestRealFile_IMEForceResync(t *testing.T) {
|
||||||
t.Fatalf("wait for frame: %v", err)
|
t.Fatalf("wait for frame: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// A caret deep in the file, in absolute file runes.
|
|
||||||
const caretByte = 10000
|
const caretByte = 10000
|
||||||
caretRune := utf8.RuneCountInString(model[:caretByte])
|
caretRune := utf8.RuneCountInString(model[:caretByte])
|
||||||
if err := h.WithState(func(st *editor.State) {
|
if err := h.WithState(func(st *editor.State) {
|
||||||
|
|
@ -241,17 +240,13 @@ func TestRealFile_IMEForceResync(t *testing.T) {
|
||||||
Handler: editor.HandleIMECommit,
|
Handler: editor.HandleIMECommit,
|
||||||
Data: editor.IMECommit{StartRune: start, EndRune: end, Text: text},
|
Data: editor.IMECommit{StartRune: start, EndRune: end, Text: text},
|
||||||
}})
|
}})
|
||||||
// The logic goroutine applies commits asynchronously; wait for the
|
// The logic applies commits asynchronously; wait for the resulting
|
||||||
// resulting frame before asserting on the derived state.
|
// frame before asserting on the derived state.
|
||||||
prev := h.FrameCount()
|
prev := h.FrameCount()
|
||||||
if _, err := h.WaitForFrameCount(prev+1, 5*time.Second); err != nil {
|
if _, err := h.WaitForFrameCount(prev+1, 5*time.Second); err != nil {
|
||||||
t.Fatalf("wait for commit frame: %v", err)
|
t.Fatalf("wait for commit frame: %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// forceInLatestFrame reports whether the latest delivered frame's editor
|
|
||||||
// element carries the forced-resync flag (the state flag is consumed by
|
|
||||||
// the very frame that follows the arming commit, so the frame is the
|
|
||||||
// observable artifact).
|
|
||||||
forceInLatestFrame := func() bool {
|
forceInLatestFrame := func() bool {
|
||||||
frames := h.GetFrames()
|
frames := h.GetFrames()
|
||||||
if len(frames) == 0 {
|
if len(frames) == 0 {
|
||||||
|
|
@ -266,38 +261,47 @@ func TestRealFile_IMEForceResync(t *testing.T) {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
// Two anomalous commits (an empty fix-up and a drifted insertion):
|
// A normal commit at the caret arms nothing.
|
||||||
// below the threshold, no resync.
|
commit(caretRune, caretRune, "q")
|
||||||
|
if forceInLatestFrame() {
|
||||||
|
t.Fatal("resync armed by a normal commit")
|
||||||
|
}
|
||||||
|
|
||||||
|
// A small drifted commit (Gboard fix-up at a stale position): the frame
|
||||||
|
// emitted by that commit carries the resync flag.
|
||||||
commit(caretRune+5, caretRune+5, "")
|
commit(caretRune+5, caretRune+5, "")
|
||||||
commit(caretRune+6, caretRune+6, "z")
|
|
||||||
if forceInLatestFrame() {
|
|
||||||
t.Fatalf("resync armed after only 2 anomalous commits")
|
|
||||||
}
|
|
||||||
|
|
||||||
// A normal commit resets the streak: two more anomalies must not arm.
|
|
||||||
commit(caretRune+10, caretRune+10, "q")
|
|
||||||
commit(caretRune+11, caretRune+11, "")
|
|
||||||
commit(caretRune+12, caretRune+12, "")
|
|
||||||
if forceInLatestFrame() {
|
|
||||||
t.Fatalf("resync armed after a reset streak")
|
|
||||||
}
|
|
||||||
|
|
||||||
// One more anomaly makes three in a row: the frame emitted by that
|
|
||||||
// commit carries the flag on its editor element (the state flag is
|
|
||||||
// consumed by that same frame).
|
|
||||||
commit(caretRune+13, caretRune+13, "")
|
|
||||||
if !forceInLatestFrame() {
|
if !forceInLatestFrame() {
|
||||||
t.Fatalf("resync not armed after 3 consecutive anomalous commits")
|
t.Fatal("resync not armed after a drift-snapped commit")
|
||||||
}
|
}
|
||||||
|
|
||||||
// A later frame (no new anomalies) carries no flag: the resync fires
|
// One-shot: a later frame carries no flag.
|
||||||
// exactly once.
|
|
||||||
prev := h.FrameCount()
|
prev := h.FrameCount()
|
||||||
h.SendConfig(780, 400)
|
h.SendConfig(780, 400)
|
||||||
if _, err := h.WaitForFrameCount(prev+1, 5*time.Second); err != nil {
|
if _, err := h.WaitForFrameCount(prev+1, 5*time.Second); err != nil {
|
||||||
t.Fatalf("wait for frame: %v", err)
|
t.Fatalf("wait for frame: %v", err)
|
||||||
}
|
}
|
||||||
if forceInLatestFrame() {
|
if forceInLatestFrame() {
|
||||||
t.Fatalf("resync flag not one-shot")
|
t.Fatal("resync flag not one-shot")
|
||||||
|
}
|
||||||
|
|
||||||
|
// A distant replacement commit (width > 2: the autocorrect class that
|
||||||
|
// the old guard let through) is snapped to the caret and arms the
|
||||||
|
// resync.
|
||||||
|
commit(caretRune+20000, caretRune+20005, "silly")
|
||||||
|
if !forceInLatestFrame() {
|
||||||
|
t.Fatal("resync not armed after a distant commit")
|
||||||
|
}
|
||||||
|
|
||||||
|
// The distant commit must have landed at the caret, not at 20000:
|
||||||
|
// the caret advances by exactly the inserted text length.
|
||||||
|
var cur int
|
||||||
|
if err := h.WithState(func(st *editor.State) {
|
||||||
|
cur = st.Editor.CursorPosition
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatalf("WithState: %v", err)
|
||||||
|
}
|
||||||
|
// "q" (1) + "silly" (5) inserted at the original caret.
|
||||||
|
if want := caretByte + 6; cur != want {
|
||||||
|
t.Fatalf("cursor = %d, want %d (distant commit must land at the caret)", cur, want)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user