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

fix(plugin-ui): shelf children must not anchor — the per-draw anchor pass was undoing collapse/reflow geometry

Browse source 
EVIDENCE (live UI probe dump at 1280x720, build 2ebcc0164): after collapsing
the plugin shelf, PluginSidePanel rect=(1240,233,24,28) was correct, but
ShelfGripPanel rect=(1240,233,20,18) and the toggle rect=(1260,233,16,18)
stayed UNCHANGED from the expanded geometry. The toggle then sat outside the
24px shelf and the ancestor clip removed it, so the owner saw the tab with
no "<".

ROOT CAUSE: UiElement.ApplyAnchor (src/AcDream.App/UI/UiElement.cs ~829-856)
runs for every child on every draw (called at :699). For any child whose
Anchors != AnchorEdges.None, it captures the Left/Top/Width/Height margins
ONCE (_anchorCaptured) on the first draw and re-applies that snapshot every
subsequent draw, overwriting whatever PluginSidePanel.LayoutChrome/Reflow had
just set. The grip, the toggle, and each PluginShelfButton entry were
constructed with the default Anchors (Left|Top), so their first-draw
geometry froze. The shelf itself already used AnchorEdges.None for exactly
this reason. Unit tests never caught it because UiRoot.Tick does not draw —
the anchor snapshot only exists after a real Draw pass, and the prior
draw-level toggle tests only ever drew once, before any collapse.

FIX: set Anchors = AnchorEdges.None on _grip, _toggle (PluginSidePanel
constructor) and each PluginShelfButton entry (PluginSidePanel.Add) — the
shelf is the sole layout owner of these children and anchoring is the wrong
mechanism for them, not a per-reflow patch via ResetAnchorCapture().
PluginMinimizeButton is untouched (it is a child of the plugin window and
deliberately anchors Top|Right).

TESTS (tests/AcDream.App.Tests/UI/PluginSidePanelToggleGlyphClipTests.cs):
- Collapse_AfterADraw_RepositionsGripAndToggle_NotFrozenAtExpandedGeometry:
  draws the shelf, collapses via a real UiRoot press/release, draws again,
  and asserts the grip/toggle geometry actually reflects the collapsed
  Width/Height rather than the frozen expanded snapshot. Failed-first
  (pre-fix) at line 364 with "Expected: 8, Actual: 20" (grip.Width frozen at
  the pre-collapse value instead of the new collapsed Width - ToggleWidth).
- MultiColumnReflow_AfterADraw_EveryRemainingButtonMatchesAFreshSinglePassLayout:
  12 entries, draw, unregister one window (a real removal), draw again, and
  compares every surviving button's geometry against an independent
  reference shelf built directly with the same final 11-entry set. Failed-
  first with "Expected: 16, Actual: 48" (a surviving button's Left frozen at
  its stale 12-entry column/row instead of the fresh 11-entry reflow).

Both tests use font: null (bitmap fallback) so they run in every CI lane
without an installed retail DAT, unlike the Lane=InstalledDat tests above.

VERIFY: dotnet build (Release) green for src/AcDream.App and the test
project. Targeted filter (PluginSidePanel|Markup|UiRootInput): 130/130
passed. Full tests/AcDream.App.Tests suite: 7353 passed / 97 skipped / 36
failed - matching the stated baseline (7351/97/36) plus the two new tests;
the 36 failures are the pre-existing environment-gated set (installed-DAT
version mismatch, Linux-only waiter, Lane=Manual live-mount probes) and are
unrelated to this change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-06 16:04:04 +02:00
parent 466272ec55
commit 47eb2d575b
2 changed files with 258 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

26
src/AcDream.App/UI/PluginSidePanel.cs
View file

