From 8e5840359f116a8971e54caa03e23bdeaadd3825 Mon Sep 17 00:00:00 2001 From: Weizhen Huang Date: Sat, 13 Apr 2024 11:28:09 +0200 Subject: [PATCH] Refactor: deduplicate assignment of background light group by ensuring `KernelLight.lightgroup` is properly assigned in `device_update_light()`. The value is later retrieved via `lamp_lightgroup(kg, lamp)`. `light_manager->device_update()` is called after `background->device_update()`, so the background light group should already been assigned when `device_update_lights()` is called. Pull Request: https://projects.blender.org/blender/blender/pulls/120714 --- .../kernel/integrator/shade_dedicated_light.h | 5 +---- intern/cycles/kernel/integrator/shade_surface.h | 7 +------ intern/cycles/kernel/integrator/shade_volume.h | 5 +---- intern/cycles/scene/light.cpp | 14 ++++++++++---- 4 files changed, 13 insertions(+), 18 deletions(-) diff --git a/intern/cycles/kernel/integrator/shade_dedicated_light.h b/intern/cycles/kernel/integrator/shade_dedicated_light.h index 9f2259faccb..7c7841d5d02 100644 --- a/intern/cycles/kernel/integrator/shade_dedicated_light.h +++ b/intern/cycles/kernel/integrator/shade_dedicated_light.h @@ -114,10 +114,7 @@ ccl_device bool shadow_linking_shade_light(KernelGlobals kg, bsdf_spectrum = light_eval * mis_weight * INTEGRATOR_STATE(state, shadow_link, dedicated_light_weight); - - // TODO(: De-duplicate with the shade_surface. - // Possibly by ensuring ls->group is always assigned properly. - light_group = ls.type != LIGHT_BACKGROUND ? ls.group : kernel_data.background.lightgroup; + light_group = ls.group; return true; } diff --git a/intern/cycles/kernel/integrator/shade_surface.h b/intern/cycles/kernel/integrator/shade_surface.h index a2cb97b8fb6..6415d2e5e20 100644 --- a/intern/cycles/kernel/integrator/shade_surface.h +++ b/intern/cycles/kernel/integrator/shade_surface.h @@ -345,13 +345,8 @@ ccl_device } /* Branch off shadow kernel. */ - - // TODO(: De-duplicate with the shade_Dedicated_light. - // Possibly by ensuring ls->group is always assigned properly. - const int light_group = ls.type != LIGHT_BACKGROUND ? ls.group : - kernel_data.background.lightgroup; IntegratorShadowState shadow_state = integrate_direct_light_shadow_init_common( - kg, state, &ray, bsdf_eval_sum(&bsdf_eval), light_group, mnee_vertex_count); + kg, state, &ray, bsdf_eval_sum(&bsdf_eval), ls.group, mnee_vertex_count); if (is_transmission) { #ifdef __VOLUME__ diff --git a/intern/cycles/kernel/integrator/shade_volume.h b/intern/cycles/kernel/integrator/shade_volume.h index 9caa9e9b1c0..e77a69ea94c 100644 --- a/intern/cycles/kernel/integrator/shade_volume.h +++ b/intern/cycles/kernel/integrator/shade_volume.h @@ -866,10 +866,7 @@ ccl_device_forceinline void integrate_volume_direct_light( INTEGRATOR_STATE_WRITE(shadow_state, shadow_path, throughput) = throughput_phase; /* Write Light-group, +1 as light-group is int but we need to encode into a uint8_t. */ - INTEGRATOR_STATE_WRITE( - shadow_state, shadow_path, lightgroup) = (ls.type != LIGHT_BACKGROUND) ? - ls.group + 1 : - kernel_data.background.lightgroup + 1; + INTEGRATOR_STATE_WRITE(shadow_state, shadow_path, lightgroup) = ls.group + 1; # ifdef __PATH_GUIDING__ INTEGRATOR_STATE_WRITE(shadow_state, shadow_path, unlit_throughput) = unlit_throughput; diff --git a/intern/cycles/scene/light.cpp b/intern/cycles/scene/light.cpp index ad79f607c41..a415c37ad5a 100644 --- a/intern/cycles/scene/light.cpp +++ b/intern/cycles/scene/light.cpp @@ -1376,12 +1376,18 @@ void LightManager::device_update_lights(Device *device, DeviceScene *dscene, Sce klights[light_index].tfm = light->tfm; klights[light_index].itfm = transform_inverse(light->tfm); - auto it = scene->lightgroups.find(light->lightgroup); - if (it != scene->lightgroups.end()) { - klights[light_index].lightgroup = it->second; + /* Light group. */ + if (light->light_type == LIGHT_BACKGROUND) { + klights[light_index].lightgroup = dscene->data.background.lightgroup; } else { - klights[light_index].lightgroup = LIGHTGROUP_NONE; + auto it = scene->lightgroups.find(light->lightgroup); + if (it != scene->lightgroups.end()) { + klights[light_index].lightgroup = it->second; + } + else { + klights[light_index].lightgroup = LIGHTGROUP_NONE; + } } klights[light_index].light_set_membership = light->light_set_membership;