From 4553cb4412d80b64cb78488f229e9313538b51f7 Mon Sep 17 00:00:00 2001 From: DJLegends Date: Thu, 14 May 2026 04:13:37 -0500 Subject: [PATCH 1/5] Add BSGeometry bone weight loading from mesh data GetShapeBoneWeights had no path for BSGeometry shapes. Starfield meshes store per-vertex bone weights in BSGeometryMeshData::skinWeights as packed uint16 values (65535 = 1.0). Add a BSGeometry case that reads these weights, bridging the gap that caused all SF mesh vertices to appear unweighted in Outfit Studio. Co-Authored-By: Claude Opus 4.6 --- src/NifFile.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/NifFile.cpp b/src/NifFile.cpp index 5959190..8f16a64 100644 --- a/src/NifFile.cpp +++ b/src/NifFile.cpp @@ -2561,6 +2561,22 @@ uint32_t NifFile::GetShapeBoneWeights(NiShape* shape, return static_cast(outWeights.size()); } + auto bsGeom = dynamic_cast(shape); + if (bsGeom) { + auto* geomData = dynamic_cast(bsGeom->GetGeomData()); + if (geomData && !geomData->skinWeights.empty()) { + outWeights.reserve(geomData->skinWeights.size()); + for (uint16_t vid = 0; vid < geomData->skinWeights.size(); vid++) { + for (auto& bw : geomData->skinWeights[vid]) { + if (bw.boneIndex == boneIndex && bw.weight != 0) { + outWeights.emplace(vid, bw.weight / 65535.0f); + } + } + } + return static_cast(outWeights.size()); + } + } + auto skinInst = hdr.GetBlock(shape->SkinInstanceRef()); if (!skinInst) return 0; From d6098ba5fe040b452ecc8e7ad54039cf0de0babc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 17 May 2026 06:44:28 +0000 Subject: [PATCH 2/5] Fix BSGeometry bone weight loop overflow and add regression test Address Copilot review notes on the BSGeometry GetShapeBoneWeights path: - The vertex loop counter was uint16_t while skinWeights.size() is 32-bit, causing an infinite loop on meshes with more than 65535 vertices. Iterate with a size_t index capped at the uint16_t-keyed output map's range. - Add a Starfield regression test that loads the external mesh data and verifies GetShapeBoneWeights returns normalized weights in [0, 1]. https://claude.ai/code/session_01M73z1PscSn8M97dpyPYUW5 --- src/NifFile.cpp | 10 +++++++-- tests/TestNifFile.cpp | 51 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/NifFile.cpp b/src/NifFile.cpp index 8f16a64..5723ea1 100644 --- a/src/NifFile.cpp +++ b/src/NifFile.cpp @@ -2566,10 +2566,16 @@ uint32_t NifFile::GetShapeBoneWeights(NiShape* shape, auto* geomData = dynamic_cast(bsGeom->GetGeomData()); if (geomData && !geomData->skinWeights.empty()) { outWeights.reserve(geomData->skinWeights.size()); - for (uint16_t vid = 0; vid < geomData->skinWeights.size(); vid++) { + // outWeights is keyed by uint16_t, so vertex indices are capped at + // 0xFFFF; iterate with a wide index to avoid overflowing the loop + // counter on meshes with more than 65535 vertices. + size_t vertCount = geomData->skinWeights.size(); + if (vertCount > 0x10000) + vertCount = 0x10000; + for (size_t vid = 0; vid < vertCount; vid++) { for (auto& bw : geomData->skinWeights[vid]) { if (bw.boneIndex == boneIndex && bw.weight != 0) { - outWeights.emplace(vid, bw.weight / 65535.0f); + outWeights.emplace(static_cast(vid), bw.weight / 65535.0f); } } } diff --git a/tests/TestNifFile.cpp b/tests/TestNifFile.cpp index c5b5783..43c7af2 100644 --- a/tests/TestNifFile.cpp +++ b/tests/TestNifFile.cpp @@ -359,6 +359,57 @@ TEST_CASE("Load and save file (SF)", "[NifFile]") { REQUIRE(CompareBinaryFiles(fileOutput, fileExpected)); } +TEST_CASE("BSGeometry bone weights are normalized (SF)", "[NifFile]") { + constexpr auto fileName = "TestNifFile_SF"; + const auto [fileInput, fileOutput, fileExpected] = GetFileTuple(fileName, nifSuffix); + + NifFile nif; + REQUIRE(nif.Load(fileInput) == 0); + + auto shapes = nif.GetShapes(); + REQUIRE(!shapes.empty()); + + size_t weightedVertices = 0; + + for (auto& s : shapes) { + auto* bsGeom = dynamic_cast(s); + REQUIRE(bsGeom != nullptr); + + auto meshPaths = nif.GetExternalGeometryPathRefs(s); + uint8_t meshIndex = 0; + for (auto meshPath : meshPaths) { + std::string meshPathStr = meshPath.get(); + const auto [meshFileInput, meshFileOutput, meshFileExpected] + = GetFileTuple(meshPathStr.c_str(), meshSuffix); + const std::filesystem::path meshInputPath = std::filesystem::u8path(meshFileInput); + auto meshStream = GetBinaryInputFileStream(meshInputPath); + REQUIRE(meshStream); + REQUIRE(nif.LoadExternalShapeData(s, *meshStream, meshIndex)); + meshStream.reset(); + meshIndex++; + } + + std::vector bones; + nif.GetShapeBoneList(s, bones); + + for (uint32_t boneIndex = 0; boneIndex < bones.size(); boneIndex++) { + std::unordered_map weights; + nif.GetShapeBoneWeights(s, boneIndex, weights); + + for (const auto& [vid, weight] : weights) { + REQUIRE(weight >= 0.0f); + REQUIRE(weight <= 1.0f); + } + + weightedVertices += weights.size(); + } + } + + // The Starfield mesh fixture is skinned, so the BSGeometry path must + // actually return per-vertex weights (regression for the new code path). + REQUIRE(weightedVertices > 0); +} + TEST_CASE("Load external and save as internal mesh data (SF)", "[NifFile]") { constexpr auto fileName = "TestNifFile_ToInternalMesh_SF"; const auto [fileInput, fileOutput, fileExpected] = GetFileTuple(fileName, nifSuffix); From 860d4ec25107ddc5abff7b9d1243e462762b0141 Mon Sep 17 00:00:00 2001 From: ousnius Date: Sun, 17 May 2026 09:48:49 +0200 Subject: [PATCH 3/5] Cap outWeights to vertCount Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/NifFile.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NifFile.cpp b/src/NifFile.cpp index 5723ea1..44052a9 100644 --- a/src/NifFile.cpp +++ b/src/NifFile.cpp @@ -2565,13 +2565,13 @@ uint32_t NifFile::GetShapeBoneWeights(NiShape* shape, if (bsGeom) { auto* geomData = dynamic_cast(bsGeom->GetGeomData()); if (geomData && !geomData->skinWeights.empty()) { - outWeights.reserve(geomData->skinWeights.size()); // outWeights is keyed by uint16_t, so vertex indices are capped at // 0xFFFF; iterate with a wide index to avoid overflowing the loop // counter on meshes with more than 65535 vertices. size_t vertCount = geomData->skinWeights.size(); if (vertCount > 0x10000) vertCount = 0x10000; + outWeights.reserve(vertCount); for (size_t vid = 0; vid < vertCount; vid++) { for (auto& bw : geomData->skinWeights[vid]) { if (bw.boneIndex == boneIndex && bw.weight != 0) { From bf9d952c13f9fb12b733e9f5603a35cd40ae8136 Mon Sep 17 00:00:00 2001 From: ousnius Date: Sun, 17 May 2026 09:54:43 +0200 Subject: [PATCH 4/5] Simplify tuple usage and add sanity checks in tests In tests/TestNifFile.cpp (BSGeometry bone weights test) replace structured bindings that returned full file tuples with std::get<0>() to only retrieve the input path (fileInput / meshFileInput). Add REQUIRE checks to ensure external mesh path lists and mesh path strings are not empty before attempting to open streams. These changes remove unused tuple elements and add safer assertions to avoid opening invalid paths during the test. --- tests/TestNifFile.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/TestNifFile.cpp b/tests/TestNifFile.cpp index 43c7af2..29bf55f 100644 --- a/tests/TestNifFile.cpp +++ b/tests/TestNifFile.cpp @@ -361,7 +361,7 @@ TEST_CASE("Load and save file (SF)", "[NifFile]") { TEST_CASE("BSGeometry bone weights are normalized (SF)", "[NifFile]") { constexpr auto fileName = "TestNifFile_SF"; - const auto [fileInput, fileOutput, fileExpected] = GetFileTuple(fileName, nifSuffix); + const auto fileInput = std::get<0>(GetFileTuple(fileName, nifSuffix)); NifFile nif; REQUIRE(nif.Load(fileInput) == 0); @@ -376,11 +376,14 @@ TEST_CASE("BSGeometry bone weights are normalized (SF)", "[NifFile]") { REQUIRE(bsGeom != nullptr); auto meshPaths = nif.GetExternalGeometryPathRefs(s); + REQUIRE(!meshPaths.empty()); + uint8_t meshIndex = 0; for (auto meshPath : meshPaths) { std::string meshPathStr = meshPath.get(); - const auto [meshFileInput, meshFileOutput, meshFileExpected] - = GetFileTuple(meshPathStr.c_str(), meshSuffix); + REQUIRE(!meshPathStr.empty()); + + const auto meshFileInput = std::get<0>(GetFileTuple(meshPathStr.c_str(), meshSuffix)); const std::filesystem::path meshInputPath = std::filesystem::u8path(meshFileInput); auto meshStream = GetBinaryInputFileStream(meshInputPath); REQUIRE(meshStream); From d135c26eaf8fe770760e3f3ab4ca3b127290451d Mon Sep 17 00:00:00 2001 From: ousnius Date: Sun, 17 May 2026 10:10:21 +0200 Subject: [PATCH 5/5] Use numeric_limits for vertex/weight limits Replace hardcoded magic values (0x10000 and 65535) with constexpr derived from std::numeric_limits::max(). Cap vertCount via std::min to avoid overflow on meshes with >65535 vertices and normalize bone weights by a named maxWeightValue. Improves clarity and avoids magic numbers. --- src/NifFile.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/NifFile.cpp b/src/NifFile.cpp index 44052a9..2408c07 100644 --- a/src/NifFile.cpp +++ b/src/NifFile.cpp @@ -2568,14 +2568,15 @@ uint32_t NifFile::GetShapeBoneWeights(NiShape* shape, // outWeights is keyed by uint16_t, so vertex indices are capped at // 0xFFFF; iterate with a wide index to avoid overflowing the loop // counter on meshes with more than 65535 vertices. - size_t vertCount = geomData->skinWeights.size(); - if (vertCount > 0x10000) - vertCount = 0x10000; + constexpr size_t maxVerts = static_cast(std::numeric_limits::max()) + 1; + constexpr float maxWeightValue = static_cast(std::numeric_limits::max()); + + size_t vertCount = std::min(geomData->skinWeights.size(), maxVerts); outWeights.reserve(vertCount); for (size_t vid = 0; vid < vertCount; vid++) { for (auto& bw : geomData->skinWeights[vid]) { if (bw.boneIndex == boneIndex && bw.weight != 0) { - outWeights.emplace(static_cast(vid), bw.weight / 65535.0f); + outWeights.emplace(static_cast(vid), bw.weight / maxWeightValue); } } }