Skip to content

Perf/scoped UI clean and styles#774

Open
Colorfingers wants to merge 2 commits into
KaijuEngine:masterfrom
hapticMonkey:perf/scoped-ui-clean-and-styles
Open

Perf/scoped UI clean and styles#774
Colorfingers wants to merge 2 commits into
KaijuEngine:masterfrom
hapticMonkey:perf/scoped-ui-clean-and-styles

Conversation

@Colorfingers
Copy link
Copy Markdown
Contributor

@Colorfingers Colorfingers commented May 3, 2026

Kaiju PR: Scope UI clean and CSS re-application to subtree

Branch: perf/scoped-ui-clean-and-styles (off latest master)
Commit: 311dd482 — "Scope UI clean and CSS re-application to subtree"
Files changed: 4 (+127 / -38)

The bug, plainly

Kaiju's UI engine has a single design choice that becomes a problem at scale: any change anywhere in a document re-lays-out the entire document. That's not three different bugs — it's one architecture decision implemented in three places, and you have to fix all three for any of them to matter.


The three changes, and why each one matters

1. UI.Clean() was global, not local

UI.Clean() reads like a method on one element ("clean this thing"). It isn't. The first thing it did was root := ui.rootUI() — walk up to the document root — then iterate every active element in the entire document, re-running layout, scissor, and render on each.

So someElement.Clean() and documentRoot.Clean() did the exact same work. The receiver was decoration.

Why fixing this is safe: I audited the layout/scissor/render pipeline for cross-element reads.

  • Layout reads only the element's own pixel size and its own children (no sibling reads).
  • Scissor walks UP to ancestors only — children are clipped by parents, but parents are unchanged when you clean a subtree.
  • Render is per-element.

There are no horizontal dependencies. So a subtree clean produces correct output for the subtree without touching unrelated panels.

The fix is one line: root := ui instead of root := ui.rootUI(). The unused rootUI() helper is removed.

What's preserved: the per-frame Manager.update loop calls Clean on root UIs, and root.Clean() still walks the whole tree. So whole-doc cleans still happen when they should.


2. cleanIfNeeded() defeated #1

cleanIfNeeded() is what the per-frame loop calls. It checked anyChildDirty() — recursive: "is anything anywhere below me dirty?" — and if yes, called Clean() on root.

So even if I fixed #1 (subtree-scoped Clean), the per-frame path was still doing root.Clean every time anything was dirty. Same end result.

setDirtyInternal (the function that marks things dirty) cascades dirty downward to children but never upward to parents. So the topmost dirty element on any branch is exactly the element that originally became dirty (or its ancestor, if the change was at the root). Cleaning from there covers every dirty element exactly once.

The fix: walk down from root. At the first dirty element on each branch, Clean that subtree and stop descending. Subtrees with no dirty elements are never visited.


3. Stylizer.ApplyStyles re-styles the whole document

CSS rule application happens in two places: when the document loads (one-shot, fine) and whenever an element's id, class, parent, or position in the tree changes (frequent, hot path).

The hot-path methods — SetElementId, SetElementClasses, ChangeElementParent, DuplicateElement, DuplicateElementToParent, DuplicateElementRepeat, insertElementAt — all called Doc.ApplyStyles(), which walked every element in the document and re-evaluated every CSS rule against it. Each rule's processor (SetMargin, SetPadding, SetBackground, etc.) then called SetDirty on its target element.

Net effect: changing one element's class marked every element in the document dirty. Combined with the unscoped Clean, the next frame re-laid-out everything.

Even with fixes #1 and #2, this layer would keep dirtying the whole document and forcing whole-doc cleans on the next frame. So you have to fix this too.

The new Stylizer.ApplyStylesToElement(target) does the same selector-matching pass against the whole stylesheet (correct — selectors match wherever they match), but the clear and apply phases skip every element outside target's subtree. So unrelated elements never see ClearRules, never get a SetDirty call, never become dirty.

The hot-path mutators on Document switch from ApplyStyles to ApplyStylesToElement(elm). Doc.ApplyStyles(), the document-load setup, the window-resize handler, and RemoveElement keep using the doc-wide path because they're either one-shot or genuinely document-affecting.


Why all three are necessary

If you fix only #1: explicit someElement.Clean() calls scope correctly, but per-frame cleanIfNeeded still triggers whole-doc cleans whenever anything is dirty. No visible improvement during normal interaction.

If you fix only #2: per-frame cleans are scoped, but every CSS mutation still marks the whole doc dirty (via #3), so every frame still has dirty subtrees everywhere, so cleans still cover everything.

If you fix only #3: dirty state is now scoped, but cleanIfNeeded still walks from root via #2, and Clean still walks the whole doc via #1. Everything still re-lays-out.

The three changes are coupled: each one removes a different mechanism by which a single change cascades to the whole document. Together, a class change on one element re-styles only that element, marks only that subtree dirty, and the next frame's clean walks only that subtree. Apart, none of them have observable effect.


Caveat called out in the commit message

The scoped ApplyStylesToElement is correct under Kaiju's current selector grammar (id, class, tag, pseudo, attribute condition, descendant). If sibling combinators (+, ~) are added later, callers that previously relied on the doc-wide behavior will need to widen their scope — a sibling-targeted rule on element A would not re-evaluate when its sibling B mutates if we only re-style B's subtree.

