From 470b4cb62f543263f0087a1e9d39b4b125752d23 Mon Sep 17 00:00:00 2001 From: Mai Lavelle Date: Thu, 16 Nov 2017 03:32:02 -0500 Subject: [PATCH 1/2] Cycles: Fix crash with split branched path tracing ShaderData memory was getting clobbered in the branched path code paths. Was caused by 087331c495b04ebd37903c0dc0e46262354cf026 --- intern/cycles/kernel/split/kernel_branched.h | 21 ++++++++++++------- .../split/kernel_next_iteration_setup.h | 4 ++-- .../cycles/kernel/split/kernel_split_data.h | 5 +++-- .../kernel/split/kernel_split_data_types.h | 6 ++---- .../kernel/split/kernel_subsurface_scatter.h | 2 +- 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/intern/cycles/kernel/split/kernel_branched.h b/intern/cycles/kernel/split/kernel_branched.h index 6456636caaa..368a4395760 100644 --- a/intern/cycles/kernel/split/kernel_branched.h +++ b/intern/cycles/kernel/split/kernel_branched.h @@ -33,9 +33,9 @@ ccl_device_inline void kernel_split_branched_path_indirect_loop_init(KernelGloba BRANCHED_STORE(isect); BRANCHED_STORE(ray_state); - branched_state->sd = *kernel_split_sd(sd, ray_index); - for(int i = 0; i < branched_state->sd.num_closure; i++) { - branched_state->sd.closure[i] = kernel_split_sd(sd, ray_index)->closure[i]; + *kernel_split_sd(branched_state_sd, ray_index) = *kernel_split_sd(sd, ray_index); + for(int i = 0; i < kernel_split_sd(branched_state_sd, ray_index)->num_closure; i++) { + kernel_split_sd(branched_state_sd, ray_index)->closure[i] = kernel_split_sd(sd, ray_index)->closure[i]; } #undef BRANCHED_STORE @@ -60,9 +60,9 @@ ccl_device_inline void kernel_split_branched_path_indirect_loop_end(KernelGlobal BRANCHED_RESTORE(isect); BRANCHED_RESTORE(ray_state); - *kernel_split_sd(sd, ray_index) = branched_state->sd; - for(int i = 0; i < branched_state->sd.num_closure; i++) { - kernel_split_sd(sd, ray_index)->closure[i] = branched_state->sd.closure[i]; + *kernel_split_sd(sd, ray_index) = *kernel_split_sd(branched_state_sd, ray_index); + for(int i = 0; i < kernel_split_sd(branched_state_sd, ray_index)->num_closure; i++) { + kernel_split_sd(sd, ray_index)->closure[i] = kernel_split_sd(branched_state_sd, ray_index)->closure[i]; } #undef BRANCHED_RESTORE @@ -83,10 +83,17 @@ ccl_device_inline bool kernel_split_branched_indirect_start_shared(KernelGlobals } #define SPLIT_DATA_ENTRY(type, name, num) \ - kernel_split_state.name[inactive_ray] = kernel_split_state.name[ray_index]; + if(num) { \ + kernel_split_state.name[inactive_ray] = kernel_split_state.name[ray_index]; \ + } SPLIT_DATA_ENTRIES_BRANCHED_SHARED #undef SPLIT_DATA_ENTRY + *kernel_split_sd(sd, inactive_ray) = *kernel_split_sd(sd, ray_index); + for(int i = 0; i < kernel_split_sd(sd, ray_index)->num_closure; i++) { + kernel_split_sd(sd, inactive_ray)->closure[i] = kernel_split_sd(sd, ray_index)->closure[i]; + } + kernel_split_state.branched_state[inactive_ray].shared_sample_count = 0; kernel_split_state.branched_state[inactive_ray].original_ray = ray_index; kernel_split_state.branched_state[inactive_ray].waiting_on_shared_samples = false; diff --git a/intern/cycles/kernel/split/kernel_next_iteration_setup.h b/intern/cycles/kernel/split/kernel_next_iteration_setup.h index bb6bf1cc7e6..75a0af7567b 100644 --- a/intern/cycles/kernel/split/kernel_next_iteration_setup.h +++ b/intern/cycles/kernel/split/kernel_next_iteration_setup.h @@ -145,7 +145,7 @@ ccl_device void kernel_next_iteration_setup(KernelGlobals *kg, if(kernel_split_branched_path_surface_indirect_light_iter(kg, ray_index, 1.0f, - &kernel_split_state.branched_state[ray_index].sd, + kernel_split_sd(branched_state_sd, ray_index), true, true)) { @@ -190,7 +190,7 @@ ccl_device void kernel_next_iteration_setup(KernelGlobals *kg, if(kernel_split_branched_path_surface_indirect_light_iter(kg, ray_index, 1.0f, - &kernel_split_state.branched_state[ray_index].sd, + kernel_split_sd(branched_state_sd, ray_index), true, true)) { diff --git a/intern/cycles/kernel/split/kernel_split_data.h b/intern/cycles/kernel/split/kernel_split_data.h index fa2f0b20a83..9297e1e0ad5 100644 --- a/intern/cycles/kernel/split/kernel_split_data.h +++ b/intern/cycles/kernel/split/kernel_split_data.h @@ -34,7 +34,7 @@ ccl_device_inline uint64_t split_data_buffer_size(KernelGlobals *kg, size_t num_ uint64_t closure_size = sizeof(ShaderClosure) * (kernel_data.integrator.max_closures-1); #ifdef __BRANCHED_PATH__ - size += align_up(closure_size * num_elements, 16); + size += align_up(num_elements * (sizeof(ShaderData) + closure_size), 16); #endif size += align_up(num_elements * (sizeof(ShaderData) + closure_size), 16); @@ -60,7 +60,8 @@ ccl_device_inline void split_data_init(KernelGlobals *kg, uint64_t closure_size = sizeof(ShaderClosure) * (kernel_data.integrator.max_closures-1); #ifdef __BRANCHED_PATH__ - p += align_up(closure_size * num_elements, 16); + split_data->_branched_state_sd = (ShaderData*)p; + p += align_up(num_elements * (sizeof(ShaderData) + closure_size), 16); #endif split_data->_sd = (ShaderData*)p; diff --git a/intern/cycles/kernel/split/kernel_split_data_types.h b/intern/cycles/kernel/split/kernel_split_data_types.h index 9ac3f904819..5f40fdc9240 100644 --- a/intern/cycles/kernel/split/kernel_split_data_types.h +++ b/intern/cycles/kernel/split/kernel_split_data_types.h @@ -76,13 +76,11 @@ typedef ccl_global struct SplitBranchedState { int shared_sample_count; /* number of branched samples shared with other threads */ int original_ray; /* index of original ray when sharing branched samples */ bool waiting_on_shared_samples; - - /* Must be last in to allow for dynamic size of closures */ - struct ShaderData sd; } SplitBranchedState; #define SPLIT_DATA_BRANCHED_ENTRIES \ - SPLIT_DATA_ENTRY( SplitBranchedState, branched_state, 1) + SPLIT_DATA_ENTRY( SplitBranchedState, branched_state, 1) \ + SPLIT_DATA_ENTRY(ShaderData, _branched_state_sd, 0) #else #define SPLIT_DATA_BRANCHED_ENTRIES #endif /* __BRANCHED_PATH__ */ diff --git a/intern/cycles/kernel/split/kernel_subsurface_scatter.h b/intern/cycles/kernel/split/kernel_subsurface_scatter.h index 887c3e313d1..5bf7483e9a2 100644 --- a/intern/cycles/kernel/split/kernel_subsurface_scatter.h +++ b/intern/cycles/kernel/split/kernel_subsurface_scatter.h @@ -37,7 +37,7 @@ ccl_device_noinline bool kernel_split_branched_path_subsurface_indirect_light_it { SplitBranchedState *branched_state = &kernel_split_state.branched_state[ray_index]; - ShaderData *sd = &branched_state->sd; + ShaderData *sd = kernel_split_sd(branched_state_sd, ray_index); PathRadiance *L = &kernel_split_state.path_radiance[ray_index]; ShaderData *emission_sd = AS_SHADER_DATA(&kernel_split_state.sd_DL_shadow[ray_index]); From d697e3d46ea9829c150d94411312e4cac434e4c0 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Thu, 16 Nov 2017 12:49:31 +0100 Subject: [PATCH 2/2] BLI listbase: add bytes finding helpers. Quite similar to string ones actually, except more generic. Used in id_override_static branch currently. --- source/blender/blenlib/BLI_listbase.h | 2 ++ source/blender/blenlib/intern/listbase.c | 40 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/source/blender/blenlib/BLI_listbase.h b/source/blender/blenlib/BLI_listbase.h index c1e28d5ebc3..1e931e6f0d7 100644 --- a/source/blender/blenlib/BLI_listbase.h +++ b/source/blender/blenlib/BLI_listbase.h @@ -50,12 +50,14 @@ void *BLI_findlink(const struct ListBase *listbase, int number) ATTR_WARN_UNUSED void *BLI_findstring(const struct ListBase *listbase, const char *id, const int offset) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1); void *BLI_findstring_ptr(const struct ListBase *listbase, const char *id, const int offset) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1); void *BLI_findptr(const struct ListBase *listbase, const void *ptr, const int offset) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1); +void *BLI_listbase_bytes_find(const ListBase *listbase, const void *bytes, const size_t bytes_size, const int offset) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1, 2); /* find backwards */ void *BLI_rfindlink(const struct ListBase *listbase, int number) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1); void *BLI_rfindstring(const struct ListBase *listbase, const char *id, const int offset) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1); void *BLI_rfindstring_ptr(const struct ListBase *listbase, const char *id, const int offset) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1); void *BLI_rfindptr(const struct ListBase *listbase, const void *ptr, const int offset) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1); +void *BLI_listbase_bytes_rfind(const ListBase *listbase, const void *bytes, const size_t bytes_size, const int offset) ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(1, 2); void BLI_freelistN(struct ListBase *listbase) ATTR_NONNULL(1); void BLI_addtail(struct ListBase *listbase, void *vlink) ATTR_NONNULL(1); diff --git a/source/blender/blenlib/intern/listbase.c b/source/blender/blenlib/intern/listbase.c index 0a6d575c7d6..d2cf0cf49a2 100644 --- a/source/blender/blenlib/intern/listbase.c +++ b/source/blender/blenlib/intern/listbase.c @@ -691,6 +691,46 @@ void *BLI_rfindptr(const ListBase *listbase, const void *ptr, const int offset) return NULL; } +/** + * Finds the first element of listbase which contains the specified bytes + * at the specified offset, returning NULL if not found. + */ +void *BLI_listbase_bytes_find(const ListBase *listbase, const void *bytes, const size_t bytes_size, const int offset) +{ + Link *link = NULL; + const void *ptr_iter; + + for (link = listbase->first; link; link = link->next) { + ptr_iter = (const void *)(((const char *)link) + offset); + + if (memcmp(bytes, ptr_iter, bytes_size) == 0) { + return link; + } + } + + return NULL; +} +/* same as above but find reverse */ +/** + * Finds the last element of listbase which contains the specified bytes + * at the specified offset, returning NULL if not found. + */ +void *BLI_listbase_bytes_rfind(const ListBase *listbase, const void *bytes, const size_t bytes_size, const int offset) +{ + Link *link = NULL; + const void *ptr_iter; + + for (link = listbase->last; link; link = link->prev) { + ptr_iter = (const void *)(((const char *)link) + offset); + + if (memcmp(bytes, ptr_iter, bytes_size) == 0) { + return link; + } + } + + return NULL; +} + /** * Returns the 0-based index of the first element of listbase which contains the specified * null-terminated string at the specified offset, or -1 if not found.