[Bf-blender-cvs] [70b1c09d7a4] master: IO: Fix bug exporting dupli parent/child relations

Sybren A. Stüvel noreply at git.blender.org
Tue Jul 7 13:01:44 CEST 2020


Commit: 70b1c09d7a40912b5dccc69fc95cbee2e2388eab
Author: Sybren A. Stüvel
Date:   Tue Jul 7 12:45:30 2020 +0200
Branches: master
https://developer.blender.org/rB70b1c09d7a40912b5dccc69fc95cbee2e2388eab

IO: Fix bug exporting dupli parent/child relations

Exporting a scene to USD or Alembic would fail when there are multiple
duplicates of parent & child objects, duplicated by the same object. For
example, this happens when such a hierarchy of objects is contained in a
collection, and that collection is instanced multiple times by mesh
vertices. The problem here is that the 'parent' pointer of each
duplicated object points to the real parent; Blender would not figure
out properly which duplicated parent should be used.

This is now resolved by keeping track of the persistent ID of each
duplicated instance, which makes it possible to reconstruct the
parent-child relations of duplicated objects. This does use up some
memory for each dupli, so it could be heavy to export a Spring scene
(with all the pebbles and leaves), but it's only a small addition on top
of the USD/Alembic writer objects that have to be created anyway. At
least with this patch, they're created correctly.

Code-wise, the following changes are made:

- The export graph (that maps export parent to its export children) used
  to have as its key (Object, Duplicator). This is insufficient to
  correctly distinguish between multiple duplis of the same object by
  the same duplicator, so this is now extended to (Object, Duplicator,
  Persistent ID). To make this possible, new classes `ObjectIdentifier`
  and `PersistentID` are introduced.
- Finding the parent of a duplicated object is done via its persistent
  ID. In Python notation, the code first tries to find the parent
  instance where `child_persistent_id[1:] == parent_persistent_id[1:]`.
  If that fails, the dupli with persistent ID `child_persistent_id[1:]`
  is used as parent.

Reviewed By: sergey

Differential Revision: https://developer.blender.org/D8233

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

M	source/blender/io/alembic/exporter/abc_hierarchy_iterator.cc
M	source/blender/io/alembic/exporter/abc_hierarchy_iterator.h
M	source/blender/io/common/CMakeLists.txt
M	source/blender/io/common/IO_abstract_hierarchy_iterator.h
A	source/blender/io/common/IO_dupli_persistent_id.hh
M	source/blender/io/common/intern/abstract_hierarchy_iterator.cc
A	source/blender/io/common/intern/dupli_parent_finder.cc
A	source/blender/io/common/intern/dupli_parent_finder.hh
A	source/blender/io/common/intern/dupli_persistent_id.cc
A	source/blender/io/common/intern/object_identifier.cc
M	tests/gtests/usd/CMakeLists.txt
A	tests/gtests/usd/object_identifier_test.cc
M	tests/python/alembic_tests.py

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

