[Bf-blender-cvs] [aa4fb22cac0] master: Depsgraph: Use UUID to match modifiers

[Sergey Sharybin]

Commit: aa4fb22cac0c8b653cc9925700bfce74dcc08574
Author: Sergey Sharybin
Date:   Wed Aug 5 16:49:20 2020 +0200
Branches: master
https://developer.blender.org/rBaa4fb22cac0c8b653cc9925700bfce74dcc08574

Depsgraph: Use UUID to match modifiers

Solves possible pointer-based comparison fiasco.

Another nice outcome of this is that topology cache will now be
preserved throughout the undo system. For example, undo of object
transform will not require topology cache to be re-created.

Differential Revision: https://developer.blender.org/D8493

===================================================================

M	source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_modifier.cc
M	source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_modifier.h
M	source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_object.cc
M	source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_object.h

===================================================================

diff --git a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_modifier.cc b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_modifier.cc
index 934403674a9..25bcf2bfe72 100644
--- a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_modifier.cc
+++ b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_modifier.cc
@@ -23,28 +23,14 @@
 
 #include "intern/eval/deg_eval_runtime_backup_modifier.h"
 
+#include "DNA_modifier_types.h"
+
 namespace blender {
 namespace deg {
 
-ModifierDataBackupID::ModifierDataBackupID(const Depsgraph * /*depsgraph*/)
-    : ModifierDataBackupID(nullptr, eModifierType_None)
-{
-}
-
-ModifierDataBackupID::ModifierDataBackupID(ModifierData *modifier_data, ModifierType type)
-    : modifier_data(modifier_data), type(type)
-{
-}
-
-bool operator==(const ModifierDataBackupID &a, const ModifierDataBackupID &b)
-{
-  return a.modifier_data == b.modifier_data && a.type == b.type;
-}
-
-uint64_t ModifierDataBackupID::hash() const
+ModifierDataBackup::ModifierDataBackup(ModifierData *modifier_data)
+    : type(static_cast<ModifierType>(modifier_data->type)), runtime(modifier_data->runtime)
 {
-  uintptr_t ptr = (uintptr_t)modifier_data;
-  return (ptr >> 4) ^ (uintptr_t)type;
 }
 
 }  // namespace deg
diff --git a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_modifier.h b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_modifier.h
index a5bdf2359ee..f02dc73c392 100644
--- a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_modifier.h
+++ b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_modifier.h
@@ -25,38 +25,18 @@
 
 #include "BKE_modifier.h"
 
-#include "intern/depsgraph_type.h"
-
 struct ModifierData;
 
 namespace blender {
 namespace deg {
 
-struct Depsgraph;
-
-/* Identifier used to match modifiers to backup/restore their runtime data.
- * Identification is happening using original modifier data pointer and the
- * modifier type.
- * It is not enough to only pointer, since it's possible to have a situation
- * when modifier is removed and a new one added, and due to memory allocation
- * policy they might have same pointer.
- * By adding type into matching we are at least ensuring that modifier will not
- * try to interpret runtime data created by another modifier type. */
-class ModifierDataBackupID {
+class ModifierDataBackup {
  public:
-  ModifierDataBackupID(const Depsgraph *depsgraph);
-  ModifierDataBackupID(ModifierData *modifier_data, ModifierType type);
-
-  friend bool operator==(const ModifierDataBackupID &a, const ModifierDataBackupID &b);
+  explicit ModifierDataBackup(ModifierData *modifier_data);
 
-  uint64_t hash() const;
-
-  ModifierData *modifier_data;
   ModifierType type;
+  void *runtime;
 };
 
-/* Storage for backed up runtime modifier data. */
-typedef Map<ModifierDataBackupID, void *> ModifierRuntimeDataBackup;
-
 }  // namespace deg
 }  // namespace blender
diff --git a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_object.cc b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_object.cc
index 88334e41192..addee3dc539 100644
--- a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_object.cc
+++ b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_object.cc
@@ -62,21 +62,18 @@ void ObjectRuntimeBackup::init_from_object(Object *object)
   backup_pose_channel_runtime_data(object);
 }
 
-inline ModifierDataBackupID create_modifier_data_id(const ModifierData *modifier_data)
-{
-  return ModifierDataBackupID(modifier_data->orig_modifier_data,
-                              static_cast<ModifierType>(modifier_data->type));
-}
-
 void ObjectRuntimeBackup::backup_modifier_runtime_data(Object *object)
 {
   LISTBASE_FOREACH (ModifierData *, modifier_data, &object->modifiers) {
     if (modifier_data->runtime == nullptr) {
       continue;
     }
+
+    const SessionUUID &session_uuid = modifier_data->session_uuid;
+    BLI_assert(BLI_session_uuid_is_generated(&session_uuid));
+
     BLI_assert(modifier_data->orig_modifier_data != nullptr);
-    ModifierDataBackupID modifier_data_id = create_modifier_data_id(modifier_data);
-    modifier_runtime_data.add(modifier_data_id, modifier_data->runtime);
+    modifier_runtime_data.add(session_uuid, ModifierDataBackup(modifier_data));
     modifier_data->runtime = nullptr;
   }
 }
@@ -153,17 +150,17 @@ void ObjectRuntimeBackup::restore_modifier_runtime_data(Object *object)
 {
   LISTBASE_FOREACH (ModifierData *, modifier_data, &object->modifiers) {
     BLI_assert(modifier_data->orig_modifier_data != nullptr);
-    ModifierDataBackupID modifier_data_id = create_modifier_data_id(modifier_data);
-    void *runtime = modifier_runtime_data.pop_default(modifier_data_id, nullptr);
-    if (runtime != nullptr) {
-      modifier_data->runtime = runtime;
+    const SessionUUID &session_uuid = modifier_data->session_uuid;
+    optional<ModifierDataBackup> backup = modifier_runtime_data.pop_try(session_uuid);
+    if (backup.has_value()) {
+      modifier_data->runtime = backup->runtime;
     }
   }
 
-  for (ModifierRuntimeDataBackup::Item item : modifier_runtime_data.items()) {
-    const ModifierTypeInfo *modifier_type_info = BKE_modifier_get_info(item.key.type);
+  for (ModifierDataBackup &backup : modifier_runtime_data.values()) {
+    const ModifierTypeInfo *modifier_type_info = BKE_modifier_get_info(backup.type);
     BLI_assert(modifier_type_info != nullptr);
-    modifier_type_info->freeRuntimeData(item.value);
+    modifier_type_info->freeRuntimeData(backup.runtime);
   }
 }
 
diff --git a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_object.h b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_object.h
index a10f15634ce..5f6b443a2b2 100644
--- a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_object.h
+++ b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup_object.h
@@ -36,6 +36,8 @@ struct Object;
 namespace blender {
 namespace deg {
 
+struct Depsgraph;
+
 class ObjectRuntimeBackup {
  public:
   ObjectRuntimeBackup(const Depsgraph *depsgraph);
@@ -56,7 +58,7 @@ class ObjectRuntimeBackup {
   Object_Runtime runtime;
   short base_flag;
   unsigned short base_local_view_bits;
-  ModifierRuntimeDataBackup modifier_runtime_data;
+  Map<SessionUUID, ModifierDataBackup> modifier_runtime_data;
   Map<SessionUUID, bPoseChannel_Runtime> pose_channel_runtime_data;
 };