From 623a050269923dc15551bd3ab487865b767b5b15 Mon Sep 17 00:00:00 2001 From: Clay Date: Tue, 17 Oct 2023 13:02:46 +0200 Subject: [PATCH] Ensure that only visible paired lights are used This is a longstanding issue in both the Mobile and GL Compatibility renderer. Meshes pair with all lights that touch them, and then at draw time, we send all paired lights indices to the shader (even if that light isn't visible). The problem is that non-visible lights aren't uploaded to the GPU and don't have an index. So we end up using a bogus index --- drivers/gles3/rasterizer_scene_gles3.cpp | 10 ++ .../forward_mobile/render_forward_mobile.cpp | 104 +++++++++++------- .../forward_mobile/render_forward_mobile.h | 3 +- .../storage_rd/forward_id_storage.h | 2 +- .../renderer_rd/storage_rd/light_storage.cpp | 4 +- .../storage_rd/texture_storage.cpp | 2 +- 6 files changed, 83 insertions(+), 42 deletions(-) diff --git a/drivers/gles3/rasterizer_scene_gles3.cpp b/drivers/gles3/rasterizer_scene_gles3.cpp index fc08f1cf38b..1f10f4b3539 100644 --- a/drivers/gles3/rasterizer_scene_gles3.cpp +++ b/drivers/gles3/rasterizer_scene_gles3.cpp @@ -1254,9 +1254,14 @@ void RasterizerSceneGLES3::_fill_render_list(RenderListType p_render_list, const inst->light_passes.clear(); inst->spot_light_gl_cache.clear(); inst->omni_light_gl_cache.clear(); + uint64_t current_frame = RSG::rasterizer->get_frame_number(); + if (inst->paired_omni_light_count) { for (uint32_t j = 0; j < inst->paired_omni_light_count; j++) { RID light_instance = inst->paired_omni_lights[j]; + if (GLES3::LightStorage::get_singleton()->light_instance_get_render_pass(light_instance) != current_frame) { + continue; + } RID light = GLES3::LightStorage::get_singleton()->light_instance_get_base_light(light_instance); int32_t shadow_id = GLES3::LightStorage::get_singleton()->light_instance_get_shadow_id(light_instance); @@ -1277,6 +1282,9 @@ void RasterizerSceneGLES3::_fill_render_list(RenderListType p_render_list, const if (inst->paired_spot_light_count) { for (uint32_t j = 0; j < inst->paired_spot_light_count; j++) { RID light_instance = inst->paired_spot_lights[j]; + if (GLES3::LightStorage::get_singleton()->light_instance_get_render_pass(light_instance) != current_frame) { + continue; + } RID light = GLES3::LightStorage::get_singleton()->light_instance_get_base_light(light_instance); int32_t shadow_id = GLES3::LightStorage::get_singleton()->light_instance_get_shadow_id(light_instance); @@ -1712,6 +1720,8 @@ void RasterizerSceneGLES3::_setup_lights(const RenderDataGLES3 *p_render_data, b r_spot_light_count++; } break; } + + li->last_pass = RSG::rasterizer->get_frame_number(); } if (r_omni_light_count) { diff --git a/servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.cpp b/servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.cpp index 8a672d8628f..9e8d654b4ef 100644 --- a/servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.cpp +++ b/servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.cpp @@ -53,6 +53,7 @@ RendererRD::ForwardID RenderForwardMobile::ForwardIDStorageMobile::allocate_forw index = forward_id_allocators[p_type].allocations.size(); forward_id_allocators[p_type].allocations.push_back(true); forward_id_allocators[p_type].map.push_back(0xFF); + forward_id_allocators[p_type].last_pass.push_back(0); } else { forward_id_allocators[p_type].allocations[index] = true; } @@ -64,44 +65,72 @@ void RenderForwardMobile::ForwardIDStorageMobile::free_forward_id(RendererRD::Fo forward_id_allocators[p_type].allocations[p_id] = false; } -void RenderForwardMobile::ForwardIDStorageMobile::map_forward_id(RendererRD::ForwardIDType p_type, RendererRD::ForwardID p_id, uint32_t p_index) { +void RenderForwardMobile::ForwardIDStorageMobile::map_forward_id(RendererRD::ForwardIDType p_type, RendererRD::ForwardID p_id, uint32_t p_index, uint64_t p_last_pass) { forward_id_allocators[p_type].map[p_id] = p_index; + forward_id_allocators[p_type].last_pass[p_id] = p_last_pass; } void RenderForwardMobile::fill_push_constant_instance_indices(SceneState::InstanceData *p_instance_data, const GeometryInstanceForwardMobile *p_instance) { - // First zero out our indices. + uint64_t current_frame = RSG::rasterizer->get_frame_number(); p_instance_data->omni_lights[0] = 0xFFFFFFFF; p_instance_data->omni_lights[1] = 0xFFFFFFFF; + uint32_t idx = 0; + for (uint32_t i = 0; i < p_instance->omni_light_count; i++) { + uint32_t ofs = idx < 4 ? 0 : 1; + uint32_t shift = (idx & 0x3) << 3; + uint32_t mask = ~(0xFF << shift); + + if (forward_id_storage_mobile->forward_id_allocators[RendererRD::FORWARD_ID_TYPE_OMNI_LIGHT].last_pass[p_instance->omni_lights[i]] == current_frame) { + p_instance_data->omni_lights[ofs] &= mask; + p_instance_data->omni_lights[ofs] |= uint32_t(forward_id_storage_mobile->forward_id_allocators[RendererRD::FORWARD_ID_TYPE_OMNI_LIGHT].map[p_instance->omni_lights[i]]) << shift; + idx++; + } + } + p_instance_data->spot_lights[0] = 0xFFFFFFFF; p_instance_data->spot_lights[1] = 0xFFFFFFFF; + idx = 0; + for (uint32_t i = 0; i < p_instance->spot_light_count; i++) { + uint32_t ofs = idx < 4 ? 0 : 1; + uint32_t shift = (idx & 0x3) << 3; + uint32_t mask = ~(0xFF << shift); + if (forward_id_storage_mobile->forward_id_allocators[RendererRD::FORWARD_ID_TYPE_SPOT_LIGHT].last_pass[p_instance->spot_lights[i]] == current_frame) { + p_instance_data->spot_lights[ofs] &= mask; + p_instance_data->spot_lights[ofs] |= uint32_t(forward_id_storage_mobile->forward_id_allocators[RendererRD::FORWARD_ID_TYPE_SPOT_LIGHT].map[p_instance->spot_lights[i]]) << shift; + idx++; + } + } + p_instance_data->decals[0] = 0xFFFFFFFF; p_instance_data->decals[1] = 0xFFFFFFFF; + idx = 0; + for (uint32_t i = 0; i < p_instance->decals_count; i++) { + uint32_t ofs = idx < 4 ? 0 : 1; + uint32_t shift = (idx & 0x3) << 3; + uint32_t mask = ~(0xFF << shift); + if (forward_id_storage_mobile->forward_id_allocators[RendererRD::FORWARD_ID_TYPE_DECAL].last_pass[p_instance->decals[i]] == current_frame) { + p_instance_data->decals[ofs] &= mask; + p_instance_data->decals[ofs] |= uint32_t(forward_id_storage_mobile->forward_id_allocators[RendererRD::FORWARD_ID_TYPE_DECAL].map[p_instance->decals[i]]) << shift; + idx++; + } + } + p_instance_data->reflection_probes[0] = 0xFFFFFFFF; p_instance_data->reflection_probes[1] = 0xFFFFFFFF; - for (uint32_t i = 0; i < MAX_RDL_CULL; i++) { - uint32_t ofs = i < 4 ? 0 : 1; - uint32_t shift = (i & 0x3) << 3; + idx = 0; + for (uint32_t i = 0; i < p_instance->reflection_probe_count; i++) { + uint32_t ofs = idx < 4 ? 0 : 1; + uint32_t shift = (idx & 0x3) << 3; uint32_t mask = ~(0xFF << shift); - if (i < p_instance->omni_light_count) { - p_instance_data->omni_lights[ofs] &= mask; - p_instance_data->omni_lights[ofs] |= uint32_t(forward_id_storage_mobile->forward_id_allocators[RendererRD::FORWARD_ID_TYPE_OMNI_LIGHT].map[p_instance->omni_lights[i]]) << shift; - } - if (i < p_instance->spot_light_count) { - p_instance_data->spot_lights[ofs] &= mask; - p_instance_data->spot_lights[ofs] |= uint32_t(forward_id_storage_mobile->forward_id_allocators[RendererRD::FORWARD_ID_TYPE_SPOT_LIGHT].map[p_instance->spot_lights[i]]) << shift; - } - if (i < p_instance->decals_count) { - p_instance_data->decals[ofs] &= mask; - p_instance_data->decals[ofs] |= uint32_t(forward_id_storage_mobile->forward_id_allocators[RendererRD::FORWARD_ID_TYPE_DECAL].map[p_instance->decals[i]]) << shift; - } - if (i < p_instance->reflection_probe_count) { + if (forward_id_storage_mobile->forward_id_allocators[RendererRD::FORWARD_ID_TYPE_REFLECTION_PROBE].last_pass[p_instance->reflection_probes[i]] == current_frame) { p_instance_data->reflection_probes[ofs] &= mask; p_instance_data->reflection_probes[ofs] |= uint32_t(forward_id_storage_mobile->forward_id_allocators[RendererRD::FORWARD_ID_TYPE_REFLECTION_PROBE].map[p_instance->reflection_probes[i]]) << shift; + idx++; } } } @@ -539,7 +568,6 @@ void RenderForwardMobile::_setup_lightmaps(const RenderDataRD *p_render_data, co void RenderForwardMobile::_pre_opaque_render(RenderDataRD *p_render_data) { RendererRD::LightStorage *light_storage = RendererRD::LightStorage::get_singleton(); - RendererRD::TextureStorage *texture_storage = RendererRD::TextureStorage::get_singleton(); p_render_data->cube_shadows.clear(); p_render_data->shadows.clear(); @@ -598,28 +626,11 @@ void RenderForwardMobile::_pre_opaque_render(RenderDataRD *p_render_data) { //full barrier here, we need raster, transfer and compute and it depends from the previous work RD::get_singleton()->barrier(RD::BARRIER_MASK_ALL_BARRIERS, RD::BARRIER_MASK_ALL_BARRIERS); - - bool using_shadows = true; - - if (p_render_data->reflection_probe.is_valid()) { - if (!RSG::light_storage->reflection_probe_renders_shadows(light_storage->reflection_probe_instance_get_probe(p_render_data->reflection_probe))) { - using_shadows = false; - } - } else { - //do not render reflections when rendering a reflection probe - light_storage->update_reflection_probe_buffer(p_render_data, *p_render_data->reflection_probes, p_render_data->scene_data->cam_transform.affine_inverse(), p_render_data->environment); - } - - uint32_t directional_light_count = 0; - uint32_t positional_light_count = 0; - light_storage->update_light_buffers(p_render_data, *p_render_data->lights, p_render_data->scene_data->cam_transform, p_render_data->shadow_atlas, using_shadows, directional_light_count, positional_light_count, p_render_data->directional_light_soft_shadows); - texture_storage->update_decal_buffer(*p_render_data->decals, p_render_data->scene_data->cam_transform); - - p_render_data->directional_light_count = directional_light_count; } void RenderForwardMobile::_render_scene(RenderDataRD *p_render_data, const Color &p_default_bg_color) { RendererRD::LightStorage *light_storage = RendererRD::LightStorage::get_singleton(); + RendererRD::TextureStorage *texture_storage = RendererRD::TextureStorage::get_singleton(); ERR_FAIL_NULL(p_render_data); @@ -668,6 +679,25 @@ void RenderForwardMobile::_render_scene(RenderDataRD *p_render_data, const Color bool using_subpass_transparent = true; bool using_subpass_post_process = true; + bool using_shadows = true; + + if (p_render_data->reflection_probe.is_valid()) { + if (!RSG::light_storage->reflection_probe_renders_shadows(light_storage->reflection_probe_instance_get_probe(p_render_data->reflection_probe))) { + using_shadows = false; + } + } else { + //do not render reflections when rendering a reflection probe + light_storage->update_reflection_probe_buffer(p_render_data, *p_render_data->reflection_probes, p_render_data->scene_data->cam_transform.affine_inverse(), p_render_data->environment); + } + + // Update light and decal buffer first so we know what lights and decals are safe to pair with. + uint32_t directional_light_count = 0; + uint32_t positional_light_count = 0; + light_storage->update_light_buffers(p_render_data, *p_render_data->lights, p_render_data->scene_data->cam_transform, p_render_data->shadow_atlas, using_shadows, directional_light_count, positional_light_count, p_render_data->directional_light_soft_shadows); + texture_storage->update_decal_buffer(*p_render_data->decals, p_render_data->scene_data->cam_transform); + + p_render_data->directional_light_count = directional_light_count; + // fill our render lists early so we can find out if we use various features _fill_render_list(RENDER_LIST_OPAQUE, p_render_data, PASS_MODE_COLOR); render_list[RENDER_LIST_OPAQUE].sort_by_key(); diff --git a/servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.h b/servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.h index 50bf83b6129..c8e42e2cf1d 100644 --- a/servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.h +++ b/servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.h @@ -535,6 +535,7 @@ protected: struct ForwardIDAllocator { LocalVector allocations; LocalVector map; + LocalVector last_pass; }; ForwardIDAllocator forward_id_allocators[RendererRD::FORWARD_ID_MAX]; @@ -542,7 +543,7 @@ protected: public: virtual RendererRD::ForwardID allocate_forward_id(RendererRD::ForwardIDType p_type) override; virtual void free_forward_id(RendererRD::ForwardIDType p_type, RendererRD::ForwardID p_id) override; - virtual void map_forward_id(RendererRD::ForwardIDType p_type, RendererRD::ForwardID p_id, uint32_t p_index) override; + virtual void map_forward_id(RendererRD::ForwardIDType p_type, RendererRD::ForwardID p_id, uint32_t p_index, uint64_t p_last_pass) override; virtual bool uses_forward_ids() const override { return true; } }; diff --git a/servers/rendering/renderer_rd/storage_rd/forward_id_storage.h b/servers/rendering/renderer_rd/storage_rd/forward_id_storage.h index bedf5e80c78..c8f8d4f7f2d 100644 --- a/servers/rendering/renderer_rd/storage_rd/forward_id_storage.h +++ b/servers/rendering/renderer_rd/storage_rd/forward_id_storage.h @@ -59,7 +59,7 @@ public: virtual RendererRD::ForwardID allocate_forward_id(RendererRD::ForwardIDType p_type) { return -1; } virtual void free_forward_id(RendererRD::ForwardIDType p_type, RendererRD::ForwardID p_id) {} - virtual void map_forward_id(RendererRD::ForwardIDType p_type, RendererRD::ForwardID p_id, uint32_t p_index) {} + virtual void map_forward_id(RendererRD::ForwardIDType p_type, RendererRD::ForwardID p_id, uint32_t p_index, uint64_t p_last_pass) {} virtual bool uses_forward_ids() const { return false; } }; diff --git a/servers/rendering/renderer_rd/storage_rd/light_storage.cpp b/servers/rendering/renderer_rd/storage_rd/light_storage.cpp index 4fd33ad71ac..1f6d1021f49 100644 --- a/servers/rendering/renderer_rd/storage_rd/light_storage.cpp +++ b/servers/rendering/renderer_rd/storage_rd/light_storage.cpp @@ -793,7 +793,7 @@ void LightStorage::update_light_buffers(RenderDataRD *p_render_data, const Paged real_t distance = (i < omni_light_count) ? omni_light_sort[index].depth : spot_light_sort[index].depth; if (using_forward_ids) { - forward_id_storage->map_forward_id(type == RS::LIGHT_OMNI ? RendererRD::FORWARD_ID_TYPE_OMNI_LIGHT : RendererRD::FORWARD_ID_TYPE_SPOT_LIGHT, light_instance->forward_id, index); + forward_id_storage->map_forward_id(type == RS::LIGHT_OMNI ? RendererRD::FORWARD_ID_TYPE_OMNI_LIGHT : RendererRD::FORWARD_ID_TYPE_SPOT_LIGHT, light_instance->forward_id, index, light_instance->last_pass); } Transform3D light_transform = light_instance->transform; @@ -1670,7 +1670,7 @@ void LightStorage::update_reflection_probe_buffer(RenderDataRD *p_render_data, c ReflectionProbeInstance *rpi = reflection_sort[i].probe_instance; if (using_forward_ids) { - forward_id_storage->map_forward_id(FORWARD_ID_TYPE_REFLECTION_PROBE, rpi->forward_id, i); + forward_id_storage->map_forward_id(FORWARD_ID_TYPE_REFLECTION_PROBE, rpi->forward_id, i, rpi->last_pass); } ReflectionProbe *probe = reflection_probe_owner.get_or_null(rpi->probe); diff --git a/servers/rendering/renderer_rd/storage_rd/texture_storage.cpp b/servers/rendering/renderer_rd/storage_rd/texture_storage.cpp index 166b8508645..474ec897ef2 100644 --- a/servers/rendering/renderer_rd/storage_rd/texture_storage.cpp +++ b/servers/rendering/renderer_rd/storage_rd/texture_storage.cpp @@ -2862,7 +2862,7 @@ void TextureStorage::update_decal_buffer(const PagedArray &p_decals, const Decal *decal = decal_sort[i].decal; if (using_forward_ids) { - forward_id_storage->map_forward_id(FORWARD_ID_TYPE_DECAL, decal_instance->forward_id, i); + forward_id_storage->map_forward_id(FORWARD_ID_TYPE_DECAL, decal_instance->forward_id, i, RSG::rasterizer->get_frame_number()); } decal_instance->cull_mask = decal->cull_mask;