Fix UV2 coordinates and various problems with FBX

- Document no longer uses unordered maps
- Removed some usages of &GetRequiredToken replaced with safe *GetRequiredToken() function
- Added parser debugging
- Added ERR_FAIL_CONDS for unsupported mesh formats (we can add these later super easy to do now)
- Add memory debugging for the Tokens and the TokenParser to make it safe
- Add memory initialisation to mesh.cpp surface_tool.h and mesh.h
- Initialize boolean flags properly
- Refactored to correct naming for the fbx_mesh_data.h so you know what data you are working on
- Disabled corruption caused by the FIXME:
- Fixed document reading indexes and index_to_direct vs indexes mode

modules/fbx/data/fbx_material.cpp

@@ -351,9 +351,6 @@ Ref<SpatialMaterial> FBXMaterial::import_material(ImportState &state) {
 		}
 	}
 
-	// FBX culling normally is front only.
-	spatial_material->set_cull_mode(SpatialMaterial::CULL_FRONT);
-
 	// Set the material features.
 	for (int x = 0; x < material_info.features.size(); x++) {
 		if (spatial_material.is_null()) {
@@ -407,8 +404,8 @@ Ref<SpatialMaterial> FBXMaterial::import_material(ImportState &state) {
 				} else if (extension == "TGA") {
 
 					// The stored file is a TGA.
-					image = Image::_tga_mem_loader_func(mapping.texture->Media()->Content(), mapping.texture->Media()->ContentLength());
-					ERR_CONTINUE_MSG(image.is_valid() == false, "FBX Embedded TGA image load fail.");
+					//image = Image::_tga_mem_loader_func(mapping.texture->Media()->Content(), mapping.texture->Media()->ContentLength());
+					//ERR_CONTINUE_MSG(image.is_valid() == false, "FBX Embedded TGA image load fail.");
 
 				} else if (extension == "WEBP") {
 

modules/fbx/data/fbx_mesh_data.h

@@ -303,8 +303,8 @@ struct FBXMeshData : Reference {
 	HashMap<int, R> extract_per_vertex_data(
 			int p_vertex_count,
 			const std::vector<Assimp::FBX::MeshGeometry::Edge> &p_edges,
-			const std::vector<int> &p_face_indices,
-			const Assimp::FBX::MeshGeometry::MappingData<T> &p_fbx_data,
+			const std::vector<int> &p_mesh_indices,
+			const Assimp::FBX::MeshGeometry::MappingData<T> &p_mapping_data,
 			R (*collector_function)(const Vector<VertexData<T> > *p_vertex_data, R p_fall_back),
 			R p_fall_back) const;
 

modules/fbx/data/fbx_skeleton.cpp

@@ -49,7 +49,7 @@ void FBXSkeleton::init_skeleton(const ImportState &state) {
 			} else {
 				// root node must never be a skeleton to prevent cyclic skeletons from being allowed (skeleton in a skeleton)
 				fbx_node->godot_node->add_child(skeleton);
-				skeleton->set_owner(state.root);
+				skeleton->set_owner(state.root_owner);
 				skeleton->set_name("Skeleton");
 				print_verbose("created armature skeleton for root");
 			}

modules/fbx/editor_scene_importer_fbx.cpp

@@ -743,7 +743,7 @@ EditorSceneImporterFBX::_generate_scene(const String &p_path,
 					print_verbose("Creating animation player");
 					state.animation_player = memnew(AnimationPlayer);
 					state.root->add_child(state.animation_player);
-					state.animation_player->set_owner(state.root);
+					state.animation_player->set_owner(state.root_owner);
 				}
 
 				Ref<Animation> animation;
@@ -815,7 +815,7 @@ EditorSceneImporterFBX::_generate_scene(const String &p_path,
 						String target_name = ImportUtils::FBXNodeToName(target->Name());
 
 						const Assimp::FBX::PropertyTable &properties = curve_node->Props();
-						bool got_x, got_y, got_z;
+						bool got_x = false, got_y = false, got_z = false;
 						float offset_x = Assimp::FBX::PropertyGet<float>(properties, "d|X", got_x);
 						float offset_y = Assimp::FBX::PropertyGet<float>(properties, "d|Y", got_y);
 						float offset_z = Assimp::FBX::PropertyGet<float>(properties, "d|Z", got_z);

thirdparty/assimp_fbx/FBXBinaryTokenizer.cpp

@@ -96,20 +96,16 @@ namespace FBX {
 //}
 // ------------------------------------------------------------------------------------------------
 Token::Token(const char *sbegin, const char *send, TokenType type, size_t offset) :
-#ifdef DEBUG
-		contents(sbegin, static_cast<size_t>(send - sbegin)),
-#endif
 		sbegin(sbegin),
 		send(send),
 		type(type),
 		line(offset),
 		column(BINARY_MARKER) {
-	//ai_assert(sbegin);
-	//ai_assert(send);
-
-	// binary tokens may have zero length because they are sometimes dummies
-	// inserted by TokenizeBinary()
-	//ai_assert(send >= sbegin);
+#ifdef DEBUG_ENABLED
+        contents = std::string(sbegin, static_cast<size_t>(send - sbegin));
+#endif
+		// calc length
+		// measure from sBegin to sEnd and validate?
 }
 
 namespace {
@@ -410,8 +406,6 @@ bool ReadScope(TokenList &output_tokens, const char *input, const char *&cursor,
 // ------------------------------------------------------------------------------------------------
 // TODO: Test FBX Binary files newer than the 7500 version to check if the 64 bits address behaviour is consistent
 void TokenizeBinary(TokenList &output_tokens, const char *input, size_t length) {
-	//ai_assert(input);
-	//ASSIMP_LOG_DEBUG("Tokenizing binary FBX file");
 
 	if (length < 0x1b) {
 		//TokenizeError("file is too short",0);

thirdparty/assimp_fbx/FBXMaterial.cpp

@@ -261,7 +261,21 @@ Video::Video(uint64_t id, const Element &element, const Document &doc, const std
 	const Scope &sc = GetRequiredScope(element);
 
 	const Element *const Type = sc["Type"];
-	const Element *const FileName = sc.FindElementCaseInsensitive("FileName"); //some files retain the information as "Filename", others "FileName", who knows
+	// File Version 7500 Crashes if this is not checked fully.
+	// As of writing this comment 7700 exists, in August 2020
+	Element * FileName = nullptr;
+	if(HasElement(sc, "Filename") )
+	{
+		FileName = (Element*) sc["Filename"];
+	}
+	else if(HasElement(sc, "FileName"))
+	{
+		FileName = (Element*) sc["FileName"];
+	}
+	else {
+		print_error("file has invalid video material returning...");
+		return;
+	}
 	const Element *const RelativeFilename = sc["RelativeFilename"];
 	const Element *const Content = sc["Content"];
 

thirdparty/assimp_fbx/FBXMeshGeometry.cpp

@@ -92,14 +92,11 @@ const Skin *Geometry::DeformerSkin() const {
 // ------------------------------------------------------------------------------------------------
 MeshGeometry::MeshGeometry(uint64_t id, const Element &element, const std::string &name, const Document &doc) :
 		Geometry(id, element, name, doc) {
-	const Scope *sc = element.Compound();
-	if (!sc) {
-		DOMError("failed to read Geometry object (class: Mesh), no data scope found");
-	}
+	print_verbose("mesh name: " + String(name.c_str()) );
 
-	if (!HasElement(*sc, "Vertices")) {
-		return; // this happened!
-	}
+	const Scope *sc = element.Compound();
+	ERR_FAIL_COND_MSG(sc == nullptr, "failed to read geometry, prevented crash");
+	ERR_FAIL_COND_MSG(!HasElement(*sc, "Vertices"), "Detected mesh with no vertexes, didn't populate the mesh");
 
 	// must have Mesh elements:
 	const Element &Vertices = GetRequiredElement(*sc, "Vertices", &element);
@@ -110,84 +107,90 @@ MeshGeometry::MeshGeometry(uint64_t id, const Element &element, const std::strin
 		ParseVectorDataArray(m_edges, element_edges);
 	}
 
-	// optional Mesh elements:
-	const ElementCollection &Layer = sc->GetCollection("Layer");
-
 	// read mesh data into arrays
-	// todo: later we can actually store arrays for these :)
-	// and not vector3
 	ParseVectorDataArray(m_vertices, Vertices);
 	ParseVectorDataArray(m_face_indices, PolygonVertexIndex);
 
-	if (m_vertices.empty()) {
-		print_error("encountered mesh with no vertices");
-	}
+	ERR_FAIL_COND_MSG(m_vertices.empty(), "mesh with no vertexes in FBX file, did you mean to delete it?");
+	ERR_FAIL_COND_MSG(m_face_indices.empty(), "mesh has no faces, was this intended?");
 
-	if (m_face_indices.empty()) {
-		print_error("encountered mesh with no faces");
-	}
+	// Retrieve layer elements, for all of the mesh
+	const ElementCollection &Layer = sc->GetCollection("Layer");
+
+	// Store all layers
+	std::vector<std::tuple<int, std::string>> valid_layers;
 
 	// now read the sub mesh information from the geometry (normals, uvs, etc)
 	for (ElementMap::const_iterator it = Layer.first; it != Layer.second; ++it) {
-		//const TokenList &tokens = (*it).second->Tokens();
-		const Scope &layer = GetRequiredScope(*(*it).second);
-		const ElementCollection &LayerElement = layer.GetCollection("LayerElement");
+		const Scope *layer = GetRequiredScope(it->second);
+		const ElementCollection &LayerElement = layer->GetCollection("LayerElement");
 		for (ElementMap::const_iterator eit = LayerElement.first; eit != LayerElement.second; ++eit) {
-			std::string layer_name = (*eit).first;
-			Element *element_layer = (*eit).second;
+			std::string layer_name = eit->first;
+			Element *element_layer = eit->second;
 			const Scope &layer_element = GetRequiredScope(*element_layer);
+
+			// Actual usable 'type' LayerElementUV, LayerElementNormal, etc
 			const Element &Type = GetRequiredElement(layer_element, "Type");
 			const Element &TypedIndex = GetRequiredElement(layer_element, "TypedIndex");
 			const std::string &type = ParseTokenAsString(GetRequiredToken(Type, 0));
 			const int typedIndex = ParseTokenAsInt(GetRequiredToken(TypedIndex, 0));
 
-			// get object / mesh directly from the FBX by the element ID.
-			const Scope &top = GetRequiredScope(element);
-
-			// Get collection of elements from the NormalLayerMap
-			// this must contain our proper elements.
-			const ElementCollection candidates = top.GetCollection(type);
-
-			/* typedef std::vector< Scope* > ScopeList;
-			 * typedef std::fbx_unordered_multimap< std::string, Element* > ElementMap;
-			 * typedef std::pair<ElementMap::const_iterator,ElementMap::const_iterator> ElementCollection;
-			 */
-
-			for (ElementMap::const_iterator cand_iter = candidates.first; cand_iter != candidates.second; ++cand_iter) {
-				std::string val = (*cand_iter).first;
-				//Element *element = (*canditer).second;
-
-				const Scope &layer_scope = GetRequiredScope(*(*cand_iter).second);
-				const Token &layer_token = GetRequiredToken(*(*cand_iter).second, 0);
-				const int index = ParseTokenAsInt(layer_token);
-				if (index == typedIndex) {
-					const std::string &MappingInformationType = ParseTokenAsString(GetRequiredToken(
-							GetRequiredElement(layer_scope, "MappingInformationType"), 0));
-
-					const std::string &ReferenceInformationType = ParseTokenAsString(GetRequiredToken(
-							GetRequiredElement(layer_scope, "ReferenceInformationType"), 0));
-
-					// Not required:
-					// LayerElementTangent
-					// LayerElementBinormal - perpendicular to tangent.
-					if (type == "LayerElementUV") {
-						if (index == 0) {
-							m_uv_0 = resolve_vertex_data_array<Vector2>(layer_scope, MappingInformationType, ReferenceInformationType, "UV");
-						} else if (index == 1) {
-							m_uv_1 = resolve_vertex_data_array<Vector2>(layer_scope, MappingInformationType, ReferenceInformationType, "UV");
-						}
-					} else if (type == "LayerElementMaterial") {
-						m_material_allocation_ids = resolve_vertex_data_array<int>(layer_scope, MappingInformationType, ReferenceInformationType, "Materials");
-					} else if (type == "LayerElementNormal") {
-						m_normals = resolve_vertex_data_array<Vector3>(layer_scope, MappingInformationType, ReferenceInformationType, "Normals");
-					} else if (type == "LayerElementColor") {
-						m_colors = resolve_vertex_data_array<Color>(layer_scope, MappingInformationType, ReferenceInformationType, "Colors");
+			// we only need the layer name and the typed index.
+			valid_layers.push_back(std::tuple<int, std::string>(typedIndex, type));
+		}
+	}
+
+	// get object / mesh directly from the FBX by the element ID.
+	const Scope &top = GetRequiredScope(element);
+
+	// iterate over all layers for the mesh (uvs, normals, smoothing groups, colors, etc)
+	for( size_t x = 0; x < valid_layers.size(); x++) {
+		const int layer_id = std::get<0>(valid_layers[x]);
+		const std::string &layer_type_name = std::get<1>(valid_layers[x]);
+
+		// Get collection of elements from the XLayerMap (example: LayerElementUV)
+		// this must contain our proper elements.
+
+		// This is stupid, because it means we select them ALL not just the one we want.
+		// but it's fine we can match by id.
+		GetRequiredElement(top, layer_type_name);
+		const ElementCollection &candidates = top.GetCollection(layer_type_name);
+
+		ElementMap::const_iterator iter;
+		for( iter = candidates.first; iter != candidates.second; ++iter)
+		{
+			const Scope *layer_scope = GetRequiredScope(iter->second);
+			TokenPtr layer_token = GetRequiredToken(iter->second, 0);
+			const int index = ParseTokenAsInt(*layer_token);
+
+			ERR_FAIL_COND_MSG(layer_scope == nullptr, "prevented crash, layer scope is invalid");
+
+			if (index == layer_id) {
+				const std::string &MappingInformationType = ParseTokenAsString(GetRequiredToken(
+						GetRequiredElement(*layer_scope, "MappingInformationType"), 0));
+
+				const std::string &ReferenceInformationType = ParseTokenAsString(GetRequiredToken(
+						GetRequiredElement(*layer_scope, "ReferenceInformationType"), 0));
+
+				if (layer_type_name == "LayerElementUV") {
+					if (index == 0) {
+						m_uv_0 = resolve_vertex_data_array<Vector2>(layer_scope, MappingInformationType, ReferenceInformationType, "UV");
+					} else if (index == 1) {
+						m_uv_1 = resolve_vertex_data_array<Vector2>(layer_scope, MappingInformationType, ReferenceInformationType, "UV");
 					}
+				} else if (layer_type_name == "LayerElementMaterial") {
+					m_material_allocation_ids = resolve_vertex_data_array<int>(layer_scope, MappingInformationType, ReferenceInformationType, "Materials");
+				} else if (layer_type_name == "LayerElementNormal") {
+					m_normals = resolve_vertex_data_array<Vector3>(layer_scope, MappingInformationType, ReferenceInformationType, "Normals");
+				} else if (layer_type_name == "LayerElementColor") {
+					m_colors = resolve_vertex_data_array<Color>(layer_scope, MappingInformationType, ReferenceInformationType, "Colors");
 				}
 			}
 		}
 	}
 
+	print_verbose("Mesh statistics \nuv_0: " + m_uv_0.debug_info() + "\nuv_1: " + m_uv_1.debug_info() + "\nvertices: " + itos(m_vertices.size()) );
+
 	// Compose the edge of the mesh.
 	// You can see how the edges are stored into the FBX here: https://gist.github.com/AndreaCatania/da81840f5aa3b2feedf189e26c5a87e6
 	for (size_t i = 0; i < m_edges.size(); i += 1) {
@@ -196,7 +199,7 @@ MeshGeometry::MeshGeometry(uint64_t id, const Element &element, const std::strin
 		int polygon_vertex_1;
 		if (polygon_vertex_0 < 0) {
 			// The polygon_vertex_0 points to the end of a polygon, so it's
-			// connected with the begining of polygon in the edge list.
+			// connected with the beginning of polygon in the edge list.
 
 			// Fist invert the vertex.
 			polygon_vertex_0 = ~polygon_vertex_0;
@@ -268,12 +271,12 @@ const MeshGeometry::MappingData<Vector3> &MeshGeometry::get_normals() const {
 }
 
 const MeshGeometry::MappingData<Vector2> &MeshGeometry::get_uv_0() const {
-	print_verbose("uv 0 size: " + itos(m_uv_0.data.size()));
+	print_verbose("get uv_0 " + m_uv_0.debug_info() );
 	return m_uv_0;
 }
 
 const MeshGeometry::MappingData<Vector2> &MeshGeometry::get_uv_1() const {
-	print_verbose("uv 1 size: " + itos(m_uv_1.data.size()));
+	print_verbose("get uv_1 " + m_uv_1.debug_info() );
 	return m_uv_1;
 }
 
@@ -301,26 +304,29 @@ MeshGeometry::Edge MeshGeometry::get_edge(const std::vector<Edge> &p_map, int p_
 
 template <class T>
 MeshGeometry::MappingData<T> MeshGeometry::resolve_vertex_data_array(
-		const Scope &source,
+		const Scope *source,
 		const std::string &MappingInformationType,
 		const std::string &ReferenceInformationType,
 		const std::string &dataElementName) {
 
+	ERR_FAIL_COND_V_MSG(source == nullptr, MappingData<T>(), "Invalid scope operator preventing memory corruption");
+
 	// UVIndex, MaterialIndex, NormalIndex, etc..
 	std::string indexDataElementName = dataElementName + "Index";
 	// goal: expand everything to be per vertex
 
 	ReferenceType l_ref_type = ReferenceType::direct;
 
-	// purposefully merging legacy to IndexToDirect
-	if (ReferenceInformationType == "IndexToDirect" || ReferenceInformationType == "Index") {
-		// set non legacy index to direct mapping
+	// Read the reference type into the enumeration
+	if (ReferenceInformationType == "IndexToDirect") {
 		l_ref_type = ReferenceType::index_to_direct;
-
-		// override invalid files - should not happen but if it does we're safe.
-		if (!HasElement(source, indexDataElementName)) {
-			l_ref_type = ReferenceType::direct;
-		}
+	} else if (ReferenceInformationType == "Index") {
+		// set non legacy index to direct mapping
+		l_ref_type = ReferenceType::index;
+	} else if(ReferenceInformationType == "Direct") {
+		l_ref_type = ReferenceType::direct;
+	} else {
+		ERR_FAIL_V_MSG(MappingData<T>(), "invalid reference type has the FBX format changed?");
 	}
 
 	MapType l_map_type = MapType::none;
@@ -347,10 +353,10 @@ MeshGeometry::MappingData<T> MeshGeometry::resolve_vertex_data_array(
 	tempData.ref_type = l_ref_type;
 
 	// parse data into array
-	ParseVectorDataArray(tempData.data, GetRequiredElement(source, dataElementName));
+	ParseVectorDataArray(tempData.data, GetRequiredElement(*source, dataElementName));
 
 	// index array wont always exist
-	const Element *element = GetOptionalElement(source, indexDataElementName);
+	const Element *element = GetOptionalElement(*source, indexDataElementName);
 	if (element) {
 		ParseVectorDataArray(tempData.index, *element);
 	}

thirdparty/assimp_fbx/FBXMeshGeometry.h

@@ -114,7 +114,8 @@ class MeshGeometry : public Geometry {
 
 	enum class ReferenceType {
 		direct = 0,
-		index_to_direct = 1
+		index = 1,
+		index_to_direct = 2
 	};
 
 	template <class T>
@@ -125,6 +126,11 @@ class MeshGeometry : public Geometry {
 		/// The meaning of the indices depends from the `MapType`.
 		/// If `ref_type` is `direct` this map is hollow.
 		std::vector<int> index;
+
+		String debug_info() const
+		{
+			return "indexes: " + itos(index.size()) + " data: " +itos(data.size());
+		}
 	};
 
 	struct Edge {
@@ -170,7 +176,7 @@ class MeshGeometry : public Geometry {
 
 	template <class T>
 	MappingData<T> resolve_vertex_data_array(
-			const Scope &source,
+			const Scope *source,
 			const std::string &MappingInformationType,
 			const std::string &ReferenceInformationType,
 			const std::string &dataElementName);