-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix multiclient jitter with separate render threads for each client #2519
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exciting!!
nerfstudio/viewer_beta/viewer.py
Outdated
@@ -329,7 +342,7 @@ def update_scene(self, step: int, num_rays_per_batch: Optional[int] = None) -> N | |||
""" | |||
self.step = step | |||
|
|||
if self.camera_state is None: | |||
if len(self.render_statemachines) is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this condition still doing something? Will this ever be None
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops I changed it now to if len(self.render_statemachines) == 0
nerfstudio/viewer_beta/viewer.py
Outdated
@@ -184,24 +185,30 @@ def nested_folder_install(folder_labels: List[str], element: ViewerElement): | |||
] | |||
for c in self.viewer_controls: | |||
c._setup(self) | |||
self.render_statemachine.start() | |||
|
|||
def get_camera_state(self, client): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this: def get_camera_state(self, client: viser.ClientHandle) -> CameraState:
I accidentally overlooked "Returns the Cameras object representing the current camera for the viewer." What is the desired behavior now that we want multiple clients to be able to control the same viewer? |
I'm changing it to take |
RenderStateMachine
threads are created for each newly connected client in replacement of the old universal render thread --> clients now able to control their own cameras