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

Revisiting #1504 and fix colmap world coordinate transform #2793

Merged
merged 2 commits into from
Jan 19, 2024

Conversation

jb-ye
Copy link
Collaborator

@jb-ye jb-ye commented Jan 19, 2024

Okay, I spent some time to research how we maintain world coordinate conventions between colmap and nerfstudio, and I found the current implementation is not correct (original introduced in a very earlier commit #338 ). Let me first describe my understanding, then what the existing code does and finally what should be the fix. This fix addresses a historic issue: #1504

The Problem

Given a c2w transform read from colmap, we need apply two transforms, one multiplied from right, one multiplied from left.

  1. opencv to opengl convention transform A which was for changing camera coordinate
  2. colmap to nerfstudio convention transform B which was for changing world coordinate
    Ideally, the code should look like
c2w_new = B * c2w * A

Here

A=array([
       [ 1, 0,  0,  0],
       [ 0, -1,  0,  0],
       [ 0, 0, -1,  0],
       [ 0, 0,  0,  1]])

Now, let's look at the existing code

# effectively multiply A from right
c2w[0:3, 1:3] *= -1
# effectively multiply B from left
c2w = c2w[np.array([1, 0, 2, 3]), :]
c2w[2, :] *= -1

Here as implemented in the current code:

# incorrect
B=array([
       [ 0, 1,  0,  0],
       [ 1, 0,  0,  0],
       [ 0, 0, -1,  0],
       [ 0, 0,  0,  1]])

However, the second step was not correctly implemented. Because in Colmapworld coordinate convention, the y direction is often interpreted as DOWN direction. This is because the bundle adjustment optimization often starts from using the first opencv camera frame as world frame. Assuming the optimization hasn't deviate the first frame's pose a lot, the y direction in colmap world would be close to DOWN direction.

In order to transform the world convention from colmap to nerfstudio, the correct B should be

# the fix in this PR
B=array([
       [ 1, 0,  0,  0],
       [ 0, 0,  1,  0],
       [ 0, -1, 0,  0],
       [ 0, 0,  0,  1]])

where we map (-y) in colmap world to z in nerfstudio world. And the code should be corrected effectively as

c2w[0:3, 1:3] *= -1
c2w = c2w[np.array([0, 2, 1, 3]), :]
c2w[2, :] *= -1

Later in nerfstudio commit history, we introduce new feature --orientation_method in colmap parser, and the choice of B becomes irrelevant if the orientation_method is not none. The viewer will always display correct orientation as long as the orientation_method is not none. So in order to test my fix, I run ns-train with ns-train gaussian-splatting ... colmap --orientation_method=none and open up my viewer to verify the initial world coordinate setup. See pictures below on mipnerf360 bicycle dataset:

Without fix
Screenshot from 2024-01-18 23-15-30

With fix

Screenshot from 2024-01-18 22-07-33

Summary of this change

  • [No Effect] If orientation_method is not none (by default it is set to up), the change of this PR has no material impact on outcomes of train (e.g. checkpoint, ply exporter) or viewer. The scene will be oriented as instructed by the parameter. For regular user, whom never touch this parameter other than the default setting, this PR has no impacts for them.
  • [Effect 1] If orientation_method is none, this is a case we want to properly transform the world coordinate of colmap project to nerfstudio convention.
  • [Effect 2] We introduce a new option to colmap parser (assume_colmap_world_coordinate_convention): If orientation_method is none, and assume_colmap_world_coordinate_convention is False (by default it is true), we will not apply B transform at all.

Why we need this change?

Users may want to orient the scene using some prior knowledge instead of using what is provided by the nerfstudio, therefore we need effect 1. User may want to orient the scene using nerfstudio world convention instead of colmap convention, then we need effect 2.

Other related issues that can find answer in this PR:

#2784

@jb-ye
Copy link
Collaborator Author

jb-ye commented Jan 19, 2024

@kerrj @tancik Could you take a look? This is a fix for a historic issue.

@brentyi
Copy link
Collaborator

brentyi commented Jan 19, 2024

Nit: I think you have a typo in your A matrix. (the x component should be positive).

But the details in the PR are appreciated and I agree with what's written.

@brentyi brentyi enabled auto-merge (squash) January 19, 2024 04:53
@jb-ye
Copy link
Collaborator Author

jb-ye commented Jan 19, 2024

Nit: I think you have a typo in your A matrix. (the x component should be positive).

But the details in the PR are appreciated and I agree with what's written.

I fixed the typo in main thread. Thx

@brentyi brentyi merged commit 25a00a0 into nerfstudio-project:main Jan 19, 2024
4 checks passed
@jb-ye jb-ye mentioned this pull request Jan 19, 2024
ArpegorPSGH pushed a commit to ArpegorPSGH/nerfstudio that referenced this pull request Jun 22, 2024
@desifisker
Copy link

so, for instance, now we can geo-register images in colmap, obtain the cameras.bin and images.bin and train a nerfacto model that will be correctly scaled to the size of the real world (same coord system/scale as colmap) using effect 1 and this update?

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.

3 participants