Skip to content

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

Merged
stanoddly merged 5 commits into
mainfrom
ui-update-phase
Sep 10, 2026
Merged

Build the Pixely.Ui tree in the update phase instead of during rendering#481
stanoddly merged 5 commits into
mainfrom
ui-update-phase

Conversation

@botoddly

@botoddly botoddly commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

UiRenderer.Render built the element tree. Two lines of it — _root.SetViewportSize(...) and _root.Update() — and both reach application code: pointer enter and leave as layout moves under a stationary pointer, focus lost when a focused element leaves the tree, FocusChanged through to ITextInputService.Start/Stop (an SDL call), the viewport event, and every custom MeasureContent, ArrangeContent and Drawable.Paint in the tree.

RenderCoordinator.Execute runs every renderer for a window in Order sequence over one shared render context and command buffer. So that application code ran between other renderers' Render calls, with a live CommandBuffer. A renderer that reads domain data is entitled to assume it holds still for the frame, and this broke that for every other renderer in the window. What masked it is UseUi's default order: 10_000, which happens to put the UI last — a default argument, not an invariant.

Verified before fixing: three temporary tests against UiRoot, making the exact calls Render made, showed a stationary pointer receiving OnPointerLeave from a sibling's height changing, SetViewportSize notifying synchronously, and hiding a focused element raising FocusChanged(null).

Pixely.Pencuil never had this — its build is PencilSystem.Update, and PencuilRenderer only reads completed instruction buffers.

The build moves to the update phase

UiUpdateSystem : IUpdatable, IOrderable is what UseUi now registers to drive the root. It takes Func<Vector2Int> and Func<bool> rather than a Window, because RenderSizeInPixels and IsVisible are non-virtual SDL calls and a system holding a window cannot be constructed in a test — the same objection that ruled out folding the build into UiInputSystem, which is untouched.

It skips hidden windows before reading the size, and skips a zero-or-negative viewport. Both are deliberate: today a hidden window never builds because the only caller of Update is a renderer RenderCoordinator already skipped, and moving the build out of render removes that protection.

The renderer can no longer start a build

UiRenderer holds IUiPaintSource instead of UiRoot — the completed instructions and batches, the viewport they were built for, the current viewport, and a build counter. No Update, no SetViewportSize. This is what enforces the boundary: not a convention and not a test, but the renderer's dependency having nothing on it to call. Same-assembly code can still cast back to UiRoot on purpose; the point is that no ordinary edit reaches a build by accident.

All five members are forwarded explicitly from UiRoot. Four of them are internal, an internal member cannot implicitly implement an interface one, and widening is unavailable because PaintInstruction and PaintBatch are internal types.

UiRoot.BuildVersion is zero-based and rises with every completed build; each renderer tracks the version it last painted, so a renderer that missed a build still repaints rather than depending on having been the caller that triggered it.

A pre-existing bug, fixed first

Rebuild derived its viewport from _viewportSize at entry, ran callbacks, then recorded PaintedViewportSize = _viewportSize — so a callback calling SetViewportSize in between made the root claim the geometry it had just laid out was built for a viewport it never saw. It now snapshots at entry and records the snapshot. Independent of the move, but the new renderer guard would have exposed it.

API

UseUi now takes one order per phase, grouped, with clearTarget after them:

UseUi(renderOrder: 10_000, updateOrder: 10_000, inputOrder: -10_000, clearTarget: false)

updateOrder is new. order is renamed renderOrder, because with three of them the bare name no longer says which phase it governs. This is a break for positional callers: the old order was order, inputOrder, clearTarget, so UseUi(scope, 100, -100, true) no longer compiles — -100 lands on updateOrder and true on inputOrder. Named arguments are unaffected apart from orderrenderOrder. All three tutorials call UseUi() with no arguments.

The break: a custom IRenderContext whose colour target is the same format as the window but a different size works today, because Render read ColorTarget.Size and laid out against it. The update phase has no render context, so the build now lays out against the window and the renderer refuses to draw it into a differently sized target — that configuration goes blank.

An earlier revision of this PR added a viewportSource delegate to UseUi so the caller could restate the target size by hand. That is dropped: it is public API added for a configuration nothing uses, it can be got wrong as silently as the break it patches, and the size is something the framework already knows and should supply itself. A follow-up proposal covers making it work by default. All three tutorials use parameterless UseUi() over BasicRenderContext, whose target is the swapchain, so the default path is unaffected.

No generic no-scope overload was added: it would make UseUi<MyContext>(default) ambiguous between ViewScope and int.

Behaviour to accept

  • A resize landing between the update phase and render clears the UI for one frame; the next update rebuilds. Resize is seamless today, so this is a real regression in smoothness and the price of the boundary. Both staleness checks are what make it a blank frame rather than the previous frame's geometry stretched into a new texture.
  • The UI now builds on a frame where swapchain acquisition fails, and on the frame a quit is requested — both return before Render, but the update phase has already run.
  • A system running after UiUpdateSystem that dirties the UI is shown one frame later. That is what updateOrder means.
  • Equal updateOrder values are unspecified, not registration order: ServiceRegistry sorts with List.Sort, which is unstable.
  • A hidden window shown after its update system already ran can present the old texture, or clear, until the next update.

Multi-window

Each UseUi(viewScope, ...) registers its own root, input system, update system and renderer; ScopedUiRoot.GetRequired throws if a scope is configured twice; RenderCoordinator filters renderers by scope. PixelyApp.Update iterates every IUpdatable with no scope filter, which is correct — scope is bound at registration and never consulted per frame.

ResolveUpdateTargets exists so that is testable. The system itself holds only closures, and nothing outside can tell which window they captured without calling them (SDL) or reflecting over compiler-generated fields; returning the resolved (UiRoot, Window) pair makes the lookup observable headlessly. The hazard it guards is real and silent: GetWindow's viewScope parameter is defaulted, so dropping it compiles and binds every window's UI to the first one.

Tests

New: the Rebuild viewport snapshot and its recovery; BuildVersion semantics; the staleness and repaint predicates including the missed-build case; the update system building, and not building for a hidden window or an empty viewport; ordering; per-scope root and window resolution.

Reviewed by codex across four rounds — the plan through three, the implementation through one, which found the ordering test asserting something that held regardless of order.

Fixes #479

@stanoddly
stanoddly enabled auto-merge (squash) September 10, 2026 06:07
@stanoddly
stanoddly merged commit 487f5d0 into main Sep 10, 2026
1 check passed
@stanoddly
stanoddly deleted the ui-update-phase branch September 10, 2026 06:12
@github-actions

Copy link
Copy Markdown

✅ Development package Pixely 0.0.34 published successfully.

Workflow run (attempt 1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pixely.Ui builds its element tree inside IRenderer.Render

2 participants