From 67379d1f9a5b36c0bb8a1c761db18cc8c984b9ba Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 29 Jul 2026 19:38:52 +0200 Subject: [PATCH] fix(ui): UiField wrapped-line cache coherent with the text at mouse-hit time Fixes the crash the user hit twice today (captured in artifacts/coldeve-acceptance-20260729/crash-hunt.log): clicking into a multiline UiField - the examination window's inscription field - after the text had changed since the last draw threw an unhandled ArgumentOutOfRangeException from String.Substring and took the whole client down (UiField.MeasureRange <- HitChar <- OnEvent MouseDown). Root cause: _wrappedLines is a DRAW-side cache (rebuilt only in DrawMultiLine) consumed by the INPUT side (HitChar on MouseDown and drag-select MouseMove). Input events are pumped before the frame's draw, so a mutation (backspace, SetText, paste) followed by a click in the same pumped frame handed HitChar wrap lines describing the OLD, longer text; measuring those stale ranges ran past the end of the live string. Fix: text mutations now bump a version (the _text field became a private property so every existing mutation site participates without churn), the draw records which version its wrap lines describe, and HitChar proves coherence via EnsureWrappedLinesCurrent() - rebuilding with the last draw width when stale. Rebuilding rather than clamping keeps caret placement CORRECT against the live text, not merely non-throwing. Two inversion-sensitive regression tests reproduce the exact crash sequence (wrap long text, shrink without a draw, click); they throw without the HitChar coherence call. App tests 3,962 passed / 3 skipped (3,960 + 2 new). Co-Authored-By: Claude Fable 5 --- src/AcDream.App/UI/UiField.cs | 49 +++++++++++++++++- tests/AcDream.App.Tests/UI/UiFieldTests.cs | 58 ++++++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/src/AcDream.App/UI/UiField.cs b/src/AcDream.App/UI/UiField.cs index 8cb0af79..7c760973 100644 --- a/src/AcDream.App/UI/UiField.cs +++ b/src/AcDream.App/UI/UiField.cs @@ -84,7 +84,27 @@ public sealed class UiField : UiElement public Action? OnFocusGained { get; set; } public Action? OnFocusLost { get; set; } - private string _text = ""; + private string _textValue = ""; + + /// + /// Every mutation bumps so the wrapped-line + /// cache can prove coherence. The cache is rebuilt at draw time, but + /// mouse hits arrive through input events that can precede the next + /// draw — a backspace followed by a click in the same pumped frame used + /// to hand wrap lines describing the OLD, longer + /// text, and measuring that stale range crashed with + /// ArgumentOutOfRangeException (the 2026-07-29 inscription-field crash). + /// + private string _text + { + get => _textValue; + set + { + _textValue = value; + _textVersion++; + } + } + private int _caret; private int? _selAnchor; // selection fixed end (null = no selection); span = [min,max] with _caret public string Text => _text; @@ -101,6 +121,9 @@ public sealed class UiField : UiElement private float _scrollX; // horizontal pixel scroll so the caret stays in the field private IReadOnlyList _wrappedLines = Array.Empty(); private float _wrappedLineHeight = 14f; + private int _textVersion; + private int _wrappedVersion = -1; + private float _wrappedWidth; private bool _suppressNextNewlineChar; // Held-key auto-repeat (Silk delivers one KeyDown per physical press). @@ -440,6 +463,8 @@ public sealed class UiField : UiElement float visibleHeight = MathF.Max(1f, Height - (2f * Padding)); IReadOnlyList lines = BuildWrappedLines(visibleWidth); _wrappedLines = lines; + _wrappedVersion = _textVersion; + _wrappedWidth = visibleWidth; _wrappedLineHeight = lineHeight; Scroll.LineHeight = Math.Max(1, (int)MathF.Round(lineHeight)); @@ -592,10 +617,32 @@ public sealed class UiField : UiElement ?? value.Length * 8f; } + /// + /// Rebuilds the wrapped-line cache when the text has changed since the + /// last draw. Mouse hits arrive through input events pumped BEFORE the + /// frame's draw, so a mutation (backspace, SetText, paste) followed by a + /// click in the same frame would otherwise measure ranges of the OLD + /// text against the new string — the 2026-07-29 inscription-field + /// crash. Rebuilding (rather than clamping) keeps the caret placement + /// correct, not merely non-throwing. + /// + internal void EnsureWrappedLinesCurrent() + { + if (_wrappedVersion == _textVersion && _wrappedLines.Count > 0) + return; + + float width = _wrappedWidth > 0f + ? _wrappedWidth + : MathF.Max(1f, Width - (2f * Padding)); + _wrappedLines = BuildWrappedLines(width); + _wrappedVersion = _textVersion; + } + private int HitChar(float localX, float localY) { if (OneLine) return HitCharX(localX); + EnsureWrappedLinesCurrent(); if (_wrappedLines.Count == 0) return _text.Length; diff --git a/tests/AcDream.App.Tests/UI/UiFieldTests.cs b/tests/AcDream.App.Tests/UI/UiFieldTests.cs index 8487e65b..21abf165 100644 --- a/tests/AcDream.App.Tests/UI/UiFieldTests.cs +++ b/tests/AcDream.App.Tests/UI/UiFieldTests.cs @@ -70,6 +70,64 @@ public class UiFieldTests Assert.True(input.HistoryCount <= 100); } + [Fact] + public void MultilineClick_AfterTextShrankSinceLastWrap_DoesNotThrowAndPlacesCaretInNewText() + { + // The 2026-07-29 inscription-field crash: the wrapped-line cache is + // rebuilt at DRAW time, but mouse events are pumped BEFORE the + // frame's draw — a backspace/SetText followed by a click in the same + // frame handed HitChar wrap lines describing the OLD, longer text, + // and MeasureRange threw ArgumentOutOfRangeException from + // String.Substring. HitChar must prove wrap coherence itself. + var input = new UiField + { + OneLine = false, + Selectable = true, + Width = 120, + Height = 80, + }; + input.SetText( + "a long inscription that wraps across multiple lines when it " + + "is measured with the fallback eight pixel glyph width"); + // Simulate the draw-time cache build for the CURRENT (long) text. + input.EnsureWrappedLinesCurrent(); + + // Text shrinks with no draw in between — the cached lines now + // describe ranges far beyond the live string. + input.SetText("hi"); + + // Click low and to the right, where a stale line would demand a + // substring past the end of "hi". Pre-fix: throws. Post-fix: the + // cache rebuilds and the caret lands inside the new text. + var exception = Record.Exception(() => input.OnEvent( + new UiEvent(0u, input, UiEventType.MouseDown, Data1: 90, Data2: 60))); + + Assert.Null(exception); + Assert.InRange(input.CaretPos, 0, input.Text.Length); + } + + [Fact] + public void MultilineClick_AfterBackspacesSinceLastWrap_DoesNotThrow() + { + var input = new UiField + { + OneLine = false, + Selectable = true, + Width = 96, + Height = 60, + }; + input.SetText("wrapped inscription text under edit right now"); + input.EnsureWrappedLinesCurrent(); + for (int i = 0; i < 30; i++) + input.Backspace(); + + var exception = Record.Exception(() => input.OnEvent( + new UiEvent(0u, input, UiEventType.MouseDown, Data1: 80, Data2: 40))); + + Assert.Null(exception); + Assert.InRange(input.CaretPos, 0, input.Text.Length); + } + [Fact] public void CharacterFilter_rejectsDisallowedInput() {