Fix ui.scene.move_camera() recreating the camera controls when the up vector is unchanged

[ptruka]

Motivation

Fixes #6241.

scene.move_camera() disposes and recreates the Three.js controls object on every call, so any configuration applied to it (enableRotate, screenSpacePanning, mouseButtons, custom properties) is reset to the control class defaults whenever the camera moves.

scene.js already intends this to be conditional (camera_up_changed), but scene.py resolves each up_* argument against the stored camera before sending it, so the values are never null and the guard is always true.

nicegui/nicegui/elements/scene/scene.py

Line 261 in 1e3b021

self.camera.up_x = self.camera.up_x if up_x is None else up_x

Because the rebuild happens in the tween's onComplete, a ui.run_javascript() issued right after move_camera() is also a race: whether the configuration survives depends on round-trip timing.

Implementation

The guard now compares the requested up vector against the current one instead of checking whether an argument was supplied, so it reflects whether the vector actually changes.
The resolved target vector is hoisted into a THREE.Vector3 that both the guard and the tween target use, which keeps the existing null fallbacks in one place.

This is done entirely in scene.js; the Python API and the arguments sent to the client are unchanged. An alternative discussed in the issue: sending the raw arguments so the existing null handling applies. This would additionally change which pose wins for unspecified axes (Python's stored camera state vs. the browser's current pose), which is a separate behavioral question and is deliberately not part of this PR.

ui.scene_view resolves up_* the same way but creates no controls object, so it is unaffected.

The test locks in both directions: the configuration survives a camera move with an unchanged up vector, and the controls are still rebuilt when the up vector changes.
It asserts on enableRotate (a public OrbitControls property) rather than a marker of its own, and uses the camera arriving at its target as the barrier, since onUpdate and onComplete run in the same tween update.

Progress

• The PR title is a short phrase starting with a verb like "Add ...", "Fix ...", "Update ...", "Remove ...", etc.
• The implementation is complete. (Otherwise, open a draft PR.)
• This PR does not address a security issue. (Security fixes must be coordinated via the security advisory process before opening a PR.)
• Pytests have been added/updated, or the Implementation section explains why they are not necessary.
• No breaking changes to the public API.

[evnchn]

Nice catch, and thanks for the fix + test! 🙏 The old condition really was always true (move_camera resolves the up vector Python-side and always sends concrete floats), so the controls got thrown away on every camera move — comparing values instead is the right idea.

Two things I ran into while looking at it:

1. control_type='trackball' slips through. TrackballControls mutates camera.up in place while rotating (object.up.applyQuaternion(...) in _rotateCamera), so once the user has dragged the scene, the live camera.up no longer matches the sticky server-side value and the rebuild fires again. Same for a non-axis-aligned up: the tween writes start + (end - start), which can land 1 ULP off, so equals() stays false forever [corrected below — it costs one extra rebuild, not a permanent one] after e.g. move_camera(up_x=0.1, up_y=0.2, up_z=0.3).

Both would disappear if, instead of disposing and recreating, we just refreshed the one thing that actually goes stale — OrbitControls caches _quat/_quatInverse from camera.up at construction time and never updates them. That would also stop a genuine up change from wiping the user's controls config, which the current code still does. Happy to be wrong here — what do you think?

2. The test may be flaky. wait_for(... camera.position.x == 1) is an exact float compare, but controls.update() runs every frame after the tween and re-derives the position through a spherical round-trip, so x often comes back as 1.0000000000000002. pytest.approx (or reading camera_tween._object, like tests/test_scene_view.py does) would make it deterministic.

Two smaller notes

• The rebuild was also the only thing that ever re-ran TrackballControls.handleResize() — it caches the canvas rect in its constructor and nothing else refreshes it, so after a window resize trackball dragging will now stay misaligned until reload. This turned out to be a pre-existing bug on main rather than anything this PR introduces, so I filed it separately as TrackballControls keeps a stale canvas rect after a window resize, so ui.scene rotation becomes misaligned #6247 — nothing to do here.
• CONTRIBUTING.md reserves NOTE: for cross-file coupling / mirrored changes; this one is local rationale, so a plain comment fits better.

None of this is a big deal — the direction is good, and the trackball case is easy to miss. Let me know if you'd like a hand with any of it!

---

Edit — I ran both claims instead of leaving them as theory, and one was overstated. Sorry about that. 🙇

