QA

[GaneshPatil7517]

@sponce and @EdwardMoyse
Given Phoenix’s production usage and my planned GUI modernization, what exact acceptance criteria would you want me to satisfy before we switch from the current GUI to the new implementation (feature parity checklist, performance threshold, and rollback plan), so I can optimize for long-term maintainability rather than just migration speed

[EdwardMoyse]

Hi @GaneshPatil7517 - we would essentially want all of the same functionality. I am also pretty happy with the look and feel, personally, but this is open to discussion of course. Anything which can be done to simplify the maintenance burden would be great, and anything to help make it work e.g. in AR/VR would also be brilliant.

[GaneshPatil7517]

Hi @GaneshPatil7517 - we would essentially want all of the same functionality. I am also pretty happy with the look and feel, personally, but this is open to discussion of course. Anything which can be done to simplify the maintenance burden would be great, and anything to help make it work e.g. in AR/VR would also be brilliant.

Thanks @EdwardMoyse, that's very helpful for scoping the work.

I'll treat full feature parity as the hard requirement every existing control will be validated against the PhoenixUI interface before any switch. For the look and feel, I'll keep it close to what's there now unless we agree on changes during implementation.

On maintenance: lil-gui (my leading candidate) is actively maintained by the original dat.gui author, has TypeScript types built in, and a smaller footprint so that directly addresses the maintenance angle.

The AR/VR point is interesting I'll look into how the GUI layer interacts with WebXR sessions and whether the replacement can be made XR-aware (e.g., rendering controls as an in-scene overlay rather than a DOM panel). I'll flag what's feasible during prototyping in the bonding period.

[EdwardMoyse]

Hi, I just wanted to comment here that we do not use dat.GUI except as a legacy GUI. All of the stuff you see in the examples is PhoenixUI. We kept dat.GUI as a simple alternative, but at this point we could consider dropping it completely.

[GaneshPatil7517]

Hello @EdwardMoyse and @sponce
While reviewing the rendering pipeline and the library-framework boundary, I noticed that the Three.js animation loop (set up via ThreeManager.setAnimationLoop()) runs inside Angular's zone - there's no NgZone.runOutsideAngular() wrapping it.

This means every requestAnimationFrame tick (~60/sec) triggers Angular change detection across the entire component tree, even when nothing in the Angular UI has changedp - only the WebGL canvas is being repainted.

In a typical Three.js + Angular setup, the recommended pattern is:

constructor(private ngZone: NgZone) {}

init() {
  this.ngZone.runOutsideAngular(() => {
    renderer.setAnimationLoop(() => this.render());
  });
}

Then re-enter the zone only when the UI actually needs to update (e.g., a user toggle, event load callback).

However, Phoenix has a specific complication: the UIManager.updateUI() callback is called inside the animation loop (uiLoop in event-display.ts), and the PhoenixMenuNode callbacks mutate Three.js state synchronously - meaning Angular's zone awareness of those callbacks is what currently keeps the UI bindings in sync without explicit ChangeDetectorRef.markForCheck() calls.

My question: Has running the animation loop outside Angular's zone been considered before? If the standalone migration + signals work goes well, this could be a natural follow-up - signals with OnPush change detection would give us explicit control over when Angular re-renders, and we could safely move the render loop out of the zone. But I want to understand if there's a historical reason it was kept inside the zone (e.g., stats panel updates, overlay sync, or XR session callbacks that depend on zone-triggered change detection).

[EdwardMoyse]

Hi @GaneshPatil7517 - I don't remember this being discussed. The closest we had was #205. Your suggestion does sound very interesting though!

[GaneshPatil7517]

Hi @GaneshPatil7517 - I don't remember this being discussed. The closest we had was #205. Your suggestion does sound very interesting though!

Thanks @EdwardMoyse I've read through #205 and the history there.

The issue was opened in 2021 by @ntadej Phoenix renders at high FPS even when idle. @9inpachi attempted render-on-demand in PR #233, and the conclusion was: "We already have this in our ThreeManager. The hard part is to actually call it in tons of places and then manage every update."

Looking at the current code, the render-on-demand approach was never fully landed ThreeManager.setAnimationLoop() still runs a continuous loop via renderer.setAnimationLoop(), rendering every frame unconditionally.

I think there are actually two separate performance problems here, and they can be addressed in phases:

Phase 1 NgZone isolation (quick win, low risk):
Move the animation loop outside Angular's zone with ngZone.runOutsideAngular(). This doesn't stop continuous rendering, but it eliminates ~60 unnecessary Angular change detection cycles/sec. The uiLoop callback (which calls UIManager.updateUI()) would re-enter the zone only when it actually mutates Angular state. This is a targeted change that doesn't require touching "tons of places" it's a wrapper around the existing setAnimationLoop() call.

Phase 2 Render-on-demand (larger effort, builds on Phase 1):
After the standalone migration + signals work, components would use OnPush change detection and signals instead of relying on zone-triggered detection. At that point, we could replace the continuous loop with on-demand rendering triggered by: camera movement (OrbitControls change event), TWEEN animations, collection visibility toggles, event loading, and XR sessions. This is the harder problem @9inpachi described, but signals + OnPush make it more tractable because Angular no longer needs the zone to know when to update.

Phase 1 fits cleanly into my GSoC timeline. Phase 2 could be a follow-up either late in the GSoC period or post-GSoC, depending on how the earlier migration work lands.

Would this phased approach make sense? Happy to prototype Phase 1 during the bonding period to measure the actual CPU impact.

[EdwardMoyse]

I think a phased approach is a good idea.

[GaneshPatil7517]

I think a phased approach is a good idea.

Thanks @EdwardMoyse! I'll plan to prototype Phase 1 during the bonding period and share the profiling results. Looking forward to it.

[GaneshPatil7517]

Hey @sponce and @EdwardMoyse, while waiting for selections I've started working on a couple of the proposed deliverables:

• NgZone Phase 1 isolation (from our discussion) wrapping the animation loop with runOutsideAngular() to eliminate ~60 unnecessary change detection cycles/sec

• Replace lodash cloneDeep with structuredClone removing the lodash dependency

Will open PRs shortly. Happy to adjust direction based on your feedback.