Move vkb::filesystem to components

tomadamatkinson commented 9 months ago

Description

Simplifies our filesystem implementation by using std::filesystem underneath. This is the filesystem used in #846 which can be easily used in different executables to perform small tasks (See shader_reflector and variant_reflector. Moved to components and can be used using vkb__filesystem. Filesystem is influenced by the decisions made on the framework/2.0 branch

The existing "legacy" api is kept to maintain compatibility with samples. In the future asset loaders may choose to use the filesystem directly with vkb::filesystem::get()

Merging this will help simplify the review on #846

Tested on MacOS (M1), Linux (Ubuntu 22.04), Windows (11) and Android (Pixel 3). Samples work as expected

General Checklist:

Please ensure the following points are checked:

[x] My code follows the coding style
[x] I have reviewed file licenses
[x] I have commented any added functions (in line with Doxygen)
[x] I have commented any code that could be hard to understand
[x] My changes do not add any new compiler warnings
[x] My changes do not add any new validation layer errors or warnings
[x] I have used existing framework/helper functions where possible
[x] My changes do not add any regressions
[x] I have tested every sample to ensure everything runs correctly
[x] This PR describes the scope and expected impact of the changes I am making

Note: The Samples CI runs a number of checks including:
- [ ] I have updated the header Copyright to reflect the current year (CI build will fail if Copyright is out of date)
- [ ] My changes build on Windows, Linux, macOS and Android. Otherwise I have documented any exceptions

Sample Checklist

If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist:

[ ] I have tested the sample on at least one compliant Vulkan implementation
[ ] If the sample is vendor-specific, I have tagged it appropriately
[ ] I have stated on what implementation the sample has been tested so that others can test on different implementations and platforms
[ ] Any dependent assets have been merged and published in downstream modules
[ ] For new samples, I have added a paragraph with a summary to the appropriate chapter in the samples readme
[ ] For new samples, I have added a tutorial README.md file to guide users through what they need to know to implement code using this feature. For example, see conditional_rendering
[ ] For new samples, I have added a link to the Antora navigation so that the sample will be listed at the Vulkan documentation site

gary-sweet commented 9 months ago

I'm seeing a couple of build problems. The first is a redefinition of LOGE:

[100%] Building CXX object app/CMakeFiles/vulkan_samples.dir/main.cpp.o
In file included from Vulkan-Samples/components/filesystem/include/filesystem/filesystem.hpp:26,
                 from Vulkan-Samples/app/main.cpp:23:
Vulkan-Samples/components/core/include/core/util/logging.hpp:28: warning: "LOGE" redefined
   28 | #define LOGE(...) spdlog::error("{}", fmt::format(__VA_ARGS__));
      | 
In file included from Vulkan-Samples/app/main.cpp:18:
Vulkan-Samples/framework/common/logging.h:35: note: this is the location of the previous definition
   35 | #define LOGE(...) spdlog::error("[{}:{}] {}", __FILENAME__, __LINE__, fmt::format(__VA_ARGS__));

gary-sweet commented 9 months ago

The second issue is that std::filesystem isn't available before gcc8 unless you explicitly include stdc++fs as a link library.

We've had to work around that a number of times using something like:

if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.0)
   target_link_libraries(${TGT} PRIVATE stdc++fs)
endif()

tomadamatkinson commented 8 months ago

I'm seeing a couple of build problems. The first is a redefinition of LOGE:

[100%] Building CXX object app/CMakeFiles/vulkan_samples.dir/main.cpp.o
In file included from Vulkan-Samples/components/filesystem/include/filesystem/filesystem.hpp:26,
                 from Vulkan-Samples/app/main.cpp:23:
Vulkan-Samples/components/core/include/core/util/logging.hpp:28: warning: "LOGE" redefined
   28 | #define LOGE(...) spdlog::error("{}", fmt::format(__VA_ARGS__));
      | 
In file included from Vulkan-Samples/app/main.cpp:18:
Vulkan-Samples/framework/common/logging.h:35: note: this is the location of the previous definition
   35 | #define LOGE(...) spdlog::error("[{}:{}] {}", __FILENAME__, __LINE__, fmt::format(__VA_ARGS__));

Due to a conflict with logging.h and logging.hpp I will remove the original version in favour of the one that is ported to the components folders

tomadamatkinson commented 8 months ago

Thanks all for taking the time to review! Branch is updated

KhronosGroup / Vulkan-Samples