[FEATURE] Interactive viewer plugins #2004

Milotrince · 2025-11-17T23:44:30Z

Description

nice-to-have feature, allows users to integrate interactive viewer plugins.

Related Issue

N/A

Motivation and Context

ViewerPlugin enables directly interfacing with pyviewer mouse/keyboard controls.
Replaces pynput
Keybind callback registration

How Has This Been / Can This Be Tested?

added tests/test_viewer.py

Screenshots (if appropriate):

Screen.Recording.2025-11-17.at.18.03.51.mov

Checklist:

I read the CONTRIBUTING document.
I followed the Submitting Code Changes section of CONTRIBUTING document.
I tagged the title correctly (including BUG FIX/FEATURE/MISC/BREAKING)
I updated the documentation accordingly or no change is needed.
I tested my changes and added instructions on how to test it for reviewers.
I have added tests to cover my changes.
All new and existing tests passed.

duburcqa · 2025-11-18T12:58:46Z

This feature looks very convenient!

Milotrince · 2025-11-24T02:46:35Z

Should we keep under genesis/ext/pyrender/interaction/plugins/ or genesis/vis/plugins ?
Do we want to use Vec3, Quat, Color, Ray, etc. classes in the codebase?
now that pyrender viewer mouse/keyboard interaction is exposed, we can get rid of pynput in examples

duburcqa · 2025-11-24T11:01:41Z

Should we keep under genesis/ext/pyrender/interaction/plugins/ or genesis/vis/plugins ?

Do you consider this as a "universal" plugin or something pyrender-specific? This would give you the answer :)

Do we want to use Vec3, Quat, Color, Ray, etc. classes in the codebase?

I'm not a fan of these objects. Overly complicated and not very pythonic in my view.

now that pyrender viewer mouse/keyboard interaction is exposed, we can get rid of pynput in examples

That would be great! But we need a way to expose this in a clean way.

duburcqa · 2026-01-01T09:49:22Z

@Milotrince If you could finish this PR I would be happy to review it and merge it :)

Milotrince · 2026-01-11T05:02:59Z

@duburcqa Could I get your opinion on the state of the PR? there are some design decisions I'd like your input on!

