erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases 6 Packages Wiki Activity Actions

merge(vt): list scrollbar no longer snaps back every frame (reveal-clamp only on a changed selection)

Browse source 
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-07 18:21:21 +02:00
commit b4fe180070
2 changed files with 285 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

50
src/AcDream.App/UI/UiMarkupList.cs
View file

@ -116,6 +116,28 @@ public sealed class UiMarkupList : UiElement
private int _topRow;
private IReadOnlyList<UiMarkupListColumn>? _columns;
/// <summary>
/// Owner live-client report 2026-09-07 ("Scrolling in advanced options
/// does not work... or it works sometimes"): the last
/// <see cref="SelectedIndexSource"/> value the "keep selection visible"
/// clamp below (<see cref="OnDraw"/>/<see cref="DrawColumns"/>) actually
/// reacted to. A real bound list (e.g. MossTank's Advanced Options,
/// <c>selected="{SelectedAdvancedOptionIndex}"</c>) keeps a STABLE
/// selected index while the user scrolls elsewhere with the scrollbar —
/// re-running the reveal clamp on EVERY frame regardless of whether
/// selection actually changed snapped <see cref="_topRow"/> straight back
/// to the (unchanged) selected row on the very next draw, undoing the
/// scrollbar arrow/track/thumb interaction the same frame it happened.
/// Gating the clamp on an observed CHANGE in the selected index — the
/// only time retail HudList-style lists scroll to reveal a row — lets a
/// stable selection coexist with the user scrolling away from it via the
/// bar, while still auto-revealing a genuinely NEW selection exactly
/// once. Sentinel <see cref="int.MinValue"/> so the very first draw with
/// ANY selected index (including the valid -1 "nothing selected") still
/// runs the clamp once.
/// </summary>
private int _lastRevealedSelected = int.MinValue;
/// <summary>
/// Pixel-based scroll projection used ONLY to feed
/// <see cref="UiScrollbar.ThumbRect"/>'s geometry math (thumb
@ -164,12 +186,16 @@ public sealed class UiMarkupList : UiElement
: 0f;
int visibleRows = VisibleRows;
int selected = SelectedIndexSource();
if (selected >= 0 && selected < items.Count)
if (selected != _lastRevealedSelected)
{
if (selected < _topRow)
_topRow = selected;
else if (selected >= _topRow + visibleRows)
_topRow = selected - visibleRows + 1;
_lastRevealedSelected = selected;
if (selected >= 0 && selected < items.Count)
{
if (selected < _topRow)
_topRow = selected;
else if (selected >= _topRow + visibleRows)
_topRow = selected - visibleRows + 1;
}
}
ClampTop(items.Count, visibleRows);
@ -383,12 +409,16 @@ public sealed class UiMarkupList : UiElement
ComputeColumnLayout(columns, contentWidth);
int selected = SelectedIndexSource();
if (selected >= 0 && selected < rowCount)
if (selected != _lastRevealedSelected)
{
if (selected < _topRow)
_topRow = selected;
else if (selected >= _topRow + visibleRows)
_topRow = selected - visibleRows + 1;
_lastRevealedSelected = selected;
if (selected >= 0 && selected < rowCount)
{
if (selected < _topRow)
_topRow = selected;
else if (selected >= _topRow + visibleRows)
_topRow = selected - visibleRows + 1;
}
}
ClampTop(rowCount, visibleRows);

245
tests/AcDream.App.Tests/UI/UiMarkupListScrollSelectionPersistenceTests.cs Normal file
View file