Today this is a non-issue because the selector matcher in engine/ui/markup/css/reader.go doesn't parse those combinators. Worth noting in case it changes.


The proof

Discovered while profiling drag-drop in a heavily-populated downstream editor: dropping a model on the stage was triggering 18 separate Clean calls covering ~1500 elements — basically the entire UI — every drop. Icons flickered, panels went blank momentarily. After all three changes, a drop cleans only the affected subtrees (the new entity's hierarchy row + the details panel). Nothing else is touched.


Was this a downstream-only bug?

Worth challenging directly: was this caused by the downstream's GLB / spawn code, or is it Kaiju's?

The GLB code path on drop is dropContentspawnContentAtMousespawnModel → parse GLB, upload textures, attach mesh, push drawing. All of that is 3D scene work. None of it touches the UI document, marks UI elements dirty, or calls ApplyStyles. If that were the only thing happening, the trace would have shown zero Clean calls.

What actually drove the 18 Cleans, per the trace, was two trigger points — both of which are vanilla editor-UI code that exists in Kaiju too:

  1. DetailsUI.entitySelectedreload()dui.detailsArea.UI.Clean() (synchronous; line is in Kaiju's stage_workspace_details_ui.go).
  2. HierarchyUI.entityCreated and entitySelected calling Doc.DuplicateElement, Doc.SetElementId, Doc.SetElementClasses (each of which calls Doc.ApplyStyles() doc-wide).

What the downstream did was amplify the symptom, not cause it:

  • More populated docs (Stage workspace also surfaces a Catalog of content entries) → each whole-doc walk processes more elements.
  • Floating-panel chrome (titlebars, resize handles per panel) → more elements per workspace.
  • Catalog visible during drop → users see the icon flicker, where in Kaiju today the same invalidation happens but isn't looking at icon panels at the same time.

The underlying mechanism — Doc.ApplyStyles being doc-wide and cleanIfNeeded walking from root — is pure Kaiju, predates the downstream work, and would bite any consumer whose docs grew past a few hundred elements.

Kaiju's editor today is small enough that the user might not notice this overhead, which is why the design has survived. But it's the kind of thing that gets worse as docs grow, and the fix is local and safe enough that it's worth doing now rather than after someone hits the wall.


Files in this PR

File Change
src/engine/ui/ui.go Subtree-scoped UI.Clean; top-most-dirty cleanIfNeeded; removed unused rootUI and anyChildDirty helpers.
src/engine/ui/markup/document/html_style_interfaces.go Added ApplyStylesToElement to the Stylizer interface.
src/engine/ui/markup/css/reader.go Implemented Stylizer.ApplyStylesToElement.
src/engine/ui/markup/document/html_parser.go Added Doc.ApplyStylesToElement; switched SetElementId / SetElementClasses / ChangeElementParent / DuplicateElement / DuplicateElementToParent / DuplicateElementRepeat (and its WithoutApplyStyles variant) / insertElementAt to scoped variant.

No public API removed. One method added to a public interface (Stylizer.ApplyStylesToElement) — any external Stylizer implementations will need to implement it.

Three coupled changes that stop one event from re-laying-out the whole
document. Pure perf — no UX or API removed.

ui.Clean now walks only the receiver's subtree instead of always walking
from rootUI. Cross-element layout, scissor, and render have no sibling
dependencies (children read only their own layout and ancestor scissors,
both unchanged when cleaning a subtree), so a subtree clean produces
correct output without touching unrelated panels. The per-frame
Manager.update path still calls Clean on root UIs, so root.Clean still
covers the whole tree when needed. The unused rootUI helper is removed.

cleanIfNeeded now walks down to the topmost dirty element on each branch
and cleans only that subtree, instead of force-cleaning from root
whenever any descendant is dirty. setDirtyInternal cascades dirty
downward but never upward, so the topmost dirty element on each branch
covers every dirty element exactly once.

Stylizer.ApplyStylesToElement is added (clears + re-applies CSS rules
only inside target's subtree) and the Doc.* mutators that previously
re-styled the whole document now scope to the affected element:
SetElementId, SetElementClasses, ChangeElementParent, DuplicateElement,
DuplicateElementToParent, DuplicateElementRepeat (and the
WithoutApplyStyles variant that despite its name was applying), and
insertElementAt. Doc.ApplyStyles, the document-load setup, the
window-resize handler, and RemoveElement still use the doc-wide path
since those are genuinely document-affecting or one-shot.

Caveat: scoped ApplyStylesToElement is correct under the engine's current
selector grammar (id, class, tag, pseudo, attribute condition,
descendant). If sibling combinators (`+`, `~`) are added later, the
scope must widen accordingly.

Discovered while profiling drag-drop in a heavily-populated editor:
dropping a model was triggering 18 separate clean calls covering ~1500
elements across multiple roots, because every Doc.* mutator marked
every element in the document dirty. With these three changes a drop
cleans only the affected subtrees.
Keeps Claude Code's local working directory out of the repo.
@Colorfingers
Copy link
Copy Markdown
Contributor Author

I'm realizing this messes up the formatting of the Settings links... use with caution.

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.

Setup and introduction

1 participant