Closed pablospe closed 9 months ago
@Phil26AT @sarlinpe I was wondering if you could take a look to this PR.
LGTM. I don't use Docker so I don't really understand all these details.
@pablospe Since COLMAP 3.9.1 the quaternions follow the Eigen format (xyzw instead of wxyz) but this wasn't updated here - so the user need to manually handle any conversion.
Yes, you are right, this is not handle it here. Should we revert these changes?
I'd rather revert for now and do a proper upgrade in https://github.com/cvg/pyceres/pull/27 - can you submit one and keep the Docker stuff if you need it?
Yes, it makes sense to revert. I'll create another PR with only the Dockerfile. You could also take the changes from this PR, like the rename of WorldToImage for ImgFromCam: see in bundle.cc.
One question, it only uses only the camera models from Colmap, right? I was wondering if it would make sense to simply take a copy the models.h
to avoid Colmap dependency in this repo? It is like a circular dependency, for compiling Colmap you need ceres, and for pyceres you need Colmap :)
My plan is to move the bundle & PGO cost functions to pycolmap, such that pyceres handles only the core features without any dependency on COLMAP. But this requires some updates upstream to add covariances.
Related PR (for the Dockerfile part): https://github.com/cvg/pyceres/pull/31
This PR updates the changes made to Colmap 3.9 and adds a Dockerfile. Usage:
A possible improvement for a future pull request is to create a runtime docker stage that only installs the essential dependencies and uses the compiled ceres-solver and colmap from the builder stage (this will reduce the docker image size considerably).