@ -0,0 +1,245 @@
using System.Linq;
using System.Numerics;
using AcDream.App.Rendering;
using AcDream.App.Rendering.Gpu;
using AcDream.App.Tests.Rendering.Gpu;
using AcDream.App.UI;
using Xunit;
namespace AcDream.App.Tests.UI;
/// <summary>
/// Owner live-client report 2026-09-07 ("Scrolling in advanced options does
/// not work. Scrollbar shows, but can't click or drag slidebar, or it works
/// sometimes"): root cause is in <see cref="UiMarkupList"/> itself, not the
/// coordinate frame between the popup/group/panel nesting.
///
/// <para>
/// <see cref="UiMarkupList.OnDraw"/>/<see cref="UiMarkupList.DrawColumns"/>
/// run a "keep the selected row visible" clamp UNCONDITIONALLY on every
/// single draw call (every frame), not only when the selection actually
/// changes. A real plugin list (e.g. MossTank's Advanced Options,
/// <c>selected="{SelectedAdvancedOptionIndex}"</c>) keeps a STABLE selected
/// index while the user scrolls elsewhere with the scrollbar — but because
/// that index does not change, the very next frame's <c>OnDraw</c> sees the
/// (unchanged) selected row now outside the just-scrolled view and snaps
/// <c>_topRow</c> straight back to reveal it, undoing the arrow click or
/// thumb drag before the user ever sees the new scroll position. This
/// reproduces exactly the "sometimes works" symptom: a scroll that keeps the
/// stable selected row in view survives (looks like it "worked"); one that
/// moves the selected row out of view gets silently reverted on the next
/// frame.
/// </para>
///
/// <para>
/// Every test here builds a REAL nested Panel &gt; Group &gt; List tree (like
/// MossTank's Advanced Options group living inside its panel) at non-zero
/// offsets at every level, and drives it exclusively through
/// <see cref="UiRoot"/>'s public mouse entry points — the same path
/// <c>GameWindow</c> uses — to also rule out a coordinate-frame bug in the
/// hit-test/dispatch chain (candidate 1 in the investigation). A stray
/// coordinate-frame bug would make the FIRST click (the arrow / thumb press)
/// silently miss the scrollbar; these tests assert that press succeeds AND
/// that the resulting scroll position survives a subsequent redraw (the
/// real regression).
/// </para>
/// </summary>
public sealed class UiMarkupListScrollSelectionPersistenceTests
{
private sealed class TestElement : UiElement { }
private const float ListWidth = 100f;
private const float ListHeight = 100f; // RowHeight 10 => 10 visible rows
private const float RowHeight = 10f;
private const int RowCount = 40;
// Non-zero nesting offsets at every level (panel > group > list), mirroring
// MossTank's own panel(28,42) > group(8,42) > list(4,24) structure.
private const float PanelLeft = 50f, PanelTop = 60f;
private const float GroupLeft = 10f, GroupTop = 20f;
private const float ListLeft = 5f, ListTop = 15f;
private static (uint tex, int w, int h) Resolve(uint id) => (id, 16, 16);
private sealed class Harness
{
public readonly UiRoot Root = new() { Width = 1000f, Height = 800f };
public readonly TestElement Panel;
public readonly TestElement Group;
public readonly UiMarkupList List;
public Harness(UiMarkupList list)
{
List = list;
Panel = new TestElement
{
Left = PanelLeft, Top = PanelTop, Width = 900f, Height = 700f,
Draggable = true,
};
Group = new TestElement
{
Left = GroupLeft, Top = GroupTop, Width = 800f, Height = 600f,
};
List.Left = ListLeft; List.Top = ListTop;
Group.AddChild(List);
Panel.AddChild(Group);
Root.AddChild(Panel);
}
/// <summary>Absolute root-space screen coords for a point expressed in
/// the list's own local space — the same translation
/// <see cref="UiElement.ScreenPosition"/> performs by walking every
/// ancestor's Left/Top.</summary>
public (int x, int y) ToScreen(float localX, float localY) =>
((int)(PanelLeft + GroupLeft + ListLeft + localX),
(int)(PanelTop + GroupTop + ListTop + localY));
public void Click(float localX, float localY)
{
var (x, y) = ToScreen(localX, localY);
Root.OnMouseDown(UiMouseButton.Left, x, y);
Root.OnMouseUp(UiMouseButton.Left, x, y);
}
public void PressMoveRelease(float downLocalX, float downLocalY, float moveLocalX, float moveLocalY)
{
var (dx, dy) = ToScreen(downLocalX, downLocalY);
Root.OnMouseDown(UiMouseButton.Left, dx, dy);
var (mx, my) = ToScreen(moveLocalX, moveLocalY);
Root.OnMouseMove(mx, my);
Root.OnMouseUp(UiMouseButton.Left, mx, my);
}
}
// ── Single-column mode ───────────────────────────────────────────────
[Fact]
public void SingleColumn_DownArrowClick_TopRowSurvivesTheNextDraw()
{
var list = new UiMarkupList
{
Width = ListWidth, Height = ListHeight, RowHeight = RowHeight,
SpriteResolve = Resolve,
// A REALISTIC stable selection (unlike the -1 the existing
// scrollbar tests use) — the user has row 0 selected and is not
// touching selection while scrolling elsewhere via the bar.
SelectedIndexSource = () => 0,
ItemsSource = () => Enumerable.Range(0, RowCount).Select(i => $"row{i}").ToArray(),
};
var h = new Harness(list);
h.Root.DrawSelfAndChildren(NullCtx());
// Down-arrow occupies the bottom 16px of the reserved scrollbar
// column: x in [84,100), y in [84,100).
h.Click(localX: 90, localY: 90);
// Simulate the NEXT render frame — this is where the unconditional
// reveal-selected clamp (pre-fix) reverts the scroll.
h.Root.DrawSelfAndChildren(NullCtx());
int? selected = null;
list.SelectionChanged = row => selected = row;
// Click the first visible row (local y in row 0's band).
h.Click(localX: 10, localY: 2);
Assert.Equal(1, selected);
}
[Fact]
public void SingleColumn_ThumbDrag_TopRowAdvancesThreeRows_AndSurvivesTheNextDraw()
{
var list = new UiMarkupList
{
Width = ListWidth, Height = ListHeight, RowHeight = RowHeight,
SpriteResolve = Resolve,
SelectedIndexSource = () => 0,
ItemsSource = () => Enumerable.Range(0, RowCount).Select(i => $"row{i}").ToArray(),
};
var h = new Harness(list);
h.Root.DrawSelfAndChildren(NullCtx());
// Track: y in [16,84) (Height 100 - 16 up - 16 down). Thumb starts at
// the very top (topRow=0): height = max(8, 68*10/40=17) = 17, so
// y in [16,33). Press at the thumb's own top edge (y=16, zero drag
// offset) and drag down 5px — by the exact geometry in this file's
// header comment, this lands on topRow=3.
h.PressMoveRelease(downLocalX: 90, downLocalY: 16, moveLocalX: 90, moveLocalY: 21);
h.Root.DrawSelfAndChildren(NullCtx());
int? selected = null;
list.SelectionChanged = row => selected = row;
h.Click(localX: 10, localY: 2);
Assert.Equal(3, selected);
}
// ── Multi-column mode ────────────────────────────────────────────────
[Fact]
public void Columns_DownArrowClick_TopRowSurvivesTheNextDraw()
{
var list = new UiMarkupList
{
Width = ListWidth, Height = ListHeight, RowHeight = RowHeight,
SpriteResolve = Resolve,
SelectedIndexSource = () => 0,
Columns = new[]
{
UiMarkupListColumn.Text(
ListWidth, () => Enumerable.Range(0, RowCount).Select(i => $"row{i}").ToArray(), null),
},
};
var h = new Harness(list);
h.Root.DrawSelfAndChildren(NullCtx());
h.Click(localX: 90, localY: 90);
h.Root.DrawSelfAndChildren(NullCtx());
int? selected = null;
list.SelectionChanged = row => selected = row;
h.Click(localX: 10, localY: 2);
Assert.Equal(1, selected);
}
[Fact]
public void Columns_ThumbDrag_TopRowAdvancesThreeRows_AndSurvivesTheNextDraw()
{
var list = new UiMarkupList
{
Width = ListWidth, Height = ListHeight, RowHeight = RowHeight,
SpriteResolve = Resolve,
SelectedIndexSource = () => 0,
Columns = new[]
{
UiMarkupListColumn.Text(
ListWidth, () => Enumerable.Range(0, RowCount).Select(i => $"row{i}").ToArray(), null),
},
};
var h = new Harness(list);
h.Root.DrawSelfAndChildren(NullCtx());
h.PressMoveRelease(downLocalX: 90, downLocalY: 16, moveLocalX: 90, moveLocalY: 21);
h.Root.DrawSelfAndChildren(NullCtx());
int? selected = null;
list.SelectionChanged = row => selected = row;
h.Click(localX: 10, localY: 2);
Assert.Equal(3, selected);
}
private static UiRenderContext NullCtx()
{
var device = new RecordingGpuDevice();
var renderer = new TextRenderer(device, new NullGpuFrameSource(), "unused");
renderer.Begin(new Vector2(1000f, 800f));
return new UiRenderContext(renderer, new Vector2(1000f, 800f));
}
private sealed class NullGpuFrameSource : ICurrentGpuFrameSource
{
public IGpuFrame? CurrentFrame => null;
}
}