diff --git a/src/AcDream.App/UI/UiMarkupList.cs b/src/AcDream.App/UI/UiMarkupList.cs index ddebc0a6e..d886893be 100644 --- a/src/AcDream.App/UI/UiMarkupList.cs +++ b/src/AcDream.App/UI/UiMarkupList.cs @@ -116,6 +116,28 @@ public sealed class UiMarkupList : UiElement private int _topRow; private IReadOnlyList? _columns; + /// + /// Owner live-client report 2026-09-07 ("Scrolling in advanced options + /// does not work... or it works sometimes"): the last + /// value the "keep selection visible" + /// clamp below (/) actually + /// reacted to. A real bound list (e.g. MossTank's Advanced Options, + /// selected="{SelectedAdvancedOptionIndex}") 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 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 so the very first draw with + /// ANY selected index (including the valid -1 "nothing selected") still + /// runs the clamp once. + /// + private int _lastRevealedSelected = int.MinValue; + /// /// Pixel-based scroll projection used ONLY to feed /// '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); diff --git a/tests/AcDream.App.Tests/UI/UiMarkupListScrollSelectionPersistenceTests.cs b/tests/AcDream.App.Tests/UI/UiMarkupListScrollSelectionPersistenceTests.cs new file mode 100644 index 000000000..e23f2be0c --- /dev/null +++ b/tests/AcDream.App.Tests/UI/UiMarkupListScrollSelectionPersistenceTests.cs @@ -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; + +/// +/// 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 itself, not the +/// coordinate frame between the popup/group/panel nesting. +/// +/// +/// / +/// 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, +/// selected="{SelectedAdvancedOptionIndex}") 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 OnDraw sees the +/// (unchanged) selected row now outside the just-scrolled view and snaps +/// _topRow 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. +/// +/// +/// +/// Every test here builds a REAL nested Panel > Group > List tree (like +/// MossTank's Advanced Options group living inside its panel) at non-zero +/// offsets at every level, and drives it exclusively through +/// 's public mouse entry points — the same path +/// GameWindow 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). +/// +/// +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); + } + + /// Absolute root-space screen coords for a point expressed in + /// the list's own local space — the same translation + /// performs by walking every + /// ancestor's Left/Top. + 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; + } +}