From d4ceade5eadea48aaab8e2bbf462ba996dab5fa6 Mon Sep 17 00:00:00 2001 From: Weizhen Huang Date: Tue, 6 Aug 2024 15:37:49 +0200 Subject: [PATCH] Fix: Cycles BVH2 and Embree missing some transparent shadow bounces the code snippet is supposed to compute the maximal `isect.t` in the array, which is used to determine if subsequent intersections should be added. However, the previous implementation includes the old `isect.t` which is going to be replaced, resulting an overestimation of `tmax_hits` and thus missing closer intersections. For BVH2, the issue is fixed by computing the `max_t` after a new entry is inserted. For Embree, the issue is fixed by finding the `second_largest_t` as well, and compare that with the new insertion to find the new `max_t`. Pull Request: https://projects.blender.org/blender/blender/pulls/125739 --- intern/cycles/kernel/bvh/shadow_all.h | 52 +++++++++++++-------------- intern/cycles/kernel/device/cpu/bvh.h | 23 ++++++++---- 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/intern/cycles/kernel/bvh/shadow_all.h b/intern/cycles/kernel/bvh/shadow_all.h index 07863539b5d..13754a62a99 100644 --- a/intern/cycles/kernel/bvh/shadow_all.h +++ b/intern/cycles/kernel/bvh/shadow_all.h @@ -62,6 +62,8 @@ ccl_device_inline const float tmax = ray->tmax; float tmax_hits = tmax; + uint isect_index = 0; + *r_num_recorded_hits = 0; *r_throughput = 1.0f; @@ -250,38 +252,32 @@ ccl_device_inline if (record_intersection) { /* Test if we need to record this transparent intersection. */ - const uint max_record_hits = min(max_hits, INTEGRATOR_SHADOW_ISECT_SIZE); - if (*r_num_recorded_hits < max_record_hits || isect.t < tmax_hits) { - /* If maximum number of hits was reached, replace the intersection with the - * highest distance. We want to find the N closest intersections. */ - const uint num_recorded_hits = min(*r_num_recorded_hits, max_record_hits); - uint isect_index = num_recorded_hits; - if (num_recorded_hits + 1 >= max_record_hits) { - float max_t = INTEGRATOR_STATE_ARRAY(state, shadow_isect, 0, t); - uint max_recorded_hit = 0; - - for (uint i = 1; i < num_recorded_hits; ++i) { - const float isect_t = INTEGRATOR_STATE_ARRAY(state, shadow_isect, i, t); - if (isect_t > max_t) { - max_recorded_hit = i; - max_t = isect_t; - } - } - - if (num_recorded_hits >= max_record_hits) { - isect_index = max_recorded_hit; - } - - /* Limit the ray distance and stop counting hits beyond this. */ - tmax_hits = max(isect.t, max_t); - } - - integrator_state_write_shadow_isect(state, &isect, isect_index); - } /* Always increase the number of recorded hits, even beyond the maximum, * so that we can detect this and trace another ray if needed. */ ++(*r_num_recorded_hits); + + const uint max_record_hits = min(max_hits, INTEGRATOR_SHADOW_ISECT_SIZE); + if (*r_num_recorded_hits <= max_record_hits || isect.t < tmax_hits) { + integrator_state_write_shadow_isect(state, &isect, isect_index); + + if (*r_num_recorded_hits >= max_record_hits) { + /* If the maximum number of hits is reached, find the furthest intersection to + replace it with the next closer one. We want N closest intersections. */ + isect_index = 0; + tmax_hits = INTEGRATOR_STATE_ARRAY(state, shadow_isect, 0, t); + for (uint i = 1; i < max_record_hits; ++i) { + const float isect_t = INTEGRATOR_STATE_ARRAY(state, shadow_isect, i, t); + if (isect_t > tmax_hits) { + isect_index = i; + tmax_hits = isect_t; + } + } + } + else { + isect_index = *r_num_recorded_hits; + } + } } } } diff --git a/intern/cycles/kernel/device/cpu/bvh.h b/intern/cycles/kernel/device/cpu/bvh.h index 1d0eb558143..edc387edee8 100644 --- a/intern/cycles/kernel/device/cpu/bvh.h +++ b/intern/cycles/kernel/device/cpu/bvh.h @@ -393,24 +393,32 @@ ccl_device_forceinline void kernel_embree_filter_occluded_shadow_all_func_impl( float max_t = INTEGRATOR_STATE_ARRAY(ctx->isect_s, shadow_isect, 0, t); numhit_t max_recorded_hit = numhit_t(0); + float second_largest_t = 0.0f; for (numhit_t i = numhit_t(1); i < max_record_hits; ++i) { const float isect_t = INTEGRATOR_STATE_ARRAY(ctx->isect_s, shadow_isect, i, t); if (isect_t > max_t) { + second_largest_t = max_t; max_recorded_hit = i; max_t = isect_t; } + else if (isect_t > second_largest_t) { + second_largest_t = isect_t; + } + } + + if (isect_index == max_record_hits && current_isect.t >= max_t) { + /* `ctx->max_t` was initialized to `ray->tmax` before the index exceeds the limit. Now that + * we have looped through the array, we can properly clamp `ctx->max_t`. */ + ctx->max_t = max_t; + return; } isect_index = max_recorded_hit; - /* Limit the ray distance and avoid processing hits beyond this. */ - ctx->max_t = max_t; - - /* If it's further away than max_t, we don't record this transparent intersection. */ - if (current_isect.t >= max_t) { - return; - } + /* After replacing `max_t` with `current_isect.t`, the new largest t would be either + * `current_isect.t` or the second largest t before. */ + ctx->max_t = max(second_largest_t, current_isect.t); } integrator_state_write_shadow_isect(ctx->isect_s, ¤t_isect, isect_index); @@ -870,6 +878,7 @@ ccl_device_intersect bool kernel_embree_intersect_shadow_all(KernelGlobals kg, ctx.opaque_hit = false; ctx.isect_s = state; ctx.max_hits = numhit_t(max_hits); + ctx.max_t = ray->tmax; ctx.ray = ray; RTCRay rtc_ray; kernel_embree_setup_ray(*ray, rtc_ray, visibility);