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

fix(CT-GF1): review fix round — literal DrawHere clip shape, empty-clip cull, popup input routing

Browse source 
Applies all 11 items from the Opus dual-lens review of 989f6652 (0
blockers, 7 SHOULD-FIX, 4 NOTE):

- S2: UiElement.DrawSelfAndChildren now pushes the ambient clip right
  after PushAlpha and wraps OnDraw + the children walk +
  OnDrawAfterChildren in ONE block — the literal UIRegion::DrawHere
  @0x0069FA30 shape, which clips an 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). Deleted the two now-redundant
  ad-hoc self-clips this supersedes: UiText.DrawText and
  UiField.DrawMultiLine both pushed their own (0,0,Width,Height) —
  exactly what the new ambient clip already provides one level up.
  Kept UiButton.DrawBlockLabel's clip: it clips to LabelBox/ValueBox,
  an authored INNER sub-rect that can be smaller than and offset from
  the button's own full rect — a genuine narrower viewport, not a
  redundant duplicate.
- S3: deleted UiItemList's `ClipsChildren => CellWidth > 0f` override
  — correct under the old opt-in-false default, inverted under the
  new default-true (an unconfigured list would stop clipping instead
  of clipping like everything else).
- S4: pinned the escaped-popup input path end to end. New
  UiAncestorClipTests test mounts a menu inside a short window on a
  real UiRoot, opens it, and proves a click in the escaped popup
  region reaches the menu through UiRoot.PopupHit (a plain top-down
  walk is proven to reject the same point first). UiRoot.WantsMouse
  now also checks PopupHit — it previously only checked Captured/
  HitTestTopDown, so a game action could fire underneath an open
  dropdown's escaped region. OnMouseDown/OnScroll already routed
  through PopupHit first (#374); unchanged.
- S5: strengthened the Titles-divider regression test's positive
  half. The old assertion only checked SOME quad's Y fell in a band —
  vacuously true given other same-band content. Now asserts the
  divider's exact rect (X and Y), then diffs against the same rect
  with the divider hidden (Visible=false) to prove the quad was
  actually attributable to it.
- S1: added UiWindowDrawCaptureSweepTests — Character/Chat/Vendor/
  Options mounted through their real production Bind entry points
  with a non-zero sprite resolver, drawn via RecordingGpuDevice,
  asserting a per-window vertex floor (~40-45% of this session's
  observed baseline: Character 588, Chat 162, Vendor 54, Options 240)
  plus one key sprite id read LIVE off the bound controller/element
  (never hardcoded). Character's key sprite (RetailChromeSprites.
  TopEdge) specifically exercises OnDrawAfterChildren, the exact path
  S2's caution note flagged. Inventory/Paperdoll/social/map-house
  skipped — no single fixture-driven top-level Bind entry point.
- S6: added the CT-GF1 subsection to the campaign plan's ledger
  (989f6652 + this fix round; CT7 re-gate still owed).
- S7: UiRenderContext.PushClipUnbounded now resets to the CANVAS rect
  (0,0,ScreenSize), not null — retail's own popup region is
  SCREEN-clipped (UIElement_Menu::MakePopup spawns a top-level region
  bounded by the screen), not truly unbounded. AD-113 amended.
- N1: UiRoot overrides ClipsChildren => false — the root's own region
  IS the screen (the viewport already scissors it), so this is a
  safety net against a momentarily zero-sized root silently blanking
  the whole UI tree under the new ancestor-clip default.
- N2: added the empty-clip subtree cull (retail's var_24 gate
  @0x0069FB8E) to DrawSelfAndChildren only — DrawOverlays is a wholly
  separate traversal untouched by this change. New test proves a menu
  inside a fully-clipped (zero-width) window still draws its open
  popup via the overlay pass while the main pass draws nothing.
- N3: CT7 script §5 now 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 the re-gate.
- N4: verification below covers both the working tree and the clean
  committed tree.

Decomp anchors: UIRegion::DrawHere @0x0069FA30 (var_24 gate
@0x0069FB8E); UIElement_Text::DrawSelf @0x00467AA0 (self-clip);
UIRegion::DrawSelf @0x0069F1A0; UIElement_Menu::MakePopup (screen-
clipped popup region).

