Skip to content
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

Add CameraTrajectoryPanel to view/edit camera trajectories #153

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jkulhanek
Copy link
Contributor

image

@jkulhanek
Copy link
Contributor Author

Please let me know your thoughts on this.

@brentyi
Copy link
Collaborator

brentyi commented Dec 29, 2023

Cool! It seems like there are a lot of things here that could be used to improve the Nerfstudio / viewer beta render panel.

As an initial thought: existing GUI elements in viser all synchronize state with the server, so changes to the GUI persist if you refresh the page, and synchronize across multiple connected clients. It would be nice to make this panel consistent with that.

Related, my main concern is generalizability: one of the core focuses of viser (in contrast to the original Nerfstudio viewer) was to replace hardcoded frontend logic with more modular UI components, and then shift as much high-level logic to Python as possible. This makes the frontend easier to integrate with things implemented in Python, like the recent Perspective/Fisheye/Equirectangular preview support added in Nerfstudio: nerfstudio-project/nerfstudio#2667.

Are there disadvantages to breaking down this panel implementation into smaller+more general frontend components with Python APIs, which could then be stitched back together in Python to produce the same functionality?

  • The multislider for keyframe timing + keyframe reordering UI look particularly nice!
  • The camera path upload would require finishing Handlers and Messages for uploading files #121.
  • The rest of the UI looks like it could already be put together via some permutations of .add_gui_button(), .add_gui_number(), .add_gui_vector2(), etc, although effort would be needed to get things as pretty as what you have. (icons for the number inputs, etc)

@jkulhanek
Copy link
Contributor Author

I see. I will try to make it more modular. My original motivation was to make it fast enough to save the callback roundtrip eg when you move the slider. I am thinking about the following separation: (player+frustum it controls), (multislider), (camera list) + standard components.

@brentyi
Copy link
Collaborator

brentyi commented Dec 30, 2023

Thanks @jkulhanek!

Yeah, when the slider is moved what we currently have is something like:

  • [client -> server] GUI update message (triggers GUI update callback)
  • [server -> client] visualized frustum update
  • [server -> client] camera pose update (does not trigger camera update callback; could be optimized!!)
  • [client -> server] camera pose update (triggers camera update callback)
  • [server -> client] new rendered image

...interspersed with logic for various locks, thread spawning, windowing, etc. Agree that it would be nice to optimize this whole thing. 🙃

That said, I'd still prefer to minimize compromising on the goals that have guided viser development so far. To me this reduces to:

  • Prioritizing versatility/modularity/reusability over performance or application-specific UI/UX advantages
  • Building something generally useful that happens to work for Nerfstudio (as opposed to something for Nerfstudio that happens to be generally useful)
  • Keeping things focused + maintanable; being conservative about adding features!

For the render panel specifically, it would be nice if we can integrate the changes in this PR without regressing on some of the new features in the beta viewer, like being able to check the "Move Keyframes" box and change the position/orientation of keyframes, or being able to click on frustums for specific keyframes and adjust the FOV of just that keyframe.

@brentyi brentyi force-pushed the main branch 5 times, most recently from 3f8725e to 3e41b46 Compare March 30, 2024 04:07
@brentyi brentyi force-pushed the main branch 11 times, most recently from 6ea47b3 to 7a6ab4a Compare May 25, 2024 07:14
@brentyi brentyi force-pushed the main branch 5 times, most recently from dcd992b to cd3c214 Compare June 7, 2024 03:20
@brentyi brentyi force-pushed the jkulhanek/camera-trajectory-panel branch from 07c90eb to 6370966 Compare June 7, 2024 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants