From e71ca93031d91df2d7a43b00d60b427548625ed0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Foucault?= Date: Tue, 18 Jun 2024 12:39:16 +0200 Subject: [PATCH 1/5] Fix: EEVEE: Missing barrier in light sorting compute shader At high light count, this missing barriers would produce invalid, non-unique `prefix_sum` indices. This then resulted in some slots inside `out_light_buf` never written to, leaving undefined data inside them. If the buffer was cleared to zero, these undefined light slots would be interpreted as sun lights and the shadow setup compute pass would critically fail because of out of bound memory. Fix #123195 --- .../eevee_next/shaders/eevee_light_culling_sort_comp.glsl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source/blender/draw/engines/eevee_next/shaders/eevee_light_culling_sort_comp.glsl b/source/blender/draw/engines/eevee_next/shaders/eevee_light_culling_sort_comp.glsl index 5565bccabb0..04cd43667b1 100644 --- a/source/blender/draw/engines/eevee_next/shaders/eevee_light_culling_sort_comp.glsl +++ b/source/blender/draw/engines/eevee_next/shaders/eevee_light_culling_sort_comp.glsl @@ -55,6 +55,8 @@ void main() } } } + + barrier(); } if (valid_thread) { From 1ab66a6b4fe17131eb9eaa57712b6422eb250ae0 Mon Sep 17 00:00:00 2001 From: Christoph Lendenfeld Date: Tue, 18 Jun 2024 13:54:36 +0200 Subject: [PATCH 2/5] Fix #116138: Hidden bones cannot be active This PR modifies the check for visibility when selected in the outliner. With the change the visibility check only affects the selection state, not the active state. The check has also been swapped with a function call to `ANIM_bone_is_visible_editbone`, so now correctly works with collection visibility. This PR also fixes the same issue for `Bone` and `bPoseChannel` Co-authored-by: Cedric-Hutchings Pull Request: https://projects.blender.org/blender/blender/pulls/123237 --- .../blender/editors/armature/armature_add.cc | 4 - .../blender/editors/armature/armature_edit.cc | 2 - .../editors/armature/armature_select.cc | 1 - .../editors/armature/armature_utils.cc | 11 -- source/blender/editors/armature/pose_edit.cc | 3 - source/blender/editors/include/ED_armature.hh | 1 - .../editors/space_outliner/outliner_select.cc | 156 +++++++++--------- .../editors/space_view3d/view3d_select.cc | 1 - 8 files changed, 76 insertions(+), 103 deletions(-) diff --git a/source/blender/editors/armature/armature_add.cc b/source/blender/editors/armature/armature_add.cc index 14caa6adf8a..2045b6b2113 100644 --- a/source/blender/editors/armature/armature_add.cc +++ b/source/blender/editors/armature/armature_add.cc @@ -1148,8 +1148,6 @@ static int armature_duplicate_selected_exec(bContext *C, wmOperator *op) postEditBoneDuplicate(arm->edbo, ob); - ED_armature_edit_validate_active(arm); - WM_event_add_notifier(C, NC_OBJECT | ND_BONE_SELECT, ob); DEG_id_tag_update(&ob->id, ID_RECALC_SELECT); } @@ -1424,8 +1422,6 @@ static int armature_symmetrize_exec(bContext *C, wmOperator *op) postEditBoneDuplicate(arm->edbo, obedit); - ED_armature_edit_validate_active(arm); - WM_event_add_notifier(C, NC_OBJECT | ND_BONE_SELECT, obedit); DEG_id_tag_update(&obedit->id, ID_RECALC_SELECT); } diff --git a/source/blender/editors/armature/armature_edit.cc b/source/blender/editors/armature/armature_edit.cc index 5cb9cca441c..4e2dfc7d8d0 100644 --- a/source/blender/editors/armature/armature_edit.cc +++ b/source/blender/editors/armature/armature_edit.cc @@ -1523,7 +1523,6 @@ static int armature_hide_exec(bContext *C, wmOperator *op) if (!changed) { continue; } - ED_armature_edit_validate_active(arm); ED_armature_edit_sync_selection(arm->edbo); WM_event_add_notifier(C, NC_OBJECT | ND_BONE_SELECT, obedit); @@ -1581,7 +1580,6 @@ static int armature_reveal_exec(bContext *C, wmOperator *op) } if (changed) { - ED_armature_edit_validate_active(arm); ED_armature_edit_sync_selection(arm->edbo); WM_event_add_notifier(C, NC_OBJECT | ND_BONE_SELECT, obedit); diff --git a/source/blender/editors/armature/armature_select.cc b/source/blender/editors/armature/armature_select.cc index 2a4ef634428..fe30196a345 100644 --- a/source/blender/editors/armature/armature_select.cc +++ b/source/blender/editors/armature/armature_select.cc @@ -1291,7 +1291,6 @@ bool ED_armature_edit_select_op_from_tagged(bArmature *arm, const int sel_op) } ED_armature_edit_sync_selection(arm->edbo); - ED_armature_edit_validate_active(arm); } return changed; diff --git a/source/blender/editors/armature/armature_utils.cc b/source/blender/editors/armature/armature_utils.cc index 87b31ba125f..2ac535d81d6 100644 --- a/source/blender/editors/armature/armature_utils.cc +++ b/source/blender/editors/armature/armature_utils.cc @@ -60,17 +60,6 @@ void ED_armature_edit_sync_selection(ListBase *edbo) } } -void ED_armature_edit_validate_active(bArmature *arm) -{ - EditBone *ebone = arm->act_edbone; - - if (ebone) { - if (ebone->flag & BONE_HIDDEN_A) { - arm->act_edbone = nullptr; - } - } -} - /** \} */ /* -------------------------------------------------------------------- */ diff --git a/source/blender/editors/armature/pose_edit.cc b/source/blender/editors/armature/pose_edit.cc index 9aa40b787be..7914ea16c9b 100644 --- a/source/blender/editors/armature/pose_edit.cc +++ b/source/blender/editors/armature/pose_edit.cc @@ -672,9 +672,6 @@ static int hide_pose_bone_fn(Object *ob, Bone *bone, void *ptr) bone->flag |= BONE_HIDDEN_P; /* only needed when 'hide_select' is true, but harmless. */ bone->flag &= ~BONE_SELECTED; - if (arm->act_bone == bone) { - arm->act_bone = nullptr; - } count += 1; } } diff --git a/source/blender/editors/include/ED_armature.hh b/source/blender/editors/include/ED_armature.hh index 86f729c896a..d1790e686c5 100644 --- a/source/blender/editors/include/ED_armature.hh +++ b/source/blender/editors/include/ED_armature.hh @@ -202,7 +202,6 @@ void ED_armature_undosys_type(UndoType *ut); /** Sync selection to parent for connected children. */ void ED_armature_edit_sync_selection(ListBase *edbo); -void ED_armature_edit_validate_active(bArmature *arm); /** * \param clear_connected: When false caller is responsible for keeping the flag in a valid state. */ diff --git a/source/blender/editors/space_outliner/outliner_select.cc b/source/blender/editors/space_outliner/outliner_select.cc index 7a158ef14d3..012b73d854f 100644 --- a/source/blender/editors/space_outliner/outliner_select.cc +++ b/source/blender/editors/space_outliner/outliner_select.cc @@ -526,47 +526,45 @@ static void tree_element_posechannel_activate(bContext *C, bArmature *arm = static_cast(ob->data); bPoseChannel *pchan = static_cast(te->directdata); - if (!(pchan->bone->flag & BONE_HIDDEN_P)) { - if (set != OL_SETSEL_EXTEND) { - /* Single select forces all other bones to get unselected. */ - const Vector objects = BKE_object_pose_array_get_unique( - scene, view_layer, nullptr); + if (set != OL_SETSEL_EXTEND) { + /* Single select forces all other bones to get unselected. */ + const Vector objects = BKE_object_pose_array_get_unique(scene, view_layer, nullptr); - for (Object *ob : objects) { - Object *ob_iter = BKE_object_pose_armature_get(ob); + for (Object *ob : objects) { + Object *ob_iter = BKE_object_pose_armature_get(ob); - /* Sanity checks. */ - if (ELEM(nullptr, ob_iter, ob_iter->pose, ob_iter->data)) { - continue; - } + /* Sanity checks. */ + if (ELEM(nullptr, ob_iter, ob_iter->pose, ob_iter->data)) { + continue; + } - LISTBASE_FOREACH (bPoseChannel *, pchannel, &ob_iter->pose->chanbase) { - pchannel->bone->flag &= ~(BONE_TIPSEL | BONE_SELECTED | BONE_ROOTSEL); - } + LISTBASE_FOREACH (bPoseChannel *, pchannel, &ob_iter->pose->chanbase) { + pchannel->bone->flag &= ~(BONE_TIPSEL | BONE_SELECTED | BONE_ROOTSEL); + } - if (ob != ob_iter) { - DEG_id_tag_update(static_cast(ob_iter->data), ID_RECALC_SELECT); - } + if (ob != ob_iter) { + DEG_id_tag_update(static_cast(ob_iter->data), ID_RECALC_SELECT); } } - - if ((set == OL_SETSEL_EXTEND) && (pchan->bone->flag & BONE_SELECTED)) { - pchan->bone->flag &= ~BONE_SELECTED; - } - else { - pchan->bone->flag |= BONE_SELECTED; - arm->act_bone = pchan->bone; - } - - if (recursive) { - /* Recursive select/deselect */ - do_outliner_bone_select_recursive( - arm, pchan->bone, (pchan->bone->flag & BONE_SELECTED) != 0); - } - - WM_event_add_notifier(C, NC_OBJECT | ND_BONE_ACTIVE, ob); - DEG_id_tag_update(&arm->id, ID_RECALC_SELECT); } + + if ((set == OL_SETSEL_EXTEND) && (pchan->bone->flag & BONE_SELECTED)) { + pchan->bone->flag &= ~BONE_SELECTED; + } + else { + if (ANIM_bone_is_visible(arm, pchan->bone)) { + pchan->bone->flag |= BONE_SELECTED; + } + arm->act_bone = pchan->bone; + } + + if (recursive) { + /* Recursive select/deselect */ + do_outliner_bone_select_recursive(arm, pchan->bone, (pchan->bone->flag & BONE_SELECTED) != 0); + } + + WM_event_add_notifier(C, NC_OBJECT | ND_BONE_ACTIVE, ob); + DEG_id_tag_update(&arm->id, ID_RECALC_SELECT); } static void tree_element_bone_activate(bContext *C, @@ -580,36 +578,36 @@ static void tree_element_bone_activate(bContext *C, bArmature *arm = (bArmature *)tselem->id; Bone *bone = static_cast(te->directdata); - if (!(bone->flag & BONE_HIDDEN_P)) { - BKE_view_layer_synced_ensure(scene, view_layer); - Object *ob = BKE_view_layer_active_object_get(view_layer); - if (ob) { - if (set != OL_SETSEL_EXTEND) { - /* single select forces all other bones to get unselected */ - for (Bone *bone_iter = static_cast(arm->bonebase.first); bone_iter != nullptr; - bone_iter = bone_iter->next) - { - bone_iter->flag &= ~(BONE_TIPSEL | BONE_SELECTED | BONE_ROOTSEL); - do_outliner_bone_select_recursive(arm, bone_iter, false); - } + BKE_view_layer_synced_ensure(scene, view_layer); + Object *ob = BKE_view_layer_active_object_get(view_layer); + if (ob) { + if (set != OL_SETSEL_EXTEND) { + /* single select forces all other bones to get unselected */ + for (Bone *bone_iter = static_cast(arm->bonebase.first); bone_iter != nullptr; + bone_iter = bone_iter->next) + { + bone_iter->flag &= ~(BONE_TIPSEL | BONE_SELECTED | BONE_ROOTSEL); + do_outliner_bone_select_recursive(arm, bone_iter, false); } } - - if (set == OL_SETSEL_EXTEND && (bone->flag & BONE_SELECTED)) { - bone->flag &= ~BONE_SELECTED; - } - else { - bone->flag |= BONE_SELECTED; - arm->act_bone = bone; - } - - if (recursive) { - /* Recursive select/deselect */ - do_outliner_bone_select_recursive(arm, bone, (bone->flag & BONE_SELECTED) != 0); - } - - WM_event_add_notifier(C, NC_OBJECT | ND_BONE_ACTIVE, ob); } + + if (set == OL_SETSEL_EXTEND && (bone->flag & BONE_SELECTED)) { + bone->flag &= ~BONE_SELECTED; + } + else { + if (ANIM_bone_is_visible(arm, bone)) { + bone->flag |= BONE_SELECTED; + } + arm->act_bone = bone; + } + + if (recursive) { + /* Recursive select/deselect */ + do_outliner_bone_select_recursive(arm, bone, (bone->flag & BONE_SELECTED) != 0); + } + + WM_event_add_notifier(C, NC_OBJECT | ND_BONE_ACTIVE, ob); } /** Edit-bones only draw in edit-mode armature. */ @@ -618,9 +616,12 @@ static void tree_element_active_ebone__sel(bContext *C, bArmature *arm, EditBone if (sel) { arm->act_edbone = ebone; } - ED_armature_ebone_select_set(ebone, sel); + if (ANIM_bone_is_visible_editbone(arm, ebone)) { + ED_armature_ebone_select_set(ebone, sel); + } WM_event_add_notifier(C, NC_OBJECT | ND_BONE_ACTIVE, CTX_data_edit_object(C)); } + static void tree_element_ebone_activate(bContext *C, const Scene *scene, ViewLayer *view_layer, @@ -633,28 +634,23 @@ static void tree_element_ebone_activate(bContext *C, EditBone *ebone = static_cast(te->directdata); if (set == OL_SETSEL_NORMAL) { - if (!(ebone->flag & BONE_HIDDEN_A)) { + ObjectsInModeParams ob_params{}; + ob_params.object_mode = OB_MODE_EDIT; + ob_params.no_dup_data = true; - ObjectsInModeParams ob_params{}; - ob_params.object_mode = OB_MODE_EDIT; - ob_params.no_dup_data = true; + Vector bases = BKE_view_layer_array_from_bases_in_mode_params( + scene, view_layer, nullptr, &ob_params); + ED_armature_edit_deselect_all_multi_ex(bases); - Vector bases = BKE_view_layer_array_from_bases_in_mode_params( - scene, view_layer, nullptr, &ob_params); - ED_armature_edit_deselect_all_multi_ex(bases); - - tree_element_active_ebone__sel(C, arm, ebone, true); - } + tree_element_active_ebone__sel(C, arm, ebone, true); } else if (set == OL_SETSEL_EXTEND) { - if (!(ebone->flag & BONE_HIDDEN_A)) { - if (!(ebone->flag & BONE_SELECTED)) { - tree_element_active_ebone__sel(C, arm, ebone, true); - } - else { - /* entirely selected, so de-select */ - tree_element_active_ebone__sel(C, arm, ebone, false); - } + if (!(ebone->flag & BONE_SELECTED)) { + tree_element_active_ebone__sel(C, arm, ebone, true); + } + else { + /* entirely selected, so de-select */ + tree_element_active_ebone__sel(C, arm, ebone, false); } } diff --git a/source/blender/editors/space_view3d/view3d_select.cc b/source/blender/editors/space_view3d/view3d_select.cc index 368256d8c8a..b50f62bf815 100644 --- a/source/blender/editors/space_view3d/view3d_select.cc +++ b/source/blender/editors/space_view3d/view3d_select.cc @@ -5054,7 +5054,6 @@ static bool armature_circle_select(const ViewContext *vc, if (data.is_changed) { ED_armature_edit_sync_selection(arm->edbo); - ED_armature_edit_validate_active(arm); WM_main_add_notifier(NC_OBJECT | ND_BONE_SELECT, vc->obedit); } return data.is_changed; From 0dc5abc9e5dc8b57e3266d202132687e0ebc7deb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Foucault?= Date: Tue, 18 Jun 2024 14:52:58 +0200 Subject: [PATCH 3/5] Fix: EEVEE: Infinite loop in shadow update This happened in the following render test: `render/light/all_light_types_in_volume.blend` Unfortunately it seems non-deterministic. To fix this, we change the heuristic to jump out of the shadow update loop. Also introduce a upper bound to the number of iteration. On top of this, add a flush for every loop to avoid huge command buffer submission. Thanks @pragma37 for the fix. --- .../engines/eevee_next/eevee_shader_shared.hh | 6 ++++++ .../draw/engines/eevee_next/eevee_shadow.cc | 21 ++++++++++++++----- .../draw/engines/eevee_next/eevee_shadow.hh | 2 +- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/source/blender/draw/engines/eevee_next/eevee_shader_shared.hh b/source/blender/draw/engines/eevee_next/eevee_shader_shared.hh index a29b277522e..7661ca9f5e8 100644 --- a/source/blender/draw/engines/eevee_next/eevee_shader_shared.hh +++ b/source/blender/draw/engines/eevee_next/eevee_shader_shared.hh @@ -1395,6 +1395,12 @@ BLI_STATIC_ASSERT_ALIGN(ShadowPagesInfoData, 16) struct ShadowStatistics { /** Statistics that are read back to CPU after a few frame (to avoid stall). */ + /** + * WARNING: Excepting `view_needed_count` it is uncertain if these are accurate. + * This is because `eevee_shadow_page_allocate_comp` runs on all pages even for + * directional. There might be some lingering states somewhere as relying on + * `page_update_count` was causing non-deterministic infinite loop. Needs further investigation. + */ int page_used_count; int page_update_count; int page_allocated_count; diff --git a/source/blender/draw/engines/eevee_next/eevee_shadow.cc b/source/blender/draw/engines/eevee_next/eevee_shadow.cc index bd892781f32..7930fb60dc8 100644 --- a/source/blender/draw/engines/eevee_next/eevee_shadow.cc +++ b/source/blender/draw/engines/eevee_next/eevee_shadow.cc @@ -1134,8 +1134,14 @@ float ShadowModule::screen_pixel_radius(const float4x4 &wininv, return math::distance(p0, p1) / min_dim; } -bool ShadowModule::shadow_update_finished() +bool ShadowModule::shadow_update_finished(int loop_count) { + if (loop_count >= (SHADOW_MAX_TILEMAP * SHADOW_TILEMAP_LOD) / SHADOW_VIEW_MAX) { + /* We have reach the maximum theoretical number of updates. + * This can indicate a problem in the statistic buffer readback or update tagging. */ + return true; + } + if (!inst_.is_image_render()) { /* For viewport, only run the shadow update once per redraw. * This avoids the stall from the read-back and freezes from long shadow update. */ @@ -1154,7 +1160,7 @@ bool ShadowModule::shadow_update_finished() statistics_buf_.current().read(); ShadowStatistics stats = statistics_buf_.current(); /* Rendering is finished if we rendered all the remaining pages. */ - return stats.page_rendered_count == stats.page_update_count; + return stats.view_needed_count <= SHADOW_VIEW_MAX; } int ShadowModule::max_view_per_tilemap() @@ -1231,8 +1237,8 @@ void ShadowModule::set_view(View &view, int2 extent) } inst_.hiz_buffer.update(); - bool first_loop = true; + int loop_count = 0; do { DRW_stats_group_start("Shadow"); { @@ -1245,7 +1251,7 @@ void ShadowModule::set_view(View &view, int2 extent) * test casters only against the static tilemaps instead of all of them. */ inst_.manager->submit(caster_update_ps_, view); } - if (assign_if_different(first_loop, false)) { + if (loop_count == 0) { inst_.manager->submit(jittered_transparent_caster_update_ps_, view); } inst_.manager->submit(tilemap_usage_ps_, view); @@ -1259,6 +1265,8 @@ void ShadowModule::set_view(View &view, int2 extent) * If parameter buffer exceeds limits, then other work will not be impacted. */ bool use_flush = (shadow_technique == ShadowTechnique::TILE_COPY) && (GPU_backend_get_type() == GPU_BACKEND_METAL); + /* Flush every loop as these passes are very heavy. */ + use_flush |= loop_count != 0; if (use_flush) { GPU_flush(); @@ -1300,7 +1308,10 @@ void ShadowModule::set_view(View &view, int2 extent) GPU_memory_barrier(GPU_BARRIER_SHADER_IMAGE_ACCESS | GPU_BARRIER_TEXTURE_FETCH); } DRW_stats_group_end(); - } while (!shadow_update_finished()); + + loop_count++; + + } while (!shadow_update_finished(loop_count)); if (prev_fb) { GPU_framebuffer_bind(prev_fb); diff --git a/source/blender/draw/engines/eevee_next/eevee_shadow.hh b/source/blender/draw/engines/eevee_next/eevee_shadow.hh index ce0a8edf739..d9711f85f21 100644 --- a/source/blender/draw/engines/eevee_next/eevee_shadow.hh +++ b/source/blender/draw/engines/eevee_next/eevee_shadow.hh @@ -371,7 +371,7 @@ class ShadowModule { private: void remove_unused(); void debug_page_map_call(DRWPass *pass); - bool shadow_update_finished(); + bool shadow_update_finished(int loop_count); /** Compute approximate punctual shadow pixel world space radius, 1 unit away of the light. */ float tilemap_pixel_radius(); From 56c1163c2126ab68e15cc4aacba12fa277afc120 Mon Sep 17 00:00:00 2001 From: Patrick Mours Date: Tue, 18 Jun 2024 15:27:14 +0200 Subject: [PATCH 4/5] Fix: Cycles OptiX wrong stack size for OSL pipeline The callables generated by OSL reference other external functions (defined in the OSL services module), in which case OptiX cannot calculate the right stack size just based on the callable alone, it needs to know all functions linked together in the pipeline to get to an accurate result. `optixProgramGroupGetStackSize` has an optional pipeline argument for this purpose, so make use of that to ensure the correct stack size is calculated. Ref #122779 Pull Request: https://projects.blender.org/blender/blender/pulls/123368 --- intern/cycles/device/optix/device_impl.cpp | 35 ++++++++++++++-------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/intern/cycles/device/optix/device_impl.cpp b/intern/cycles/device/optix/device_impl.cpp index 9901803b234..a22daf168d2 100644 --- a/intern/cycles/device/optix/device_impl.cpp +++ b/intern/cycles/device/optix/device_impl.cpp @@ -916,27 +916,14 @@ bool OptiXDevice::load_osl_kernels() context, group_descs, 2, &group_options, nullptr, 0, &osl_groups[i * 2])); } - OptixStackSizes stack_size[NUM_PROGRAM_GROUPS] = {}; - vector osl_stack_size(osl_groups.size()); - /* Update SBT with new entries. */ sbt_data.alloc(NUM_PROGRAM_GROUPS + osl_groups.size()); for (int i = 0; i < NUM_PROGRAM_GROUPS; ++i) { optix_assert(optixSbtRecordPackHeader(groups[i], &sbt_data[i])); -# if OPTIX_ABI_VERSION >= 84 - optix_assert(optixProgramGroupGetStackSize(groups[i], &stack_size[i], nullptr)); -# else - optix_assert(optixProgramGroupGetStackSize(groups[i], &stack_size[i])); -# endif } for (size_t i = 0; i < osl_groups.size(); ++i) { if (osl_groups[i] != NULL) { optix_assert(optixSbtRecordPackHeader(osl_groups[i], &sbt_data[NUM_PROGRAM_GROUPS + i])); -# if OPTIX_ABI_VERSION >= 84 - optix_assert(optixProgramGroupGetStackSize(osl_groups[i], &osl_stack_size[i], nullptr)); -# else - optix_assert(optixProgramGroupGetStackSize(osl_groups[i], &osl_stack_size[i])); -# endif } else { /* Default to "__direct_callable__dummy_services", so that OSL evaluation for empty @@ -982,6 +969,28 @@ bool OptiXDevice::load_osl_kernels() 0, &pipelines[PIP_SHADE])); + /* Get program stack sizes. */ + OptixStackSizes stack_size[NUM_PROGRAM_GROUPS] = {}; + vector osl_stack_size(osl_groups.size()); + + for (int i = 0; i < NUM_PROGRAM_GROUPS; ++i) { +# if OPTIX_ABI_VERSION >= 84 + optix_assert(optixProgramGroupGetStackSize(groups[i], &stack_size[i], nullptr)); +# else + optix_assert(optixProgramGroupGetStackSize(groups[i], &stack_size[i])); +# endif + } + for (size_t i = 0; i < osl_groups.size(); ++i) { + if (osl_groups[i] != NULL) { +# if OPTIX_ABI_VERSION >= 84 + optix_assert(optixProgramGroupGetStackSize( + osl_groups[i], &osl_stack_size[i], pipelines[PIP_SHADE])); +# else + optix_assert(optixProgramGroupGetStackSize(osl_groups[i], &osl_stack_size[i])); +# endif + } + } + const unsigned int css = std::max(stack_size[PG_RGEN_SHADE_SURFACE_RAYTRACE].cssRG, stack_size[PG_RGEN_SHADE_SURFACE_MNEE].cssRG); unsigned int dss = 0; From 979e14296555690a85f915e039c5629dfda6b95c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Foucault?= Date: Mon, 17 Jun 2024 19:08:37 +0200 Subject: [PATCH 5/5] Fix: EEVEE: Object holdout not working This implement the holdout flag by switching to the holdout case in the shader. This has a few benefits: - Doesn't recompile the shaders. - Makes the object infos mandatory (already the case in practice) - Handle transparent materials properly, keeping the transparency working. Fix #123284 Pull Request: https://projects.blender.org/blender/blender/pulls/123315 --- .../draw/engines/eevee_next/eevee_material.cc | 3 --- .../draw/engines/eevee_next/eevee_shader.cc | 9 -------- .../shaders/eevee_attributes_lib.glsl | 2 +- .../shaders/eevee_nodetree_lib.glsl | 4 ++-- .../shaders/eevee_surf_deferred_frag.glsl | 23 ++++++++++++++----- .../shaders/eevee_surf_forward_frag.glsl | 9 +++++++- .../shaders/eevee_surf_hybrid_frag.glsl | 23 ++++++++++++++----- .../shaders/infos/eevee_material_info.hh | 17 +++++++++++--- source/blender/draw/intern/draw_resource.hh | 3 +++ .../blender/draw/intern/draw_shader_shared.hh | 3 ++- .../intern/shaders/draw_object_infos_info.hh | 1 + source/blender/gpu/intern/gpu_codegen.cc | 4 ---- 12 files changed, 65 insertions(+), 36 deletions(-) diff --git a/source/blender/draw/engines/eevee_next/eevee_material.cc b/source/blender/draw/engines/eevee_next/eevee_material.cc index abec9b7599c..deadf652852 100644 --- a/source/blender/draw/engines/eevee_next/eevee_material.cc +++ b/source/blender/draw/engines/eevee_next/eevee_material.cc @@ -421,9 +421,6 @@ Material &MaterialModule::material_sync(Object *ob, ::Material *MaterialModule::material_from_slot(Object *ob, int slot) { - if (ob->base_flag & BASE_HOLDOUT) { - return BKE_material_default_holdout(); - } ::Material *ma = BKE_object_material_get(ob, slot + 1); if (ma == nullptr) { if (ob->type == OB_VOLUME) { diff --git a/source/blender/draw/engines/eevee_next/eevee_shader.cc b/source/blender/draw/engines/eevee_next/eevee_shader.cc index 4c214792805..aef6ddda05f 100644 --- a/source/blender/draw/engines/eevee_next/eevee_shader.cc +++ b/source/blender/draw/engines/eevee_next/eevee_shader.cc @@ -406,12 +406,6 @@ void ShaderModule::material_create_info_amend(GPUMaterial *gpumat, GPUCodegenOut GPUCodegenOutput &codegen = *codegen_; ShaderCreateInfo &info = *reinterpret_cast(codegen.create_info); - /* WORKAROUND: Replace by new ob info. */ - int64_t ob_info_index = info.additional_infos_.first_index_of_try("draw_object_infos"); - if (ob_info_index != -1) { - info.additional_infos_[ob_info_index] = "draw_object_infos_new"; - } - /* WORKAROUND: Add new ob attr buffer. */ if (GPU_material_uniform_attributes(gpumat) != nullptr) { info.additional_info("draw_object_attribute_new"); @@ -588,9 +582,6 @@ void ShaderModule::material_create_info_amend(GPUMaterial *gpumat, GPUCodegenOut info.vertex_inputs_.clear(); /* Volume materials require these for loading the grid attributes from smoke sims. */ info.additional_info("draw_volume_infos"); - if (ob_info_index == -1) { - info.additional_info("draw_object_infos_new"); - } } break; case MAT_GEOM_POINT_CLOUD: diff --git a/source/blender/draw/engines/eevee_next/shaders/eevee_attributes_lib.glsl b/source/blender/draw/engines/eevee_next/shaders/eevee_attributes_lib.glsl index 23f04d9e213..9a4bbce31b3 100644 --- a/source/blender/draw/engines/eevee_next/shaders/eevee_attributes_lib.glsl +++ b/source/blender/draw/engines/eevee_next/shaders/eevee_attributes_lib.glsl @@ -22,7 +22,7 @@ int g_attr_id = 0; /* Point clouds and curves are not compatible with volume grids. * They will fallback to their own attributes loading. */ #if defined(MAT_VOLUME) && !defined(MAT_GEOM_CURVES) && !defined(MAT_GEOM_POINT_CLOUD) -# if defined(OBINFO_LIB) && !defined(MAT_GEOM_WORLD) +# if defined(VOLUME_INFO_LIB) && !defined(MAT_GEOM_WORLD) # define GRID_ATTRIBUTES # endif diff --git a/source/blender/draw/engines/eevee_next/shaders/eevee_nodetree_lib.glsl b/source/blender/draw/engines/eevee_next/shaders/eevee_nodetree_lib.glsl index 72905e25fcd..9a9f048b57a 100644 --- a/source/blender/draw/engines/eevee_next/shaders/eevee_nodetree_lib.glsl +++ b/source/blender/draw/engines/eevee_next/shaders/eevee_nodetree_lib.glsl @@ -731,7 +731,7 @@ float film_scaling_factor_get() /* Point clouds and curves are not compatible with volume grids. * They will fallback to their own attributes loading. */ #if defined(MAT_VOLUME) && !defined(MAT_GEOM_CURVES) && !defined(MAT_GEOM_POINT_CLOUD) -# if defined(OBINFO_LIB) && !defined(MAT_GEOM_WORLD) +# if defined(VOLUME_INFO_LIB) && !defined(MAT_GEOM_WORLD) /* We could just check for GRID_ATTRIBUTES but this avoids for header dependency. */ # define GRID_ATTRIBUTES_LOAD_POST # endif @@ -740,7 +740,7 @@ float film_scaling_factor_get() float attr_load_temperature_post(float attr) { #ifdef GRID_ATTRIBUTES_LOAD_POST - /* Bring the into standard range without having to modify the grid values */ + /* Bring the value into standard range without having to modify the grid values */ attr = (attr > 0.01) ? (attr * drw_volume.temperature_mul + drw_volume.temperature_bias) : 0.0; #endif return attr; diff --git a/source/blender/draw/engines/eevee_next/shaders/eevee_surf_deferred_frag.glsl b/source/blender/draw/engines/eevee_next/shaders/eevee_surf_deferred_frag.glsl index c08384f06a7..591f8667d7d 100644 --- a/source/blender/draw/engines/eevee_next/shaders/eevee_surf_deferred_frag.glsl +++ b/source/blender/draw/engines/eevee_next/shaders/eevee_surf_deferred_frag.glsl @@ -50,9 +50,20 @@ void main() float thickness = nodetree_thickness() * thickness_mode; /** Transparency weight is already applied through dithering, remove it from other closures. */ - float transparency = 1.0 - average(g_transmittance); - float transparency_rcp = safe_rcp(transparency); - g_emission *= transparency_rcp; + float alpha = 1.0 - average(g_transmittance); + float alpha_rcp = safe_rcp(alpha); + + /* Object holdout. */ + eObjectInfoFlag ob_flag = eObjectInfoFlag(floatBitsToUint(drw_infos[resource_id].infos.w)); + if (flag_test(ob_flag, OBJECT_HOLDOUT)) { + /* alpha is set from rejected pixels / dithering. */ + g_holdout = 1.0; + + /* Set alpha to 0.0 so that lighting is not computed. */ + alpha_rcp = 0.0; + } + + g_emission *= alpha_rcp; ivec2 out_texel = ivec2(gl_FragCoord.xy); @@ -72,12 +83,12 @@ void main() /* ----- GBuffer output ----- */ GBufferData gbuf_data; - gbuf_data.closure[0] = g_closure_get_resolved(0, transparency_rcp); + gbuf_data.closure[0] = g_closure_get_resolved(0, alpha_rcp); #if CLOSURE_BIN_COUNT > 1 - gbuf_data.closure[1] = g_closure_get_resolved(1, transparency_rcp); + gbuf_data.closure[1] = g_closure_get_resolved(1, alpha_rcp); #endif #if CLOSURE_BIN_COUNT > 2 - gbuf_data.closure[2] = g_closure_get_resolved(2, transparency_rcp); + gbuf_data.closure[2] = g_closure_get_resolved(2, alpha_rcp); #endif gbuf_data.surface_N = g_data.N; gbuf_data.thickness = thickness; diff --git a/source/blender/draw/engines/eevee_next/shaders/eevee_surf_forward_frag.glsl b/source/blender/draw/engines/eevee_next/shaders/eevee_surf_forward_frag.glsl index f8aa1ee5536..0bda20c85f3 100644 --- a/source/blender/draw/engines/eevee_next/shaders/eevee_surf_forward_frag.glsl +++ b/source/blender/draw/engines/eevee_next/shaders/eevee_surf_forward_frag.glsl @@ -49,6 +49,13 @@ void main() nodetree_surface(closure_rand); + eObjectInfoFlag ob_flag = eObjectInfoFlag(floatBitsToUint(drw_infos[resource_id].infos.w)); + if (flag_test(ob_flag, OBJECT_HOLDOUT)) { + g_holdout = 1.0; + } + + g_holdout = saturate(g_holdout); + vec3 radiance, transmittance; forward_lighting_eval(g_thickness, radiance, transmittance); @@ -64,6 +71,6 @@ void main() radiance *= 1.0 - saturate(g_holdout); - out_radiance = vec4(radiance, 0.0); + out_radiance = vec4(radiance, g_holdout); out_transmittance = vec4(transmittance, saturate(average(transmittance))); } diff --git a/source/blender/draw/engines/eevee_next/shaders/eevee_surf_hybrid_frag.glsl b/source/blender/draw/engines/eevee_next/shaders/eevee_surf_hybrid_frag.glsl index 8e4cfb8bbea..b8651e497ca 100644 --- a/source/blender/draw/engines/eevee_next/shaders/eevee_surf_hybrid_frag.glsl +++ b/source/blender/draw/engines/eevee_next/shaders/eevee_surf_hybrid_frag.glsl @@ -53,9 +53,20 @@ void main() g_holdout = saturate(g_holdout); /** Transparency weight is already applied through dithering, remove it from other closures. */ - float transparency = 1.0 - average(g_transmittance); - float transparency_rcp = safe_rcp(transparency); - g_emission *= transparency_rcp; + float alpha = 1.0 - average(g_transmittance); + float alpha_rcp = safe_rcp(alpha); + + /* Object holdout. */ + eObjectInfoFlag ob_flag = eObjectInfoFlag(floatBitsToUint(drw_infos[resource_id].infos.w)); + if (flag_test(ob_flag, OBJECT_HOLDOUT)) { + /* alpha is set from rejected pixels / dithering. */ + g_holdout = 1.0; + + /* Set alpha to 0.0 so that lighting is not computed. */ + alpha_rcp = 0.0; + } + + g_emission *= alpha_rcp; ivec2 out_texel = ivec2(gl_FragCoord.xy); @@ -75,12 +86,12 @@ void main() /* ----- GBuffer output ----- */ GBufferData gbuf_data; - gbuf_data.closure[0] = g_closure_get_resolved(0, transparency_rcp); + gbuf_data.closure[0] = g_closure_get_resolved(0, alpha_rcp); #if CLOSURE_BIN_COUNT > 1 - gbuf_data.closure[1] = g_closure_get_resolved(1, transparency_rcp); + gbuf_data.closure[1] = g_closure_get_resolved(1, alpha_rcp); #endif #if CLOSURE_BIN_COUNT > 2 - gbuf_data.closure[2] = g_closure_get_resolved(2, transparency_rcp); + gbuf_data.closure[2] = g_closure_get_resolved(2, alpha_rcp); #endif gbuf_data.surface_N = g_data.N; gbuf_data.thickness = g_thickness; diff --git a/source/blender/draw/engines/eevee_next/shaders/infos/eevee_material_info.hh b/source/blender/draw/engines/eevee_next/shaders/infos/eevee_material_info.hh index 081484d2acd..3f6e80e5acc 100644 --- a/source/blender/draw/engines/eevee_next/shaders/infos/eevee_material_info.hh +++ b/source/blender/draw/engines/eevee_next/shaders/infos/eevee_material_info.hh @@ -54,7 +54,10 @@ GPU_SHADER_CREATE_INFO(eevee_geom_mesh) .vertex_in(1, Type::VEC3, "nor") .vertex_source("eevee_geom_mesh_vert.glsl") .vertex_out(eevee_surf_iface) - .additional_info("draw_modelmat_new", "draw_resource_id_varying", "draw_view"); + .additional_info("draw_modelmat_new", + "draw_object_infos_new", + "draw_resource_id_varying", + "draw_view"); GPU_SHADER_INTERFACE_INFO(eevee_surf_point_cloud_iface, "point_cloud_interp") .smooth(Type::FLOAT, "radius") @@ -71,6 +74,7 @@ GPU_SHADER_CREATE_INFO(eevee_geom_point_cloud) .vertex_out(eevee_surf_point_cloud_flat_iface) .additional_info("draw_pointcloud_new", "draw_modelmat_new", + "draw_object_infos_new", "draw_resource_id_varying", "draw_view"); @@ -91,7 +95,10 @@ GPU_SHADER_CREATE_INFO(eevee_geom_gpencil) .define("MAT_GEOM_GPENCIL") .vertex_source("eevee_geom_gpencil_vert.glsl") .vertex_out(eevee_surf_iface) - .additional_info("draw_gpencil_new", "draw_resource_id_varying", "draw_resource_id_new"); + .additional_info("draw_gpencil_new", + "draw_object_infos_new", + "draw_resource_id_varying", + "draw_resource_id_new"); GPU_SHADER_INTERFACE_INFO(eevee_surf_curve_iface, "curve_interp") .smooth(Type::VEC2, "barycentric_coords") @@ -111,6 +118,7 @@ GPU_SHADER_CREATE_INFO(eevee_geom_curves) .vertex_out(eevee_surf_curve_iface) .vertex_out(eevee_surf_curve_flat_iface) .additional_info("draw_modelmat_new", + "draw_object_infos_new", "draw_resource_id_varying", "draw_view", "draw_hair_new", @@ -122,7 +130,10 @@ GPU_SHADER_CREATE_INFO(eevee_geom_world) .builtins(BuiltinBits::VERTEX_ID) .vertex_source("eevee_geom_world_vert.glsl") .vertex_out(eevee_surf_iface) - .additional_info("draw_modelmat_new", "draw_resource_id_varying", "draw_view"); + .additional_info("draw_modelmat_new", + "draw_object_infos_new", /* Unused, but allow debug compilation. */ + "draw_resource_id_varying", + "draw_view"); /** \} */ diff --git a/source/blender/draw/intern/draw_resource.hh b/source/blender/draw/intern/draw_resource.hh index b3fa92dfbc4..ab6f0c6f01e 100644 --- a/source/blender/draw/intern/draw_resource.hh +++ b/source/blender/draw/intern/draw_resource.hh @@ -73,6 +73,8 @@ inline void ObjectInfos::sync(const blender::draw::ObjectRef ref, bool is_active { object_attrs_len = 0; object_attrs_offset = 0; + bool is_holdout = (ref.object->base_flag & BASE_HOLDOUT) || + (ref.object->visibility_flag & OB_HOLDOUT); ob_color = ref.object->color; index = ref.object->index; @@ -85,6 +87,7 @@ inline void ObjectInfos::sync(const blender::draw::ObjectRef ref, bool is_active flag, ref.object->base_flag & BASE_FROM_SET, eObjectInfoFlag::OBJECT_FROM_SET); SET_FLAG_FROM_TEST( flag, ref.object->transflag & OB_NEG_SCALE, eObjectInfoFlag::OBJECT_NEGATIVE_SCALE); + SET_FLAG_FROM_TEST(flag, is_holdout, eObjectInfoFlag::OBJECT_HOLDOUT); if (ref.dupli_object == nullptr) { /* TODO(fclem): this is rather costly to do at draw time. Maybe we can diff --git a/source/blender/draw/intern/draw_shader_shared.hh b/source/blender/draw/intern/draw_shader_shared.hh index 86a8e0db746..30b6ab30982 100644 --- a/source/blender/draw/intern/draw_shader_shared.hh +++ b/source/blender/draw/intern/draw_shader_shared.hh @@ -153,8 +153,9 @@ enum eObjectInfoFlag : uint32_t { OBJECT_FROM_SET = (1u << 2u), OBJECT_ACTIVE = (1u << 3u), OBJECT_NEGATIVE_SCALE = (1u << 4u), + OBJECT_HOLDOUT = (1u << 5u), /* Avoid skipped info to change culling. */ - OBJECT_NO_INFO = ~OBJECT_NEGATIVE_SCALE + OBJECT_NO_INFO = ~OBJECT_HOLDOUT }; struct ObjectInfos { diff --git a/source/blender/draw/intern/shaders/draw_object_infos_info.hh b/source/blender/draw/intern/shaders/draw_object_infos_info.hh index 9bc812cf7aa..91497b459b3 100644 --- a/source/blender/draw/intern/shaders/draw_object_infos_info.hh +++ b/source/blender/draw/intern/shaders/draw_object_infos_info.hh @@ -18,6 +18,7 @@ GPU_SHADER_CREATE_INFO(draw_object_infos) GPU_SHADER_CREATE_INFO(draw_volume_infos) .typedef_source("draw_shader_shared.hh") + .define("VOLUME_INFO_LIB") .uniform_buf(DRW_OBJ_DATA_INFO_UBO_SLOT, "VolumeInfos", "drw_volume", Frequency::BATCH); GPU_SHADER_CREATE_INFO(draw_curves_infos) diff --git a/source/blender/gpu/intern/gpu_codegen.cc b/source/blender/gpu/intern/gpu_codegen.cc index f09e2066daa..e5c418e9cd7 100644 --- a/source/blender/gpu/intern/gpu_codegen.cc +++ b/source/blender/gpu/intern/gpu_codegen.cc @@ -282,10 +282,6 @@ class GPUCodegen { create_info = new GPUCodegenCreateInfo("codegen"); output.create_info = reinterpret_cast( static_cast(create_info)); - - if (GPU_material_flag_get(mat_, GPU_MATFLAG_OBJECT_INFO)) { - create_info->additional_info("draw_object_infos"); - } } ~GPUCodegen()