Skip to content

Pixely.Ui builds its element tree inside IRenderer.Render #479

Description

@botoddly

Problem

UiRenderer<TRenderContext>.Render builds the UI tree, so application code runs inside IRenderer<T>.Render (src/Pixely.Ui/UiRenderer.cs:141-148):

_root.SetViewportSize(new Vector2Int(targetSize.Width, targetSize.Height));
bool rebuilt = _root.Update();

Both lines reach application callbacks:

  • SetViewportSize raises ViewportChanged (UiRoot.cs:477-492).
  • Update()Rebuild()_pointerRouter.Revalidate() (UiRoot.cs:539) delivers IPointerTarget.OnPointerEnter / OnPointerLeave / OnPointerCancel. This is Revalidate's stated purpose: layout moving under a pointer that did not move.
  • Update()Rebuild()_focusRouter.Revalidate() (UiRoot.cs:543) delivers IFocusTarget.OnFocusLost.
  • Update()'s finallyReportFocus()FocusChangedUiInputSystem.SetTextInputActiveITextInputService.Start/Stop, an SDL platform call issued with a live CommandBuffer.
  • Rebuild() also runs every application extension point in the tree: MeasureContent / ArrangeContent on custom elements and layouts (Element.cs:377-393) and Drawable.Paint on custom drawables (Drawable.cs:7-15).

Verified with three throwaway tests against UiRoot (the exact calls Render makes), all green: parking a pointer over a target and changing a sibling's height then calling Update() delivers OnPointerLeave with no pointer event involved; SetViewportSize raises ViewportChanged synchronously; hiding a focused element and calling Update() raises FocusChanged(null).

RenderCoordinator<TRenderContext>.Execute runs every renderer for a window in Order sequence over one shared render context and command buffer, then submits (src/Pixely/RenderOrchestration/RenderCoordinator.cs:33-46). Application code running inside one renderer's Render therefore executes between other renderers' Render calls. A renderer cannot assume the domain data it reads is stable for the frame, which is an assumption every renderer is entitled to make.

Pixely.Pencuil does not have this problem: the build is PencilSystem.Update, an IUpdatable, and PencuilRenderer only reads completed instruction buffers.

Why it is not visible today

UseUi defaults to order: 10_000, which puts the UI last, so mutations land after every other renderer has drawn and surface as a one-frame tear rather than a mid-frame inconsistency. Register a renderer above 10 000, or pass a lower order, and it stops being hidden. A default argument is not an invariant.

This is not corruption inside UiRenderer itself. Rebuild() runs the callbacks before layer.Paint(...) and re-collects hit targets when they dirty layout. It does not re-run measure/arrange, so a callback that invalidates measure yields one frame at stale layout, self-corrected next frame. The defect is the contract violation across renderers.

Related latent bug

Independent of the above, Rebuild() derives its viewport from _viewportSize at entry (UiRoot.cs:523-524), runs callbacks at :539 and :543, then records PaintedViewportSize = _viewportSize at :562. A callback calling SetViewportSize in between makes the root claim geometry was built for a viewport it never saw. The current target-size guard masks this; the change below would expose it, so it is fixed first.

Proposed direction

  1. Rebuild snapshots the viewport at entry and records the snapshot. NeedsUpdate() already returns true on PaintedViewportSize != _viewportSize (UiRoot.cs:594-599), so a callback that moved it leaves the root dirty for the next update.
  2. New internal sealed class UiUpdateSystem : IUpdatable, IOrderable, registered per scope by UseUi, which sets the viewport and calls Update(). It takes Func<Vector2Int> and Func<bool> rather than a Window, because RenderSizeInPixels and IsVisible are non-virtual SDL calls and the system has to be constructible headlessly — the same reason it is not folded into UiInputSystem, which UiInputSystemTests.cs:175 constructs with a Func<Size<uint>>.
  3. Not merged into UiRenderer: IRenderer<T> : IOrderable (IRenderer.cs:3) and the IUpdatable registry sorts on that same property (PixelyAppBuilder.cs:21-27), so one class would collapse render order and update order into one number. They are unrelated decisions.
  4. New internal IUiPaintSource exposing instructions, batches, both viewport sizes and a BuildVersion. UiRenderer holds that instead of UiRoot, so no ordinary edit can reach Update again. Implemented explicitly on UiRoot, since the members are internal and PaintInstruction / PaintBatch are internal types.
  5. Renderer keeps both staleness checks — painted viewport against current viewport, and against the render target — and repaints on BuildVersion rather than on having been the caller.
  6. UseUi gains updateOrder (default 10 000) and an optional viewportSource, both appended after clearTarget so positional callers keep working. No generic no-scope overload is added: it would make UseUi<MyContext>(default) ambiguous between ViewScope and int.

Multi-window is preserved: each scope registers its own root, input system, update system and renderer; scope is bound at registration and never consulted per frame. Equal updateOrder values are unordered — ServiceRegistry sorts with List.Sort (ServiceRegistry.cs:92-105), which is unstable — so this must not be documented as registration order.

Accepted behaviour changes

  • A resize landing between update and render clears the UI for one frame. Resizing is seamless today, so this is a real regression in smoothness and is the price of the phase boundary.
  • Breaking: a custom IRenderContext whose colour target is the same format as the window but a different size works today and stops working unless it passes viewportSource.
  • The UI builds on a frame where swapchain acquisition fails, and on the frame a quit is requested (PixelyApp.cs:37-47); today both skip it.
  • A system running after UiUpdateSystem has its UI changes picked up a frame late. That is what updateOrder means.
  • A hidden or zero-area window does not build, preserving today's behaviour.

Acceptance criteria

  • UiRenderer.Render calls neither SetViewportSize nor Update, enforced by it holding IUiPaintSource.
  • The tree is built once per frame in the update phase, per ViewScope.
  • Rebuild records the viewport it actually laid out against.
  • A test proves each ViewScope resolves its own root and window through the real registration path, headlessly.
  • Multi-window and the existing tests/Pixely.Ui.Tests suite pass unchanged.
  • docs/ui.md documents the update phase, updateOrder, and when viewportSource is required.

Sibling of #449, which is the same missing IOrderable on Pencuil's build system.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions