-
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
Update exporter.py to export sh_degree 0 case #3371 #3374
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.
Seems overall reasonable to me! Is there anywhere to reference which PLY keys are supported by which third-party tools, though? The f_dc_{0,1,2}
behavior introduced in this PR seems standard but I'm not actually sure why/when the colors
key from the original behavior is useful.
The The |
Got it! Maybe I'm still wondering: is there anybody relying on the Since it's relatively rare to train models without spherical harmonics I'm personally okay with changing the default behavior here. @akristoffersen Justin commented that you might know? |
Changing back to "red", "green", "blue" keys instead of "colors" is also an option, since they people can directly load the PLY as point cloud in meshlab or similar tools. |
The exporter is actually currently broken when sh_degree == 0. See: #3377 Maybe this is beyond the initial scope of this PR, but it might make sense to add logic to this PR to break apart
It would probably also be good to change the first check of
|
@MLLeKander Thanks very much, let me update my PR with the proposed fix. |
One suggestion: what if we have a field
? I'm not sure about the naming, but something like this seems slightly more consistent to me. Since then by default we can support all of the viewers that support |
I don't see a reason to not respect the In
It seems like these properties are only used in the exporter. And in
Note that I haven't gotten a chance to test/verify this code, so take it with a grain of salt. |
Change to write sh coefficients instead of color values
Add sh0 renderer case for model.config.sh_degree == 0
@MLLeKander Could you take a look at my latest revision (maybe test your color-only model)? |
The rgb color mode looks great. However, it seems that the result when In the |
@MLLeKander @brentyi I think it is now ready to merge. Let me know if you have other suggestions. |
Just verified, both |
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 too, thanks @jb-ye @MLLeKander!
Recover PR #3371