Merge pull request #57551 from lawnjelly/portals_improve_merging

doc/classes/CullInstance.xml

@@ -15,6 +15,10 @@
 	<methods>
 	</methods>
 	<members>
+		<member name="allow_merging" type="bool" setter="set_allow_merging" getter="get_allow_merging" default="true">
+			This allows fine control over the mesh merging feature in the [RoomManager].
+			Setting this option to [code]false[/code] can be used to prevent an instance being merged.
+		</member>
 		<member name="autoplace_priority" type="int" setter="set_portal_autoplace_priority" getter="get_portal_autoplace_priority" default="0">
 			When set to [code]0[/code], [CullInstance]s will be autoplaced in the [Room] with the highest priority.
 			When set to a value other than [code]0[/code], the system will attempt to autoplace in a [Room] with the [code]autoplace_priority[/code], if it is present.

scene/3d/cull_instance.cpp

@@ -45,9 +45,12 @@ void CullInstance::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("set_portal_mode", "mode"), &CullInstance::set_portal_mode);
 	ClassDB::bind_method(D_METHOD("get_portal_mode"), &CullInstance::get_portal_mode);
 
-	ClassDB::bind_method(D_METHOD("set_include_in_bound"), &CullInstance::set_include_in_bound);
+	ClassDB::bind_method(D_METHOD("set_include_in_bound", "enabled"), &CullInstance::set_include_in_bound);
 	ClassDB::bind_method(D_METHOD("get_include_in_bound"), &CullInstance::get_include_in_bound);
 
+	ClassDB::bind_method(D_METHOD("set_allow_merging", "enabled"), &CullInstance::set_allow_merging);
+	ClassDB::bind_method(D_METHOD("get_allow_merging"), &CullInstance::get_allow_merging);
+
 	ClassDB::bind_method(D_METHOD("set_portal_autoplace_priority", "priority"), &CullInstance::set_portal_autoplace_priority);
 	ClassDB::bind_method(D_METHOD("get_portal_autoplace_priority"), &CullInstance::get_portal_autoplace_priority);
 
@@ -61,11 +64,13 @@ void CullInstance::_bind_methods() {
 
 	ADD_PROPERTY(PropertyInfo(Variant::INT, "portal_mode", PROPERTY_HINT_ENUM, "Static,Dynamic,Roaming,Global,Ignore"), "set_portal_mode", "get_portal_mode");
 	ADD_PROPERTY(PropertyInfo(Variant::BOOL, "include_in_bound"), "set_include_in_bound", "get_include_in_bound");
+	ADD_PROPERTY(PropertyInfo(Variant::BOOL, "allow_merging"), "set_allow_merging", "get_allow_merging");
 	ADD_PROPERTY(PropertyInfo(Variant::INT, "autoplace_priority", PROPERTY_HINT_RANGE, "-16,16,1", PROPERTY_USAGE_DEFAULT), "set_portal_autoplace_priority", "get_portal_autoplace_priority");
 }
 
 CullInstance::CullInstance() {
 	_portal_mode = PORTAL_MODE_STATIC;
 	_include_in_bound = true;
+	_allow_merging = true;
 	_portal_autoplace_priority = 0;
 }

scene/3d/cull_instance.h

@@ -48,9 +48,12 @@ public:
 	void set_portal_mode(CullInstance::PortalMode p_mode);
 	CullInstance::PortalMode get_portal_mode() const;
 
-	void set_include_in_bound(bool p_enable) { _include_in_bound = p_enable; }
+	void set_include_in_bound(bool p_enabled) { _include_in_bound = p_enabled; }
 	bool get_include_in_bound() const { return _include_in_bound; }
 
+	void set_allow_merging(bool p_enabled) { _allow_merging = p_enabled; }
+	bool get_allow_merging() const { return _allow_merging; }
+
 	void set_portal_autoplace_priority(int p_priority) { _portal_autoplace_priority = p_priority; }
 	int get_portal_autoplace_priority() const { return _portal_autoplace_priority; }
 
