Cleanup: Improvements to Outliner tree writing code, add assert

Comments there weren't really helpful, took me a while to get what they
try to say. This attempts to add a better explanation. Also add an
assert for a previous, implicit (but commented) assumption, and some
minor cleanups.

1 files changed, 21 insertions, 10 deletions

diff --git a/source/blender/blenkernel/intern/screen.c b/source/blender/blenkernel/intern/screen.c
index 66ec887b3d4..f1eba64e401 100644
--- a/source/blender/blenkernel/intern/screen.c
+++ b/source/blender/blenkernel/intern/screen.c
@@ -1123,32 +1123,43 @@ static void write_uilist(BlendWriter *writer, uiList *ui_list)
   }
 }
 
-static void write_space_outliner(BlendWriter *writer, SpaceOutliner *space_outliner)
+static void write_space_outliner(BlendWriter *writer, const SpaceOutliner *space_outliner)
 {
   BLI_mempool *ts = space_outliner->treestore;
 
   if (ts) {
-    int elems = BLI_mempool_len(ts);
+    const int elems = BLI_mempool_len(ts);
     /* linearize mempool to array */
     TreeStoreElem *data = elems ? BLI_mempool_as_arrayN(ts, "TreeStoreElem") : NULL;
 
     if (data) {
-      /* In this block we use the memory location of the treestore
-       * but _not_ its data, the addresses in this case are UUID's,
-       * since we can't rely on malloc giving us different values each time.
+      BLO_write_struct(writer, SpaceOutliner, space_outliner);
+
+      /* To store #TreeStore (instead of the mempool), two unique memory addresses are needed,
+       * which can be used to identify the data on read:
+       * 1) One for the #TreeStore data itself.
+       * 2) One for the array of #TreeStoreElem's inside #TreeStore (#TreeStore.data).
+       *
+       * For 1) we just use the mempool's address (#SpaceOutliner::treestore).
+       * For 2) we don't have such a direct choice. We can't just use the array's address from
+       * above, since that may not be unique over all Outliners. So instead use an address relative
+       * to 1).
        */
-      TreeStore ts_flat = {0};
+      /* TODO the mempool could be moved to #SpaceOutliner_Runtime so that #SpaceOutliner could
+       * hold the #TreeStore directly. */
 
-      /* we know the treestore is at least as big as a pointer,
-       * so offsetting works to give us a UUID. */
+      /* Address relative to the tree-store, as noted above.  */
       void *data_addr = (void *)POINTER_OFFSET(ts, sizeof(void *));
+      /* There should be plenty of memory addresses within the mempool data that we can point into,
+       * just double-check we don't potentially end up with a memory address that another DNA
+       * struct might use. Assumes BLI_mempool uses the guarded allocator. */
+      BLI_assert(MEM_allocN_len(ts) >= sizeof(void *) * 2);
 
+      TreeStore ts_flat = {0};
       ts_flat.usedelem = elems;
       ts_flat.totelem = elems;
       ts_flat.data = data_addr;
 
-      BLO_write_struct(writer, SpaceOutliner, space_outliner);
-
       BLO_write_struct_at_address(writer, TreeStore, ts, &ts_flat);
       BLO_write_struct_array_at_address(writer, TreeStoreElem, elems, data_addr, data);