[Bf-blender-cvs] [59bcca3016a] T102766: Refactor liboverride diffing, by un-threading restoration step.
Bastien Montagne
noreply at git.blender.org
Mon Dec 26 07:27:42 CET 2022
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. @@
More information about the Bf-blender-cvs
mailing list