[Bf-blender-cvs] [460e0a1347e] master: Revert "Performance: Remap multiple items in UI"

Jeroen Bakker noreply at git.blender.org
Tue Jan 25 15:33:58 CET 2022


Commit: 460e0a1347e50d33f5d42235ee2d9cb7208cdc4f
Author: Jeroen Bakker
Date:   Tue Jan 25 15:31:46 2022 +0100
Branches: master
https://developer.blender.org/rB460e0a1347e50d33f5d42235ee2d9cb7208cdc4f

Revert "Performance: Remap multiple items in UI"

This reverts commit 948211679f2a0681421160be0d3b90f507bc0be7.
This commit introduced some regressions in the test suite.
As this change is a core part of blender Bastien and I decided to revert
it as the solution isn't clear and needs more investigation.

The following tests FAILED:
	 62 - blendfile_liblink (SEGFAULT)
	 63 - blendfile_library_overrides (SEGFAULT)

It fails in (id_us_ensure_real)

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

M	source/blender/blenkernel/BKE_lib_remap.h
M	source/blender/blenkernel/BKE_screen.h
M	source/blender/blenkernel/CMakeLists.txt
M	source/blender/blenkernel/intern/lib_id_delete.c
D	source/blender/blenkernel/intern/lib_id_remapper.cc
D	source/blender/blenkernel/intern/lib_id_remapper_test.cc
M	source/blender/blenkernel/intern/lib_override.c
M	source/blender/blenkernel/intern/lib_remap.c
M	source/blender/editors/include/ED_util.h
M	source/blender/editors/space_action/space_action.c
M	source/blender/editors/space_buttons/space_buttons.c
M	source/blender/editors/space_clip/space_clip.c
M	source/blender/editors/space_file/space_file.c
M	source/blender/editors/space_graph/space_graph.c
M	source/blender/editors/space_image/space_image.c
M	source/blender/editors/space_nla/space_nla.c
M	source/blender/editors/space_node/space_node.cc
M	source/blender/editors/space_outliner/space_outliner.cc
M	source/blender/editors/space_sequencer/space_sequencer.c
M	source/blender/editors/space_spreadsheet/space_spreadsheet.cc
M	source/blender/editors/space_text/space_text.c
M	source/blender/editors/space_view3d/space_view3d.c
M	source/blender/editors/util/ed_util.c
M	source/blender/windowmanager/WM_api.h
M	source/blender/windowmanager/intern/wm_event_system.c
M	source/blender/windowmanager/intern/wm_init_exit.c

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

