From 5373a48ca762398fd943c598476b6a7737195c86 Mon Sep 17 00:00:00 2001 From: Yuting Ye Date: Tue, 11 Nov 2025 18:05:05 -0800 Subject: [PATCH] Add tests to python binding (#752) Summary: Add python tests for marker sequence io in fbx and the unified save function. Reviewed By: jeongseok-meta Differential Revision: D85739418 --- momentum/io/fbx/fbx_io.cpp | 3 + momentum/io/fbx/openfbx_loader.cpp | 43 +++-- momentum/io/marker/marker_io.cpp | 7 +- pymomentum/geometry/momentum_io.cpp | 62 +++--- pymomentum/test/test_fbx.py | 288 ++++++++++++++++++++++++++++ pymomentum/test/test_fbx_io.py | 92 --------- 6 files changed, 345 insertions(+), 150 deletions(-) delete mode 100644 pymomentum/test/test_fbx_io.py diff --git a/momentum/io/fbx/fbx_io.cpp b/momentum/io/fbx/fbx_io.cpp index 9d02839212..3779b79154 100644 --- a/momentum/io/fbx/fbx_io.cpp +++ b/momentum/io/fbx/fbx_io.cpp @@ -323,6 +323,9 @@ std::vector<::fbxsdk::FbxNode*> createMarkerNodes( return markerNodes; } + // Set the framerate for the scene + setFrameRate(scene, framerate); + // Create a root node for all markers ::fbxsdk::FbxNode* markersRootNode = ::fbxsdk::FbxNode::Create(scene, "Markers"); ::fbxsdk::FbxNull* markersRootAttr = ::fbxsdk::FbxNull::Create(scene, "MarkersRootNull"); diff --git a/momentum/io/fbx/openfbx_loader.cpp b/momentum/io/fbx/openfbx_loader.cpp index 8a3cf59f5d..60069e038a 100644 --- a/momentum/io/fbx/openfbx_loader.cpp +++ b/momentum/io/fbx/openfbx_loader.cpp @@ -1058,6 +1058,7 @@ parseMarkerSequence(const ofbx::IScene* scene, const ofbx::Object* root, const f markersRoot = findMarkersRoot(root); if (!markersRoot) { + // Return empty sequence with no frames return result; } @@ -1076,12 +1077,14 @@ parseMarkerSequence(const ofbx::IScene* scene, const ofbx::Object* root, const f } if (markerNodes.empty()) { + // Return empty sequence with no frames return result; } // Get animation data for all marker nodes const ofbx::AnimationStack* animStack = scene->getAnimationStack(0); if (!animStack) { + // Return empty sequence with no frames return result; } @@ -1105,6 +1108,7 @@ parseMarkerSequence(const ofbx::IScene* scene, const ofbx::Object* root, const f } if (markerAnimCurves.empty()) { + // Return empty sequence with no frames return result; } @@ -1131,35 +1135,46 @@ parseMarkerSequence(const ofbx::IScene* scene, const ofbx::Object* root, const f } if (translationCurve) { - // Collect all unique keyframe times across X, Y, Z channels + // Get the individual animation curves for X, Y, Z components + const ofbx::AnimationCurve* curveX = translationCurve->getCurve(0); + const ofbx::AnimationCurve* curveY = translationCurve->getCurve(1); + const ofbx::AnimationCurve* curveZ = translationCurve->getCurve(2); + + // Collect all unique keyframe times from all three curves std::set keyframeTimes; - for (int iChannel = 0; iChannel < 3; ++iChannel) { - const ofbx::AnimationCurve* channel = translationCurve->getCurve(iChannel); - if (channel == nullptr) { - continue; + if (curveX) { + const int keyCount = curveX->getKeyCount(); + const ofbx::i64* times = curveX->getKeyTime(); + for (int i = 0; i < keyCount; ++i) { + keyframeTimes.insert(times[i]); } - const int keyCount = channel->getKeyCount(); - if (keyCount <= 0) { - continue; + } + if (curveY) { + const int keyCount = curveY->getKeyCount(); + const ofbx::i64* times = curveY->getKeyTime(); + for (int i = 0; i < keyCount; ++i) { + keyframeTimes.insert(times[i]); } - const ofbx::i64* times = channel->getKeyTime(); + } + if (curveZ) { + const int keyCount = curveZ->getKeyCount(); + const ofbx::i64* times = curveZ->getKeyTime(); for (int i = 0; i < keyCount; ++i) { keyframeTimes.insert(times[i]); } } - // For each unique keyframe time, add a marker at that frame - // This matches GLTF behavior: only add markers at explicitly keyed frames - for (ofbx::i64 fbxTime : keyframeTimes) { + // Add markers only at frames where keyframes exist + for (const ofbx::i64 fbxTime : keyframeTimes) { const double timeInSeconds = ofbx::fbxTimeToSeconds(fbxTime); - const auto frameIndex = static_cast(std::lround(timeInSeconds * fps)); + const size_t frameIndex = std::round(timeInSeconds * fps); // Skip if frame index is out of bounds if (frameIndex >= numFrames) { continue; } - // Evaluate position at this specific keyframe time + // Evaluate position at this keyframe time const ofbx::DVec3 position = translationCurve->getNodeLocalTransform(timeInSeconds); Marker marker; diff --git a/momentum/io/marker/marker_io.cpp b/momentum/io/marker/marker_io.cpp index c6d0969f01..dea6396f87 100644 --- a/momentum/io/marker/marker_io.cpp +++ b/momentum/io/marker/marker_io.cpp @@ -87,12 +87,9 @@ int findMainSubjectIndex(std::span markerSequences) { } // Special case when there's only one actor. + // Return the actor even if it has no markers (empty sequence is valid). if (numActors == 1) { - if (maxMarkers.at(0) > 0) { - return 0; - } else { - return -1; - } + return 0; } // find the max marker count with a name diff --git a/pymomentum/geometry/momentum_io.cpp b/pymomentum/geometry/momentum_io.cpp index 80598547fa..b4b2806b09 100644 --- a/pymomentum/geometry/momentum_io.cpp +++ b/pymomentum/geometry/momentum_io.cpp @@ -167,26 +167,18 @@ void saveFBXCharacterToFile( const std::optional>>& markers, const std::optional& coordSystemInfo, std::string_view fbxNamespace) { - if (motion.has_value() && offsets.has_value()) { - momentum::saveFbx( - path, - character, - motion.value().transpose(), - offsets.value(), - fps, - true, /*saveMesh*/ - coordSystemInfo.value_or(momentum::FBXCoordSystemInfo()), - false, /*permissive*/ - markers.value_or(std::vector>{}), - fbxNamespace); - } else { - momentum::saveFbxModel( - path, - character, - coordSystemInfo.value_or(momentum::FBXCoordSystemInfo()), - false, /*permissive*/ - fbxNamespace); - } + // Always use saveFbx to support markers even without motion + momentum::saveFbx( + path, + character, + motion.has_value() ? motion.value().transpose() : Eigen::MatrixXf(), + offsets.has_value() ? offsets.value() : Eigen::VectorXf(), + fps, + true, /*saveMesh*/ + coordSystemInfo.value_or(momentum::FBXCoordSystemInfo()), + false, /*permissive*/ + markers.value_or(std::vector>{}), + fbxNamespace); } void saveFBXCharacterToFileWithJointParams( @@ -197,25 +189,17 @@ void saveFBXCharacterToFileWithJointParams( const std::optional>>& markers, const std::optional& coordSystemInfo, std::string_view fbxNamespace) { - if (jointParams.has_value()) { - momentum::saveFbxWithJointParams( - path, - character, - jointParams.value().transpose(), - fps, - true, /*saveMesh*/ - coordSystemInfo.value_or(momentum::FBXCoordSystemInfo()), - false, /*permissive*/ - markers.value_or(std::vector>{}), - fbxNamespace); - } else { - momentum::saveFbxModel( - path, - character, - coordSystemInfo.value_or(momentum::FBXCoordSystemInfo()), - false, /*permissive*/ - fbxNamespace); - } + // Always use saveFbxWithJointParams to support markers even without motion + momentum::saveFbxWithJointParams( + path, + character, + jointParams.has_value() ? jointParams.value().transpose() : Eigen::MatrixXf(), + fps, + true, /*saveMesh*/ + coordSystemInfo.value_or(momentum::FBXCoordSystemInfo()), + false, /*permissive*/ + markers.value_or(std::vector>{}), + fbxNamespace); } void saveCharacterToFile( diff --git a/pymomentum/test/test_fbx.py b/pymomentum/test/test_fbx.py index 97492e1fa9..2405e61517 100644 --- a/pymomentum/test/test_fbx.py +++ b/pymomentum/test/test_fbx.py @@ -6,6 +6,7 @@ # pyre-strict import math import pkgutil +import tempfile import unittest import numpy as np @@ -14,6 +15,17 @@ class TestFBX(unittest.TestCase): + def setUp(self) -> None: + self.character = pym_geometry.create_test_character() + torch.manual_seed(0) # ensure repeatability + + nBatch = 5 + nParams = self.character.parameter_transform.size + self.model_params = pym_geometry.uniform_random_to_model_parameters( + self.character, torch.rand(nBatch, nParams) + ).double() + self.joint_params = self.character.parameter_transform.apply(self.model_params) + def test_load_animation(self) -> None: """ Loads a very simple 3-bone chain with animation from FBX and verifies that @@ -111,3 +123,279 @@ def test_load_animation(self) -> None: skel_state_last[joint3][0:3], torch.Tensor([4, 0, 0]), atol=1e-5 ) ) + + def test_save_motions_with_model_params(self) -> None: + if not pym_geometry.is_fbxsdk_available(): + return + + # Test saving with model parameters + with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: + offsets = np.zeros(self.joint_params.shape[1]) + pym_geometry.Character.save_fbx( + path=temp_file.name, + character=self.character, + motion=self.model_params.numpy(), + offsets=offsets, + fps=60, + ) + self._verify_fbx(temp_file.name) + + # Test saving with model parameters + with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: + offsets = np.zeros(self.joint_params.shape[1]) + pym_geometry.Character.save_fbx( + path=temp_file.name, + character=self.character, + motion=self.model_params.numpy(), + offsets=offsets, + fps=60, + coord_system_info=pym_geometry.FBXCoordSystemInfo( + pym_geometry.FBXUpVector.YAxis, + pym_geometry.FBXFrontVector.ParityEven, + pym_geometry.FBXCoordSystem.LeftHanded, + ), + ) + self._verify_fbx(temp_file.name) + + def test_save_motions_with_joint_params(self) -> None: + if not pym_geometry.is_fbxsdk_available(): + return + + # Test saving with joint parameters + with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: + pym_geometry.Character.save_fbx_with_joint_params( + path=temp_file.name, + character=self.character, + joint_params=self.joint_params.numpy(), + fps=60, + ) + self._verify_fbx(temp_file.name) + + # Test saving with joint parameters using non-default coord-system + with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: + pym_geometry.Character.save_fbx_with_joint_params( + path=temp_file.name, + character=self.character, + joint_params=self.joint_params.numpy(), + fps=60, + coord_system_info=pym_geometry.FBXCoordSystemInfo( + pym_geometry.FBXUpVector.YAxis, + pym_geometry.FBXFrontVector.ParityEven, + pym_geometry.FBXCoordSystem.RightHanded, + ), + ) + self._verify_fbx(temp_file.name) + + def _verify_fbx(self, file_name: str) -> None: + # Load FBX file + _, motion, fps = pym_geometry.Character.load_fbx_with_motion(file_name) + self.assertEqual(1, len(motion)) + self.assertEqual(motion[0].shape, self.joint_params.shape) + self.assertEqual(fps, 60) + + def test_save_with_namespace(self) -> None: + if not pym_geometry.is_fbxsdk_available(): + return + + """Test FBX save with namespace parameter.""" + with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: + offsets = np.zeros(self.joint_params.shape[1]) + # Save with namespace + pym_geometry.Character.save_fbx( + path=temp_file.name, + character=self.character, + motion=self.model_params.numpy(), + offsets=offsets, + fps=60, + fbx_namespace="test_ns", + ) + # Verify file can be loaded + self._verify_fbx(temp_file.name) + + def test_save_with_joint_params_and_namespace(self) -> None: + if not pym_geometry.is_fbxsdk_available(): + return + + """Test FBX save with joint params and namespace parameter.""" + with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: + pym_geometry.Character.save_fbx_with_joint_params( + path=temp_file.name, + character=self.character, + joint_params=self.joint_params.numpy(), + fps=60, + fbx_namespace="test_ns", + ) + # Verify file can be loaded + self._verify_fbx(temp_file.name) + + def test_unified_save_fbx(self) -> None: + if not pym_geometry.is_fbxsdk_available(): + return + + """Test unified save function with FBX extension.""" + with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: + # Use unified save function - should auto-detect FBX format + pym_geometry.Character.save( + path=temp_file.name, + character=self.character, + fps=60, + motion=self.model_params.numpy(), + ) + # Verify file can be loaded + self._verify_fbx(temp_file.name) + + def test_marker_sequence_fbx_roundtrip(self) -> None: + """Test saving and loading marker sequences with FBX. + + Note: FBX format does not preserve occlusion information. Occluded markers + are not saved, and when loaded, all markers are interpolated for all frames. + """ + if not pym_geometry.is_fbxsdk_available(): + return + + with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: + # Create test marker sequence without occlusion + # (FBX doesn't preserve occlusion state) + nFrames = 5 + markers_per_frame = [] + for frame in range(nFrames): + frame_markers = [] + for i in range(3): + marker = pym_geometry.Marker( + name=f"marker_{i}", + pos=np.array( + [float(frame + i), float(i), float(frame)], dtype=np.float32 + ), + occluded=False, # FBX doesn't support occlusion + ) + frame_markers.append(marker) + markers_per_frame.append(frame_markers) + + # Save with unified function + pym_geometry.Character.save( + path=temp_file.name, + character=self.character, + fps=60, + markers=markers_per_frame, + ) + + # Load markers using load_markers function + marker_sequences = pym_geometry.load_markers(temp_file.name) + self.assertEqual(len(marker_sequences), 1) + + loaded_sequence = marker_sequences[0] + self.assertEqual(len(loaded_sequence.frames), nFrames) + self.assertEqual(loaded_sequence.fps, 60.0) + + # Verify marker data - all markers should be present in all frames + for _frame_idx, frame in enumerate(loaded_sequence.frames): + self.assertEqual(len(frame), 3) # All 3 markers present + # Verify marker names + marker_names = {m.name for m in frame} + self.assertEqual(marker_names, {"marker_0", "marker_1", "marker_2"}) + + def test_marker_sequence_fbx_with_motion(self) -> None: + """Test saving markers and motion together in FBX.""" + if not pym_geometry.is_fbxsdk_available(): + return + + with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: + # Create test marker sequence + nFrames = 3 + markers_per_frame = [] + for frame in range(nFrames): + frame_markers = [] + for i in range(2): + marker = pym_geometry.Marker( + name=f"marker_{i}", + pos=np.array([float(i), float(frame), 0.0], dtype=np.float32), + occluded=False, + ) + frame_markers.append(marker) + markers_per_frame.append(frame_markers) + + # Save with both motion and markers + pym_geometry.Character.save( + path=temp_file.name, + character=self.character, + motion=self.model_params[:nFrames].numpy(), + fps=60, + markers=markers_per_frame, + ) + + # Load and verify markers + marker_sequences = pym_geometry.load_markers(temp_file.name) + self.assertEqual(len(marker_sequences), 1) + self.assertEqual(len(marker_sequences[0].frames), nFrames) + + # Load and verify motion + _loaded_char, motion, _loaded_fps = ( + pym_geometry.Character.load_fbx_with_motion(temp_file.name) + ) + self.assertEqual(len(motion), 1) + self.assertEqual(motion[0].shape[0], nFrames) + + def test_marker_sequence_sparse_keyframes(self) -> None: + if not pym_geometry.is_fbxsdk_available(): + return + + """Test that marker sequences support sparse keyframes (not all frames have markers).""" + with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: + # Create sparse marker sequence - only keyframe at frame 0 and 2 + nFrames = 3 + markers_per_frame = [] + + # Frame 0: has markers + frame_markers_0 = [ + pym_geometry.Marker( + name="marker_0", + pos=np.array([0.0, 0.0, 0.0], dtype=np.float32), + occluded=False, + ) + ] + markers_per_frame.append(frame_markers_0) + + # Frame 1: empty - no keyframe + markers_per_frame.append([]) + + # Frame 2: has markers + frame_markers_2 = [ + pym_geometry.Marker( + name="marker_0", + pos=np.array([2.0, 2.0, 2.0], dtype=np.float32), + occluded=False, + ) + ] + markers_per_frame.append(frame_markers_2) + + # Save + pym_geometry.Character.save( + path=temp_file.name, + character=self.character, + fps=60, + markers=markers_per_frame, + ) + + # Load markers + marker_sequences = pym_geometry.load_markers(temp_file.name) + self.assertEqual(len(marker_sequences), 1) + + loaded_sequence = marker_sequences[0] + # Should have 3 frames total (sparse support) + self.assertEqual(len(loaded_sequence.frames), nFrames) + + def test_load_markers_empty_file(self) -> None: + if not pym_geometry.is_fbxsdk_available(): + return + + """Test loading markers from a file without markers.""" + with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: + # Save character without markers + pym_geometry.Character.save( + path=temp_file.name, character=self.character, fps=60 + ) + + # Load markers - should return empty sequence + marker_sequences = pym_geometry.load_markers(temp_file.name) + self.assertEqual(len(marker_sequences), 1) + self.assertEqual(len(marker_sequences[0].frames), 0) diff --git a/pymomentum/test/test_fbx_io.py b/pymomentum/test/test_fbx_io.py deleted file mode 100644 index 4a8b5c56c7..0000000000 --- a/pymomentum/test/test_fbx_io.py +++ /dev/null @@ -1,92 +0,0 @@ -# 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. - -# pyre-strict -import tempfile -import unittest - -import numpy as np -import pymomentum.geometry as pym_geometry -import torch - - -class TestFBXIO(unittest.TestCase): - def setUp(self) -> None: - self.character = pym_geometry.create_test_character() - torch.manual_seed(0) # ensure repeatability - - nBatch = 5 - nParams = self.character.parameter_transform.size - self.model_params = pym_geometry.uniform_random_to_model_parameters( - self.character, torch.rand(nBatch, nParams) - ).double() - self.joint_params = self.character.parameter_transform.apply(self.model_params) - - def test_save_motions_with_model_params(self) -> None: - # TODO: Disable this test programmatically if momentum is not built with FBX support - # Test saving with model parameters - with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: - offsets = np.zeros(self.joint_params.shape[1]) - pym_geometry.Character.save_fbx( - path=temp_file.name, - character=self.character, - motion=self.model_params.numpy(), - offsets=offsets, - fps=60, - ) - self._verify_fbx(temp_file.name) - - # Test saving with model parameters - with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: - offsets = np.zeros(self.joint_params.shape[1]) - pym_geometry.Character.save_fbx( - path=temp_file.name, - character=self.character, - motion=self.model_params.numpy(), - offsets=offsets, - fps=60, - coord_system_info=pym_geometry.FBXCoordSystemInfo( - pym_geometry.FBXUpVector.YAxis, - pym_geometry.FBXFrontVector.ParityEven, - pym_geometry.FBXCoordSystem.LeftHanded, - ), - ) - self._verify_fbx(temp_file.name) - - def test_save_motions_with_joint_params(self) -> None: - # TODO: Disable this test programmatically if momentum is not built with FBX support - # Test saving with joint parameters - with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: - pym_geometry.Character.save_fbx_with_joint_params( - path=temp_file.name, - character=self.character, - joint_params=self.joint_params.numpy(), - fps=60, - ) - self._verify_fbx(temp_file.name) - - # Test saving with joint parameters using non-default coord-system - with tempfile.NamedTemporaryFile(suffix=".fbx") as temp_file: - pym_geometry.Character.save_fbx_with_joint_params( - path=temp_file.name, - character=self.character, - joint_params=self.joint_params.numpy(), - fps=60, - coord_system_info=pym_geometry.FBXCoordSystemInfo( - pym_geometry.FBXUpVector.YAxis, - pym_geometry.FBXFrontVector.ParityEven, - pym_geometry.FBXCoordSystem.RightHanded, - ), - ) - self._verify_fbx(temp_file.name) - - def _verify_fbx(self, file_name: str) -> None: - # Load FBX file - l_character, motion, fps = pym_geometry.Character.load_fbx_with_motion( - file_name - ) - self.assertEqual(1, len(motion)) - self.assertEqual(motion[0].shape, self.joint_params.shape) - self.assertEqual(fps, 60)