fix #374: open dropdown popups get first claim on pointer routing

Campaign OP gate 2 root cause: UiElement.HitTest walks siblings
front-to-back by z-order, so an OPEN UiMenu's extended button+popup
hit-test union was never consulted when a LATER sibling's rect overlapped
the popup area — on the Config tab every dropdown has rows below it, so
Resolution-item clicks toggled the Full Screen / VSync rows underneath
(the gate session's persisted fullscreen/vsync flips were exactly those
stolen clicks). Latent since UiMenu existed; vendor/chat menus only
worked by z-order luck.

Fix: UiMenu's open/close now registers with UiRoot (SetActivePopup /
ClearActivePopup); a registered popup gets FIRST claim on mouse-down,
scroll, and hover routing; a press outside a live popup dismisses it and
is SWALLOWED (the dismissing click must not act on what sat underneath);
hidden/detached owners self-heal the registration on the next pointer
event. UiMenu gains the IsOpen seam and a single SetOpen writer.

Also in this commit, from the same investigation:
- SilkRuntimeDisplayWindowTarget.Apply documents the fullscreen half
  honestly: IViewProperties.VideoMode is READ-ONLY, so a resolution pick
  while fullscreen cannot switch the display mode through Silk's
  abstract API — split out as #376 (native glfwSetWindowMonitor port)
  rather than half-shipping untested native interop at a gate tail.
- Gate script §OP6 step 8 re-scoped: test resolution in WINDOWED mode.

Regressed by tests/AcDream.App.Tests/UI/UiMenuPopupRoutingTests.cs —
4 tests driving the real UiRoot input path on a mounted overlapping
tree, with an in-test overlap CONTROL click so the popup assertions
cannot pass vacuously (the #372 lesson: only mount+drive-input tests
catch this class; every fixture-conformance test stayed green through
this bug).

Full Release suite: 13,081 passed / 4 skipped / 0 failed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-11 15:17:17 +02:00
parent 07c0c2c7b9
commit 355c86a6f6
6 changed files with 388 additions and 10 deletions

View file

