Vulkan-Samples
Vulkan-Samples copied to clipboard
KhronosGroup
Framework includes multiple versions of json.hpp
Both tinygltf and hwcpipe embed different versions of the nlohmann json.hpp header in their respective source trees, and both of them use the header publicly so downstream users get one or the other depending of the dependency order.
Further, the two versions use different names for their include guards (NLOHMANN_JSON_HPP from tinygltf and INCLUDE_NLOHMANN_JSON_HPP_ from hwcpipe, which is the newer of the two), so it's possible that files end up with both includes.
A short term fix would be to update to a newer version of tinygltf, which would result in both headers using the same include guard and therefore should prevent inclusion of both files into any individual source file. A longer term fix would be to address #893 and ensure the packaging library correctly forces both hwcpipe and tinygltf to depend on the same version of nlohmann, rather than embedding different versions.
An alternative for tinygltf is to define TINYGLTF_NO_INCLUDE_JSON and use the version of nlohmann json that we want. In either case, we likely need to bump our tinygltf version as its 5 years old!
Not sure where HWCPipe includes nlohmann json. Looked through its source tree and couldn't find it. Thats not to say it isn't there!
We are wanting to improve the gltf loader to support more gltf extensions. When we do this, that would be a good time to resolve this issue
I second that. While tinygltf is indeed outdated, we don't actually use any functionality that would require us an update. On the other hand updating would require us the test all the samples and make sure stuff still works as expected. Just like Tom this should be part of our larger framework rework roadmap.
The version of TinyGLTF currently embedded doesn't support TINYGLTF_NO_INCLUDE_JSON, however, I do notice now that it's inclusion of json.hpp only happens inside a #if defined(TINYGLTF_IMPLEMENTATION) block, so the problem is significantly more tractable as its simply a matter of declaring the proper include guard define inside the gltf_loader.cpp right after #define TINYGLTF_IMPLEMENTATION to ensure the outdated JSON header isn't included along with the one from hwcpipe.
I guess we fixed this by accident? I can only find the json.hpp used by tinygltf on the current main branch. If so we can safely close this,
Yes, the hcwpipe update changed it so that only one json parser is in the path now.