Merge pull request godotengine#44799 from RevoluPowered/fbx-fix-zero-…

…scaling mesh indexing failing with small scale values
ivalylo · Jan 26, 2021 · 1f5669d · 1f5669d
2 parents e330b94 + 86c7faa
commit 1f5669d
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 5 deletions.
diff --git a/editor/plugins/node_3d_editor_plugin.cpp b/editor/plugins/node_3d_editor_plugin.cpp
@@ -3321,6 +3321,21 @@ void Node3DEditorViewport::update_transform_gizmo_view() {
 
 	xform.basis.scale(scale);
 
+	// if the determinant is zero, we should disable the gizmo from being rendered
+	// this prevents supplying bad values to the renderer and then having to filter it out again
+	if (xform.basis.determinant() == 0) {
+		for (int i = 0; i < 3; i++) {
+			RenderingServer::get_singleton()->instance_set_visible(move_gizmo_instance[i], false);
+			RenderingServer::get_singleton()->instance_set_visible(move_plane_gizmo_instance[i], false);
+			RenderingServer::get_singleton()->instance_set_visible(rotate_gizmo_instance[i], false);
+			RenderingServer::get_singleton()->instance_set_visible(scale_gizmo_instance[i], false);
+			RenderingServer::get_singleton()->instance_set_visible(scale_plane_gizmo_instance[i], false);
+		}
+		// Rotation white outline
+		RenderingServer::get_singleton()->instance_set_visible(rotate_gizmo_instance[3], false);
+		return;
+	}
+
 	for (int i = 0; i < 3; i++) {
 		RenderingServer::get_singleton()->instance_set_transform(move_gizmo_instance[i], xform);
 		RenderingServer::get_singleton()->instance_set_visible(move_gizmo_instance[i], spatial_editor->is_gizmo_visible() && (spatial_editor->get_tool_mode() == Node3DEditor::TOOL_MODE_SELECT || spatial_editor->get_tool_mode() == Node3DEditor::TOOL_MODE_MOVE));

diff --git a/modules/fbx/data/fbx_material.h b/modules/fbx/data/fbx_material.h
@@ -217,9 +217,6 @@ struct FBXMaterial : public Reference {
 		{ "TransparencyFactor", PROPERTY_DESC_TRANSPARENT },
 		{ "Maya|opacity", PROPERTY_DESC_TRANSPARENT },
 
-		/* Metallic */
-		{ "Shininess", PROPERTY_DESC_METALLIC },
-		{ "Reflectivity", PROPERTY_DESC_METALLIC },
 		{ "Maya|metalness", PROPERTY_DESC_METALLIC },
 		{ "Maya|metallic", PROPERTY_DESC_METALLIC },
 
@@ -241,6 +238,8 @@ struct FBXMaterial : public Reference {
 		{ "Maya|emissionColor", PROPERTY_DESC_EMISSIVE_COLOR },
 
 		/* Ignore */
+		{ "Shininess", PROPERTY_DESC_IGNORE },
+		{ "Reflectivity", PROPERTY_DESC_IGNORE },
 		{ "Maya|diffuseRoughness", PROPERTY_DESC_IGNORE },
 		{ "Maya", PROPERTY_DESC_IGNORE },
 		{ "Diffuse", PROPERTY_DESC_ALBEDO_COLOR },

diff --git a/modules/fbx/data/pivot_transform.cpp b/modules/fbx/data/pivot_transform.cpp
@@ -76,12 +76,14 @@ void PivotTransform::ReadTransformChain() {
 	const Vector3 &Scaling = ImportUtils::safe_import_vector3(FBXDocParser::PropertyGet<Vector3>(props, "Lcl Scaling", ok));
 	if (ok) {
 		scaling = Scaling;
+	} else {
+		scaling = Vector3(1, 1, 1);
 	}
 	const Vector3 &GeometricScaling = ImportUtils::safe_import_vector3(FBXDocParser::PropertyGet<Vector3>(props, "GeometricScaling", ok));
 	if (ok) {
 		geometric_scaling = GeometricScaling;
 	} else {
-		geometric_scaling = Vector3(0, 0, 0);
+		geometric_scaling = Vector3(1, 1, 1);
 	}
 
 	const Vector3 &GeometricRotation = ImportUtils::safe_import_vector3(FBXDocParser::PropertyGet<Vector3>(props, "GeometricRotation", ok));
@@ -207,6 +209,8 @@ Transform PivotTransform::ComputeGlobalTransform(Vector3 p_translation, Quat p_r
 	Transform local_transform = T * Roff * Rp * Rpre * R * Rpost.affine_inverse() * Rp.affine_inverse() * Soff * Sp * S * Sp.affine_inverse();
 	//Transform local_translation_pivoted = Transform(Basis(), LocalTransform.origin);
 
+	ERR_FAIL_COND_V_MSG(local_transform.basis.determinant() == 0, Transform(), "Det == 0 prevented in scene file");
+
 	// manual hack to force SSC not to be compensated for - until we can handle it properly with tests
 	return parent_global_xform * local_transform;
 }
@@ -290,5 +294,14 @@ void PivotTransform::Execute() {
 	ComputePivotTransform();
 
 	ImportUtils::debug_xform("global xform: ", GlobalTransform);
+
+	if (LocalTransform.basis.determinant() == 0) {
+		print_error("Serious det == 0!");
+	}
+
+	if (GlobalTransform.basis.determinant() == 0) {
+		print_error("Serious! node has det == 0!");
+	}
+
 	computed_global_xform = true;
 }
diff --git a/servers/rendering/renderer_scene_cull.cpp b/servers/rendering/renderer_scene_cull.cpp
@@ -1253,7 +1253,8 @@ void RendererSceneCull::_update_instance(Instance *p_instance) {
 		scene_render->geometry_instance_set_transform(geom->geometry_instance, p_instance->transform, p_instance->aabb, p_instance->transformed_aabb);
 	}
 
-	if (p_instance->scenario == nullptr || !p_instance->visible || Math::is_zero_approx(p_instance->transform.basis.determinant())) {
+	// note: we had to remove is equal approx check here, it meant that det == 0.000004 won't work, which is the case for some of our scenes.
+	if (p_instance->scenario == nullptr || !p_instance->visible || p_instance->transform.basis.determinant() == 0) {
 		p_instance->prev_transformed_aabb = p_instance->transformed_aabb;
 		return;
 	}