@ -24,6 +24,86 @@ What does NOT go here:
- Every session: scan OPEN issues at start; promote/close anything we touched during the session before ending.
- Promoting to a Phase: mark as `DONE (promoted to Phase X)` + commit SHA where the Phase entry landed.
## #376 — Fullscreen resolution picks cannot switch the display mode (Silk API limit; needs native glfwSetWindowMonitor)
**Status:** OPEN — filed 2026-08-11, split from #374's investigation.
While FULLSCREEN, the visible resolution is the display's video mode, and
Silk's abstract windowing API cannot change it:
`IViewProperties.VideoMode` is read-only, and Silk fullscreen is
desktop-mode borderless. `SilkRuntimeDisplayWindowTarget.Apply`
(`src/AcDream.App/Settings/RuntimeSettingsTargets.cs`) therefore applies a
Config-tab resolution pick to the WINDOWED size only — visible immediately
in windowed mode, and on the next return to windowed when picked while
fullscreen. Retail's own fullscreen switch is a real display-mode change
(`Device::ForceDisplayResolution`, `gmClient::Init @0x004047af`). Fix
shape: a native GLFW port — reach the underlying handle and call
`glfwSetWindowMonitor(window, monitor, 0, 0, width, height, refresh)`
through `Silk.NET.GLFW` when fullscreen, keeping the abstract path for
windowed. Needs a physical gate (mode switches can black-screen on bad
modes; validate against the monitor's mode list first).
## #375 — Configure Keyboard screen renders as a visual mess at the live mount (missing button/tab captions, buttons outside the window, overlapping text)
**Status:** OPEN — filed 2026-08-11 at Campaign OP's second connected gate
(user report, verbatim observations): "all buttons lacked descriptive
text", "some buttons were outside of the window", "lacked text in the
tabs", "text next to the buttons was overlapping. Looked like a mess."
The screen OPENED (the `[options] gameplay button 0x10000204 clicked`
log line fired and the user saw the window), so the OP8 mount/wiring is
alive — the defect family is presentation at the LIVE mount.
**Same false-negative class as #372:** `KeyboardConfigControllerTests`
runs green against the committed `keyboard_config_21000009.json` fixture,
so the structural conformance suite cannot see whatever the live
DAT mount does differently (string resolution, template sizing,
anchor baselines, window extent). Diagnosis must start from live-mount
evidence, not the fixture: extend the `ACDREAM_PROBE_LIVE_MOUNT=1` probe
(`OptionsPanelLiveMountProbeTests` pattern) to mount `0x21000009` against
the real DATs and dump per-element rect + resolved caption, then compare
against the user's four observations. Candidate families (to CONFIRM, not
assume): caption lookups missing their real string table (the exact
`0x2300000D` class from #372's session), row-template text elements
sized/positioned from degenerate baselines, and the screen's authored
extent vs where children actually land.
**Blocks the OP8 connected gate.**
## #374 — Config tab: picking a new Resolution does not resize the window (live gate failure)
**Status:** ROOT-CAUSED + FIXED (this commit) — pending the user's
re-gate. Filed 2026-08-11 at Campaign OP's second connected gate ("I was
not able to change the screen resolution").
**ROOT CAUSE — popup hit-test priority, a UiRoot-level routing hole.**
`UiElement.HitTest` walks siblings front-to-back by z-order; an OPEN
`UiMenu` extends its hit area beyond its own rect (the button+popup union
in `UiMenu.OnHitTest`), but any sibling added AFTER the menu whose rect
overlaps the popup area wins the walk before the menu's extended
hit-test is ever consulted. On the Config tab every dropdown has rows
BELOW it — so clicking a Resolution popup item actually clicked the rows
underneath (the session's persisted `fullscreen: true` + `vsync: false`
flips were exactly such stolen clicks toggling the Full Screen / VSync
rows under the open popup). Vendor's and chat's menus only ever worked
because no overlapping sibling sat in front of them — the hole was
latent since UiMenu existed. **Fix:** an open popup registers with
`UiRoot` (`SetActivePopup`) and gets FIRST claim on mouse-down, scroll,
and hover routing; a press outside the popup dismisses it and is
SWALLOWED (the standard dropdown-dismiss gesture — the dismissing click
must not act on whatever sat underneath); hidden/detached owners
self-heal the registration. Regressed by
`tests/AcDream.App.Tests/UI/UiMenuPopupRoutingTests.cs` (4 tests, with
an in-test overlap CONTROL so the assertions cannot pass vacuously).
The investigation also surfaced the fullscreen half — a resolution pick
while fullscreen cannot switch the display mode through Silk's abstract
API at all — split out as #376. And the same session's log showed 64
full `settings.json` disk writes from slider drags (one per drag tick);
noted here as a minor perf observation, not yet its own issue.
**Re-gate (§OP6 step 8): test the Resolution row in WINDOWED mode** —
the pick should now register and resize immediately; fullscreen mode
switching stays #376.
## #373 — Configure Keyboard: DAT `ActionMap.ConflictingMaps` not consulted — the combat cluster raises false conflict prompts
**Status:** OPEN — filed 2026-08-11 at Campaign OP slice OP8's re-review

View file

@ -603,8 +603,16 @@ should show no caption.
### Live rows — display
8. **Open the Resolution menu (Graphics Options, first row) and pick a
different resolution.** The window should resize IMMEDIATELY, live, no
restart needed.
different resolution — IN WINDOWED MODE.** The window should resize
IMMEDIATELY, live, no restart needed. **Gate-2 re-test note (#374):**
the first gate's failure here was a client-wide dropdown routing bug —
clicks on an open popup's items landed on the rows UNDERNEATH it (your
session's Full Screen and VSync flips were those stolen clicks). Fixed;
dropdown items now win while a popup is open, and a click outside an
open popup dismisses it without acting on what's below. While
FULLSCREEN a resolution pick cannot switch the display mode yet
(Silk API limit, #376) — it applies on the next return to windowed;
do not report that as a step-8 failure.
9. **Toggle "Full Screen".** The window should switch between windowed and
fullscreen IMMEDIATELY, live.
10. **Toggle "Sync To Refresh" (VSync) and drag the "Field of View"

View file

@ -56,8 +56,9 @@ internal sealed class SilkRuntimeDisplayWindowTarget : IRuntimeDisplayWindowTarg
public void Apply(DisplaySettings display)
{
ArgumentNullException.ThrowIfNull(display);
if (TryParseResolution(display.Resolution, out int width, out int height)
&& (_window.Size.X != width || _window.Size.Y != height))
bool haveResolution =
TryParseResolution(display.Resolution, out int width, out int height);
if (haveResolution && (_window.Size.X != width || _window.Size.Y != height))
{
_window.Size = new Vector2D<int>(width, height);
}
@ -65,6 +66,16 @@ internal sealed class SilkRuntimeDisplayWindowTarget : IRuntimeDisplayWindowTarg
WindowState desired = display.Fullscreen
? WindowState.Fullscreen
: WindowState.Normal;
// #374 investigation note: while FULLSCREEN, the visible resolution
// is the display's video mode, and Silk's abstract windowing API
// cannot change it (IViewProperties.VideoMode is read-only; Silk
// fullscreen is desktop-mode borderless). The Size write above is
// therefore only visible in windowed mode — a resolution pick while
// fullscreen changes what a later return to windowed restores, not
// the fullscreen mode itself. Retail's own fullscreen resolution
// switch (Device::ForceDisplayResolution) needs a native
// glfwSetWindowMonitor port — issue #376.
if (_window.WindowState != desired)
_window.WindowState = desired;
}

View file

@ -201,6 +201,28 @@ public sealed class UiMenu : UiElement
public Vector4 TextColorGhosted { get; set; } = new(0.5f, 0.5f, 0.5f, 1f);
private bool _open;
/// <summary>Whether the popup is currently open (test/inspection seam,
/// same rationale as <see cref="PopupScroll"/>/<see cref="CurrentArrowCapSprite"/>).</summary>
public bool IsOpen => _open;
/// <summary>The ONLY writer of <see cref="_open"/>: keeps the root's
/// transient-popup registration (#374 — an open popup gets first claim
/// on pointer routing, because the sibling z-order walk would otherwise
/// hand popup-area clicks to whatever front sibling overlaps it) in
/// lockstep with the widget's own state. A detached menu (no root yet)
/// still toggles locally — registration happens against the root that
/// dispatches the events, which by construction exists whenever a real
/// pointer event reaches this widget.</summary>
private void SetOpen(bool value)
{
if (_open == value) return;
_open = value;
if (FindRoot() is not { } root) return;
if (value) root.SetActivePopup(this, () => SetOpen(false));
else root.ClearActivePopup(this);
}
// Interior = the row content; Outer = interior + the 8-piece bevel ring.
// Scrollable: always exactly one column (RowsPerColumn is the VISIBLE window,
// not a wrap threshold), widened by the docked scrollbar's own authored width.
@ -555,11 +577,11 @@ public sealed class UiMenu : UiElement
OnSelect?.Invoke(Items[idx].Payload);
}
}
_open = false;
SetOpen(false);
return true;
}
_open = !_open; // toggle on button click
SetOpen(!_open); // toggle on button click
return true;
}
@ -581,7 +603,7 @@ public sealed class UiMenu : UiElement
{
OnSelect?.Invoke(Items[idx].Payload);
}
_open = false;
SetOpen(false);
return true;
}
@ -611,7 +633,7 @@ public sealed class UiMenu : UiElement
}
// Clicked the bevel ring — close, matching the grid path.
_open = false;
SetOpen(false);
return true;
}