diff --git a/source/blender/blenkernel/BKE_lib_remap.h b/source/blender/blenkernel/BKE_lib_remap.h
index cc970342fbb..d8842dbce7f 100644
--- a/source/blender/blenkernel/BKE_lib_remap.h
+++ b/source/blender/blenkernel/BKE_lib_remap.h
@@ -38,9 +38,6 @@
 extern "C" {
 #endif
 
-struct ID;
-struct IDRemapper;
-
 /* BKE_libblock_free, delete are declared in BKE_lib_id.h for convenience. */
 
 /* Also IDRemap->flag. */
@@ -100,19 +97,6 @@ enum {
   ID_REMAP_FORCE_OBDATA_IN_EDITMODE = 1 << 9,
 };
 
-/**
- * Replace all references in given Main using the given \a mappings
- *
- * \note Is preferred over BKE_libblock_remap_locked due to performance.
- */
-void BKE_libblock_remap_multiple_locked(struct Main *bmain,
-                                        const struct IDRemapper *mappings,
-                                        const short remap_flags);
-
-void BKE_libblock_remap_multiple(struct Main *bmain,
-                                 const struct IDRemapper *mappings,
-                                 const short remap_flags);
-
 /**
  * Replace all references in given Main to \a old_id by \a new_id
  * (if \a new_id is NULL, it unlinks \a old_id).
@@ -162,61 +146,12 @@ void BKE_libblock_relink_to_newid(struct Main *bmain, struct ID *id, int remap_f
     ATTR_NONNULL();
 
 typedef void (*BKE_library_free_notifier_reference_cb)(const void *);
-typedef void (*BKE_library_remap_editor_id_reference_cb)(const struct IDRemapper *mappings);
+typedef void (*BKE_library_remap_editor_id_reference_cb)(struct ID *, struct ID *);
 
 void BKE_library_callback_free_notifier_reference_set(BKE_library_free_notifier_reference_cb func);
 void BKE_library_callback_remap_editor_id_reference_set(
     BKE_library_remap_editor_id_reference_cb func);
 
-/* IDRemapper */
-struct IDRemapper;
-typedef enum IDRemapperApplyResult {
-  /** No remapping rules available for the source. */
-  ID_REMAP_RESULT_SOURCE_UNAVAILABLE,
-  /** Source isn't mappable (e.g. NULL). */
-  ID_REMAP_RESULT_SOURCE_NOT_MAPPABLE,
-  /** Source has been remapped to a new pointer. */
-  ID_REMAP_RESULT_SOURCE_REMAPPED,
-  /** Source has been set to NULL. */
-  ID_REMAP_RESULT_SOURCE_UNASSIGNED,
-} IDRemapperApplyResult;
-
-typedef enum IDRemapperApplyOptions {
-  ID_REMAP_APPLY_UPDATE_REFCOUNT = (1 << 0),
-  ID_REMAP_APPLY_ENSURE_REAL = (1 << 1),
-
-  ID_REMAP_APPLY_DEFAULT = 0,
-} IDRemapperApplyOptions;
-
-typedef void (*IDRemapperIterFunction)(struct ID *old_id, struct ID *new_id, void *user_data);
-
-/**
- * Create a new ID Remapper.
- *
- * An ID remapper stores multiple remapping rules.
- */
-struct IDRemapper *BKE_id_remapper_create(void);
-
-void BKE_id_remapper_clear(struct IDRemapper *id_remapper);
-bool BKE_id_remapper_is_empty(const struct IDRemapper *id_remapper);
-/** Free the given ID Remapper. */
-void BKE_id_remapper_free(struct IDRemapper *id_remapper);
-/** Add a new remapping. */
-void BKE_id_remapper_add(struct IDRemapper *id_remapper, struct ID *old_id, struct ID *new_id);
-
-/**
- * Apply a remapping.
- *
- * Update the id pointer stored in the given r_id_ptr if a remapping rule exists.
- */
-IDRemapperApplyResult BKE_id_remapper_apply(const struct IDRemapper *id_remapper,
-                                            struct ID **r_id_ptr,
-                                            IDRemapperApplyOptions options);
-bool BKE_id_remapper_has_mapping_for(const struct IDRemapper *id_remapper, uint64_t type_filter);
-void BKE_id_remapper_iter(const struct IDRemapper *id_remapper,
-                          IDRemapperIterFunction func,
-                          void *user_data);
-
 #ifdef __cplusplus
 }
 #endif
diff --git a/source/blender/blenkernel/BKE_screen.h b/source/blender/blenkernel/BKE_screen.h
index c85ae04a492..63f6fca2a9d 100644
--- a/source/blender/blenkernel/BKE_screen.h
+++ b/source/blender/blenkernel/BKE_screen.h
@@ -38,7 +38,6 @@ struct BlendLibReader;
 struct BlendWriter;
 struct Header;
 struct ID;
-struct IDRemapper;
 struct LibraryForeachIDData;
 struct ListBase;
 struct Menu;
@@ -118,7 +117,10 @@ typedef struct SpaceType {
   bContextDataCallback context;
 
   /* Used when we want to replace an ID by another (or NULL). */
-  void (*id_remap)(struct ScrArea *area, struct SpaceLink *sl, const struct IDRemapper *mappings);
+  void (*id_remap)(struct ScrArea *area,
+                   struct SpaceLink *sl,
+                   struct ID *old_id,
+                   struct ID *new_id);
 
   int (*space_subtype_get)(struct ScrArea *area);
   void (*space_subtype_set)(struct ScrArea *area, int value);
diff --git a/source/blender/blenkernel/CMakeLists.txt b/source/blender/blenkernel/CMakeLists.txt
index 6d6579f49f6..41ca8084849 100644
--- a/source/blender/blenkernel/CMakeLists.txt
+++ b/source/blender/blenkernel/CMakeLists.txt
@@ -179,7 +179,6 @@ set(SRC
   intern/lib_id.c
   intern/lib_id_delete.c
   intern/lib_id_eval.c
-  intern/lib_id_remapper.cc
   intern/lib_override.c
   intern/lib_query.c
   intern/lib_remap.c
@@ -824,7 +823,6 @@ if(WITH_GTESTS)
     intern/idprop_serialize_test.cc
     intern/lattice_deform_test.cc
     intern/layer_test.cc
-    intern/lib_id_remapper_test.cc
     intern/lib_id_test.cc
     intern/lib_remap_test.cc
     intern/tracking_test.cc
diff --git a/source/blender/blenkernel/intern/lib_id_delete.c b/source/blender/blenkernel/intern/lib_id_delete.c
index f4dd67cac28..6d2e89187f7 100644
--- a/source/blender/blenkernel/intern/lib_id_delete.c
+++ b/source/blender/blenkernel/intern/lib_id_delete.c
@@ -154,10 +154,7 @@ void BKE_id_free_ex(Main *bmain, void *idv, int flag, const bool use_flag_from_i
     }
 
     if (remap_editor_id_reference_cb) {
-      struct IDRemapper *remapper = BKE_id_remapper_create();
-      BKE_id_remapper_add(remapper, id, NULL);
-      remap_editor_id_reference_cb(remapper);
-      BKE_id_remapper_free(remapper);
+      remap_editor_id_reference_cb(id, NULL);
     }
   }
 
@@ -295,40 +292,32 @@ static size_t id_delete(Main *bmain, const bool do_tagged_deletion)
      * Note that we go forward here, since we want to check dependencies before users
      * (e.g. meshes before objects).
      * Avoids to have to loop twice. */
-    struct IDRemapper *remapper = BKE_id_remapper_create();
     for (i = 0; i < base_count; i++) {
       ListBase *lb = lbarray[i];
       ID *id, *id_next;
-      BKE_id_remapper_clear(remapper);
 
       for (id = lb->first; id; id = id_next) {
         id_next = id->next;
         /* NOTE: in case we delete a library, we also delete all its datablocks! */
         if ((id->tag & tag) || (id->lib != NULL && (id->lib->id.tag & tag))) {
           id->tag |= tag;
-          BKE_id_remapper_add(remapper, id, NULL);
-        }
-      }
 
-      if (BKE_id_remapper_is_empty(remapper)) {
-        continue;
+          /* Will tag 'never NULL' users of this ID too.
+           * Note that we cannot use BKE_libblock_unlink() here, since it would ignore indirect
+           * (and proxy!) links, this can lead to nasty crashing here in second,
+           * actual deleting loop.
+           * Also, this will also flag users of deleted data that cannot be unlinked
+           * (object using deleted obdata, etc.), so that they also get deleted. */
+          BKE_libblock_remap_locked(bmain,
+                                    id,
+                                    NULL,
+                                    (ID_REMAP_FLAG_NEVER_NULL_USAGE |
+                                     ID_REMAP_FORCE_NEVER_NULL_USAGE |
+                                     ID_REMAP_FORCE_INTERNAL_RUNTIME_POINTERS));
+        }
       }
-
-      /* Will tag 'never NULL' users of this ID too.
-       * Note that we cannot use BKE_libblock_unlink() here, since it would ignore indirect
-       * (and proxy!) links, this can lead to nasty crashing here in second,
-       * actual deleting loop.
-       * Also, this will also flag users of deleted data that cannot be unlinked
-       * (object using deleted obdata, etc.), so that they also get deleted. */
-      BKE_libblock_remap_multiple_locked(bmain,
-                                         remapper,
-                                         (ID_REMAP_FLAG_NEVER_NULL_USAGE |
-                                          ID_REMAP_FORCE_NEVER_NULL_USAGE |
-                                          ID_REMAP_FORCE_INTERNAL_RUNTIME_POINTERS));
     }
-    BKE_id_remapper_free(remapper);
   }
-
   BKE_main_unlock(bmain);
 
   /* In usual reversed order, such that all usage of a given ID, even 'never NULL' ones,
diff --git a/source/blender/blenkernel/intern/lib_id_remapper.cc b/source/blender/blenkernel/intern/lib_id_remapper.cc
deleted file mode 100644
index c1734c9826a..00000000000
--- a/source/blender/blenkernel/intern/lib_id_remapper.cc
+++ /dev/null
@@ -1,175 +0,0 @@
-/*
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
- *
- * The Original Code is Copyright (C) 2022 by Blender Foundation.
- */
-
-#include "DNA_ID.h"
-
-#include "BKE_idtype.h"
-#include "BKE_lib_id.h"
-#include "BKE_lib_remap.h"
-
-#include "MEM_guardedalloc.h"
-
-#include "BLI_map.hh"
-
-using IDTypeFilter = uint64_t;
-
-namespace blender::bke::id::remapper {
-struct IDRemapper {
- private:
-  Map<ID *, ID *> mappings;
-  IDTypeFilter source_types = 0;
-
- public:
-  void clear()
-  {
-    mappings.clear();
-    source_types = 0;
-  }
-
-  bool is_empty() const
-  {
-    return mappings.is_empty();
-  }
-
-  void add(ID *old_id, ID *new_id)
-  {
-    BLI_assert(old_id != nullptr);
-    BLI_assert(new_id == nullptr || (GS(old_id->name) == GS(new

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list