Fix never ending loop with overlapping probes

drivers/gles3/storage/light_storage.cpp

@@ -541,6 +541,10 @@ void LightStorage::reflection_probe_instance_free(RID p_instance) {
 void LightStorage::reflection_probe_instance_set_transform(RID p_instance, const Transform3D &p_transform) {
 }
 
+bool LightStorage::reflection_probe_has_atlas_index(RID p_instance) {
+	return false;
+}
+
 void LightStorage::reflection_probe_release_atlas_index(RID p_instance) {
 }
 

drivers/gles3/storage/light_storage.h

@@ -601,6 +601,7 @@ public:
 	virtual RID reflection_probe_instance_create(RID p_probe) override;
 	virtual void reflection_probe_instance_free(RID p_instance) override;
 	virtual void reflection_probe_instance_set_transform(RID p_instance, const Transform3D &p_transform) override;
+	virtual bool reflection_probe_has_atlas_index(RID p_instance) override;
 	virtual void reflection_probe_release_atlas_index(RID p_instance) override;
 	virtual bool reflection_probe_instance_needs_redraw(RID p_instance) override;
 	virtual bool reflection_probe_instance_has_reflection(RID p_instance) override;

servers/rendering/dummy/storage/light_storage.h

@@ -135,6 +135,7 @@ public:
 	virtual RID reflection_probe_instance_create(RID p_probe) override { return RID(); }
 	virtual void reflection_probe_instance_free(RID p_instance) override {}
 	virtual void reflection_probe_instance_set_transform(RID p_instance, const Transform3D &p_transform) override {}
+	virtual bool reflection_probe_has_atlas_index(RID p_instance) override { return false; }
 	virtual void reflection_probe_release_atlas_index(RID p_instance) override {}
 	virtual bool reflection_probe_instance_needs_redraw(RID p_instance) override { return false; }
 	virtual bool reflection_probe_instance_has_reflection(RID p_instance) override { return false; }

servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp

@@ -1615,15 +1615,16 @@ void RenderForwardClustered::_render_scene(RenderDataRD *p_render_data, const Co
 
 		gi.setup_voxel_gi_instances(p_render_data, p_render_data->render_buffers, p_render_data->scene_data->cam_transform, *p_render_data->voxel_gi_instances, p_render_data->voxel_gi_count);
 	} else {
-		ERR_PRINT("No render buffer nor reflection atlas, bug"); //should never happen, will crash
+		ERR_PRINT("No render buffer nor reflection atlas, bug"); // Should never happen!
 		current_cluster_builder = nullptr;
+		return; // No point in continuing, we'll just crash.
 	}
 
-	if (current_cluster_builder != nullptr) {
-		p_render_data->cluster_buffer = current_cluster_builder->get_cluster_buffer();
-		p_render_data->cluster_size = current_cluster_builder->get_cluster_size();
-		p_render_data->cluster_max_elements = current_cluster_builder->get_max_cluster_elements();
-	}
+	ERR_FAIL_NULL(current_cluster_builder);
+
+	p_render_data->cluster_buffer = current_cluster_builder->get_cluster_buffer();
+	p_render_data->cluster_size = current_cluster_builder->get_cluster_size();
+	p_render_data->cluster_max_elements = current_cluster_builder->get_max_cluster_elements();
 
 	_update_vrs(rb);
 

servers/rendering/renderer_rd/storage_rd/light_storage.cpp

@@ -1370,6 +1370,17 @@ void LightStorage::reflection_probe_instance_set_transform(RID p_instance, const
 	rpi->dirty = true;
 }
 
+bool LightStorage::reflection_probe_has_atlas_index(RID p_instance) {
+	ReflectionProbeInstance *rpi = reflection_probe_instance_owner.get_or_null(p_instance);
+	ERR_FAIL_NULL_V(rpi, false);
+
+	if (rpi->atlas.is_null()) {
+		return false;
+	}
+
+	return rpi->atlas_index >= 0;
+}
+
 void LightStorage::reflection_probe_release_atlas_index(RID p_instance) {
 	ReflectionProbeInstance *rpi = reflection_probe_instance_owner.get_or_null(p_instance);
 	ERR_FAIL_NULL(rpi);
@@ -1384,6 +1395,14 @@ void LightStorage::reflection_probe_release_atlas_index(RID p_instance) {
 
 	// TODO investigate if this is enough? shouldn't we be freeing our textures and framebuffers?
 
+	if (rpi->rendering) {
+		// We were cancelled mid rendering, trigger refresh.
+		rpi->rendering = false;
+		rpi->dirty = true;
+		rpi->processing_layer = 1;
+		rpi->processing_side = 0;
+	}
+
 	rpi->atlas_index = -1;
 	rpi->atlas = RID();
 }
@@ -1535,11 +1554,10 @@ bool LightStorage::reflection_probe_instance_postprocess_step(RID p_instance) {
 	ReflectionProbeInstance *rpi = reflection_probe_instance_owner.get_or_null(p_instance);
 	ERR_FAIL_NULL_V(rpi, false);
 	ERR_FAIL_COND_V(!rpi->rendering, false);
-	ERR_FAIL_COND_V(rpi->atlas.is_null(), false);
 
 	ReflectionAtlas *atlas = reflection_atlas_owner.get_or_null(rpi->atlas);
 	if (!atlas || rpi->atlas_index == -1) {
-		//does not belong to an atlas anymore, cancel (was removed from atlas or atlas changed while rendering)
+		// Does not belong to an atlas anymore, cancel (was removed from atlas or atlas changed while rendering).
 		rpi->rendering = false;
 		return false;
 	}

servers/rendering/renderer_rd/storage_rd/light_storage.h

@@ -277,7 +277,6 @@ private:
 		int processing_layer = 1;
 		int processing_side = 0;
 
-		uint32_t render_step = 0;
 		uint64_t last_pass = 0;
 		uint32_t cull_mask = 0;
 
@@ -848,6 +847,7 @@ public:
 	virtual RID reflection_probe_instance_create(RID p_probe) override;
 	virtual void reflection_probe_instance_free(RID p_instance) override;
 	virtual void reflection_probe_instance_set_transform(RID p_instance, const Transform3D &p_transform) override;
+	virtual bool reflection_probe_has_atlas_index(RID p_instance) override;
 	virtual void reflection_probe_release_atlas_index(RID p_instance) override;
 	virtual bool reflection_probe_instance_needs_redraw(RID p_instance) override;
 	virtual bool reflection_probe_instance_has_reflection(RID p_instance) override;

servers/rendering/renderer_scene_cull.cpp

@@ -3482,8 +3482,13 @@ bool RendererSceneCull::_render_reflection_probe_step(Instance *p_instance, int
 
 	if (p_step == 0) {
 		if (!RSG::light_storage->reflection_probe_instance_begin_render(reflection_probe->instance, scenario->reflection_atlas)) {
-			return true; //all full
+			return true; // All full, no atlas entry to render to.
 		}
+	} else if (!RSG::light_storage->reflection_probe_has_atlas_index(reflection_probe->instance)) {
+		// We don't have an atlas to render to, just round off.
+		// This is likely due to the atlas being reset.
+		// If so the probe will be marked as dirty and start over.
+		return true;
 	}
 
 	if (p_step >= 0 && p_step < 6) {
@@ -3558,6 +3563,7 @@ void RendererSceneCull::render_probes() {
 	/* REFLECTION PROBES */
 
 	SelfList<InstanceReflectionProbeData> *ref_probe = reflection_probe_render_list.first();
+	Vector<SelfList<InstanceReflectionProbeData> *> done_list;
 
 	bool busy = false;
 
@@ -3573,7 +3579,7 @@ void RendererSceneCull::render_probes() {
 
 				bool done = _render_reflection_probe_step(ref_probe->self()->owner, ref_probe->self()->render_step);
 				if (done) {
-					reflection_probe_render_list.remove(ref_probe);
+					done_list.push_back(ref_probe);
 				} else {
 					ref_probe->self()->render_step++;
 				}
@@ -3588,13 +3594,18 @@ void RendererSceneCull::render_probes() {
 					step++;
 				}
 
-				reflection_probe_render_list.remove(ref_probe);
+				done_list.push_back(ref_probe);
 			} break;
 		}
 
 		ref_probe = next;
 	}
 
+	// Now remove from our list
+	for (SelfList<InstanceReflectionProbeData> *rp : done_list) {
+		reflection_probe_render_list.remove(rp);
+	}
+
 	/* VOXEL GIS */
 
 	SelfList<InstanceVoxelGIData> *voxel_gi = voxel_gi_update_list.first();

servers/rendering/storage/light_storage.h

@@ -143,6 +143,7 @@ public:
 	virtual RID reflection_probe_instance_create(RID p_probe) = 0;
 	virtual void reflection_probe_instance_free(RID p_instance) = 0;
 	virtual void reflection_probe_instance_set_transform(RID p_instance, const Transform3D &p_transform) = 0;
+	virtual bool reflection_probe_has_atlas_index(RID p_instance) = 0;
 	virtual void reflection_probe_release_atlas_index(RID p_instance) = 0;
 	virtual bool reflection_probe_instance_needs_redraw(RID p_instance) = 0;
 	virtual bool reflection_probe_instance_has_reflection(RID p_instance) = 0;