From 2131ef0a2093cd2a19dc925b503e4dca41eddedb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Foucault?= Date: Thu, 6 Mar 2025 19:15:29 +0100 Subject: [PATCH] DRW: Use depsgraph update count to replace view_update This avoid having to flush explicitely and the need for syncing. It also removes a lot of complexity in the process. These updates are not granular and do not need to so much boiler plate code. The depsgraph update counter now becomes atomic to avoid undefined behavior when a depsgraph is being destroyed and its memory reused (same thinking as the non-copy-on-eval IDs). I tested some use cases (object update, sculpt update, shading update) and they are all working. Pull Request: https://projects.blender.org/blender/blender/pulls/135580 --- source/blender/draw/DRW_engine.hh | 11 -- .../draw/engines/eevee_next/eevee_engine.cc | 9 +- .../draw/engines/eevee_next/eevee_instance.cc | 11 +- .../draw/engines/eevee_next/eevee_instance.hh | 2 - .../engines/workbench/workbench_engine.cc | 29 ++-- .../engines/workbench/workbench_private.hh | 2 +- .../draw/engines/workbench/workbench_state.cc | 4 +- source/blender/draw/intern/DRW_render.hh | 1 + source/blender/draw/intern/draw_manager_c.cc | 128 ------------------ .../blender/editors/render/render_update.cc | 16 --- .../editors/space_view3d/space_view3d.cc | 31 ----- 11 files changed, 20 insertions(+), 224 deletions(-) diff --git a/source/blender/draw/DRW_engine.hh b/source/blender/draw/DRW_engine.hh index 0c9749e0ae5..6d359d1ae03 100644 --- a/source/blender/draw/DRW_engine.hh +++ b/source/blender/draw/DRW_engine.hh @@ -38,17 +38,6 @@ bool DRW_engine_render_support(DrawEngineType *draw_engine_type); void DRW_engine_external_free(RegionView3D *rv3d); -struct DRWUpdateContext { - Main *bmain; - Depsgraph *depsgraph; - Scene *scene; - ViewLayer *view_layer; - ARegion *region; - View3D *v3d; - RenderEngineType *engine_type; -}; -void DRW_notify_view_update(const DRWUpdateContext *update_ctx); - enum eDRWSelectStage { DRW_SELECT_PASS_PRE = 1, DRW_SELECT_PASS_POST, diff --git a/source/blender/draw/engines/eevee_next/eevee_engine.cc b/source/blender/draw/engines/eevee_next/eevee_engine.cc index a802ba0b285..dba43ec17b5 100644 --- a/source/blender/draw/engines/eevee_next/eevee_engine.cc +++ b/source/blender/draw/engines/eevee_next/eevee_engine.cc @@ -125,13 +125,6 @@ static void eevee_cache_finish(void *vedata) reinterpret_cast(vedata)->instance->end_sync(); } -static void eevee_view_update(void *vedata) -{ - if (eevee::Instance *instance = reinterpret_cast(vedata)->instance) { - instance->view_update(); - } -} - static void eevee_engine_free() { eevee::ShaderModule::module_free(); @@ -193,7 +186,7 @@ DrawEngineType draw_engine_eevee_next_type = { /*cache_populate*/ &eevee_cache_populate, /*cache_finish*/ &eevee_cache_finish, /*draw_scene*/ &eevee_draw_scene, - /*view_update*/ &eevee_view_update, + /*view_update*/ nullptr, /*id_update*/ nullptr, /*render_to_image*/ &eevee_render_to_image, /*store_metadata*/ &eevee_store_metadata, diff --git a/source/blender/draw/engines/eevee_next/eevee_instance.cc b/source/blender/draw/engines/eevee_next/eevee_instance.cc index 251a9aa4032..77fff254912 100644 --- a/source/blender/draw/engines/eevee_next/eevee_instance.cc +++ b/source/blender/draw/engines/eevee_next/eevee_instance.cc @@ -82,6 +82,10 @@ void Instance::init(const int2 &output_res, } if (is_viewport()) { + /* Note: Do not update the value here as we use it during sync for checking ID updates. */ + if (depsgraph_last_update_ != DEG_get_update_count(depsgraph)) { + sampling.reset(); + } if (assign_if_different(debug_mode, (eDebugMode)G.debug_value)) { sampling.reset(); } @@ -191,13 +195,6 @@ void Instance::update_eval_members() nullptr; } -void Instance::view_update() -{ - if (is_viewport()) { - sampling.reset(); - } -} - /** \} */ /* -------------------------------------------------------------------- */ diff --git a/source/blender/draw/engines/eevee_next/eevee_instance.hh b/source/blender/draw/engines/eevee_next/eevee_instance.hh index 30864c5c9a9..e5a0431669e 100644 --- a/source/blender/draw/engines/eevee_next/eevee_instance.hh +++ b/source/blender/draw/engines/eevee_next/eevee_instance.hh @@ -193,8 +193,6 @@ class Instance { const View3D *v3d = nullptr, const RegionView3D *rv3d = nullptr); - void view_update(); - void begin_sync(); void object_sync(ObjectRef &ob_ref); void end_sync(); diff --git a/source/blender/draw/engines/workbench/workbench_engine.cc b/source/blender/draw/engines/workbench/workbench_engine.cc index e2df9b69167..40626e1bddd 100644 --- a/source/blender/draw/engines/workbench/workbench_engine.cc +++ b/source/blender/draw/engines/workbench/workbench_engine.cc @@ -63,6 +63,9 @@ class Instance { * They never get actually used. */ Vector dummy_gpu_materials_ = {1, nullptr, {}}; + /* Used to detect any scene data update. */ + uint64_t depsgraph_last_update_ = 0; + public: Span get_dummy_gpu_materials(int material_count) { @@ -72,9 +75,12 @@ class Instance { return dummy_gpu_materials_.as_span().slice(IndexRange(material_count)); }; - void init(Object *camera_ob = nullptr) + void init(Depsgraph *depsgraph, Object *camera_ob = nullptr) { - scene_state_.init(camera_ob); + bool scene_updated = assign_if_different(depsgraph_last_update_, + DEG_get_update_count(depsgraph)); + + scene_state_.init(scene_updated, camera_ob); shadow_ps_.init(scene_state_, resources_); resources_.init(scene_state_); @@ -515,11 +521,6 @@ class Instance { GPU_render_step(); } } - - void reset_taa_sample() - { - scene_state_.reset_taa_next_sample = true; - } }; } // namespace blender::workbench @@ -544,7 +545,7 @@ static void workbench_engine_init(void *vedata) ved->instance = new workbench::Instance(); } - ved->instance->init(); + ved->instance->init(DRW_context_state_get()->depsgraph); } static void workbench_cache_init(void *vedata) @@ -587,14 +588,6 @@ static void workbench_engine_free() workbench::ShaderCache::release(); } -static void workbench_view_update(void *vedata) -{ - WORKBENCH_Data *ved = reinterpret_cast(vedata); - if (ved->instance) { - ved->instance->reset_taa_sample(); - } -} - /* RENDER */ static bool workbench_render_framebuffers_init() @@ -743,7 +736,7 @@ static void workbench_render_to_image(void *vedata, DRW_cache_restart(); blender::draw::View::default_set(float4x4(viewmat), float4x4(winmat)); - ved->instance->init(camera_ob); + ved->instance->init(depsgraph, camera_ob); draw::Manager &manager = *DRW_manager_get(); manager.begin_sync(); @@ -795,7 +788,7 @@ DrawEngineType draw_engine_workbench = { /*cache_populate*/ &workbench_cache_populate, /*cache_finish*/ &workbench_cache_finish, /*draw_scene*/ &workbench_draw_scene, - /*view_update*/ &workbench_view_update, + /*view_update*/ nullptr, /*id_update*/ nullptr, /*render_to_image*/ &workbench_render_to_image, /*store_metadata*/ nullptr, diff --git a/source/blender/draw/engines/workbench/workbench_private.hh b/source/blender/draw/engines/workbench/workbench_private.hh index e6c6a6ac4f6..d514bc2746c 100644 --- a/source/blender/draw/engines/workbench/workbench_private.hh +++ b/source/blender/draw/engines/workbench/workbench_private.hh @@ -164,7 +164,7 @@ struct SceneState { /* When r == -1.0 the shader uses the vertex color */ Material material_attribute_color = Material(float3(-1.0f)); - void init(Object *camera_ob = nullptr); + void init(bool scene_updated, Object *camera_ob = nullptr); }; struct MaterialTexture { diff --git a/source/blender/draw/engines/workbench/workbench_state.cc b/source/blender/draw/engines/workbench/workbench_state.cc index 00c401b4c80..759eb855a10 100644 --- a/source/blender/draw/engines/workbench/workbench_state.cc +++ b/source/blender/draw/engines/workbench/workbench_state.cc @@ -24,9 +24,9 @@ namespace blender::workbench { -void SceneState::init(Object *camera_ob /*=nullptr*/) +void SceneState::init(bool scene_updated, Object *camera_ob /*=nullptr*/) { - bool reset_taa = reset_taa_next_sample; + bool reset_taa = reset_taa_next_sample || scene_updated; reset_taa_next_sample = false; const DRWContextState *context = DRW_context_state_get(); diff --git a/source/blender/draw/intern/DRW_render.hh b/source/blender/draw/intern/DRW_render.hh index 9825a6de514..9d16900cce8 100644 --- a/source/blender/draw/intern/DRW_render.hh +++ b/source/blender/draw/intern/DRW_render.hh @@ -72,6 +72,7 @@ struct DrawEngineType { void (*draw_scene)(void *vedata); + /* TODO(fclem): Remove. */ void (*view_update)(void *vedata); /* TODO(fclem): Remove. */ void (*id_update)(void *vedata, ID *id); diff --git a/source/blender/draw/intern/draw_manager_c.cc b/source/blender/draw/intern/draw_manager_c.cc index acd48a4734f..ae6948b2685 100644 --- a/source/blender/draw/intern/draw_manager_c.cc +++ b/source/blender/draw/intern/draw_manager_c.cc @@ -110,8 +110,6 @@ #include "DRW_select_buffer.hh" -static CLG_LogRef LOG = {"draw.manager"}; - /* -------------------------------------------------------------------- */ /** \name Settings * @@ -1199,129 +1197,6 @@ static bool drw_gpencil_engine_needed(Depsgraph *depsgraph, View3D *v3d) DEG_id_type_any_exists(depsgraph, ID_GP)); } -/* -------------------------------------------------------------------- */ -/** \name View Update - * \{ */ - -void DRW_notify_view_update(const DRWUpdateContext *update_ctx) -{ - RenderEngineType *engine_type = update_ctx->engine_type; - ARegion *region = update_ctx->region; - View3D *v3d = update_ctx->v3d; - RegionView3D *rv3d = static_cast(region->regiondata); - Depsgraph *depsgraph = update_ctx->depsgraph; - Scene *scene = update_ctx->scene; - ViewLayer *view_layer = update_ctx->view_layer; - - GPUViewport *viewport = WM_draw_region_get_viewport(region); - if (!viewport) { - return; - } - - const bool gpencil_engine_needed = drw_gpencil_engine_needed(depsgraph, v3d); - - /* XXX Really nasty locking. But else this could be executed by the - * material previews thread while rendering a viewport. - * - * Check for recursive lock which can deadlock. This should not - * happen, but in case there is a bug where depsgraph update is called - * during drawing we try not to hang Blender. */ - if (!BLI_ticket_mutex_lock_check_recursive(system_gpu_context_mutex)) { - CLOG_ERROR(&LOG, "GPU context already bound"); - BLI_assert_unreachable(); - return; - } - - DRWContext draw_ctx; - drw_set(draw_ctx); - - BKE_view_layer_synced_ensure(scene, view_layer); - drw_get().draw_ctx = {}; - drw_get().draw_ctx.region = region; - drw_get().draw_ctx.rv3d = rv3d; - drw_get().draw_ctx.v3d = v3d; - drw_get().draw_ctx.scene = scene; - drw_get().draw_ctx.view_layer = view_layer; - drw_get().draw_ctx.obact = BKE_view_layer_active_object_get(view_layer); - drw_get().draw_ctx.engine_type = engine_type; - drw_get().draw_ctx.depsgraph = depsgraph; - drw_get().draw_ctx.object_mode = OB_MODE_OBJECT; - - /* Custom lightweight initialize to avoid resetting the memory-pools. */ - drw_get().viewport = viewport; - drw_get().data = drw_viewport_data_ensure(drw_get().viewport); - - /* Separate update for each stereo view. */ - int view_count = GPU_viewport_is_stereo_get(viewport) ? 2 : 1; - for (int view = 0; view < view_count; view++) { - drw_get().view_data_active = drw_get().data->view_data[view]; - - drw_engines_enable(view_layer, engine_type, gpencil_engine_needed); - drw_engines_data_validate(); - - DRW_view_data_engines_view_update(drw_get().view_data_active); - - drw_engines_disable(); - } - - drw_manager_exit(&draw_ctx); - - BLI_ticket_mutex_unlock(system_gpu_context_mutex); -} - -/* update a viewport which belongs to a GPUOffscreen */ -static void drw_notify_view_update_offscreen(Depsgraph *depsgraph, - RenderEngineType *engine_type, - ARegion *region, - View3D *v3d, - GPUViewport *viewport) -{ - - if (viewport && GPU_viewport_do_update(viewport)) { - - Scene *scene = DEG_get_evaluated_scene(depsgraph); - ViewLayer *view_layer = DEG_get_evaluated_view_layer(depsgraph); - RegionView3D *rv3d = static_cast(region->regiondata); - - const bool gpencil_engine_needed = drw_gpencil_engine_needed(depsgraph, v3d); - - DRWContext draw_ctx; - drw_set(draw_ctx); - - BKE_view_layer_synced_ensure(scene, view_layer); - drw_get().draw_ctx = {}; - drw_get().draw_ctx.region = region; - drw_get().draw_ctx.rv3d = rv3d; - drw_get().draw_ctx.v3d = v3d; - drw_get().draw_ctx.scene = scene; - drw_get().draw_ctx.view_layer = view_layer; - drw_get().draw_ctx.obact = BKE_view_layer_active_object_get(view_layer); - drw_get().draw_ctx.engine_type = engine_type; - drw_get().draw_ctx.depsgraph = depsgraph; - - /* Custom lightweight initialize to avoid resetting the memory-pools. */ - drw_get().viewport = viewport; - drw_get().data = drw_viewport_data_ensure(drw_get().viewport); - - /* Separate update for each stereo view. */ - int view_count = GPU_viewport_is_stereo_get(viewport) ? 2 : 1; - for (int view = 0; view < view_count; view++) { - drw_get().view_data_active = drw_get().data->view_data[view]; - - drw_engines_enable(view_layer, engine_type, gpencil_engine_needed); - drw_engines_data_validate(); - - DRW_view_data_engines_view_update(drw_get().view_data_active); - - drw_engines_disable(); - } - - drw_manager_exit(&draw_ctx); - } -} - -/** \} */ - /* -------------------------------------------------------------------- */ /** \name Callbacks * \{ */ @@ -1643,9 +1518,6 @@ void DRW_draw_render_loop_offscreen(Depsgraph *depsgraph, if (viewport == nullptr) { render_viewport = GPU_viewport_create(); } - else { - drw_notify_view_update_offscreen(depsgraph, engine_type, region, v3d, render_viewport); - } GPU_viewport_bind_from_offscreen(render_viewport, ofs, is_xr_surface); diff --git a/source/blender/editors/render/render_update.cc b/source/blender/editors/render/render_update.cc index 1d56c406c59..edc6ecf6d43 100644 --- a/source/blender/editors/render/render_update.cc +++ b/source/blender/editors/render/render_update.cc @@ -63,14 +63,12 @@ void ED_render_view3d_update(Depsgraph *depsgraph, { Main *bmain = DEG_get_bmain(depsgraph); Scene *scene = DEG_get_input_scene(depsgraph); - ViewLayer *view_layer = DEG_get_input_view_layer(depsgraph); LISTBASE_FOREACH (ARegion *, region, &area->regionbase) { if (region->regiontype != RGN_TYPE_WINDOW) { continue; } - View3D *v3d = static_cast(area->spacedata.first); RegionView3D *rv3d = static_cast(region->regiondata); RenderEngine *engine = rv3d->view_render ? RE_view_engine_get(rv3d->view_render) : nullptr; @@ -96,20 +94,6 @@ void ED_render_view3d_update(Depsgraph *depsgraph, CTX_free(C); } - - if (!updated) { - continue; - } - - DRWUpdateContext drw_context = {nullptr}; - drw_context.bmain = bmain; - drw_context.depsgraph = depsgraph; - drw_context.scene = scene; - drw_context.view_layer = view_layer; - drw_context.region = region; - drw_context.v3d = v3d; - drw_context.engine_type = ED_view3d_engine_type(scene, v3d->shading.type); - DRW_notify_view_update(&drw_context); } } diff --git a/source/blender/editors/space_view3d/space_view3d.cc b/source/blender/editors/space_view3d/space_view3d.cc index 4b0283ffb89..c8c9f545902 100644 --- a/source/blender/editors/space_view3d/space_view3d.cc +++ b/source/blender/editors/space_view3d/space_view3d.cc @@ -1392,27 +1392,6 @@ static void view3d_main_region_listener(const wmRegionListenerParams *params) } } -static void view3d_do_msg_notify_workbench_view_update(bContext *C, - wmMsgSubscribeKey * /*msg_key*/, - wmMsgSubscribeValue *msg_val) -{ - Scene *scene = CTX_data_scene(C); - ScrArea *area = (ScrArea *)msg_val->user_data; - View3D *v3d = (View3D *)area->spacedata.first; - if (v3d->shading.type == OB_SOLID) { - RenderEngineType *engine_type = ED_view3d_engine_type(scene, v3d->shading.type); - DRWUpdateContext drw_context = {nullptr}; - drw_context.bmain = CTX_data_main(C); - drw_context.depsgraph = CTX_data_depsgraph_pointer(C); - drw_context.scene = scene; - drw_context.view_layer = CTX_data_view_layer(C); - drw_context.region = (ARegion *)(msg_val->owner); - drw_context.v3d = v3d; - drw_context.engine_type = engine_type; - DRW_notify_view_update(&drw_context); - } -} - static void view3d_main_region_message_subscribe(const wmRegionMessageSubscribeParams *params) { wmMsgBus *mbus = params->message_bus; @@ -1453,11 +1432,6 @@ static void view3d_main_region_message_subscribe(const wmRegionMessageSubscribeP msg_sub_value_region_tag_redraw.user_data = region; msg_sub_value_region_tag_redraw.notify = ED_region_do_msg_notify_tag_redraw; - wmMsgSubscribeValue msg_sub_value_workbench_view_update{}; - msg_sub_value_workbench_view_update.owner = region; - msg_sub_value_workbench_view_update.user_data = area; - msg_sub_value_workbench_view_update.notify = view3d_do_msg_notify_workbench_view_update; - for (int i = 0; i < ARRAY_SIZE(type_array); i++) { msg_key_params.ptr.type = type_array[i]; WM_msg_subscribe_rna_params(mbus, &msg_key_params, &msg_sub_value_region_tag_redraw, __func__); @@ -1493,11 +1467,6 @@ static void view3d_main_region_message_subscribe(const wmRegionMessageSubscribeP case OB_MODE_PARTICLE_EDIT: WM_msg_subscribe_rna_anon_type(mbus, ParticleEdit, &msg_sub_value_region_tag_redraw); break; - - case OB_MODE_SCULPT: - WM_msg_subscribe_rna_anon_prop( - mbus, WorkSpace, tools, &msg_sub_value_workbench_view_update); - break; default: break; }