-
Notifications
You must be signed in to change notification settings - Fork 257
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
Fix missing +0.5 in calculating uv coordinate #151
Conversation
My experiment GSplat 0.1.8 vs 0.1.9 vs 0.1.10 (your PR)Red = GSplat 0.1.10
Truck (251 images @ 1957 x 1051 already undistorted with colmap )Purancak (my private dataset, 599 images @ 1995 x 1494 already undistorted with colmap) |
@jb-ye I don't think this is correct. Whether or not the center of the top left pixel is (0.5, 0.5) or (0.0, 0.0) is a matter of convention. In OpenGL, it's (0.5, 0.5), in OpenCV, for example, it's (0, 0). (EDIT: see links). Which one is correct? It depends on the camera calibration. AFAIK,
Links:
|
@ichsan2895 Just want to confirm, your results are based on colmap, right? |
@oseiskar I did some research, here are my findings: (1) nerfstudio assume +0.5 offset for all image coordinates when applying calibration intrinsics. See this line. (2) calibration software actually use the convention of feature detection outputs to decide if it wants to apply +0.5. For example, as described here: colmap assume the center of top-left pixel to be (0.5, 0.5) because its feature detection inputs follow the same convention. In section: https://colmap.github.io/tutorial.html#feature-detection-and-extraction it says
Kalibr uses AprilTag detection, and detected coordinate will add +0.5 offset here. (3) This observation is further confirmed by how the those libraries implement rescale function (e.g. here for colmap and here for nerfstudio). If and only if they assume the image coordinate offset to be +0.5, one can apply the simple rescale formula (4) PR #97 actually change the behavior of project_gaussians() and we observe slight decrease in splatfacto quality. |
Yes, colmap 3.8 with CUDA. |
@jb-ye Thank you for pointing that out 👍 I stand corrected. So apparently only OpenCV uses the (0,0) convention and even there it's a bit unclearly documented (opencv/opencv#10130). Another possibility could be just changing the calibration. Subtracting -0.5 from principal point X/Y could do the same and it could be possible to support both (0,0) and (0.5,0.5) conventions if necessary. However, the approach in this PR is cleaner if the 0.5,0.5 convention is consistently followed in the gsplat & Nerfstudio codebases. Perhaps documenting this more clearly than most software does could be good extra improvement. |
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 seems reasonable to me, thank you for bringing this. for future reference, let's document this somewhere.
In previous PR #97 I realized the author didn't add back the +0.5 term in the rasterize_forward() calls.
Just to clarify we need this 0.5 term. For an image, a pixel whose index is at (i, j) represents an integrated box of area 1, whose center is actually at (i + 0.5, j + 0.5). So when computing a point (x, y, z) whose projection locates at pixel (i, j), the following equation is established:
For example, given a image of width 999x999 and cx = 499.5 and cy=499.5 (half width, height) The pixel index at (499, 499) corresponds to the exact principal point of the image.
In our previous implementation before #97, we minus 0.5 in the xys, which I think doesn't make enough sense, either. The proper way is to add 0.5 to the px, py in rasterize_forward() calls. Without resolving this issue, the project_gaussian() and rasterize_forward() are mismatched by 0.5 pixel and causing splatfacto training to be degraded.
This PR also bump the version to 0.1.10 since this is important bug fix for splatfacto.