Alembic: fixed importer
authorSybren A. Stüvel <sybren@stuvel.eu>
Wed, 15 Feb 2017 16:29:26 +0000 (17:29 +0100)
committerSybren A. Stüvel <sybren@stuvel.eu>
Thu, 6 Apr 2017 14:04:31 +0000 (16:04 +0200)
The importer was guessing whether an Alembic IXform object was part of a
child object, or should be represented as an Empty in Blender. By reversing
the order in which objects are visited, the children can now claim their
parent as part of the same object (so IPolyMesh claims its parent IXform
as part of the same Blender object). This results in much less guesswork.

I've also removed similar guesswork from the code that sets parent pointers,
by simply searching for the parent in a hierarchical way, instead of trying
to predict (again) which IXforms were turned into empties.

Also, visit_object() now actually visits the object -- previously it only
visited its children, and assumed the object it was called on was already
handled by a previous call.

source/blender/alembic/intern/alembic_capi.cc

index 6c6f64e1669e2b49dbca4352134302920c1850f1..cf058be3aee9a2d1b8e55b036183da7200c9c272 100644 (file)
@@ -152,7 +152,7 @@ static void gather_objects_paths(const IObject &object, ListBase *object_paths)
                                        }
 #if 0
                                        else {
-                                               std::cerr << "Skipping " << child.getFullName() << '\n';
+                                               std::cerr << "gather_objects_paths(" << object.getFullName() << "): Skipping " << child.getFullName() << '\n';
                                        }
 #endif
                                }
