From 80f105e924fa2d0ec1880d453d23756caa6026a7 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Mon, 10 Jul 2023 16:02:31 +0200 Subject: [PATCH] Fix #109462: Incorrect "No Cache" simulation cache invalidation The "No Cache" simulation nodes option effectively changes the cache to work in a "realtime mode" where there are only two states, the current and previous frame. Whenever the current frame doesn't increase, the previous state should reset. This didn't happen properly, and it was hard to verify because the code was shared with the regular "cache on" mode. Instead, separate the caching more in the code, using a different struct to store the two "realtime" states. Also clarify that we don't support animation of the "No Cache" option by disabling support for that in RNA. Pull Request: https://projects.blender.org/blender/blender/pulls/109741 --- .../blenkernel/BKE_simulation_state.hh | 11 +- .../blenkernel/intern/simulation_state.cc | 11 +- source/blender/makesrna/intern/rna_object.cc | 3 +- source/blender/modifiers/intern/MOD_nodes.cc | 161 +++++++++++------- 4 files changed, 109 insertions(+), 77 deletions(-) diff --git a/source/blender/blenkernel/BKE_simulation_state.hh b/source/blender/blenkernel/BKE_simulation_state.hh index 65f788aae27..30c210154d7 100644 --- a/source/blender/blenkernel/BKE_simulation_state.hh +++ b/source/blender/blenkernel/BKE_simulation_state.hh @@ -150,6 +150,13 @@ struct StatesAroundFrame { const ModifierSimulationStateAtFrame *next = nullptr; }; +struct ModifierSimulationCacheRealtime { + std::unique_ptr prev_state; + std::unique_ptr current_state; + SubFrame prev_frame; + SubFrame current_frame; +}; + class ModifierSimulationCache { private: mutable std::mutex states_at_frames_mutex_; @@ -170,6 +177,9 @@ class ModifierSimulationCache { public: CacheState cache_state = CacheState::Valid; + /** A non-persistent cache used only to pass simulation state data from one frame to the next. */ + ModifierSimulationCacheRealtime realtime_cache; + void try_discover_bake(StringRefNull absolute_bake_dir); bool has_state_at_frame(const SubFrame &frame) const; @@ -184,7 +194,6 @@ class ModifierSimulationCache { } void reset(); - void clear_prev_states(); }; /** diff --git a/source/blender/blenkernel/intern/simulation_state.cc b/source/blender/blenkernel/intern/simulation_state.cc index ac4356a8901..f1998d51104 100644 --- a/source/blender/blenkernel/intern/simulation_state.cc +++ b/source/blender/blenkernel/intern/simulation_state.cc @@ -232,20 +232,13 @@ void ModifierSimulationState::ensure_bake_loaded(const bNodeTree &ntree) const bake_loaded_ = true; } -void ModifierSimulationCache::clear_prev_states() -{ - std::lock_guard lock(states_at_frames_mutex_); - std::unique_ptr temp = std::move(states_at_frames_.last()); - states_at_frames_.clear_and_shrink(); - bdata_sharing_.reset(); - states_at_frames_.append(std::move(temp)); -} - void ModifierSimulationCache::reset() { std::lock_guard lock(states_at_frames_mutex_); states_at_frames_.clear(); bdata_sharing_.reset(); + this->realtime_cache.current_state.reset(); + this->realtime_cache.prev_state.reset(); this->cache_state = CacheState::Valid; } diff --git a/source/blender/makesrna/intern/rna_object.cc b/source/blender/makesrna/intern/rna_object.cc index 9cc2443b9ca..65308e7b746 100644 --- a/source/blender/makesrna/intern/rna_object.cc +++ b/source/blender/makesrna/intern/rna_object.cc @@ -3506,7 +3506,8 @@ static void rna_def_object(BlenderRNA *brna) prop = RNA_def_property(srna, "use_simulation_cache", PROP_BOOLEAN, PROP_NONE); RNA_def_property_boolean_sdna(prop, nullptr, "flag", OB_FLAG_USE_SIMULATION_CACHE); RNA_def_property_ui_text( - prop, "Use Simulation Cache", "Cache all frames during simulation nodes playback"); + prop, "Use Simulation Cache", "Cache frames during simulation nodes playback"); + RNA_def_property_clear_flag(prop, PROP_ANIMATABLE); RNA_def_property_update(prop, NC_OBJECT | ND_DRAW, nullptr); rna_def_object_visibility(srna); diff --git a/source/blender/modifiers/intern/MOD_nodes.cc b/source/blender/modifiers/intern/MOD_nodes.cc index 1be2ec91596..fc78d9ab88a 100644 --- a/source/blender/modifiers/intern/MOD_nodes.cc +++ b/source/blender/modifiers/intern/MOD_nodes.cc @@ -677,78 +677,115 @@ static void prepare_simulation_states_for_evaluation(const NodesModifierData &nm } } - if (DEG_is_active(ctx.depsgraph)) { + if (ctx.object->flag & OB_FLAG_USE_SIMULATION_CACHE) { + if (DEG_is_active(ctx.depsgraph)) { - { - /* Invalidate cached data on user edits. */ - if (nmd.modifier.flag & eModifierFlag_UserModified) { - if (simulation_cache.cache_state != bke::sim::CacheState::Baked) { - simulation_cache.invalidate(); + { + /* Invalidate cached data on user edits. */ + if (nmd.modifier.flag & eModifierFlag_UserModified) { + if (simulation_cache.cache_state != bke::sim::CacheState::Baked) { + simulation_cache.invalidate(); + } } } - } - { - /* Reset cached data if necessary. */ + { + /* Reset cached data if necessary. */ + const bke::sim::StatesAroundFrame sim_states = simulation_cache.get_states_around_frame( + current_frame); + if (simulation_cache.cache_state == bke::sim::CacheState::Invalid && + (current_frame == start_frame || + (sim_states.current == nullptr && sim_states.prev == nullptr && + sim_states.next != nullptr))) + { + simulation_cache.reset(); + } + } + /* Decide if a new simulation state should be created in this evaluation. */ const bke::sim::StatesAroundFrame sim_states = simulation_cache.get_states_around_frame( current_frame); - if (simulation_cache.cache_state == bke::sim::CacheState::Invalid && - (current_frame == start_frame || - (sim_states.current == nullptr && sim_states.prev == nullptr && - sim_states.next != nullptr))) - { - simulation_cache.reset(); + if (simulation_cache.cache_state != bke::sim::CacheState::Baked) { + if (sim_states.current == nullptr) { + if (is_start_frame || !simulation_cache.has_states()) { + bke::sim::ModifierSimulationState ¤t_sim_state = + simulation_cache.get_state_at_frame_for_write(current_frame); + exec_data.current_simulation_state_for_write = ¤t_sim_state; + exec_data.simulation_time_delta = 0.0f; + if (!is_start_frame) { + /* When starting a new simulation at another frame than the start frame, + * it can't match what would be baked, so invalidate it immediately. */ + simulation_cache.invalidate(); + } + } + else if (sim_states.prev != nullptr && sim_states.next == nullptr) { + const float max_delta_frames = 1.0f; + const float scene_delta_frames = float(current_frame) - float(sim_states.prev->frame); + const float delta_frames = std::min(max_delta_frames, scene_delta_frames); + if (delta_frames != scene_delta_frames) { + simulation_cache.invalidate(); + } + bke::sim::ModifierSimulationState ¤t_sim_state = + simulation_cache.get_state_at_frame_for_write(current_frame); + exec_data.current_simulation_state_for_write = ¤t_sim_state; + const float delta_seconds = delta_frames / FPS; + exec_data.simulation_time_delta = delta_seconds; + } + } } } - /* Decide if a new simulation state should be created in this evaluation. */ + + /* Load read-only states to give nodes access to cached data. */ const bke::sim::StatesAroundFrame sim_states = simulation_cache.get_states_around_frame( current_frame); - if (simulation_cache.cache_state != bke::sim::CacheState::Baked) { - if (sim_states.current == nullptr) { - if (is_start_frame || !simulation_cache.has_states()) { - bke::sim::ModifierSimulationState ¤t_sim_state = - simulation_cache.get_state_at_frame_for_write(current_frame); - exec_data.current_simulation_state_for_write = ¤t_sim_state; - exec_data.simulation_time_delta = 0.0f; - if (!is_start_frame) { - /* When starting a new simulation at another frame than the start frame, it can't match - * what would be baked, so invalidate it immediately. */ - simulation_cache.invalidate(); - } - } - else if (sim_states.prev != nullptr && sim_states.next == nullptr) { - const float max_delta_frames = 1.0f; - const float scene_delta_frames = float(current_frame) - float(sim_states.prev->frame); - const float delta_frames = std::min(max_delta_frames, scene_delta_frames); - if (delta_frames != scene_delta_frames) { - simulation_cache.invalidate(); - } - bke::sim::ModifierSimulationState ¤t_sim_state = - simulation_cache.get_state_at_frame_for_write(current_frame); - exec_data.current_simulation_state_for_write = ¤t_sim_state; - const float delta_seconds = delta_frames / FPS; - exec_data.simulation_time_delta = delta_seconds; - } + if (sim_states.current) { + sim_states.current->state.ensure_bake_loaded(*nmd.node_group); + exec_data.current_simulation_state = &sim_states.current->state; + } + if (sim_states.prev) { + sim_states.prev->state.ensure_bake_loaded(*nmd.node_group); + exec_data.prev_simulation_state = &sim_states.prev->state; + if (sim_states.next) { + sim_states.next->state.ensure_bake_loaded(*nmd.node_group); + exec_data.next_simulation_state = &sim_states.next->state; + exec_data.simulation_state_mix_factor = + (float(current_frame) - float(sim_states.prev->frame)) / + (float(sim_states.next->frame) - float(sim_states.prev->frame)); } } } + else { + if (DEG_is_active(ctx.depsgraph)) { + bke::sim::ModifierSimulationCacheRealtime &realtime_cache = simulation_cache.realtime_cache; - /* Load read-only states to give nodes access to cached data. */ - const bke::sim::StatesAroundFrame sim_states = simulation_cache.get_states_around_frame( - current_frame); - if (sim_states.current) { - sim_states.current->state.ensure_bake_loaded(*nmd.node_group); - exec_data.current_simulation_state = &sim_states.current->state; - } - if (sim_states.prev) { - sim_states.prev->state.ensure_bake_loaded(*nmd.node_group); - exec_data.prev_simulation_state = &sim_states.prev->state; - if (sim_states.next) { - sim_states.next->state.ensure_bake_loaded(*nmd.node_group); - exec_data.next_simulation_state = &sim_states.next->state; - exec_data.simulation_state_mix_factor = - (float(current_frame) - float(sim_states.prev->frame)) / - (float(sim_states.next->frame) - float(sim_states.prev->frame)); + /* Reset the cache when going backwards in time. */ + if (realtime_cache.prev_frame >= current_frame) { + simulation_cache.reset(); + } + + /* Advance in time, making the last "current" state the new "previous" state. */ + realtime_cache.prev_frame = realtime_cache.current_frame; + realtime_cache.prev_state = std::move(realtime_cache.current_state); + if (realtime_cache.prev_state) { + exec_data.prev_simulation_state = realtime_cache.prev_state.get(); + } + + /* Create a new current state used to pass the data to the next frame. */ + realtime_cache.current_state = std::make_unique(); + realtime_cache.current_frame = current_frame; + exec_data.current_simulation_state_for_write = realtime_cache.current_state.get(); + exec_data.current_simulation_state = exec_data.current_simulation_state_for_write; + + /* Calculate the delta time. */ + if (realtime_cache.prev_state) { + const float max_delta_frames = 1.0f; + const float scene_delta_frames = float(current_frame) - float(realtime_cache.prev_frame); + const float delta_frames = std::min(max_delta_frames, scene_delta_frames); + const float delta_seconds = delta_frames / FPS; + exec_data.simulation_time_delta = delta_seconds; + } + else { + exec_data.simulation_time_delta = 0.0f; + } } } } @@ -841,14 +878,6 @@ static void modifyGeometry(ModifierData *md, nmd_orig->runtime_eval_log = eval_log.release(); } - if (DEG_is_active(ctx->depsgraph)) { - /* When caching is turned off, remove all states except the last which was just created in this - * evaluation. Check if active status to avoid changing original data in other depsgraphs. */ - if (!(ctx->object->flag & OB_FLAG_USE_SIMULATION_CACHE)) { - nmd_orig->simulation_cache->ptr->clear_prev_states(); - } - } - if (use_orig_index_verts || use_orig_index_edges || use_orig_index_polys) { if (Mesh *mesh = geometry_set.get_mesh_for_write()) { /* Add #CD_ORIGINDEX layers if they don't exist already. This is required because the