Build the Pixely.Ui tree in the update phase instead of during rendering

docs/ui.md

@@ -53,6 +53,24 @@ A layer is measured against the viewport and arranged to it, so its own `Width`,
 
 A layer does not block the pointer by being on top. Only an `IPointerTarget` is hit-tested at all, so a modal backdrop has to be one; an ordinary panel over a button lets the button through. A target that declines a button does not fall through to a UI target beneath it either — the event is simply left unconsumed for whatever is outside the UI.
 
+## When the tree is built
+
+`UseUi` registers a system that builds the tree in the update phase, before anything renders. Building is not a passive walk: it raises pointer enter and leave as layout moves under a stationary pointer, raises focus lost when a focused element leaves the tree, and runs every custom element, layout and drawable in it. A renderer may not raise those — the renderers sharing a frame all read domain data over one command buffer and are entitled to it not changing underneath them — so the UI renderer only paints what the build already produced.
+
+`UseUi` takes one order per phase — `renderOrder`, `updateOrder`, `inputOrder` — and lower runs first in all three.
+
+`updateOrder` says when the tree is built relative to the other updatables. It defaults to `10_000` so the UI builds after ordinary order-0 game systems and views sync against the state this frame produced. Equal orders are unspecified rather than registration order. A system that runs after the build and dirties the UI has its change shown on the next frame, not this one.
+
+```csharp
+builder.UseUi(updateOrder: 500);
+```
+
+The viewport event is raised by `SetViewportSize`, which the same system calls immediately before the build, not by the build itself. Pointer and focus callbacks still arrive during event routing as they always did, `RemoveLayer` still reconciles immediately, and `UiRoot.Update` stays public for an application that wants to drive a root itself.
+
+A hidden or zero-area window does not build. A window resized between the update phase and rendering shows one blank UI frame, because the instructions describe the previous size; the next update catches up.
+
+The build lays out against the window's render size. A render context whose colour target is a different size than the window is not supported: the UI is laid out for the window and the renderer refuses to draw it into a target of another size, so it stays blank.
+
 ## Sizing
 
 `Sizing` is per axis, set through `Element.Width` and `Element.Height`:

src/Pixely.Ui/IUiPaintSource.cs

@@ -0,0 +1,22 @@
+namespace Pixely.Ui;
+
+/// <summary>
+/// What a renderer is allowed to see of a <see cref="UiRoot"/>: the completed instructions, and the
+/// facts needed to decide whether they are still current. Deliberately without <c>Update</c> and
+/// <c>SetViewportSize</c>. Building the tree raises application callbacks, and a renderer may not
+/// raise those: the renderers sharing a frame are entitled to domain data that does not change
+/// underneath them. Same-assembly code can still cast back to <see cref="UiRoot"/>; the point is
+/// that no ordinary edit reaches a build by accident.
+/// </summary>
+internal interface IUiPaintSource
+{
+    IReadOnlyList<PaintInstruction> Instructions { get; }
+
+    IReadOnlyList<PaintBatch> Batches { get; }
+
+    Vector2Int PaintedViewportSize { get; }
+
+    Vector2Int ViewportSize { get; }
+
+    ulong BuildVersion { get; }
+}

src/Pixely.Ui/UiExtensions.cs

@@ -15,27 +15,37 @@ public static class UiExtensions
     /// </summary>
     public static PixelyAppBuilder UseUi(
         this PixelyAppBuilder appBuilder,
-        int order = 10_000,
+        int renderOrder = 10_000,
+        int updateOrder = 10_000,
         int inputOrder = -10_000,
         bool clearTarget = false)
     {
-        return UseUi<BasicRenderContext>(appBuilder, default, order, inputOrder, clearTarget);
+        return UseUi<BasicRenderContext>(appBuilder, default, renderOrder, updateOrder, inputOrder, clearTarget);
     }
 
     public static PixelyAppBuilder UseUi(
         this PixelyAppBuilder appBuilder,
         ViewScope viewScope,
-        int order = 10_000,
+        int renderOrder = 10_000,
+        int updateOrder = 10_000,
         int inputOrder = -10_000,
         bool clearTarget = false)
     {
-        return UseUi<BasicRenderContext>(appBuilder, viewScope, order, inputOrder, clearTarget);
+        return UseUi<BasicRenderContext>(appBuilder, viewScope, renderOrder, updateOrder, inputOrder, clearTarget);
     }
 
