From 101a35cc2d6418847421014f0df8e904b1bbdeca Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 25 Jun 2026 15:14:47 +0200 Subject: [PATCH] =?UTF-8?q?fix(studio):=20Task=203=20review=20=E2=80=94=20?= =?UTF-8?q?UiRoot.Pick,=20RenderStack=20IDisposable,=20dt=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review follow-ups to the ImGui inspector: - Add public UiRoot.Pick(x,y) over the private HitTestTopDown (honors Z-order + modal exclusivity); StudioWindow uses it instead of a manual UiElement.HitTest with subtracted ScreenPosition. - RenderStack : IDisposable — disposes the GL pieces it owns in one place; StudioWindow OnClosing + Dispose both call _stack?.Dispose(), closing the error-path leak (only UiHost was disposed on the Dispose-without-OnClosing path). - Drop the stale _dt field; OnRender passes its own dt to Tick + BeginFrame. - Fix a stale PanelFbo comment. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/AcDream.App/Rendering/RenderBootstrap.cs | 15 +++++++- src/AcDream.App/Studio/PanelFbo.cs | 2 +- src/AcDream.App/Studio/StudioWindow.cs | 36 +++++++------------- src/AcDream.App/UI/UiRoot.cs | 4 +++ 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/AcDream.App/Rendering/RenderBootstrap.cs b/src/AcDream.App/Rendering/RenderBootstrap.cs index 8f8a1001..6a23a0d2 100644 --- a/src/AcDream.App/Rendering/RenderBootstrap.cs +++ b/src/AcDream.App/Rendering/RenderBootstrap.cs @@ -22,8 +22,21 @@ public sealed record RenderStack( Wb.WbDrawDispatcher DrawDispatcher, SceneLightingUboBinding LightingUbo, AcDream.App.UI.UiHost UiHost, - AcDream.App.UI.UiDatFont? VitalsDatFont) + AcDream.App.UI.UiDatFont? VitalsDatFont) : System.IDisposable { + /// Dispose the GL pieces this stack OWNS (everything created in + /// ). + are caller-owned + /// and NOT disposed here. Called once at studio teardown. + public void Dispose() + { + DrawDispatcher.Dispose(); + MeshAdapter.Dispose(); + TextureCache.Dispose(); + MeshShader.Dispose(); + LightingUbo.Dispose(); + UiHost.Dispose(); + } + /// /// Resolves a sprite id (0x06xxxxxx) to a (GL handle, width, height) triple. /// Copied verbatim from GameWindow's ResolveChrome closure — it calls diff --git a/src/AcDream.App/Studio/PanelFbo.cs b/src/AcDream.App/Studio/PanelFbo.cs index 91270d97..de6aabb4 100644 --- a/src/AcDream.App/Studio/PanelFbo.cs +++ b/src/AcDream.App/Studio/PanelFbo.cs @@ -56,7 +56,7 @@ public sealed unsafe class PanelFbo : IDisposable _gl.BindFramebuffer(FramebufferTarget.Framebuffer, _fbo); _gl.Viewport(0, 0, (uint)width, (uint)height); _gl.Disable(EnableCap.ScissorTest); - _gl.ClearColor(0.18f, 0.18f, 0.18f, 1f); // dark bg matching the old direct draw + _gl.ClearColor(0.18f, 0.18f, 0.18f, 1f); // opaque dark-grey canvas background (the FBO IS the canvas) _gl.ClearDepth(1.0); _gl.DepthMask(true); _gl.Clear(ClearBufferMask.ColorBufferBit | ClearBufferMask.DepthBufferBit); diff --git a/src/AcDream.App/Studio/StudioWindow.cs b/src/AcDream.App/Studio/StudioWindow.cs index b77d1420..151464e2 100644 --- a/src/AcDream.App/Studio/StudioWindow.cs +++ b/src/AcDream.App/Studio/StudioWindow.cs @@ -24,7 +24,7 @@ namespace AcDream.App.Studio; /// Tree pane — the element hierarchy; click-to-select. /// Properties pane — geometry/anchors/flags of the selected element. /// Click-to-inspect — a left-click in the canvas selects the topmost -/// element under the cursor via . +/// element under the cursor via . /// /// /// The window is intentionally thin: no game world, no physics, no streaming — @@ -127,12 +127,7 @@ public sealed class StudioWindow : IDisposable _inspector = new StudioInspector(); } - private double _dt; - - private void OnUpdate(double dt) - { - _dt = dt; - } + private void OnUpdate(double dt) { } private void OnRender(double dt) { @@ -143,8 +138,8 @@ public sealed class StudioWindow : IDisposable int w = _window!.Size.X; int h = _window!.Size.Y; - // 1. Tick the UI widgets. - _stack.UiHost.Tick(_dt); + // 1. Tick the UI widgets (OnRender's own dt — Update + Render fire with the same delta). + _stack.UiHost.Tick(dt); // 2. Render the panel into the off-screen FBO; get the color texture. // The FBO is the same logical size as the window, so element rects map 1:1 to @@ -163,13 +158,12 @@ public sealed class StudioWindow : IDisposable if (panelTex != 0) click = _inspector.DrawCanvas((nint)panelTex, w, h); - // 6. If the user clicked inside the canvas, hit-test the panel tree and select. - if (click is { } c && _panelRoot is not null) + // 6. If the user clicked inside the canvas, hit-test the whole UI tree (UiRoot.Pick honors + // Z-order + modal exclusivity) and select the topmost element. The canvas click coord is + // in the same root-space the FBO was drawn in, so it maps 1:1. + if (click is { } c) { - // UiElement.HitTest is internal — accessible from AcDream.App.Studio (same assembly). - // We call it on the panel root with coords relative to its top-left. - var screenPos = _panelRoot.ScreenPosition; - var hit = _panelRoot.HitTest(c.x - screenPos.X, c.y - screenPos.Y); + var hit = _stack.UiHost.Root.Pick(c.x, c.y); if (hit is not null) _inspector.Selected = hit; } @@ -191,12 +185,7 @@ public sealed class StudioWindow : IDisposable _panelFbo?.Dispose(); _imgui = null; _panelFbo = null; - _stack?.DrawDispatcher.Dispose(); - _stack?.MeshAdapter.Dispose(); - _stack?.TextureCache.Dispose(); - _stack?.MeshShader.Dispose(); - _stack?.LightingUbo.Dispose(); - _stack?.UiHost.Dispose(); + _stack?.Dispose(); // whole render stack: dispatcher/mesh/textures/shader/ubo/uihost _dats?.Dispose(); _dats = null; _stack = null; @@ -210,8 +199,9 @@ public sealed class StudioWindow : IDisposable _panelFbo = null; _window?.Dispose(); _window = null; - // If OnClosing wasn't called (e.g. exception before Run()), clean up anyway. - _stack?.UiHost.Dispose(); + // If OnClosing wasn't called (e.g. an exception before Run() completed), dispose the FULL + // stack anyway — the review flagged that disposing only UiHost here leaked the rest. + _stack?.Dispose(); _dats?.Dispose(); _dats = null; _stack = null; diff --git a/src/AcDream.App/UI/UiRoot.cs b/src/AcDream.App/UI/UiRoot.cs index 7740fddc..251b66c1 100644 --- a/src/AcDream.App/UI/UiRoot.cs +++ b/src/AcDream.App/UI/UiRoot.cs @@ -655,6 +655,10 @@ public sealed class UiRoot : UiElement return (null, 0, 0); } + /// Public hit-test for tooling (the UI Studio inspector): the topmost element under + /// (x,y) in root space, honoring modal exclusivity + Z-order. Wraps the private HitTestTopDown. + public UiElement? Pick(int x, int y) => HitTestTopDown(x, y).element; + private static UiElement? FindWindow(UiElement? e) { while (e is not null)