Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix never ending loop with overlapping probes #89134

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions drivers/gles3/storage/light_storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
}

Expand Down
1 change: 1 addition & 0 deletions drivers/gles3/storage/light_storage.h
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,7 @@ class LightStorage : public RendererLightStorage {
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;
Expand Down
1 change: 1 addition & 0 deletions servers/rendering/dummy/storage/light_storage.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ class LightStorage : public RendererLightStorage {
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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
BastiaanOlij marked this conversation as resolved.
Show resolved Hide resolved

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);

Expand Down
22 changes: 20 additions & 2 deletions servers/rendering/renderer_rd/storage_rd/light_storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
}
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion servers/rendering/renderer_rd/storage_rd/light_storage.h
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,6 @@ class LightStorage : public RendererLightStorage {
int processing_layer = 1;
int processing_side = 0;

uint32_t render_step = 0;
uint64_t last_pass = 0;
uint32_t cull_mask = 0;

Expand Down Expand Up @@ -848,6 +847,7 @@ class LightStorage : public RendererLightStorage {
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;
Expand Down
17 changes: 14 additions & 3 deletions servers/rendering/renderer_scene_cull.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;

Expand All @@ -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++;
}
Expand All @@ -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();
Expand Down
1 change: 1 addition & 0 deletions servers/rendering/storage/light_storage.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ class RendererLightStorage {
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;
Expand Down
Loading