@@ -397,114 +397,134 @@ void ABC_export(
 
 /* ********************** Import file ********************** */
 
-static void visit_object(const IObject &object,
+/**
+ * Generates an AbcObjectReader for this Alembic object and its children.
+ *
+ * \param object The Alembic IObject to visit.
+ * \param readers The created AbcObjectReader * will be appended to this vector.
+ * \param readers_map The created AbcObjectReader * will be appended to this
+ *                    map, keyed by its full name in Alembic.
+ * \param settings Import settings, not used directly but passed to the
+ *                 AbcObjectReader subclass constructors.
+ * \return whether this IObject claims its parent as part of the same object
+ *         (for example an IPolyMesh object would claim its parent, as the mesh
+ *         is interpreted as the object's data, and the parent IXform as its
+ *         Blender object).
+ */
+static bool visit_object(const IObject &object,
                          std::vector<AbcObjectReader *> &readers,
                          GHash *parent_map,
                          ImportSettings &settings)
 {
        if (!object.valid()) {
-               return;
+               std::cerr << "  - " << object.getFullName() << ": object is invalid, skipping it and all its children.\n";
+               return false;
        }
 
-       for (int i = 0; i < object.getNumChildren(); ++i) {
-               IObject child = object.getChild(i);
-
-               if (!child.valid()) {
-                       continue;
-               }
+       // The interpretation of data by the children determine the role of this object.
+       // This is especially important for Xform objects, as they can be either part of a Blender object
+       // or a Blender object (Empty) themselves.
+       size_t children_claiming_this_object = 0;
+       size_t num_children = object.getNumChildren();
+       for (size_t i = 0; i < num_children; ++i) {
+               bool child_claims_this_object = visit_object(object.getChild(i), readers, parent_map, settings);
+               children_claiming_this_object += child_claims_this_object ? 1 : 0;
+       }
 
-               AbcObjectReader *reader = NULL;
+       AbcObjectReader *reader = NULL;
+       const MetaData &md = object.getMetaData();
+       bool parent_is_part_of_this_object = false;
 
-               const MetaData &md = child.getMetaData();
+       if (!object.getParent()) {
+               // The root itself is not an object we should import.
+       }
+       else if (IXform::matches(md)) {
+               bool create_empty;
 
-               if (IXform::matches(md)) {
-                       bool create_xform = false;
+               /* An xform can either be a Blender Object (if it contains a mesh, for exapmle),
+                * but it can also be an Empty. Its correct translation to Blender's data model
+                * depends on its children. */
 
-                       /* Check whether or not this object is a Maya locator, which is
-                        * similar to empties used as parent object in Blender. */
-                       if (has_property(child.getProperties(), "locator")) {
-                               create_xform = true;
-                       }
-                       else {
-                               /* Avoid creating an empty object if the child of this transform
-                                * is not a transform (that is an empty). */
-                               if (child.getNumChildren() == 1) {
-                                       if (IXform::matches(child.getChild(0).getMetaData())) {
-                                               create_xform = true;
-                                       }
-#if 0
-                                       else {
-                                               std::cerr << "Skipping " << child.getFullName() << '\n';
-                                       }
-#endif
-                               }
-                               else {
-                                       create_xform = true;
-                               }
-                       }
-
-                       if (create_xform) {
-                               reader = new AbcEmptyReader(child, settings);
-                       }
+               /* Check whether or not this object is a Maya locator, which is
+                * similar to empties used as parent object in Blender. */
+               if (has_property(object.getProperties(), "locator")) {
+                       create_empty = true;
                }
-               else if (IPolyMesh::matches(md)) {
-                       reader = new AbcMeshReader(child, settings);
+               else {
+                       create_empty = children_claiming_this_object == 0;
                }
-               else if (ISubD::matches(md)) {
-                       reader = new AbcSubDReader(child, settings);
+
+               if (create_empty) {
+                       reader = new AbcEmptyReader(object, settings);
                }
-               else if (INuPatch::matches(md)) {
+       }
+       else if (IPolyMesh::matches(md)) {
+               reader = new AbcMeshReader(object, settings);
+               parent_is_part_of_this_object = true;
+       }
+       else if (ISubD::matches(md)) {
+               reader = new AbcSubDReader(object, settings);
+               parent_is_part_of_this_object = true;
+       }
+       else if (INuPatch::matches(md)) {
 #ifdef USE_NURBS
-                       /* TODO(kevin): importing cyclic NURBS from other software crashes
-                        * at the moment. This is due to the fact that NURBS in other
-                        * software have duplicated points which causes buffer overflows in
-                        * Blender. Need to figure out exactly how these points are
-                        * duplicated, in all cases (cyclic U, cyclic V, and cyclic UV).
-                        * Until this is fixed, disabling NURBS reading. */
-                       reader = new AbcNurbsReader(child, settings);
+               /* TODO(kevin): importing cyclic NURBS from other software crashes
+                * at the moment. This is due to the fact that NURBS in other
+                * software have duplicated points which causes buffer overflows in
+                * Blender. Need to figure out exactly how these points are
+                * duplicated, in all cases (cyclic U, cyclic V, and cyclic UV).
+                * Until this is fixed, disabling NURBS reading. */
+               reader = new AbcNurbsReader(object, settings);
+               parent_is_part_of_this_object = true;
 #endif
-               }
-               else if (ICamera::matches(md)) {
-                       reader = new AbcCameraReader(child, settings);
-               }
-               else if (IPoints::matches(md)) {
-                       reader = new AbcPointsReader(child, settings);
-               }
-               else if (IMaterial::matches(md)) {
-                       /* Pass for now. */
-               }
-               else if (ILight::matches(md)) {
-                       /* Pass for now. */
-               }
-               else if (IFaceSet::matches(md)) {
-                       /* Pass, those are handled in the mesh reader. */
-               }
-               else if (ICurves::matches(md)) {
-                       reader = new AbcCurveReader(child, settings);
-               }
-               else {
-                       assert(false);
-               }
-
-               if (reader) {
-                       readers.push_back(reader);
-                       reader->incref();
+       }
+       else if (ICamera::matches(md)) {
+               reader = new AbcCameraReader(object, settings);
+               parent_is_part_of_this_object = true;
+       }
+       else if (IPoints::matches(md)) {
+               reader = new AbcPointsReader(object, settings);
+               parent_is_part_of_this_object = true;
+       }
+       else if (IMaterial::matches(md)) {
+               /* Pass for now. */
+       }
+       else if (ILight::matches(md)) {
+               /* Pass for now. */
+       }
+       else if (IFaceSet::matches(md)) {
+               /* Pass, those are handled in the mesh reader. */
+       }
+       else if (ICurves::matches(md)) {
+               reader = new AbcCurveReader(object, settings);
+               parent_is_part_of_this_object = true;
+       }
+       else {
+               std::cerr << "object is of unsupported schema type "
+                         << "'" << object.getMetaData().get("schemaObjTitle") << "'"
+                         << std::endl;
+               BLI_assert(false);
+               return false;
+       }
 
-                       AlembicObjectPath *abc_path = static_cast<AlembicObjectPath *>(
-                                                         MEM_callocN(sizeof(AlembicObjectPath), "AlembicObjectPath"));
+       if (reader) {
+               readers.push_back(reader);
+               reader->incref();
 
-                       BLI_strncpy(abc_path->path, child.getFullName().c_str(), PATH_MAX);
+               AlembicObjectPath *abc_path = static_cast<AlembicObjectPath *>(
+                                                 MEM_callocN(sizeof(AlembicObjectPath), "AlembicObjectPath"));
 
-                       BLI_addtail(&settings.cache_file->object_paths, abc_path);
+               BLI_strncpy(abc_path->path, object.getFullName().c_str(), PATH_MAX);
 
-                       /* Cast to `void *` explicitly to avoid compiler errors because it
-                        * is a `const char *` which the compiler cast to `const void *`
-                        * instead of the expected `void *`. */
-                       BLI_ghash_insert(parent_map, (void *)child.getFullName().c_str(), reader);
-               }
+               BLI_addtail(&settings.cache_file->object_paths, abc_path);
 
-               visit_object(child, readers, parent_map, settings);
+               /* Cast to `void *` explicitly to avoid compiler errors because it
+                * is a `const char *` which the compiler cast to `const void *`
+                * instead of the expected `void *`. */
+               BLI_ghash_insert(parent_map, (void *)object.getFullName().c_str(), reader);
        }
+
+       return parent_is_part_of_this_object;
 }
 
 enum {
@@ -599,7 +619,6 @@ static void import_startjob(void *user_data, short *stop, short *do_update, floa
        data->parent_map = BLI_ghash_str_new("alembic parent ghash");
 
        /* Parse Alembic Archive. */
-
        visit_object(archive->getTop(), data->readers, data->parent_map, data->settings);
 
        if (G.is_break) {
@@ -629,6 +648,9 @@ static void import_startjob(void *user_data, short *stop, short *do_update, floa
                        min_time = std::min(min_time, reader->minTime());
                        max_time = std::max(max_time, reader->maxTime());
                }
+               else {
+                       std::cerr << "Object " << reader->name() << " in Alembic file " << data->filename << " is invalid.\n";
+               }
 
                *data->progress = 0.1f + 0.6f * (++i / size);
                *data->do_update = true;
@@ -662,29 +684,27 @@ static void import_startjob(void *user_data, short *stop, short *do_update, floa
                const AbcObjectReader *parent_reader = NULL;
                const IObject &iobject = reader->iobject();
 
-               IObject parent = iobject.getParent();
-
-               if (!IXform::matches(iobject.getHeader())) {
-                       /* In the case of an non XForm node, the parent is the transform
-                        * matrix of the data itself, so we get the its grand parent.
-                        */
+               /* Find the parent reader by going up in the Alembic hierarchy until we find it.
+                * Some Xform Alembic objects do not produce an AbcEmptyReader, since they
+                * translate to a Blender object with a reader attached to the Xform's child. */
+               IObject alembic_parent = iobject.getParent();
 
-                       /* Special case with object only containing a mesh and some strands,
-                        * we want both objects to be parented to the same object. */
-                       if (!is_mesh_and_strands(parent)) {
-                               parent = parent.getParent();
+               while (alembic_parent) {
+                       parent_reader = reinterpret_cast<AbcObjectReader *>(
+                                           BLI_ghash_lookup(data->parent_map, alembic_parent.getFullName().c_str()));
+                       if (parent_reader != NULL) {
+                               break;  // found the parent reader.
                        }
-               }
 
-               parent_reader = reinterpret_cast<AbcObjectReader *>(
-                                   BLI_ghash_lookup(data->parent_map, parent.getFullName().c_str()));
+                       alembic_parent = alembic_parent.getParent();
+               }
 
                if (parent_reader) {
-                       Object *parent = parent_reader->object();
+                       Object *blender_parent = parent_reader->object();
 
-                       if (parent != NULL && reader->object() != parent) {
+                       if (blender_parent != NULL && reader->object() != blender_parent) {
                                Object *ob = reader->object();
-                               ob->parent = parent;
+                               ob->parent = blender_parent;
                        }
                }