The float point is one extra rebuild, not a permanent one. The residue is real, but the following move tweens from it onto the exact value and settles:

[after up=(0.1, 0.2, 0.3)] browser camera.up = [0.1, 0.2, 0.30000000000000004]
  python-side sticky value  = [0.1, 0.2, 0.3]
[plain move #1] controls REBUILT
[plain move #2] controls ALIVE
[plain move #3] controls ALIVE

Across 484 realistic up-vector component pairs, 96 leave residue and 0 fail to converge on the next move. A minor wart — please don't let it hold the PR up.

The trackball point does hold, and re-arms on every drag:

controls class: TrackballControls
up before drag: [0, 0, 1]
[no drag]    plain move_camera -> controls ALIVE
up after drag: [-0.4357545513303538, 0.3486541493959340, -0.8297941040426114]
[after drag] plain move_camera -> controls REBUILT

Same one-shot pattern, but a user drags constantly, so in practice it keeps firing.

[ptruka]

Thanks, you're right that the trackball case was broken, and my fix was wrong in a way the orbit test couldn't catch. It's fixed in the latest push, along with your two smaller notes.

The mistake was checking against the live camera.up. The rebuild actually needs the up-vector that the controls were originally built with. With TrackballControls, those two stop matching as soon as the user drags. So now that vector gets saved when the controls are created, and both places that create controls call a single create_controls().

[evnchn]

Posted by Claude Code on evnchn's behalf.

Verdict: it holds. I re-ran both of my original checks against aa509be and both are clean.

Snapshotting the up-vector at construction is the right invariant, and it's a better one than I was reaching for. Because both guard inputs are now persistent — the sticky Python value and controls_up — the guard also survives a tween that gets interrupted mid-flight, which comparing against the live camera.up would not have.

You also closed the float-residue wart I explicitly said not to hold the PR up for: camera.up.copy(target_up) makes the next comparison bit-exact, so it's zero extra rebuilds now instead of one.

Nothing blocking from me. One nit worth a line (an adversarial second-model pass found a narrow path where the new guard is wrong; I ran it and it reproduces), plus two notes for whoever merges — neither of those is on you.

Both original findings re-checked — before vs. after

Same two MREs from my first pass, unchanged, run against the new head.

Trackball drift — a real mouse drag, then plain move_camera(x=...):

	9fbc6cb (previous push)	aa509be (now)
plain move, no drag	ALIVE	ALIVE
plain move after drag	REBUILT	ALIVE
plain move again	REBUILT	ALIVE

controls class: TrackballControls
up before drag: [0, 0, 1]
[no drag]    plain move_camera -> controls ALIVE
up after drag:  [-0.43575455133035385, 0.34865414939593403, -0.8297941040426114]
[after drag] plain move_camera -> controls ALIVE
[after drag] plain move_camera again -> controls ALIVE

Float residue — after move_camera(up_x=0.1, up_y=0.2, up_z=0.3):

[after up=(0.1, 0.2, 0.3)] browser camera.up = [0.1, 0.2, 0.3]
  python-side sticky value    = [0.1, 0.2, 0.3]
  bit-exact match?            = True
[plain move #1, no up args] controls ALIVE
[plain move #2, no up args] controls ALIVE
[plain move #3, no up args] controls ALIVE

Previously camera.up.z came back as 0.30000000000000004 and the following move was REBUILT. The copy(target_up) line removes that entirely.

Seen-to-fail: the new trackball test really does guard the bug

Three-way, reverting only nicegui/elements/scene/scene.js and keeping both tests:

scene.js from	..._keeps_controls_unless_up_vector_changes	..._keeps_trackball_controls_after_rotating
main (unfixed)	FAIL	FAIL
9fbc6cb (previous push)	pass	FAIL
aa509be (this push)	pass	pass

The middle row is the one that matters — the new test fails on exactly the version whose orbit test couldn't catch anything, with the symptom named directly:

E   assert False is True
E    +  where False = execute_script('return getElement(4).controls.staticMoving')

Full tests/test_scene.py is green (20 passed, no skips), and I ran the two new tests 10× back-to-back for flakiness — 10/10. pre-commit clean on both changed files.

staticMoving is a good pick, incidentally: it's a real TrackballControls property that resets on rebuild, so the test doesn't need a marker of its own.

Probed for new holes — two call sequences, both clean

The thing I wanted to break was controls_up going stale, since nothing outside create_controls() ever writes it.

Drag, then a genuine up change — must still rebuild:

[after drag]          cup=[0, 0, 1]  up=[0.0142, 0.8795, -0.4755]
[genuine up change]   controls REBUILT   cup=[0, 1, 0]  up=[0, 1, 0]
[plain move after]    controls ALIVE     cup=[0, 1, 0]  up=[0, 1, 0]

A long up-changing tween interrupted mid-flight by another move_camera — the stopped tween's onComplete never fires, so the rebuild is skipped and camera.up is stranded mid-interpolation:

[mid-tween]                  controls ALIVE     cup=[0, 0, 1]  up=[0, 0.0926, 0.9073]
[interrupted by plain move]  controls REBUILT   cup=[0, 1, 0]  up=[0, 1, 0]
[next plain move]            controls ALIVE     cup=[0, 1, 0]  up=[0, 1, 0]

It self-heals: Python has already committed the new up to its sticky state, so the interrupting move carries it as target_up, still sees the stale controls_up, and rebuilds on its completion. This is the case the live-camera.up version would have gotten wrong, because mid-interpolation the live vector matches neither value.

Nit: the guard is wrong on the null up path (not reachable from scene.move_camera())

I ran the new version past a second model with "assume this is broken" framing, because my first pass missed the trackball case and I didn't want to repeat that. It found one thing I hadn't, and it reproduces:

target_up falls back to the live camera.up when an up component is null, but the guard compares that against controls_up. After a trackball drag those two differ, so a JS caller that omits the up vector gets a rebuild it never asked for:

getElement(id).move_camera(1, null, null, null, null, null, null, null, null, 0)

scene.js from	staticMoving after the call
main	true — controls survived
aa509be	false — spuriously rebuilt

So on that one path this is a small step back from main, which got it right by accident (all-null meant camera_up_changed was false).

Reachability is zero from Python — I checked every call site, and both scene.py and scene_view.py always resolve the sticky state before run_method, so null can only arrive from a hand-written ui.run_javascript() call against an undocumented internal. That's why it's a nit and not a finding.

It's still worth closing, because it's the same bug class as the one you just fixed, and the guard is the one line that now has to be right. One extra condition does it:

const camera_up_changed =
  (up_x !== null || up_y !== null || up_z !== null) && !this.controls_up.equals(target_up);

The up_* values can't be null today, so the added clause is inert on every real call — it just makes the JS method honour the contract its own nine fallbacks advertise. The alternative reading is that those fallbacks are dead code and should go, but that's a bigger change than this PR should carry.

I applied that line before suggesting it rather than eyeballing it: the JS call above goes back to staticMoving = true, both of your new tests still pass, both of my MREs still come out clean, and tests/test_scene.py is still 20/20 with pre-commit green.

Still open, but a design question rather than a defect

A genuine up change still wipes the user's controls config, which was the second half of my first comment and is the part your reply doesn't cover:

[after move_camera(up_x=0, up_y=1, up_z=0)]
  enableRotate      = True   (was set to False)
  mouseButtons.LEFT = 0      (was set to 2)

I don't think this should hold the PR up — it's much rarer than the every-move case you've fixed, and the fix here is a strict improvement either way. Worth saying that create_controls() makes the narrower version easy to add later: refreshing OrbitControls' cached _quat/_quatInverse instead of disposing would need a per-class branch there and nothing else, since TrackballControls reads camera.up live and doesn't cache it at all.

Maintainer's call whether that's this PR or a follow-up.

CI has never run on this PR

Both pushes are sitting at action_required — the first-time-contributor gate, so no workflow has executed on either head:

$ gh run list --repo zauberzeug/nicegui --branch fix-ui.scene.move_camera
completed  action_required  ...  CI Gate  pull_request  31107637862  0s  2026-08-06T13:47:58Z
completed  action_required  ...  CI Gate  pull_request  31080526355  0s  2026-08-06T07:20:15Z

Worth someone clicking approve before merge. Everything above is macOS + headless Chrome at the same 600×600 the CI fixture uses, but it's still my machine, and ..._keeps_trackball_controls_after_rotating is the first test in this repo to drive a real mouse drag through ActionChains, so CI is genuinely new ground for it rather than a formality.

I tried to pre-empt that in a Linux container and could not get a usable signal — Debian's arm64 Chromium fails the entire pre-existing scene suite there on WebGL console errors, so it says nothing about this PR either way. Flagging the gap rather than papering over it.