View file

@ -463,6 +463,67 @@ public sealed class UiRoot : UiElement
WorldMouseMoveFallThrough?.Invoke(x, y);
}
// ── Popup routing (#374) ────────────────────────────────────────────
//
// An OPEN transient popup (a UiMenu dropdown) extends its owner's
// hit-test area beyond the owner's own rect (UiMenu.OnHitTest's
// button+popup union). But HitTestTopDown walks SIBLINGS front-to-back
// by z-order, and any sibling added after the owner whose rect overlaps
// the popup area wins the walk before the owner's extended OnHitTest is
// ever consulted — on the Options panel's Config tab every dropdown has
// rows BELOW it, so item clicks landed on those rows instead (toggling
// Full Screen / VSync underneath the open Resolution popup). Vendor's
// and chat's menus only ever worked because no overlapping sibling sat
// in front of them. While a popup is registered it gets FIRST claim on
// pointer events; a press outside it dismisses it and is swallowed (the
// standard dropdown-dismiss gesture — the dismissing click must not
// fall through and act on whatever sat under the popup).
private UiElement? _activePopup;
private Action? _activePopupDismiss;
/// <summary>Registers <paramref name="popup"/> as the transient popup
/// with first claim on pointer routing. Replaces any prior registration
/// (its owner keeps its own open state; the previous dismiss is invoked
/// so owner state cannot go stale).</summary>
internal void SetActivePopup(UiElement popup, Action dismiss)
{
if (!ReferenceEquals(_activePopup, popup))
_activePopupDismiss?.Invoke();
_activePopup = popup;
_activePopupDismiss = dismiss;
}
/// <summary>Clears the registration if <paramref name="popup"/> holds it
/// (the owner closed itself — item picked, bevel click, forced close).</summary>
internal void ClearActivePopup(UiElement popup)
{
if (!ReferenceEquals(_activePopup, popup)) return;
_activePopup = null;
_activePopupDismiss = null;
}
/// <summary>The registered popup's hit-test claim on (x,y), with stale
/// registrations (owner hidden/detached, e.g. its window closed while
/// open) self-healing to a dismissed, unregistered state.</summary>
private UiElement? PopupHit(int x, int y)
{
if (_activePopup is not { } popup) return null;
for (UiElement? e = popup; e is not null; e = e.Parent)
{
if (ReferenceEquals(e, this)) break;
if (!e.Visible || !e.Enabled || e.Parent is null)
{
var stale = _activePopupDismiss;
_activePopup = null;
_activePopupDismiss = null;
stale?.Invoke();
return null;
}
}
var pp = popup.ScreenPosition;
return popup.HitTest(x - pp.X, y - pp.Y);
}
public void OnMouseDown(UiMouseButton btn, int x, int y, uint flags = 0)
{
MouseX = x; MouseY = y;
@ -473,7 +534,30 @@ public sealed class UiRoot : UiElement
if (Modal is not null && !ContainsAbsolute(Modal, x, y))
return;
var (target, _, _) = HitTestTopDown(x, y);
UiElement? target;
if (_activePopup is not null)
{
target = PopupHit(x, y);
if (target is null)
{
if (_activePopup is not null)
{
// Press outside a live popup: dismiss it, swallow the press.
var dismiss = _activePopupDismiss;
_activePopup = null;
_activePopupDismiss = null;
dismiss?.Invoke();
return;
}
// Stale registration self-healed inside PopupHit — fall
// through to the ordinary walk for this press.
(target, _, _) = HitTestTopDown(x, y);
}
}
else
{
(target, _, _) = HitTestTopDown(x, y);
}
if (target is null)
{
// Clicking the 3D world exits write mode (no submit) and returns control to
@ -696,6 +780,18 @@ public sealed class UiRoot : UiElement
public void OnScroll(int dy)
{
// An open popup (dropdown) claims the wheel first — its scrollable
// list must scroll even where a front sibling overlaps it (#374).
if (PopupHit(MouseX, MouseY) is { } popupTarget)
{
var pp = popupTarget.ScreenPosition;
var pe = new UiEvent(popupTarget.EventId, popupTarget, UiEventType.Scroll,
Data0: dy,
Data1: (int)(MouseX - pp.X), Data2: (int)(MouseY - pp.Y));
BubbleEvent(popupTarget, in pe);
return;
}
// Scroll goes to the widget under the cursor (not the focused one).
var (target, lx, ly) = HitTestTopDown(MouseX, MouseY);
if (target is null)
@ -956,7 +1052,11 @@ public sealed class UiRoot : UiElement
private void UpdateHover(int x, int y)
{
var (w, _, _) = HitTestTopDown(x, y);
// An open popup claims hover first (#374) — its item highlight must
// track the cursor even where a front sibling overlaps the popup.
UiElement? w = PopupHit(x, y);
if (w is null)
(w, _, _) = HitTestTopDown(x, y);
if (ReferenceEquals(w, _hoverWidget))
{
if (w?.ReceivesHoverMouseMove == true)

View file

@ -0,0 +1,157 @@
using AcDream.App.UI;
namespace AcDream.App.Tests.UI;
/// <summary>
/// #374 (Campaign OP gate 2): an OPEN UiMenu dropdown must get first claim on
/// pointer events. UiElement.HitTest walks siblings front-to-back by z-order,
/// so any sibling added AFTER the menu whose rect overlaps the popup area used
/// to win the walk before the menu's extended OnHitTest union was consulted —
/// on the Options panel's Config tab every dropdown has rows below it, so
/// picking a Resolution item actually toggled the Full Screen / VSync rows
/// underneath the popup. These tests drive the REAL UiRoot input path
/// (synthetic OnMouseDown/OnMouseUp on a mounted tree), the exact class of
/// coverage the fixture-conformance suites cannot provide (#372's lesson).
/// </summary>
public sealed class UiMenuPopupRoutingTests
{
private sealed class ClickRecorder : UiElement
{
public int MouseDowns;
public int Clicks;
public override bool HandlesClick => true;
public override bool OnEvent(in UiEvent e)
{
if (e.Type == UiEventType.MouseDown) { MouseDowns++; return true; }
if (e.Type == UiEventType.Click) { Clicks++; return true; }
return false;
}
}
private static (UiRoot root, UiMenu menu, ClickRecorder stealer, Func<string?> selected)
BuildOverlappingTree()
{
var root = new UiRoot { Width = 800, Height = 600 };
var panel = new UiPanel { Left = 0, Top = 0, Width = 300, Height = 300 };
string? picked = null;
var menu = new UiMenu
{
Left = 10, Top = 20, Width = 46, Height = 20,
OpenUpward = false,
Items = new[]
{
new UiMenu.MenuItem("A", "a"),
new UiMenu.MenuItem("B", "b"),
},
};
menu.OnSelect = p => picked = p as string;
// The sibling "row below" — added AFTER the menu, so the front-to-back
// sibling walk consults it FIRST. Its rect (screen y 44..74) overlaps
// the open popup's first item row (screen y 45..62).
var stealer = new ClickRecorder { Left = 0, Top = 44, Width = 250, Height = 30 };
panel.AddChild(menu);
panel.AddChild(stealer);
root.AddChild(panel);
return (root, menu, stealer, () => picked);
}
private static void Click(UiRoot root, int x, int y)
{
root.OnMouseDown(UiMouseButton.Left, x, y);
root.OnMouseUp(UiMouseButton.Left, x, y);
}
[Fact]
public void OpenPopup_ItemClick_ReachesTheMenu_NotTheOverlappingFrontSibling()
{
var (root, menu, stealer, selected) = BuildOverlappingTree();
// CONTROL: with the menu closed, this exact point belongs to the
// stealer — proving the overlap is real, so the popup assertion below
// cannot pass vacuously on wrong geometry.
Click(root, 30, 50);
Assert.Equal(1, stealer.MouseDowns);
Click(root, 30, 30); // the menu button (screen 10..56 x 20..40)
Assert.True(menu.IsOpen);
// First popup item row: menu-local (20, 30) → interior (15, 5) → row 0.
// The same screen point the control click just proved the stealer owns.
Click(root, 30, 50);
Assert.Equal("a", selected());
Assert.False(menu.IsOpen);
Assert.Equal(1, stealer.MouseDowns); // unchanged — the popup won
Assert.Equal(1, stealer.Clicks);
}
[Fact]
public void OpenPopup_OutsidePress_DismissesAndSwallows_ThenNormalRoutingResumes()
{
var (root, menu, stealer, selected) = BuildOverlappingTree();
Click(root, 30, 30);
Assert.True(menu.IsOpen);
// (230, 60): inside the stealer's rect (x < 250, y in 44..74) but past
// the popup's right edge (screen x 10 + outer width 201 = 211). The
// dismissing press must close the popup WITHOUT acting on the widget
// underneath — the standard dropdown-dismiss gesture, and exactly the
// accidental-toggle hazard #374's gate session demonstrated.
Click(root, 230, 60);
Assert.False(menu.IsOpen);
Assert.Null(selected());
Assert.Equal(0, stealer.MouseDowns);
Assert.Equal(0, stealer.Clicks);
// With the popup gone, the same spot routes normally again.
Click(root, 230, 60);
Assert.Equal(1, stealer.MouseDowns);
Assert.Equal(1, stealer.Clicks);
}
[Fact]
public void ReopeningAfterDismiss_StillRoutesItemClicks()
{
var (root, menu, _, selected) = BuildOverlappingTree();
Click(root, 30, 30); // open
Click(root, 230, 60); // dismiss (swallowed)
Assert.False(menu.IsOpen);
Click(root, 30, 30); // reopen
Assert.True(menu.IsOpen);
Click(root, 30, 50); // pick item A again
Assert.Equal("a", selected());
}
[Fact]
public void HidingThePopupsWindowWhileOpen_SelfHeals_NoStalePopupRouting()
{
var (root, menu, stealer, selected) = BuildOverlappingTree();
Click(root, 30, 30);
Assert.True(menu.IsOpen);
// The panel hosting the menu closes while the popup is open (e.g. the
// Options panel is toggled shut). The stale registration must not keep
// swallowing clicks for an invisible popup.
menu.Parent!.Visible = false;
Click(root, 230, 60); // self-heal press (dismissed, swallowed)
Assert.False(menu.IsOpen);
Click(root, 230, 60);
// The stealer is inside the hidden panel too, so the press falls
// through to nothing — the point is that the popup no longer claims it.
Assert.Equal(0, stealer.MouseDowns);
menu.Parent!.Visible = true;
Click(root, 230, 60);
Assert.Equal(1, stealer.MouseDowns);
Assert.Null(selected());
}
}