@@ -63,7 +66,8 @@ protected:
 
 private:
 	PortalMode _portal_mode;
-	bool _include_in_bound;
+	bool _include_in_bound : 1;
+	bool _allow_merging : 1;
 
 	// Allows instances to prefer to be autoplaced
 	// in specific RoomGroups. This allows building exteriors

scene/3d/mesh_instance.cpp

@@ -847,10 +847,40 @@ void MeshInstance::create_debug_tangents() {
 	}
 }
 
-bool MeshInstance::is_mergeable_with(const MeshInstance &p_other) {
+bool MeshInstance::is_mergeable_with(Node *p_other) const {
+	const MeshInstance *mi = Object::cast_to<MeshInstance>(p_other);
+
+	if (mi) {
+		return _is_mergeable_with(*mi);
+	}
+
+	return false;
+}
+
+bool MeshInstance::_is_mergeable_with(const MeshInstance &p_other) const {
 	if (!get_mesh().is_valid() || !p_other.get_mesh().is_valid()) {
 		return false;
 	}
+	if (!get_allow_merging() || !p_other.get_allow_merging()) {
+		return false;
+	}
+
+	// various settings that must match
+	if (get_material_overlay() != p_other.get_material_overlay()) {
+		return false;
+	}
+	if (get_material_override() != p_other.get_material_override()) {
+		return false;
+	}
+	if (get_cast_shadows_setting() != p_other.get_cast_shadows_setting()) {
+		return false;
+	}
+	if (get_flag(FLAG_USE_BAKED_LIGHT) != p_other.get_flag(FLAG_USE_BAKED_LIGHT)) {
+		return false;
+	}
+	if (is_visible() != p_other.is_visible()) {
+		return false;
+	}
 
 	Ref<Mesh> rmesh_a = get_mesh();
 	Ref<Mesh> rmesh_b = p_other.get_mesh();
@@ -860,11 +890,6 @@ bool MeshInstance::is_mergeable_with(const MeshInstance &p_other) {
 		return false;
 	}
 
-	// overlay materials must match
-	if (get_material_overlay() != p_other.get_material_overlay()) {
-		return false;
-	}
-
 	for (int n = 0; n < num_surfaces; n++) {
 		// materials must match
 		if (get_active_material(n) != p_other.get_active_material(n)) {
@@ -903,7 +928,7 @@ bool MeshInstance::is_mergeable_with(const MeshInstance &p_other) {
 	return true;
 }
 
-void MeshInstance::_merge_into_mesh_data(const MeshInstance &p_mi, int p_surface_id, PoolVector<Vector3> &r_verts, PoolVector<Vector3> &r_norms, PoolVector<real_t> &r_tangents, PoolVector<Color> &r_colors, PoolVector<Vector2> &r_uvs, PoolVector<Vector2> &r_uv2s, PoolVector<int> &r_inds) {
+void MeshInstance::_merge_into_mesh_data(const MeshInstance &p_mi, const Transform &p_dest_tr_inv, int p_surface_id, PoolVector<Vector3> &r_verts, PoolVector<Vector3> &r_norms, PoolVector<real_t> &r_tangents, PoolVector<Color> &r_colors, PoolVector<Vector2> &r_uvs, PoolVector<Vector2> &r_uv2s, PoolVector<int> &r_inds) {
 	_merge_log("\t\t\tmesh data from " + p_mi.get_name());
 
 	// get the mesh verts in local space
@@ -923,7 +948,32 @@ void MeshInstance::_merge_into_mesh_data(const MeshInstance &p_mi, int p_surface
 	PoolVector<Vector2> uv2s = arrays[VS::ARRAY_TEX_UV2];
 	PoolVector<int> indices = arrays[VS::ARRAY_INDEX];
 
-	// NEW .. the checking for valid triangles should be on WORLD SPACE vertices,
+	// The attributes present must match the first mesh for the attributes
+	// to remain in sync. Here we reject meshes with different attributes.
+	// We could alternatively invent missing attributes.
+	// This should hopefully be already caught by the mesh_format, but is included just in case here.
+
+	// Don't perform these checks on the first Mesh, the first Mesh is a master
+	// and determines the attributes we want to be present.
+	if (r_verts.size() != 0) {
+		if ((bool)r_norms.size() != (bool)normals.size()) {
+			ERR_FAIL_MSG("Attribute mismatch with first Mesh (Normals), ignoring surface.");
+		}
+		if ((bool)r_tangents.size() != (bool)tangents.size()) {
+			ERR_FAIL_MSG("Attribute mismatch with first Mesh (Tangents), ignoring surface.");
+		}
+		if ((bool)r_colors.size() != (bool)colors.size()) {
+			ERR_FAIL_MSG("Attribute mismatch with first Mesh (Colors), ignoring surface.");
+		}
+		if ((bool)r_uvs.size() != (bool)uvs.size()) {
+			ERR_FAIL_MSG("Attribute mismatch with first Mesh (UVs), ignoring surface.");
+		}
+		if ((bool)r_uv2s.size() != (bool)uv2s.size()) {
+			ERR_FAIL_MSG("Attribute mismatch with first Mesh (UV2s), ignoring surface.");
+		}
+	}
+
+	// The checking for valid triangles should be on WORLD SPACE vertices,
 	// NOT model space
 
 	// special case, if no indices, create some
@@ -938,6 +988,11 @@ void MeshInstance::_merge_into_mesh_data(const MeshInstance &p_mi, int p_surface
 	// transform verts to world space
 	Transform tr = p_mi.get_global_transform();
 
+	// But relative to the destination transform.
+	// This can either be identity (when the destination is global space),
+	// or the global transform of the owner MeshInstance (if using local space is selected).
+	tr = p_dest_tr_inv * tr;
+
 	// to transform normals
 	Basis normal_basis = tr.basis.inverse();
 	normal_basis.transpose();
@@ -985,7 +1040,7 @@ void MeshInstance::_merge_into_mesh_data(const MeshInstance &p_mi, int p_surface
 	}
 }
 
-bool MeshInstance::_ensure_indices_valid(PoolVector<int> &r_indices, const PoolVector<Vector3> &p_verts) {
+bool MeshInstance::_ensure_indices_valid(PoolVector<int> &r_indices, const PoolVector<Vector3> &p_verts) const {
 	// no indices? create some
 	if (!r_indices.size()) {
 		_merge_log("\t\t\t\tindices are blank, creating...");
@@ -1022,7 +1077,7 @@ bool MeshInstance::_ensure_indices_valid(PoolVector<int> &r_indices, const PoolV
 }
 
 // check for invalid tris, or make a list of the valid triangles, depending on whether r_inds is set
-bool MeshInstance::_check_for_valid_indices(const PoolVector<int> &p_inds, const PoolVector<Vector3> &p_verts, LocalVector<int, int32_t> *r_inds) {
+bool MeshInstance::_check_for_valid_indices(const PoolVector<int> &p_inds, const PoolVector<Vector3> &p_verts, LocalVector<int, int32_t> *r_inds) const {
 	int nTris = p_inds.size();
 	nTris /= 3;
 	int indCount = 0;
@@ -1077,7 +1132,7 @@ bool MeshInstance::_check_for_valid_indices(const PoolVector<int> &p_inds, const
 	return true;
 }
 
-bool MeshInstance::_triangle_is_degenerate(const Vector3 &p_a, const Vector3 &p_b, const Vector3 &p_c, real_t p_epsilon) {
+bool MeshInstance::_triangle_is_degenerate(const Vector3 &p_a, const Vector3 &p_b, const Vector3 &p_c, real_t p_epsilon) const {
 	// not interested in the actual area, but numerical stability
 	Vector3 edge1 = p_b - p_a;
 	Vector3 edge2 = p_c - p_a;
@@ -1096,9 +1151,10 @@ bool MeshInstance::_triangle_is_degenerate(const Vector3 &p_a, const Vector3 &p_
 	return false;
 }
 
-bool MeshInstance::create_by_merging(Vector<MeshInstance *> p_list) {
-	// must be at least 2 meshes to merge
-	if (p_list.size() < 2) {
+// If p_check_compatibility is set to false you MUST have performed a prior check using
+// is_mergeable_with, otherwise you could get mismatching surface formats leading to graphical errors etc.
+bool MeshInstance::merge_meshes(Vector<MeshInstance *> p_list, bool p_use_global_space, bool p_check_compatibility) {
+	if (p_list.size() < 1) {
 		// should not happen but just in case
 		return false;
 	}
@@ -1106,9 +1162,42 @@ bool MeshInstance::create_by_merging(Vector<MeshInstance *> p_list) {
 	// use the first mesh instance to get common data like number of surfaces
 	const MeshInstance *first = p_list[0];
 
+	// Mesh compatibility checking. This is relatively expensive, so if done already (e.g. in Room system)
+	// this step can be avoided.
+	LocalVector<bool> compat_list;
+	if (p_check_compatibility) {
+		compat_list.resize(p_list.size());
+
+		for (int n = 0; n < p_list.size(); n++) {
+			compat_list[n] = false;
+		}
+
+		compat_list[0] = true;
+
+		for (uint32_t n = 1; n < compat_list.size(); n++) {
+			compat_list[n] = first->_is_mergeable_with(*p_list[n]);
+
+			if (compat_list[n] == false) {
+				WARN_PRINT("MeshInstance " + p_list[n]->get_name() + " is incompatible for merging with " + first->get_name() + ", ignoring.");
+			}
+		}
+	}
+
 	Ref<ArrayMesh> am;
 	am.instance();
 
+	// If we want a local space result, we need the world space transform of this MeshInstance
+	// available to back transform verts from world space.
+	Transform dest_tr_inv;
+	if (!p_use_global_space) {
+		if (is_inside_tree()) {
+			dest_tr_inv = get_global_transform();
+			dest_tr_inv.affine_invert();
+		} else {
+			WARN_PRINT("MeshInstance must be inside tree to merge using local space, falling back to global space.");
+		}
+	}
+
 	for (int s = 0; s < first->get_mesh()->get_surface_count(); s++) {
 		PoolVector<Vector3> verts;
 		PoolVector<Vector3> normals;
@@ -1119,7 +1208,12 @@ bool MeshInstance::create_by_merging(Vector<MeshInstance *> p_list) {
 		PoolVector<int> inds;
 
 		for (int n = 0; n < p_list.size(); n++) {
-			_merge_into_mesh_data(*p_list[n], s, verts, normals, tangents, colors, uvs, uv2s, inds);
+			// Ignore if the mesh is incompatible
+			if (p_check_compatibility && (!compat_list[n])) {
+				continue;
+			}
+
+			_merge_into_mesh_data(*p_list[n], dest_tr_inv, s, verts, normals, tangents, colors, uvs, uv2s, inds);
 		} // for n through source meshes
 
 		if (!verts.size()) {
@@ -1167,13 +1261,16 @@ bool MeshInstance::create_by_merging(Vector<MeshInstance *> p_list) {
 		set_surface_material(n, first->get_active_material(n));
 	}
 
-	// set overlay material
+	// set some properties to match the merged meshes
 	set_material_overlay(first->get_material_overlay());
+	set_material_override(first->get_material_override());
+	set_cast_shadows_setting(first->get_cast_shadows_setting());
+	set_flag(FLAG_USE_BAKED_LIGHT, first->get_flag(FLAG_USE_BAKED_LIGHT));
 
 	return true;
 }
 
-void MeshInstance::_merge_log(String p_string) {
+void MeshInstance::_merge_log(String p_string) const {
 	print_verbose(p_string);
 }
 

scene/3d/mesh_instance.h

@@ -96,11 +96,12 @@ protected:
 
 private:
 	// merging
-	void _merge_into_mesh_data(const MeshInstance &p_mi, int p_surface_id, PoolVector<Vector3> &r_verts, PoolVector<Vector3> &r_norms, PoolVector<real_t> &r_tangents, PoolVector<Color> &r_colors, PoolVector<Vector2> &r_uvs, PoolVector<Vector2> &r_uv2s, PoolVector<int> &r_inds);
-	bool _ensure_indices_valid(PoolVector<int> &r_indices, const PoolVector<Vector3> &p_verts);
-	bool _check_for_valid_indices(const PoolVector<int> &p_inds, const PoolVector<Vector3> &p_verts, LocalVector<int, int32_t> *r_inds);
-	bool _triangle_is_degenerate(const Vector3 &p_a, const Vector3 &p_b, const Vector3 &p_c, real_t p_epsilon);
-	void _merge_log(String p_string);
+	bool _is_mergeable_with(const MeshInstance &p_other) const;
+	void _merge_into_mesh_data(const MeshInstance &p_mi, const Transform &p_dest_tr_inv, int p_surface_id, PoolVector<Vector3> &r_verts, PoolVector<Vector3> &r_norms, PoolVector<real_t> &r_tangents, PoolVector<Color> &r_colors, PoolVector<Vector2> &r_uvs, PoolVector<Vector2> &r_uv2s, PoolVector<int> &r_inds);
+	bool _ensure_indices_valid(PoolVector<int> &r_indices, const PoolVector<Vector3> &p_verts) const;
+	bool _check_for_valid_indices(const PoolVector<int> &p_inds, const PoolVector<Vector3> &p_verts, LocalVector<int, int32_t> *r_inds) const;
+	bool _triangle_is_degenerate(const Vector3 &p_a, const Vector3 &p_b, const Vector3 &p_c, real_t p_epsilon) const;
+	void _merge_log(String p_string) const;
 
 protected:
 	bool _set(const StringName &p_name, const Variant &p_value);
@@ -144,8 +145,8 @@ public:
 	void create_debug_tangents();
 
 	// merging
-	bool is_mergeable_with(const MeshInstance &p_other);
-	bool create_by_merging(Vector<MeshInstance *> p_list);
+	bool is_mergeable_with(Node *p_other) const;
+	bool merge_meshes(Vector<MeshInstance *> p_list, bool p_use_global_space, bool p_check_compatibility);
 
 	virtual AABB get_aabb() const;
 	virtual PoolVector<Face3> get_faces(uint32_t p_usage_flags) const;

scene/3d/room_manager.cpp

@@ -2139,8 +2139,7 @@ void RoomManager::_merge_meshes_in_room(Room *p_room) {
 			if (!bf.get_bit(c)) {
 				MeshInstance *b = source_meshes[c];
 
-				//				if (_are_meshes_mergeable(a, b)) {
-				if (a->is_mergeable_with(*b)) {
+				if (a->is_mergeable_with(b)) {
 					merge_list.push_back(b);
 					bf.set_bit(c, true);
 				}
@@ -2157,7 +2156,7 @@ void RoomManager::_merge_meshes_in_room(Room *p_room) {
 
 			_merge_log("\t\t" + merged->get_name());
 
-			if (merged->create_by_merging(merge_list)) {
+			if (merged->merge_meshes(merge_list, true, false)) {
 				// set all the source meshes to portal mode ignore so not shown
 				for (int i = 0; i < merge_list.size(); i++) {
 					merge_list[i]->set_portal_mode(CullInstance::PORTAL_MODE_IGNORE);