eth-ait / aitviewer

A set of tools to visualize and interact with sequences of 3D data.
MIT License
553 stars 49 forks source link

Can not run multiple viewers sequencially #12

Closed MarilynKeller closed 1 year ago

MarilynKeller commented 2 years ago

With the current code, the following crashes on the last line:

    v = Viewer()
    v.run()
    v = Viewer()
    v.run()

With the error:

Traceback (most recent call last):
  File "analysis/joint_projection_unposed.py", line 164, in <module>
    v.run()
  File "/home/kellerm/Code/aitviewer/aitviewer/viewer.py", line 274, in run
    self._init_scene()
  File "/home/kellerm/Code/aitviewer/aitviewer/viewer.py", line 260, in _init_scene
    self.scene.make_renderable(self.ctx)
  File "/home/kellerm/Code/aitviewer/aitviewer/scene/scene.py", line 149, in make_renderable
    r.make_renderable(self.ctx)
  File "/home/kellerm/Code/aitviewer/aitviewer/scene/node.py", line 359, in _decorator
    func(self, *args, **kwargs)
  File "/home/kellerm/Code/aitviewer/aitviewer/renderables/meshes.py", line 382, in make_renderable
    self.smooth_vao = ctx.vertex_array(self.smooth_prog,
  File "/home/kellerm/Code/skeleton_inference/skel_env/lib/python3.8/site-packages/moderngl/context.py", line 1140, in vertex_array
    return self._vertex_array(*args, **kwargs)
  File "/home/kellerm/Code/skeleton_inference/skel_env/lib/python3.8/site-packages/moderngl/context.py", line 1169, in _vertex_array
    res.mglo, res._glo = self.mglo.vertex_array(program.mglo, content, index_buffer_mglo,
moderngl.error.Error: the program belongs to a different context
Segmentation fault (core dumped)

The problem is in aitviewer/enderables/meshes.py : make_renderable : l.351, the shader program is not reloaded with the new context. The old shader program instance is used and it has the context of the first viewer, hence the mismatch.

I see that the shader program is not reloaded everytime l.351 is executed thanks to a cache @functools.lru_cache() in aitviewer/shaders.py. I guess this cache should be cleared in between two viewers creation.

This is the hack I have so far that fixes it:

def clear_shader_cache():
        # Import and list all the functions that need the cache to be cleared
        from aitviewer.shaders import get_smooth_lit_with_edges_program, get_flat_lit_with_edges_program, get_smooth_lit_texturized_program, get_simple_unlit_program, get_cylinder_program, get_screen_texture_program, get_chessboard_program
        for f in [get_smooth_lit_with_edges_program, get_flat_lit_with_edges_program, get_smooth_lit_texturized_program, get_simple_unlit_program, get_cylinder_program, get_screen_texture_program, get_chessboard_program]:
           # Try clearing the cache
            try:
                f.cache_clear()
                print(f'Clearing cache for {f}')
            except:
                pass
    #Viewer.py
    def run(self, *args, log=True):
        ...
        self.on_close()
        self.window.destroy()
        self.clear_shader_cache() # I add this line to clear the shader cache 
        ...

 With that, the following code works:
 for i in range(10):
    v = Viewer()
    v.run()
    # closing manually the window


    But still this tends to crash randomly with a segfault and no error message if I close the window too fast.
ramenguy99 commented 2 years ago

Hi, thanks for reporting this issue! We added the clearing of the shader cache when the viewer is closed as you suggested.

We also noticed some problems with running the viewer multiple times from the same script, they seem to be related to the moderngl-window implementation of the pyqt5 window. In our tests switching to the pyglet window seems to fix the problem, but we prefer to keep the pyqt5 window as default since it has shown to be more reliable in other aspects and has better performance. You can switch to the pyglet window by changing the window_type = "pyqt5" to window_type = "pyglet" in the Viewer class definition, we will also add a more convenient way to set the window type from the configuration file in the next release (this change is already up in the latest dev branch). Let us know if we can be of any further help!