From ab18be967ae341dbba0eb35cbbf35492456f159f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20M=C3=BCller?= <34514239+appgurueu@users.noreply.github.com> Date: Fri, 19 Jan 2024 15:52:20 +0100 Subject: [PATCH] Move from tinygltf to tiniergltf (#13) * Remove tinygltf * Integrate tiniergltf * Update build.yml to include jsoncpp * Namespace target * Undo noexcept removal, readd move constructor * Remove debug throw * Remove now obsolete build code * Bump CMake minimum version to 3.12 * Fix oops * Remove unnecessary install/export * Remove tinygltf from Config.cmake.in * Take inspiration from Minetest's FindJson.cmake * Move tiniergltf to separate repo * CI: Install git * Bump tiniergltf version (obtain jsoncpp via FetchContent) * Remove jsoncpp from build dependency list --- Config.cmake.in | 3 - source/Irrlicht/CGLTFMeshFileLoader.cpp | 162 ++++++++++++------------ source/Irrlicht/CGLTFMeshFileLoader.h | 21 ++- source/lib/tinygltf/CMakeLists.txt | 65 ---------- src/CMakeLists.txt | 25 ++-- 5 files changed, 100 insertions(+), 176 deletions(-) delete mode 100644 source/lib/tinygltf/CMakeLists.txt diff --git a/Config.cmake.in b/Config.cmake.in index 7d5417483..6a55a6f67 100644 --- a/Config.cmake.in +++ b/Config.cmake.in @@ -2,9 +2,6 @@ include(CMakeFindDependencyMacro) -find_package(tinygltf 2.6 REQUIRED - PATHS "${CMAKE_CURRENT_LIST_DIR}" - if(NOT TARGET IrrlichtMt::IrrlichtMt) # private dependency only explicitly needed with static libs if(@USE_SDL2@ AND NOT @BUILD_SHARED_LIBS@) diff --git a/source/Irrlicht/CGLTFMeshFileLoader.cpp b/source/Irrlicht/CGLTFMeshFileLoader.cpp index d347a2921..2d19cadba 100644 --- a/source/Irrlicht/CGLTFMeshFileLoader.cpp +++ b/source/Irrlicht/CGLTFMeshFileLoader.cpp @@ -12,15 +12,14 @@ #include "SColor.h" #include "SMesh.h" #include "vector3d.h" - -#define TINYGLTF_IMPLEMENTATION -#include - #include #include #include +#include +#include #include #include +#include #include /* Notes on the coordinate system. @@ -81,13 +80,23 @@ bool CGLTFMeshFileLoader::isALoadableFileExtension( */ IAnimatedMesh* CGLTFMeshFileLoader::createMesh(io::IReadFile* file) { - tinygltf::Model model {}; - - if (file->getSize() <= 0 || !tryParseGLTF(file, model)) { + if (file->getSize() <= 0) { + return nullptr; + } + std::optional model = tryParseGLTF(file); + if (!model.has_value()) { return nullptr; } - MeshExtractor parser(std::move(model)); + if (!(model->buffers.has_value() + && model->bufferViews.has_value() + && model->accessors.has_value() + && model->meshes.has_value() + && model->nodes.has_value())) { + return nullptr; + } + + MeshExtractor parser(std::move(model.value())); SMesh* baseMesh(new SMesh {}); loadPrimitives(parser, baseMesh); @@ -123,13 +132,13 @@ void CGLTFMeshFileLoader::loadPrimitives( } CGLTFMeshFileLoader::MeshExtractor::MeshExtractor( - const tinygltf::Model& model) noexcept + const tiniergltf::GlTF& model) noexcept : m_model(model) { } CGLTFMeshFileLoader::MeshExtractor::MeshExtractor( - const tinygltf::Model&& model) noexcept + const tiniergltf::GlTF&& model) noexcept : m_model(model) { } @@ -141,11 +150,14 @@ std::vector CGLTFMeshFileLoader::MeshExtractor::getIndices( const std::size_t meshIdx, const std::size_t primitiveIdx) const { + // FIXME this need not exist. What do we do if it doesn't? const auto accessorIdx = getIndicesAccessorIdx(meshIdx, primitiveIdx); - const auto& buf = getBuffer(accessorIdx); + + const auto& buf = getBuffer(accessorIdx.value()); + // FIXME check accessor type (which could also be u8 or u32). std::vector indices{}; - const auto count = getElemCount(accessorIdx); + const auto count = getElemCount(accessorIdx.value()); for (std::size_t i = 0; i < count; ++i) { std::size_t elemIdx = count - i - 1; indices.push_back(readPrimitive( @@ -170,14 +182,14 @@ std::vector CGLTFMeshFileLoader::MeshExtractor::getVertices( const auto normalAccessorIdx = getNormalAccessorIdx( meshIdx, primitiveIdx); - if (normalAccessorIdx != static_cast(-1)) { - copyNormals(normalAccessorIdx, vertices); + if (normalAccessorIdx.has_value()) { + copyNormals(normalAccessorIdx.value(), vertices); } const auto tCoordAccessorIdx = getTCoordAccessorIdx( meshIdx, primitiveIdx); - if (tCoordAccessorIdx != static_cast(-1)) { - copyTCoords(tCoordAccessorIdx, vertices); + if (tCoordAccessorIdx.has_value()) { + copyTCoords(tCoordAccessorIdx.value(), vertices); } return vertices; @@ -188,7 +200,7 @@ std::vector CGLTFMeshFileLoader::MeshExtractor::getVertices( */ std::size_t CGLTFMeshFileLoader::MeshExtractor::getMeshCount() const { - return m_model.meshes.size(); + return m_model.meshes->size(); } /** @@ -197,7 +209,7 @@ std::size_t CGLTFMeshFileLoader::MeshExtractor::getMeshCount() const std::size_t CGLTFMeshFileLoader::MeshExtractor::getPrimitiveCount( const std::size_t meshIdx) const { - return m_model.meshes[meshIdx].primitives.size(); + return m_model.meshes->at(meshIdx).primitives.size(); } /** @@ -314,10 +326,15 @@ void CGLTFMeshFileLoader::MeshExtractor::copyTCoords( core::vector3df CGLTFMeshFileLoader::MeshExtractor::getScale() const { core::vector3df buffer{1.0f,1.0f,1.0f}; - if (m_model.nodes[0].scale.size() == 3) { - buffer.X = static_cast(m_model.nodes[0].scale[0]); - buffer.Y = static_cast(m_model.nodes[0].scale[1]); - buffer.Z = static_cast(m_model.nodes[0].scale[2]); + // FIXME this just checks the first node + const auto &node = m_model.nodes->at(0); + // FIXME this does not take the matrix into account + // (fix: properly map glTF -> Irrlicht node hierarchy) + if (std::holds_alternative(node.transform)) { + const auto &trs = std::get(node.transform); + buffer.X = static_cast(trs.scale[0]); + buffer.Y = static_cast(trs.scale[1]); + buffer.Z = static_cast(trs.scale[2]); } return buffer; } @@ -331,7 +348,7 @@ core::vector3df CGLTFMeshFileLoader::MeshExtractor::getScale() const std::size_t CGLTFMeshFileLoader::MeshExtractor::getElemCount( const std::size_t accessorIdx) const { - return m_model.accessors[accessorIdx].count; + return m_model.accessors->at(accessorIdx).count; } /** @@ -344,9 +361,10 @@ std::size_t CGLTFMeshFileLoader::MeshExtractor::getElemCount( std::size_t CGLTFMeshFileLoader::MeshExtractor::getByteStride( const std::size_t accessorIdx) const { - const auto& accessor = m_model.accessors[accessorIdx]; - const auto& view = m_model.bufferViews[accessor.bufferView]; - return accessor.ByteStride(view); + const auto& accessor = m_model.accessors->at(accessorIdx); + // FIXME this does not work with sparse / zero-initialized accessors + const auto& view = m_model.bufferViews->at(accessor.bufferView.value()); + return view.byteStride.value_or(accessor.elementSize()); } /** @@ -359,7 +377,7 @@ std::size_t CGLTFMeshFileLoader::MeshExtractor::getByteStride( bool CGLTFMeshFileLoader::MeshExtractor::isAccessorNormalized( const std::size_t accessorIdx) const { - const auto& accessor = m_model.accessors[accessorIdx]; + const auto& accessor = m_model.accessors->at(accessorIdx); return accessor.normalized; } @@ -370,9 +388,10 @@ bool CGLTFMeshFileLoader::MeshExtractor::isAccessorNormalized( CGLTFMeshFileLoader::BufferOffset CGLTFMeshFileLoader::MeshExtractor::getBuffer( const std::size_t accessorIdx) const { - const auto& accessor = m_model.accessors[accessorIdx]; - const auto& view = m_model.bufferViews[accessor.bufferView]; - const auto& buffer = m_model.buffers[view.buffer]; + const auto& accessor = m_model.accessors->at(accessorIdx); + // FIXME this does not work with sparse / zero-initialized accessors + const auto& view = m_model.bufferViews->at(accessor.bufferView.value()); + const auto& buffer = m_model.buffers->at(view.buffer); return BufferOffset(buffer.data, view.byteOffset); } @@ -385,11 +404,11 @@ CGLTFMeshFileLoader::BufferOffset CGLTFMeshFileLoader::MeshExtractor::getBuffer( * Type: Integer * Required: NO */ -std::size_t CGLTFMeshFileLoader::MeshExtractor::getIndicesAccessorIdx( +std::optional CGLTFMeshFileLoader::MeshExtractor::getIndicesAccessorIdx( const std::size_t meshIdx, const std::size_t primitiveIdx) const { - return m_model.meshes[meshIdx].primitives[primitiveIdx].indices; + return m_model.meshes->at(meshIdx).primitives[primitiveIdx].indices; } /** @@ -397,13 +416,15 @@ std::size_t CGLTFMeshFileLoader::MeshExtractor::getIndicesAccessorIdx( * Documentation: https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#meshes-overview * Type: VEC3 (Float) * ! Required: YES (Appears so, needs another pair of eyes to research.) + * Second pair of eyes says: "When positions are not specified, client implementations SHOULD skip primitive’s rendering" */ std::size_t CGLTFMeshFileLoader::MeshExtractor::getPositionAccessorIdx( const std::size_t meshIdx, const std::size_t primitiveIdx) const { - return m_model.meshes[meshIdx].primitives[primitiveIdx] - .attributes.find("POSITION")->second; + // FIXME position-less primitives should be skipped. + return m_model.meshes->at(meshIdx).primitives[primitiveIdx] + .attributes.position.value(); } /** @@ -412,70 +433,53 @@ std::size_t CGLTFMeshFileLoader::MeshExtractor::getPositionAccessorIdx( * Type: VEC3 (Float) * ! Required: NO (Appears to not be, needs another pair of eyes to research.) */ -std::size_t CGLTFMeshFileLoader::MeshExtractor::getNormalAccessorIdx( +std::optional CGLTFMeshFileLoader::MeshExtractor::getNormalAccessorIdx( const std::size_t meshIdx, const std::size_t primitiveIdx) const { - const auto& attributes = m_model.meshes[meshIdx] - .primitives[primitiveIdx].attributes; - const auto result = attributes.find("NORMAL"); - - if (result == attributes.end()) { - return -1; - } else { - return result->second; - } + return m_model.meshes->at(meshIdx).primitives[primitiveIdx].attributes.normal; } /** - * The index of the accessor that contains the NORMALs. + * The index of the accessor that contains the TEXCOORDs. * Documentation: https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#meshes-overview * Type: VEC3 (Float) * ! Required: YES (Appears so, needs another pair of eyes to research.) */ -std::size_t CGLTFMeshFileLoader::MeshExtractor::getTCoordAccessorIdx( +std::optional CGLTFMeshFileLoader::MeshExtractor::getTCoordAccessorIdx( const std::size_t meshIdx, const std::size_t primitiveIdx) const { - const auto& attributes = m_model.meshes[meshIdx] - .primitives[primitiveIdx].attributes; - const auto result = attributes.find("TEXCOORD_0"); - - if (result == attributes.end()) { - return -1; - } else { - return result->second; - } + const auto& texcoords = m_model.meshes->at(meshIdx).primitives[primitiveIdx].attributes.texcoord; + if (!texcoords.has_value()) + return std::nullopt; + return texcoords->at(0); } /** - * This is where the actual model's GLTF file is loaded and parsed by tinygltf. + * This is where the actual model's GLTF file is loaded and parsed by tiniergltf. */ -bool CGLTFMeshFileLoader::tryParseGLTF(io::IReadFile* file, - tinygltf::Model& model) +std::optional CGLTFMeshFileLoader::tryParseGLTF(io::IReadFile* file) { - tinygltf::TinyGLTF loader {}; - - // Stop embedded textures from making model fail to load - loader.SetImageLoader(nullptr, nullptr); - - std::string err {}; - std::string warn {}; - - auto buf = std::make_unique(file->getSize()); - file->read(buf.get(), file->getSize()); - - if (warn != "") { - os::Printer::log(warn.c_str(), ELL_WARNING); - } - - if (err != "") { - os::Printer::log(err.c_str(), ELL_ERROR); - return false; + auto size = file->getSize(); + auto buf = std::make_unique(size + 1); + file->read(buf.get(), size); + // We probably don't need this, but add it just to be sure. + buf[size] = '\0'; + Json::CharReaderBuilder builder; + const std::unique_ptr reader(builder.newCharReader()); + Json::Value json; + JSONCPP_STRING err; + if (!reader->parse(buf.get(), buf.get() + size, &json, &err)) { + return std::nullopt; + } + try { + return tiniergltf::GlTF(json); + } catch (const std::runtime_error &e) { + return std::nullopt; + } catch (const std::out_of_range &e) { + return std::nullopt; } - - return loader.LoadASCIIFromString(&model, &err, &warn, buf.get(), - file->getSize(), "", 1); } } // namespace scene diff --git a/source/Irrlicht/CGLTFMeshFileLoader.h b/source/Irrlicht/CGLTFMeshFileLoader.h index 3a9970d7f..5fd49e4e7 100644 --- a/source/Irrlicht/CGLTFMeshFileLoader.h +++ b/source/Irrlicht/CGLTFMeshFileLoader.h @@ -11,7 +11,7 @@ #include "vector2d.h" #include "vector3d.h" -#include +#include #include #include @@ -52,9 +52,9 @@ private: public: using vertex_t = video::S3DVertex; - MeshExtractor(const tinygltf::Model& model) noexcept; + MeshExtractor(const tiniergltf::GlTF& model) noexcept; - MeshExtractor(const tinygltf::Model&& model) noexcept; + MeshExtractor(const tiniergltf::GlTF&& model) noexcept; /* Gets indices for the given mesh/primitive. * @@ -71,7 +71,7 @@ private: std::size_t getPrimitiveCount(const std::size_t meshIdx) const; private: - tinygltf::Model m_model; + tiniergltf::GlTF m_model; template static T readPrimitive(const BufferOffset& readFrom); @@ -110,31 +110,26 @@ private: BufferOffset getBuffer(const std::size_t accessorIdx) const; - std::size_t getIndicesAccessorIdx(const std::size_t meshIdx, + std::optional getIndicesAccessorIdx(const std::size_t meshIdx, const std::size_t primitiveIdx) const; std::size_t getPositionAccessorIdx(const std::size_t meshIdx, const std::size_t primitiveIdx) const; /* Get the accessor id of the normals of a primitive. - * - * -1 is returned if none are present. */ - std::size_t getNormalAccessorIdx(const std::size_t meshIdx, + std::optional getNormalAccessorIdx(const std::size_t meshIdx, const std::size_t primitiveIdx) const; /* Get the accessor id for the tcoords of a primitive. - * - * -1 is returned if none are present. */ - std::size_t getTCoordAccessorIdx(const std::size_t meshIdx, + std::optional getTCoordAccessorIdx(const std::size_t meshIdx, const std::size_t primitiveIdx) const; }; void loadPrimitives(const MeshExtractor& parser, SMesh* mesh); - static bool tryParseGLTF(io::IReadFile* file, - tinygltf::Model& model); + std::optional tryParseGLTF(io::IReadFile* file); }; } // namespace scene diff --git a/source/lib/tinygltf/CMakeLists.txt b/source/lib/tinygltf/CMakeLists.txt deleted file mode 100644 index 4647ecbce..000000000 --- a/source/lib/tinygltf/CMakeLists.txt +++ /dev/null @@ -1,65 +0,0 @@ -cmake_minimum_required(VERSION 3.5) - -project(tinygltf - VERSION 2.6.3 - LANGUAGES CXX -) - -add_library(tinygltf INTERFACE) -add_library(tinygltf::tinygltf ALIAS tinygltf) - -target_compile_definitions(tinygltf - INTERFACE - TINYGLTF_NO_EXTERNAL_IMAGE - TINYGLTF_NO_STB_IMAGE - TINYGLTF_NO_STB_IMAGE_WRITE -) - -target_include_directories(tinygltf - INTERFACE - "$" - "$" -) - -include(GNUInstallDirs) - -install(TARGETS tinygltf - EXPORT tinygltf-export - DESTINATION "${CMAKE_INSTALL_LIBDIR}" -) - -export(EXPORT tinygltf-export - FILE "${CMAKE_BINARY_DIR}/cmake/tinygltfTargets.cmake" - NAMESPACE tinygltf:: -) - -install( - DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" - DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/tinygltf" -) - -install(EXPORT tinygltf-export - FILE tinygltfTargets.cmake - NAMESPACE tinygltf:: - DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/tinygltf" -) - -include(CMakePackageConfigHelpers) -configure_package_config_file("${PROJECT_SOURCE_DIR}/tinygltfConfig.cmake.in" - "${CMAKE_BINARY_DIR}/cmake/tinygltfConfig.cmake" - INSTALL_DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/tinygltf" - NO_SET_AND_CHECK_MACRO - NO_CHECK_REQUIRED_COMPONENTS_MACRO -) - -write_basic_package_version_file( - "${CMAKE_BINARY_DIR}/cmake/tinygltfConfigVersion.cmake" - COMPATIBILITY AnyNewerVersion -) - -install( - FILES - "${CMAKE_BINARY_DIR}/cmake/tinygltfConfig.cmake" - "${CMAKE_BINARY_DIR}/cmake/tinygltfConfigVersion.cmake" - DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/tinygltf" -) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index b914aa61d..e59448965 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -24,14 +24,18 @@ if(CMAKE_BUILD_TYPE STREQUAL "Debug") add_definitions(-D_DEBUG) endif() set(CMAKE_POSITION_INDEPENDENT_CODE TRUE) -set(CMAKE_CXX_STANDARD 14) +set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) if(CMAKE_CXX_COMPILER_ID MATCHES "^(GNU|Clang|AppleClang)$") set(CMAKE_CXX_FLAGS_RELEASE "-O3") set(CMAKE_CXX_FLAGS_DEBUG "-g") +<<<<<<< HEAD:src/CMakeLists.txt add_compile_options(-Wall -pipe -fno-exceptions) +======= + add_compile_options(-Wall -pipe -fno-rtti) +>>>>>>> 1320fc97a (Move from tinygltf to tiniergltf (#13)):source/Irrlicht/CMakeLists.txt # Enable SSE for floating point math on 32-bit x86 by default # reasoning see minetest issue #11810 and https://gcc.gnu.org/wiki/FloatingPointMath @@ -327,22 +331,11 @@ endif() # Only for the glTF loader and dependents; we can't link targets # to object libraries on cmake<3.12 so usage requirements don't -# propogate to all dependents. -get_property(tinygltf_INCLUDE_DIRS - TARGET tinygltf::tinygltf +# propagate to all dependents. +get_property(tiniergltf_INCLUDE_DIRS + TARGET tiniergltf PROPERTY INTERFACE_INCLUDE_DIRECTORIES ) -get_property(tinygltf_COMPILE_DEFS - TARGET tinygltf::tinygltf - PROPERTY INTERFACE_COMPILE_DEFINITIONS -) -if(CMAKE_VERSION VERSION_LESS 3.12) - foreach(TINYGLTF_DEF ${tinygltf_COMPILE_DEFS}) - add_definitions("-D${TINYGLTF_DEF}") - endforeach(TINYGLTF_DEF) -else() - add_compile_definitions(${tinygltf_COMPILE_DEFS}) -endif() set(link_includes "${PROJECT_SOURCE_DIR}/include" @@ -351,7 +344,7 @@ set(link_includes "${ZLIB_INCLUDE_DIR}" "${JPEG_INCLUDE_DIR}" "${PNG_INCLUDE_DIR}" - "${tinygltf_INCLUDE_DIRS}" + "${tiniergltf_INCLUDE_DIRS}" "$<$:${SDL2_INCLUDE_DIRS}>" ${OPENGL_INCLUDE_DIR}