-
Notifications
You must be signed in to change notification settings - Fork 315
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
Added orthographic projection #349
Added orthographic projection #349
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.
This is very useful!! It seems like a handful of users are looking for this support from #349! Left a few minor comments.
gsplat/cuda/_torch_impl.py
Outdated
width: int, | ||
height: int, | ||
) -> Tuple[Tensor, Tensor]: | ||
"""PyTorch implementation of prespective projection for 3D Gaussians. |
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.
typo: "perspective projection"
gsplat/cuda/_wrapper.py
Outdated
@@ -77,12 +77,13 @@ def quat_scale_to_covar_preci( | |||
return covars if compute_covar else None, precis if compute_preci else None | |||
|
|||
|
|||
def persp_proj( | |||
def proj( |
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.
For the sake of backward compatibility, I think we should keep the function persp_proj
and have another ortho_proj
. The rest of the code does not need to change (i.e., these two APIs could share the underlying implementation)
Another solution which is also fine with me, is to have this proj
function and keep the original persp_proj
but raise deprecated warning inside of it. This would also avoid sudden interrupt to the users.
tests/test_basic.py
Outdated
@@ -174,6 +180,8 @@ def test_projection(test_data, fused: bool, calc_compensations: bool): | |||
quats = test_data["quats"] | |||
scales = test_data["scales"] | |||
means = test_data["means"] | |||
ortho = test_data.get("ortho", 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.
You can set ortho
as an argument of this function and decorate this function with pytest
to specify this argument to be with options True and False. This way pytest would test over all choices. See examples here:
gsplat/tests/test_rasterization.py
Lines 21 to 23 in 45d196a
@pytest.mark.parametrize("packed", [True, False]) | |
def test_rasterization( | |
per_view_color: bool, sh_degree: Optional[int], render_mode: str, packed: bool |
Test is failing because of format. Could you run
|
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! Thanks!
Pleasure doing business with you :) |
Added orthographic projection (nerfstudio-project#349)
Should be useful :)