rohanpsingh / mujoco-python-viewer

Simple renderer for use with MuJoCo (>=2.1.2) Python Bindings.
BSD 2-Clause "Simplified" License
163 stars 24 forks source link

[Won't Merge] Refactoring #18

Open jaku-jaku opened 1 year ago

jaku-jaku commented 1 year ago

Hi Rohan, Here is my revision of the viewer:

Major refactoring:

viewer.process_safe() # process interactions and updates from callbacks
viewer.update_safe() # update mujoco engine
viewer.render_safe() # render on screen
self.mj_viewer.render_sensor_cameras_safe() # render cameras off screen

Let me know if you have any comments. Thank you for your initial works.

Best Jack

rohanpsingh commented 1 year ago

Hi @jaku-jaku I took a brief look at your changes, I think some of the general use functionalities could've be merged here too :)

Nevertheless, since you opened issue https://github.com/rohanpsingh/mujoco-python-viewer/issues/16 I've been working on rendering multiple cameras on the side, besides some other cool features. Hope I can spend more time on this soon.

constant time keyboard callback action through hashing instead of linear time with if-else

Did this give you noticeably better performance?

Thanks.

jaku-jaku commented 1 year ago

Hi @jaku-jaku I took a brief look at your changes, I think some of the general use functionalities could've be merged here too :)

Yeah, but I kinda refactored almost everything into my personal style I preferred, that's why it might be hard to merge in. I personally feel the refactored version will be easier to collaborate on.

The main intention was to make the code architecture clean and easy to scale with more key registrations in the future, and serve as GUI interface in joint with main engine code. The rendering will be invoked by the engine thread, so separating the GUI callbacks and Rendering pipeline will be a good measure to void any potential bugs.

Lemme know your thoughts.
An example of the implementation will be: https://github.com/UW-Advanced-Robotics-Lab/simulation-mujoco-summit-wam

Nevertheless, since you opened issue #16 I've been working on rendering multiple cameras on the side, besides some other cool features. Hope I can spend more time on this soon.

Nice, many thanks! I was attempting as well, and I have added that pipeline in. Later, I also notice 'dm_control' has the pipeline for multi-camera, but I have already worked based on your viewer. So I rendered off screen in viewer, and rendered on-screen with cv2 thread in my engine. The result looks like:

waterloo_steel

Did this give you noticeably better performance?

It's not much noticeable, but I just feel the If-else statement (used in mujoco-py) is quite itchy (from C/C++ POV), and can be buggy.

So, in the engine code, I can run in the update functions (or maybe separate thread in the future):

        # - render current view:
        for i in range(10):
            mujoco.mj_step(self.mj_model._model, self.mj_data._data) # stepping couple times
        self.mj_viewer.process_safe() # process GUI interfaces (keys registered till this update and mouse interactions) that was registered in callbacks
        self.mj_viewer.update_safe() # update the scene via mujoco2.2.x
        self.mj_viewer.render_safe() # render the raw main viewer off screen, and display on_screen with overlays
        self.mj_viewer.render_sensor_cameras_safe() # iterate through camera configurations, update the scene and render cameras off-screen