Skip to content

Feat: More controls for ui.scene #4820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

evnchn
Copy link
Collaborator

@evnchn evnchn commented May 31, 2025

Motivation

Addresses NiceGUI Wishlist (view), how we are stuck with OrbitControls

Implementation

  • Grab all of the controls related to moving the camera
  • Add setup_controls and fetch_controls to scene.js
  • Add THREE.Clock(); and feed the delta to get FlyControls and FirstPersonControls happy.
  • Some controls don't have target. Use this.controls.target?. to get those happy.

Progress

  • I chose a meaningful title that completes the sentence: "If applied, this PR will..."
  • The implementation is complete.
  • Pytests have been added (or are not necessary).
  • Documentation has been added (or is not necessary).

Issue: Some controls don't work properly. Do we want to drop them and release the working ones first, or spend more time get them working?

Test script:

control_types = [
    ('Orbit controls (default)', None),  # Before the PR
    ('Arcball controls', 'arcball'),
    ('Trackball controls', 'trackball'),
    ('Map controls', 'map'),
    ('Fly controls', 'fly'),
    ('First-person controls', 'first-person'),
    ('Pointer lock controls', 'pointer-lock'),
]

for label, control_type in control_types:
    ui.label(label)
    with ui.scene(width=285, height=220, **({'control_type': control_type} if control_type else {})) as scene:
        scene.sphere()
        scene.move_camera(x=1, y=-1, z=1.5)

We have decided to ship the working ones.

  • WORKING: Orbit controls: Working
  • Arcball controls: Has a tendency to jump around sproadically
  • WORKING: Trackball controls: Working
  • WORKING: Map controls: Working
  • Fly controls: Cannot control direction
  • First-person controls: Cannot control direction
  • Pointer lock controls: Cannot move camera

Feature request: #3710

@evnchn evnchn marked this pull request as draft May 31, 2025 21:55
@evnchn evnchn added feature Type/scope: New feature or enhancement in progress Status: Someone is working on it 🟡 medium Priority: Relevant, but not essential 🌿 intermediate Difficulty: Requires moderate understanding labels May 31, 2025
@evnchn
Copy link
Collaborator Author

evnchn commented May 31, 2025

@falkoschindler
Copy link
Contributor

Wow, I didn't know that there are so many controls! Map controls seem to be very useful for 3D maps in RoSys!

Anyway, I don't think we need all of them at once. If this PR can get some of them to work and lay the foundation for adding more of them later, this would be just fine.

Regarding the implementation: Do we really need to import them in an async mounted() method? I'm not sure if making this method async could have unwanted side effects. But I guess you don't want to fetch all controls at once, but only the one which is needed.

@evnchn
Copy link
Collaborator Author

evnchn commented Jun 1, 2025

Do we really need to import them in an async mounted() method? I'm not sure if making this method async could have unwanted side effects.

Yes there were unwanted side effects: Failing tests and broken ui.scene_view()

Since three.js is so much larger in comparison to the controls, it isn't worth it to investigate in dynamically loading just the control we are using IMO. If you think you can do it, feel free to take a shot.

So far, only TrackballControls and MapControls work flawlessly, so those are the two I included in this PR.

Ready for review IMO.

@evnchn evnchn marked this pull request as ready for review June 1, 2025 18:17
@evnchn evnchn changed the title [WIP] Feat: More controls for ui.scene Feat: More controls for ui.scene Jun 1, 2025
@evnchn evnchn removed the in progress Status: Someone is working on it label Jun 1, 2025
@falkoschindler falkoschindler added the review Status: PR is open and needs review label Jun 2, 2025
@falkoschindler falkoschindler self-requested a review June 2, 2025 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Type/scope: New feature or enhancement 🌿 intermediate Difficulty: Requires moderate understanding 🟡 medium Priority: Relevant, but not essential review Status: PR is open and needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants