Legacy offline reconstruction C++ Runner

[yuecideng]

initialize the workflow for the whole system.

Will make it ready for review when complete the migration of python version and performance test.

Update:
The whole system is nearly identical with python version of legacy reconstruction system, except the PoseEstiamtion part, where the python version uses opencv orb to find correspodences, and the cpp version uses fpfh to do this.

---

This change is 

[update-docs]

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

[yxlao]

Reviewed 6 of 6 files at r2, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @theNded and @yuecideng)

---

examples/cpp/CMakeLists.txt line 64 at r2 (raw file):

open3d_add_example(TriangleMesh)
open3d_add_example(LegacyReconstruction Open3D::3rdparty_jsoncpp Open3D::3rdparty_openmp)
if (TARGET Open3D::3rdparty_openmp)

Not needing if (TARGET Open3D::3rdparty_openmp) seems to work for your case. Could you remove this in TrimMeshBasedOnPointCloud as well and see if the CI passes?

---

examples/cpp/LegacyReconstruction.cpp line 172 at r2 (raw file):

    return 0;
}

Add an empty line to the end to avoid the warning.

---

examples/cpp/LegacyReconstructionUtil.h line 1 at r2 (raw file):

// ----------------------------------------------------------------------------

We'll discuss in meetings if we shall move this to the pipeline namespace. For example, create a directory called pipelines/reconstruction.

---

examples/cpp/LegacyReconstructionUtil.h line 42 at r2 (raw file):

// ============== Helper functions for file system ==============
std::string PadZeroToNumber(int num, int size) {

fmt:: can do this, e.g. https://stackoverflow.com/q/64305897/1255535.

---

examples/cpp/LegacyReconstructionUtil.h line 50 at r2 (raw file):

}

std::string ElapseTimeToHMS(double seconds) {

We can move this to cpp/open3d/utility/Timer.h.

For example:

double Timer::GetDurationInMillisecond() const; // Replace the old one. The old one is not clear about the unit.
double Timer::GetDurationInSecond() const; // Seconds are useful sometimes.
std::tuple<int, int, int> Timer::GetDurationInHMS() const; // Shall these be int?

---

examples/cpp/LegacyReconstructionUtil.h line 71 at r2 (raw file):

}

std::string FloatToString(float f, int precision = 3) {

Same, fmt can do that.

We use fmt library globally in Open3D, even in headers (since Logging.h is using it).

---

examples/cpp/LegacyReconstructionUtil.h line 77 at r2 (raw file):

}

std::string JoinPath(const std::string& path1, const std::string& path2) {

Move to cpp/open3d/utility/FileSystem.h and implement with https://en.cppreference.com/w/cpp/filesystem/path/append

---

examples/cpp/LegacyReconstructionUtil.h line 86 at r2 (raw file):

}

std::string JoinPath(const std::vector<std::string>& paths) {

Same, move to FileSystem.h/.cpp.

---

examples/cpp/LegacyReconstructionUtil.h line 94 at r2 (raw file):

}

std::string AddIfExist(const std::string& path,

This can be potentially implemented with https://stackoverflow.com/a/612176/1255535. Also move to FileSystem.h.

---

examples/cpp/LegacyReconstructionUtil.h line 1424 at r2 (raw file):

}  // namespace legacy_reconstruction
}  // namespace examples
}  // namespace open3d

Same. Add an empty line to the end to avoid the warning.

[yuecideng]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @theNded, @yuecideng, and @yxlao)

---

examples/cpp/CMakeLists.txt line 64 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

Not needing if (TARGET Open3D::3rdparty_openmp) seems to work for your case. Could you remove this in TrimMeshBasedOnPointCloud as well and see if the CI passes?

Do you mean only keeping this line open3d_add_example(TrimMeshBasedOnPointCloud Open3D::3rdparty_openmp)?

Maybe in CI WITH_OPENMP=ON is default so it will pass in my case? I will test it but I think adding if (TARGET Open3D::3rdparty_openmp) for them is reasonable...

---

examples/cpp/LegacyReconstructionUtil.h line 50 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

We can move this to cpp/open3d/utility/Timer.h.

For example:

double Timer::GetDurationInMillisecond() const; // Replace the old one. The old one is not clear about the unit.
double Timer::GetDurationInSecond() const; // Seconds are useful sometimes.
std::tuple<int, int, int> Timer::GetDurationInHMS() const; // Shall these be int?

Maybe std::tuple<int, int, double> Timer::GetDurationInHMS() const;? Because somtimes we would like to see second in more precisely.

[yuecideng]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @theNded, @yuecideng, and @yxlao)

---

examples/cpp/LegacyReconstructionUtil.h line 42 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

