Use fmt library in compiled (non-header-only) mode

[yxlao]

Current issue

Currently, master branch fails to build on on Windows 11 + CUDA 11.x with the latest cub:

  GlobalOptimization.cpp
C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v11.6\include\cub/device/dispatch/dispatch_segmented_sort.cuh(338): error : invalid combination of type
specifiers [C:\Users\yixing\repo\Open3D\build\cpp\open3d\core\core.vcxproj]

C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v11.6\include\cub/device/dispatch/dispatch_segmented_sort.cuh(338): error : expected an identifier [C:\U
sers\yixing\repo\Open3D\build\cpp\open3d\core\core.vcxproj]

C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v11.6\include\cub/device/dispatch/dispatch_segmented_sort.cuh(379): error : expected a member name [C:\U
sers\yixing\repo\Open3D\build\cpp\open3d\core\core.vcxproj]

This is the same issue as described https://github.com/ptheywood/cub-1.15-windows.h-demo

#if defined(_WIN32)
    #include <Windows.h>
#endif
#include <cub/cub.cuh>
int main(int argc, char* argv[]) {
    return EXIT_SUCCESS;
}

D:/a/jitify-cub-1.15-MSVC-demo/jitify-cub-1.15-MSVC-demo/build/_deps/thrust-src/dependencies/cub\cub/device/dispatch/dispatch_segmented_sort.cuh(338): error : invalid combination of type specifiers [D:\a\jitify-cub-1.15-MSVC-demo\jitify-cub-1.15-MSVC-demo\build\jitify-before-cub.vcxproj]
D:/a/jitify-cub-1.15-MSVC-demo/jitify-cub-1.15-MSVC-demo/build/_deps/thrust-src/dependencies/cub\cub/device/dispatch/dispatch_segmented_sort.cuh(338): error : expected an identifier [D:\a\jitify-cub-1.15-MSVC-demo\jitify-cub-1.15-MSVC-demo\build\jitify-before-cub.vcxproj]
D:/a/jitify-cub-1.15-MSVC-demo/jitify-cub-1.15-MSVC-demo/build/_deps/thrust-src/dependencies/cub\cub/device/dispatch/dispatch_segmented_sort.cuh(379): error : expected a member name [D:\a\jitify-cub-1.15-MSVC-demo\jitify-cub-1.15-MSVC-demo\build\jitify-before-cub.vcxproj]

Solution

The main idea is to avoid winows.h being introduced to files as much as possible. Currently, windows.h is introduced by fmt, when fmt is used as header-only mode. If we use fmt in compiled mode, it will not propagate the windows.h header.

References:

• Faster compilation: https://stackoverflow.com/a/68020236/1255535
• Avoid system header such as windows.h: https://stackoverflow.com/a/67993640/1255535

Notes

Currently, we still require #4691. In #4691, the windows.h is directly used in FileSystem.cpp. To avoid windows.h in FileSystem.cpp, we'll need to upgrade to C++17 and std::filesystem, which is future work.

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.

[errissa]

This PR introduces another 3rdparty library configuration and build step which (slightly) increases the Open3D build time. Given that fmt is available as header-only I think there has to be a good reason to prefer building it. Is there no alternative to avoiding the windows.h dependency?

Reviewed 4 of 8 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @yxlao)

---

cpp/open3d/core/SizeVector.cpp, line 29 at r2 (raw file):

#include "open3d/core/SizeVector.h"

#include <algorithm>

why did this become necessary?

---

cpp/open3d/core/Tensor.h, line 29 at r2 (raw file):

#pragma once

#include <algorithm>

See previous comment.

---

cpp/open3d/utility/Logging.cpp, line 31 at r2 (raw file):

#include <fmt/core.h>
#include <fmt/printf.h>
#include <fmt/ranges.h>

Aren't these redundant? They are included in Logging.h

[yxlao]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @errissa)

---

cpp/open3d/core/SizeVector.cpp, line 29 at r2 (raw file):
During debugging, when I removed all fmt headers, the compiler complains about missing the <algorithm> header here.

<algorithm> is brought in by the fmt headers. If Logging.h does not use fmt headers at all, we will need to include <algorithm> here.

I was following this documentation: https://google.github.io/styleguide/cppguide.html#Include_What_You_Use:

Do not rely on transitive inclusions. This allows people to remove no-longer-needed #include statements from their headers without breaking clients. This also applies to related headers - foo.cc should include bar.h if it uses a symbol from it even if foo.h includes bar.h.

---

cpp/open3d/core/Tensor.h, line 29 at r2 (raw file):

Previously, errissa (Rene) wrote…

See previous comment.

Same as above.

---

cpp/open3d/utility/Logging.cpp, line 31 at r2 (raw file):
Same as above.

In https://google.github.io/styleguide/cppguide.html#Include_What_You_Use :

This also applies to related headers - foo.cc should include bar.h if it uses a symbol from it even if foo.h includes bar.h.

[errissa]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yxlao)

[JackBoosY]

To anyone who met this issue again:
I was posted this issue in nvidia side: https://developer.nvidia.com/nvidia_bug/3597169
And the member told me this issue was fixed by NVIDIA/cub#423, it should be fine in cub 1.16.0.