-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 direct render+export and notifications #3220
base: main
Are you sure you want to change the base?
Conversation
Nice PR. @ginazhouhuiwu, is it possible to also add the option to "capture frame" or something similar, where only a single image is saved instead of a video. |
that sounds like a great feature I will try implementing! Thanks for taking a look : ) |
Yeah, it has been a feature that I have seen some users bring up a few times in the past, just to quickly take some snapshots from a certain angle (perhaps using the same camera intrinsics as the original training dataset, but I think that is configurable in the gui anyways). Just some additional thoughts ... :) |
Super cool! Would also be great to build out some functionality for previewing the render as it goes and perhaps canceling it. Would be cool to also integrate with viser's download functionality to download the final render directly to someone's machine :) |
Local download! Supported for both render and exports now : ) Screen.Recording.2024-06-21.at.5.50.44.PM.mov |
Some finishing touches with suggestions from @kerrj, namely disabling render button when render is already in progress or if no keyframes are added, only enabling download button once file is ready, and allowing render to be canceled. The generate command button is added back as an additional option. This PR is ready once nerfstudio-project/viser#225 is and I need to clean pyright. EDIT: is ready now : ) Screen.Recording.2024-08-26.at.8.25.46.PM.1.1.mov |
nerfstudio/scripts/render.py
Outdated
@@ -432,6 +455,11 @@ class BaseRender: | |||
"""If true, checks line-of-sight occlusions when computing camera distance and rejects cameras not visible to each other""" | |||
camera_idx: Optional[int] = None | |||
"""Index of the training camera to render.""" | |||
kill_flag: List[bool] = field(default_factory=lambda: [False]) |
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 not expose this in the config? it feels like it should be an invisible class self._kill_flag
variable, then the kill() function sets that to true.
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.
Still a hack but one option is:
# `tyro.conf.Suppress` hides this field from the generated `ns-render` CLI.
_kill_flag: tyro.conf.Suppress[List[bool]] = field(default_factory=lambda: [False])
nerfstudio/scripts/render.py
Outdated
@@ -442,6 +470,8 @@ class RenderCameraPath(BaseRender): | |||
"""Filename of the camera path to render.""" | |||
output_format: Literal["images", "video"] = "video" | |||
"""How to save output data.""" | |||
complete: bool = True |
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.
Same with this, it seems like complete should be a @property
that gets managed internally
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.
I'm not really following the connection to @property
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.
I can hack around this too by suppressing? I think as a field it does need to still exist for the progress(notification) update.
Some observations from testing it out:
|
Ok sg thanks for testing! I can probably add some error messages in the ui |
How is this going? would be good to merge soon so it doesn't go forgotten about |
Directly render and export things from the viewer, with notifications!
Uses a simple notification addition from nerfstudio-project/viser#225.
Screen.Recording.2024-06-12.at.10.17.01.PM.online-video-cutter.com.1.mp4
TODO: