From 17513a83908da47980a24977afdb2f1fb047ad77 Mon Sep 17 00:00:00 2001 From: Aaron Kimbrell Date: Mon, 28 Sep 2026 10:44:30 -0500 Subject: [PATCH] fix: splitting and normalizing models moves every bone and rigid, in file order Lxfml::NormalizePosition moved only the first bone of each part, and never the rigid systems, so after a save a flexible part's other bones and every Rigid kept their world positions while the rest of the model was moved to its own origin. Now every Bone and every Rigid moves by the same amount, and the box that places the model holds every bone. Split maps every bone of a part too, so a rigid system holding only a later bone keeps its brick. Also: - a model with no readable bone is kept as it is at the origin instead of getting a center 10000 up - the math is done in doubles and numbers are written as the shortest text that reads back as the same float (-0.4, not -0.400002; 12.1678, not 12.1677) - bricks and rigid systems come out in the file's order, so the same model always splits into the same bytes - parts over 5 MB are normalized like any other (the input is capped at 10 MB), and the loop's safety limit no longer drops every later model The pivot is still snapped to the 0.8 grid (the bricks don't move in the world; the model's position stays on the grid), which can put it half a stud from the middle. Co-Authored-By: Claude Opus 5.5 --- dCommon/Lxfml.cpp | 262 +++++++++++++----------------- tests/dCommonTests/LxfmlTests.cpp | 108 +++++++++++- 2 files changed, 221 insertions(+), 149 deletions(-) diff --git a/dCommon/Lxfml.cpp b/dCommon/Lxfml.cpp index f93d6d657..5899169fb 100644 --- a/dCommon/Lxfml.cpp +++ b/dCommon/Lxfml.cpp @@ -10,6 +10,8 @@ #include #include #include +#include +#include namespace { // The base LXFML xml file to use when creating new models. @@ -73,132 +75,107 @@ Lxfml::Contents Lxfml::ReadContents(const std::string_view data) { return contents; } +namespace { + // A transformation attribute: 9 rotation values kept as written, then the position + struct Transformation { + std::vector rotation; + double x{}, y{}, z{}; + }; + + std::optional ParseTransformation(const char* text) { + if (!text) return std::nullopt; + auto split = GeneralUtils::SplitString(text, ','); + if (split.size() < 12) return std::nullopt; + const auto x = GeneralUtils::TryParse(split[9]); + const auto y = GeneralUtils::TryParse(split[10]); + const auto z = GeneralUtils::TryParse(split[11]); + if (!x || !y || !z) return std::nullopt; + split.resize(9); + return Transformation{ std::move(split), *x, *y, *z }; + } + + // The shortest text that reads back as the same float + std::string FormatNumber(const double value) { + char buffer[32]; + const auto [end, error] = std::to_chars(buffer, buffer + sizeof(buffer), static_cast(value)); + return error == std::errc() ? std::string(buffer, end) : std::to_string(value); + } + + std::string FormatTransformation(const Transformation& transformation) { + std::string text; + for (const auto& value : transformation.rotation) text += value + ','; + return text + FormatNumber(transformation.x) + ',' + FormatNumber(transformation.y) + ',' + FormatNumber(transformation.z); + } + + // Every element named `name` under `parent`'s children named `childName` (e.g. every Bone of every Part) + template + void ForEachGrandchild(tinyxml2::XMLElement* parent, const char* childName, const char* name, Visit&& visit) { + if (!parent) return; + for (auto* child = parent->FirstChildElement(childName); child; child = child->NextSiblingElement(childName)) { + for (auto* element = child->FirstChildElement(name); element; element = element->NextSiblingElement(name)) visit(element); + } + } +} + Lxfml::Result Lxfml::NormalizePosition(const std::string_view data, const NiPoint3& curPosition) { Result toReturn; - - // Handle empty or invalid input - if (data.empty()) { - return toReturn; - } - + if (data.empty()) return toReturn; + tinyxml2::XMLDocument doc; // Use length-based parsing to avoid expensive string copy - const auto err = doc.Parse(data.data(), data.size()); - if (err != tinyxml2::XML_SUCCESS) { - return toReturn; - } + if (doc.Parse(data.data(), data.size()) != tinyxml2::XML_SUCCESS) return toReturn; + auto* lxfml = doc.FirstChildElement("LXFML"); + if (!lxfml) return toReturn; - TinyXmlUtils::DocumentReader reader(doc); - std::map transformations; - - auto lxfml = reader["LXFML"]; - if (!lxfml) { - return toReturn; - } - - // First get all the positions of bricks - for (const auto& brick : lxfml["Bricks"]) { - const auto* part = brick.FirstChildElement("Part"); - while (part) { - const auto* bone = part->FirstChildElement("Bone"); - if (bone) { - auto* transformation = bone->Attribute("transformation"); - if (transformation) { - auto* refID = bone->Attribute("refID"); - if (refID) transformations[refID] = transformation; - } - } - part = part->NextSiblingElement("Part"); + // Every bone of every part (flexible parts have several), and every rigid of every rigid system: both are + // positioned in the same space, so both move + std::vector bones; + if (auto* bricks = lxfml->FirstChildElement("Bricks")) { + for (auto* brick = bricks->FirstChildElement("Brick"); brick; brick = brick->NextSiblingElement("Brick")) { + ForEachGrandchild(brick, "Part", "Bone", [&bones](tinyxml2::XMLElement* bone) { bones.push_back(bone); }); } } + std::vector rigids; + ForEachGrandchild(lxfml->FirstChildElement("RigidSystems"), "RigidSystem", "Rigid", [&rigids](tinyxml2::XMLElement* rigid) { rigids.push_back(rigid); }); - // These points are well out of bounds for an actual player - NiPoint3 lowest{ 10'000.0f, 10'000.0f, 10'000.0f }; - NiPoint3 highest{ -10'000.0f, -10'000.0f, -10'000.0f }; - - NiPoint3 delta = NiPoint3Constant::ZERO; + // The new origin: the middle of the bricks' origins, on the floor of the lowest one. x and z are snapped to the + // LEGO grid (0.8) so the model's position stays on it; the bricks don't move in the world either way, only the + // model's pivot does. + double rootX = curPosition.x, rootY = curPosition.y, rootZ = curPosition.z; if (curPosition == NiPoint3Constant::ZERO) { - // Calculate the lowest and highest points on the entire model - for (const auto& transformation : transformations | std::views::values) { - auto split = GeneralUtils::SplitString(transformation, ','); - if (split.size() < 12) continue; - - auto xOpt = GeneralUtils::TryParse(split[9]); - auto yOpt = GeneralUtils::TryParse(split[10]); - auto zOpt = GeneralUtils::TryParse(split[11]); - - if (!xOpt.has_value() || !yOpt.has_value() || !zOpt.has_value()) continue; - - auto x = xOpt.value(); - auto y = yOpt.value(); - auto z = zOpt.value(); - if (x < lowest.x) lowest.x = x; - if (y < lowest.y) lowest.y = y; - if (z < lowest.z) lowest.z = z; - - if (highest.x < x) highest.x = x; - if (highest.y < y) highest.y = y; - if (highest.z < z) highest.z = z; + bool any = false; + double minX{}, minY{}, minZ{}, maxX{}, maxY{}, maxZ{}; + for (const auto* bone : bones) { + const auto transformation = ParseTransformation(bone->Attribute("transformation")); + if (!transformation) continue; + const auto& [rotation, x, y, z] = *transformation; + minX = any ? std::min(minX, x) : x; maxX = any ? std::max(maxX, x) : x; + minY = any ? std::min(minY, y) : y; maxY = any ? std::max(maxY, y) : y; + minZ = any ? std::min(minZ, z) : z; maxZ = any ? std::max(maxZ, z) : z; + any = true; } - - delta = (highest - lowest) / 2.0f; - } else { - lowest = curPosition; - highest = curPosition; - delta = NiPoint3Constant::ZERO; + // Nothing to place it by: keep the model as it is, at the origin + if (!any) { + toReturn.lxfml = std::string(data); + return toReturn; + } + rootX = (minX + maxX) / 2.0; + rootY = minY; + rootZ = (minZ + maxZ) / 2.0; } + rootX = GeneralUtils::RountToNearestEven(rootX, 0.8); + rootZ = GeneralUtils::RountToNearestEven(rootZ, 0.8); - auto newRootPos = lowest + delta; - - // Need to snap this chosen position to the nearest valid spot - // on the LEGO grid - newRootPos.x = GeneralUtils::RountToNearestEven(newRootPos.x, 0.8f); - newRootPos.z = GeneralUtils::RountToNearestEven(newRootPos.z, 0.8f); - - // Clamp the Y to the lowest point on the model - newRootPos.y = lowest.y; - - // Adjust all positions to account for the new origin - for (auto& transformation : transformations | std::views::values) { - auto split = GeneralUtils::SplitString(transformation, ','); - if (split.size() < 12) { - continue; - } - - auto xOpt = GeneralUtils::TryParse(split[9]); - auto yOpt = GeneralUtils::TryParse(split[10]); - auto zOpt = GeneralUtils::TryParse(split[11]); - - if (!xOpt.has_value() || !yOpt.has_value() || !zOpt.has_value()) { - continue; - } - auto x = xOpt.value() - newRootPos.x + curPosition.x; - auto y = yOpt.value() - newRootPos.y + curPosition.y; - auto z = zOpt.value() - newRootPos.z + curPosition.z; - std::stringstream stream; - for (int i = 0; i < 9; i++) { - stream << split[i]; - stream << ','; - } - stream << x << ',' << y << ',' << z; - transformation = stream.str(); - } - - // Finally write the new transformation back into the lxfml - for (auto& brick : lxfml["Bricks"]) { - auto* part = brick.FirstChildElement("Part"); - while (part) { - auto* bone = part->FirstChildElement("Bone"); - if (bone) { - auto* transformation = bone->Attribute("transformation"); - if (transformation) { - auto* refID = bone->Attribute("refID"); - if (refID) { - bone->SetAttribute("transformation", transformations[refID].c_str()); - } - } - } - part = part->NextSiblingElement("Part"); + // Everything moves by the same amount: onto the new origin, then by the given position + const double offsetX = curPosition.x - rootX, offsetY = curPosition.y - rootY, offsetZ = curPosition.z - rootZ; + for (auto* elements : { &bones, &rigids }) { + for (auto* element : *elements) { + auto transformation = ParseTransformation(element->Attribute("transformation")); + if (!transformation) continue; + transformation->x += offsetX; + transformation->y += offsetY; + transformation->z += offsetZ; + element->SetAttribute("transformation", FormatTransformation(*transformation).c_str()); } } @@ -206,12 +183,10 @@ Lxfml::Result Lxfml::NormalizePosition(const std::string_view data, const NiPoin doc.Print(&printer); toReturn.lxfml = printer.CStr(); - toReturn.center = newRootPos; + toReturn.center = NiPoint3(static_cast(rootX), static_cast(rootY), static_cast(rootZ)); return toReturn; } -// Deep-clone an XMLElement (attributes, text, and child elements) into a target document -// with maximum depth protection to prevent infinite loops static tinyxml2::XMLElement* CloneElementDeep(const tinyxml2::XMLElement* src, tinyxml2::XMLDocument& dstDoc, int maxDepth = 100) { if (!src || maxDepth <= 0) return nullptr; auto* dst = dstDoc.NewElement(src->Name()); @@ -269,20 +244,22 @@ std::vector Lxfml::Split(const std::string_view data, const NiPoi std::unordered_map partRefToBrick; std::unordered_map boneRefToPartRef; std::unordered_map brickByRef; + std::vector brickOrder; auto* bricksParent = lxfml->FirstChildElement("Bricks"); if (bricksParent) { for (auto* brick = bricksParent->FirstChildElement("Brick"); brick; brick = brick->NextSiblingElement("Brick")) { const char* brickRef = brick->Attribute("refID"); if (brickRef) brickByRef.emplace(std::string(brickRef), brick); + brickOrder.push_back(brick); for (auto* part = brick->FirstChildElement("Part"); part; part = part->NextSiblingElement("Part")) { const char* partRef = part->Attribute("refID"); if (partRef) { partRefToPart.emplace(std::string(partRef), part); partRefToBrick.emplace(std::string(partRef), brick); } - auto* bone = part->FirstChildElement("Bone"); - if (bone) { + // Flexible parts have a bone per section + for (auto* bone = part->FirstChildElement("Bone"); bone; bone = bone->NextSiblingElement("Bone")) { const char* boneRef = bone->Attribute("refID"); if (boneRef) boneRefToPartRef.emplace(std::string(boneRef), partRef ? std::string(partRef) : std::string()); } @@ -326,16 +303,17 @@ std::vector Lxfml::Split(const std::string_view data, const NiPoi auto* outRigidSystems = outRoot->FirstChildElement("RigidSystems"); auto* outGroupSystems = outRoot->FirstChildElement("GroupSystems"); - // clone and insert bricks - for (const auto& bref : bricksToInclude) { - auto it = brickByRef.find(bref); - if (it == brickByRef.end()) continue; - tinyxml2::XMLElement* cloned = CloneElementDeep(it->second, outDoc); + // clone and insert bricks and rigid systems in the order the file has them, so the same model always + // comes out the same + for (auto* brick : brickOrder) { + const char* bref = brick->Attribute("refID"); + // (a refID used twice: the first brick with it, as the maps have it) + if (!bref || !bricksToInclude.contains(bref) || brickByRef.at(bref) != brick) continue; + tinyxml2::XMLElement* cloned = CloneElementDeep(brick, outDoc); if (cloned) outBricks->InsertEndChild(cloned); } - - // clone and insert rigidsystems - for (auto* rsPtr : rigidSystemsToInclude) { + for (auto* rsPtr : rigidSystems) { + if (std::find(rigidSystemsToInclude.begin(), rigidSystemsToInclude.end(), rsPtr) == rigidSystemsToInclude.end()) continue; tinyxml2::XMLElement* cloned = CloneElementDeep(rsPtr, outDoc); if (cloned) outRigidSystems->InsertEndChild(cloned); } @@ -353,18 +331,10 @@ std::vector Lxfml::Split(const std::string_view data, const NiPoi outGroupSystems->InsertEndChild(newGS); } - // Print to string + // Print to string, then normalize position and compute center (the input is at most 10 MB, so each part is too) tinyxml2::XMLPrinter printer; outDoc.Print(&printer); - // Normalize position and compute center using existing helper - std::string xmlString = printer.CStr(); - if (xmlString.size() > 5000000) { // 5MB limit for normalization - Result emptyResult; - emptyResult.lxfml = xmlString; - return emptyResult; - } - auto normalized = NormalizePosition(xmlString, curPosition); - return normalized; + return NormalizePosition(printer.CStr(), curPosition); }; // 1) Process groups (each top-level Group becomes one output; nested groups are included) @@ -400,8 +370,7 @@ std::vector Lxfml::Split(const std::string_view data, const NiPoi } auto partIt = partRefToPart.find(pref); if (partIt != partRefToPart.end()) { - auto* bone = partIt->second->FirstChildElement("Bone"); - if (bone) { + for (auto* bone = partIt->second->FirstChildElement("Bone"); bone; bone = bone->NextSiblingElement("Bone")) { const char* bref = bone->Attribute("refID"); if (bref) boneRefsIncluded.insert(std::string(bref)); } @@ -487,8 +456,7 @@ std::vector Lxfml::Split(const std::string_view data, const NiPoi } auto partIt = partRefToPart.find(pref); if (partIt != partRefToPart.end()) { - auto* bone = partIt->second->FirstChildElement("Bone"); - if (bone) { + for (auto* bone = partIt->second->FirstChildElement("Bone"); bone; bone = bone->NextSiblingElement("Bone")) { const char* bref = bone->Attribute("refID"); if (bref) boneRefsIncluded.insert(std::string(bref)); } @@ -498,11 +466,8 @@ std::vector Lxfml::Split(const std::string_view data, const NiPoi } } - if (iteration >= maxIterations) { - // Iteration limit reached, stop processing to prevent infinite loops - // The file is likely malformed, so just skip further processing - return results; - } + // (Every pass that goes on adds a rigid system or group, so the limit is never reached; hitting it anyway still + // outputs what was collected, and the rest of the file comes out as further models below.) // include bricks from bricksIncluded into used set for (const auto& b : bricksIncluded) usedBrickRefs.insert(b); @@ -541,7 +506,10 @@ std::vector Lxfml::Split(const std::string_view data, const NiPoi } // 3) Any remaining bricks not included become their own files - for (const auto& [bref, brickPtr] : brickByRef) { + for (auto* brick : brickOrder) { + const char* brefAttr = brick->Attribute("refID"); + if (!brefAttr) continue; + const std::string bref(brefAttr); if (usedBrickRefs.find(bref) != usedBrickRefs.end()) continue; std::unordered_set bricksIncluded{ bref }; auto normalized = makeOutput(bricksIncluded, {}); diff --git a/tests/dCommonTests/LxfmlTests.cpp b/tests/dCommonTests/LxfmlTests.cpp index 0fefaebbf..a4eabcf95 100644 --- a/tests/dCommonTests/LxfmlTests.cpp +++ b/tests/dCommonTests/LxfmlTests.cpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include using namespace TinyXmlUtils; @@ -27,6 +29,16 @@ std::string SerializeElement(tinyxml2::XMLElement* elem) { return std::string(p.CStr()); }; +// A rigid system by what it holds (its rigids' refIDs and boneRefs): splitting moves its transformations +std::string RigidSystemKey(tinyxml2::XMLElement* rigidSystem) { + std::string key; + for (auto* rigid = rigidSystem->FirstChildElement("Rigid"); rigid; rigid = rigid->NextSiblingElement("Rigid")) { + key += std::string(rigid->Attribute("refID") ? rigid->Attribute("refID") : "") + ":" + (rigid->Attribute("boneRefs") ? rigid->Attribute("boneRefs") : "") + ";"; + } + for (auto* joint = rigidSystem->FirstChildElement("Joint"); joint; joint = joint->NextSiblingElement("Joint")) key += SerializeElement(joint); + return key; +} + // Helper function to test splitting functionality static void TestSplitUsesAllBricksAndNoDuplicatesHelper(const std::string& filename) { // Read the LXFML file @@ -50,7 +62,7 @@ static void TestSplitUsesAllBricksAndNoDuplicatesHelper(const std::string& filen std::unordered_set originalRigidSet; if (auto* rsParent = doc.FirstChildElement("LXFML")->FirstChildElement("RigidSystems")) { for (auto* rs = rsParent->FirstChildElement("RigidSystem"); rs; rs = rs->NextSiblingElement("RigidSystem")) { - originalRigidSet.insert(SerializeElement(rs)); + originalRigidSet.insert(RigidSystemKey(rs)); } } @@ -102,7 +114,7 @@ static void TestSplitUsesAllBricksAndNoDuplicatesHelper(const std::string& filen // collect rigid systems in this output if (auto* rsParent = outDoc.FirstChildElement("LXFML")->FirstChildElement("RigidSystems")) { for (auto* rs = rsParent->FirstChildElement("RigidSystem"); rs; rs = rs->NextSiblingElement("RigidSystem")) { - auto s = SerializeElement(rs); + auto s = RigidSystemKey(rs); // no duplicate allowed across outputs ASSERT_EQ(usedRigidSet.find(s), usedRigidSet.end()) << "Duplicate RigidSystem across splits"; usedRigidSet.insert(s); @@ -430,3 +442,95 @@ TEST(LxfmlTests, ReadContentsListsBricksAndTheirBox) { EXPECT_EQ(nothing.boxMin, NiPoint3Constant::ZERO); EXPECT_TRUE(Lxfml::ReadContents("").designIds.empty()); } + +namespace { + const std::string PROBE_HEAD = R"()"; + + // Two models: bricks 0 and 1 in one rigid system at x 100.4; a flexible part (two bones) and an upside down brick, + // joined in another rigid system, around x 1222 + const std::string TWO_MODELS = PROBE_HEAD + + R"()" + R"()" + R"()" + R"()" + R"()" + R"()" + R"()" + R"()"; + + // refID -> transformation of every element named `name` + std::map Transformations(const std::string& lxfml, const char* name) { + tinyxml2::XMLDocument doc; + doc.Parse(lxfml.c_str()); + std::map out; + std::function walk = [&](const tinyxml2::XMLElement* element) { + for (; element; element = element->NextSiblingElement()) { + if (std::string(element->Name()) == name) out[element->Attribute("refID")] = element->Attribute("transformation"); + walk(element->FirstChildElement()); + } + }; + walk(doc.FirstChildElement()); + return out; + } + + std::vector BrickOrder(const std::string& lxfml) { + tinyxml2::XMLDocument doc; + doc.Parse(lxfml.c_str()); + std::vector out; + for (auto* brick = doc.FirstChildElement("LXFML")->FirstChildElement("Bricks")->FirstChildElement("Brick"); brick; brick = brick->NextSiblingElement("Brick")) out.push_back(brick->Attribute("refID")); + return out; + } +} + +TEST(LxfmlTests, SplitMovesRigidsWithTheirBones) { + const auto results = Lxfml::Split(TWO_MODELS); + ASSERT_EQ(results.size(), 2); + EXPECT_EQ(results[0].center, NiPoint3(100.8f, 433.92f, -62.4f)); + const auto bones = Transformations(results[0].lxfml, "Bone"); + EXPECT_EQ(bones.at("0"), "1,0,0,0,1,0,0,0,1,-0.4,0,-0.4"); + EXPECT_EQ(bones.at("1"), "1,0,0,0,1,0,0,0,1,-0.4,0.96,-0.4"); + // The rigid system sits where its first bone does, as the game writes it + EXPECT_EQ(Transformations(results[0].lxfml, "Rigid").at("0"), bones.at("0")); +} + +TEST(LxfmlTests, SplitMovesEveryBoneOfAFlexiblePart) { + const auto results = Lxfml::Split(TWO_MODELS); + ASSERT_EQ(results.size(), 2); + // The box of every bone (1210 to 1236.8): centred at 1223.4, snapped to the grid at 1223.2 (z -10.8, halfway, to the even -11.2) + EXPECT_EQ(results[1].center, NiPoint3(1223.2f, 433.92f, -11.2f)); + const auto bones = Transformations(results[1].lxfml, "Bone"); + EXPECT_EQ(bones.at("2"), "1,0,0,0,1,0,0,0,1,11.2,0,0.4"); + EXPECT_EQ(bones.at("3"), "1,0,0,0,1,0,0,0,1,13.6,0,0.4"); + EXPECT_EQ(bones.at("4"), "1,0,0,0,-1,0,0,0,-1,-13.2,0,0.4"); + const auto rigids = Transformations(results[1].lxfml, "Rigid"); + EXPECT_EQ(rigids.at("1"), bones.at("2")); + EXPECT_EQ(rigids.at("2"), bones.at("4")); +} + +TEST(LxfmlTests, SplitKeepsTheFileOrder) { + const auto results = Lxfml::Split(TWO_MODELS); + ASSERT_EQ(results.size(), 2); + EXPECT_EQ(BrickOrder(results[0].lxfml), (std::vector{ "0", "1" })); + EXPECT_EQ(BrickOrder(results[1].lxfml), (std::vector{ "2", "3" })); + // And the same model always comes out the same + EXPECT_EQ(Lxfml::Split(TWO_MODELS)[1].lxfml, results[1].lxfml); +} + +TEST(LxfmlTests, NormalizeWithoutReadableBonesKeepsTheModel) { + const std::string lxfml = PROBE_HEAD + R"()"; + const auto result = Lxfml::NormalizePosition(lxfml); + // Not somewhere far above the world: left as it is, at the origin + EXPECT_EQ(result.center, NiPoint3Constant::ZERO); + EXPECT_EQ(result.lxfml, lxfml); +} + +TEST(LxfmlTests, NormalizeAtAPositionKeepsTheBricksInPlace) { + // Bones already relative to the model; the given position is snapped to the grid and the bones make up for it + const std::string lxfml = PROBE_HEAD + + R"()" + R"()"; + const auto result = Lxfml::NormalizePosition(lxfml, NiPoint3(10.5f, 5.0f, -3.0f)); + EXPECT_EQ(result.center, NiPoint3(10.4f, 5.0f, -3.2f)); + EXPECT_EQ(Transformations(result.lxfml, "Bone").at("0"), "1,0,0,0,1,0,0,0,1,0.1,0,0.2"); + EXPECT_EQ(Transformations(result.lxfml, "Rigid").at("0"), "1,0,0,0,1,0,0,0,1,0.1,0,0.2"); +}