Merge pull request #87478 from AThousandShips/tile_thread_fix_2

Prevent threading problems in `TileMap`
godotengine · Feb 27, 2024 · 33bc762 · 33bc762
2 parents a586e86 + fe203d7
commit 33bc762
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 14 deletions.
diff --git a/doc/classes/TileMap.xml b/doc/classes/TileMap.xml
@@ -5,7 +5,7 @@
 	</brief_description>
 	<description>
 		Node for 2D tile-based maps. Tilemaps use a [TileSet] which contain a list of tiles which are used to create grid-based maps. A TileMap may have several layers, layouting tiles on top of each other.
-		For performance reasons, all TileMap updates are batched at the end of a frame. Notably, this means that scene tiles from a [TileSetScenesCollectionSource] may be initialized after their parent.
+		For performance reasons, all TileMap updates are batched at the end of a frame. Notably, this means that scene tiles from a [TileSetScenesCollectionSource] may be initialized after their parent. This is only queued when inside the scene tree.
 		To force an update earlier on, call [method update_internals].
 	</description>
 	<tutorials>

diff --git a/editor/plugins/tiles/tiles_editor_plugin.cpp b/editor/plugins/tiles/tiles_editor_plugin.cpp
@@ -69,9 +69,6 @@ void TilesEditorUtils::_thread_func(void *ud) {
 }
 
 void TilesEditorUtils::_thread() {
-	CallQueue queue;
-	MessageQueue::set_thread_singleton_override(&queue);
-
 	pattern_thread_exited.clear();
 	while (!pattern_thread_exit.is_set()) {
 		pattern_preview_sem.wait();
@@ -131,8 +128,6 @@ void TilesEditorUtils::_thread() {
 				// Add the viewport at the last moment to avoid rendering too early.
 				callable_mp((Node *)EditorNode::get_singleton(), &Node::add_child).call_deferred(viewport, false, Node::INTERNAL_MODE_DISABLED);
 
-				MessageQueue::get_singleton()->flush();
-
 				RS::get_singleton()->connect(SNAME("frame_pre_draw"), callable_mp(const_cast<TilesEditorUtils *>(this), &TilesEditorUtils::_preview_frame_started), Object::CONNECT_ONE_SHOT);
 
 				pattern_preview_done.wait();
@@ -145,11 +140,7 @@ void TilesEditorUtils::_thread() {
 				viewport->queue_free();
 			}
 		}
-
-		MessageQueue::get_singleton()->flush();
 	}
-
-	MessageQueue::get_singleton()->flush();
 	pattern_thread_exited.set();
 }
 

diff --git a/scene/2d/tile_map_layer.cpp b/scene/2d/tile_map_layer.cpp
@@ -1625,8 +1625,11 @@ void TileMapLayer::_queue_internal_update() {
 	if (pending_update) {
 		return;
 	}
-	pending_update = true;
-	callable_mp(this, &TileMapLayer::_deferred_internal_update).call_deferred();
+	// Don't update when outside the tree, it doesn't do anything useful, and causes threading problems.
+	if (is_inside_tree()) {
+		pending_update = true;
+		callable_mp(this, &TileMapLayer::_deferred_internal_update).call_deferred();
+	}
 }
 
 void TileMapLayer::_deferred_internal_update() {
@@ -1695,7 +1698,8 @@ void TileMapLayer::_notification(int p_what) {
 
 		case NOTIFICATION_EXIT_TREE: {
 			dirty.flags[DIRTY_FLAGS_LAYER_IN_TREE] = true;
-			_queue_internal_update();
+			// Update immediately on exiting.
+			update_internals();
 		} break;
 
 		case TileMap::NOTIFICATION_ENTER_CANVAS: {
@@ -1705,7 +1709,8 @@ void TileMapLayer::_notification(int p_what) {
 
 		case TileMap::NOTIFICATION_EXIT_CANVAS: {
 			dirty.flags[DIRTY_FLAGS_LAYER_IN_CANVAS] = true;
-			_queue_internal_update();
+			// Update immediately on exiting.
+			update_internals();
 		} break;
 
 		case TileMap::NOTIFICATION_VISIBILITY_CHANGED: {