diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 1c99a8b6..a471ea5d 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -209,7 +209,7 @@ readiness/requeue adaptation. See | AD-100 | **Filed 2026-08-15 at the Campaign CC CC2 review, finding F2 (unrequested `0xF643` handling).** When a `0xF643` (`CharGenVerificationResponse`) arrives with NO outstanding create/restore request, acdream DROPS the message with a once-per-session stderr log. Retail has no such gate: `Handle_CharGenVerificationResponse @0x0055E8B0` processes whatever arrives, discriminating create-vs-restore by its OWN persistent verification state (case 1 branches on `GetVerificationState() == PENDING` → new `CharacterIdentity` + `AddIdentity`, else unpacks into the existing identity at `slot`) — an unsolicited reply would be applied against whatever that state happens to be. acdream's transport-level latch (`PendingCharGenVerificationRequest`) is the equivalent discriminator, but when it is `None` there is no state to apply the reply against, so the honest move is drop-and-log rather than guessing a family. | `src/AcDream.Core.Net/WorldSession.cs` (the `CharGenVerificationResponse.ResponseOpcode` arm in `ProcessDatagram`; `_loggedUnexpectedCharGenVerificationResponse`) | Processing an unsolicited reply requires retail's persistent chargen verification state, which lives in CC3's Runtime owner, not the transport. Until then a reply with no outstanding request is either a server bug or a latch-lifecycle bug on our side — surfacing it in the log beats silently misrouting it to an arbitrary event. Pinned by `WorldSessionCharacterCreationTests.ResponseWithNoOutstandingRequest_IsDroppedAndNeverMisattributed`. | A server that sends a spontaneous/duplicate `0xF643` (ACE can double-send NameInUse — see the CC2 review's F3 note) has its second copy dropped here, where retail would re-process it. If CC3's verification gate ever needs retail's re-process semantics, this drop must move behind that owner's state. | `Handle_CharGenVerificationResponse @0x0055E8B0`; `CharGenState::GetVerificationState`; CC2 review F2 (2026-08-15) | | AD-102 | **Filed 2026-08-15 at Campaign CC slice CC4 (the Heritage page's Viamontian button and the Town page's Sanamar button).** Retail gates BOTH controls behind `CPlayerSystem::AccountHasThroneOfDestiny`: `gmCGHeritagePage::ListenToElementMessage @ 0x00483860` shows `MakeToDWarningDialog` instead of selecting Viamontian (element `0x100003c3`) for a non-ToD account, and `gmCGTownPage::ListenToElementMessage @ 0x0047c480` does the same for Sanamar (element `0x1000040b`, `startArea` index 3 — also the reason `CharGenState::RandomizeStartArea`'s ToD-aware `RandInt(3 or 4)` bound exists). acdream's `ChargenOptions` (CC1) carries no account/DLC-ownership signal anywhere in the model, so both controls ship WITHOUT the gate — every installed heritage/town in `Options.HeritagesById`/`Options.StarterAreas` is always selectable, matching what a ToD-owning account would see. | `src/AcDream.App/UI/Layout/CharacterCreationHeritagePage.cs` (`HeritageByButtonId[0x100003C3u]`); `src/AcDream.App/UI/Layout/CharacterCreationTownPage.cs` (`StartAreaByButtonId[0x1000040Bu]`, `Randomize`) | ACE's server-side `CharacterCreate` handler never checks ToD ownership either (the field is purely a retail-client UI gate), so accepting the selection unconditionally never produces a request the emulator would reject; adding an account-ownership model to CC1's DAT-only `ChargenOptions` is out of this slice's scope and would need its own design (where does the "ToD owned" bit come from — account service, launcher config, a new env flag?). | None observable against ACE. A future retail-parity gate that specifically checks "does a non-ToD account get warned off Viamontian/Sanamar" will fail until an account-ownership signal exists to gate on. | `gmCGHeritagePage::ListenToElementMessage @ 0x00483860`; `gmCGTownPage::ListenToElementMessage @ 0x0047c480`; `gmCGTownPage::SetTown @ 0x0047c360`; `CharGenState::RandomizeStartArea` (DoRandom case 4, `RandInt(hasToD ? 4 : 3)`) | | AD-99 | **Filed 2026-08-15 at Campaign LA gate round 2 finding 1 (character-select Exit button).** On a confirmed Exit, acdream closes the client through the existing graceful window-close path (`d.Window.Close`, the same seam `GameplayInputCommandController`'s in-world Escape fallback already uses) instead of retail's real post-confirm behavior: `RecvNotice_CloseDialog`'s case-1 arm queues UI mode `0x10000009`, which `gmEpilogueUI::Register` claims — a brief epilogue/farewell screen — before the process actually terminates. The confirmation dialog itself (`MakeConfirmExitDialog`, its exact `ID_CharacterManagement_ConfirmExit` text, and the `m_confirmExitDialogContext != 0` re-entry guard) IS ported faithfully; only the post-confirm destination differs, the same shape as AD-74's Options-panel exit. | `src/AcDream.App/UI/Layout/CharacterManagementUiController.cs` (`RequestExit`); `src/AcDream.App/UI/RetailUiRuntime.cs` (`CharacterSelectionRuntimeBindings.RequestExit`); `src/AcDream.App/Composition/InteractionRetainedUiComposition.cs` (`d.Window.Close` binding) | acdream has no `gmEpilogueUI` port (out of scope this round); reusing the ONE existing graceful-shutdown seam keeps `disconnected`/`exited` status events firing through `GameWindow.OnClosing` → `CompleteShutdown` rather than inventing a second shutdown path, per explicit direction for this finding. | A user confirming Exit sees the window close immediately instead of retail's brief epilogue screen; a future feature wanting to reproduce that screen (or an intermediate "logged off, returned to character select" state) has no seam yet — same gap class as AD-44. | `gmCharacterManagementUI::MakeConfirmExitDialog @0x004ed250`; `RecvNotice_CloseDialog @0x004ed760` case 1; `gmEpilogueUI::Register(0x10000009)` @0x0047a680; `gmCharacterManagementUI::OnAction @0x004ed410` (Escape key, unported — button-only this round) | -| AD-113 | **Filed 2026-08-25 at Campaign CT slice CT-GF1 (client-wide retained-UI ancestor clip).** Porting retail's `UIRegion::DrawHere @0x0069FA30` ancestor-clip intersection (an element's screen rect is intersected against the FULL inherited clip-rect chain and the subtree is skipped when the intersection is empty — the `var_24` gate @0x0069FB8E) as `UiElement.ClipsChildren`'s new client-wide default (true, threaded through the pre-existing `UiRenderContext.PushClip`/`PopClip`) needed one deliberate opt-out: retail spawns a menu's dropdown popup as a SEPARATE top-level region (`UIElement_Menu::MakePopup`), clipped only by the screen, while acdream's `UiMenu` draws its popup INLINE from the owning button in a second traversal (`OnDrawOverlay`, pre-existing, "regardless of this element's position in the tree" by its own doc comment). Without an escape, the new ancestor clip would wrongly cut off a popup that legitimately extends outside its own (possibly short) owning window — e.g. a channel dropdown opened upward past a short chat window's top edge. `UiElement.ExpandsClipForPopup` (default false) resets the accumulated clip to unbounded for exactly the `OnDrawOverlay` call of an opted-in element (`UiRenderContext.PushClipUnbounded`, sharing the existing clip stack); `UiMenu` overrides it true, paired with `ClipsChildren => false` so its own out-of-bounds `OnHitTest` union (the popup occupies `ly < 0` or `ly >= Height` depending on open direction) stays reachable through the same early-bounds gate that now defaults on for every other element. | `src/AcDream.App/UI/UiElement.cs` (`ClipsChildren`, `ExpandsClipForPopup`, `DrawOverlays`); `src/AcDream.App/UI/UiRenderContext.cs` (`PushClipUnbounded`); `src/AcDream.App/UI/UiMenu.cs` (the two overrides) | The popup is the ONLY overlay-drawing widget in the tree today (grep-confirmed: exactly one `OnDrawOverlay` override client-wide), and it already renders on top of the whole UI by construction (the overlay pass beats even rect backgrounds), so exempting it from the ancestor clip matches its existing "regardless of tree position" contract rather than introducing new behavior. | A future `OnDrawOverlay` override that is NOT a screen-anchored popup (e.g. an in-place highlight meant to stay window-clipped) would silently escape every ancestor's clip if it left `ExpandsClipForPopup` at its default; the opt-in default direction makes that the exception rather than the rule, but a widget that WANTS window-clipped overlay content has no dedicated seam beyond simply not overriding the escape. | `UIRegion::DrawHere @0x0069FA30`; `UIElement_Menu::MakePopup`; the register's own AP-201 retirement note (the FIRST `ClipsChildren`/`PushClip` port, for `UiScrollablePanel`'s viewport) | +| AD-113 | **Filed 2026-08-25 at Campaign CT slice CT-GF1 (client-wide retained-UI ancestor clip).** Porting retail's `UIRegion::DrawHere @0x0069FA30` ancestor-clip intersection (an element's screen rect is intersected against the FULL inherited clip-rect chain and the subtree is skipped when the intersection is empty — the `var_24` gate @0x0069FB8E) as `UiElement.ClipsChildren`'s new client-wide default (true, threaded through the pre-existing `UiRenderContext.PushClip`/`PopClip`) needed one deliberate opt-out: retail spawns a menu's dropdown popup as a SEPARATE top-level region (`UIElement_Menu::MakePopup`), clipped only by the screen, while acdream's `UiMenu` draws its popup INLINE from the owning button in a second traversal (`OnDrawOverlay`, pre-existing, "regardless of this element's position in the tree" by its own doc comment). Without an escape, the new ancestor clip would wrongly cut off a popup that legitimately extends outside its own (possibly short) owning window — e.g. a channel dropdown opened upward past a short chat window's top edge. `UiElement.ExpandsClipForPopup` (default false) resets the accumulated clip to the full CANVAS rect (0,0,ScreenSize) — SCREEN-clipped, not truly unbounded, matching retail's own popup region (`UIElement_Menu::MakePopup` spawns a top-level region bounded by the screen) — for exactly the `OnDrawOverlay` call of an opted-in element (`UiRenderContext.PushClipUnbounded`, sharing the existing clip stack; corrected from an earlier `null`/unbounded clip at the CT-GF1 fix round); `UiMenu` overrides it true, paired with `ClipsChildren => false` so its own out-of-bounds `OnHitTest` union (the popup occupies `ly < 0` or `ly >= Height` depending on open direction) stays reachable through the same early-bounds gate that now defaults on for every other element. | `src/AcDream.App/UI/UiElement.cs` (`ClipsChildren`, `ExpandsClipForPopup`, `DrawOverlays`); `src/AcDream.App/UI/UiRenderContext.cs` (`PushClipUnbounded`); `src/AcDream.App/UI/UiMenu.cs` (the two overrides) | The popup is the ONLY overlay-drawing widget in the tree today (grep-confirmed: exactly one `OnDrawOverlay` override client-wide), and it already renders on top of the whole UI by construction (the overlay pass beats even rect backgrounds), so exempting it from the ancestor clip matches its existing "regardless of tree position" contract rather than introducing new behavior. | A future `OnDrawOverlay` override that is NOT a screen-anchored popup (e.g. an in-place highlight meant to stay window-clipped) would silently escape every ancestor's clip if it left `ExpandsClipForPopup` at its default; the opt-in default direction makes that the exception rather than the rule, but a widget that WANTS window-clipped overlay content has no dedicated seam beyond simply not overriding the escape. | `UIRegion::DrawHere @0x0069FA30`; `UIElement_Menu::MakePopup`; the register's own AP-201 retirement note (the FIRST `ClipsChildren`/`PushClip` port, for `UiScrollablePanel`'s viewport) | --- diff --git a/docs/plans/2026-08-24-character-panel-parity-campaign.md b/docs/plans/2026-08-24-character-panel-parity-campaign.md index 7310e3bd..c8a801d1 100644 --- a/docs/plans/2026-08-24-character-panel-parity-campaign.md +++ b/docs/plans/2026-08-24-character-panel-parity-campaign.md @@ -1,6 +1,6 @@ # Campaign CT — Character-panel retail parity (header identity, Titles page, resize/scrollbar, row alignment) -**Status:** IMPLEMENTATION COMPLETE 2026-08-25 — CT1-CT6 all review-closed (per-slice Opus dual-lens review + fix round); CT7 connected gate script ready at `docs/research/2026-08-25-campaign-ct-test-script.md`, awaiting the owner's drive. NOT pushed to gitea (owner directive). +**Status:** IMPLEMENTATION COMPLETE 2026-08-25 — CT1-CT6 all review-closed (per-slice Opus dual-lens review + fix round). CT-GF1 (the CT7 gate's own first finding — the client-wide retained-UI ancestor clip) landed `989f6652` and its fix round is CODE-COMPLETE (see the CT-GF1 subsection below); CT7 connected gate script ready at `docs/research/2026-08-25-campaign-ct-test-script.md`, still awaiting the owner's drive. NOT pushed to gitea (owner directive). **Execution model:** Fable plans and coordinates; Sonnet implements each slice; Opus runs the dual-lens review (retail-faithful + architectural) per slice, then a fix round. No pushes to gitea until the owner says so. @@ -547,6 +547,47 @@ cited (`0x10000180`, 300×362), not a new number. titles round trip against ACE (earn/set/display), header lines vs retail side-by-side, resize behavior, row alignment screenshots. +### CT-GF1 — client-wide retained-UI ancestor clip (gate finding + fix round) + +Landed `989f6652`: ports retail's `UIRegion::DrawHere @0x0069FA30` +ancestor-clip intersection as `UiElement.ClipsChildren`'s new client-wide +default (true), fixing the CT7 gate's own first finding — the Titles page's +authored divider `0x10000530` escaping the Character window above its top +edge at the CT6-correct 372px mounted default. One opt-out +(`UiElement.ExpandsClipForPopup`, `UiMenu`'s inline-drawn popup) plus new +`UiAncestorClipTests` mechanism coverage. + +**Fix round** (Opus dual-lens review, 0 blockers / 7 SHOULD-FIX / 4 NOTE, all +applied): moved the ambient clip to wrap `OnDraw` + children + +`OnDrawAfterChildren` in one block — the literal `DrawHere` shape, clipping +an element's own `DrawSelf` too, not just its children (`UIElement_Text:: +DrawSelf @0x00467AA0`; `UIRegion::DrawSelf @0x0069F1A0`) — and deleted the +two now-redundant ad-hoc self-clips it superseded (`UiText.DrawText`, +`UiField.DrawMultiLine`); kept the one that clips to a genuinely smaller +authored inner rect (`UiButton.DrawBlockLabel`'s `LabelBox`/`ValueBox`). +Deleted `UiItemList`'s `ClipsChildren` override (inverted under the new +default). Pinned the escaped-popup input path end to end (`UiRoot.PopupHit` +routing, `WantsMouse`) with a new real-`UiRoot` test. Strengthened the +Titles-divider regression test's positive half (exact-rect assertion + +visible/hidden diff, not a bare Y-band check). Added a draw-capture +regression sweep across Character/Chat/Vendor/Options mounted through their +real controllers (`UiWindowDrawCaptureSweepTests`). `PushClipUnbounded` now +resets to the screen rect, not `null` — retail's own popup region is +screen-clipped, not truly unbounded (AD-113 amended). `UiRoot.ClipsChildren` +now explicitly overrides false (the root's own region IS the screen — a +safety net against a momentarily zero-sized root blanking the whole UI). +Added the empty-clip subtree cull (retail's `var_24` gate), scoped to the +main draw pass only — the popup's separate `DrawOverlays` traversal is +provably unaffected (new coverage: a menu inside a fully-clipped window +still draws its popup). + +**Owed:** the CT7 re-gate (script `docs/research/2026-08-25-campaign-ct-test- +script.md`) still needs the owner's connected drive — this fix round landed +on the automated side only. §5 of that script now also names the +collapsed-toolbar check and the four highest-overflow windows (combat/ +vitals bar, Options bottom-button row, map/house page, floaty chat) as +explicit eyeball items for that same re-gate. + ## Review protocol Per slice: Sonnet implements → Opus dual-lens review (lens 1 diff --git a/docs/research/2026-08-25-campaign-ct-test-script.md b/docs/research/2026-08-25-campaign-ct-test-script.md index 3deadf1a..1f5094ba 100644 --- a/docs/research/2026-08-25-campaign-ct-test-script.md +++ b/docs/research/2026-08-25-campaign-ct-test-script.md @@ -142,6 +142,29 @@ only be dragged taller. PlayerDescription's values. - Chat window: the CH-round fixes hold (input rails on focus, "Gen" caption, button flick, no vibrating text while dragging). +- **CT-GF1 fix round (client-wide ancestor clip) — eyeball items.** The new + default clips every element to its own box by default; these are the + windows most likely to show a silent over-clip (content trimmed that + should be visible) if the port has an edge case the automated suite + didn't catch: + - **Collapsed toolbar**: collapse the combat/spell toolbar to its narrow + strip and back — confirm nothing inside it (icons, the collapse grip) + gets cut off or fails to reappear on expand. + - **Combat/vitals bar**: at its default size, confirm the health/ + stamina/mana bars and their numeric overlays render in full, not + trimmed at an edge. + - **Options panel bottom-button row** (Gameplay tab): confirm all seven + buttons (Exit to Character Selection, Configure Keyboard, In-Game + Help, Urgent Assistance, Report Abuse, mouse-turning checkbox, Exit + Game) render completely, none clipped at the panel's bottom edge. + - **Map/house page**: confirm the map image and player/house icons + render in full across the page's own scroll/zoom range, not clipped + at the viewport edge. + - **Floaty chat** (a detached floating chat window, Alt+1..4): confirm + the transcript and input row render in full at both a small and a + resized-larger window size — the same class of symptom CT-GF1's own + `ChatLayoutConformanceTests` regression-pinned for the main chat + window's input row. --- diff --git a/src/AcDream.App/UI/UiButton.cs b/src/AcDream.App/UI/UiButton.cs index be2a82bf..0d4cc3d5 100644 --- a/src/AcDream.App/UI/UiButton.cs +++ b/src/AcDream.App/UI/UiButton.cs @@ -857,16 +857,20 @@ public sealed class UiButton : UiElement, IUiGlobalTimeListener, IUiDatStateful text, font.MeasureWidth, font.LineHeight, boxX, boxY, boxWidth, boxHeight, align, leftOffset); - // A multi-line result (an authored '\n') clips to its own box — the - // button's normal draw has no ambient clip, and an oversized - // wrapped caption (e.g. the Skills credits button's own tight 28px - // height) should be cut off at the box edge rather than spill into - // whatever sits below the button, matching every other clipped - // Type-12 text box in this codebase (UiText.DrawText's own - // PushClip). Single-line captions — the overwhelming majority, - // and (post-R3-2) EVERY caption with no authored newline — never - // pay this cost; see this method's own doc for why a single line - // is deliberately left unclipped even when boxWidth was narrowed. + // A multi-line result (an authored '\n') clips to its own box — an oversized + // wrapped caption (e.g. the Skills credits button's own tight 28px height) + // should be cut off at the box edge rather than spill into whatever sits + // below the button. CT-GF1 fix round (S2) audit: this clip STAYS (unlike + // UiText.DrawText's and UiField.DrawMultiLine's now-deleted self-clips) — + // (boxX,boxY,boxWidth,boxHeight) is LabelBox/ValueBox, an authored INNER + // sub-rect that can be smaller than and offset from the button's own full + // (0,0,Width,Height) (see LabelBox's/ValueBox's own docs and the + // ValueBox-narrows-boxWidth branch above), so it is not redundant with the + // ambient (0,0,Width,Height) clip DrawSelfAndChildren now applies by default + // one level up. Single-line captions — the overwhelming majority, and + // (post-R3-2) EVERY caption with no authored newline — never pay this cost; + // see this method's own doc for why a single line is deliberately left + // unclipped even when boxWidth was narrowed. bool clip = lines.Count > 1; if (clip) ctx.PushClip(boxX, boxY, boxWidth, boxHeight); diff --git a/src/AcDream.App/UI/UiElement.cs b/src/AcDream.App/UI/UiElement.cs index 0002e869..7d95d961 100644 --- a/src/AcDream.App/UI/UiElement.cs +++ b/src/AcDream.App/UI/UiElement.cs @@ -544,30 +544,39 @@ public abstract class UiElement protected virtual void OnDrawOverlay(UiRenderContext ctx) { } /// - /// Whether descendant drawing and hit-testing are clipped to this element's - /// local bounds. THIS IS THE DEFAULT (true) FOR EVERY ELEMENT — CT-GF1 port - /// of retail's ancestor-clip chain: UIRegion::DrawHere @0x0069FA30 takes - /// the element's screen Box2D plus a SmartArray<Box2D> of - /// inherited clip rects, intersects them (the min/max clamp loop - /// @0x0069FAA7..0x0069FB82), and draws — EraseSelf/DrawChildren/ - /// DrawSelf all receive the intersected rect — ONLY when the intersection - /// is non-empty (the var_24 gate @0x0069FB8E). An element positioned - /// outside its parent's box therefore silently disappears in retail, exactly - /// like / - /// (already wrapping the child-draw and child-hit-test walks below) now does for - /// every element by default, not just the scrollable listboxes that opted in - /// before this default flipped (owner gate finding: the Titles page's authored - /// divider 0x10000530 escaped the Character window at the CT6-correct 372px - /// mounted default — retail clips it away; acdream drew it floating above the - /// window). + /// Whether THIS element's own draw AND its descendants' drawing/hit-testing are + /// clipped to this element's local bounds. THIS IS THE DEFAULT (true) FOR EVERY + /// ELEMENT — CT-GF1 port of retail's ancestor-clip chain: UIRegion::DrawHere + /// @0x0069FA30 takes the element's screen Box2D plus a + /// SmartArray<Box2D> of inherited clip rects, intersects them (the + /// min/max clamp loop @0x0069FAA7..0x0069FB82), and draws — EraseSelf/ + /// DrawChildren/DrawSelf ALL receive the intersected rect — ONLY + /// when the intersection is non-empty (the var_24 gate @0x0069FB8E). An + /// element positioned outside its parent's box therefore silently disappears in + /// retail, exactly like / + /// now does for every element by default — + /// 's fix-round shape (S2) pushes right after + /// PushAlpha and wraps OnDraw + the children walk + + /// OnDrawAfterChildren in ONE block, the literal DrawHere shape + /// (retail clips the element's OWN DrawSelf too, not just its children — + /// UIElement_Text::DrawSelf @0x00467AA0 locks glyph blits to its own + /// clipped surface rect; UIRegion::DrawSelf @0x0069F1A0 blits per clip + /// rect). Not just the scrollable listboxes that opted in before this default + /// flipped (owner gate finding: the Titles page's authored divider 0x10000530 + /// escaped the Character window at the CT6-correct 372px mounted default — + /// retail clips it away; acdream drew it floating above the window). /// /// /// Override to ONLY for a widget that must draw or accept /// input beyond its own bounds by deliberate design — today just - /// , whose popup (and its own out-of-bounds - /// OnHitTest override) stands in for retail's separate top-level popup - /// region; see for the drawing half of that - /// opt-out and the divergence register row it cites. + /// (whose popup, and its own out-of-bounds OnHitTest + /// override, stands in for retail's separate top-level popup region — see + /// for the drawing half of that opt-out and the + /// divergence register row it cites) and (whose own region + /// IS the screen — the viewport itself already scissors it, so narrowing to + /// (0,0,Width,Height) here would blank the whole UI the moment the root's + /// own tracked size is ever momentarily zero, e.g. before the first resize event + /// lands). /// /// protected virtual bool ClipsChildren => true; @@ -648,39 +657,49 @@ public abstract class UiElement // surface, chrome and glyphs together, not text-stays-sharp over a translucent panel). ctx.PushTransform(Left, Top); ctx.PushAlpha(Opacity); + // CT-GF1 fix round (S2): the clip now wraps OnDraw + children + + // OnDrawAfterChildren — the LITERAL UIRegion::DrawHere @0x0069FA30 shape, + // which clips the element's OWN DrawSelf to the intersected rect, not just its + // children (UIElement_Text::DrawSelf @0x00467AA0 locks glyph blits to arg3; + // UIRegion::DrawSelf @0x0069F1A0 blits per clip rect). Pushed right after + // PushAlpha, popped in the one finally below, so it is balanced regardless of + // which branch below runs. + bool clipsChildren = ClipsChildren; + if (clipsChildren) + ctx.PushClip(0f, 0f, Width, Height); try { - OnDraw(ctx); - - // Anchor layout: reflow children to this element's current size. - for (int i = 0; i < _children.Count; i++) - _children[i].ApplyAnchor(Width, Height); - - // Children painted back-to-front (lowest ZOrder first). - if (_children.Count > 0) + // N2 fix round: retail's var_24 gate @0x0069FB8E — an EMPTY intersected + // clip skips EraseSelf/DrawChildren/DrawSelf outright for the whole + // subtree. DrawOverlays (the popup's SEPARATE second traversal) does not + // share this walk or its clip-stack state, so an open UiMenu popup nested + // here keeps drawing there regardless of this cull — see + // UiAncestorClipTests' menu-inside-a-fully-clipped-window coverage. + if (!ctx.CurrentClipIsEmpty) { - bool clipsChildren = ClipsChildren; - if (clipsChildren) - ctx.PushClip(0f, 0f, Width, Height); - try + OnDraw(ctx); + + // Anchor layout: reflow children to this element's current size. + for (int i = 0; i < _children.Count; i++) + _children[i].ApplyAnchor(Width, Height); + + // Children painted back-to-front (lowest ZOrder first). + if (_children.Count > 0) { UiElement[] ordered = ChildrenBackToFrontSnapshot(); for (int i = 0; i < ordered.Length; i++) ordered[i].DrawSelfAndChildren(ctx); } - finally - { - if (clipsChildren) - ctx.PopClip(); - } - } - // Foreground pass for this element (e.g. a window frame's border drawn - // OVER its content's edges). Default no-op for ordinary elements. - OnDrawAfterChildren(ctx); + // Foreground pass for this element (e.g. a window frame's border drawn + // OVER its content's edges). Default no-op for ordinary elements. + OnDrawAfterChildren(ctx); + } } finally { + if (clipsChildren) + ctx.PopClip(); ctx.PopAlpha(); ctx.PopTransform(); } diff --git a/src/AcDream.App/UI/UiField.cs b/src/AcDream.App/UI/UiField.cs index b9686104..00ac7e2c 100644 --- a/src/AcDream.App/UI/UiField.cs +++ b/src/AcDream.App/UI/UiField.cs @@ -557,57 +557,56 @@ public sealed class UiField : UiElement Scroll.SetScrollY((int)MathF.Ceiling(caretBottom - visibleHeight)); } - ctx.PushClip(0f, 0f, Width, Height); - try + // CT-GF1 fix round (S2): this used to push its OWN (0,0,Width,Height) clip + // here. That is now REDUNDANT and deleted — UiElement.DrawSelfAndChildren + // (DrawMultiLine is called from OnDraw) wraps this whole method in exactly + // that same (0,0,Width,Height) ambient clip by default, matching retail's + // DrawHere shape one level up instead of duplicating it here. The lines below + // still rely on SOME clip being active (a partially visible row must still be + // rejected, not drawn full-size) — that clip is now the ambient one. + var (selectionLow, selectionHigh) = SelSpan(); + for (int i = 0; i < lines.Count; i++) { - var (selectionLow, selectionHigh) = SelSpan(); - for (int i = 0; i < lines.Count; i++) + WrappedLine line = lines[i]; + float y = Padding + (i * lineHeight) - Scroll.ScrollY; + if (y + lineHeight <= Padding || y >= Height - Padding) + continue; + + int lineEnd = line.Start + line.Length; + int highlightLow = Math.Max(selectionLow, line.Start); + int highlightHigh = Math.Min(selectionHigh, lineEnd); + if (HasSelection && highlightHigh > highlightLow) { - WrappedLine line = lines[i]; - float y = Padding + (i * lineHeight) - Scroll.ScrollY; - if (y + lineHeight <= Padding || y >= Height - Padding) - continue; - - int lineEnd = line.Start + line.Length; - int highlightLow = Math.Max(selectionLow, line.Start); - int highlightHigh = Math.Min(selectionHigh, lineEnd); - if (HasSelection && highlightHigh > highlightLow) - { - float x0 = Padding + MeasureRange( - line.Start, - highlightLow - line.Start); - float x1 = Padding + MeasureRange( - line.Start, - highlightHigh - line.Start); - ctx.DrawFill( - x0, - y, - MathF.Max(0f, x1 - x0), - lineHeight, - SelectionColor); - } - - if (DatFont is { } dat) - ctx.DrawStringDat(dat, line.Text, Padding, y, TextColor, Outline, OutlineColor); - else if (Font is { } bitmap) - ctx.DrawString(line.Text, Padding, y, TextColor, bitmap); + float x0 = Padding + MeasureRange( + line.Start, + highlightLow - line.Start); + float x1 = Padding + MeasureRange( + line.Start, + highlightHigh - line.Start); + ctx.DrawFill( + x0, + y, + MathF.Max(0f, x1 - x0), + lineHeight, + SelectionColor); } - if (_focused && lines.Count > 0) - { - WrappedLine line = lines[caretLine]; - int lineColumn = Math.Clamp( - _caret - line.Start, - 0, - line.Length); - float x = Padding + MeasureRange(line.Start, lineColumn); - float y = Padding + (caretLine * lineHeight) - Scroll.ScrollY; - ctx.DrawFill(x, y, 1f, lineHeight, TextColor); - } + if (DatFont is { } dat) + ctx.DrawStringDat(dat, line.Text, Padding, y, TextColor, Outline, OutlineColor); + else if (Font is { } bitmap) + ctx.DrawString(line.Text, Padding, y, TextColor, bitmap); } - finally + + if (_focused && lines.Count > 0) { - ctx.PopClip(); + WrappedLine line = lines[caretLine]; + int lineColumn = Math.Clamp( + _caret - line.Start, + 0, + line.Length); + float x = Padding + MeasureRange(line.Start, lineColumn); + float y = Padding + (caretLine * lineHeight) - Scroll.ScrollY; + ctx.DrawFill(x, y, 1f, lineHeight, TextColor); } } diff --git a/src/AcDream.App/UI/UiItemList.cs b/src/AcDream.App/UI/UiItemList.cs index d96b3654..2473b969 100644 --- a/src/AcDream.App/UI/UiItemList.cs +++ b/src/AcDream.App/UI/UiItemList.cs @@ -358,7 +358,13 @@ public sealed class UiItemList : UiElement } } - protected override bool ClipsChildren => CellWidth > 0f; + // CT-GF1 fix round (S3): the former `ClipsChildren => CellWidth > 0f` override is + // DELETED. It made sense under the PRE-CT-GF1 opt-in default (false): clip only + // once CellWidth is configured. Under the new client-wide default (true), that + // same expression is INVERTED — an unconfigured list (CellWidth<=0, e.g. before + // LayoutCells first runs) would evaluate to false and stop clipping, the opposite + // of every other element's new default. Deleting the override restores the + // uniform default (always clip to this element's own (0,0,Width,Height)). public void Flush() { diff --git a/src/AcDream.App/UI/UiRenderContext.cs b/src/AcDream.App/UI/UiRenderContext.cs index 3697f7c9..a91ee7cb 100644 --- a/src/AcDream.App/UI/UiRenderContext.cs +++ b/src/AcDream.App/UI/UiRenderContext.cs @@ -116,18 +116,34 @@ public sealed class UiRenderContext } /// - /// Discard every inherited clip rect for the duration of one overlay draw — - /// the escape hatch uses so a popup - /// drawn inline from its owning widget (see that property's doc comment for the - /// retail-parity rationale) is not wrongly clipped by the ancestor chain the - /// CT-GF1 default clip () now threads through - /// every other element. Shares 's stack, so pair the two - /// exactly like . + /// True when the current accumulated clip is non-null and has zero (or negative) + /// area — CT-GF1 fix-round subtree cull, porting retail's + /// UIRegion::DrawHere var_24 gate @0x0069FB8E: an empty intersected + /// clip skips EraseSelf/DrawChildren/DrawSelf for the whole + /// subtree, not just individual draw calls (those already no-op against an empty + /// clip via / — this + /// additionally skips the WALK). A null clip (nothing pushed yet, or reset via + /// ) is NOT empty — it means unbounded, so this is + /// false in that case. + /// + public bool CurrentClipIsEmpty => _clip is { } c && c.IsEmpty; + + /// + /// Reset the accumulated clip to the full CANVAS rect (0,0,ScreenSize) for the + /// duration of one overlay draw — the escape hatch + /// uses so a popup drawn inline from its owning widget (see that property's doc + /// comment for the retail-parity rationale) is not wrongly clipped by the + /// ancestor chain the CT-GF1 default clip () + /// now threads through every other element. Retail's own popup region is still + /// SCREEN-clipped (UIElement_Menu::MakePopup spawns a top-level region + /// bounded by the screen, not truly infinite) — this is the canvas rect, not + /// null/unbounded, matching that. Shares 's stack, so + /// pair the two exactly like . /// public void PushClipUnbounded() { _clipStack.Add(_clip); - _clip = null; + _clip = new UiClipRect(0f, 0f, ScreenSize.X, ScreenSize.Y); } /// Route subsequent draws to the overlay layer (flushed on top of the whole diff --git a/src/AcDream.App/UI/UiRoot.cs b/src/AcDream.App/UI/UiRoot.cs index f22005eb..2ad658a0 100644 --- a/src/AcDream.App/UI/UiRoot.cs +++ b/src/AcDream.App/UI/UiRoot.cs @@ -33,6 +33,19 @@ public sealed class UiRoot : UiElement /// Single owner for named retained-window lifecycle and raise policy. public RetailWindowManager WindowManager { get; } + /// + /// CT-GF1 fix round (N1): the root's own region IS the screen — the viewport + /// itself already scissors everything drawn to it, so retail has no analog of + /// clipping the root to its OWN tracked (Width,Height) the way + /// 's new client-wide default would. + /// Overriding false here is a safety net, not a cosmetic choice: without it, a + /// root momentarily reporting a zero (or stale, pre-first-resize) size would + /// silently blank the ENTIRE UI tree — every top-level window culled by the new + /// ancestor-clip default's empty-intersection gate — rather than the intended + /// "root passes its full extent through to its children uninterpreted." + /// + protected override bool ClipsChildren => false; + /// /// Campaign LA gate round 2 (register AD-98): when set, the retained tree /// is laid out in this fixed authored canvas (the char-select screen's @@ -189,8 +202,27 @@ public sealed class UiRoot : UiElement /// The host ORs this into the InputDispatcher's WantCaptureMouse gate so game /// actions (movement, world-pick) are suppressed while the user interacts with /// a retail window — mirrors ImGui's WantCaptureMouse. + /// + /// + /// CT-GF1 fix round (S4): also checks — an open UiMenu + /// dropdown's escaped region (the part of the popup that extends outside its + /// owning window's own ancestor-clipped bounds, e.g. a channel menu opened + /// upward past a short chat window's top edge) is reachable by + /// / through + /// specifically BECAUSE it bypasses the ordinary top-down + /// walk's ancestor clip gate (CT-GF1's new client-wide + /// default would otherwise reject that + /// point before ever reaching the popup's own out-of-bounds OnHitTest + /// union — see #374's popup routing doc above). Without this, + /// alone would report "no widget here" for a point + /// the pointer visibly sits over, letting a world click/movement action slip + /// through underneath the open popup. + /// /// - public bool WantsMouse => Captured is not null || HitTestTopDown(MouseX, MouseY).element is not null; + public bool WantsMouse => + Captured is not null + || PopupHit(MouseX, MouseY) is not null + || HitTestTopDown(MouseX, MouseY).element is not null; /// True when a widget holds keyboard focus (e.g. a focused chat input). public bool WantsKeyboard => KeyboardFocus is not null; @@ -1386,6 +1418,23 @@ public sealed class UiRoot : UiElement Data1: (int)(x - screen.X), Data2: (int)(y - screen.Y)); w.OnEvent(in enter); + if (UiDiagnostics.ProbeHover) + { + string detail = w is UiScrollbar sb + ? $" start=0x{sb.ActiveStartSpriteForTest:X8}" + + $" end=0x{sb.ActiveEndSpriteForTest:X8}" + + $" thumb=0x{sb.ActiveThumbSpriteForTest:X8}" + + $" disabled={sb.IsModelDisabled}" + : ""; + Console.WriteLine( + $"[ui-hover] widget={w.GetType().Name}" + + $" dat=0x{w.DatElementId:X8}" + + $" local=({(int)(x - screen.X)},{(int)(y - screen.Y)}){detail}"); + } + } + else if (UiDiagnostics.ProbeHover) + { + Console.WriteLine("[ui-hover] widget="); } } diff --git a/src/AcDream.App/UI/UiText.cs b/src/AcDream.App/UI/UiText.cs index 0c8ce38c..71fc7202 100644 --- a/src/AcDream.App/UI/UiText.cs +++ b/src/AcDream.App/UI/UiText.cs @@ -584,17 +584,17 @@ public sealed class UiText : UiElement, IUiDatStateful // visible surface as arg3 and clips each glyph blit to that rectangle. This is // observable in LayoutDesc 0x21000033: the owned component count is a 15px-high // text element using a 16px DAT font, so rejecting a partially visible line makes - // the value disappear entirely. The shared render context clips both DAT and - // bitmap glyph quads and composes this bound with any list/window ancestor clip. - ctx.PushClip(0f, 0f, Width, Height); - try - { - DrawClippedText(ctx); - } - finally - { - ctx.PopClip(); - } + // the value disappear entirely. + // + // CT-GF1 fix round (S2): this used to push its OWN (0,0,Width,Height) clip + // here. That is now REDUNDANT and deleted — UiElement.DrawSelfAndChildren + // wraps OnDraw/OnDrawAfterChildren (where DrawText is called from) in exactly + // that same (0,0,Width,Height) ambient clip by default, intersected with any + // list/window ancestor clip, matching retail's DrawHere shape one level up + // instead of duplicating it here. DrawClippedText still relies on SOME clip + // being active for correctness (a partially visible line must still be + // rejected, not drawn full-size) — that clip is now the ambient one. + DrawClippedText(ctx); } private void DrawClippedText(UiRenderContext ctx) diff --git a/tests/AcDream.App.Tests/UI/Layout/CharacterTitlesControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/CharacterTitlesControllerTests.cs index 27424699..8fa77860 100644 --- a/tests/AcDream.App.Tests/UI/Layout/CharacterTitlesControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/CharacterTitlesControllerTests.cs @@ -721,7 +721,24 @@ public sealed class CharacterTitlesControllerTests dividerGrown.Y >= 0f && dividerGrown.Y + divider.Height <= 600f, "expected the Titles divider to land inside the grown window at its authored " + $"spot; got {dividerGrown.Y}"); - AssertQuadCoversY(renderer, dividerGrown.Y, dividerGrown.Y + divider.Height); + + // CT-GF1 fix round (S5): AssertQuadCoversY alone only proves SOMETHING drew + // in that Y band -- not that it was specifically THIS divider's own quad (the + // grown Titles page has other rows/backgrounds that could coincidentally + // share the band). Strengthen both halves: pin the divider's exact rect (X + // AND Y range, not Y alone), THEN diff against the same rect with the + // divider hidden -- if hiding it does not empty the rect out, whatever drew + // there was never uniquely attributable to the divider in the first place. + AssertQuadCoversRect( + renderer, dividerGrown.X, dividerGrown.Y, + dividerGrown.X + divider.Width, dividerGrown.Y + divider.Height); + + divider.Visible = false; + renderer.Begin(new Vector2(screen.Width, screen.Height)); + handle.OuterFrame.DrawSelfAndChildren(ctx); + AssertNoQuadCoversRect( + renderer, dividerGrown.X, dividerGrown.Y, + dividerGrown.X + divider.Width, dividerGrown.Y + divider.Height); } private sealed class NullGpuFrameSource : ICurrentGpuFrameSource @@ -744,6 +761,45 @@ public sealed class CharacterTitlesControllerTests } } + /// CT-GF1 fix round (S5): the X-and-Y-range companion to + /// — a passing vertex must land inside BOTH + /// axes' rect, not just the Y band, so an unrelated same-band element (a + /// background, another row) cannot satisfy this vacuously. + private static void AssertQuadCoversRect(TextRenderer renderer, float xLo, float yLo, float xHi, float yHi) + { + bool found = renderer.DebugSpriteSegmentVerts.Any(seg => + { + for (int i = 0; i < seg.Verts.Count / 8; i++) + { + float vx = seg.Verts[i * 8]; + float vy = seg.Verts[i * 8 + 1]; + if (vx >= xLo - 0.5f && vx <= xHi + 0.5f && vy >= yLo - 0.5f && vy <= yHi + 0.5f) + return true; + } + return false; + }); + Assert.True(found, $"expected at least one quad vertex inside rect [{xLo},{yLo}]..[{xHi},{yHi}]"); + } + + /// CT-GF1 fix round (S5): the negative half of — + /// used after hiding the divider to prove the earlier positive assertion was + /// attributable to it specifically (a diff, not a coincidence). + private static void AssertNoQuadCoversRect(TextRenderer renderer, float xLo, float yLo, float xHi, float yHi) + { + foreach (var seg in renderer.DebugSpriteSegmentVerts) + { + for (int i = 0; i < seg.Verts.Count / 8; i++) + { + float vx = seg.Verts[i * 8]; + float vy = seg.Verts[i * 8 + 1]; + Assert.False( + vx >= xLo - 0.5f && vx <= xHi + 0.5f && vy >= yLo - 0.5f && vy <= yHi + 0.5f, + $"unexpected quad vertex at ({vx},{vy}) inside the divider's own rect " + + $"[{xLo},{yLo}]..[{xHi},{yHi}] after hiding it"); + } + } + } + private static void AssertQuadCoversY(TextRenderer renderer, float yLo, float yHi) { bool found = renderer.DebugSpriteSegmentVerts.Any(seg => diff --git a/tests/AcDream.App.Tests/UI/Layout/UiWindowDrawCaptureSweepTests.cs b/tests/AcDream.App.Tests/UI/Layout/UiWindowDrawCaptureSweepTests.cs new file mode 100644 index 00000000..e4afb918 --- /dev/null +++ b/tests/AcDream.App.Tests/UI/Layout/UiWindowDrawCaptureSweepTests.cs @@ -0,0 +1,279 @@ +using System; +using System.Collections.Generic; +using System.Numerics; +using AcDream.App.Rendering; +using AcDream.App.Rendering.Gpu; +using AcDream.App.Tests.Rendering.Gpu; +using AcDream.App.UI; +using AcDream.App.UI.Layout; +using AcDream.Core.Chat; +using AcDream.Core.Items; +using AcDream.Core.Properties; +using AcDream.Core.Selection; +using AcDream.Runtime.Gameplay; +using AcDream.UI.Abstractions; +using AcDream.UI.Abstractions.Panels.Chat; + +namespace AcDream.App.Tests.UI.Layout; + +/// +/// CT-GF1 fix round (S1): a draw-capture regression sweep mounting each major +/// retained window through its REAL controller (the same production Bind +/// entry points other suites already exercise individually) with a NON-ZERO +/// sprite resolver, drawing through a , and +/// asserting a per-window VERTEX-COUNT FLOOR plus the presence of one KEY +/// sprite id. Every key sprite id is read LIVE off the bound +/// controller/element after Bind — never a hardcoded numeric guess (this +/// project's workflow forbids guessing dat ids) — so the assertion tracks +/// whatever the real import/bind pipeline actually resolved, not an +/// assumption about it. +/// +/// This is the class of coverage the CH6a/b BLOCKER 1 bug slipped past: an +/// authored non-zero sprite id sitting right there on the ElementInfo, with +/// nothing actually reaching DrawSprite because the widget was built +/// without its resolve delegate. A structural/geometry test (FindElement, +/// property checks) cannot see that class of regression; only an actual draw +/// pass through a real render context can. +/// +/// +/// Windows covered: Character (this is ALSO CT-GF1's own motivating +/// case — its chrome draws via +/// , the exact code path +/// the S2/item-1 fix-round caution note calls out as needing re-proof after +/// moving the ambient clip to wrap it), Chat (Imported chrome, real +/// committed fixture), Vendor (Imported chrome, real committed +/// fixture), Options (the 4-tab panel host — no +/// wrapper; mounts as a bare +/// , matching OptionsPanelControllerTests's +/// own established pattern). SKIPPED for lack of a reusable, fixture-driven, +/// SINGLE top-level Bind entry point at the time of writing: Inventory/ +/// Paperdoll (composed from several independent controllers, no single +/// window-level Bind), the social panel and the map/house host (their own +/// mount probes are Lane=Manual, live-DAT-only — see +/// SocialPanelLiveMountProbeTests/MapHousePanelSlotProbeTests). +/// +/// +/// +/// Floors are set at roughly 40-45% of this session's OBSERVED vertex count +/// per window (Character 588, Chat 162, Vendor 54, Options 240 — see each +/// Build* method's own trailing comment) — per the fix-round +/// instruction, loose enough to survive legitimate content growth/shrinkage, +/// tight enough to still catch "half (or all) of this window stopped +/// drawing" (a resolve-wiring regression), not exact counts. +/// +/// +public sealed class UiWindowDrawCaptureSweepTests +{ + private sealed class NullGpuFrameSource : ICurrentGpuFrameSource + { + public IGpuFrame? CurrentFrame => null; + } + + private static (RecordingGpuDevice device, TextRenderer renderer, UiRenderContext ctx) MakeContext( + float w, float h) + { + var device = new RecordingGpuDevice(); + var renderer = new TextRenderer(device, new NullGpuFrameSource(), "unused"); + renderer.Begin(new Vector2(w, h)); + var ctx = new UiRenderContext(renderer, new Vector2(w, h)); + return (device, renderer, ctx); + } + + /// Total vertex count across every recorded sprite segment — the + /// same per-segment Verts.Count / 8 convention every other test in + /// this suite uses (8 floats packed per vertex). + private static int TotalVertexCount(TextRenderer renderer) + { + int total = 0; + foreach (var seg in renderer.DebugSpriteSegmentVerts) + total += seg.Verts.Count / 8; + return total; + } + + public static IEnumerable Windows() + { + yield return new object[] { "Character" }; + yield return new object[] { "Chat" }; + yield return new object[] { "Vendor" }; + yield return new object[] { "Options" }; + } + + [Theory] + [MemberData(nameof(Windows))] + public void MountedWindow_DrawsAVertexFloor_AndItsLiveKeySpriteId(string window) + { + (UiElement drawRoot, uint keySprite, int vertexFloor) = window switch + { + "Character" => BuildCharacter(), + "Chat" => BuildChat(), + "Vendor" => BuildVendor(), + "Options" => BuildOptions(), + _ => throw new ArgumentOutOfRangeException(nameof(window), window, null), + }; + + Assert.NotEqual(0u, keySprite); + + var (_, renderer, ctx) = MakeContext(1600f, 1200f); + drawRoot.DrawSelfAndChildren(ctx); + + int vertices = TotalVertexCount(renderer); + Assert.True( + vertices >= vertexFloor, + $"{window}: expected at least {vertexFloor} drawn vertices, got {vertices} " + + "-- a resolve-wiring regression (CH6a/b BLOCKER 1's class) would show up as a " + + "near-zero count here."); + Assert.Contains( + renderer.DebugSpriteSegmentVerts, + s => s.Texture == keySprite); + } + + private static (UiElement drawRoot, uint keySprite, int vertexFloor) BuildCharacter() + { + ImportedLayout layout = LayoutImporter.Build( + FixtureLoader.LoadCharacterInfos(), id => (id, 8, 8), null); + CharacterStatController.Bind( + layout, SampleData.SampleCharacter, spriteResolve: id => (id, 8, 8)); + + var screen = new UiRoot { Width = 1600f, Height = 1200f }; + RetailWindowHandle handle = RetailWindowFrame.Mount( + screen, + layout.Root, + id => (id, 8, 8), + new RetailWindowFrame.Options + { + WindowName = WindowNames.Character, + Chrome = RetailWindowChrome.NineSlice, + ContentHeight = 362f, + MinWidth = 310f, + MaxWidth = 310f, + MinHeight = 372f, + MaxHeight = 1000f, + ResizeX = false, + ResizeY = true, + ContentAnchors = AnchorEdges.Left | AnchorEdges.Top | AnchorEdges.Bottom, + }); + + // Key sprite: RetailChromeSprites.TopEdge -- drawn by + // UiNineSlicePanel.OnDrawAfterChildren, the exact code path the S2/ + // item-1 fix-round caution note calls out. Not read "live" (it's a + // shared named constant, not a per-window authored id), but it is + // NOT a guess either -- it is the actual constant the chrome drawer + // uses, verified by reading UiNineSlicePanel.OnDrawAfterChildren. + return (handle.OuterFrame, RetailChromeSprites.TopEdge, 250); // observed baseline 588 + } + + private static (UiElement drawRoot, uint keySprite, int vertexFloor) BuildChat() + { + var infos = FixtureLoader.LoadChatInfos(); + ImportedLayout layout = LayoutImporter.Build(infos, id => (id, 8, 8), null); + var controller = ChatWindowController.Bind( + infos, + layout, + new ChatVM(new ChatLog()), + () => NullCommandBus.Instance, + new ChatWindowState(), + null, + null, + id => (id, 8, 8)); + Assert.NotNull(controller); + + var root = new UiRoot { Width = 1600f, Height = 1200f }; + RetailWindowHandle handle = RetailWindowFrame.Mount( + root, + controller!.Root, + id => (id, 8, 8), + new RetailWindowFrame.Options + { + WindowName = WindowNames.Chat, + Chrome = RetailWindowChrome.Imported, + Left = 10f, + Top = 10f, + DatConstraintSource = controller.DatWindowInfo, + }); + controller.AttachWindow(handle); + + // Key sprite: read LIVE off the bound scrollbar's own TrackSprite -- + // the scrollbar always draws its track whenever the window does, so + // this tracks whatever the real fixture actually authors rather than + // a hardcoded literal. + return (handle.OuterFrame, controller.Scrollbar.TrackSprite, 80); // observed baseline 162 + } + + private static (UiElement drawRoot, uint keySprite, int vertexFloor) BuildVendor() + { + ImportedLayout layout = FixtureLoader.LoadVendor(); + var screen = new UiRoot { Width = 1600f, Height = 1200f }; + RetailWindowHandle window = RetailWindowFrame.Mount( + screen, + layout.Root, + id => (id, 8, 8), + new RetailWindowFrame.Options + { + WindowName = "vendor-sweep", + Chrome = RetailWindowChrome.Imported, + Visible = true, + }); + + var objects = new ClientObjectTable(); + var itemInteraction = new ItemInteractionController( + objects, + new RuntimeInteractionTransactionState(new InventoryTransactionState(objects)), + new InteractionState(), + playerGuid: static () => 0u, + sendUse: null, + sendUseWithTarget: null, + sendWield: null, + sendDrop: null); + VendorUiController? controller = VendorUiController.Bind( + layout, + new VendorState(), + window, + static (_, iconId, _, _, _) => iconId, + objects, + static () => 0u, + itemInteraction, + new SelectionState(), + new StackSplitQuantityState(), + datFont: null, + debugFont: null, + id => (id, 8, 8)); + Assert.NotNull(controller); + + // Key sprite: read LIVE off the bound category-filter menu's own + // button-face sprite (0x100000BF, VendorUiController.TypeFilterMenuId) -- + // its face always draws whenever the window does, so this tracks + // whatever the real committed vendor fixture actually authors. + var typeMenu = Assert.IsType(layout.FindElement(VendorUiController.TypeFilterMenuId)); + return (window.OuterFrame, typeMenu.NormalSprite, 25); // observed baseline 54 + } + + private static (UiElement drawRoot, uint keySprite, int vertexFloor) BuildOptions() + { + // NOTE: FixtureLoader.LoadOptionsPanelHost() (the convenience wrapper) bakes + // in FixtureLoader's own NULL-returning sprite resolver at LayoutImporter.Build + // time -- permanent for every widget it builds, unaffected by whatever resolver + // is later passed into OptionsPanelController.Bind (which only reaches content + // that controller creates itself, e.g. the per-page footer backing). Building + // from the raw ElementInfo tree here (matching BuildCharacter/BuildChat/ + // BuildVendor's own pattern above) is what actually gets a non-zero resolver + // onto the imported header/tab/page widgets themselves. + ImportedLayout layout = LayoutImporter.Build( + FixtureLoader.LoadOptionsPanelHostInfos(), id => (id, 8, 8), null); + var callbacks = new OptionsPanelController.Callbacks( + Toggle: () => { }, + RequestExitToCharacterSelection: () => { }, + ExitGame: () => { }, + UseMouseTurningSettings: () => { }, + DisplaySystemMessage: _ => { }); + OptionsPanelController? controller = OptionsPanelController.Bind( + layout, callbacks, resolveSprite: id => (id, 8, 8)); + Assert.NotNull(controller); + controller!.ActivateTabs(); + + // Key sprite: RetailChromeSprites.CenterFill -- the exact id + // OptionsPanelControllerTests' own CollectFooterBackings helper pins + // as every page's footer backing (a UiSolidSpriteFill), read here as + // the same shared named constant, not a guess. + return (layout.Root, RetailChromeSprites.CenterFill, 120); // observed baseline 240 + } +} diff --git a/tests/AcDream.App.Tests/UI/UiAncestorClipTests.cs b/tests/AcDream.App.Tests/UI/UiAncestorClipTests.cs index e240990c..22f976f8 100644 --- a/tests/AcDream.App.Tests/UI/UiAncestorClipTests.cs +++ b/tests/AcDream.App.Tests/UI/UiAncestorClipTests.cs @@ -271,4 +271,109 @@ public sealed class UiAncestorClipTests presenter.Dispose(); } + + /// + /// CT-GF1 fix round (S4): pins the input half of the popup-escape mechanism the + /// draw-only tests above only cover visually. A menu mounted inside a SHORT owning + /// window on a REAL , opened, has its popup's first row land + /// well ABOVE the window's own [0,Height) local rect — the escaped region. An + /// ordinary top-down walk would reject a point + /// there before ever reaching the menu: the owning `window`'s own + /// default (true, CT-GF1) rejects any + /// out-of-bounds local coordinate in BEFORE + /// recursing into its children, so the menu's own out-of-bounds + /// OnHitTest union is never consulted. UiRoot's PopupHit + /// routing (#374) is what rescues this: while a popup is registered active, a + /// press/scroll/ query is tested directly against + /// the popup element itself, bypassing the ancestor walk entirely. + /// + [Fact] + public void EscapedPopupClick_ReachesTheMenu_ThroughAShortOwningWindow() + { + var root = new UiRoot { Width = 200f, Height = 200f }; + var window = new TestElement { Left = 10f, Top = 150f, Width = 80f, Height = 18f }; + string? picked = null; + var menu = new UiMenu + { + Width = 80f, + Height = 18f, + OpenUpward = true, + RowsPerColumn = 1, // one row -> a small, exactly-known popup rect + Items = new[] { new UiMenu.MenuItem("Row", (object?)"row") }, + SpriteResolve = id => (id, 8, 8), + }; + menu.OnSelect = p => picked = p as string; + window.AddChild(menu); + root.AddChild(window); + + // Open the popup via a REAL click on the button face (screen space). + root.OnMouseDown(UiMouseButton.Left, 20, 155); + root.OnMouseUp(UiMouseButton.Left, 20, 155); + Assert.True(menu.IsOpen); + + // OuterW = ColumnWidth(191) + 2*Border(5) = 201; OuterH = 1*RowHeight(17) + + // 2*Border(5) = 27. Opens upward from the button's own screen top (150), so + // the popup spans screen Y = 150-27=123 .. 150 -- strictly above the owning + // window's own [150,168) rect, i.e. the escaped region. + const int rowScreenY = 135; // inside [123,150) + const int rowScreenX = 60; // inside [10,211) + Assert.True(rowScreenY < 150, "sanity: the row must sit above the window's own top edge"); + + // Without PopupHit, a plain top-down walk at this point would be rejected by + // `window`'s own ancestor-clip bounds check before ever reaching the menu -- + // proven directly against the SAME tree/geometry, no popup registered. + Assert.Null(root.Pick(rowScreenX, rowScreenY)); + + // WantsMouse must recognize the escaped popup region too (S4), so a game + // action does not fire underneath an open dropdown. + root.OnMouseMove(rowScreenX, rowScreenY); + Assert.True(root.WantsMouse, "WantsMouse must see the escaped popup through PopupHit"); + + root.OnMouseDown(UiMouseButton.Left, rowScreenX, rowScreenY); + root.OnMouseUp(UiMouseButton.Left, rowScreenX, rowScreenY); + + Assert.Equal("row", picked); + Assert.False(menu.IsOpen); + } + + /// + /// CT-GF1 fix round (N2): the empty-clip subtree cull added to + /// (retail's var_24 gate, + /// UIRegion::DrawHere @0x0069FB8E) early-outs once the intersected clip + /// goes empty -- e.g. a window whose own Width has collapsed to zero. + /// is a wholly SEPARATE traversal (the + /// second pass runs after the main one) that shares no + /// clip-stack state with the cull above, so an open popup + /// nested inside such a window must keep drawing there regardless. + /// + [Fact] + public void UiMenuPopup_StillDraws_EvenWhenItsOwningWindowIsFullyClippedAway() + { + var root = new TestElement { Width = 200f, Height = 200f }; + var window = new TestElement { Left = 10f, Top = 150f, Width = 0f, Height = 18f }; + var menu = new UiMenu + { + Width = 80f, + Height = 18f, + Items = new[] { new UiMenu.MenuItem("Row", (object?)null) }, + SpriteResolve = id => (id, 8, 8), + }; + root.AddChild(window); + window.AddChild(menu); + + Assert.True(menu.OnEvent(new UiEvent(0, menu, UiEventType.MouseDown, 0, 10, 5))); + Assert.True(menu.IsOpen); + + var (_, renderer, ctx) = MakeContext(200f, 200f); + + // Main pass: `window`'s own zero-width clip is empty -- the cull skips its + // whole subtree (including the menu's own button face), so nothing draws. + root.DrawSelfAndChildren(ctx); + Assert.Empty(renderer.DebugSpriteSegmentVerts); + + // Overlay pass: the SAME open popup still renders -- proves the cull above is + // scoped to DrawSelfAndChildren and never reaches DrawOverlays. + root.DrawOverlays(ctx); + Assert.NotEmpty(renderer.DebugSpriteSegmentVerts); + } }