From 8017d8e8035916d49d075d30a07b3df362347ae2 Mon Sep 17 00:00:00 2001 From: Chris Twigg Date: Fri, 7 Nov 2025 15:03:54 -0800 Subject: [PATCH 1/2] Add round-trip test for parameter transform. Differential Revision: D86350961 --- CMakeLists.txt | 1 + .../io/skeleton/parameter_transform_io.cpp | 156 ++++++++++++++++++ momentum/io/skeleton/parameter_transform_io.h | 19 +++ momentum/test/io/io_model_parser_test.cpp | 104 ++++++++++++ 4 files changed, 280 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index c83cacae5e..4e38b516ec 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -749,6 +749,7 @@ if(MOMENTUM_BUILD_TESTING) character_test_helpers io_skeleton io_test_helper + character_test_helpers_gtest ) mt_test( diff --git a/momentum/io/skeleton/parameter_transform_io.cpp b/momentum/io/skeleton/parameter_transform_io.cpp index fcf93c80c4..32726e41b8 100644 --- a/momentum/io/skeleton/parameter_transform_io.cpp +++ b/momentum/io/skeleton/parameter_transform_io.cpp @@ -104,6 +104,7 @@ std::tuple loadModelDefinitionFromStream( pt = parseParameterTransform(data, skeleton); pt.parameterSets = parseParameterSets(data, pt); + pt.poseConstraints = parsePoseConstraints(data, pt); pl = parseParameterLimits(data, skeleton, pt); return res; @@ -435,4 +436,159 @@ std::tuple loadModelDefinition( return loadModelDefinitionFromStream(inputStream, skeleton); } +std::string writeParameterTransform( + const ParameterTransform& parameterTransform, + const Skeleton& skeleton) { + std::ostringstream oss; + + // Write each joint parameter definition + for (size_t iJoint = 0; iJoint < skeleton.joints.size(); ++iJoint) { + const auto& joint = skeleton.joints[iJoint]; + + for (size_t iParam = 0; iParam < kParametersPerJoint; ++iParam) { + const size_t jointParamIndex = iJoint * kParametersPerJoint + iParam; + + // Skip inactive joint parameters + if (!parameterTransform.activeJointParams[jointParamIndex]) { + continue; + } + + // Write joint parameter name + oss << joint.name << "." << kJointParameterNames[iParam] << " = "; + + // Collect all model parameters that influence this joint parameter + std::vector> influences; + for (Eigen::Index iModelParam = 0; iModelParam < parameterTransform.transform.cols(); + ++iModelParam) { + const float weight = parameterTransform.transform.coeff(jointParamIndex, iModelParam); + if (weight != 0.0f) { + influences.emplace_back(iModelParam, weight); + } + } + + // Write the offset if it exists + const float offset = parameterTransform.offsets[jointParamIndex]; + bool needsPlus = false; + + if (!influences.empty()) { + for (size_t i = 0; i < influences.size(); ++i) { + if (i > 0) { + oss << " + "; + } + const auto [modelParamIndex, weight] = influences[i]; + oss << weight << "*" << parameterTransform.name[modelParamIndex]; + } + needsPlus = true; + } + + if (offset != 0.0f) { + if (needsPlus) { + oss << " + "; + } + oss << offset; + } + + oss << "\n"; + } + } + + return oss.str(); +} + +std::string writeParameterSets(const ParameterSets& parameterSets) { + std::ostringstream oss; + + for (const auto& [name, paramSet] : parameterSets) { + oss << "parameterset " << name; + + // Write all active parameters in the set + for (size_t i = 0; i < paramSet.size(); ++i) { + if (paramSet.test(i)) { + // We need the parameter name, but we don't have access to parameterTransform here + // This is a limitation - we'll need to pass it in + oss << " param_" << i; + } + } + oss << "\n"; + } + + return oss.str(); +} + +std::string writePoseConstraints(const PoseConstraints& poseConstraints) { + std::ostringstream oss; + + for (const auto& [name, constraint] : poseConstraints) { + oss << "poseconstraints " << name; + + // Write all parameter=value pairs + for (const auto& [paramIndex, value] : constraint.parameterIdValue) { + // We need the parameter name, but we don't have access to parameterTransform here + oss << " param_" << paramIndex << "=" << value; + } + oss << "\n"; + } + + return oss.str(); +} + +std::string writeModelDefinition( + const Skeleton& skeleton, + const ParameterTransform& parameterTransform, + const ParameterLimits& parameterLimits) { + std::ostringstream oss; + + // Write header + oss << "Momentum Model Definition V1.0\n\n"; + + // Write ParameterTransform section + if (!parameterTransform.name.empty()) { + oss << "[ParameterTransform]\n"; + oss << writeParameterTransform(parameterTransform, skeleton); + oss << "\n"; + } + + // Write ParameterSets section + if (!parameterTransform.parameterSets.empty()) { + oss << "[ParameterSets]\n"; + for (const auto& [name, paramSet] : parameterTransform.parameterSets) { + oss << "parameterset " << name; + + // Write all active parameters in the set + for (size_t i = 0; i < paramSet.size() && i < parameterTransform.name.size(); ++i) { + if (paramSet.test(i)) { + oss << " " << parameterTransform.name[i]; + } + } + oss << "\n"; + } + oss << "\n"; + } + + // Write PoseConstraints section + if (!parameterTransform.poseConstraints.empty()) { + oss << "[PoseConstraints]\n"; + for (const auto& [name, constraint] : parameterTransform.poseConstraints) { + oss << "poseconstraints " << name; + + // Write all parameter=value pairs + for (const auto& [paramIndex, value] : constraint.parameterIdValue) { + if (paramIndex < parameterTransform.name.size()) { + oss << " " << parameterTransform.name[paramIndex] << "=" << value; + } + } + oss << "\n"; + } + oss << "\n"; + } + + // Write ParameterLimits section using existing function + if (!parameterLimits.empty()) { + oss << "[ParameterLimits]\n"; + oss << writeParameterLimits(parameterLimits, skeleton, parameterTransform); + } + + return oss.str(); +} + } // namespace momentum diff --git a/momentum/io/skeleton/parameter_transform_io.h b/momentum/io/skeleton/parameter_transform_io.h index fc250c7d11..e1f9851b35 100644 --- a/momentum/io/skeleton/parameter_transform_io.h +++ b/momentum/io/skeleton/parameter_transform_io.h @@ -38,4 +38,23 @@ std::tuple loadModelDefinition( std::span rawData, const Skeleton& skeleton); +// Write functions to serialize model definition components +std::string writeParameterTransform( + const ParameterTransform& parameterTransform, + const Skeleton& skeleton); + +std::string writeParameterSets(const ParameterSets& parameterSets); + +std::string writePoseConstraints(const PoseConstraints& poseConstraints); + +/// Write complete model definition file +/// @param skeleton The character's skeletal structure +/// @param parameterTransform Maps model parameters to joint parameters +/// @param parameterLimits Constraints on model parameters (can be empty) +/// @return String containing the complete model definition +std::string writeModelDefinition( + const Skeleton& skeleton, + const ParameterTransform& parameterTransform, + const ParameterLimits& parameterLimits); + } // namespace momentum diff --git a/momentum/test/io/io_model_parser_test.cpp b/momentum/test/io/io_model_parser_test.cpp index f522bb9b73..f00f933cc5 100644 --- a/momentum/test/io/io_model_parser_test.cpp +++ b/momentum/test/io/io_model_parser_test.cpp @@ -6,7 +6,10 @@ */ #include +#include +#include #include +#include using namespace momentum; @@ -103,3 +106,104 @@ limit param2 minmax [-2.0, 2.0] 1.0 EXPECT_TRUE(limitsContent.find("limit param1 minmax") != std::string::npos); EXPECT_TRUE(limitsContent.find("limit param2 minmax") != std::string::npos); } + +TEST(IoModelParserTest, WriteModelDefinition_RoundTrip) { + // Create original character + Character original; + original.name = "test_character"; + + // Create a simple skeleton + original.skeleton.joints.resize(2); + original.skeleton.joints[0].name = "root"; + original.skeleton.joints[0].parent = kInvalidIndex; + original.skeleton.joints[1].name = "child"; + original.skeleton.joints[1].parent = 0; + + // Create parameter transform + original.parameterTransform.name = {"tx", "ty", "tz"}; + original.parameterTransform.activeJointParams.setConstant( + original.skeleton.joints.size() * kParametersPerJoint, false); + original.parameterTransform.offsets.setZero( + original.skeleton.joints.size() * kParametersPerJoint); + + // Set up transform matrix (2 joints * 7 params per joint = 14 rows, 3 parameters = 3 cols) + original.parameterTransform.transform.resize( + original.skeleton.joints.size() * kParametersPerJoint, 3); + std::vector> triplets; + + // root.tx = 1.0 * tx + original.parameterTransform.activeJointParams[0] = true; + triplets.emplace_back(0, 0, 1.0f); + + // root.ty = 1.0 * ty + original.parameterTransform.activeJointParams[1] = true; + triplets.emplace_back(1, 1, 1.0f); + + // child.tz = 2.0 * tz + 0.5 + original.parameterTransform.activeJointParams[7 + 2] = true; + triplets.emplace_back(7 + 2, 2, 2.0f); + original.parameterTransform.offsets[7 + 2] = 0.5f; + + original.parameterTransform.transform.setFromTriplets(triplets.begin(), triplets.end()); + + // Add parameter sets + ParameterSet ps1; + ps1.set(0, true); // tx + ps1.set(1, true); // ty + original.parameterTransform.parameterSets["translation_xy"] = ps1; + + // Add pose constraints + PoseConstraint pc; + pc.parameterIdValue.emplace_back(0, 0.0f); // tx = 0 + pc.parameterIdValue.emplace_back(1, 0.0f); // ty = 0 + original.parameterTransform.poseConstraints["bind_pose"] = pc; + + // Create parameter limits + ParameterLimit limit1; + limit1.type = LimitType::MinMax; + limit1.weight = 1.0f; + limit1.data.minMax.parameterIndex = 0; + limit1.data.minMax.limits = Eigen::Vector2f(-1.0f, 1.0f); + original.parameterLimits.push_back(limit1); + + ParameterLimit limit2; + limit2.type = LimitType::MinMax; + limit2.weight = 1.0f; + limit2.data.minMax.parameterIndex = 1; + limit2.data.minMax.limits = Eigen::Vector2f(-2.0f, 2.0f); + original.parameterLimits.push_back(limit2); + + // Write model definition + const std::string written = writeModelDefinition( + original.skeleton, original.parameterTransform, original.parameterLimits); + + // Verify the output contains expected sections + EXPECT_TRUE(written.find("Momentum Model Definition V1.0") != std::string::npos); + EXPECT_TRUE(written.find("[ParameterTransform]") != std::string::npos); + EXPECT_TRUE(written.find("[ParameterSets]") != std::string::npos); + EXPECT_TRUE(written.find("[PoseConstraints]") != std::string::npos); + EXPECT_TRUE(written.find("[ParameterLimits]") != std::string::npos); + + // Verify parameter transform content + EXPECT_TRUE(written.find("root.tx = 1*tx") != std::string::npos); + EXPECT_TRUE(written.find("root.ty = 1*ty") != std::string::npos); + EXPECT_TRUE(written.find("child.tz = 2*tz + 0.5") != std::string::npos); + + // Verify parameter sets + EXPECT_TRUE(written.find("parameterset translation_xy tx ty") != std::string::npos); + + // Verify pose constraints + EXPECT_TRUE(written.find("poseconstraints bind_pose tx=0 ty=0") != std::string::npos); + + // Round-trip test: parse what we wrote and build a new character + Character roundtrip; + roundtrip.name = "test_character"; + roundtrip.skeleton = original.skeleton; + std::tie(roundtrip.parameterTransform, roundtrip.parameterLimits) = loadModelDefinition( + std::span( + reinterpret_cast(written.data()), written.size()), + original.skeleton); + + // Use the comprehensive compareChars helper to verify everything matches + compareChars(original, roundtrip, false); +} From bae435230eef1902714eea17bb921d047b0d95d0 Mon Sep 17 00:00:00 2001 From: Chris Twigg Date: Fri, 7 Nov 2025 16:00:19 -0800 Subject: [PATCH 2/2] Rework how we handle sections in the parameter transform. (#790) Summary: Pull Request resolved: https://github.com/facebookresearch/momentum/pull/790 After D86041361 landed, we suddenly started getting a ton of warnings of the format: [MOMENTUM][WARNING]: Invalid line under [ParameterSets] section; ignoring limit b_l_pinky3_rz minmax [-0.1, 1.57] This was because we suddenly started flagging when we saw something in the parameter sets section that didn't look like a parameterset. This is IMO a good change to make, but it causes problems with the current parser. It basically just runs all the parsers (parameter sets, parameter transform, parameter limits) over the entire file, basically ignoring the sections. Thus, when the parametersets parser sees a line that looks like a parameter limit, it issues a warning. One fix would be to just back out that change. However, I do think the current approach is not ideal, because it means we have to constantly update all the parsers to ignore stuff from the other parsers, which is potentially error-prone. Instead, I'm suggesting going ahead and breaking the document up into sections in a pre-process. Then for each section we can parse just that section. This turned out to be a little trickier than I expected, however, because in D86041361 we also added the ability to have the same section appear more than once in the file. We could just glue them together, but this screws up line numbers (used for better error messages). Therefore I'm introducing a new glass that appends all the sections _implicitly_, keeping tracking of the start/end line numbers for each. Reviewed By: jeongseok-meta, cstollmeta Differential Revision: D86348623 --- cmake/build_variables.bzl | 2 + momentum/io/skeleton/parameter_limits_io.cpp | 32 ++- momentum/io/skeleton/parameter_limits_io.h | 15 +- .../io/skeleton/parameter_transform_io.cpp | 241 +++++++++++++----- momentum/io/skeleton/parameter_transform_io.h | 30 ++- momentum/io/skeleton/utility.cpp | 55 ++++ momentum/io/skeleton/utility.h | 67 +++++ momentum/test/io/io_model_parser_test.cpp | 2 +- 8 files changed, 368 insertions(+), 76 deletions(-) create mode 100644 momentum/io/skeleton/utility.cpp create mode 100644 momentum/io/skeleton/utility.h diff --git a/cmake/build_variables.bzl b/cmake/build_variables.bzl index 716053fbcb..5c5f0a8904 100644 --- a/cmake/build_variables.bzl +++ b/cmake/build_variables.bzl @@ -384,6 +384,7 @@ io_skeleton_public_headers = [ "io/skeleton/parameter_limits_io.h", "io/skeleton/parameter_transform_io.h", "io/skeleton/parameters_io.h", + "io/skeleton/utility.h", ] io_skeleton_sources = [ @@ -392,6 +393,7 @@ io_skeleton_sources = [ "io/skeleton/parameter_limits_io.cpp", "io/skeleton/parameter_transform_io.cpp", "io/skeleton/parameters_io.cpp", + "io/skeleton/utility.cpp", ] io_skeleton_test_sources = [ diff --git a/momentum/io/skeleton/parameter_limits_io.cpp b/momentum/io/skeleton/parameter_limits_io.cpp index f0831037ae..9e42888db5 100644 --- a/momentum/io/skeleton/parameter_limits_io.cpp +++ b/momentum/io/skeleton/parameter_limits_io.cpp @@ -12,6 +12,8 @@ #include "momentum/character/parameter_transform.h" #include "momentum/character/skeleton.h" #include "momentum/common/log.h" +#include "momentum/common/string.h" +#include "momentum/io/skeleton/utility.h" #include "momentum/math/constants.h" #include "momentum/math/utility.h" @@ -609,23 +611,30 @@ void parseEllipsoid(const std::string& jointName, ParameterLimits& pl, Tokenizer } // namespace +// Internal overload that accepts SectionContent ParameterLimits parseParameterLimits( - const std::string& data, + const io_detail::SectionContent& content, const Skeleton& skeleton, const ParameterTransform& parameterTransform) { ParameterLimits pl; - std::istringstream f(data); + auto iterator = content.begin(); std::string line; - size_t lineIndex = 0; - while (std::getline(f, line)) { - ++lineIndex; + while (iterator.getline(line)) { + const size_t lineIndex = iterator.currentLine(); // erase all comments line = line.substr(0, line.find_first_of('#')); + // Skip empty lines + if (trim(line).empty()) { + continue; + } + // ignore everything but limits if (line.find("limit") != 0) { + MT_LOGW( + "Ignoring invalid line under [ParameterLimits] section at line {}: {}", lineIndex, line); continue; } @@ -672,6 +681,19 @@ ParameterLimits parseParameterLimits( return pl; } +// Public API wrapper for backward compatibility +ParameterLimits parseParameterLimits( + const std::string& data, + const Skeleton& skeleton, + const ParameterTransform& parameterTransform, + size_t lineOffset) { + io_detail::SectionContent content; + if (!data.empty()) { + content.addSegment(data, lineOffset); + } + return parseParameterLimits(content, skeleton, parameterTransform); +} + namespace { std::string vecToString(Eigen::Ref vec) { diff --git a/momentum/io/skeleton/parameter_limits_io.h b/momentum/io/skeleton/parameter_limits_io.h index 7a54dea214..782633e5a3 100644 --- a/momentum/io/skeleton/parameter_limits_io.h +++ b/momentum/io/skeleton/parameter_limits_io.h @@ -12,11 +12,24 @@ namespace momentum { +namespace io_detail { +// Forward declaration +class SectionContent; +} // namespace io_detail + +// Internal overload for use within momentum parsing ParameterLimits parseParameterLimits( - const std::string& data, + const io_detail::SectionContent& content, const Skeleton& skeleton, const ParameterTransform& parameterTransform); +// Public API for external use +ParameterLimits parseParameterLimits( + const std::string& data, + const Skeleton& skeleton, + const ParameterTransform& parameterTransform, + size_t lineOffset = 0); + std::string writeParameterLimits( const ParameterLimits& parameterLimits, const Skeleton& skeleton, diff --git a/momentum/io/skeleton/parameter_transform_io.cpp b/momentum/io/skeleton/parameter_transform_io.cpp index 32726e41b8..b306508b69 100644 --- a/momentum/io/skeleton/parameter_transform_io.cpp +++ b/momentum/io/skeleton/parameter_transform_io.cpp @@ -12,6 +12,7 @@ #include "momentum/common/string.h" #include "momentum/io/common/stream_utils.h" #include "momentum/io/skeleton/parameter_limits_io.h" +#include "momentum/io/skeleton/utility.h" #include "momentum/math/utility.h" #include @@ -22,22 +23,44 @@ namespace momentum { namespace { -std::unordered_map loadMomentumModelCommon(std::istream& input) { +using io_detail::SectionContent; + +// Section name constants to avoid typos +constexpr const char* kParameterTransformSection = "ParameterTransform"; +constexpr const char* kParameterSetsSection = "ParameterSets"; +constexpr const char* kPoseConstraintsSection = "PoseConstraints"; +constexpr const char* kParameterLimitsSection = "Limits"; + +bool isKnownSection(const std::string& sectionName) { + return sectionName == kParameterTransformSection || sectionName == kParameterSetsSection || + sectionName == kPoseConstraintsSection || sectionName == kParameterLimitsSection; +} + +std::unordered_map loadMomentumModelCommon(std::istream& input) { if (!input) { + MT_LOGW("Unable to read parameter transform data."); return {}; } - std::unordered_map result; + std::unordered_map result; std::string sectionName; std::string sectionContent; + size_t sectionStartLine = 0; + size_t linesNotInSectionCount = 0; + constexpr size_t kMaxLinesNotInSectionWarnings = 5; std::string line; + size_t lineNumber = 0; GetLineCrossPlatform(input, line); + ++lineNumber; if (trim(line) != "Momentum Model Definition V1.0") { + MT_LOGW( + "Invalid model definition file; expected 'Momentum Model Definition V1.0', got {}", line); return result; } while (GetLineCrossPlatform(input, line)) { + ++lineNumber; // erase all comments line = trim(line.substr(0, line.find_first_of('#'))); @@ -51,27 +74,43 @@ std::unordered_map loadMomentumModelCommon(std::istrea std::string newSectionName; if (re2::RE2::FullMatch(line, reg, &newSectionName)) { // new section, store old section - if (!sectionName.empty()) { - if (result.find(sectionName) != result.end()) { - MT_LOGW("Repeated section [{}] found; make sure it's intentional.", sectionName); + if (!sectionName.empty() && !sectionContent.empty()) { + if (isKnownSection(sectionName)) { + result[sectionName].addSegment(sectionContent, sectionStartLine); } - result[sectionName] += sectionContent; + } + + // Check if this is a known section type + if (!isKnownSection(newSectionName)) { + MT_LOGW("Unexpected section found at line {}: [{}]", lineNumber, newSectionName); } // start new section sectionName = newSectionName; sectionContent.clear(); + sectionStartLine = lineNumber + 1; // Content starts on the next line after section header } else if (!sectionName.empty()) { - sectionContent += line + "\n"; + // Only accumulate content for known sections + if (isKnownSection(sectionName)) { + sectionContent += line + "\n"; + } + } else { + // Line is not in a section and is not a comment + if (linesNotInSectionCount < kMaxLinesNotInSectionWarnings) { + MT_LOGW("Line {} is not in a section and is not a comment: {}", lineNumber, line); + ++linesNotInSectionCount; + if (linesNotInSectionCount == kMaxLinesNotInSectionWarnings) { + MT_LOGW("Suppressing further warnings about lines not in sections"); + } + } } } // store last section - if (!sectionName.empty()) { - if (result.find(sectionName) != result.end()) { - MT_LOGW("Repeated section [{}] found; make sure it's intentional.", sectionName); + if (!sectionName.empty() && !sectionContent.empty()) { + if (isKnownSection(sectionName)) { + result[sectionName].addSegment(sectionContent, sectionStartLine); } - result[sectionName] += sectionContent; } return result; @@ -82,30 +121,33 @@ std::tuple loadModelDefinitionFromStream( const Skeleton& skeleton) { MT_THROW_IF(!instream, "Unable to read parameter transform data."); - std::string data; - std::string line; - GetLineCrossPlatform(instream, line); + const auto sections = loadMomentumModelCommon(instream); - while (GetLineCrossPlatform(instream, line)) { - // erase all comments - line = trim(line.substr(0, line.find_first_of('#'))); + std::tuple res; + ParameterTransform& pt = std::get<0>(res); + ParameterLimits& pl = std::get<1>(res); - // skip empty lines or comment lines - if (line.empty()) { - continue; - } + const auto ptIt = sections.find(kParameterTransformSection); + if (ptIt == sections.end()) { + pt = parseParameterTransform(momentum::io_detail::SectionContent{}, skeleton); + } else { + pt = parseParameterTransform(ptIt->second, skeleton); + } - data += line + "\n"; + const auto psIt = sections.find(kParameterSetsSection); + if (psIt != sections.end()) { + pt.parameterSets = parseParameterSets(psIt->second, pt); } - std::tuple res; - ParameterTransform& pt = std::get<0>(res); - ParameterLimits& pl = std::get<1>(res); + const auto pcIt = sections.find(kPoseConstraintsSection); + if (pcIt != sections.end()) { + pt.poseConstraints = parsePoseConstraints(pcIt->second, pt); + } - pt = parseParameterTransform(data, skeleton); - pt.parameterSets = parseParameterSets(data, pt); - pt.poseConstraints = parsePoseConstraints(data, pt); - pl = parseParameterLimits(data, skeleton, pt); + const auto plIt = sections.find(kParameterLimitsSection); + if (plIt != sections.end()) { + pl = parseParameterLimits(plIt->second, skeleton, pt); + } return res; } @@ -212,7 +254,13 @@ std::unordered_map loadMomentumModel(const filesystem: std::ifstream infile(filename); MT_THROW_IF(!infile.is_open(), "Cannot find file {}", filename.string()); - return loadMomentumModelCommon(infile); + const auto sections = loadMomentumModelCommon(infile); + + std::unordered_map result; + for (const auto& [sectionName, content] : sections) { + result[sectionName] = content.toString(); + } + return result; } std::unordered_map loadMomentumModelFromBuffer( @@ -222,10 +270,19 @@ std::unordered_map loadMomentumModelFromBuffer( } ispanstream inputStream(buffer); - return loadMomentumModelCommon(inputStream); + const auto sections = loadMomentumModelCommon(inputStream); + + std::unordered_map result; + for (const auto& [sectionName, content] : sections) { + result[sectionName] = content.toString(); + } + return result; } -ParameterTransform parseParameterTransform(const std::string& data, const Skeleton& skeleton) { +// Internal overload that accepts SectionContent +ParameterTransform parseParameterTransform( + const SectionContent& content, + const Skeleton& skeleton) { ParameterTransform pt; pt.activeJointParams.setConstant(skeleton.joints.size() * kParametersPerJoint, false); pt.offsets.setZero(skeleton.joints.size() * kParametersPerJoint); @@ -233,24 +290,16 @@ ParameterTransform parseParameterTransform(const std::string& data, const Skelet // triplet list std::vector> triplets; - std::istringstream f(data); + auto iterator = content.begin(); std::string line; - while (std::getline(f, line)) { + while (iterator.getline(line)) { + const size_t lineIndex = iterator.currentLine(); + // erase all comments line = line.substr(0, line.find_first_of('#')); - // ignore limit lines - if (line.find("limit") == 0) { - continue; - } - - // load parameterset definitions - if (line.find("parameterset") == 0) { - continue; - } - - // load poseconstraints - if (line.find("poseconstraints") == 0) { + // Skip empty lines + if (trim(line).empty()) { continue; } @@ -259,17 +308,25 @@ ParameterTransform parseParameterTransform(const std::string& data, const Skelet // ------------------------------------------------ const auto pTokens = tokenize(line, "="); if (pTokens.size() != 2) { - MT_LOGW("Invalid line under [ParameterTransform] section; ignoring\n{}", line); + MT_LOGW( + "Ignoring invalid line under [ParameterTransform] section at line {}: {}", + lineIndex, + line); continue; } // split pToken[0] into joint name and attribute name const auto aTokens = tokenize(pTokens[0], "."); - MT_THROW_IF(aTokens.size() != 2, "Unknown joint name in expression : {}", line); + MT_THROW_IF( + aTokens.size() != 2, "Unknown joint name in expression at line {}: {}", lineIndex, line); // get the right joint to modify const size_t jointIndex = skeleton.getJointIdByName(trim(aTokens[0])); - MT_THROW_IF(jointIndex == kInvalidIndex, "Unknown joint name in expression : {}", line); + MT_THROW_IF( + jointIndex == kInvalidIndex, + "Unknown joint name in expression at line {}: {}", + lineIndex, + line); // the first pToken is the name of the joint and it's attribute type size_t attributeIndex = kInvalidIndex; @@ -282,7 +339,11 @@ ParameterTransform parseParameterTransform(const std::string& data, const Skelet } // if we didn't find a right name exit with an error - MT_THROW_IF(attributeIndex == kInvalidIndex, "Unknown channel name in expression : {}", line); + MT_THROW_IF( + attributeIndex == kInvalidIndex, + "Unknown channel name in expression at line {}: {}", + lineIndex, + line); // enable the attribute in the skeleton if we have a parameter controlling it pt.activeJointParams[jointIndex * kParametersPerJoint + attributeIndex] = 1; @@ -309,18 +370,37 @@ ParameterTransform parseParameterTransform(const std::string& data, const Skelet return pt; } -ParameterSets parseParameterSets(const std::string& data, const ParameterTransform& pt) { +// Public API wrapper for backward compatibility +ParameterTransform +parseParameterTransform(const std::string& data, const Skeleton& skeleton, size_t lineOffset) { + SectionContent content; + if (!data.empty()) { + content.addSegment(data, lineOffset); + } + return parseParameterTransform(content, skeleton); +} + +// Internal overload that accepts SectionContent +ParameterSets parseParameterSets(const SectionContent& content, const ParameterTransform& pt) { ParameterSets result; - std::istringstream f(data); + auto iterator = content.begin(); std::string line; - while (std::getline(f, line)) { + while (iterator.getline(line)) { + const size_t lineIndex = iterator.currentLine(); + // erase all comments line = line.substr(0, line.find_first_of('#')); + // Skip empty lines + if (trim(line).empty()) { + continue; + } + // Skip if not parameterset definitions if (line.find("parameterset") != 0) { - MT_LOGW("Invalid line under [ParameterSets] section; ignoring\n{}", line); + MT_LOGW( + "Ignoring invalid line under [ParameterSets] section at line {}: {}", lineIndex, line); continue; } @@ -328,7 +408,8 @@ ParameterSets parseParameterSets(const std::string& data, const ParameterTransfo const auto pTokens = tokenize(line, " \t\r\n"); MT_THROW_IF( pTokens.size() < 2, - "Could not parse parameterset line in parameter configuration : {}", + "Could not parse parameterset line in parameter configuration at line {}: {}", + lineIndex, line); ParameterSet ps; @@ -344,7 +425,8 @@ ParameterSets parseParameterSets(const std::string& data, const ParameterTransfo MT_THROW_IF( parameterIndex == kInvalidIndex, - "Could not parse parameterset line in parameter configuration : {}. Found unknown parameter name {}.", + "Could not parse parameterset line in parameter configuration at line {}: {}. Found unknown parameter name {}.", + lineIndex, line, parameterName); @@ -357,18 +439,37 @@ ParameterSets parseParameterSets(const std::string& data, const ParameterTransfo return result; } -PoseConstraints parsePoseConstraints(const std::string& data, const ParameterTransform& pt) { +// Public API wrapper for backward compatibility +ParameterSets +parseParameterSets(const std::string& data, const ParameterTransform& pt, size_t lineOffset) { + SectionContent content; + if (!data.empty()) { + content.addSegment(data, lineOffset); + } + return parseParameterSets(content, pt); +} + +// Internal overload that accepts SectionContent +PoseConstraints parsePoseConstraints(const SectionContent& content, const ParameterTransform& pt) { PoseConstraints result; - std::istringstream f(data); + auto iterator = content.begin(); std::string line; - while (std::getline(f, line)) { + while (iterator.getline(line)) { + const size_t lineIndex = iterator.currentLine(); + // erase all comments line = line.substr(0, line.find_first_of('#')); - // load parameterset definitions + // Skip empty lines + if (trim(line).empty()) { + continue; + } + + // Skip if not poseconstraints definitions if (line.find("poseconstraints") != 0) { - MT_LOGW("Invalid line under [PoseConstraints] section; ignoring\n{}", line); + MT_LOGW( + "Ignoring invalid line under [PoseConstraints] section at line {}: {}", lineIndex, line); continue; } @@ -376,7 +477,8 @@ PoseConstraints parsePoseConstraints(const std::string& data, const ParameterTra const auto pTokens = tokenize(line, " \t\r\n"); MT_THROW_IF( pTokens.size() < 2, - "Could not parse 'poseconstraints' line in parameter configuration : {}", + "Could not parse 'poseconstraints' line in parameter configuration at line {}: {}", + lineIndex, line); PoseConstraint ps; @@ -398,7 +500,8 @@ PoseConstraints parsePoseConstraints(const std::string& data, const ParameterTra MT_THROW_IF( parameterIndex == kInvalidIndex, - "Could not parse 'poseconstraints' line in parameter configuration : {}", + "Could not parse 'poseconstraints' line in parameter configuration at line {}: {}", + lineIndex, line); ps.parameterIdValue.emplace_back(parameterIndex, std::stof(cTokens[1])); @@ -410,6 +513,16 @@ PoseConstraints parsePoseConstraints(const std::string& data, const ParameterTra return result; } +// Public API wrapper for backward compatibility +PoseConstraints +parsePoseConstraints(const std::string& data, const ParameterTransform& pt, size_t lineOffset) { + SectionContent content; + if (!data.empty()) { + content.addSegment(data, lineOffset); + } + return parsePoseConstraints(content, pt); +} + std::tuple loadModelDefinition( const filesystem::path& filename, const Skeleton& skeleton) { @@ -584,7 +697,7 @@ std::string writeModelDefinition( // Write ParameterLimits section using existing function if (!parameterLimits.empty()) { - oss << "[ParameterLimits]\n"; + oss << "[" << kParameterLimitsSection << "]\n"; oss << writeParameterLimits(parameterLimits, skeleton, parameterTransform); } diff --git a/momentum/io/skeleton/parameter_transform_io.h b/momentum/io/skeleton/parameter_transform_io.h index e1f9851b35..2e1689072d 100644 --- a/momentum/io/skeleton/parameter_transform_io.h +++ b/momentum/io/skeleton/parameter_transform_io.h @@ -18,16 +18,36 @@ namespace momentum { +namespace io_detail { +// Forward declaration +class SectionContent; +} // namespace io_detail + std::unordered_map loadMomentumModel(const filesystem::path& filename); std::unordered_map loadMomentumModelFromBuffer( std::span buffer); -ParameterTransform parseParameterTransform(const std::string& data, const Skeleton& skeleton); - -ParameterSets parseParameterSets(const std::string& data, const ParameterTransform& pt); - -PoseConstraints parsePoseConstraints(const std::string& data, const ParameterTransform& pt); +// Internal overloads for use within momentum parsing +ParameterTransform parseParameterTransform( + const io_detail::SectionContent& content, + const Skeleton& skeleton); +ParameterSets parseParameterSets( + const io_detail::SectionContent& content, + const ParameterTransform& pt); +PoseConstraints parsePoseConstraints( + const io_detail::SectionContent& content, + const ParameterTransform& pt); + +// Public APIs for external use +ParameterTransform +parseParameterTransform(const std::string& data, const Skeleton& skeleton, size_t lineOffset = 0); + +ParameterSets +parseParameterSets(const std::string& data, const ParameterTransform& pt, size_t lineOffset = 0); + +PoseConstraints +parsePoseConstraints(const std::string& data, const ParameterTransform& pt, size_t lineOffset = 0); // load transform definition from file std::tuple loadModelDefinition( diff --git a/momentum/io/skeleton/utility.cpp b/momentum/io/skeleton/utility.cpp new file mode 100644 index 0000000000..d92dfbae9c --- /dev/null +++ b/momentum/io/skeleton/utility.cpp @@ -0,0 +1,55 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include "momentum/io/skeleton/utility.h" + +namespace momentum { +namespace io_detail { + +// LineIterator implementation +SectionContent::LineIterator::LineIterator(const std::vector& segments) + : segments_(segments), segmentIndex_(0), lineInSegment_(0) { + if (!segments_.empty()) { + currentStream_.str(segments_[0].content); + } +} + +bool SectionContent::LineIterator::getline(std::string& line) { + while (segmentIndex_ < segments_.size()) { + if (std::getline(currentStream_, line)) { + ++lineInSegment_; + return true; + } + + // Move to next segment + ++segmentIndex_; + lineInSegment_ = 0; + if (segmentIndex_ < segments_.size()) { + currentStream_.clear(); + currentStream_.str(segments_[segmentIndex_].content); + } + } + return false; +} + +size_t SectionContent::LineIterator::currentLine() const { + if (segmentIndex_ >= segments_.size()) { + return 0; + } + return segments_[segmentIndex_].startLine + lineInSegment_ - 1; +} + +std::string SectionContent::toString() const { + std::string result; + for (const auto& segment : segments_) { + result += segment.content; + } + return result; +} + +} // namespace io_detail +} // namespace momentum diff --git a/momentum/io/skeleton/utility.h b/momentum/io/skeleton/utility.h new file mode 100644 index 0000000000..e2f99d9d5e --- /dev/null +++ b/momentum/io/skeleton/utility.h @@ -0,0 +1,67 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include +#include +#include + +namespace momentum { +namespace io_detail { + +/// Represents a single segment of a section (when sections are split across the file) +struct SectionSegment { + std::string content; + size_t startLine; +}; + +/// Holds all segments of a section and provides line-aware iteration +/// This is used internally to track line numbers across potentially duplicate sections +class SectionContent { + public: + void addSegment(std::string_view content, size_t startLine) { + segments_.push_back({std::string(content), startLine}); + } + + [[nodiscard]] bool empty() const { + return segments_.empty(); + } + + /// Iterator that walks through all segments while tracking line numbers + class LineIterator { + public: + explicit LineIterator(const std::vector& segments); + + /// Get the next line from the sections + /// @return true if a line was read, false if end of all segments reached + bool getline(std::string& line); + + /// Get the current line number in the original file + [[nodiscard]] size_t currentLine() const; + + private: + const std::vector& segments_; + size_t segmentIndex_; + size_t lineInSegment_; + std::istringstream currentStream_; + }; + + [[nodiscard]] LineIterator begin() const { + return LineIterator(segments_); + } + + /// Get all content as a single concatenated string (for backward compatibility) + [[nodiscard]] std::string toString() const; + + private: + std::vector segments_; +}; + +} // namespace io_detail +} // namespace momentum diff --git a/momentum/test/io/io_model_parser_test.cpp b/momentum/test/io/io_model_parser_test.cpp index f00f933cc5..20bd583341 100644 --- a/momentum/test/io/io_model_parser_test.cpp +++ b/momentum/test/io/io_model_parser_test.cpp @@ -182,7 +182,7 @@ TEST(IoModelParserTest, WriteModelDefinition_RoundTrip) { EXPECT_TRUE(written.find("[ParameterTransform]") != std::string::npos); EXPECT_TRUE(written.find("[ParameterSets]") != std::string::npos); EXPECT_TRUE(written.find("[PoseConstraints]") != std::string::npos); - EXPECT_TRUE(written.find("[ParameterLimits]") != std::string::npos); + EXPECT_TRUE(written.find("[Limits]") != std::string::npos); // Verify parameter transform content EXPECT_TRUE(written.find("root.tx = 1*tx") != std::string::npos);