From a5f4f1e2cea831415c045be4927320dc060798c9 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Wed, 4 Nov 2015 21:30:25 +0500 Subject: OpenSubdiv: Use pool for delayed OpenGL buffers free when freeing from non-main thread This is really similar to what GPU module was already doing. There are number of possible improvements still: - Re-use allocated VAOs when requesting new ones instead of going to the trouble of freeing VAO and then re-creating it again. - Move VAO handling to GPU module. Fixes T46589: OpenSubdiv crash with drivers --- source/blender/blenkernel/BKE_subsurf.h | 6 ++ source/blender/blenkernel/intern/CCGSubSurf.c | 5 +- .../blender/blenkernel/intern/CCGSubSurf_intern.h | 8 ++ .../blenkernel/intern/CCGSubSurf_opensubdiv.c | 88 +++++++++++++++++++++- source/blender/blenkernel/intern/scene.c | 64 ---------------- source/blender/windowmanager/CMakeLists.txt | 3 - source/blender/windowmanager/SConscript | 1 - source/blender/windowmanager/intern/wm_draw.c | 8 ++ source/blender/windowmanager/intern/wm_init_exit.c | 6 +- 9 files changed, 112 insertions(+), 77 deletions(-) (limited to 'source') diff --git a/source/blender/blenkernel/BKE_subsurf.h b/source/blender/blenkernel/BKE_subsurf.h index 161ab2f2fbd..f52bb2ab9cb 100644 --- a/source/blender/blenkernel/BKE_subsurf.h +++ b/source/blender/blenkernel/BKE_subsurf.h @@ -140,5 +140,11 @@ typedef struct CCGDerivedMesh { struct EdgeHash *ehash; } CCGDerivedMesh; +#ifdef WITH_OPENSUBDIV +/* TODO(sergey): Not really ideal place, but we don't currently have better one. */ +void BKE_subsurf_osd_init(void); +void BKE_subsurf_free_unused_buffers(void); +void BKE_subsurf_osd_cleanup(void); #endif +#endif diff --git a/source/blender/blenkernel/intern/CCGSubSurf.c b/source/blender/blenkernel/intern/CCGSubSurf.c index 95ddb9d5498..828a6bb16ac 100644 --- a/source/blender/blenkernel/intern/CCGSubSurf.c +++ b/source/blender/blenkernel/intern/CCGSubSurf.c @@ -334,11 +334,10 @@ void ccgSubSurf_free(CCGSubSurf *ss) openSubdiv_deleteEvaluatorDescr(ss->osd_evaluator); } if (ss->osd_mesh != NULL) { - /* TODO(sergey): Make sure free happens form the main thread! */ - openSubdiv_deleteOsdGLMesh(ss->osd_mesh); + ccgSubSurf__delete_osdGLMesh(ss->osd_mesh); } if (ss->osd_vao != 0) { - glDeleteVertexArrays(1, &ss->osd_vao); + ccgSubSurf__delete_vertex_array(ss->osd_vao); } if (ss->osd_coarse_coords != NULL) { MEM_freeN(ss->osd_coarse_coords); diff --git a/source/blender/blenkernel/intern/CCGSubSurf_intern.h b/source/blender/blenkernel/intern/CCGSubSurf_intern.h index e2b42065382..e0771020350 100644 --- a/source/blender/blenkernel/intern/CCGSubSurf_intern.h +++ b/source/blender/blenkernel/intern/CCGSubSurf_intern.h @@ -303,6 +303,14 @@ void ccgSubSurf__sync_legacy(CCGSubSurf *ss); void ccgSubSurf__sync_opensubdiv(CCGSubSurf *ss); +/* Delayed free routines. Will do actual free if called from + * main thread and schedule free for later free otherwise. + */ + +void ccgSubSurf__delete_osdGLMesh(struct OpenSubdiv_GLMesh *osd_mesh); +void ccgSubSurf__delete_vertex_array(unsigned int vao); +void ccgSubSurf__delete_pending(void); + /* * CCGSubSurf_opensubdiv_converter.c * */ struct OpenSubdiv_Converter; diff --git a/source/blender/blenkernel/intern/CCGSubSurf_opensubdiv.c b/source/blender/blenkernel/intern/CCGSubSurf_opensubdiv.c index 9d5ef1079b8..0f61a50ed13 100644 --- a/source/blender/blenkernel/intern/CCGSubSurf_opensubdiv.c +++ b/source/blender/blenkernel/intern/CCGSubSurf_opensubdiv.c @@ -28,12 +28,15 @@ #include "BLI_sys_types.h" // for intptr_t support #include "BLI_utildefines.h" /* for BLI_assert */ +#include "BLI_listbase.h" #include "BLI_math.h" +#include "BLI_threads.h" #include "CCGSubSurf.h" #include "CCGSubSurf_intern.h" #include "BKE_DerivedMesh.h" +#include "BKE_subsurf.h" #include "DNA_userdef_types.h" @@ -236,7 +239,7 @@ bool ccgSubSurf_prepareGLMesh(CCGSubSurf *ss, bool use_osd_glsl) if (ss->osd_mesh_invalid) { if (ss->osd_mesh != NULL) { - openSubdiv_deleteOsdGLMesh(ss->osd_mesh); + ccgSubSurf__delete_osdGLMesh(ss->osd_mesh); ss->osd_mesh = NULL; } ss->osd_mesh_invalid = false; @@ -897,8 +900,7 @@ void ccgSubSurf__sync_opensubdiv(CCGSubSurf *ss) void ccgSubSurf_free_osd_mesh(CCGSubSurf *ss) { if (ss->osd_mesh != NULL) { - /* TODO(sergey): Make sure free happens form the main thread! */ - openSubdiv_deleteOsdGLMesh(ss->osd_mesh); + ccgSubSurf__delete_osdGLMesh(ss->osd_mesh); ss->osd_mesh = NULL; } if (ss->osd_vao != 0) { @@ -921,4 +923,84 @@ void ccgSubSurf_getMinMax(CCGSubSurf *ss, float r_min[3], float r_max[3]) } } +/* ** Delayed delete routines ** */ + +typedef struct OsdDeletePendingItem { + struct OsdDeletePendingItem *next, *prev; + OpenSubdiv_GLMesh *osd_mesh; + unsigned int vao; +} OsdDeletePendingItem; + +static SpinLock delete_spin; +static ListBase delete_pool = {NULL, NULL}; + +static void delete_pending_push(OpenSubdiv_GLMesh *osd_mesh, + unsigned int vao) +{ + OsdDeletePendingItem *new_entry = MEM_mallocN(sizeof(OsdDeletePendingItem), + "opensubdiv delete entry"); + new_entry->osd_mesh = osd_mesh; + new_entry->vao = vao; + BLI_spin_lock(&delete_spin); + BLI_addtail(&delete_pool, new_entry); + BLI_spin_unlock(&delete_spin); +} + +void ccgSubSurf__delete_osdGLMesh(OpenSubdiv_GLMesh *osd_mesh) +{ + if (BLI_thread_is_main()) { + openSubdiv_deleteOsdGLMesh(osd_mesh); + } + else { + delete_pending_push(osd_mesh, 0); + } +} + +void ccgSubSurf__delete_vertex_array(unsigned int vao) +{ + if (BLI_thread_is_main()) { + glDeleteVertexArrays(1, &vao); + } + else { + delete_pending_push(NULL, vao); + } +} + +void ccgSubSurf__delete_pending(void) +{ + OsdDeletePendingItem *entry; + BLI_assert(BLI_thread_is_main()); + BLI_spin_lock(&delete_spin); + for (entry = delete_pool.first; entry != NULL; entry = entry->next) { + if (entry->osd_mesh != NULL) { + openSubdiv_deleteOsdGLMesh(entry->osd_mesh); + } + if (entry->vao != 0) { + glDeleteVertexArrays(1, &entry->vao); + } + } + BLI_freelistN(&delete_pool); + BLI_spin_unlock(&delete_spin); +} + +/* ** Public API ** */ + +void BKE_subsurf_osd_init(void) +{ + openSubdiv_init(); + BLI_spin_init(&delete_spin); +} + +void BKE_subsurf_free_unused_buffers(void) +{ + ccgSubSurf__delete_pending(); +} + +void BKE_subsurf_osd_cleanup(void) +{ + openSubdiv_cleanup(); + ccgSubSurf__delete_pending(); + BLI_spin_end(&delete_spin); +} + #endif /* WITH_OPENSUBDIV */ diff --git a/source/blender/blenkernel/intern/scene.c b/source/blender/blenkernel/intern/scene.c index b3fa6018517..47f9aecfc63 100644 --- a/source/blender/blenkernel/intern/scene.c +++ b/source/blender/blenkernel/intern/scene.c @@ -1376,11 +1376,6 @@ static void scene_do_rb_simulation_recursive(Scene *scene, float ctime) */ #define MBALL_SINGLETHREAD_HACK -/* Need this because CCFDM holds some OpenGL resources. */ -#ifdef WITH_OPENSUBDIV -# define OPENSUBDIV_GL_WORKAROUND -#endif - #ifdef WITH_LEGACY_DEPSGRAPH typedef struct StatisicsEntry { struct StatisicsEntry *next, *prev; @@ -1582,56 +1577,6 @@ static bool scene_need_update_objects(Main *bmain) DAG_id_type_tagged(bmain, ID_AR); /* Armature */ } -#ifdef OPENSUBDIV_GL_WORKAROUND -/* CCG DrivedMesh currently hold some OpenGL handles, which could only be - * released from the main thread. - * - * Ideally we need to use gpu_buffer_free, but it's a bit tricky because - * some buffers are only accessible from OpenSubdiv side. - */ -static void scene_free_unused_opensubdiv_cache(Scene *scene) -{ - Base *base; - for (base = scene->base.first; base; base = base->next) { - Object *object = base->object; - if (object->type == OB_MESH && object->recalc & OB_RECALC_DATA) { - ModifierData *md = object->modifiers.last; - if (md != NULL && md->type == eModifierType_Subsurf) { - SubsurfModifierData *smd = (SubsurfModifierData *) md; - const bool object_in_editmode = (object->mode == OB_MODE_EDIT); - const bool use_simple = (smd->subdivType == ME_SIMPLE_SUBSURF); - if (!smd->use_opensubdiv || - DAG_get_eval_flags_for_object(scene, object) & DAG_EVAL_NEED_CPU) - { - if (smd->mCache != NULL) { - ccgSubSurf_free_osd_mesh(smd->mCache); - } - if (smd->emCache != NULL) { - ccgSubSurf_free_osd_mesh(smd->emCache); - } - } - if (smd->mCache != NULL) { - if (object_in_editmode || - ccgSubSurf_getSimpleSubdiv(smd->mCache) != use_simple) - { - ccgSubSurf_free(smd->mCache); - smd->mCache = NULL; - } - } - if (smd->emCache != NULL) { - if (!object_in_editmode || - ccgSubSurf_getSimpleSubdiv(smd->emCache) != use_simple) - { - ccgSubSurf_free(smd->emCache); - smd->emCache = NULL; - } - } - } - } - } -} -#endif - static void scene_update_objects(EvaluationContext *eval_ctx, Main *bmain, Scene *scene, Scene *scene_parent) { TaskScheduler *task_scheduler = BLI_task_scheduler_get(); @@ -1650,10 +1595,6 @@ static void scene_update_objects(EvaluationContext *eval_ctx, Main *bmain, Scene return; } -#ifdef OPENSUBDIV_GL_WORKAROUND - scene_free_unused_opensubdiv_cache(scene); -#endif - state.eval_ctx = eval_ctx; state.scene = scene; state.scene_parent = scene_parent; @@ -1827,11 +1768,6 @@ void BKE_scene_update_tagged(EvaluationContext *eval_ctx, Main *bmain, Scene *sc else #endif { -#ifdef OPENSUBDIV_GL_WORKAROUND - if (DEG_needs_eval(scene->depsgraph)) { - scene_free_unused_opensubdiv_cache(scene); - } -#endif DEG_evaluate_on_refresh(eval_ctx, scene->depsgraph, scene); /* TODO(sergey): This is to beocme a node in new depsgraph. */ BKE_mask_update_scene(bmain, scene); diff --git a/source/blender/windowmanager/CMakeLists.txt b/source/blender/windowmanager/CMakeLists.txt index c5e8cbf1260..eaea70adc83 100644 --- a/source/blender/windowmanager/CMakeLists.txt +++ b/source/blender/windowmanager/CMakeLists.txt @@ -142,9 +142,6 @@ endif() if(WITH_OPENSUBDIV) add_definitions(-DWITH_OPENSUBDIV) - list(APPEND INC - ../../../intern/opensubdiv - ) endif() if(WIN32) diff --git a/source/blender/windowmanager/SConscript b/source/blender/windowmanager/SConscript index ec1b265d800..1a058ca3136 100644 --- a/source/blender/windowmanager/SConscript +++ b/source/blender/windowmanager/SConscript @@ -93,6 +93,5 @@ if env['OURPLATFORM'] in ('linux', 'openbsd3', 'sunos5', 'freebsd7', 'freebsd8', if env['WITH_BF_OPENSUBDIV']: defs.append("WITH_OPENSUBDIV") - incs += ' #intern/opensubdiv' env.BlenderLib ( 'bf_windowmanager', sources, Split(incs), defines=defs, libtype=['core'], priority=[5] ) diff --git a/source/blender/windowmanager/intern/wm_draw.c b/source/blender/windowmanager/intern/wm_draw.c index 16fe9ca5142..6ad0405c56f 100644 --- a/source/blender/windowmanager/intern/wm_draw.c +++ b/source/blender/windowmanager/intern/wm_draw.c @@ -69,6 +69,10 @@ #include "wm_window.h" #include "wm_event_system.h" +#ifdef WITH_OPENSUBDIV +# include "BKE_subsurf.h" +#endif + /* swap */ #define WIN_NONE_OK 0 #define WIN_BACK_OK 1 @@ -1002,6 +1006,10 @@ void wm_draw_update(bContext *C) wmWindow *win; int drawmethod; +#ifdef WITH_OPENSUBDIV + BKE_subsurf_free_unused_buffers(); +#endif + GPU_free_unused_buffers(); for (win = wm->windows.first; win; win = win->next) { diff --git a/source/blender/windowmanager/intern/wm_init_exit.c b/source/blender/windowmanager/intern/wm_init_exit.c index ba4a807dbd7..d528b658836 100644 --- a/source/blender/windowmanager/intern/wm_init_exit.c +++ b/source/blender/windowmanager/intern/wm_init_exit.c @@ -118,7 +118,7 @@ #include "COM_compositor.h" #ifdef WITH_OPENSUBDIV -# include "opensubdiv_capi.h" +# include "BKE_subsurf.h" #endif static void wm_init_reports(bContext *C) @@ -196,7 +196,7 @@ void WM_init(bContext *C, int argc, const char **argv) GPU_set_gpu_mipmapping(U.use_gpu_mipmap); #ifdef WITH_OPENSUBDIV - openSubdiv_init(); + BKE_subsurf_osd_init(); #endif UI_init(); @@ -547,7 +547,7 @@ void WM_exit_ext(bContext *C, const bool do_python) #endif #ifdef WITH_OPENSUBDIV - openSubdiv_cleanup(); + BKE_subsurf_osd_cleanup(); #endif if (!G.background) { -- cgit v1.2.3