diff --git a/source/blender/io/alembic/exporter/abc_hierarchy_iterator.cc b/source/blender/io/alembic/exporter/abc_hierarchy_iterator.cc
index 90004c0e85b..c83eaf3eede 100644
--- a/source/blender/io/alembic/exporter/abc_hierarchy_iterator.cc
+++ b/source/blender/io/alembic/exporter/abc_hierarchy_iterator.cc
@@ -107,20 +107,23 @@ AbstractHierarchyIterator::ExportGraph::key_type ABCHierarchyIterator::
     determine_graph_index_object(const HierarchyContext *context)
 {
   if (params_.flatten_hierarchy) {
-    return std::make_pair(nullptr, nullptr);
+    return ObjectIdentifier::for_graph_root();
   }
 
   return AbstractHierarchyIterator::determine_graph_index_object(context);
 }
 
 AbstractHierarchyIterator::ExportGraph::key_type ABCHierarchyIterator::determine_graph_index_dupli(
-    const HierarchyContext *context, const std::set<Object *> &dupli_set)
+    const HierarchyContext *context,
+    const DupliObject *dupli_object,
+    const DupliParentFinder &dupli_parent_finder)
 {
   if (params_.flatten_hierarchy) {
-    return std::make_pair(nullptr, nullptr);
+    return ObjectIdentifier::for_graph_root();
   }
 
-  return AbstractHierarchyIterator::determine_graph_index_dupli(context, dupli_set);
+  return AbstractHierarchyIterator::determine_graph_index_dupli(
+      context, dupli_object, dupli_parent_finder);
 }
 
 Alembic::Abc::OObject ABCHierarchyIterator::get_alembic_parent(
diff --git a/source/blender/io/alembic/exporter/abc_hierarchy_iterator.h b/source/blender/io/alembic/exporter/abc_hierarchy_iterator.h
index edcb31806ba..3fe2d2c43d2 100644
--- a/source/blender/io/alembic/exporter/abc_hierarchy_iterator.h
+++ b/source/blender/io/alembic/exporter/abc_hierarchy_iterator.h
@@ -67,7 +67,9 @@ class ABCHierarchyIterator : public AbstractHierarchyIterator {
   virtual ExportGraph::key_type determine_graph_index_object(
       const HierarchyContext *context) override;
   virtual AbstractHierarchyIterator::ExportGraph::key_type determine_graph_index_dupli(
-      const HierarchyContext *context, const std::set<Object *> &dupli_set) override;
+      const HierarchyContext *context,
+      const DupliObject *dupli_object,
+      const DupliParentFinder &dupli_parent_finder) override;
 
   virtual AbstractHierarchyWriter *create_transform_writer(
       const HierarchyContext *context) override;
diff --git a/source/blender/io/common/CMakeLists.txt b/source/blender/io/common/CMakeLists.txt
index 4ed6f12762e..d2be4bbb58f 100644
--- a/source/blender/io/common/CMakeLists.txt
+++ b/source/blender/io/common/CMakeLists.txt
@@ -31,8 +31,13 @@ set(INC_SYS
 
 set(SRC
   intern/abstract_hierarchy_iterator.cc
+  intern/object_identifier.cc
+  intern/dupli_parent_finder.cc
+  intern/dupli_persistent_id.cc
 
   IO_abstract_hierarchy_iterator.h
+  IO_dupli_persistent_id.hh
+  intern/dupli_parent_finder.hh
 )
 
 set(LIB
diff --git a/source/blender/io/common/IO_abstract_hierarchy_iterator.h b/source/blender/io/common/IO_abstract_hierarchy_iterator.h
index 5f84fd48b71..2669f137fd4 100644
--- a/source/blender/io/common/IO_abstract_hierarchy_iterator.h
+++ b/source/blender/io/common/IO_abstract_hierarchy_iterator.h
@@ -36,6 +36,8 @@
 #ifndef __ABSTRACT_HIERARCHY_ITERATOR_H__
 #define __ABSTRACT_HIERARCHY_ITERATOR_H__
 
+#include "IO_dupli_persistent_id.hh"
+
 #include <map>
 #include <set>
 #include <string>
@@ -52,6 +54,7 @@ namespace blender {
 namespace io {
 
 class AbstractHierarchyWriter;
+class DupliParentFinder;
 
 /* HierarchyContext structs are created by the AbstractHierarchyIterator. Each HierarchyContext
  * struct contains everything necessary to export a single object to a file. */
@@ -60,6 +63,7 @@ struct HierarchyContext {
   Object *object; /* Evaluated object. */
   Object *export_parent;
   Object *duplicator;
+  PersistentID persistent_id;
   float matrix_world[4][4];
   std::string export_name;
 
@@ -161,6 +165,35 @@ class EnsuredWriter {
   AbstractHierarchyWriter *operator->();
 };
 
+/* Unique identifier for a (potentially duplicated) object.
+ *
+ * Instances of this class serve as key in the export graph of the
+ * AbstractHierarchyIterator. */
+class ObjectIdentifier {
+ public:
+  Object *object;
+  Object *duplicated_by; /* nullptr for real objects. */
+  PersistentID persistent_id;
+
+ protected:
+  ObjectIdentifier(Object *object, Object *duplicated_by, const PersistentID &persistent_id);
+
+ public:
+  ObjectIdentifier(const ObjectIdentifier &other);
+  ~ObjectIdentifier();
+
+  static ObjectIdentifier for_graph_root();
+  static ObjectIdentifier for_real_object(Object *object);
+  static ObjectIdentifier for_hierarchy_context(const HierarchyContext *context);
+  static ObjectIdentifier for_duplicated_object(const DupliObject *dupli_object,
+                                                Object *duplicated_by);
+
+  bool is_root() const;
+};
+
+bool operator<(const ObjectIdentifier &obj_ident_a, const ObjectIdentifier &obj_ident_b);
+bool operator==(const ObjectIdentifier &obj_ident_a, const ObjectIdentifier &obj_ident_b);
+
 /* AbstractHierarchyIterator iterates over objects in a dependency graph, and constructs export
  * writers. These writers are then called to perform the actual writing to a USD or Alembic file.
  *
@@ -172,14 +205,10 @@ class AbstractHierarchyIterator {
  public:
   /* Mapping from export path to writer. */
   typedef std::map<std::string, AbstractHierarchyWriter *> WriterMap;
-  /* Pair of a (potentially duplicated) object and its duplicator (or nullptr).
-   * This is typically used to store a pair of HierarchyContext::object and
-   * HierarchyContext::duplicator. */
-  typedef std::pair<Object *, Object *> DupliAndDuplicator;
   /* All the children of some object, as per the export hierarchy. */
   typedef std::set<HierarchyContext *> ExportChildren;
   /* Mapping from an object and its duplicator to the object's export-children. */
-  typedef std::map<DupliAndDuplicator, ExportChildren> ExportGraph;
+  typedef std::map<ObjectIdentifier, ExportChildren> ExportGraph;
   /* Mapping from ID to its export path. This is used for instancing; given an
    * instanced datablock, the export path of the original can be looked up. */
   typedef std::map<ID *, std::string> ExportPathMap;
@@ -237,7 +266,7 @@ class AbstractHierarchyIterator {
   void visit_object(Object *object, Object *export_parent, bool weak_export);
   void visit_dupli_object(DupliObject *dupli_object,
                           Object *duplicator,
-                          const std::set<Object *> &dupli_set);
+                          const DupliParentFinder &dupli_parent_finder);
 
   void context_update_for_graph_index(HierarchyContext *context,
                                       const ExportGraph::key_type &graph_index) const;
@@ -291,8 +320,10 @@ class AbstractHierarchyIterator {
   virtual bool should_visit_dupli_object(const DupliObject *dupli_object) const;
 
   virtual ExportGraph::key_type determine_graph_index_object(const HierarchyContext *context);
-  virtual ExportGraph::key_type determine_graph_index_dupli(const HierarchyContext *context,
-                                                            const std::set<Object *> &dupli_set);
+  virtual ExportGraph::key_type determine_graph_index_dupli(
+      const HierarchyContext *context,
+      const DupliObject *dupli_object,
+      const DupliParentFinder &dupli_parent_finder);
 
   /* These functions should create an AbstractHierarchyWriter subclass instance, or return
    * nullptr if the object or its data should not be exported. Returning a nullptr for
diff --git a/source/blender/io/common/IO_dupli_persistent_id.hh b/source/blender/io/common/IO_dupli_persistent_id.hh
new file mode 100644
index 00000000000..dc70c1cdf31
--- /dev/null
+++ b/source/blender/io/common/IO_dupli_persistent_id.hh
@@ -0,0 +1,61 @@
+/*
+ * 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) 2020 Blender Foundation.
+ * All rights reserved.
+ */
+#ifndef __IO_COMMON_DUPLI_PERSISTENT_ID_H__
+#define __IO_COMMON_DUPLI_PERSISTENT_ID_H__
+
+#include "BKE_duplilist.h"
+
+#include "DNA_object_types.h" /* For MAX_DUPLI_RECUR */
+
+#include <array>
+#include <optional>
+#include <ostream>
+
+namespace blender::io {
+
+/* Wrapper for DupliObject::persistent_id that can act as a map key. */
+class PersistentID {
+ protected:
+  constexpr static int array_length_ = MAX_DUPLI_RECUR;
+  typedef std::array<int, array_length_> PIDArray;
+  PIDArray persistent_id_;
+
+  explicit PersistentID(const PIDArray &persistent_id_values);
+
+ public:
+  PersistentID();
+  explicit PersistentID(const DupliObject *dupli_ob);
+
+  /* Return true iff the persistent IDs are the same, ignoring the first digit. */
+  bool is_from_same_instancer_as(const PersistentID &other) const;
+
+  /* Construct the persistent ID of this instance's instancer. */
+  PersistentID instancer_pid() const;
+
+  friend bool operator==(const PersistentID &persistent_id_a, const PersistentID &persistent_id_b);
+  friend bool operator<(const PersistentID &persistent_id_a, const PersistentID &persistent_id_b);
+  friend std::ostream &operator<<(std::ostream &os, const PersistentID &persistent_id);
+
+ private:
+  void copy_values_from(const PIDArray &persistent_id_values);
+};
+
+}  // namespace blender::io
+
+#endif  // __IO_COMMON_DUPLI_PARENT_FINDER_H__
diff --git a/source/blender/io/common/intern/abstract_hierarchy_iterator.cc b/source/blender/io/common/intern/abstract_hierarchy_iterator.cc
index 1d67792053a..c8d916c0950 100644
--- a/source/blen

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list