From a6a43b11fc2a459e99e0ced18c7f2c2adff78eda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20T=C3=B6nne?= Date: Wed, 26 Apr 2023 15:42:30 +0200 Subject: [PATCH 1/2] Fix crash in spreadsheet while baking simulation cache. The spreadsheet keeps a GVArray of the data during drawing, which is shared with the cache. Baking operator can update the cached data, making the spreadsheet copy invalid. Typical approach here: disable all UI updates while `G.rendering` is enabled, which also works as a "mutex" for baking. --- .../editors/space_spreadsheet/space_spreadsheet.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/source/blender/editors/space_spreadsheet/space_spreadsheet.cc b/source/blender/editors/space_spreadsheet/space_spreadsheet.cc index 5ea179dcd9a..95f9e12d127 100644 --- a/source/blender/editors/space_spreadsheet/space_spreadsheet.cc +++ b/source/blender/editors/space_spreadsheet/space_spreadsheet.cc @@ -4,6 +4,7 @@ #include "BLI_listbase.h" +#include "BKE_global.h" #include "BKE_lib_remap.h" #include "BKE_screen.h" @@ -417,6 +418,10 @@ static void update_visible_columns(ListBase &columns, DataSource &data_source) static void spreadsheet_main_region_draw(const bContext *C, ARegion *region) { + if (G.is_rendering) { + return; + } + SpaceSpreadsheet *sspreadsheet = CTX_wm_space_spreadsheet(C); sspreadsheet->runtime->cache.set_all_unused(); spreadsheet_update_context(C); @@ -513,6 +518,9 @@ static void spreadsheet_header_region_init(wmWindowManager * /*wm*/, ARegion *re static void spreadsheet_header_region_draw(const bContext *C, ARegion *region) { + if (G.is_rendering) { + return; + } spreadsheet_update_context(C); ED_region_header(C, region); } @@ -566,6 +574,9 @@ static void spreadsheet_footer_region_init(wmWindowManager * /*wm*/, ARegion *re static void spreadsheet_footer_region_draw(const bContext *C, ARegion *region) { + if (G.is_rendering) { + return; + } SpaceSpreadsheet *sspreadsheet = CTX_wm_space_spreadsheet(C); SpaceSpreadsheet_Runtime *runtime = sspreadsheet->runtime; std::stringstream ss; @@ -630,6 +641,9 @@ static void spreadsheet_dataset_region_listener(const wmRegionListenerParams *pa static void spreadsheet_dataset_region_draw(const bContext *C, ARegion *region) { + if (G.is_rendering) { + return; + } spreadsheet_update_context(C); ED_region_panels(C, region); } -- 2.30.2 From ffc7069f1b4ab5364994f1718156b12d993ca0cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20T=C3=B6nne?= Date: Wed, 26 Apr 2023 15:45:04 +0200 Subject: [PATCH 2/2] Fix crash in timeline during simulation cache bake. Timeline accesses the cache to determine baked frames, which can collide with the bake operator inserting new frames. The cache needs a mutex to prevent concurrent access to the cache frame list. --- .../blenkernel/BKE_simulation_state.hh | 1 + .../blenkernel/intern/simulation_state.cc | 44 ++++++++++++------- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/source/blender/blenkernel/BKE_simulation_state.hh b/source/blender/blenkernel/BKE_simulation_state.hh index db059764abc..731e67a0dfe 100644 --- a/source/blender/blenkernel/BKE_simulation_state.hh +++ b/source/blender/blenkernel/BKE_simulation_state.hh @@ -135,6 +135,7 @@ struct StatesAroundFrame { class ModifierSimulationCache { private: + mutable std::mutex states_at_frames_mutex_; Vector> states_at_frames_; std::unique_ptr bdata_sharing_; diff --git a/source/blender/blenkernel/intern/simulation_state.cc b/source/blender/blenkernel/intern/simulation_state.cc index 703a174648c..08260109637 100644 --- a/source/blender/blenkernel/intern/simulation_state.cc +++ b/source/blender/blenkernel/intern/simulation_state.cc @@ -59,24 +59,28 @@ void ModifierSimulationCache::try_discover_bake(const StringRefNull meta_dir, this->reset(); - for (const int i : IndexRange(dir_entries_num)) { - const direntry &dir_entry = dir_entries[i]; - const StringRefNull dir_entry_path = dir_entry.path; - if (!dir_entry_path.endswith(".json")) { - continue; + { + std::lock_guard lock(states_at_frames_mutex_); + + for (const int i : IndexRange(dir_entries_num)) { + const direntry &dir_entry = dir_entries[i]; + const StringRefNull dir_entry_path = dir_entry.path; + if (!dir_entry_path.endswith(".json")) { + continue; + } + char modified_file_name[FILENAME_MAX]; + BLI_strncpy(modified_file_name, dir_entry.relname, sizeof(modified_file_name)); + BLI_str_replace_char(modified_file_name, '_', '.'); + + const SubFrame frame = std::stof(modified_file_name); + + auto new_state_at_frame = std::make_unique(); + new_state_at_frame->frame = frame; + new_state_at_frame->state.bdata_dir_ = bdata_dir; + new_state_at_frame->state.meta_path_ = dir_entry.path; + new_state_at_frame->state.owner_ = this; + states_at_frames_.append(std::move(new_state_at_frame)); } - char modified_file_name[FILENAME_MAX]; - BLI_strncpy(modified_file_name, dir_entry.relname, sizeof(modified_file_name)); - BLI_str_replace_char(modified_file_name, '_', '.'); - - const SubFrame frame = std::stof(modified_file_name); - - auto new_state_at_frame = std::make_unique(); - new_state_at_frame->frame = frame; - new_state_at_frame->state.bdata_dir_ = bdata_dir; - new_state_at_frame->state.meta_path_ = dir_entry.path; - new_state_at_frame->state.owner_ = this; - states_at_frames_.append(std::move(new_state_at_frame)); } bdata_sharing_ = std::make_unique(); @@ -86,6 +90,7 @@ void ModifierSimulationCache::try_discover_bake(const StringRefNull meta_dir, bool ModifierSimulationCache::has_state_at_frame(const SubFrame &frame) const { + std::lock_guard lock(states_at_frames_mutex_); for (const auto &item : states_at_frames_) { if (item->frame == frame) { return true; @@ -96,12 +101,14 @@ bool ModifierSimulationCache::has_state_at_frame(const SubFrame &frame) const bool ModifierSimulationCache::has_states() const { + std::lock_guard lock(states_at_frames_mutex_); return !states_at_frames_.is_empty(); } const ModifierSimulationState *ModifierSimulationCache::get_state_at_exact_frame( const SubFrame &frame) const { + std::lock_guard lock(states_at_frames_mutex_); for (const auto &item : states_at_frames_) { if (item->frame == frame) { return &item->state; @@ -113,6 +120,7 @@ const ModifierSimulationState *ModifierSimulationCache::get_state_at_exact_frame ModifierSimulationState &ModifierSimulationCache::get_state_at_frame_for_write( const SubFrame &frame) { + std::lock_guard lock(states_at_frames_mutex_); for (const auto &item : states_at_frames_) { if (item->frame == frame) { return item->state; @@ -126,6 +134,7 @@ ModifierSimulationState &ModifierSimulationCache::get_state_at_frame_for_write( StatesAroundFrame ModifierSimulationCache::get_states_around_frame(const SubFrame &frame) const { + std::lock_guard lock(states_at_frames_mutex_); StatesAroundFrame states_around_frame; for (const auto &item : states_at_frames_) { if (item->frame < frame) { @@ -195,6 +204,7 @@ void ModifierSimulationState::ensure_bake_loaded() const void ModifierSimulationCache::reset() { + std::lock_guard lock(states_at_frames_mutex_); states_at_frames_.clear(); bdata_sharing_.reset(); cache_state_ = CacheState::Valid; -- 2.30.2