From c60ed2587ddfa1aa8835658d3f75a954c2cac4ec Mon Sep 17 00:00:00 2001 From: Aaron Franke Date: Thu, 12 Oct 2023 17:37:03 -0500 Subject: [PATCH] Fix desynced duplicate GLTFNode transform properties --- modules/gltf/gltf_document.cpp | 84 ++++++++++++--------------- modules/gltf/structures/gltf_node.cpp | 16 ++--- modules/gltf/structures/gltf_node.h | 6 +- 3 files changed, 46 insertions(+), 60 deletions(-) diff --git a/modules/gltf/gltf_document.cpp b/modules/gltf/gltf_document.cpp index 2acca5b856d..778b02d849b 100644 --- a/modules/gltf/gltf_document.cpp +++ b/modules/gltf/gltf_document.cpp @@ -430,20 +430,22 @@ Error GLTFDocument::_serialize_nodes(Ref p_state) { } if (gltf_node->skeleton != -1 && gltf_node->skin < 0) { } - if (gltf_node->xform != Transform3D()) { - node["matrix"] = _xform_to_array(gltf_node->xform); - } - - if (!gltf_node->rotation.is_equal_approx(Quaternion())) { - node["rotation"] = _quaternion_to_array(gltf_node->rotation); - } - - if (!gltf_node->scale.is_equal_approx(Vector3(1.0f, 1.0f, 1.0f))) { - node["scale"] = _vec3_to_arr(gltf_node->scale); - } - - if (!gltf_node->position.is_zero_approx()) { - node["translation"] = _vec3_to_arr(gltf_node->position); + if (gltf_node->transform.basis.is_orthogonal()) { + // An orthogonal transform is decomposable into TRS, so prefer that. + const Vector3 position = gltf_node->get_position(); + if (!position.is_zero_approx()) { + node["translation"] = _vec3_to_arr(position); + } + const Quaternion rotation = gltf_node->get_rotation(); + if (!rotation.is_equal_approx(Quaternion())) { + node["rotation"] = _quaternion_to_array(rotation); + } + const Vector3 scale = gltf_node->get_scale(); + if (!scale.is_equal_approx(Vector3(1.0f, 1.0f, 1.0f))) { + node["scale"] = _vec3_to_arr(scale); + } + } else { + node["matrix"] = _xform_to_array(gltf_node->transform); } if (gltf_node->children.size()) { Array children; @@ -609,20 +611,17 @@ Error GLTFDocument::_parse_nodes(Ref p_state) { node->skin = n["skin"]; } if (n.has("matrix")) { - node->xform = _arr_to_xform(n["matrix"]); + node->transform = _arr_to_xform(n["matrix"]); } else { if (n.has("translation")) { - node->position = _arr_to_vec3(n["translation"]); + node->set_position(_arr_to_vec3(n["translation"])); } if (n.has("rotation")) { - node->rotation = _arr_to_quaternion(n["rotation"]); + node->set_rotation(_arr_to_quaternion(n["rotation"])); } if (n.has("scale")) { - node->scale = _arr_to_vec3(n["scale"]); + node->set_scale(_arr_to_vec3(n["scale"])); } - - node->xform.basis.set_quaternion_scale(node->rotation, node->scale); - node->xform.origin = node->position; } if (n.has("extensions")) { @@ -4802,10 +4801,10 @@ Error GLTFDocument::_create_skeletons(Ref p_state) { node->set_name(_gen_unique_bone_name(p_state, skel_i, node->get_name())); skeleton->add_bone(node->get_name()); - skeleton->set_bone_rest(bone_index, node->xform); - skeleton->set_bone_pose_position(bone_index, node->position); - skeleton->set_bone_pose_rotation(bone_index, node->rotation.normalized()); - skeleton->set_bone_pose_scale(bone_index, node->scale); + skeleton->set_bone_rest(bone_index, node->transform); + skeleton->set_bone_pose_position(bone_index, node->get_position()); + skeleton->set_bone_pose_rotation(bone_index, node->get_rotation()); + skeleton->set_bone_pose_scale(bone_index, node->get_scale()); if (node->parent >= 0 && p_state->nodes[node->parent]->skeleton == skel_i) { const int bone_parent = skeleton->find_bone(p_state->nodes[node->parent]->get_name()); @@ -5511,10 +5510,7 @@ GLTFLightIndex GLTFDocument::_convert_light(Ref p_state, Light3D *p_l } void GLTFDocument::_convert_spatial(Ref p_state, Node3D *p_spatial, Ref p_node) { - Transform3D xform = p_spatial->get_transform(); - p_node->scale = xform.basis.get_scale(); - p_node->rotation = xform.basis.get_rotation_quaternion(); - p_node->position = xform.origin; + p_node->transform = p_spatial->get_transform(); } Node3D *GLTFDocument::_generate_spatial(Ref p_state, const GLTFNodeIndex p_node_index) { @@ -5628,7 +5624,7 @@ void GLTFDocument::_convert_csg_shape_to_gltf(CSGShape3D *p_current, GLTFNodeInd GLTFMeshIndex mesh_i = p_state->meshes.size(); p_state->meshes.push_back(gltf_mesh); p_gltf_node->mesh = mesh_i; - p_gltf_node->xform = csg->get_meshes()[0]; + p_gltf_node->transform = csg->get_meshes()[0]; p_gltf_node->set_name(_gen_unique_name(p_state, csg->get_name())); } #endif // MODULE_CSG_ENABLED @@ -5704,7 +5700,7 @@ void GLTFDocument::_convert_grid_map_to_gltf(GridMap *p_grid_map, GLTFNodeIndex gltf_mesh->set_mesh(_mesh_to_importer_mesh(p_grid_map->get_mesh_library()->get_item_mesh(cell))); new_gltf_node->mesh = p_state->meshes.size(); p_state->meshes.push_back(gltf_mesh); - new_gltf_node->xform = cell_xform * p_grid_map->get_transform(); + new_gltf_node->transform = cell_xform * p_grid_map->get_transform(); new_gltf_node->set_name(_gen_unique_name(p_state, p_grid_map->get_mesh_library()->get_item_name(cell))); } } @@ -5772,7 +5768,7 @@ void GLTFDocument::_convert_multi_mesh_instance_to_gltf( Ref new_gltf_node; new_gltf_node.instantiate(); new_gltf_node->mesh = mesh_index; - new_gltf_node->xform = transform; + new_gltf_node->transform = transform; new_gltf_node->set_name(_gen_unique_name(p_state, p_multi_mesh_instance->get_name())); p_gltf_node->children.push_back(p_state->nodes.size()); p_state->nodes.push_back(new_gltf_node); @@ -5797,10 +5793,7 @@ void GLTFDocument::_convert_skeleton_to_gltf(Skeleton3D *p_skeleton3d, Refset_name(_gen_unique_name(p_state, skeleton->get_bone_name(bone_i))); - Transform3D xform = skeleton->get_bone_pose(bone_i); - joint_node->scale = xform.basis.get_scale(); - joint_node->rotation = xform.basis.get_rotation_quaternion(); - joint_node->position = xform.origin; + joint_node->transform = skeleton->get_bone_pose(bone_i); joint_node->joint = true; GLTFNodeIndex current_node_i = p_state->nodes.size(); p_state->scene_nodes.insert(current_node_i, skeleton); @@ -5949,7 +5942,7 @@ void GLTFDocument::_generate_scene_node(Ref p_state, const GLTFNodeIn Array args; args.append(p_scene_root); current_node->propagate_call(StringName("set_owner"), args); - current_node->set_transform(gltf_node->xform); + current_node->set_transform(gltf_node->transform); } p_state->scene_nodes.insert(p_node_index, current_node); @@ -6276,7 +6269,7 @@ void GLTFDocument::_import_animation(Ref p_state, AnimationPlayer *p_ if (track.position_track.values.size()) { bool is_default = true; //discard the track if all it contains is default values if (p_remove_immutable_tracks) { - Vector3 base_pos = p_state->nodes[track_i.key]->position; + Vector3 base_pos = gltf_node->get_position(); for (int i = 0; i < track.position_track.times.size(); i++) { int value_index = track.position_track.interpolation == GLTFAnimation::INTERP_CUBIC_SPLINE ? (1 + i * 3) : i; ERR_FAIL_COND_MSG(value_index >= track.position_track.values.size(), "Animation sampler output accessor with 'CUBICSPLINE' interpolation doesn't have enough elements."); @@ -6301,7 +6294,7 @@ void GLTFDocument::_import_animation(Ref p_state, AnimationPlayer *p_ if (track.rotation_track.values.size()) { bool is_default = true; //discard the track if all it contains is default values if (p_remove_immutable_tracks) { - Quaternion base_rot = p_state->nodes[track_i.key]->rotation.normalized(); + Quaternion base_rot = gltf_node->get_rotation(); for (int i = 0; i < track.rotation_track.times.size(); i++) { int value_index = track.rotation_track.interpolation == GLTFAnimation::INTERP_CUBIC_SPLINE ? (1 + i * 3) : i; ERR_FAIL_COND_MSG(value_index >= track.rotation_track.values.size(), "Animation sampler output accessor with 'CUBICSPLINE' interpolation doesn't have enough elements."); @@ -6326,7 +6319,7 @@ void GLTFDocument::_import_animation(Ref p_state, AnimationPlayer *p_ if (track.scale_track.values.size()) { bool is_default = true; //discard the track if all it contains is default values if (p_remove_immutable_tracks) { - Vector3 base_scale = p_state->nodes[track_i.key]->scale; + Vector3 base_scale = gltf_node->get_scale(); for (int i = 0; i < track.scale_track.times.size(); i++) { int value_index = track.scale_track.interpolation == GLTFAnimation::INTERP_CUBIC_SPLINE ? (1 + i * 3) : i; ERR_FAIL_COND_MSG(value_index >= track.scale_track.values.size(), "Animation sampler output accessor with 'CUBICSPLINE' interpolation doesn't have enough elements."); @@ -6357,15 +6350,15 @@ void GLTFDocument::_import_animation(Ref p_state, AnimationPlayer *p_ Vector3 base_scale = Vector3(1, 1, 1); if (rotation_idx == -1) { - base_rot = p_state->nodes[track_i.key]->rotation.normalized(); + base_rot = gltf_node->get_rotation(); } if (position_idx == -1) { - base_pos = p_state->nodes[track_i.key]->position; + base_pos = gltf_node->get_position(); } if (scale_idx == -1) { - base_scale = p_state->nodes[track_i.key]->scale; + base_scale = gltf_node->get_scale(); } bool last = false; @@ -6472,10 +6465,7 @@ void GLTFDocument::_convert_mesh_instances(Ref p_state) { if (!mi) { continue; } - Transform3D mi_xform = mi->get_transform(); - node->scale = mi_xform.basis.get_scale(); - node->rotation = mi_xform.basis.get_rotation_quaternion(); - node->position = mi_xform.origin; + node->transform = mi->get_transform(); Node *skel_node = mi->get_node_or_null(mi->get_skeleton_path()); Skeleton3D *godot_skeleton = Object::cast_to(skel_node); diff --git a/modules/gltf/structures/gltf_node.cpp b/modules/gltf/structures/gltf_node.cpp index 30895034a93..03f71525fd9 100644 --- a/modules/gltf/structures/gltf_node.cpp +++ b/modules/gltf/structures/gltf_node.cpp @@ -89,11 +89,11 @@ void GLTFNode::set_height(int p_height) { } Transform3D GLTFNode::get_xform() { - return xform; + return transform; } void GLTFNode::set_xform(Transform3D p_xform) { - xform = p_xform; + transform = p_xform; } GLTFMeshIndex GLTFNode::get_mesh() { @@ -129,27 +129,27 @@ void GLTFNode::set_skeleton(GLTFSkeletonIndex p_skeleton) { } Vector3 GLTFNode::get_position() { - return position; + return transform.origin; } void GLTFNode::set_position(Vector3 p_position) { - position = p_position; + transform.origin = p_position; } Quaternion GLTFNode::get_rotation() { - return rotation; + return transform.basis.get_rotation_quaternion(); } void GLTFNode::set_rotation(Quaternion p_rotation) { - rotation = p_rotation; + transform.basis.set_quaternion_scale(p_rotation, transform.basis.get_scale()); } Vector3 GLTFNode::get_scale() { - return scale; + return transform.basis.get_scale(); } void GLTFNode::set_scale(Vector3 p_scale) { - scale = p_scale; + transform.basis = transform.basis.orthonormalized() * Basis::from_scale(p_scale); } Vector GLTFNode::get_children() { diff --git a/modules/gltf/structures/gltf_node.h b/modules/gltf/structures/gltf_node.h index c2d2f644954..aba73741276 100644 --- a/modules/gltf/structures/gltf_node.h +++ b/modules/gltf/structures/gltf_node.h @@ -40,18 +40,14 @@ class GLTFNode : public Resource { friend class GLTFDocument; private: - // matrices need to be transformed to this GLTFNodeIndex parent = -1; int height = -1; - Transform3D xform; + Transform3D transform; GLTFMeshIndex mesh = -1; GLTFCameraIndex camera = -1; GLTFSkinIndex skin = -1; GLTFSkeletonIndex skeleton = -1; bool joint = false; - Vector3 position; - Quaternion rotation; - Vector3 scale = Vector3(1, 1, 1); Vector children; GLTFLightIndex light = -1; Dictionary additional_data;