fix(studio): Task 3 review — UiRoot.Pick, RenderStack IDisposable, dt cleanup

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) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-06-25 15:14:47 +02:00
parent d2240974ec
commit 101a35cc2d
4 changed files with 32 additions and 25 deletions

View file

@ -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);

View file

@ -24,7 +24,7 @@ namespace AcDream.App.Studio;
/// <item>Tree pane — the element hierarchy; click-to-select.</item>
/// <item>Properties pane — geometry/anchors/flags of the selected element.</item>
/// <item>Click-to-inspect — a left-click in the canvas selects the topmost
/// element under the cursor via <see cref="UiRoot.HitTest"/>.</item>
/// element under the cursor via <see cref="UiRoot.Pick"/>.</item>
/// </list>
///
/// 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;