@ -224,11 +224,29 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState
BorderThickness = 1f;
Visible = false;
// Anchors = None on both: the shelf is the SOLE layout owner of these
// children (LayoutChrome, called from Reflow, repositions them every
// time the shelf's own Width/Height changes — collapse/expand,
// entries added/removed, row-wrap). UiElement.ApplyAnchor is called
// for every child on every draw; for any child whose Anchors is not
// AnchorEdges.None it captures the Left/Top/Width/Height margins
// ONCE on the FIRST draw and re-applies that frozen snapshot on
// every subsequent draw, silently overwriting whatever LayoutChrome
// had just written. Left at the default Left|Top, the grip/toggle
// therefore stayed pinned at their EXPANDED geometry after a
// collapse (owner report 2026-09-06: the toggle ended up outside
// the collapsed shelf and the ancestor clip removed it — "I dont
// see the < after I minimize the window"). Anchoring is simply the
// wrong mechanism for a child whose layout owner already re-derives
// its full geometry every reflow; ResetAnchorCapture() would only
// patch this one call site; the actual fix is to never engage the
// capture-and-freeze machinery for these children at all.
_grip = new ShelfGripPanel
{
WindowMoveHandle = true,
BackgroundColor = Vector4.Zero,
BorderColor = Vector4.Zero,
Anchors = AnchorEdges.None,
};
_toggle = new UiSimpleButton
{
@ -238,6 +256,7 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState
DatFont = _font,
Outline = true,
TextSource = () => _collapsed ? "<" : ">",
Anchors = AnchorEdges.None,
};
_toggle.Click += ToggleCollapsed;
AddChild(_grip);
@ -285,6 +304,13 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState
{
Width = ButtonExtent,
Height = ButtonExtent,
// Anchors = None for the same reason as _grip/_toggle above:
// Reflow() is the sole owner of each entry button's Left/Top
// (row-wrap column/row placement), rewritten on every add/
// remove/collapse. Left at the default anchor, ApplyAnchor would
// freeze a button at whatever column/row it first drew in and
// never let a later removal's row-wrap actually move it.
Anchors = AnchorEdges.None,
};
button.Click += () =>
{

232
tests/AcDream.App.Tests/UI/PluginSidePanelToggleGlyphClipTests.cs
View file

@ -266,4 +266,236 @@ public sealed class PluginSidePanelToggleGlyphClipTests
Assert.True(shelf.Height > 20f, $"collapsed shelf height ({shelf.Height}) is not button-sized");
Assert.NotEqual(expandedHeight, shelf.Height);
}
/// <summary>
/// Owner-reported defect (2026-09-06 live probe): after collapsing the
/// shelf, the grip and toggle stayed pinned at the EXPANDED geometry —
/// the toggle ended up outside the 24px collapsed shelf and the ancestor
/// clip removed it, so the owner saw the tab with no <c>&lt;</c>.
///
/// <para>
/// Root cause: <see cref="UiElement.ApplyAnchor"/> is called for every
/// child on every draw. For any child whose <see cref="UiElement.Anchors"/>
/// is not <see cref="AnchorEdges.None"/>, it captures the Left/Top/Width/
/// Height margins ONCE on the FIRST draw and re-applies that snapshot on
/// every subsequent draw — overwriting whatever
/// <see cref="PluginSidePanel.LayoutChrome"/>/<c>Reflow</c> set in between.
/// <see cref="UiRoot.Tick"/> never draws, so a test that only ticks (as
/// every prior collapse test in this tree does) never exercises this path
/// at all — this test drives two REAL <see cref="UiRoot.Draw"/> passes,
/// with a real toggle click in between, to reproduce it.
/// </para>
///
/// <para>
/// Uses a bitmap-fallback font (<c>font: null</c>) deliberately, per the
/// investigation's direction, so this reproduction runs in every CI lane
/// without an installed retail DAT — unlike the <c>Lane=InstalledDat</c>
/// tests above, which require one.
/// </para>
/// </summary>
[Fact]
public void Collapse_AfterADraw_RepositionsGripAndToggle_NotFrozenAtExpandedGeometry()
{
var root = new UiRoot { Width = 800f, Height = 600f };
using var shelf = new PluginSidePanel(root.WindowManager, _ => (0u, 0, 0), font: null);
root.AddChild(shelf);
for (int i = 0; i < 2; i++)
{
var frame = new UiPanel { Width = 200f, Height = 100f };
root.AddChild(frame);
RetailWindowHandle handle = root.WindowManager.Register(
$"plugin:acdream.test:{i}", frame);
shelf.Add(
new PluginUiOwner($"acdream.test.{i}", $"Test Plugin {i}"),
new PluginPanelDescriptor("main", $"Test Plugin {i}"),
handle);
}
root.Tick(0.016d, 16L);
var device = new RecordingGpuDevice();
var renderer = new TextRenderer(device, new NullGpuFrameSource(), "unused");
var ctx = new UiRenderContext(renderer, new Vector2(800f, 600f));
// ── First draw: this is what CAPTURES the anchor snapshot (at the
// expanded geometry) — nothing is wrong yet, since the snapshot
// matches the current (correct) layout. ──────────────────────────
renderer.Begin(new Vector2(800f, 600f));
root.Draw(ctx);
UiSimpleButton toggle = Assert.Single(
shelf.Children,
c => c is UiSimpleButton && c is not PluginSidePanel.PluginShelfButton) as UiSimpleButton
?? throw new InvalidOperationException("toggle button not found");
UiElement grip = Assert.Single(
shelf.Children,
c => c is UiPanel && c is not UiSimpleButton);
float expandedShelfHeight = shelf.Height;
float expandedBandHeight = shelf.ExpandedGripBandHeight;
float toggleWidth = toggle.Width;
Assert.Equal(0f, grip.Left);
Assert.Equal(0f, grip.Top);
Assert.Equal(shelf.Width - toggleWidth, grip.Width, 3);
Assert.Equal(expandedBandHeight, grip.Height, 3);
Assert.Equal(shelf.Width - toggleWidth, toggle.Left, 3);
Assert.Equal(0f, toggle.Top);
Assert.Equal(expandedBandHeight, toggle.Height, 3);
// ── Collapse via a REAL press/release through UiRoot (the production
// input path), not the state controller directly. ────────────────
int toggleX = (int)shelf.Left + (int)shelf.Width - 8;
int toggleY = (int)shelf.Top + 4;
root.OnMouseDown(UiMouseButton.Left, toggleX, toggleY);
root.OnMouseUp(UiMouseButton.Left, toggleX, toggleY);
Assert.True(shelf.CaptureWindowState().Collapsed);
// ── Second draw: must reflect the COLLAPSED geometry. Pre-fix, the
// anchor snapshot captured above wins instead: grip/toggle stay at
// their EXPANDED Left/Width/Height regardless of what LayoutChrome
// just wrote via Reflow. ───────────────────────────────────────────
renderer.Begin(new Vector2(800f, 600f));
root.Draw(ctx);
Assert.Equal(0f, grip.Left);
Assert.Equal(0f, grip.Top);
Assert.Equal(shelf.Width - toggleWidth, grip.Width, 3);
Assert.Equal(shelf.Height, grip.Height, 3);
Assert.Equal(shelf.Width - toggleWidth, toggle.Left, 3);
Assert.Equal(0f, toggle.Top);
Assert.Equal(shelf.Height, toggle.Height, 3);
Assert.True(
toggle.Left + toggle.Width <= shelf.Width + 0.01f,
$"toggle (Left={toggle.Left}, Width={toggle.Width}) lies outside " +
$"the collapsed shelf (Width={shelf.Width}) — the ancestor clip " +
"would remove it, matching the owner's \"no <\" report.");
// ── Expand again: the geometry must be restored too, via another
// real click. ──────────────────────────────────────────────────────
int toggleX2 = (int)shelf.Left + (int)shelf.Width - 8;
int toggleY2 = (int)shelf.Top + 4;
root.OnMouseDown(UiMouseButton.Left, toggleX2, toggleY2);
root.OnMouseUp(UiMouseButton.Left, toggleX2, toggleY2);
Assert.False(shelf.CaptureWindowState().Collapsed);
renderer.Begin(new Vector2(800f, 600f));
root.Draw(ctx);
Assert.Equal(expandedShelfHeight, shelf.Height, 3);
Assert.Equal(0f, grip.Left);
Assert.Equal(0f, grip.Top);
Assert.Equal(shelf.Width - toggleWidth, grip.Width, 3);
Assert.Equal(expandedBandHeight, grip.Height, 3);
Assert.Equal(shelf.Width - toggleWidth, toggle.Left, 3);
Assert.Equal(0f, toggle.Top);
Assert.Equal(expandedBandHeight, toggle.Height, 3);
}
/// <summary>
/// Same double-draw pin as
/// <see cref="Collapse_AfterADraw_RepositionsGripAndToggle_NotFrozenAtExpandedGeometry"/>,
/// but for the <see cref="PluginSidePanel.PluginShelfButton"/> ENTRIES
/// rather than the grip/toggle: a multi-column row-wrapped layout, draw,
/// remove one entry (a real window unregister — the same path a plugin
/// unload takes), draw again. Every remaining button's Left/Top must
/// match what a FRESH single-pass layout of the same final entry set
/// would produce — not a stale position captured from the 12-entry
/// layout on the first draw.
///
/// <para>
/// Compares against an independent "reference" shelf built directly with
/// the final (post-removal) entry set and drawn exactly once, so the
/// expected geometry is derived from the real layout math rather than
/// re-hard-coded private constants (<c>OuterPadding</c>/<c>ButtonExtent</c>/
/// <c>ButtonGap</c> are private to <see cref="PluginSidePanel"/>).
/// </para>
/// </summary>
[Fact]
public void MultiColumnReflow_AfterADraw_EveryRemainingButtonMatchesAFreshSinglePassLayout()
{
const int entryCount = 12;
var root = new UiRoot { Width = 800f, Height = 260f };
using var shelf = new PluginSidePanel(root.WindowManager, _ => (0u, 0, 0), font: null);
root.AddChild(shelf);
var handles = new RetailWindowHandle[entryCount];
for (int i = 0; i < entryCount; i++)
{
var frame = new UiPanel { Width = 200f, Height = 100f };
root.AddChild(frame);
handles[i] = root.WindowManager.Register($"plugin:acdream.test:{i}", frame);
shelf.Add(
new PluginUiOwner($"acdream.test.{i}", $"Test Plugin {i}"),
new PluginPanelDescriptor("main", $"Test Plugin {i}"),
handles[i]);
}
root.Tick(0.016d, 16L);
var device = new RecordingGpuDevice();
var renderer = new TextRenderer(device, new NullGpuFrameSource(), "unused");
var ctx = new UiRenderContext(renderer, new Vector2(800f, 600f));
// ── First draw: captures anchors at the full 12-entry layout. ─────
renderer.Begin(new Vector2(800f, 600f));
root.Draw(ctx);
// ── Remove the FIRST entry — every remaining button shifts down one
// slot, so this exercises the widest possible reflow. ─────────────
root.WindowManager.Unregister(handles[0].Name);
Assert.Equal(entryCount - 1, shelf.EntryCount);
// ── Second draw: must reflect the fresh 11-entry reflow. Pre-fix,
// each surviving button stays pinned at its ORIGINAL 12-entry
// position/size instead. ───────────────────────────────────────────
renderer.Begin(new Vector2(800f, 600f));
root.Draw(ctx);
List<PluginSidePanel.PluginShelfButton> afterRemoval =
shelf.Children.OfType<PluginSidePanel.PluginShelfButton>().ToList();
Assert.Equal(entryCount - 1, afterRemoval.Count);
// ── Reference: an independent shelf built directly with the SAME
// final 11-entry set (never having been through a 12-entry layout),
// ticked and drawn exactly once. ───────────────────────────────────
var referenceRoot = new UiRoot { Width = 800f, Height = 260f };
using var referenceShelf = new PluginSidePanel(
referenceRoot.WindowManager, _ => (0u, 0, 0), font: null);
referenceRoot.AddChild(referenceShelf);
for (int i = 1; i < entryCount; i++)
{
var frame = new UiPanel { Width = 200f, Height = 100f };
referenceRoot.AddChild(frame);
RetailWindowHandle handle = referenceRoot.WindowManager.Register(
$"plugin:acdream.reference:{i}", frame);
referenceShelf.Add(
new PluginUiOwner($"acdream.reference.{i}", $"Test Plugin {i}"),
new PluginPanelDescriptor("main", $"Test Plugin {i}"),
handle);
}
referenceRoot.Tick(0.016d, 16L);
var referenceDevice = new RecordingGpuDevice();
var referenceRenderer = new TextRenderer(referenceDevice, new NullGpuFrameSource(), "unused");
var referenceCtx = new UiRenderContext(referenceRenderer, new Vector2(800f, 600f));
referenceRenderer.Begin(new Vector2(800f, 600f));
referenceRoot.Draw(referenceCtx);
List<PluginSidePanel.PluginShelfButton> reference =
referenceShelf.Children.OfType<PluginSidePanel.PluginShelfButton>().ToList();
Assert.Equal(reference.Count, afterRemoval.Count);
for (int i = 0; i < afterRemoval.Count; i++)
{
Assert.Equal(reference[i].Left, afterRemoval[i].Left, 3);
Assert.Equal(reference[i].Top, afterRemoval[i].Top, 3);
Assert.Equal(reference[i].Width, afterRemoval[i].Width, 3);
Assert.Equal(reference[i].Height, afterRemoval[i].Height, 3);
}
Assert.Equal(shelf.Width, referenceShelf.Width, 3);
Assert.Equal(shelf.Height, referenceShelf.Height, 3);
}
}