[Bf-blender-cvs] [18387ad496] id_override_static: Big refactor of differential override storage.

Bastien Montagne noreply at git.blender.org
Tue Feb 28 14:16:37 CET 2017


Commit: 18387ad49616f6fb3fde42c8115193d76d9a97ed
Author: Bastien Montagne
Date:   Tue Feb 28 10:44:07 2017 +0100
Branches: id_override_static
https://developer.blender.org/rB18387ad49616f6fb3fde42c8115193d76d9a97ed

Big refactor of differential override storage.

On second thought, modifying real data-block to store diff in it is...
just bad. Because it affects actual data, because it involves complicated
and heavy processes like remapping, and because it stores useless values
in .blend file (i.e. values that would give garbage results if e.g.
opening .blend file with older version of Blender, etc.).

So instead, decided to save an extra data-block in .blend file each time
we have some differential overrides.

Note that for now, we do full copy of datablock into its extra 'storage'
sibbling for .blend file. This is bad (think about double copy of heavy
mesh...) and will have to be fixed. Whole copy process will need serious
work anyway, for that kind of features we need to be able to copy
without increase usercount of IDs etc.

In fact, here, maybe we even need a smarter version of copy that would
only duplicate sub-data as needed by diff-overriden properties...

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

M	source/blender/blenkernel/BKE_library_override.h
M	source/blender/blenkernel/intern/library.c
M	source/blender/blenkernel/intern/library_override.c
M	source/blender/blenloader/intern/readfile.c
M	source/blender/blenloader/intern/writefile.c
M	source/blender/makesdna/DNA_ID.h
M	source/blender/makesrna/RNA_access.h
M	source/blender/makesrna/intern/rna_access.c

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

diff --git a/source/blender/blenkernel/BKE_library_override.h b/source/blender/blenkernel/BKE_library_override.h
index 777c245d97..07854ebef0 100644
--- a/source/blender/blenkernel/BKE_library_override.h
+++ b/source/blender/blenkernel/BKE_library_override.h
@@ -57,13 +57,22 @@ struct IDOverridePropertyOperation *BKE_override_property_operation_get(
 bool BKE_override_status_check_local(struct ID *local);
 bool BKE_override_status_check_reference(struct ID *local);
 
-bool BKE_override_operations_store_start(struct ID *local);
-void BKE_override_operations_store_end(struct ID *local);
-
 bool BKE_override_operations_create(struct ID *local, const bool no_skip);
 
-void BKE_override_update(struct Main *bmain, struct ID *local, const bool do_init);
-void BKE_main_override_update(struct Main *bmain, const bool do_init);
+void BKE_override_update(struct Main *bmain, struct ID *local);
+void BKE_main_override_update(struct Main *bmain);
+
+
+/* Storage (.blend file writing) part. */
+
+/* For now, we just use a temp main list. */
+typedef struct Main OverrideStorage;
+
+OverrideStorage *BKE_override_operations_store_initialize(void);
+struct ID *BKE_override_operations_store_start(OverrideStorage *override_storage, struct ID *local);
+void BKE_override_operations_store_end(OverrideStorage *override_storage, struct ID *local);
+void BKE_override_operations_store_finalize(OverrideStorage *override_storage);
+
 
 
 #endif  /* __BKE_LIBRARY_OVERRIDE_H__ */
diff --git a/source/blender/blenkernel/intern/library.c b/source/blender/blenkernel/intern/library.c
index c599814bc5..4326d8415c 100644
--- a/source/blender/blenkernel/intern/library.c
+++ b/source/blender/blenkernel/intern/library.c
@@ -1190,6 +1190,8 @@ void BKE_libblock_copy_data(ID *id, const ID *id_from, const bool do_action)
 	if (id_from->properties)
 		id->properties = IDP_CopyProperty(id_from->properties);
 
+	/* For now, just never copy override stuff... */
+
 	/* the duplicate should get a copy of the animdata */
 	id_copy_animdata(id, do_action);
 }
diff --git a/source/blender/blenkernel/intern/library_override.c b/source/blender/blenkernel/intern/library_override.c
index b674a26f1f..2bf87af369 100644
--- a/source/blender/blenkernel/intern/library_override.c
+++ b/source/blender/blenkernel/intern/library_override.c
@@ -35,7 +35,6 @@
 #include "DNA_object_types.h"
 
 #include "BKE_depsgraph.h"
