From 05d31222f713cd152b86b589ecb7dec1bc986ad7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20M=C3=BCller?= <34514239+appgurueu@users.noreply.github.com> Date: Fri, 6 Dec 2024 18:05:03 +0100 Subject: [PATCH] Allow non-normalized weights in glTF models (#15310) We are being lax here, but the glTF specification just requires that "when the weights are stored using float component type, their linear sum SHOULD be as close as reasonably possible to 1.0 for a given vertex" In particular weights > 1 and weight sums well below or above 1 can be observed in models exported by Blender if they aren't manually normalized. These fail the glTF validator but Irrlicht normalizes weights itself so we can support them just fine. The docs have been updated to recommend normalizing weights (as well as documenting the status of interpolation support). Weights < 0, most of them close to 0, also occur. Consistent with Irrlicht, we ignore them, but we also raise a warning. --- doc/lua_api.md | 11 +++++++++-- irr/src/CGLTFMeshFileLoader.cpp | 31 ++++++++++++++++++++----------- irr/src/CGLTFMeshFileLoader.h | 8 ++++---- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/doc/lua_api.md b/doc/lua_api.md index 2d5405090..449309493 100644 --- a/doc/lua_api.md +++ b/doc/lua_api.md @@ -309,17 +309,24 @@ it unlocks no special rendering features. Binary glTF (`.glb`) files are supported and recommended over `.gltf` files due to their space savings. -This means that many glTF features are not supported *yet*, including: +Bone weights should be normalized, e.g. using ["normalize all" in Blender](https://docs.blender.org/manual/en/4.2/grease_pencil/modes/weight_paint/weights_menu.html#normalize-all). + +You can use the [Khronos glTF validator](https://github.com/KhronosGroup/glTF-Validator) +to check whether a model is a valid glTF file. + +Many glTF features are not supported *yet*, including: * Animations * Only a single animation is supported, use frame ranges within this animation. + * Only linear interpolation is supported. * Cameras * Materials * Only base color textures are supported * Backface culling is overridden * Double-sided materials don't work * Alternative means of supplying data - * Embedded images + * Embedded images. You can use `gltfutil.py` from the + [modding tools](https://github.com/minetest/modtools) to strip or extract embedded images. * References to files via URIs Textures are supplied solely via the same means as for the other model file formats: diff --git a/irr/src/CGLTFMeshFileLoader.cpp b/irr/src/CGLTFMeshFileLoader.cpp index 9045569a6..139b4f670 100644 --- a/irr/src/CGLTFMeshFileLoader.cpp +++ b/irr/src/CGLTFMeshFileLoader.cpp @@ -305,7 +305,7 @@ SelfType::createNormalizedValuesAccessor( } } -template +template std::array SelfType::getNormalizedValues( const NormalizedValuesAccessor &accessor, const std::size_t i) @@ -313,17 +313,19 @@ std::array SelfType::getNormalizedValues( std::array values; if (std::holds_alternative>>(accessor)) { const auto u8s = std::get>>(accessor).get(i); - for (u8 i = 0; i < N; ++i) - values[i] = static_cast(u8s[i]) / std::numeric_limits::max(); + for (std::size_t j = 0; j < N; ++j) + values[j] = static_cast(u8s[j]) / std::numeric_limits::max(); } else if (std::holds_alternative>>(accessor)) { const auto u16s = std::get>>(accessor).get(i); - for (u8 i = 0; i < N; ++i) - values[i] = static_cast(u16s[i]) / std::numeric_limits::max(); + for (std::size_t j = 0; j < N; ++j) + values[j] = static_cast(u16s[j]) / std::numeric_limits::max(); } else { values = std::get>>(accessor).get(i); - for (u8 i = 0; i < N; ++i) { - if (values[i] < 0 || values[i] > 1) - throw std::runtime_error("invalid normalized value"); + if constexpr (validate) { + for (std::size_t j = 0; j < N; ++j) { + if (values[j] < 0 || values[j] > 1) + throw std::runtime_error("invalid normalized value"); + } } } return values; @@ -493,6 +495,7 @@ void SelfType::MeshExtractor::addPrimitive( const auto weightAccessor = createNormalizedValuesAccessor<4>(m_gltf_model, weights->at(set)); + bool negative_weights = false; for (std::size_t v = 0; v < n_vertices; ++v) { std::array jointIdxs; if (std::holds_alternative>>(jointAccessor)) { @@ -501,14 +504,18 @@ void SelfType::MeshExtractor::addPrimitive( } else if (std::holds_alternative>>(jointAccessor)) { jointIdxs = std::get>>(jointAccessor).get(v); } - std::array strengths = getNormalizedValues(weightAccessor, v); + + // Be lax: We can allow weights that aren't normalized. Irrlicht already normalizes them. + // The glTF spec only requires that these be "as close to 1 as reasonably possible". + auto strengths = getNormalizedValues<4, false>(weightAccessor, v); // 4 joints per set for (std::size_t in_set = 0; in_set < 4; ++in_set) { u16 jointIdx = jointIdxs[in_set]; f32 strength = strengths[in_set]; - if (strength == 0) - continue; + negative_weights = negative_weights || (strength < 0); + if (strength <= 0) + continue; // note: also ignores negative weights SkinnedMesh::SWeight *weight = m_irr_model->addWeight(m_loaded_nodes.at(skin.joints.at(jointIdx))); weight->buffer_id = meshbufNr; @@ -516,6 +523,8 @@ void SelfType::MeshExtractor::addPrimitive( weight->strength = strength; } } + if (negative_weights) + warn("negative weights"); } } diff --git a/irr/src/CGLTFMeshFileLoader.h b/irr/src/CGLTFMeshFileLoader.h index 35ee85726..1671d2903 100644 --- a/irr/src/CGLTFMeshFileLoader.h +++ b/irr/src/CGLTFMeshFileLoader.h @@ -91,7 +91,7 @@ private: const tiniergltf::GlTF &model, const std::size_t accessorIdx); - template + template static std::array getNormalizedValues( const NormalizedValuesAccessor &accessor, const std::size_t i); @@ -118,7 +118,7 @@ private: std::size_t getPrimitiveCount(const std::size_t meshIdx) const; void load(); - const std::vector &getWarnings() { + const std::unordered_set &getWarnings() { return warnings; } @@ -129,9 +129,9 @@ private: std::vector> m_mesh_loaders; std::vector m_loaded_nodes; - std::vector warnings; + std::unordered_set warnings; void warn(const std::string &warning) { - warnings.push_back(warning); + warnings.insert(warning); } void copyPositions(const std::size_t accessorIdx,