Verification (both runs green, --filter "Lane!=InstalledDat&
Lane!=PreparedPackage&Lane!=Live&Lane!=Manual&Lane!=Timing&
Lane!=Windows&Lane!=Linux&Lane!=SystemFont&Purpose!=Diagnostic&
Status!=KnownFailure"): full Release solution build green; working
tree 14,900+ tests across every project (one LandblockPresentation
PipelineTests flake reproduced ONLY under full-solution parallel
load, passes standalone and on rerun — unrelated to this change,
streaming domain); InstalledDat lane green (ACDREAM_RUN_INSTALLED_DAT
_TESTS=1, Status!=KnownFailure, 205+34+3+172 App/Content/Bake/Core
tests). Clean committed tree (git stash push -u the uncommitted
owner probe + docs files, rerun, stash pop) reported in the session
summary.

src/AcDream.App/UI/UiRoot.cs carries an unrelated, pre-existing
uncommitted owner probe (ACDREAM_PROBE_UI_HOVER) — staged selectively
(git add -p) so only this commit's own two hunks (ClipsChildren
override, WantsMouse) landed; the probe hunk is untouched and stays
uncommitted, same as before this fix round.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-25 07:11:42 +02:00
parent 989f665214
commit 025108a8aa
13 changed files with 717 additions and 120 deletions
Download patch file Download diff file Expand all files Collapse all files

101
src/AcDream.App/UI/UiElement.cs
View file

@ -544,30 +544,39 @@ public abstract class UiElement
protected virtual void OnDrawOverlay(UiRenderContext ctx) { }
/// <summary>
/// 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: <c>UIRegion::DrawHere @0x0069FA30</c> takes
/// the element's screen <c>Box2D</c> plus a <c>SmartArray&lt;Box2D&gt;</c> of
/// inherited clip rects, intersects them (the min/max clamp loop
/// @0x0069FAA7..0x0069FB82), and draws — <c>EraseSelf</c>/<c>DrawChildren</c>/
/// <c>DrawSelf</c> all receive the intersected rect — ONLY when the intersection
/// is non-empty (the <c>var_24</c> gate @0x0069FB8E). An element positioned
/// outside its parent's box therefore silently disappears in retail, exactly
/// like <see cref="UiRenderContext.PushClip"/>/<see cref="UiRenderContext.PopClip"/>
/// (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: <c>UIRegion::DrawHere
/// @0x0069FA30</c> takes the element's screen <c>Box2D</c> plus a
/// <c>SmartArray&lt;Box2D&gt;</c> of inherited clip rects, intersects them (the
/// min/max clamp loop @0x0069FAA7..0x0069FB82), and draws — <c>EraseSelf</c>/
/// <c>DrawChildren</c>/<c>DrawSelf</c> ALL receive the intersected rect — ONLY
/// when the intersection is non-empty (the <c>var_24</c> gate @0x0069FB8E). An
/// element positioned outside its parent's box therefore silently disappears in
/// retail, exactly like <see cref="UiRenderContext.PushClip"/>/
/// <see cref="UiRenderContext.PopClip"/> now does for every element by default —
/// <see cref="DrawSelfAndChildren"/>'s fix-round shape (S2) pushes right after
/// <c>PushAlpha</c> and wraps <c>OnDraw</c> + the children walk +
/// <c>OnDrawAfterChildren</c> in ONE block, the literal <c>DrawHere</c> shape
/// (retail clips the element's OWN <c>DrawSelf</c> too, not just its children —
/// <c>UIElement_Text::DrawSelf @0x00467AA0</c> locks glyph blits to its own
/// clipped surface rect; <c>UIRegion::DrawSelf @0x0069F1A0</c> 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).
///
/// <para>
/// Override to <see langword="false"/> ONLY for a widget that must draw or accept
/// input beyond its own bounds by deliberate design — today just
/// <see cref="UiMenu"/>, whose popup (and its own out-of-bounds
/// <c>OnHitTest</c> override) stands in for retail's separate top-level popup
/// region; see <see cref="ExpandsClipForPopup"/> for the drawing half of that
/// opt-out and the divergence register row it cites.
/// <see cref="UiMenu"/> (whose popup, and its own out-of-bounds <c>OnHitTest</c>
/// override, stands in for retail's separate top-level popup region — see
/// <see cref="ExpandsClipForPopup"/> for the drawing half of that opt-out and the
/// divergence register row it cites) and <see cref="UiRoot"/> (whose own region
/// IS the screen — the viewport itself already scissors it, so narrowing to
/// <c>(0,0,Width,Height)</c> 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).
/// </para>
/// </summary>
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();
}