Commit: 59bcca3016a07abe61ee8bca792c7f3e9a815e57
Author: Bastien Montagne
Date:   Sun Dec 25 23:23:40 2022 +0900
Branches: T102766
https://developer.blender.org/rB59bcca3016a07abe61ee8bca792c7f3e9a815e57

Refactor liboverride diffing, by un-threading restoration step.

Attempt to fix T102766, which reported backtrace strongly points at some
concurrency issues within exisitng liboverride diffing code that
restores forbidden changes to reference linked data values.

This commit instead add tags to mark liboverrides/properties that need
to be restored, and do so in a separate new step of diffing, from the
main thread only.

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

M       source/blender/blenkernel/BKE_lib_override.h
M       source/blender/blenkernel/intern/lib_override.cc
M       source/blender/makesdna/DNA_ID.h
M       source/blender/makesrna/RNA_access.h
M       source/blender/makesrna/intern/rna_access_compare_override.c

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

diff --git a/source/blender/blenkernel/BKE_lib_override.h 
b/source/blender/blenkernel/BKE_lib_override.h
index a98984250b9..4928ffda0ec 100644
--- a/source/blender/blenkernel/BKE_lib_override.h
+++ b/source/blender/blenkernel/BKE_lib_override.h
@@ -410,8 +410,6 @@ bool BKE_lib_override_library_status_check_reference(struct 
Main *bmain, struct
  * \note This is by far the biggest operation (the more time-consuming) of the 
three so far,
  * since it has to go over all properties in depth (all overridable ones at 
least).
  * Generating differential values and applying overrides are much cheaper.
- *
- * \return true if any library operation was created.
  */
 void BKE_lib_override_library_operations_create(struct Main *bmain,
                                                 struct ID *local,
@@ -425,6 +423,29 @@ void 
BKE_lib_override_library_main_operations_create(struct Main *bmain,
                                                      bool force_auto,
                                                      int *r_report_flags);
 
+/**
+ * Restore forbidden modified override properties to the values of their 
matching properties in the
+ * linked reference ID.
+ *
+ * \param r_report_flags #eRNAOverrideMatchResult flags giving info about the 
result of this call.
+ *
+ * \note Typically used as part of 
BKE_lib_override_library_main_operations_create process, since
+ * modifying RNA properties from non-main threads is not safe.
+ */
+void BKE_lib_override_library_operations_restore(struct Main *bmain,
+                                                 struct ID *local,
+                                                 int *r_report_flags);
+/**
+ * Restore forbidden modified override properties to the values of their 
matching properties in the
+ * linked reference ID, for all liboverride IDs tagged as needing such process 
in given `bmain`.
+ *
+ * \param r_report_flags #eRNAOverrideMatchResult flags giving info about the 
result of this call.
+ *
+ * \note Typically used as part of 
BKE_lib_override_library_main_operations_create process, since
+ * modifying RNA properties from non-main threads is not safe.
+ */
+void BKE_lib_override_library_main_operations_restore(struct Main *bmain, int 
*r_report_flags);
+
 /**
  * Reset all overrides in given \a id_root, while preserving ID relations.
  *
diff --git a/source/blender/blenkernel/intern/lib_override.cc 
b/source/blender/blenkernel/intern/lib_override.cc
index ce7abe63eac..0cbed770ed1 100644
--- a/source/blender/blenkernel/intern/lib_override.cc
+++ b/source/blender/blenkernel/intern/lib_override.cc
@@ -3297,7 +3297,10 @@ bool 
BKE_lib_override_library_status_check_reference(Main *bmain, ID *local)
   return true;
 }
 
-void BKE_lib_override_library_operations_create(Main *bmain, ID *local, int 
*r_report_flags)
+static void lib_override_library_operations_create(Main *bmain,
+                                                   ID *local,
+                                                   const eRNAOverrideMatch 
override_match_flags,
+                                                   eRNAOverrideMatchResult 
*r_report_flags)
 {
   BLI_assert(!ID_IS_LINKED(local));
   BLI_assert(local->override_library != nullptr);
@@ -3330,19 +3333,24 @@ void BKE_lib_override_library_operations_create(Main 
*bmain, ID *local, int *r_r
     RNA_id_pointer_create(local->override_library->reference, 
&rnaptr_reference);
 
     eRNAOverrideMatchResult local_report_flags = 
RNA_OVERRIDE_MATCH_RESULT_INIT;
-    RNA_struct_override_matches(
-        bmain,
-        &rnaptr_local,
-        &rnaptr_reference,
-        nullptr,
-        0,
-        local->override_library,
-        (eRNAOverrideMatch)(RNA_OVERRIDE_COMPARE_CREATE | 
RNA_OVERRIDE_COMPARE_RESTORE),
-        &local_report_flags);
+    RNA_struct_override_matches(bmain,
+                                &rnaptr_local,
+                                &rnaptr_reference,
+                                nullptr,
+                                0,
+                                local->override_library,
+                                override_match_flags,
+                                &local_report_flags);
 
     if (local_report_flags & RNA_OVERRIDE_MATCH_RESULT_RESTORED) {
       CLOG_INFO(&LOG, 2, "We did restore some properties of %s from its 
reference", local->name);
     }
+    if (local_report_flags & RNA_OVERRIDE_MATCH_RESULT_RESTORE_TAGGED) {
+      CLOG_INFO(&LOG,
+                2,
+                "We did tag some properties of %s for restoration from its 
reference",
+                local->name);
+    }
     if (local_report_flags & RNA_OVERRIDE_MATCH_RESULT_CREATED) {
       CLOG_INFO(&LOG, 2, "We did generate library override rules for %s", 
local->name);
     }
@@ -3351,9 +3359,49 @@ void BKE_lib_override_library_operations_create(Main 
*bmain, ID *local, int *r_r
     }
 
     if (r_report_flags != nullptr) {
-      *r_report_flags |= local_report_flags;
+      *r_report_flags = static_cast<eRNAOverrideMatchResult>(*r_report_flags | 
local_report_flags);
+    }
+  }
+}
+void BKE_lib_override_library_operations_create(Main *bmain, ID *local, int 
*r_report_flags)
+{
+  lib_override_library_operations_create(
+      bmain,
+      local,
+      static_cast<eRNAOverrideMatch>(RNA_OVERRIDE_COMPARE_CREATE | 
RNA_OVERRIDE_COMPARE_RESTORE),
+      reinterpret_cast<eRNAOverrideMatchResult *>(r_report_flags));
+}
+
+void BKE_lib_override_library_operations_restore(Main *bmain, ID *local, int 
*r_report_flags)
+{
+  if (!ID_IS_OVERRIDE_LIBRARY_REAL(local) || 
(local->override_library->runtime->tag &
+                                              
IDOVERRIDE_LIBRARY_RUNTIME_TAG_NEEDS_RESTORE) == 0) {
+    return;
+  }
+
+  PointerRNA rnaptr_src, rnaptr_dst;
+  RNA_id_pointer_create(local, &rnaptr_dst);
+  RNA_id_pointer_create(local->override_library->reference, &rnaptr_src);
+  RNA_struct_override_apply(
+      bmain,
+      &rnaptr_dst,
+      &rnaptr_src,
+      nullptr,
+      local->override_library,
+      
static_cast<eRNAOverrideApplyFlag>(RNA_OVERRIDE_APPLY_FLAG_SKIP_RESYNC_CHECK |
+                                         
RNA_OVERRIDE_APPLY_FLAG_RESTORE_ONLY));
+
+  LISTBASE_FOREACH_MUTABLE (
+      IDOverrideLibraryProperty *, op, &local->override_library->properties) {
+    if (op->tag & IDOVERRIDE_LIBRARY_PROPERTY_TAG_NEEDS_RETORE) {
+      BKE_lib_override_library_property_delete(local->override_library, op);
     }
   }
+  local->override_library->runtime->tag &= 
~IDOVERRIDE_LIBRARY_RUNTIME_TAG_NEEDS_RESTORE;
+
+  if (r_report_flags != nullptr) {
+    *r_report_flags |= RNA_OVERRIDE_MATCH_RESULT_RESTORED;
+  }
 }
 
 struct LibOverrideOpCreateData {
@@ -3368,8 +3416,12 @@ static void 
lib_override_library_operations_create_cb(TaskPool *__restrict pool,
   ID *id = static_cast<ID *>(taskdata);
 
   eRNAOverrideMatchResult report_flags = RNA_OVERRIDE_MATCH_RESULT_INIT;
-  BKE_lib_override_library_operations_create(
-      create_data->bmain, id, reinterpret_cast<int *>(&report_flags));
+  lib_override_library_operations_create(
+      create_data->bmain,
+      id,
+      static_cast<eRNAOverrideMatch>(RNA_OVERRIDE_COMPARE_CREATE |
+                                     RNA_OVERRIDE_COMPARE_TAG_FOR_RESTORE),
+      &report_flags);
   atomic_fetch_and_or_uint32(reinterpret_cast<uint32_t 
*>(&create_data->report_flags),
                              report_flags);
 }
@@ -3443,6 +3495,13 @@ void 
BKE_lib_override_library_main_operations_create(Main *bmain,
 
   BLI_task_pool_free(task_pool);
 
+  if (create_pool_data.report_flags & 
RNA_OVERRIDE_MATCH_RESULT_RESTORE_TAGGED) {
+    BKE_lib_override_library_main_operations_restore(
+        bmain, reinterpret_cast<int *>(&create_pool_data.report_flags));
+    create_pool_data.report_flags = static_cast<eRNAOverrideMatchResult>(
+        (create_pool_data.report_flags & 
~RNA_OVERRIDE_MATCH_RESULT_RESTORE_TAGGED));
+  }
+
   if (r_report_flags != nullptr) {
     *r_report_flags |= create_pool_data.report_flags;
   }
@@ -3456,6 +3515,28 @@ void 
BKE_lib_override_library_main_operations_create(Main *bmain,
 #endif
 }
 
+void BKE_lib_override_library_main_operations_restore(Main *bmain, int 
*r_report_flags)
+{
+  ID *id;
+
+  FOREACH_MAIN_ID_BEGIN (bmain, id) {
+    if (!(!ID_IS_LINKED(id) && ID_IS_OVERRIDE_LIBRARY_REAL(id) &&
+          (id->override_library->runtime->tag & 
IDOVERRIDE_LIBRARY_RUNTIME_TAG_NEEDS_RESTORE) !=
+              0)) {
+      continue;
+    }
+
+    /* Only restore overrides if we do have the real reference data available, 
and not some empty
+     * 'placeholder' for missing data (broken links). */
+    if (id->override_library->reference->tag & LIB_TAG_MISSING) {
+      continue;
+    }
+
+    BKE_lib_override_library_operations_restore(bmain, id, r_report_flags);
+  }
+  FOREACH_MAIN_ID_END;
+}
+
 static bool lib_override_library_id_reset_do(Main *bmain,
                                              ID *id_root,
                                              const bool 
do_reset_system_override)
diff --git a/source/blender/makesdna/DNA_ID.h b/source/blender/makesdna/DNA_ID.h
index 90d44d95139..16a8072a015 100644
--- a/source/blender/makesdna/DNA_ID.h
+++ b/source/blender/makesdna/DNA_ID.h
@@ -274,6 +274,9 @@ typedef struct IDOverrideLibraryProperty {
 enum {
   /** This override property (operation) is unused and should be removed by 
cleanup process. */
   IDOVERRIDE_LIBRARY_TAG_UNUSED = 1 << 0,
+
+  /** This override property is forbidden and should be restored to its linked 
reference value. */
+  IDOVERRIDE_LIBRARY_PROPERTY_TAG_NEEDS_RETORE = 1 << 1,
 };
 
 #
@@ -287,6 +290,12 @@ typedef struct IDOverrideLibraryRuntime {
 enum {
   /** This override needs to be reloaded. */
   IDOVERRIDE_LIBRARY_RUNTIME_TAG_NEEDS_RELOAD = 1 << 0,
+
+  /**
+   * This override contains properties with forbidden changes, which should be 
restored to their
+   * linked reference value.
+   */
+  IDOVERRIDE_LIBRARY_RUNTIME_TAG_NEEDS_RESTORE = 1 << 1,
 };
 
 /* Main container for all overriding data info of a data-block. */
diff --git a/source/blender/makesrna/RNA_access.h 
b/source/blender/makesrna/RNA_access.h
index 97bef9d4965..656b0bc93b2 100644
--- a/source/blender/makesrna/RNA_access.h
+++ b/source/blender/makesrna/RNA_ac

@@ Diff output truncated at 10240 characters. @@

_______________________________________________
Bf-blender-cvs mailing list
[email protected]
List details, subscription details or unsubscribe:
https://lists.blender.org/mailman/listinfo/bf-blender-cvs

Reply via email to