fmt:: can do this, e.g. https://stackoverflow.com/q/64305897/1255535.

Done

---

examples/cpp/LegacyReconstructionUtil.h line 50 at r2 (raw file):

Previously, yuecideng (Yueci Deng) wrote…

Maybe std::tuple<int, int, double> Timer::GetDurationInHMS() const;? Because somtimes we would like to see second in more precisely.

Done

---

examples/cpp/LegacyReconstructionUtil.h line 71 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

Same, fmt can do that.

We use fmt library globally in Open3D, even in headers (since Logging.h is using it).

Done

---

examples/cpp/LegacyReconstructionUtil.h line 77 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

Move to cpp/open3d/utility/FileSystem.h and implement with https://en.cppreference.com/w/cpp/filesystem/path/append

Done

---

examples/cpp/LegacyReconstructionUtil.h line 86 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

Same, move to FileSystem.h/.cpp.

Done

---

examples/cpp/LegacyReconstructionUtil.h line 94 at r2 (raw file):

Previously, yxlao (Yixing Lao) wrote…

This can be potentially implemented with https://stackoverflow.com/a/612176/1255535. Also move to FileSystem.h.

It seems that the original implementation is more straightforward.

[yxlao]

We have some separate discussions today:

1. We can move the example to the cpp/apps folder, as this is larger scale than an "example". We are providing an "application" for people to use with many config options. When we move to the cpp/apps folder, it is possible to create multiple files, multiple headers, making the code more structured.
2. We can move most functionality to open3d::pipelines::reconstruction namespace. The main function in the cpp/apps folder only serves as argument parsing and driver for the code. The main processing is done inside open3d::pipelines::reconstruction.
3. After 2. is done, we shall be ready to pybind open3d::pipelines::reconstruction and make the python-based reconstruction pipeline to use the same code. This will reduce code duplication.

We can achieve these step by step. For this PR, we may do step 1 first. We can discuss the design of 2. and 3. in future meetings to design it, before actual implementation.

[theNded]

We have some separate discussions today:

1. We can move the example to the cpp/apps folder, as this is larger scale than an "example". We are providing an "application" for people to use with many config options. When we move to the cpp/apps folder, it is possible to create multiple files, multiple headers, making the code more structured.
2. We can move most functionality to open3d::pipelines::reconstruction namespace. The main function in the cpp/apps folder only serves as argument parsing and driver for the code. The main processing is done inside open3d::pipelines::reconstruction.
3. After 2. is done, we shall be ready to pybind open3d::pipelines::reconstruction and make the python-based reconstruction pipeline to use the same code. This will reduce code duplication.

We can achieve these step by step. For this PR, we may do step 1 first. We can discuss the design of 2. and 3. in future meetings to design it, before actual implementation.

I disagree with 2 and 3. Python itself has a lot of flexible packages, from configargparse to torch and numpy. They are good for fast prototyping and debugging. Python examples can be self-included and reusable as reconstruction system modules.

If we ship yet another C++ pybind layer upon the existing pipeline modules, to maintain the same usability, we have to spend 10x time on C++, while the users will probably still stick to native python packages. We have to composite docs which are already lacking.

IMO it is better to spend time on refining modularized pipeline components (odometry, integrate, slac) than adding another layer to the existing layers.

[yuecideng]

We can move the example to the cpp/apps folder, as this is larger scale than an "example". We are providing an "application" for people to use with many config options.

This is a good idea, and maybe we can add more interesting and reusable applications in the future. I would like to keep contributing in this part 👀 . But my concern is that, we also have some examples (both c++ and python) that are more like a application (eg. python legacy reconstruction, c++ online/offlone slam. etc). I think they should be moved to apps as well. So maybe we should create open3d/apps/python and open3d/apps/cpp? Just a suggestion, we could discuss this in meeting.

For 2 and 3, I'm also glad to discuss them and see the pros and cons of doing this.

[yuecideng]

1. Move LegacyReconstruction to cpp/apps folder and change name into OfflineReconstruction, which make it compatible wit h tensor based offline reconstruction in the future.
2. Seperate some helper functions into several header files to make the code more structured.
3. Currently the OfflineReconstruction command line executable can be found in /bin/OfflineReconstruction/. The output path I think should be designed for a better place.
4. Currently the installation is not supported.

[yxlao]

macro(open3d_add_app_common SRC_DIR APP_NAME TARGET_NAME)

endmacro()

target_sources(xxx PRIVATE
    xxx.cpp
    xxx.cpp
)

[yuecideng]

It seems that we must add source file in add_executable, so I use the original method (GLOB) to sum up the source files inside open3d_add_app_common