-#include "BKE_global.h"  /* XXX Yuck! temp hack! */
 #include "BKE_library.h"
 #include "BKE_library_override.h"
 #include "BKE_library_remap.h"
@@ -276,76 +275,6 @@ bool BKE_override_status_check_reference(ID *local)
 }
 
 /**
- * Generate suitable 'write' data (this only affects differential override operations).
- *
- * \note ID is in 'invalid' state for all usages but being written to file, after this function has been called and
- * until \a BKE_override_operations_store_end is called to restore it. */
-bool BKE_override_operations_store_start(ID *local)
-{
-	BLI_assert(local->override != NULL);
-	bool ret = false;
-
-	/* Forcefully ensure we now about all needed poverride operations. */
-	BKE_override_operations_create(local, true);
-
-	TIMEIT_START_AVERAGED(BKE_override_operations_store_start);
-
-	/* Here we work on original local data-block, after having made a temp copy of it.
-	 * Once we are done, _store_end() will swap temp and local contents.
-	 * This allows us to keep most of original data to write (which is needed to (hopefully) avoid memory/pointers
-	 * collisions in .blend file), and also neats things like original ID name. ;) */
-	/* Note: ideally I'd rather work on copy here as well, and not touch to original at all, but then we'd have
-	 * issues with ID data itself (which is currently not swapped by BKE_id_swap()) AND pointers overlapping. */
-
-	ID *tmp_id;
-	/* XXX TODO We *need* an id_copy_nolib(), that stays out of Main and does not inc/dec ID pointers... */
-	id_copy(G.main, local, &tmp_id, false);  /* XXX ...and worse of all, this won't work with scene! */
-
-	if (tmp_id == NULL) {
-		return ret;
-	}
-
-	PointerRNA rnaptr_reference, rnaptr_final;
-	RNA_id_pointer_create(local->override->reference, &rnaptr_reference);
-	RNA_id_pointer_create(local, &rnaptr_final);
-
-	if (!RNA_struct_override_store(&rnaptr_final, &rnaptr_reference, local->override)) {
-		BKE_libblock_free_ex(G.main, tmp_id, true, false);
-	}
-	else {
-		local->tag &= ~LIB_TAG_OVERRIDE_OK;
-		local->newid = tmp_id;
-		ret = true;
-	}
-
-	TIMEIT_END_AVERAGED(BKE_override_operations_store_start);
-	return ret;
-}
-
-/** Restore given ID modified by \a BKE_override_operations_store_start, to its valid original state. */
-void BKE_override_operations_store_end(ID *local)
-{
-	BLI_assert(local->override != NULL);
-
-	ID *tmp_id = local->newid;
-	BLI_assert(tmp_id != NULL);
-
-	/* Swapping here allows us to get back original data. */
-	BKE_id_swap(local, tmp_id);
-	/* Swap above may have broken internal references to itself. */
-	BKE_libblock_relink_ex(G.main, local, tmp_id, local, false);
-	BKE_libblock_relink_ex(G.main, tmp_id, local, tmp_id, false);  /* Grrrr... */
-
-	local->tag |= LIB_TAG_OVERRIDE_OK;
-	local->newid = NULL;
-
-	BKE_libblock_free_ex(G.main, tmp_id, true, false);
-
-	/* Full rebuild of DAG! */
-	DAG_relations_tag_update(G.main);
-}
-
-/**
  * Compares local and reference data-blocks and create new override operations as needed,
  * or reset to reference values if overriding is not allowed.
  *
@@ -386,7 +315,7 @@ bool BKE_override_operations_create(ID *local, const bool no_skip)
 }
 
 /** Update given override from its reference (re-applying overriden properties). */