+    /// <param name="renderOrder">When the UI is drawn relative to the other renderers, lower first. Defaults late, so it draws over the game.</param>
+    /// <param name="updateOrder">
+    /// When the tree is built relative to the other updatables, lower first. Defaults late, so the
+    /// UI builds after ordinary order-0 game systems and views sync against the state this frame
+    /// produced. Equal orders are unspecified, not registration order.
+    /// </param>
+    /// <param name="inputOrder">When the UI sees input relative to the other subscribers, lower first. Defaults early, so it takes events before the game does.</param>
     public static PixelyAppBuilder UseUi<TRenderContext>(
         this PixelyAppBuilder appBuilder,
         ViewScope viewScope,
-        int order = 10_000,
+        int renderOrder = 10_000,
+        int updateOrder = 10_000,
         int inputOrder = -10_000,
         bool clearTarget = false)
         where TRenderContext : IRenderContext
@@ -71,11 +81,17 @@ public static PixelyAppBuilder UseUi<TRenderContext>(
                 provider.GetRequiredService<IKeyboardService>(),
                 provider.GetRequiredService<ITextInputService>()));
 
+        appBuilder.AddSingleton<UiUpdateSystem>(provider =>
+        {
+            (UiRoot root, Window window) = ResolveUpdateTargets(provider, viewScope);
+            return new UiUpdateSystem(root, () => WindowViewport(window), () => window.IsVisible, updateOrder);
+        });
+
         appBuilder.AddSingleton<IRenderer<TRenderContext>, UiRenderer<TRenderContext>>(provider =>
             UiRenderer<TRenderContext>.Create(
                 ScopedUiRoot.GetRequired(provider, viewScope).Root,
                 viewScope,
-                order,
+                renderOrder,
                 clearTarget,
                 provider.GetRequiredService<GraphicsPipelineBuilder>(),
                 provider.GetRequiredService<GpuMemorySystem>(),
@@ -86,6 +102,26 @@ public static PixelyAppBuilder UseUi<TRenderContext>(
         return appBuilder;
     }
 
+    /// <summary>
+    /// The root and window one scope's update system drives. Extracted so the scope lookup is
+    /// observable to a test: the system itself holds only closures, and nothing can tell from
+    /// outside which window they captured.
+    /// </summary>
+    internal static (UiRoot Root, Window Window) ResolveUpdateTargets(ServiceProvider provider, ViewScope viewScope)
+    {
+        // The scope has to be threaded through: GetWindow's viewScope parameter is defaulted, so
+        // dropping it compiles and silently binds every window's UI to the first one.
+        return (ScopedUiRoot.GetRequired(provider, viewScope).Root, provider.GetWindow(viewScope));
+    }
+
+    // Read once. Two reads are two SDL calls, and a resize between them pairs a width from one
+    // state with a height from another.
+    private static Vector2Int WindowViewport(Window window)
+    {
+        ShortSize size = window.RenderSizeInPixels;
+        return new Vector2Int(size.Width, size.Height);
+    }
+
     /// <summary>
     /// The window's logical size, resolved once and read per event. The window itself is resolved
     /// here rather than inside the input system, which needs the size and nothing else.

src/Pixely.Ui/UiRenderer.cs

@@ -7,8 +7,10 @@
 namespace Pixely.Ui;
 
 /// <summary>
-/// Paints a <see cref="UiRoot"/> into a persistent texture and blits that texture over the frame.
-/// The texture is only repainted when the tree changed, so a static UI costs one quad per frame.
+/// Paints a built <see cref="IUiPaintSource"/> into a persistent texture and blits that texture over
+/// the frame. The texture is only repainted when the tree changed, so a static UI costs one quad per
+/// frame. The build itself belongs to <see cref="UiUpdateSystem"/>, which is why this holds a source
+/// rather than the root.
 /// </summary>
 internal sealed class UiRenderer<TRenderContext> : IRenderer<TRenderContext>, IDisposable
     where TRenderContext : IRenderContext
@@ -27,7 +29,7 @@ internal sealed class UiRenderer<TRenderContext> : IRenderer<TRenderContext>, ID
     private readonly Sampler _sampler;
     private readonly GpuDevice _gpuDevice;
     private readonly TextureFormat _colorTargetFormat;
-    private readonly UiRoot _root;
+    private readonly IUiPaintSource _source;
     private readonly bool _clearTarget;
 
     // Solid fills sample this, which is what keeps colours and sprites on one pipeline.
@@ -36,6 +38,7 @@ internal sealed class UiRenderer<TRenderContext> : IRenderer<TRenderContext>, ID
     private Texture _retainedTexture;
     private Matrix4x4 _viewProjection;
     private bool _retainedTextureDirty = true;
+    private ulong _paintedVersion;
 
     public int Order { get; }
     public ViewScope ViewScope { get; }
@@ -45,7 +48,7 @@ internal sealed class UiRenderer<TRenderContext> : IRenderer<TRenderContext>, ID
     /// so that constructing a renderer is assignment only.
     /// </summary>
     internal static UiRenderer<TRenderContext> Create(
-        UiRoot root,
+        IUiPaintSource source,
         ViewScope viewScope,
         int order,
         bool clearTarget,
@@ -101,18 +104,18 @@ internal static UiRenderer<TRenderContext> Create(
             gpuDevice.CreateColorTargetTexture(renderSize, colorTargetFormat),
             colorTargetFormat);
 
-        return new UiRenderer<TRenderContext>(root, viewScope, order, clearTarget, gpuDevice, resources);
+        return new UiRenderer<TRenderContext>(source, viewScope, order, clearTarget, gpuDevice, resources);
     }
 
     private UiRenderer(
-        UiRoot root,
+        IUiPaintSource source,
         ViewScope viewScope,
         int order,
         bool clearTarget,
         GpuDevice gpuDevice,
         GpuResources resources)
     {
-        _root = root;
+        _source = source;
         ViewScope = viewScope;
         Order = order;
         _clearTarget = clearTarget;
@@ -143,28 +146,48 @@ public void Render(TRenderContext renderContext)
         ShortSize targetSize = renderContext.ColorTarget.Size;
         ResizeRetainedTextureIfNeeded(targetSize);
 
-        _root.SetViewportSize(new Vector2Int(targetSize.Width, targetSize.Height));
+        Vector2Int target = new(targetSize.Width, targetSize.Height);
 
-        bool rebuilt = _root.Update();
-
-        // Instructions laid out for a different viewport would draw the previous frame's geometry
-        // at the new size, so the texture is cleared until a matching build lands.
-        if (_root.PaintedViewportSize != new Vector2Int(targetSize.Width, targetSize.Height))
+        if (IsStale(_source.PaintedViewportSize, _source.ViewportSize, target))
         {
             Clear(renderContext.CommandBuffer);
             Present(renderContext.CommandBuffer, renderContext.ColorTarget);
             return;
         }
 
-        if (rebuilt || _retainedTextureDirty)
+        if (NeedsRepaint(_source.BuildVersion, _paintedVersion, _retainedTextureDirty))
         {
             Paint(renderContext.CommandBuffer);
+            _paintedVersion = _source.BuildVersion;
             _retainedTextureDirty = false;
         }
 
         Present(renderContext.CommandBuffer, renderContext.ColorTarget);
     }
 
+    /// <summary>
+    /// Whether the completed instructions describe geometry this frame cannot draw. Two questions,
+    /// and either one is enough. Did the build finish at the viewport it was asked for — a callback
+    /// can move the viewport after layout ran. And is that viewport still the target being drawn
+    /// into — a resize landing between the update phase and here breaks it. Either way the texture
+    /// is cleared until a matching build lands, because the projection is built from the target and
+    /// stretching the previous frame's geometry into it is worse than a blank one.
+    /// </summary>
+    internal static bool IsStale(Vector2Int paintedViewportSize, Vector2Int viewportSize, Vector2Int target)
+    {
+        return paintedViewportSize != viewportSize || paintedViewportSize != target;
+    }
+
+    /// <summary>
+    /// Whether the retained texture no longer shows what the source holds. Compared against the
+    /// build this renderer last painted rather than against whether a build just happened, so a
+    /// renderer that missed one still repaints instead of depending on having been its caller.
+    /// </summary>
+    internal static bool NeedsRepaint(ulong buildVersion, ulong paintedVersion, bool retainedTextureDirty)
+    {
+        return buildVersion != paintedVersion || retainedTextureDirty;
+    }
+
     private void ResizeRetainedTextureIfNeeded(ShortSize newSize)
     {
         if (_retainedTexture.Size == newSize)
@@ -180,8 +203,8 @@ private void ResizeRetainedTextureIfNeeded(ShortSize newSize)
 
     private void Paint(CommandBuffer commandBuffer)
     {
-        IReadOnlyList<PaintInstruction> instructions = _root.Instructions;
-        IReadOnlyList<PaintBatch> batches = _root.Batches;
+        IReadOnlyList<PaintInstruction> instructions = _source.Instructions;
+        IReadOnlyList<PaintBatch> batches = _source.Batches;
 
         if (instructions.Count == 0)
         {

src/Pixely.Ui/UiRoot.cs

@@ -6,7 +6,7 @@ namespace Pixely.Ui;
 /// Drives measure, arrange and paint for one viewport, and owns the state the renderer reads.
 /// Elements never run a layout pass on themselves, so the tree cannot be half-updated.
 /// </summary>
-public sealed class UiRoot
+public sealed class UiRoot : IUiPaintSource
 {
     private readonly PointerRouter _pointerRouter;
     private readonly PaintContext _paintContext = new();
@@ -52,6 +52,13 @@ public UiRoot()
 
     internal bool IsPaintDirty { get; private set; } = true;
 
+    /// <summary>
+    /// Rises with every completed build. The renderer compares it against what it last painted, so a
+    /// renderer that missed a build still repaints rather than depending on having been the caller
+    /// that triggered it.
+    /// </summary>
+    internal ulong BuildVersion { get; private set; }
+
     internal IReadOnlyList<PaintInstruction> Instructions => _paintContext.Instructions;
 
     /// <summary>Instruction runs sharing a texture and a clip, in paint order.</summary>
@@ -519,9 +526,12 @@ public bool Update()
 
     private bool Rebuild()
     {
-
-        Rectangle viewport = new(0, 0, _viewportSize.X, _viewportSize.Y);
-        Constraints constraints = Constraints.Tight(_viewportSize);
+        // Captured at entry and used for everything below, including what is recorded at the end. A
+        // callback further down can call SetViewportSize, and recording the field as it stands then
+        // would claim this geometry was built for a viewport it never saw.
+        Vector2Int viewportSize = _viewportSize;
+        Rectangle viewport = new(0, 0, viewportSize.X, viewportSize.Y);
+        Constraints constraints = Constraints.Tight(viewportSize);
 
         _paintContext.Reset(viewport);
 
@@ -559,7 +569,8 @@ private bool Rebuild()
 
         _layersChanged = false;
         IsPaintDirty = false;
-        PaintedViewportSize = _viewportSize;
+        PaintedViewportSize = viewportSize;
+        BuildVersion++;
         return true;
     }
 
@@ -608,4 +619,13 @@ private bool NeedsUpdate()
 
         return false;
     }
+
+    // Forwarded explicitly, all five of them together: four of the members are internal, an internal
+    // member cannot implicitly implement an interface one, and widening them is not available either
+    // because PaintInstruction and PaintBatch are internal types.
+    IReadOnlyList<PaintInstruction> IUiPaintSource.Instructions => Instructions;
+    IReadOnlyList<PaintBatch> IUiPaintSource.Batches => Batches;
+    Vector2Int IUiPaintSource.PaintedViewportSize => PaintedViewportSize;
+    Vector2Int IUiPaintSource.ViewportSize => ViewportSize;
+    ulong IUiPaintSource.BuildVersion => BuildVersion;
 }

src/Pixely.Ui/UiUpdateSystem.cs

@@ -0,0 +1,52 @@
+namespace Pixely.Ui;
+
+/// <summary>
+/// Builds the tree in the update phase. A build raises application callbacks — pointer enter and
+/// leave as layout moves under a stationary pointer, focus lost, and every custom element, layout
+/// and drawable in the tree — and a renderer may not run those: the renderers sharing a frame are
+/// entitled to domain data that does not change underneath them.
+/// </summary>
+internal sealed class UiUpdateSystem : IUpdatable, IOrderable
+{
+    private readonly UiRoot _root;
+    private readonly Func<Vector2Int> _viewport;
+    private readonly Func<bool> _isVisible;
+
+    /// <param name="viewport">
+    /// The size to lay out against, read per frame rather than held: it changes as the window is
+    /// resized. A delegate rather than the window itself, because the window's size and visibility
+    /// are non-virtual SDL calls and this has to be constructible in a test.
+    /// </param>
+    internal UiUpdateSystem(UiRoot root, Func<Vector2Int> viewport, Func<bool> isVisible, int updateOrder)
+    {
+        _root = root;
+        _viewport = viewport;
+        _isVisible = isVisible;
+        Order = updateOrder;
+    }
+
+    public int Order { get; }
+
+    public void Update()
+    {
+        // Visibility first, so a hidden window does not pay for a size call it will not use. Nothing
+        // skipped the build for it before this class existed either: the only caller of Update was
+        // the renderer, which RenderCoordinator had already skipped.
+        if (!_isVisible())
+        {
+            return;
+        }
+
+        Vector2Int viewport = _viewport();
+
+        // A window with no area has nothing to lay out against. Building against it would invalidate
+        // every layer now and again on restore, for a tree nothing is going to draw.
+        if (viewport.X <= 0 || viewport.Y <= 0)
+        {
+            return;
+        }
+
+        _root.SetViewportSize(viewport);
+        _root.Update();
+    }
+}