removed pynput, see examples/* changes for new usage
I kept the mouse spring interaction implementation and moved it as a plugin
there existed a register key system in pyrender viewer but was not exposed in genesis viewer. I made a Keybind/Keybindings class and exposed register_keybind to viewer

I still need to add tests.

duburcqa · 2026-01-12T09:47:37Z

Could I get your opinion on the state of the PR? there are some design decisions I'd like your input on!

Sure ! I will do it this week ! Could you fix the conflicts in the meantime? Thank you :)

duburcqa · 2026-01-13T06:45:12Z

Small comment on snapshots PR: https://huggingface.co/datasets/Genesis-Intelligence/snapshots/discussions/6

duburcqa · 2026-01-13T09:35:49Z

examples/drone/interactive_drone.py

+    # Register keybindings
+    from pyglet.window import key


Could you add from pyglet.window import key in your new keybindings module, so that users can do:

from genesis.ext.pyrender.interaction.keybindings import key

This would avoid breaking encapsulation of pyglet, that should be an implementation details for users.

Similarly, it would be great to expose your keybindings module outside ext, that is also supposed to be an implementation details for the end user. This would be relaying not the same mechanism I just mentioned. Ideally the user should be able to replace genesis.ext.pyrender.interaction.keybindings by genesis.vis.keybindings

duburcqa · 2026-01-13T09:39:13Z

Could you split the introduction of ViewerPlugin feature, with the ability to interactively add markers on geometries from the viewer? Because this PR is getting quite bug already and both features are quite orthogonal. It would make my life easier for the review.

You can open a (stacked) PR for the second part right away, merging on top of this branch instead of main (with draft status).

duburcqa · 2026-01-13T09:48:10Z

examples/drone/interactive_drone.py

+        self.rpms = [self.thrust] * 4
+        self.rpms = np.clip(self.rpms, 0, 25000)


Are you sure about your formula?

This is a bit dirty. It would be more clean to allocate rpm once (ndarray) and update its value by reference. By the way, there is no need to clip self.thrust? Here you keep incrementing something that is out of range, which is weird, because the user won't be able to decelerate for a while if accelerating too much beyond the limit.

Assuming the formula is correct:

self.thrust = min(max(self.thrust + self.thrust_delta, 0), 25000) self.rpms[:] = self.thrust

Similarly, it is really relevant to store both thrust and rpms if they are both the same but expended? Am i missing something?

duburcqa · 2026-01-13T09:51:26Z

examples/drone/interactive_drone.py

+    scene.viewer.register_keybinds(
+        *direction_keybinds(key.UP, "move_forward", (1.0, 1.0, -1.0, -1.0)),
+        *direction_keybinds(key.DOWN, "move_backward", (-1.0, -1.0, 1.0, 1.0)),
+        *direction_keybinds(key.LEFT, "move_left", (-1.0, 1.0, -1.0, 1.0)),
+        *direction_keybinds(key.RIGHT, "move_right", (1.0, -1.0, 1.0, -1.0)),
+        Keybind(key.SPACE, KeyAction.HOLD, name="accelerate", callback=controller.accelerate),
+        Keybind(key.LSHIFT, KeyAction.HOLD, name="decelerate", callback=controller.decelerate),
+    )


it would be nice to have a way to detect conflicting key bindings with the default viewer controls, then raise an exception if any, or just a warning eventually.

duburcqa · 2026-01-13T09:52:14Z

examples/IPC_Solver/ipc_arm_cloth.py

+    target_pos = [robot_init_pos.copy()]  # Use list for mutability in closures
+    target_R = [robot_init_R]  # Use list for mutability in closures


I don't understand. numpy arrays are already mutable. You can modify then in place. So doing this should not be necessary.

duburcqa · 2026-01-13T09:52:49Z

examples/IPC_Solver/ipc_arm_cloth.py

+        target_pos[0] = robot_init_pos.copy()
+        target_R[0] = robot_init_R


Just update them in place: target_pos[:] = robot_init_pos, target_R[:] = robot_init_R

duburcqa · 2026-01-13T09:53:28Z

examples/IPC_Solver/ipc_arm_cloth.py

+
+import numpy as np
 from huggingface_hub import snapshot_download
+from scipy.spatial.transform import Rotation as R


The use of scipy should be avoided if possible, but it was already there before your PR.

duburcqa · 2026-01-13T09:54:25Z

examples/IPC_Solver/ipc_arm_cloth.py

-    stop = False
    step_count = 0

-    while not stop:


Why the stop mechanism had to be removed in favor of try-except?

duburcqa · 2026-01-13T09:58:02Z

examples/sensors/lidar_teleop.py

+    def move_forward():
+        target_pos[0] += KEY_DPOS
+
+    def move_backward():
+        target_pos[0] -= KEY_DPOS
+
+    def move_right():
+        target_pos[1] -= KEY_DPOS
+
+    def move_left():
+        target_pos[1] += KEY_DPOS
+
+    def move_down():
+        target_pos[2] -= KEY_DPOS
+
+    def move_up():
+        target_pos[2] += KEY_DPOS
+
+    def roll_ccw():
+        target_euler[0] += KEY_DANGLE
+
+    def roll_cw():
+        target_euler[0] -= KEY_DANGLE
+
+    def pitch_up():
+        target_euler[1] += KEY_DANGLE
+
+    def pitch_down():
+        target_euler[1] -= KEY_DANGLE
+
+    def yaw_ccw():
+        target_euler[2] += KEY_DANGLE
+
+    def yaw_cw():
+        target_euler[2] -= KEY_DANGLE


This is very verbose for little benefit. What about only exposing translate, rotate, with some "axis: int, is_negative: bool" argument or just axis: np.ndarray, and use partial to keybind them? That would be more flexible and simpler I think.

duburcqa · 2026-01-13T09:58:30Z

examples/viewer_plugin/mesh_point_selector.py

Could we move this feature in a separate PR?

duburcqa · 2026-01-13T09:58:37Z

examples/viewer_plugin/mouse_spring.py

duburcqa · 2026-01-13T10:03:41Z

examples/keyboard_teleop.py

-        target_entity.set_qpos(np.concatenate([target_pos, target_quat]))
+    # Initialize target pose
+    target_pos = robot_init_pos.copy()
+    target_R = [robot_init_R]  # Use list to make it mutable in closures


Same as before. No need to use list encapsulation. Just modify in-place the pre-allocated array.

duburcqa · 2026-01-13T10:05:13Z

examples/keyboard_teleop.py

+
+    # Robot teleoperation callback functions
+    def move(dpos: tuple[float, float, float]):
+        global target_pos


Remove this global line entirely. It is not needed and probably confusing for users.

duburcqa · 2026-01-13T10:05:58Z

genesis/engine/sensors/raycaster.py

        ray_direction_world = ti_normalize(ti_transform_by_quat(ray_dir_local, link_quat), gs.EPS)

-        # --- 2. BVH Traversal ---
-        # FIXME: this duplicates the logic in LBVH.query() which also does traversal


Why FIXME disappeared? You actually implemented the fix?!

Milotrince force-pushed the viewer_plugin branch 2 times, most recently from 98601bb to 1e672b0 Compare November 24, 2025 21:42

Milotrince force-pushed the viewer_plugin branch from 1e672b0 to 343e8cb Compare January 4, 2026 04:40

Milotrince marked this pull request as ready for review January 11, 2026 04:56

Milotrince requested review from YilingQiao and duburcqa as code owners January 11, 2026 04:56

Milotrince force-pushed the viewer_plugin branch from 5272896 to 8e54491 Compare January 12, 2026 16:28

Milotrince added 8 commits January 12, 2026 23:17

clean rebase, initial viewer plugin

20f4c12

implement keybindings

fe61b0f

update examples and refactor out help text

0b906d7

add point mesh selector example

83296a5

rufff format

636ee30

remove pyrender vec3 utils

919ecf9

minor fixes and cleanup

bbecae1

add viewer plugin test

0a14941

Milotrince force-pushed the viewer_plugin branch from 7639573 to 0a14941 Compare January 13, 2026 04:17

cleanup examples

f530fdc

duburcqa reviewed Jan 13, 2026

View reviewed changes

examples/viewer_plugin/mesh_point_selector.py

Copy link

Collaborator

duburcqa Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move this feature in a separate PR?

duburcqa reviewed Jan 13, 2026

View reviewed changes

examples/viewer_plugin/mouse_spring.py

Copy link

Collaborator

duburcqa Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same.

duburcqa reviewed Jan 13, 2026

View reviewed changes

		self.rpms = [self.thrust] * 4
		self.rpms = np.clip(self.rpms, 0, 25000)

		target_pos = [robot_init_pos.copy()] # Use list for mutability in closures
		target_R = [robot_init_R] # Use list for mutability in closures

		target_pos[0] = robot_init_pos.copy()
		target_R[0] = robot_init_R

[FEATURE] Interactive viewer plugins #2004

Are you sure you want to change the base?

[FEATURE] Interactive viewer plugins #2004

Uh oh!

Conversation

Milotrince commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How Has This Been / Can This Be Tested?

Screenshots (if appropriate):

Checklist:

Uh oh!

duburcqa commented Nov 18, 2025

Uh oh!

Milotrince commented Nov 24, 2025

Uh oh!

duburcqa commented Nov 24, 2025

Uh oh!

duburcqa commented Jan 1, 2026

Uh oh!

Milotrince commented Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

duburcqa commented Jan 12, 2026

Uh oh!

duburcqa commented Jan 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

duburcqa commented Jan 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Milotrince commented Nov 17, 2025 •

edited

Loading

Milotrince commented Jan 11, 2026 •

edited

Loading