Merge pull request #82291 from bitsawer/fix_occlusion_culling_leak

Fix RaycastOcclusionCull World3D scenario memory leak
godotengine · Sep 28, 2023 · 4c95ebd · 4c95ebd
2 parents 2a31f2d + 7654e78
commit 4c95ebd
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 45 deletions.
diff --git a/modules/raycast/raycast_occlusion_cull.cpp b/modules/raycast/raycast_occlusion_cull.cpp
@@ -250,17 +250,15 @@ void RaycastOcclusionCull::free_occluder(RID p_occluder) {
 ////////////////////////////////////////////////////////
 
 void RaycastOcclusionCull::add_scenario(RID p_scenario) {
-	if (scenarios.has(p_scenario)) {
-		scenarios[p_scenario].removed = false;
-	} else {
-		scenarios[p_scenario] = Scenario();
-	}
+	ERR_FAIL_COND(scenarios.has(p_scenario));
+	scenarios[p_scenario] = Scenario();
 }
 
 void RaycastOcclusionCull::remove_scenario(RID p_scenario) {
-	ERR_FAIL_COND(!scenarios.has(p_scenario));
-	Scenario &scenario = scenarios[p_scenario];
-	scenario.removed = true;
+	Scenario *scenario = scenarios.getptr(p_scenario);
+	ERR_FAIL_NULL(scenario);
+	scenario->free();
+	scenarios.erase(p_scenario);
 }
 
 void RaycastOcclusionCull::scenario_set_instance(RID p_scenario, RID p_instance, RID p_occluder, const Transform3D &p_xform, bool p_enabled) {
@@ -390,15 +388,32 @@ void RaycastOcclusionCull::Scenario::_transform_vertices_range(const Vector3 *p_
 	}
 }
 
+void RaycastOcclusionCull::Scenario::free() {
+	if (commit_thread) {
+		if (commit_thread->is_started()) {
+			commit_thread->wait_to_finish();
+		}
+		memdelete(commit_thread);
+		commit_thread = nullptr;
+	}
+
+	for (int i = 0; i < 2; i++) {
+		if (ebr_scene[i]) {
+			rtcReleaseScene(ebr_scene[i]);
+			ebr_scene[i] = nullptr;
+		}
+	}
+}
+
 void RaycastOcclusionCull::Scenario::_commit_scene(void *p_ud) {
 	Scenario *scenario = (Scenario *)p_ud;
 	int commit_idx = 1 - (scenario->current_scene_idx);
 	rtcCommitScene(scenario->ebr_scene[commit_idx]);
 	scenario->commit_done = true;
 }
 
-bool RaycastOcclusionCull::Scenario::update() {
-	ERR_FAIL_NULL_V(singleton, false);
+void RaycastOcclusionCull::Scenario::update() {
+	ERR_FAIL_NULL(singleton);
 
 	if (commit_thread == nullptr) {
 		commit_thread = memnew(Thread);
@@ -409,22 +424,12 @@ bool RaycastOcclusionCull::Scenario::update() {
 			commit_thread->wait_to_finish();
 			current_scene_idx = 1 - current_scene_idx;
 		} else {
-			return false;
+			return;
 		}
 	}
 
-	if (removed) {
-		if (ebr_scene[0]) {
-			rtcReleaseScene(ebr_scene[0]);
-		}
-		if (ebr_scene[1]) {
-			rtcReleaseScene(ebr_scene[1]);
-		}
-		return true;
-	}
-
 	if (!dirty && removed_instances.is_empty() && dirty_instances_array.is_empty()) {
-		return false;
+		return;
 	}
 
 	for (const RID &scenario : removed_instances) {
@@ -480,7 +485,6 @@ bool RaycastOcclusionCull::Scenario::update() {
 	dirty = false;
 	commit_done = false;
 	commit_thread->start(&Scenario::_commit_scene, this);
-	return false;
 }
 
 void RaycastOcclusionCull::Scenario::_raycast(uint32_t p_idx, const RaycastThreadData *p_raycast_data) const {
@@ -544,13 +548,7 @@ void RaycastOcclusionCull::buffer_update(RID p_buffer, const Transform3D &p_cam_
 	}
 
 	Scenario &scenario = scenarios[buffer.scenario_rid];
-
-	bool removed = scenario.update();
-
-	if (removed) {
-		scenarios.erase(buffer.scenario_rid);
-		return;
-	}
+	scenario.update();
 
 	buffer.update_camera_rays(p_cam_transform, p_cam_projection, p_cam_orthogonal);
 
@@ -603,19 +601,7 @@ RaycastOcclusionCull::RaycastOcclusionCull() {
 
 RaycastOcclusionCull::~RaycastOcclusionCull() {
 	for (KeyValue<RID, Scenario> &K : scenarios) {
-		Scenario &scenario = K.value;
-		if (scenario.commit_thread) {
-			if (scenario.commit_thread->is_started()) {
-				scenario.commit_thread->wait_to_finish();
-			}
-			memdelete(scenario.commit_thread);
-		}
-
-		for (int i = 0; i < 2; i++) {
-			if (scenario.ebr_scene[i]) {
-				rtcReleaseScene(scenario.ebr_scene[i]);
-			}
-		}
+		K.value.free();
 	}
 
 	if (ebr_device != nullptr) {

diff --git a/modules/raycast/raycast_occlusion_cull.h b/modules/raycast/raycast_occlusion_cull.h
@@ -132,7 +132,6 @@ class RaycastOcclusionCull : public RendererSceneOcclusionCull {
 		Thread *commit_thread = nullptr;
 		bool commit_done = true;
 		bool dirty = false;
-		bool removed = false;
 
 		RTCScene ebr_scene[2] = { nullptr, nullptr };
 		int current_scene_idx = 0;
@@ -147,7 +146,8 @@ class RaycastOcclusionCull : public RendererSceneOcclusionCull {
 		void _transform_vertices_thread(uint32_t p_thread, TransformThreadData *p_data);
 		void _transform_vertices_range(const Vector3 *p_read, Vector3 *p_write, const Transform3D &p_xform, int p_from, int p_to);
 		static void _commit_scene(void *p_ud);
-		bool update();
+		void free();
+		void update();
 
 		void _raycast(uint32_t p_thread, const RaycastThreadData *p_raycast_data) const;
 		void raycast(CameraRayTile *r_rays, const uint32_t *p_valid_masks, uint32_t p_tile_count) const;