From 4b4cb55f22c28af5637819d7b2a3ecffd48fc372 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Lortie Date: Sat, 11 Mar 2023 16:25:04 -0500 Subject: [PATCH 1/8] Fix HL1MDLLoader flattened bone hierarchy. --- code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp | 43 ++++++++++++++++++--- code/AssetLib/MDL/HalfLife/HL1MDLLoader.h | 9 +++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp b/code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp index 93d37536c..237ccf3d8 100644 --- a/code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp +++ b/code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp @@ -472,12 +472,13 @@ void HL1MDLLoader::read_bones() { aiNode *bones_node = new aiNode(AI_MDL_HL1_NODE_BONES); rootnode_children_.push_back(bones_node); - bones_node->mNumChildren = static_cast(header_->numbones); - bones_node->mChildren = new aiNode *[bones_node->mNumChildren]; + + // Store roots bones IDs temporarily. + std::vector roots; // Create bone matrices in local space. for (int i = 0; i < header_->numbones; ++i) { - aiNode *bone_node = temp_bones_[i].node = bones_node->mChildren[i] = new aiNode(unique_bones_names[i]); + aiNode *bone_node = temp_bones_[i].node = new aiNode(unique_bones_names[i]); aiVector3D angles(pbone[i].value[3], pbone[i].value[4], pbone[i].value[5]); temp_bones_[i].absolute_transform = bone_node->mTransformation = @@ -485,9 +486,11 @@ void HL1MDLLoader::read_bones() { aiVector3D(pbone[i].value[0], pbone[i].value[1], pbone[i].value[2])); if (pbone[i].parent == -1) { - bone_node->mParent = scene_->mRootNode; + bone_node->mParent = bones_node; + roots.push_back(i); // This bone has no parent. Add it to the roots list. } else { - bone_node->mParent = bones_node->mChildren[pbone[i].parent]; + bone_node->mParent = temp_bones_[pbone[i].parent].node; + temp_bones_[pbone[i].parent].children.push_back(i); // Add this bone to the parent bone's children list. temp_bones_[i].absolute_transform = temp_bones_[pbone[i].parent].absolute_transform * bone_node->mTransformation; @@ -496,6 +499,36 @@ void HL1MDLLoader::read_bones() { temp_bones_[i].offset_matrix = temp_bones_[i].absolute_transform; temp_bones_[i].offset_matrix.Inverse(); } + + // Create the 'bones' root node that will contain all bone nodes. + bones_node->mNumChildren = static_cast(roots.size()); + bones_node->mChildren = new aiNode *[bones_node->mNumChildren]; + + // Build all bones children hierarchy starting from each root bone. + for (size_t i = 0; i < roots.size(); ++i) + { + const TempBone &root_bone = temp_bones_[roots[i]]; + bones_node->mChildren[i] = root_bone.node; + build_bone_children_hierarchy(root_bone); + } +} + +void HL1MDLLoader::build_bone_children_hierarchy(const TempBone &bone) +{ + if (bone.children.size() > 0) + { + aiNode* bone_node = bone.node; + bone_node->mNumChildren = static_cast(bone.children.size()); + bone_node->mChildren = new aiNode *[bone_node->mNumChildren]; + + // Build each child bone's hierarchy recursively. + for (size_t i = 0; i < bone.children.size(); ++i) + { + const TempBone &child_bone = temp_bones_[bone.children[i]]; + bone_node->mChildren[i] = child_bone.node; + build_bone_children_hierarchy(child_bone); + } + } } // ------------------------------------------------------------------------------------------------ diff --git a/code/AssetLib/MDL/HalfLife/HL1MDLLoader.h b/code/AssetLib/MDL/HalfLife/HL1MDLLoader.h index 0dba5099d..3fc84c1be 100644 --- a/code/AssetLib/MDL/HalfLife/HL1MDLLoader.h +++ b/code/AssetLib/MDL/HalfLife/HL1MDLLoader.h @@ -143,6 +143,14 @@ private: */ static bool get_num_blend_controllers(const int num_blend_animations, int &num_blend_controllers); + /** + * \brief Build a bone's node children hierarchy. + * + * \param[in] bone The bone for which we must build all children hierarchy. + */ + struct TempBone; + void build_bone_children_hierarchy(const TempBone& bone); + /** Output scene to be filled */ aiScene *scene_; @@ -203,6 +211,7 @@ private: aiNode *node; aiMatrix4x4 absolute_transform; aiMatrix4x4 offset_matrix; + std::vector children; // Bone children }; std::vector temp_bones_; From 3c2a42586925115df891df4146181e8d08563e31 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Lortie Date: Sat, 11 Mar 2023 20:46:42 -0500 Subject: [PATCH 2/8] Added a test to validate HL1 MDL bone hierarchy. --- test/models/MDL/MDL (HL1)/multiple_roots.mdl | Bin 0 -> 15372 bytes .../MDL/utMDLImporter_HL1_Nodes.cpp | 67 ++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 test/models/MDL/MDL (HL1)/multiple_roots.mdl diff --git a/test/models/MDL/MDL (HL1)/multiple_roots.mdl b/test/models/MDL/MDL (HL1)/multiple_roots.mdl new file mode 100644 index 0000000000000000000000000000000000000000..f8fc1eed708c8b3f95c5faf7c55e78cb07434771 GIT binary patch literal 15372 zcmeI2dz2LQmB;VAV0cOMRAa!xNq7pj!iYjo|0?kTL_|i6Z=t{$5_AxR0R*Cy7$d#_ zqY)p8R3aF&W{m-c5S-~MV>S#P8A~I&oDg1Z!g82*QH8nQOn!li-v1#!s^OrO=&s*5k zIBU^@1aKf^vSmea2j#MUbx{d)d)T_(HG{Ug__RoM?+M^mqU^~L5>@4l_`YdLbgu^*~+ zXrQC{rR(|HHwW8!+PwDn?~wI8{ZRP>xH3T<)bsy1>IPew&YH6 zRV)3zczAyNJL&M)L9d~4?rgr`9t`}Q(BkGL4f7WK_2TJzJ*D4W8|OAIYP|01BP{e0 zi>anRNaO*m!wu$!ULyLT+5vu`|A*5nldCGZ%95)*xhj&YOLA2wSGVNqo?JbWt7mc@ zlU%)$E0bJ(ldDg1bxp3`$#t;9w6n^}w706Nv^$RMfh+B>q9X0FOP93E>gu%5Zr##O zyLV4}?a?Fcwr9_@-(!wRJMPsh?KzW4yYAaJ?YmE(wDYcA)82b?gI)Kd-EBID?yo)C z2ChqRo%)J32DWkK#{MlvW9oN*&0_4M7N>lT>-dN(d)wcyjJJaGiGEIf#Toc_LkIAY~#vR&aHo{*WT*XPq5C*Vjb5pjqAAf z5bM;`Y3&4~Q)4yberjwF%2mIv`z7So*xUqDtENtI%1Vv7Z&T493 z>T6uN>Ob`0W$3K4eI{5rHlFe|u6*qy*3@-A(6jAR;~LYx6Kv=8?Cv$%Cu>{Nmf*bh ztvKZ;n7RjYb-&W}VZUn9J=MDDUMF>Z)zR}nU5hyn%Ji$>uA7j7M zXl{zNf5mD4igjFT>6p${=aAsM`ieD{<|tSF_C80-Pd{tQPjK#?`W^QCJUQu&Hn2U1 z;5zkHUt{VgIN|t4V{xvT_1?`lz2`^j=@#B=XM{#1@%>5W9lcEd(@J;{!dOa z8q?ea=asKG&QmpkA>--MEf7SjzIVt{|AztSgtER?N{R9`vCCeW} zepoIKYy)r8`x1EE{^`5&TVCjAuADJ4Y~(fSg?{0f3wIUXx^QGz^rhZp}xFK$c z=hx2Im46ma1LNQV*nkau13!eHN3(!th^EEf+wx<9N!3La%Z~K97&itU#x3HeC z?3fh3y>n7{GW_4&vo!y8@V;O4GxZym=7}5PhIrxi2bLDDqi)j=CxzfV*nkau13!dc zKr@eKh^E3VCEEZ@J;6 z0`ZWzA)asMwV2+rZ;dmhB`+0jq@Uq$j|<_4@I&}{Gz(~kXmXw$TUXE02c zAHomOG-!rs<|`*(YwK35*?v0M)&*~OzO%gTd3yfS!s+2MUUT4Aet&u0ec+4H+_`ai z9dSe45N~_;+soT-hclOPa2?oy4SWMXgx`i{9hxDUrklSfB$*k^zfSf)5F>1p4B`v=ai<04fJ!?bjsGy*w>^;-zWfXzKoO+^)9C z;QiG4{fJ#{#0_ynyl&;#U3Jf)zlU*f8`yvid;>p(Ux#KJnjxCzz0z;&d48YIZ~64s z?RhSfGitYdyKd(%Ik#_hsSm$VRUdwn+-`q#C+xuQi{?8yGl94vZiw%^sb%ucnat%_ zxZnw312*st{1E<5G$)`LqRD(uwClXTR8hOP^n1I`AC$(|p4elW=|}Eu-{_gYdt=Xh zH|qYh$F%U55BJO$soSgPw2*j#xFK#FUggxi7V z9;$Gk%nm-i{J8Dg?K+=PyM66|!RFWK|3_FI{<2UVzCzsx2Mi9^;Ga+Ynt_8gZipM= z=3!pF;cR9c9D)tlz&G$i_y)}o%@9pf5&S~;l+RPpQs#`b^Ti)~>N7M7GQXl@N ztUhdo|DA_NcgevZhL+}>&?`i%3Nj)7}!eO5I4kgf8IMF z*9*<};e%Vj25jIP_#ylpnyq@jLeuOit+Mld@3(JpfBv;%voD-dd(L~)!ztuG*Zc0) z(f#gj{daQLyf@t}F5acstOpa%DJC91$m?}DC;a}dRSmsu7W%v%Blov8yW3s?e+kX69k;uUxFK$c*FDvHcb)$3 z_$lMyHvN48Ht-Gn5Plt+ZD@vQno?<)t*dr_sboJ#rP9qU+qUJxm#JI!m&OnUX13@%KGpe zUPGz-=*ay~-$>oF(A*sDdz!c*Zir7j@#TFJ^{oDiy5Oh525jIP_#ym>Xg-Z*h^Bdq zb9~<*JSBq(Zug;iponh-Z0fR*jb>+7;h{)eYZav=U#RYgXWAl6>_v zUln@`4|c#k7*)ldL{zbx^u^1{G13R$f!Pn=kzPbpu^Oa`alRz^s(7pnw2J-Y6EKYQ zm*cHsANeF+meCWeVqY1A@5lf-&MIceRK+-7sHgd=cp@zA@dWu4?*^S3{4Bad@v<^l zK8^3dABOLsdlG&I-68myBwvzzRU9TGtzwOgfnm@cZ54;0riw%56svfWoQm(Df3j6{ zWE6fT$rt+2d{sO}I40mxat7}PzCL5l$_SZ&mxXsGzC+!K_zv~-nRiBpOD%pT$(JNw z6~~Kyu{JVRKFhm7|1(zcR5=?jOa56_@iaLH-$BR4&oH9wOp-52zA74-48zdDRLjZ9 zdH7j6n}(mI-ud_r9bbU&NS%DnDxM)z@b!FXz9jjoI7u$VH*|6_?}pAkkDsN}OYyRD zu3UoeFoVnR9Xh=TKSQT8@H0uiB>Ad%fm~@7&l3-Zq0?Dbaf-~r%hKs=tN1y&8sDMg znO1SKT!o)W^7(gho3DzO$Xu&9L#}~g=wO~ztd}OdES+C#6)%+e_zpApqE(zOU%<~K z`9jBAeiK)!4huaz}040FBB zD$bKT@Un89+-?=Gkvs7n=Kd9{_(i!DKa=DOYoqz9xJ=e!4Rie}?}oYGgP&zi_u*xk z`@Q%MbNw2=!y2r|&#+eO@H0uiB>Aei%06W_%;D=W40C$eDlV5t@UpDMH>~0c*@*A3 zCJ$M~8{|R!Op?!Y7@Mz(x5>9)7`a6r zX7t5ZjXqc%$=G+poxFd-7|7jT83XuHjs93=9*d1NKEd5w8OLLFW!nj+~5D<|){iahl{T<5aAUWUK7-W}7pvoEVPnR*Qg0dOV0Gkd ztTHEIW5(y?a?6;4)se|qWuAwP8Pm9*X&>ifbz~}5nHOMV#)aJ1lu?h>k?B}v&cMcu zi@CEYtFSt9C03cUurXtf++Z2A zu{z>mm3cKbX3Ul4meGjSkp`?XzkrPy*T`zin1|JoFJhH>EjDIc$6ZAGn2*(wCaf|S zU}MH2?k38(9;+h@vC3SGjTuY1pD1GqR!5q#%3Owx87sJ7C}TNRM{d9>b0s!r+{7J2 z88>2e07%~&1rvC6y!8#BHl-?WTdu{!c)tTJ!I#*91UA1vc`td6X~ zD)UZk%vdW=SjOF09k~mu%yrn9@m1Mo8TVjyWIa}y_hMtl{nBC?U&HFieOP5~z$!!J z{MpToi{{O48Z&2p`j6d2kM44z>r4MXLcKof|Bva6cdkI^3UsbO=L&SLK<5f{u0ZDs zbgn?>3UsbO=L&SLK<5hl|5*VZ0r>XiTjS&Te#G^!(PPiO>pJ2&;yV&J5;;6&^G>p17WPp7@>wobgASO zUBBoCMK>zC#iA>^y2$ySE{`ARr2V4!teWF!ZO`$2#}6Doa{Qv>i_S(H^0XZ)Qv^Mq z^>sY$((`=Z^8?S1Jiq86Nj5}cdIi28`F_#&*$F4`nU6;(@PojQ0>2peY>yN9OwxlF z`9b7Ikzb5__RJ~z%-kcn=m$kVD*DBuFM2MR9=i>RJ!HGs8Qn$4(Y5d#RScXca-yQc z=CBXCK%T2*0$m> Hierarchy; + public: /** * @note The following tests require a basic understanding @@ -63,6 +69,49 @@ public: * (Valve Developer Community). */ + // Given a model, verify that the bones nodes hierarchy is correctly formed. + void checkBoneHierarchy() { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MDL_HL1_MODELS_DIR "multiple_roots.mdl", aiProcess_ValidateDataStructure); + ASSERT_NE(nullptr, scene); + ASSERT_NE(nullptr, scene->mRootNode); + + const aiNode* node_MDL_root = scene->mRootNode->FindNode(AI_MDL_HL1_NODE_ROOT); + ASSERT_NE(nullptr, node_MDL_root); + + const aiNode *node_MDL_bones = scene->mRootNode->FindNode(AI_MDL_HL1_NODE_BONES); + ASSERT_NE(nullptr, node_MDL_bones); + ASSERT_NE(nullptr, node_MDL_bones->mParent); + ASSERT_EQ(node_MDL_root, node_MDL_bones->mParent); + + const Hierarchy expected_hierarchy = { + { 0, AI_MDL_HL1_NODE_BONES }, + { 1, "root1_bone1" }, + { 2, "root1_bone2" }, + { 3, "root1_bone4" }, + { 3, "root1_bone5" }, + { 2, "root1_bone3" }, + { 3, "root1_bone6" }, + { 1, "root2_bone1" }, + { 2, "root2_bone2" }, + { 2, "root2_bone3" }, + { 3, "root2_bone5" }, + { 2, "root2_bone4" }, + { 3, "root2_bone6" }, + { 1, "root3_bone1" }, + { 2, "root3_bone2" }, + { 2, "root3_bone3" }, + { 2, "root3_bone4" }, + { 3, "root3_bone5" }, + { 4, "root3_bone6" }, + { 4, "root3_bone7" }, + }; + + Hierarchy actual_hierarchy; + flatten_hierarchy(node_MDL_bones, actual_hierarchy); + ASSERT_EQ(expected_hierarchy, actual_hierarchy); + } + /* Given a model with bones that have empty names, verify that all the bones of the imported model have unique and no empty names. @@ -416,8 +465,26 @@ private: EXPECT_NEAR(expected[i][j], actual[i][j], abs_error); } } + + void flatten_hierarchy(const aiNode *node, Hierarchy &hierarchy) + { + flatten_hierarchy(node, hierarchy, 0); + } + + void flatten_hierarchy(const aiNode *node, Hierarchy &hierarchy, unsigned int level) + { + hierarchy.push_back({ level, node->mName.C_Str() }); + for (size_t i = 0; i < node->mNumChildren; ++i) + { + flatten_hierarchy(node->mChildren[i], hierarchy, level + 1); + } + } }; +TEST_F(utMDLImporter_HL1_Nodes, checkBoneHierarchy) { + checkBoneHierarchy(); +} + TEST_F(utMDLImporter_HL1_Nodes, emptyBonesNames) { emptyBonesNames(); } From d500f604903d982c321851d3f108098ab8ff2608 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Lortie Date: Sat, 11 Mar 2023 20:48:17 -0500 Subject: [PATCH 3/8] Adjust emptyBonesNames test. --- .../ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp b/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp index d2984477b..c94fa3f3f 100644 --- a/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp +++ b/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp @@ -143,7 +143,9 @@ public: "Bone_7" }; - expect_named_children(scene, AI_MDL_HL1_NODE_BONES, expected_bones_names); + std::vector actual_bones_names; + get_node_children_names(scene->mRootNode->FindNode(AI_MDL_HL1_NODE_BONES), actual_bones_names); + ASSERT_EQ(expected_bones_names, actual_bones_names); } /* Given a model with bodyparts that have empty names, @@ -479,6 +481,15 @@ private: flatten_hierarchy(node->mChildren[i], hierarchy, level + 1); } } + + void get_node_children_names(const aiNode *node, std::vector &names) + { + for (size_t i = 0; i < node->mNumChildren; ++i) + { + names.push_back(node->mChildren[i]->mName.C_Str()); + get_node_children_names(node->mChildren[i], names); + } + } }; TEST_F(utMDLImporter_HL1_Nodes, checkBoneHierarchy) { From 7bc4c12956a60e563df7cd08b8677f7b8ef8c355 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Lortie Date: Sat, 11 Mar 2023 22:03:29 -0500 Subject: [PATCH 4/8] Simplified HL1 MDL nodes tests. --- .../MDL/utMDLImporter_HL1_Nodes.cpp | 98 ++++++++++++------- 1 file changed, 63 insertions(+), 35 deletions(-) diff --git a/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp b/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp index c94fa3f3f..af6512c87 100644 --- a/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp +++ b/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp @@ -61,6 +61,11 @@ class utMDLImporter_HL1_Nodes : public ::testing::Test { */ typedef std::vector> Hierarchy; + /** + * @note A vector of strings. Used for symplifying syntax. + */ + typedef std::vector StringVector; + public: /** * @note The following tests require a basic understanding @@ -131,7 +136,7 @@ public: const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MDL_HL1_MODELS_DIR "unnamed_bones.mdl", aiProcess_ValidateDataStructure); EXPECT_NE(nullptr, scene); - const std::vector expected_bones_names = { + const StringVector expected_bones_names = { "Bone", "Bone_0", "Bone_1", @@ -143,7 +148,7 @@ public: "Bone_7" }; - std::vector actual_bones_names; + StringVector actual_bones_names; get_node_children_names(scene->mRootNode->FindNode(AI_MDL_HL1_NODE_BONES), actual_bones_names); ASSERT_EQ(expected_bones_names, actual_bones_names); } @@ -167,7 +172,7 @@ public: const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MDL_HL1_MODELS_DIR "unnamed_bodyparts.mdl", aiProcess_ValidateDataStructure); EXPECT_NE(nullptr, scene); - const std::vector expected_bodyparts_names = { + const StringVector expected_bodyparts_names = { "Bodypart", "Bodypart_1", "Bodypart_5", @@ -179,7 +184,10 @@ public: "Bodypart_7" }; - expect_named_children(scene, AI_MDL_HL1_NODE_BODYPARTS, expected_bodyparts_names); + StringVector actual_bodyparts_names; + // Get the bodyparts names "without" the submodels. + get_node_children_names(scene->mRootNode->FindNode(AI_MDL_HL1_NODE_BODYPARTS), actual_bodyparts_names, 0); + ASSERT_EQ(expected_bodyparts_names, actual_bodyparts_names); } /* Given a model with bodyparts that have duplicate names, @@ -201,7 +209,7 @@ public: const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MDL_HL1_MODELS_DIR "duplicate_bodyparts.mdl", aiProcess_ValidateDataStructure); EXPECT_NE(nullptr, scene); - const std::vector expected_bodyparts_names = { + const StringVector expected_bodyparts_names = { "Bodypart", "Bodypart_1", "Bodypart_2", @@ -213,7 +221,10 @@ public: "Bodypart_4" }; - expect_named_children(scene, AI_MDL_HL1_NODE_BODYPARTS, expected_bodyparts_names); + StringVector actual_bodyparts_names; + // Get the bodyparts names "without" the submodels. + get_node_children_names(scene->mRootNode->FindNode(AI_MDL_HL1_NODE_BODYPARTS), actual_bodyparts_names, 0); + ASSERT_EQ(expected_bodyparts_names, actual_bodyparts_names); } /* Given a model with several bodyparts that contains multiple @@ -243,7 +254,7 @@ public: const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MDL_HL1_MODELS_DIR "duplicate_submodels.mdl", aiProcess_ValidateDataStructure); EXPECT_NE(nullptr, scene); - const std::vector> expected_bodypart_sub_models_names = { + const std::vector expected_bodypart_sub_models_names = { { "triangle", "triangle_0", @@ -261,9 +272,13 @@ public: const aiNode *bodyparts_node = scene->mRootNode->FindNode(AI_MDL_HL1_NODE_BODYPARTS); EXPECT_NE(nullptr, bodyparts_node); EXPECT_EQ(3u, bodyparts_node->mNumChildren); - for (unsigned int i = 0; i < bodyparts_node->mNumChildren; ++i) { - expect_named_children(bodyparts_node->mChildren[i], - expected_bodypart_sub_models_names[i]); + + StringVector actual_submodels_names; + for (unsigned int i = 0; i < bodyparts_node->mNumChildren; ++i) + { + actual_submodels_names.clear(); + get_node_children_names(bodyparts_node->mChildren[i], actual_submodels_names); + ASSERT_EQ(expected_bodypart_sub_models_names[i], actual_submodels_names); } } @@ -286,7 +301,7 @@ public: const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MDL_HL1_MODELS_DIR "duplicate_sequences.mdl", aiProcess_ValidateDataStructure); EXPECT_NE(nullptr, scene); - const std::vector expected_sequence_names = { + const StringVector expected_sequence_names = { "idle_1", "idle", "idle_2", @@ -298,7 +313,9 @@ public: "idle_7" }; - expect_named_children(scene, AI_MDL_HL1_NODE_SEQUENCE_INFOS, expected_sequence_names); + StringVector actual_sequence_names; + get_node_children_names(scene->mRootNode->FindNode(AI_MDL_HL1_NODE_SEQUENCE_INFOS), actual_sequence_names); + ASSERT_EQ(expected_sequence_names, actual_sequence_names); } /* Given a model with sequences that have empty names, verify @@ -320,7 +337,7 @@ public: const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MDL_HL1_MODELS_DIR "unnamed_sequences.mdl", aiProcess_ValidateDataStructure); EXPECT_NE(nullptr, scene); - const std::vector expected_sequence_names = { + const StringVector expected_sequence_names = { "Sequence", "Sequence_1", "Sequence_0", @@ -332,7 +349,9 @@ public: "Sequence_6" }; - expect_named_children(scene, AI_MDL_HL1_NODE_SEQUENCE_INFOS, expected_sequence_names); + StringVector actual_sequence_names; + get_node_children_names(scene->mRootNode->FindNode(AI_MDL_HL1_NODE_SEQUENCE_INFOS), actual_sequence_names); + ASSERT_EQ(expected_sequence_names, actual_sequence_names); } /* Given a model with sequence groups that have duplicate names, @@ -355,7 +374,7 @@ public: const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MDL_HL1_MODELS_DIR "duplicate_sequence_groups/duplicate_sequence_groups.mdl", aiProcess_ValidateDataStructure); EXPECT_NE(nullptr, scene); - const std::vector expected_sequence_names = { + const StringVector expected_sequence_names = { "default", "SequenceGroup", "SequenceGroup_1", @@ -368,7 +387,9 @@ public: "SequenceGroup_2" }; - expect_named_children(scene, AI_MDL_HL1_NODE_SEQUENCE_GROUPS, expected_sequence_names); + StringVector actual_sequence_names; + get_node_children_names(scene->mRootNode->FindNode(AI_MDL_HL1_NODE_SEQUENCE_GROUPS), actual_sequence_names); + ASSERT_EQ(expected_sequence_names, actual_sequence_names); } /* Given a model with sequence groups that have empty names, @@ -391,7 +412,7 @@ public: const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MDL_HL1_MODELS_DIR "unnamed_sequence_groups/unnamed_sequence_groups.mdl", aiProcess_ValidateDataStructure); EXPECT_NE(nullptr, scene); - const std::vector expected_sequence_names = { + const StringVector expected_sequence_names = { "default", "SequenceGroup", "SequenceGroup_2", @@ -404,7 +425,9 @@ public: "SequenceGroup_4" }; - expect_named_children(scene, AI_MDL_HL1_NODE_SEQUENCE_GROUPS, expected_sequence_names); + StringVector actual_sequence_names; + get_node_children_names(scene->mRootNode->FindNode(AI_MDL_HL1_NODE_SEQUENCE_GROUPS), actual_sequence_names); + ASSERT_EQ(expected_sequence_names, actual_sequence_names); } /* Verify that mOffsetMatrix applies the correct @@ -449,18 +472,6 @@ public: } private: - void expect_named_children(const aiNode *parent_node, const std::vector &expected_names) { - EXPECT_NE(nullptr, parent_node); - EXPECT_EQ(expected_names.size(), parent_node->mNumChildren); - - for (unsigned int i = 0; i < parent_node->mNumChildren; ++i) - EXPECT_EQ(expected_names[i], parent_node->mChildren[i]->mName.C_Str()); - } - - void expect_named_children(const aiScene *scene, const char *node_name, const std::vector &expected_names) { - expect_named_children(scene->mRootNode->FindNode(node_name), expected_names); - } - void expect_equal_matrices(const aiMatrix4x4 &expected, const aiMatrix4x4 &actual, float abs_error) { for (int i = 0; i < 4; ++i) { for (int j = 0; j < 4; ++j) @@ -468,26 +479,43 @@ private: } } + /** Get a flattened representation of a node's hierarchy. + * \param[in] node The node. + * \param[out] hierarchy The flattened node's hierarchy. + */ void flatten_hierarchy(const aiNode *node, Hierarchy &hierarchy) { - flatten_hierarchy(node, hierarchy, 0); + flatten_hierarchy_impl(node, hierarchy, 0); } - void flatten_hierarchy(const aiNode *node, Hierarchy &hierarchy, unsigned int level) + void flatten_hierarchy_impl(const aiNode *node, Hierarchy &hierarchy, unsigned int level) { hierarchy.push_back({ level, node->mName.C_Str() }); for (size_t i = 0; i < node->mNumChildren; ++i) { - flatten_hierarchy(node->mChildren[i], hierarchy, level + 1); + flatten_hierarchy_impl(node->mChildren[i], hierarchy, level + 1); } } - void get_node_children_names(const aiNode *node, std::vector &names) + /** Get all node's children names beneath max_level. + * \param[in] node The parent node from which to get all children names. + * \param[out] names The list of children names. + * \param[in] max_level If set to -1, all children names will be collected. + */ + void get_node_children_names(const aiNode *node, StringVector &names, const int max_level = -1) + { + get_node_children_names_impl(node, names, 0, max_level); + } + + void get_node_children_names_impl(const aiNode *node, StringVector &names, int level, const int max_level = -1) { for (size_t i = 0; i < node->mNumChildren; ++i) { names.push_back(node->mChildren[i]->mName.C_Str()); - get_node_children_names(node->mChildren[i], names); + if (max_level == -1 || level < max_level) + { + get_node_children_names_impl(node->mChildren[i], names, level + 1, max_level); + } } } }; From 054dacd0687cb2c2bca6fe14b61b8063ec7ba6ac Mon Sep 17 00:00:00 2001 From: Marc-Antoine Lortie Date: Sat, 11 Mar 2023 22:32:48 -0500 Subject: [PATCH 5/8] Improved comments. --- code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp | 5 +++-- test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp b/code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp index 237ccf3d8..f3a383e8e 100644 --- a/code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp +++ b/code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp @@ -470,6 +470,7 @@ void HL1MDLLoader::read_bones() { temp_bones_.resize(header_->numbones); + // Create the main 'bones' node that will contain all MDL root bones. aiNode *bones_node = new aiNode(AI_MDL_HL1_NODE_BONES); rootnode_children_.push_back(bones_node); @@ -500,11 +501,11 @@ void HL1MDLLoader::read_bones() { temp_bones_[i].offset_matrix.Inverse(); } - // Create the 'bones' root node that will contain all bone nodes. + // Allocate memory for each MDL root bone. bones_node->mNumChildren = static_cast(roots.size()); bones_node->mChildren = new aiNode *[bones_node->mNumChildren]; - // Build all bones children hierarchy starting from each root bone. + // Build all bones children hierarchy starting from each MDL root bone. for (size_t i = 0; i < roots.size(); ++i) { const TempBone &root_bone = temp_bones_[roots[i]]; diff --git a/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp b/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp index af6512c87..5a258d0a4 100644 --- a/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp +++ b/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp @@ -81,6 +81,7 @@ public: ASSERT_NE(nullptr, scene); ASSERT_NE(nullptr, scene->mRootNode); + // First, check that "" and "" are linked. const aiNode* node_MDL_root = scene->mRootNode->FindNode(AI_MDL_HL1_NODE_ROOT); ASSERT_NE(nullptr, node_MDL_root); @@ -89,6 +90,7 @@ public: ASSERT_NE(nullptr, node_MDL_bones->mParent); ASSERT_EQ(node_MDL_root, node_MDL_bones->mParent); + // Second, verify "" hierarchy. const Hierarchy expected_hierarchy = { { 0, AI_MDL_HL1_NODE_BONES }, { 1, "root1_bone1" }, From 4c015077b840ddf87d42e70300f51161cd77c415 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Lortie Date: Sun, 12 Mar 2023 13:26:50 -0400 Subject: [PATCH 6/8] Add missing member initializer. --- code/AssetLib/MDL/HalfLife/HL1MDLLoader.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/code/AssetLib/MDL/HalfLife/HL1MDLLoader.h b/code/AssetLib/MDL/HalfLife/HL1MDLLoader.h index 3fc84c1be..286b6e64c 100644 --- a/code/AssetLib/MDL/HalfLife/HL1MDLLoader.h +++ b/code/AssetLib/MDL/HalfLife/HL1MDLLoader.h @@ -206,7 +206,8 @@ private: TempBone() : node(nullptr), absolute_transform(), - offset_matrix() {} + offset_matrix(), + children() {} aiNode *node; aiMatrix4x4 absolute_transform; From 25ab05eb49c901b54b8f8695dbcc05ab1376ac09 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Lortie Date: Tue, 14 Mar 2023 09:17:39 -0400 Subject: [PATCH 7/8] Replace typedef by using. --- test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp b/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp index 5a258d0a4..712f4da11 100644 --- a/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp +++ b/test/unit/ImportExport/MDL/utMDLImporter_HL1_Nodes.cpp @@ -59,12 +59,12 @@ class utMDLImporter_HL1_Nodes : public ::testing::Test { * @note Represents a flattened node hierarchy where each item is a pair * containing the node level and it's name. */ - typedef std::vector> Hierarchy; + using Hierarchy = std::vector>; /** * @note A vector of strings. Used for symplifying syntax. */ - typedef std::vector StringVector; + using StringVector = std::vector; public: /** From eb3b48e5239fc0dfab9d97ec2224aa65e1712dad Mon Sep 17 00:00:00 2001 From: Marc-Antoine Lortie Date: Tue, 14 Mar 2023 09:21:45 -0400 Subject: [PATCH 8/8] Invert logic in build_bone_children_hierarchy. --- code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp | 24 ++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp b/code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp index f3a383e8e..a8141fcc1 100644 --- a/code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp +++ b/code/AssetLib/MDL/HalfLife/HL1MDLLoader.cpp @@ -516,19 +516,19 @@ void HL1MDLLoader::read_bones() { void HL1MDLLoader::build_bone_children_hierarchy(const TempBone &bone) { - if (bone.children.size() > 0) - { - aiNode* bone_node = bone.node; - bone_node->mNumChildren = static_cast(bone.children.size()); - bone_node->mChildren = new aiNode *[bone_node->mNumChildren]; + if (bone.children.empty()) + return; - // Build each child bone's hierarchy recursively. - for (size_t i = 0; i < bone.children.size(); ++i) - { - const TempBone &child_bone = temp_bones_[bone.children[i]]; - bone_node->mChildren[i] = child_bone.node; - build_bone_children_hierarchy(child_bone); - } + aiNode* bone_node = bone.node; + bone_node->mNumChildren = static_cast(bone.children.size()); + bone_node->mChildren = new aiNode *[bone_node->mNumChildren]; + + // Build each child bone's hierarchy recursively. + for (size_t i = 0; i < bone.children.size(); ++i) + { + const TempBone &child_bone = temp_bones_[bone.children[i]]; + bone_node->mChildren[i] = child_bone.node; + build_bone_children_hierarchy(child_bone); } }