From 70f4dabcc56bd857d10e113ddda843ae1a6a3f29 Mon Sep 17 00:00:00 2001 From: botoddly <250804054+botoddly@users.noreply.github.com> Date: Wed, 9 Sep 2026 08:03:37 +0200 Subject: [PATCH 1/5] Record the viewport a build actually laid out for --- src/Pixely.Ui/UiRoot.cs | 11 ++-- tests/Pixely.Ui.Tests/BuildViewportTests.cs | 63 +++++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 tests/Pixely.Ui.Tests/BuildViewportTests.cs diff --git a/src/Pixely.Ui/UiRoot.cs b/src/Pixely.Ui/UiRoot.cs index 7ae41f89..ed92d466 100644 --- a/src/Pixely.Ui/UiRoot.cs +++ b/src/Pixely.Ui/UiRoot.cs @@ -519,9 +519,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 +562,7 @@ private bool Rebuild() _layersChanged = false; IsPaintDirty = false; - PaintedViewportSize = _viewportSize; + PaintedViewportSize = viewportSize; return true; } diff --git a/tests/Pixely.Ui.Tests/BuildViewportTests.cs b/tests/Pixely.Ui.Tests/BuildViewportTests.cs new file mode 100644 index 00000000..3010e27a --- /dev/null +++ b/tests/Pixely.Ui.Tests/BuildViewportTests.cs @@ -0,0 +1,63 @@ +namespace Pixely.Ui.Tests; + +/// +/// What a completed build reports about itself. A build runs application callbacks partway through, +/// and one of those can move the viewport, so what it reports has to be the viewport it actually +/// laid out for rather than whatever the root holds by the time it finishes. +/// +public class BuildViewportTests +{ + [Test] + public void Rebuilding_RecordsTheViewportItLaidOutFor_NotOneACallbackSetAfterwards() + { + (UiRoot root, RecordingPointerTarget target, Element spacer) = HoveredTarget(); + + // Moves the target out from under the stationary pointer, so revalidation runs the leave + // callback in the middle of the build, and that callback resizes the viewport. + spacer.Height = Sizing.Fixed(100); + root.Update(); + + Assert.Multiple(() => + { + Assert.That(target.Calls, Does.Contain("leave"), "the callback ran during the build"); + Assert.That(root.PaintedViewportSize, Is.EqualTo(new Vector2Int(320, 240)), "the build reports the viewport it actually used"); + Assert.That(root.ViewportSize, Is.EqualTo(new Vector2Int(640, 480))); + }); + } + + [Test] + public void ACallbackMovingTheViewport_LeavesTheRootNeedingAnotherBuild() + { + (UiRoot root, RecordingPointerTarget target, Element spacer) = HoveredTarget(); + + spacer.Height = Sizing.Fixed(100); + root.Update(); + + Assert.Multiple(() => + { + Assert.That(root.Update(), Is.True, "the root is still dirty"); + Assert.That(root.PaintedViewportSize, Is.EqualTo(new Vector2Int(640, 480)), "and the next build catches up"); + }); + } + + /// + /// A built root with the pointer parked over a target that resizes the viewport when it is told + /// the pointer left. Growing the spacer is what pushes the target out from under the pointer. + /// + private static (UiRoot Root, RecordingPointerTarget Target, Element Spacer) HoveredTarget() + { + UiRoot root = new(); + root.SetViewportSize(new Vector2Int(320, 240)); + + RecordingPointerTarget target = new() { Width = Sizing.Fixed(50), Height = Sizing.Fixed(50) }; + target.WhenLeft = () => root.SetViewportSize(new Vector2Int(640, 480)); + + Element spacer = new() { Width = Sizing.Fixed(0), Height = Sizing.Fixed(0) }; + root.AddLayer(new Column { Children = { spacer, target } }); + root.Update(); + root.PointerMoved(new Vector2Int(10, 10)); + target.Calls.Clear(); + + return (root, target, spacer); + } +} From 86a6201efabfbfbc5e16c2a0f98b4f78385dc2d5 Mon Sep 17 00:00:00 2001 From: botoddly <250804054+botoddly@users.noreply.github.com> Date: Wed, 9 Sep 2026 08:08:36 +0200 Subject: [PATCH 2/5] Build the Pixely.Ui tree in the update phase instead of during rendering --- docs/ui.md | 20 +++ src/Pixely.Ui/IUiPaintSource.cs | 22 +++ src/Pixely.Ui/UiExtensions.cs | 51 +++++- src/Pixely.Ui/UiRenderer.cs | 55 +++++-- src/Pixely.Ui/UiRoot.cs | 19 ++- src/Pixely.Ui/UiUpdateSystem.cs | 52 ++++++ tests/Pixely.Ui.Tests/BuildViewportTests.cs | 20 +++ .../UiRendererDecisionTests.cs | 44 +++++ tests/Pixely.Ui.Tests/UiUpdateSystemTests.cs | 151 ++++++++++++++++++ 9 files changed, 412 insertions(+), 22 deletions(-) create mode 100644 src/Pixely.Ui/IUiPaintSource.cs create mode 100644 src/Pixely.Ui/UiUpdateSystem.cs create mode 100644 tests/Pixely.Ui.Tests/UiRendererDecisionTests.cs create mode 100644 tests/Pixely.Ui.Tests/UiUpdateSystemTests.cs diff --git a/docs/ui.md b/docs/ui.md index e4796d23..ace03a14 100644 --- a/docs/ui.md +++ b/docs/ui.md @@ -53,6 +53,26 @@ 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. + +`updateOrder` says when, relative to the other updatables. Lower runs first, and 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. + +If the render context draws into something other than the window — a same-format colour target of a different size — pass `viewportSource` so the build lays out against that instead of the window: + +```csharp +builder.UseUi(default, viewportSource: () => new Vector2Int(640, 360)); +``` + ## Sizing `Sizing` is per axis, set through `Element.Width` and `Element.Height`: diff --git a/src/Pixely.Ui/IUiPaintSource.cs b/src/Pixely.Ui/IUiPaintSource.cs new file mode 100644 index 00000000..3af18623 --- /dev/null +++ b/src/Pixely.Ui/IUiPaintSource.cs @@ -0,0 +1,22 @@ +namespace Pixely.Ui; + +/// +/// What a renderer is allowed to see of a : the completed instructions, and the +/// facts needed to decide whether they are still current. Deliberately without Update and +/// SetViewportSize. 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 ; the point is +/// that no ordinary edit reaches a build by accident. +/// +internal interface IUiPaintSource +{ + IReadOnlyList Instructions { get; } + + IReadOnlyList Batches { get; } + + Vector2Int PaintedViewportSize { get; } + + Vector2Int ViewportSize { get; } + + ulong BuildVersion { get; } +} diff --git a/src/Pixely.Ui/UiExtensions.cs b/src/Pixely.Ui/UiExtensions.cs index bf5ea5bf..ea659034 100644 --- a/src/Pixely.Ui/UiExtensions.cs +++ b/src/Pixely.Ui/UiExtensions.cs @@ -17,9 +17,11 @@ public static PixelyAppBuilder UseUi( this PixelyAppBuilder appBuilder, int order = 10_000, int inputOrder = -10_000, - bool clearTarget = false) + bool clearTarget = false, + int updateOrder = 10_000, + Func? viewportSource = null) { - return UseUi(appBuilder, default, order, inputOrder, clearTarget); + return UseUi(appBuilder, default, order, inputOrder, clearTarget, updateOrder, viewportSource); } public static PixelyAppBuilder UseUi( @@ -27,17 +29,30 @@ public static PixelyAppBuilder UseUi( ViewScope viewScope, int order = 10_000, int inputOrder = -10_000, - bool clearTarget = false) + bool clearTarget = false, + int updateOrder = 10_000, + Func? viewportSource = null) { - return UseUi(appBuilder, viewScope, order, inputOrder, clearTarget); + return UseUi(appBuilder, viewScope, order, inputOrder, clearTarget, updateOrder, viewportSource); } + /// + /// 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. + /// + /// + /// The size to lay out against, when the render context draws into something other than the + /// window. Defaults to the window's render size, which is what the swapchain is. + /// public static PixelyAppBuilder UseUi( this PixelyAppBuilder appBuilder, ViewScope viewScope, int order = 10_000, int inputOrder = -10_000, - bool clearTarget = false) + bool clearTarget = false, + int updateOrder = 10_000, + Func? viewportSource = null) where TRenderContext : IRenderContext { ArgumentNullException.ThrowIfNull(appBuilder); @@ -71,6 +86,12 @@ public static PixelyAppBuilder UseUi( provider.GetRequiredService(), provider.GetRequiredService())); + appBuilder.AddSingleton(provider => + { + (UiRoot root, Window window) = ResolveUpdateTargets(provider, viewScope); + return new UiUpdateSystem(root, viewportSource ?? (() => WindowViewport(window)), () => window.IsVisible, updateOrder); + }); + appBuilder.AddSingleton, UiRenderer>(provider => UiRenderer.Create( ScopedUiRoot.GetRequired(provider, viewScope).Root, @@ -86,6 +107,26 @@ public static PixelyAppBuilder UseUi( return appBuilder; } + /// + /// 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. + /// + 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); + } + /// /// 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. diff --git a/src/Pixely.Ui/UiRenderer.cs b/src/Pixely.Ui/UiRenderer.cs index 844a3288..0b7b2c2e 100644 --- a/src/Pixely.Ui/UiRenderer.cs +++ b/src/Pixely.Ui/UiRenderer.cs @@ -7,8 +7,10 @@ namespace Pixely.Ui; /// -/// Paints a 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 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 , which is why this holds a source +/// rather than the root. /// internal sealed class UiRenderer : IRenderer, IDisposable where TRenderContext : IRenderContext @@ -27,7 +29,7 @@ internal sealed class UiRenderer : IRenderer, 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 : IRenderer, 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 : IRenderer, ID /// so that constructing a renderer is assignment only. /// internal static UiRenderer Create( - UiRoot root, + IUiPaintSource source, ViewScope viewScope, int order, bool clearTarget, @@ -101,18 +104,18 @@ internal static UiRenderer Create( gpuDevice.CreateColorTargetTexture(renderSize, colorTargetFormat), colorTargetFormat); - return new UiRenderer(root, viewScope, order, clearTarget, gpuDevice, resources); + return new UiRenderer(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); } + /// + /// 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. + /// + internal static bool IsStale(Vector2Int paintedViewportSize, Vector2Int viewportSize, Vector2Int target) + { + return paintedViewportSize != viewportSize || paintedViewportSize != target; + } + + /// + /// 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. + /// + 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 instructions = _root.Instructions; - IReadOnlyList batches = _root.Batches; + IReadOnlyList instructions = _source.Instructions; + IReadOnlyList batches = _source.Batches; if (instructions.Count == 0) { diff --git a/src/Pixely.Ui/UiRoot.cs b/src/Pixely.Ui/UiRoot.cs index ed92d466..cd2b232a 100644 --- a/src/Pixely.Ui/UiRoot.cs +++ b/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. /// -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; + /// + /// 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. + /// + internal ulong BuildVersion { get; private set; } + internal IReadOnlyList Instructions => _paintContext.Instructions; /// Instruction runs sharing a texture and a clip, in paint order. @@ -563,6 +570,7 @@ private bool Rebuild() _layersChanged = false; IsPaintDirty = false; PaintedViewportSize = viewportSize; + BuildVersion++; return true; } @@ -611,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 IUiPaintSource.Instructions => Instructions; + IReadOnlyList IUiPaintSource.Batches => Batches; + Vector2Int IUiPaintSource.PaintedViewportSize => PaintedViewportSize; + Vector2Int IUiPaintSource.ViewportSize => ViewportSize; + ulong IUiPaintSource.BuildVersion => BuildVersion; } diff --git a/src/Pixely.Ui/UiUpdateSystem.cs b/src/Pixely.Ui/UiUpdateSystem.cs new file mode 100644 index 00000000..4803fecd --- /dev/null +++ b/src/Pixely.Ui/UiUpdateSystem.cs @@ -0,0 +1,52 @@ +namespace Pixely.Ui; + +/// +/// 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. +/// +internal sealed class UiUpdateSystem : IUpdatable, IOrderable +{ + private readonly UiRoot _root; + private readonly Func _viewport; + private readonly Func _isVisible; + + /// + /// 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. + /// + internal UiUpdateSystem(UiRoot root, Func viewport, Func 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(); + } +} diff --git a/tests/Pixely.Ui.Tests/BuildViewportTests.cs b/tests/Pixely.Ui.Tests/BuildViewportTests.cs index 3010e27a..504be819 100644 --- a/tests/Pixely.Ui.Tests/BuildViewportTests.cs +++ b/tests/Pixely.Ui.Tests/BuildViewportTests.cs @@ -40,6 +40,26 @@ public void ACallbackMovingTheViewport_LeavesTheRootNeedingAnotherBuild() }); } + [Test] + public void BuildVersion_RisesOnABuildAndStandsStillWhenNothingChanged() + { + UiRoot root = new(); + root.SetViewportSize(new Vector2Int(320, 240)); + root.AddLayer(new Element { Width = Sizing.Fixed(10), Height = Sizing.Fixed(10) }); + + ulong beforeFirstBuild = root.BuildVersion; + root.Update(); + ulong afterFirstBuild = root.BuildVersion; + root.Update(); + + Assert.Multiple(() => + { + Assert.That(beforeFirstBuild, Is.EqualTo(0ul), "an unbuilt root has no build to report"); + Assert.That(afterFirstBuild, Is.EqualTo(1ul)); + Assert.That(root.BuildVersion, Is.EqualTo(afterFirstBuild), "a clean root does not build again"); + }); + } + /// /// A built root with the pointer parked over a target that resizes the viewport when it is told /// the pointer left. Growing the spacer is what pushes the target out from under the pointer. diff --git a/tests/Pixely.Ui.Tests/UiRendererDecisionTests.cs b/tests/Pixely.Ui.Tests/UiRendererDecisionTests.cs new file mode 100644 index 00000000..d030ee16 --- /dev/null +++ b/tests/Pixely.Ui.Tests/UiRendererDecisionTests.cs @@ -0,0 +1,44 @@ +using Pixely.RenderOrchestration; + +namespace Pixely.Ui.Tests; + +/// +/// The two decisions the renderer makes about instructions it did not build. Tested as predicates +/// because the renderer itself needs a GPU; what they do not cover is that the renderer never +/// starts a build, which makes a compile-time fact instead. +/// +public class UiRendererDecisionTests +{ + private static readonly Vector2Int _viewport = new(320, 240); + private static readonly Vector2Int _otherViewport = new(640, 480); + + [Test] + public void InstructionsBuiltForTheTargetBeingDrawnInto_AreCurrent() + { + Assert.That(UiRenderer.IsStale(_viewport, _viewport, _viewport), Is.False); + } + + [Test] + public void InstructionsBuiltForAViewportTheRootHasSinceLeft_AreStale() + { + Assert.That(UiRenderer.IsStale(_viewport, _otherViewport, _viewport), Is.True); + } + + [Test] + public void InstructionsBuiltForAViewportThatIsNotTheTarget_AreStale() + { + Assert.That(UiRenderer.IsStale(_viewport, _viewport, _otherViewport), Is.True); + } + + [Test] + public void ABuildThisRendererMissed_StillRepaints() + { + Assert.Multiple(() => + { + Assert.That(UiRenderer.NeedsRepaint(7, 5, false), Is.True, "two builds behind, not one"); + Assert.That(UiRenderer.NeedsRepaint(5, 5, false), Is.False); + Assert.That(UiRenderer.NeedsRepaint(5, 5, true), Is.True, "a resized texture holds nothing yet"); + Assert.That(UiRenderer.NeedsRepaint(0, 0, true), Is.True, "which is what paints the first frame"); + }); + } +} diff --git a/tests/Pixely.Ui.Tests/UiUpdateSystemTests.cs b/tests/Pixely.Ui.Tests/UiUpdateSystemTests.cs new file mode 100644 index 00000000..73ccd14c --- /dev/null +++ b/tests/Pixely.Ui.Tests/UiUpdateSystemTests.cs @@ -0,0 +1,151 @@ +using System.Runtime.CompilerServices; +using System.Reflection; +using Pixely.App; +using Pixely.DependencyInjection; + +namespace Pixely.Ui.Tests; + +/// +/// The build runs in the update phase. What matters is that it happens at all, that it does not +/// happen for a window nothing will draw, and that each window's system drives its own root. +/// +public class UiUpdateSystemTests +{ + [Test] + public void Updating_SetsTheViewportFromItsSourceAndBuilds() + { + UiRoot root = new(); + root.AddLayer(new Element { Width = Sizing.Fixed(10), Height = Sizing.Fixed(10) }); + + UiUpdateSystem system = new(root, () => new Vector2Int(640, 480), () => true, 0); + system.Update(); + + Assert.Multiple(() => + { + Assert.That(root.ViewportSize, Is.EqualTo(new Vector2Int(640, 480))); + Assert.That(root.PaintedViewportSize, Is.EqualTo(new Vector2Int(640, 480)), "and it built against it"); + Assert.That(root.BuildVersion, Is.EqualTo(1ul)); + }); + } + + [Test] + public void UpdatingAHiddenWindow_BuildsNothingAndDoesNotEvenAskForTheSize() + { + UiRoot root = new(); + bool sizeRead = false; + + UiUpdateSystem system = new( + root, + () => + { + sizeRead = true; + return new Vector2Int(640, 480); + }, + () => false, + 0); + + system.Update(); + + Assert.Multiple(() => + { + Assert.That(root.BuildVersion, Is.EqualTo(0ul)); + Assert.That(sizeRead, Is.False, "a hidden window does not pay for a size call it will not use"); + }); + } + + [TestCase(0, 0)] + [TestCase(640, 0)] + [TestCase(0, 480)] + [TestCase(-1, 480)] + public void UpdatingAgainstAnEmptyViewport_BuildsNothing(int width, int height) + { + UiRoot root = new(); + root.AddLayer(new Element { Width = Sizing.Fixed(10), Height = Sizing.Fixed(10) }); + + UiUpdateSystem system = new(root, () => new Vector2Int(width, height), () => true, 0); + system.Update(); + + Assert.Multiple(() => + { + Assert.That(root.BuildVersion, Is.EqualTo(0ul)); + Assert.That(root.ViewportSize, Is.EqualTo(new Vector2Int(0, 0)), "and the viewport is left alone rather than invalidating every layer"); + }); + } + + [Test] + public void TheUpdatablesRunInOrder() + { + List calls = new(); + + PixelyAppBuilder builder = new(); + builder.AddSingleton(_ => new UiUpdateSystem(new UiRoot(), () => Recording(calls, "late"), () => false, 10)); + builder.AddSingleton(_ => new UiUpdateSystem(new UiRoot(), () => Recording(calls, "early"), () => true, -10)); + + ServiceProvider provider = builder.BuildServiceProvider(); + + foreach (IUpdatable updatable in provider.GetRequiredService>()) + { + updatable.Update(); + } + + Assert.That(calls, Is.EqualTo(new[] { "early" }), "the visible one ran, and the hidden one did not"); + } + + [Test] + public void EachViewScope_ResolvesItsOwnRootAndWindow() + { + ViewScope first = new(1); + ViewScope second = new(2); + + Window firstWindow = UninitialisedWindow(first, 1); + Window secondWindow = UninitialisedWindow(second, 2); + UiRoot firstRoot = new(); + UiRoot secondRoot = new(); + + PixelyAppBuilder builder = new(); + builder.AddRegistry(); + builder.AddSingleton(firstWindow); + builder.AddSingleton(secondWindow); + builder.AddSingleton(_ => new ScopedUiRoot(first, firstRoot)); + builder.AddSingleton(_ => new ScopedUiRoot(second, secondRoot)); + + ServiceProvider provider = builder.BuildServiceProvider(); + + (UiRoot resolvedFirstRoot, Window resolvedFirstWindow) = UiExtensions.ResolveUpdateTargets(provider, first); + (UiRoot resolvedSecondRoot, Window resolvedSecondWindow) = UiExtensions.ResolveUpdateTargets(provider, second); + + Assert.Multiple(() => + { + Assert.That(resolvedFirstRoot, Is.SameAs(firstRoot)); + Assert.That(resolvedFirstWindow, Is.SameAs(firstWindow)); + Assert.That(resolvedSecondRoot, Is.SameAs(secondRoot)); + Assert.That(resolvedSecondWindow, Is.SameAs(secondWindow)); + }); + } + + private static Vector2Int Recording(List calls, string name) + { + calls.Add(name); + return new Vector2Int(0, 0); + } + + /// + /// A window that never reaches SDL. Every property this test touches is set here, which is what + /// keeps the scope lookup testable without a display. + /// + private static Window UninitialisedWindow(ViewScope viewScope, uint sdlId) + { + Window window = (Window)RuntimeHelpers.GetUninitializedObject(typeof(Window)); + SetBackingField(window, nameof(Window.ViewScope), viewScope); + + // The window registry keys on this, and two windows sharing an id is what it refuses. + SetBackingField(window, nameof(Window.SdlId), sdlId); + return window; + } + + private static void SetBackingField(Window window, string propertyName, T value) + { + FieldInfo field = typeof(Window).GetField($"<{propertyName}>k__BackingField", BindingFlags.Instance | BindingFlags.NonPublic)!; + field.SetValue(window, value); + } +} From 4938702613dd32f575e3f7d1c6dccd4c7e8eb4bf Mon Sep 17 00:00:00 2001 From: botoddly <250804054+botoddly@users.noreply.github.com> Date: Wed, 9 Sep 2026 09:44:05 +0200 Subject: [PATCH 3/5] Make the update system ordering test observe the order it claims to test --- tests/Pixely.Ui.Tests/UiUpdateSystemTests.cs | 21 ++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/tests/Pixely.Ui.Tests/UiUpdateSystemTests.cs b/tests/Pixely.Ui.Tests/UiUpdateSystemTests.cs index 73ccd14c..e7d0bda9 100644 --- a/tests/Pixely.Ui.Tests/UiUpdateSystemTests.cs +++ b/tests/Pixely.Ui.Tests/UiUpdateSystemTests.cs @@ -73,13 +73,15 @@ public void UpdatingAgainstAnEmptyViewport_BuildsNothing(int width, int height) } [Test] - public void TheUpdatablesRunInOrder() + public void TheSystemsRunInOrderAndJoinTheUpdatablesByBeingRegistered() { List calls = new(); + // Registered late first, so passing cannot be an accident of registration order — which is + // not a guarantee anyway, since the registry sorts with an unstable sort. PixelyAppBuilder builder = new(); - builder.AddSingleton(_ => new UiUpdateSystem(new UiRoot(), () => Recording(calls, "late"), () => false, 10)); - builder.AddSingleton(_ => new UiUpdateSystem(new UiRoot(), () => Recording(calls, "early"), () => true, -10)); + builder.AddSingleton(_ => new UiUpdateSystem(new UiRoot(), () => Recording(calls, "late"), () => RecordingVisible(calls, "late visible"), 10)); + builder.AddSingleton(_ => new UiUpdateSystem(new UiRoot(), () => Recording(calls, "early"), () => RecordingVisible(calls, "early visible"), -10)); ServiceProvider provider = builder.BuildServiceProvider(); @@ -88,7 +90,7 @@ public void TheUpdatablesRunInOrder() updatable.Update(); } - Assert.That(calls, Is.EqualTo(new[] { "early" }), "the visible one ran, and the hidden one did not"); + Assert.That(calls, Is.EqualTo(new[] { "early visible", "early", "late visible", "late" })); } [Test] @@ -123,12 +125,23 @@ public void EachViewScope_ResolvesItsOwnRootAndWindow() }); } + /// + /// Records that a delegate was reached, and answers so that nothing is built: a zero viewport + /// for the size, and visible for the visibility, so both delegates of both systems are reached. + /// private static Vector2Int Recording(List calls, string name) { calls.Add(name); return new Vector2Int(0, 0); } + /// + private static bool RecordingVisible(List calls, string name) + { + calls.Add(name); + return true; + } + /// /// A window that never reaches SDL. Every property this test touches is set here, which is what /// keeps the scope lookup testable without a display. From d949ae669727d496b75efcba58fc52fc4833d5e1 Mon Sep 17 00:00:00 2001 From: botoddly <250804054+botoddly@users.noreply.github.com> Date: Thu, 10 Sep 2026 07:42:29 +0200 Subject: [PATCH 4/5] Drop the viewportSource parameter from UseUi --- docs/ui.md | 6 +----- src/Pixely.Ui/UiExtensions.cs | 19 ++++++------------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/docs/ui.md b/docs/ui.md index ace03a14..63a777f6 100644 --- a/docs/ui.md +++ b/docs/ui.md @@ -67,11 +67,7 @@ The viewport event is raised by `SetViewportSize`, which the same system calls i 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. -If the render context draws into something other than the window — a same-format colour target of a different size — pass `viewportSource` so the build lays out against that instead of the window: - -```csharp -builder.UseUi(default, viewportSource: () => new Vector2Int(640, 360)); -``` +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 diff --git a/src/Pixely.Ui/UiExtensions.cs b/src/Pixely.Ui/UiExtensions.cs index ea659034..0d06d116 100644 --- a/src/Pixely.Ui/UiExtensions.cs +++ b/src/Pixely.Ui/UiExtensions.cs @@ -18,10 +18,9 @@ public static PixelyAppBuilder UseUi( int order = 10_000, int inputOrder = -10_000, bool clearTarget = false, - int updateOrder = 10_000, - Func? viewportSource = null) + int updateOrder = 10_000) { - return UseUi(appBuilder, default, order, inputOrder, clearTarget, updateOrder, viewportSource); + return UseUi(appBuilder, default, order, inputOrder, clearTarget, updateOrder); } public static PixelyAppBuilder UseUi( @@ -30,10 +29,9 @@ public static PixelyAppBuilder UseUi( int order = 10_000, int inputOrder = -10_000, bool clearTarget = false, - int updateOrder = 10_000, - Func? viewportSource = null) + int updateOrder = 10_000) { - return UseUi(appBuilder, viewScope, order, inputOrder, clearTarget, updateOrder, viewportSource); + return UseUi(appBuilder, viewScope, order, inputOrder, clearTarget, updateOrder); } /// @@ -41,18 +39,13 @@ public static PixelyAppBuilder UseUi( /// UI builds after ordinary order-0 game systems and views sync against the state this frame /// produced. Equal orders are unspecified, not registration order. /// - /// - /// The size to lay out against, when the render context draws into something other than the - /// window. Defaults to the window's render size, which is what the swapchain is. - /// public static PixelyAppBuilder UseUi( this PixelyAppBuilder appBuilder, ViewScope viewScope, int order = 10_000, int inputOrder = -10_000, bool clearTarget = false, - int updateOrder = 10_000, - Func? viewportSource = null) + int updateOrder = 10_000) where TRenderContext : IRenderContext { ArgumentNullException.ThrowIfNull(appBuilder); @@ -89,7 +82,7 @@ public static PixelyAppBuilder UseUi( appBuilder.AddSingleton(provider => { (UiRoot root, Window window) = ResolveUpdateTargets(provider, viewScope); - return new UiUpdateSystem(root, viewportSource ?? (() => WindowViewport(window)), () => window.IsVisible, updateOrder); + return new UiUpdateSystem(root, () => WindowViewport(window), () => window.IsVisible, updateOrder); }); appBuilder.AddSingleton, UiRenderer>(provider => From ce2ab87f95c738674469d9cacd0734b2b4adfa3a Mon Sep 17 00:00:00 2001 From: botoddly <250804054+botoddly@users.noreply.github.com> Date: Thu, 10 Sep 2026 08:04:36 +0200 Subject: [PATCH 5/5] Group the UseUi phase orders together and name the render one renderOrder --- docs/ui.md | 4 +++- src/Pixely.Ui/UiExtensions.cs | 26 ++++++++++++++------------ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/docs/ui.md b/docs/ui.md index 63a777f6..907fd8ad 100644 --- a/docs/ui.md +++ b/docs/ui.md @@ -57,7 +57,9 @@ A layer does not block the pointer by being on top. Only an `IPointerTarget` is `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. -`updateOrder` says when, relative to the other updatables. Lower runs first, and 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. +`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); diff --git a/src/Pixely.Ui/UiExtensions.cs b/src/Pixely.Ui/UiExtensions.cs index 0d06d116..6df30bf4 100644 --- a/src/Pixely.Ui/UiExtensions.cs +++ b/src/Pixely.Ui/UiExtensions.cs @@ -15,37 +15,39 @@ public static class UiExtensions /// 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, - int updateOrder = 10_000) + bool clearTarget = false) { - return UseUi(appBuilder, default, order, inputOrder, clearTarget, updateOrder); + return UseUi(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, - int updateOrder = 10_000) + bool clearTarget = false) { - return UseUi(appBuilder, viewScope, order, inputOrder, clearTarget, updateOrder); + return UseUi(appBuilder, viewScope, renderOrder, updateOrder, inputOrder, clearTarget); } + /// When the UI is drawn relative to the other renderers, lower first. Defaults late, so it draws over the game. /// /// 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. /// + /// When the UI sees input relative to the other subscribers, lower first. Defaults early, so it takes events before the game does. 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, - int updateOrder = 10_000) + bool clearTarget = false) where TRenderContext : IRenderContext { ArgumentNullException.ThrowIfNull(appBuilder); @@ -89,7 +91,7 @@ public static PixelyAppBuilder UseUi( UiRenderer.Create( ScopedUiRoot.GetRequired(provider, viewScope).Root, viewScope, - order, + renderOrder, clearTarget, provider.GetRequiredService(), provider.GetRequiredService(),