From 91de4a50abfc5ac88a5535e28e70947eaea7c447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Foucault?= Date: Thu, 20 Feb 2025 15:04:27 +0100 Subject: [PATCH] Cleanup: DRW: Make Open subdiv evaluator part of a DRW module This avoids the global variable access with race condition. Rel #134690 Pull Request: https://projects.blender.org/blender/blender/pulls/134855 --- source/blender/draw/DRW_engine.hh | 4 +- .../intern/draw_cache_impl_subdivision.cc | 38 +++++++++++++------ source/blender/draw/intern/draw_common_c.hh | 5 +++ source/blender/draw/intern/draw_manager_c.cc | 1 + source/blender/draw/intern/draw_manager_c.hh | 2 + .../windowmanager/intern/wm_init_exit.cc | 2 +- 6 files changed, 37 insertions(+), 15 deletions(-) diff --git a/source/blender/draw/DRW_engine.hh b/source/blender/draw/DRW_engine.hh index c81c4c2c72b..3d734636752 100644 --- a/source/blender/draw/DRW_engine.hh +++ b/source/blender/draw/DRW_engine.hh @@ -171,11 +171,9 @@ void DRW_cache_free_old_batches(Main *bmain); namespace blender::draw { +/* Free garbage collected subdivision data. */ void DRW_cache_free_old_subdiv(); -/* For the OpenGL evaluators and garbage collected subdivision data. */ -void DRW_subdiv_free(); - } // namespace blender::draw /* Never use this. Only for closing blender. */ diff --git a/source/blender/draw/intern/draw_cache_impl_subdivision.cc b/source/blender/draw/intern/draw_cache_impl_subdivision.cc index 78271f4db0a..48ea8520672 100644 --- a/source/blender/draw/intern/draw_cache_impl_subdivision.cc +++ b/source/blender/draw/intern/draw_cache_impl_subdivision.cc @@ -2,6 +2,7 @@ * * SPDX-License-Identifier: GPL-2.0-or-later */ +#include "draw_manager_c.hh" #include "draw_subdivision.hh" #include "DNA_mesh_types.h" @@ -43,6 +44,7 @@ #include "draw_cache_extract.hh" #include "draw_cache_impl.hh" #include "draw_cache_inline.hh" +#include "draw_common_c.hh" #include "draw_shader.hh" #include "draw_subdiv_shader_shared.hh" #include "mesh_extractors/extract_mesh.hh" @@ -1926,7 +1928,18 @@ void DRW_subdivide_loose_geom(DRWSubdivCache &subdiv_cache, const MeshBufferCach }); } -static OpenSubdiv_EvaluatorCache *g_evaluator_cache = nullptr; +/** + * OpenSubdiv GPU evaluation module. + */ +struct SubdivModule { + OpenSubdiv_EvaluatorCache *evaluator_cache = openSubdiv_createEvaluatorCache( + OPENSUBDIV_EVALUATOR_GPU); + + ~SubdivModule() + { + openSubdiv_deleteEvaluatorCache(evaluator_cache); + } +}; void DRW_create_subdivision(Object &ob, Mesh &mesh, @@ -1941,8 +1954,9 @@ void DRW_create_subdivision(Object &ob, const ToolSettings *ts, const bool use_hide) { - if (g_evaluator_cache == nullptr) { - g_evaluator_cache = openSubdiv_createEvaluatorCache(OPENSUBDIV_EVALUATOR_GPU); + DRWData &drw_data = *DST.vmempool; + if (drw_data.subdiv_module == nullptr) { + drw_data.subdiv_module = MEM_new("SubdivModule"); } #undef TIME_SUBDIV @@ -1963,7 +1977,7 @@ void DRW_create_subdivision(Object &ob, do_cage, ts, use_hide, - g_evaluator_cache)) + drw_data.subdiv_module->evaluator_cache)) { return; } @@ -1975,16 +1989,18 @@ void DRW_create_subdivision(Object &ob, #endif } -void DRW_subdiv_free() +void DRW_subdiv_module_free(SubdivModule *subdiv_module) { - DRW_cache_free_old_subdiv(); - - if (g_evaluator_cache) { - openSubdiv_deleteEvaluatorCache(g_evaluator_cache); - g_evaluator_cache = nullptr; - } + MEM_delete(subdiv_module); } +/** + * The `bke::subdiv::Subdiv` data is being owned the modifier. + * Since the modifier can be freed from any thread (e.g. from depsgraph multithreaded update) + * which may not have a valid GPUContext active, we move the data to discard to this free list + * until a codepath with a active GPUContext is hit. + * This is kindof garbage collection. + */ static LinkNode *gpu_subdiv_free_queue = nullptr; static ThreadMutex gpu_subdiv_queue_mutex = BLI_MUTEX_INITIALIZER; diff --git a/source/blender/draw/intern/draw_common_c.hh b/source/blender/draw/intern/draw_common_c.hh index 9be9b8eed5a..8dedebab020 100644 --- a/source/blender/draw/intern/draw_common_c.hh +++ b/source/blender/draw/intern/draw_common_c.hh @@ -29,6 +29,7 @@ namespace blender::draw { class Manager; struct CurvesModule; struct PointCloudModule; +struct SubdivModule; struct VolumeModule; } // namespace blender::draw @@ -71,6 +72,10 @@ void DRW_pointcloud_module_free(draw::PointCloudModule *module); void DRW_volume_init(DRWData *drw_data = nullptr); void DRW_volume_module_free(draw::VolumeModule *module); +/* draw_cache_impl_subdivision.cc */ + +void DRW_subdiv_module_free(draw::SubdivModule *module); + } // namespace blender::draw /* `draw_fluid.cc` */ diff --git a/source/blender/draw/intern/draw_manager_c.cc b/source/blender/draw/intern/draw_manager_c.cc index 924ab8a81a7..99e227904de 100644 --- a/source/blender/draw/intern/draw_manager_c.cc +++ b/source/blender/draw/intern/draw_manager_c.cc @@ -355,6 +355,7 @@ void DRW_viewport_data_free(DRWData *drw_data) DRW_volume_module_free(drw_data->volume_module); DRW_pointcloud_module_free(drw_data->pointcloud_module); DRW_curves_module_free(drw_data->curves_module); + DRW_subdiv_module_free(drw_data->subdiv_module); delete drw_data->default_view; MEM_freeN(drw_data); } diff --git a/source/blender/draw/intern/draw_manager_c.hh b/source/blender/draw/intern/draw_manager_c.hh index 7ff4f2b707c..4da158bffb7 100644 --- a/source/blender/draw/intern/draw_manager_c.hh +++ b/source/blender/draw/intern/draw_manager_c.hh @@ -32,6 +32,7 @@ struct Object; struct Mesh; namespace blender::draw { struct CurvesModule; +struct SubdivModule; struct VolumeModule; struct PointCloudModule; struct DRW_Attributes; @@ -72,6 +73,7 @@ struct DRWData { DRWViewData *view_data[2]; /** Module storage. */ blender::draw::CurvesModule *curves_module; + blender::draw::SubdivModule *subdiv_module; blender::draw::VolumeModule *volume_module; blender::draw::PointCloudModule *pointcloud_module; /** Default view that feeds every engine. */ diff --git a/source/blender/windowmanager/intern/wm_init_exit.cc b/source/blender/windowmanager/intern/wm_init_exit.cc index 30f10730f15..361900bc41d 100644 --- a/source/blender/windowmanager/intern/wm_init_exit.cc +++ b/source/blender/windowmanager/intern/wm_init_exit.cc @@ -596,7 +596,7 @@ void WM_exit_ex(bContext *C, const bool do_python_exit, const bool do_user_exit_ /* Free the GPU subdivision data after the database to ensure that subdivision structs used by * the modifiers were garbage collected. */ if (gpu_is_init) { - blender::draw::DRW_subdiv_free(); + blender::draw::DRW_cache_free_old_subdiv(); } ANIM_fcurves_copybuf_free();