fix(ui): UiField wrapped-line cache coherent with the text at mouse-hit time
Some checks are pending
Headless portability / portable-headless (ubuntu-latest) (push) Waiting to run
Headless portability / portable-headless (windows-latest) (push) Waiting to run
Headless portability / linux-graphical (push) Waiting to run
Headless portability / linux-vulkan (push) Waiting to run
Some checks are pending
Headless portability / portable-headless (ubuntu-latest) (push) Waiting to run
Headless portability / portable-headless (windows-latest) (push) Waiting to run
Headless portability / linux-graphical (push) Waiting to run
Headless portability / linux-vulkan (push) Waiting to run
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 <noreply@anthropic.com>
This commit is contained in:
parent
0ccbb4e52c
commit
67379d1f9a
2 changed files with 106 additions and 1 deletions
|
|
@ -84,7 +84,27 @@ public sealed class UiField : UiElement
|
||||||
public Action? OnFocusGained { get; set; }
|
public Action? OnFocusGained { get; set; }
|
||||||
public Action<string>? OnFocusLost { get; set; }
|
public Action<string>? OnFocusLost { get; set; }
|
||||||
|
|
||||||
private string _text = "";
|
private string _textValue = "";
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Every mutation bumps <see cref="_textVersion"/> 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 <see cref="HitChar"/> wrap lines describing the OLD, longer
|
||||||
|
/// text, and measuring that stale range crashed with
|
||||||
|
/// ArgumentOutOfRangeException (the 2026-07-29 inscription-field crash).
|
||||||
|
/// </summary>
|
||||||
|
private string _text
|
||||||
|
{
|
||||||
|
get => _textValue;
|
||||||
|
set
|
||||||
|
{
|
||||||
|
_textValue = value;
|
||||||
|
_textVersion++;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private int _caret;
|
private int _caret;
|
||||||
private int? _selAnchor; // selection fixed end (null = no selection); span = [min,max] with _caret
|
private int? _selAnchor; // selection fixed end (null = no selection); span = [min,max] with _caret
|
||||||
public string Text => _text;
|
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 float _scrollX; // horizontal pixel scroll so the caret stays in the field
|
||||||
private IReadOnlyList<WrappedLine> _wrappedLines = Array.Empty<WrappedLine>();
|
private IReadOnlyList<WrappedLine> _wrappedLines = Array.Empty<WrappedLine>();
|
||||||
private float _wrappedLineHeight = 14f;
|
private float _wrappedLineHeight = 14f;
|
||||||
|
private int _textVersion;
|
||||||
|
private int _wrappedVersion = -1;
|
||||||
|
private float _wrappedWidth;
|
||||||
private bool _suppressNextNewlineChar;
|
private bool _suppressNextNewlineChar;
|
||||||
|
|
||||||
// Held-key auto-repeat (Silk delivers one KeyDown per physical press).
|
// 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));
|
float visibleHeight = MathF.Max(1f, Height - (2f * Padding));
|
||||||
IReadOnlyList<WrappedLine> lines = BuildWrappedLines(visibleWidth);
|
IReadOnlyList<WrappedLine> lines = BuildWrappedLines(visibleWidth);
|
||||||
_wrappedLines = lines;
|
_wrappedLines = lines;
|
||||||
|
_wrappedVersion = _textVersion;
|
||||||
|
_wrappedWidth = visibleWidth;
|
||||||
_wrappedLineHeight = lineHeight;
|
_wrappedLineHeight = lineHeight;
|
||||||
|
|
||||||
Scroll.LineHeight = Math.Max(1, (int)MathF.Round(lineHeight));
|
Scroll.LineHeight = Math.Max(1, (int)MathF.Round(lineHeight));
|
||||||
|
|
@ -592,10 +617,32 @@ public sealed class UiField : UiElement
|
||||||
?? value.Length * 8f;
|
?? value.Length * 8f;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// 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.
|
||||||
|
/// </summary>
|
||||||
|
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)
|
private int HitChar(float localX, float localY)
|
||||||
{
|
{
|
||||||
if (OneLine)
|
if (OneLine)
|
||||||
return HitCharX(localX);
|
return HitCharX(localX);
|
||||||
|
EnsureWrappedLinesCurrent();
|
||||||
if (_wrappedLines.Count == 0)
|
if (_wrappedLines.Count == 0)
|
||||||
return _text.Length;
|
return _text.Length;
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -70,6 +70,64 @@ public class UiFieldTests
|
||||||
Assert.True(input.HistoryCount <= 100);
|
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]
|
[Fact]
|
||||||
public void CharacterFilter_rejectsDisallowedInput()
|
public void CharacterFilter_rejectsDisallowedInput()
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue