Fix #148220: Scene 'Linked Copy' does not remap to newly copied data-block.

While more of a theoritical issue currently, this change is a
pre-requisite to support properly compo nodes duplication in 'linked
copy' case (see !148210).

Also add a basic unittest for this case.

Pull Request: https://projects.blender.org/blender/blender/pulls/148222

source/blender/blenkernel/intern/scene.cc

@@ -1840,31 +1840,38 @@ Scene *BKE_scene_duplicate(Main *bmain, Scene *sce, eSceneCopyMethod type)
   id_us_min(&sce_copy->id);
   id_us_ensure_real(&sce_copy->id);
 
-  BKE_animdata_duplicate_id_action(bmain, &sce_copy->id, duplicate_flags);
+  /* Scene duplication is always root of duplication currently, and never a subprocess.
+   *
+   * Keep these around though, as this allow the rest of the duplication code to stay in sync with
+   * the layout and behavior as the other duplicate functions (see e.g. #BKE_collection_duplicate
+   * or #BKE_object_duplicate).
+   *
+   * TOOD: At some point it would be nice to deduplicate this logic and move common behavior into
+   * generic ID management code, with IDType callbacks for specific duplication behavior only. */
+  const bool is_subprocess = false;
+  const bool is_root_id = true;
+  const int copy_flags = LIB_ID_COPY_DEFAULT;
+
+  if (!is_subprocess) {
+    BKE_main_id_newptr_and_tag_clear(bmain);
+  }
+
+  if (is_root_id) {
+    /* In case root duplicated ID is linked, assume we want to get a local copy of it and
+     * duplicate all expected linked data. */
+    if (ID_IS_LINKED(sce)) {
+      duplicate_flags = (duplicate_flags | USER_DUP_LINKED_ID);
+    }
+  }
+
+  /* Usages of the duplicated scene also need to be remapped in new duplicated IDs. */
+  ID_NEW_SET(sce, sce_copy);
 
   /* Extra actions, most notably SCE_FULL_COPY also duplicates several 'children' datablocks. */
 
+  BKE_animdata_duplicate_id_action(bmain, &sce_copy->id, duplicate_flags);
+
   if (type == SCE_COPY_FULL) {
-    /* Scene duplication is always root of duplication currently. */
-    const bool is_subprocess = false;
-    const bool is_root_id = true;
-    const int copy_flags = LIB_ID_COPY_DEFAULT;
-
-    if (!is_subprocess) {
-      BKE_main_id_newptr_and_tag_clear(bmain);
-    }
-
-    /* Usages of the duplicated scene also need to be remapped in new duplicated IDs. */
-    ID_NEW_SET(sce, sce_copy);
-
-    if (is_root_id) {
-      /* In case root duplicated ID is linked, assume we want to get a local copy of it and
-       * duplicate all expected linked data. */
-      if (ID_IS_LINKED(sce)) {
-        duplicate_flags = (duplicate_flags | USER_DUP_LINKED_ID);
-      }
-    }
-
     /* Copy Freestyle LineStyle datablocks. */
     LISTBASE_FOREACH (ViewLayer *, view_layer_dst, &sce_copy->view_layers) {
       LISTBASE_FOREACH (FreestyleLineSet *, lineset, &view_layer_dst->freestyle_config.linesets) {
@@ -1911,33 +1918,6 @@ Scene *BKE_scene_duplicate(Main *bmain, Scene *sce, eSceneCopyMethod type)
                                  LIB_ID_DUPLICATE_IS_SUBPROCESS);
       }
     }
-
-    if (!is_subprocess) {
-      /* This code will follow into all ID links using an ID tagged with ID_TAG_NEW. */
-      /* Unfortunate, but with some types (e.g. meshes), an object is considered in Edit mode if
-       * its obdata contains edit mode runtime data. This can be the case of all newly duplicated
-       * objects, as even though duplicate code move the object back in Object mode, they are still
-       * using the original obdata ID, leading to them being falsely detected as being in Edit
-       * mode, and therefore not remapping their obdata to the newly duplicated one. See #139715.
-       */
-      BKE_libblock_relink_to_newid(
-          bmain, &sce_copy->id, ID_REMAP_FORCE_OBDATA_IN_EDITMODE | ID_REMAP_SKIP_USER_CLEAR);
-
-#ifndef NDEBUG
-      /* Call to `BKE_libblock_relink_to_newid` above is supposed to have cleared all those
-       * flags. */
-      ID *id_iter;
-      FOREACH_MAIN_ID_BEGIN (bmain, id_iter) {
-        BLI_assert((id_iter->tag & ID_TAG_NEW) == 0);
-      }
-      FOREACH_MAIN_ID_END;
-#endif
-
-      /* Cleanup. */
-      BKE_main_id_newptr_and_tag_clear(bmain);
-
-      BKE_main_collection_sync(bmain);
-    }
   }
   else {
     /* Remove sequencer if not full copy */
@@ -1946,6 +1926,35 @@ Scene *BKE_scene_duplicate(Main *bmain, Scene *sce, eSceneCopyMethod type)
     blender::seq::editing_free(sce_copy, true);
   }
 
+  /* The final step is to ensure that all of the newly duplicated IDs are used by other newly
+   * duplicated IDs, and some standard cleanup & updates. */
+  if (!is_subprocess) {
+    /* This code will follow into all ID links using an ID tagged with ID_TAG_NEW. */
+    /* Unfortunate, but with some types (e.g. meshes), an object is considered in Edit mode if
+     * its obdata contains edit mode runtime data. This can be the case of all newly duplicated
+     * objects, as even though duplicate code move the object back in Object mode, they are still
+     * using the original obdata ID, leading to them being falsely detected as being in Edit
+     * mode, and therefore not remapping their obdata to the newly duplicated one. See #139715.
+     */
+    BKE_libblock_relink_to_newid(
+        bmain, &sce_copy->id, ID_REMAP_FORCE_OBDATA_IN_EDITMODE | ID_REMAP_SKIP_USER_CLEAR);
+
+#ifndef NDEBUG
+    /* Call to `BKE_libblock_relink_to_newid` above is supposed to have cleared all those
+     * flags. */
+    ID *id_iter;
+    FOREACH_MAIN_ID_BEGIN (bmain, id_iter) {
+      BLI_assert((id_iter->tag & ID_TAG_NEW) == 0);
+    }
+    FOREACH_MAIN_ID_END;
+#endif
+
+    /* Cleanup. */
+    BKE_main_id_newptr_and_tag_clear(bmain);
+
+    BKE_main_collection_sync(bmain);
+  }
+
   return sce_copy;
 }
 

source/blender/blenkernel/intern/scene_test.cc

@@ -2,9 +2,24 @@
  *
  * SPDX-License-Identifier: GPL-2.0-or-later */
 
+#include "BKE_anim_data.hh"
+#include "BKE_appdir.hh"
+#include "BKE_idprop.hh"
+#include "BKE_idtype.hh"
+#include "BKE_lib_id.hh"
+#include "BKE_main.hh"
 #include "BKE_scene.hh"
 
+#include "BLO_userdef_default.h"
+
+#include "IMB_imbuf.hh"
+
+#include "DNA_action_types.h"
+#include "DNA_anim_types.h"
 #include "DNA_scene_types.h"
+#include "DNA_userdef_types.h"
+
+#include "CLG_log.h"
 
 #include "testing/testing.h"
 
@@ -41,4 +56,101 @@ TEST(scene, frame_snap_by_seconds)
   EXPECT_FLOAT_EQ(10000.0, BKE_scene_frame_snap_by_seconds(&fake_scene, 2.0, 10000.0));
 }
 
+class SceneTest : public ::testing::Test {
+ public:
+  Main *bmain;
+
+  static void SetUpTestSuite()
+  {
+    CLG_init();
+    BKE_appdir_init();
+    IMB_init();
+    BKE_idtype_init();
+    /* #BKE_scene_duplicate() uses #U::dupflag. */
+    U = blender::dna::shallow_copy(U_default);
+  }
+
+  static void TearDownTestSuite()
+  {
+    IMB_exit();
+    BKE_appdir_exit();
+    CLG_exit();
+  }
+
+  void SetUp() override
+  {
+    bmain = BKE_main_new();
+  }
+
+  void TearDown() override
+  {
+    BKE_main_free(bmain);
+  }
+};
+
+TEST_F(SceneTest, linked_copy_id_remapping)
+{
+  Scene *scene_src = BKE_id_new<Scene>(bmain, "Scene_source");
+  bAction *action_src = BKE_id_new<bAction>(bmain, "Scene_source_action");
+  BKE_animdata_set_action(nullptr, &scene_src->id, action_src);
+  AnimData *animdata_src = BKE_animdata_from_id(&scene_src->id);
+  ASSERT_NE(animdata_src, nullptr);
+  EXPECT_EQ(animdata_src->action, action_src);
+
+  constexpr blender::StringRef idp_scene2scene_name = "scene2scene";
+  constexpr blender::StringRef idp_scene2action_name = "scene2action";
+  constexpr blender::StringRef idp_action2scene_name = "action2scene";
+  constexpr blender::StringRef idp_action2action_name = "action2action";
+
+  IDProperty *scene_idgroup_src = IDP_EnsureProperties(&scene_src->id);
+  IDP_AddToGroup(scene_idgroup_src,
+                 bke::idprop::create(idp_scene2scene_name, &scene_src->id).release());
+  IDP_AddToGroup(scene_idgroup_src,
+                 bke::idprop::create(idp_scene2action_name, &action_src->id).release());
+
+  IDProperty *action_idgroup_src = IDP_EnsureProperties(&action_src->id);
+  IDP_AddToGroup(action_idgroup_src,
+                 bke::idprop::create(idp_action2scene_name, &scene_src->id).release());
+  IDP_AddToGroup(action_idgroup_src,
+                 bke::idprop::create(idp_action2action_name, &action_src->id).release());
+
+  Scene *scene_copy = BKE_scene_duplicate(bmain, scene_src, SCE_COPY_LINK_COLLECTION);
+
+  /* Source data should remain unchanged. */
+
+  EXPECT_EQ(IDP_ID_get(IDP_GetPropertyFromGroup(scene_idgroup_src, idp_scene2scene_name)),
+            &scene_src->id);
+  EXPECT_EQ(IDP_ID_get(IDP_GetPropertyFromGroup(scene_idgroup_src, idp_scene2action_name)),
+            &action_src->id);
+
+  EXPECT_EQ(IDP_ID_get(IDP_GetPropertyFromGroup(action_idgroup_src, idp_action2scene_name)),
+            &scene_src->id);
+  EXPECT_EQ(IDP_ID_get(IDP_GetPropertyFromGroup(action_idgroup_src, idp_action2action_name)),
+            &action_src->id);
+
+  /* Copied data should have its ID usages remapped to new copies if possible. */
+
+  EXPECT_NE(scene_copy, scene_src);
+  AnimData *animdata_copy = BKE_animdata_from_id(&scene_copy->id);
+  ASSERT_NE(animdata_copy, nullptr);
+  EXPECT_NE(animdata_copy, animdata_src);
+  bAction *action_copy = animdata_copy->action;
+  ASSERT_NE(action_copy, nullptr);
+  EXPECT_NE(action_copy, action_src);
+
+  IDProperty *scene_idgroup_copy = IDP_GetProperties(&scene_copy->id);
+  ASSERT_NE(scene_idgroup_copy, nullptr);
+  EXPECT_EQ(IDP_ID_get(IDP_GetPropertyFromGroup(scene_idgroup_copy, idp_scene2scene_name)),
+            &scene_copy->id);
+  EXPECT_EQ(IDP_ID_get(IDP_GetPropertyFromGroup(scene_idgroup_copy, idp_scene2action_name)),
+            &action_copy->id);
+
+  IDProperty *action_idgroup_copy = IDP_GetProperties(&action_copy->id);
+  ASSERT_NE(action_idgroup_copy, nullptr);
+  EXPECT_EQ(IDP_ID_get(IDP_GetPropertyFromGroup(action_idgroup_copy, idp_action2scene_name)),
+            &scene_copy->id);
+  EXPECT_EQ(IDP_ID_get(IDP_GetPropertyFromGroup(action_idgroup_copy, idp_action2action_name)),
+            &action_copy->id);
+}
+
 }  // namespace blender::bke::tests