From a5d3d23db4573fbf52b115aad6b0f20e93f5087b Mon Sep 17 00:00:00 2001 From: Bastiaan Olij Date: Mon, 4 Mar 2024 20:07:01 +1100 Subject: [PATCH] Fix never ending loop with overlapping probes --- drivers/gles3/storage/light_storage.cpp | 4 ++++ drivers/gles3/storage/light_storage.h | 1 + .../rendering/dummy/storage/light_storage.h | 1 + .../render_forward_clustered.cpp | 13 ++++++----- .../renderer_rd/storage_rd/light_storage.cpp | 22 +++++++++++++++++-- .../renderer_rd/storage_rd/light_storage.h | 2 +- servers/rendering/renderer_scene_cull.cpp | 17 +++++++++++--- servers/rendering/storage/light_storage.h | 1 + 8 files changed, 49 insertions(+), 12 deletions(-) diff --git a/drivers/gles3/storage/light_storage.cpp b/drivers/gles3/storage/light_storage.cpp index 2259c61e5bf..f5d1f8dabde 100644 --- a/drivers/gles3/storage/light_storage.cpp +++ b/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) { } diff --git a/drivers/gles3/storage/light_storage.h b/drivers/gles3/storage/light_storage.h index a6b236f3ece..51c5c481066 100644 --- a/drivers/gles3/storage/light_storage.h +++ b/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; diff --git a/servers/rendering/dummy/storage/light_storage.h b/servers/rendering/dummy/storage/light_storage.h index 61a825f8c58..a76305cdaa7 100644 --- a/servers/rendering/dummy/storage/light_storage.h +++ b/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; } diff --git a/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp b/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp index fa6e78750bc..33bb5459f20 100644 --- a/servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.cpp +++ b/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); diff --git a/servers/rendering/renderer_rd/storage_rd/light_storage.cpp b/servers/rendering/renderer_rd/storage_rd/light_storage.cpp index 2786af65ebc..cf8c29e6249 100644 --- a/servers/rendering/renderer_rd/storage_rd/light_storage.cpp +++ b/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; } diff --git a/servers/rendering/renderer_rd/storage_rd/light_storage.h b/servers/rendering/renderer_rd/storage_rd/light_storage.h index b3d27cc6ede..b3d6bf5254d 100644 --- a/servers/rendering/renderer_rd/storage_rd/light_storage.h +++ b/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; diff --git a/servers/rendering/renderer_scene_cull.cpp b/servers/rendering/renderer_scene_cull.cpp index 59d70958f15..4e5539e6a40 100644 --- a/servers/rendering/renderer_scene_cull.cpp +++ b/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 *ref_probe = reflection_probe_render_list.first(); + Vector *> 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 *rp : done_list) { + reflection_probe_render_list.remove(rp); + } + /* VOXEL GIS */ SelfList *voxel_gi = voxel_gi_update_list.first(); diff --git a/servers/rendering/storage/light_storage.h b/servers/rendering/storage/light_storage.h index 27407305d16..d439598f3d7 100644 --- a/servers/rendering/storage/light_storage.h +++ b/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;