-void BKE_override_update(Main *bmain, ID *local, const bool do_init)
+void BKE_override_update(Main *bmain, ID *local)
 {
 	if (local->override == NULL) {
 		return;
@@ -394,7 +323,7 @@ void BKE_override_update(Main *bmain, ID *local, const bool do_init)
 
 	/* Recursively do 'ancestors' overrides first, if any. */
 	if (local->override->reference->override && (local->override->reference->tag & LIB_TAG_OVERRIDE_OK) == 0) {
-		BKE_override_update(bmain, local->override->reference, do_init);
+		BKE_override_update(bmain, local->override->reference);
 	}
 
 	/* We want to avoid having to remap here, however creating up-to-date override is much simpler if based
@@ -415,11 +344,15 @@ void BKE_override_update(Main *bmain, ID *local, const bool do_init)
 		return;
 	}
 
-	PointerRNA rnaptr_local, rnaptr_final;
+	PointerRNA rnaptr_local, rnaptr_final, rnaptr_storage_stack, *rnaptr_storage = NULL;
 	RNA_id_pointer_create(local, &rnaptr_local);
 	RNA_id_pointer_create(tmp_id, &rnaptr_final);
+	if (local->override->storage) {
+		rnaptr_storage = &rnaptr_storage_stack;
+		RNA_id_pointer_create(local->override->storage, rnaptr_storage);
+	}
 
-	RNA_struct_override_apply(&rnaptr_final, &rnaptr_local, local->override, do_init);
+	RNA_struct_override_apply(&rnaptr_final, &rnaptr_local, rnaptr_storage, local->override);
 
 	/* This also transfers all pointers (memory) owned by local to tmp_id, and vice-versa. So when we'll free tmp_id,
 	 * we'll actually free old, outdated data from local. */
@@ -432,6 +365,14 @@ void BKE_override_update(Main *bmain, ID *local, const bool do_init)
 	/* XXX And crashing in complex cases (e.g. because depsgraph uses same data...). */
 	BKE_libblock_free_ex(bmain, tmp_id, true, false);
 
+	if (local->override->storage) {
+		/* We know this datablock is not used anywhere besides local->override->storage. */
+		/* XXX For until we get fully shadow copies, we still need to ensure storage releases
+		 *     its usage of any ID pointers it may have. */
+		BKE_libblock_free_ex(bmain, local->override->storage, true, false);
+		local->override->storage = NULL;
+	}
+
 	local->tag |= LIB_TAG_OVERRIDE_OK;
 
 	/* Full rebuild of DAG! */
@@ -439,7 +380,7 @@ void BKE_override_update(Main *bmain, ID *local, const bool do_init)
 }
 
 /** Update all overrides from given \a bmain. */
-void BKE_main_override_update(Main *bmain, const bool do_init)
+void BKE_main_override_update(Main *bmain)
 {
 	ListBase *lbarray[MAX_LIBARRAY];
 	int base_count, i;
@@ -452,8 +393,103 @@ void BKE_main_override_update(Main *bmain, const bool do_init)
 
 		for (id = lb->first; id; id = id->next) {
 			if (id->override != NULL && id->lib == NULL) {
-				BKE_override_update(bmain, id, do_init);
+				BKE_override_update(bmain, id);
 			}
 		}
 	}
 }
+
+/***********************************************************************************************************************
+ * Storage (how to wtore overriding data into .blend files).
+ *
+ * Basically:
+ * I) Only 'differential' storage needs special handling here. All others (replacing values or
+ *    inserting/removing items from a collection) can be handled with simply storing current content of local data-block.
+ * II) We store the differential value into a second 'ghost' data-block, which is an empty ID of same type as local one,
+ *     where we only define values that need differential data.
+ *
+ * This avoids us having to modify 'real' data-block at write time (and retoring it afterwards), which is inneficient,
+ * and potentially dangerous (in case of concurrent access...), while not using much extra memory in typical cases.
+ * It also ensures stored data-block always contains exact same data as "desired" ones (kind of "baked" data-blocks).
+ */
+
+/** Initialize an override storage. */
+OverrideStorage *BKE_override_operations_store_initialize(void)
+{
+	return BKE_main_new();
+}
+
+/**
+ * Generate suitable 'write' data (this only affects differential override operations).
+ *
+ * \note ID is in 'invalid' state for all usages but being written to file, after this function has been called and
+ * until \a BKE_override_operations_store_end is called to restore it. */
+ID *BKE_override_operations_store_start(OverrideStorage *override_storage, ID *local)
+{
+	BLI_assert(local->override != NULL);
+	BLI_assert(override_storage != NULL);
+
+	/* Forcefully ensure we now about all needed override operations. */
+	BKE_override_operations_create(local, true);
+
+	ID *storage_id;
+	TIMEIT_START_AVERAGED(BKE_override_operations_store_start);
+
+	/* Here we work on original local data-block, after having 

@@ Diff output truncated at 10240 characters. @@




More information about the Bf-blender-cvs mailing list