From e021cf04919f04c1962ae3f6f364f48854a6d68b Mon Sep 17 00:00:00 2001 From: Michael Kowalski Date: Wed, 29 Jan 2025 14:05:52 +0100 Subject: [PATCH] Fix: USD export: missing prototype proxies The scene graph instancing export code contains logic for determining which instances need to be converted to prototypes because the original prototypes are not included in the export (e.g., because they are not visible). This commit fixes an error in this logic, which incorrectly assumed that if the root of the original prototype is included in the export, then the entire original hierarchy beneath the root is included as well. To fix this, the logic in AbstractHierarchyIterator:: determine_duplication_references was updated so that if any descendants of an instance are converted to prototypes, the parent instance is converted to a prototype as well. This addresses the bug noted by Brecht in https://projects.blender.org/blender/blender/pulls/131707#issuecomment-1403309 Pull Request: https://projects.blender.org/blender/blender/pulls/133750 --- .../common/IO_abstract_hierarchy_iterator.h | 10 +--------- .../intern/abstract_hierarchy_iterator.cc | 19 ++++++++++++++++--- .../io/usd/intern/usd_hierarchy_iterator.cc | 6 ------ .../io/usd/intern/usd_hierarchy_iterator.hh | 5 ----- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/source/blender/io/common/IO_abstract_hierarchy_iterator.h b/source/blender/io/common/IO_abstract_hierarchy_iterator.h index 0b8405e2234..0f954314a1b 100644 --- a/source/blender/io/common/IO_abstract_hierarchy_iterator.h +++ b/source/blender/io/common/IO_abstract_hierarchy_iterator.h @@ -281,7 +281,7 @@ class AbstractHierarchyIterator { const ExportGraph::key_type &graph_index) const; void determine_export_paths(const HierarchyContext *parent_context); - void determine_duplication_references(const HierarchyContext *parent_context, + bool determine_duplication_references(const HierarchyContext *parent_context, const std::string &indent); /* These three functions create writers and call their write() method. */ @@ -365,14 +365,6 @@ class AbstractHierarchyIterator { AbstractHierarchyWriter *get_writer(const std::string &export_path) const; ExportChildren &graph_children(const HierarchyContext *context); - - /* Return true if duplication references should be resolved for the children of the given - * context. */ - virtual bool should_determine_duplication_references( - const HierarchyContext *parent_context) const - { - return parent_context != nullptr; - } }; } // namespace blender::io diff --git a/source/blender/io/common/intern/abstract_hierarchy_iterator.cc b/source/blender/io/common/intern/abstract_hierarchy_iterator.cc index a661157773c..ac515368c7b 100644 --- a/source/blender/io/common/intern/abstract_hierarchy_iterator.cc +++ b/source/blender/io/common/intern/abstract_hierarchy_iterator.cc @@ -543,11 +543,15 @@ void AbstractHierarchyIterator::determine_export_paths(const HierarchyContext *p } } -void AbstractHierarchyIterator::determine_duplication_references( +bool AbstractHierarchyIterator::determine_duplication_references( const HierarchyContext *parent_context, const std::string &indent) { ExportChildren children = graph_children(parent_context); + /* Will be set to true if any child contexts are instances that were designated + * as proxies for the original prototype.*/ + bool contains_proxy_prototype = false; + for (HierarchyContext *context : children) { if (context->duplicator != nullptr) { ID *source_id = &context->object->id; @@ -557,6 +561,7 @@ void AbstractHierarchyIterator::determine_duplication_references( /* The original was not found, so mark this instance as "the original". */ context->mark_as_not_instanced(); duplisource_export_path_[source_id] = context->export_path; + contains_proxy_prototype = true; } else { context->mark_as_instance_of(it->second); @@ -583,10 +588,18 @@ void AbstractHierarchyIterator::determine_duplication_references( } } - if (should_determine_duplication_references(context)) { - determine_duplication_references(context, indent + " "); + if (determine_duplication_references(context, indent + " ")) { + /* A descendant was designated a prototype proxy. If the current context + * is an instance, we must change it to a prototype proxy as well. */ + if (context->is_instance()) { + context->mark_as_not_instanced(); + ID *source_id = &context->object->id; + duplisource_export_path_[source_id] = context->export_path; + } + contains_proxy_prototype = true; } } + return contains_proxy_prototype; } void AbstractHierarchyIterator::make_writers(const HierarchyContext *parent_context) diff --git a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc index febbc45f72f..6f759ba1863 100644 --- a/source/blender/io/usd/intern/usd_hierarchy_iterator.cc +++ b/source/blender/io/usd/intern/usd_hierarchy_iterator.cc @@ -263,12 +263,6 @@ bool USDHierarchyIterator::include_child_writers(const HierarchyContext *context return !(params_.use_instancing && context->is_instance()); } -bool USDHierarchyIterator::should_determine_duplication_references( - const HierarchyContext *parent_context) const -{ - return !(params_.use_instancing && parent_context->is_instance()); -} - void USDHierarchyIterator::add_usd_skel_export_mapping(const Object *obj, const pxr::SdfPath &path) { if (params_.export_shapekeys && is_mesh_with_shape_keys(obj)) { diff --git a/source/blender/io/usd/intern/usd_hierarchy_iterator.hh b/source/blender/io/usd/intern/usd_hierarchy_iterator.hh index 7b30823a8ec..ef297a4599c 100644 --- a/source/blender/io/usd/intern/usd_hierarchy_iterator.hh +++ b/source/blender/io/usd/intern/usd_hierarchy_iterator.hh @@ -60,11 +60,6 @@ class USDHierarchyIterator : public AbstractHierarchyIterator { virtual bool include_data_writers(const HierarchyContext *context) const override; virtual bool include_child_writers(const HierarchyContext *context) const override; - /* Return true if duplication references should be resolved for the children of the given - * context. */ - virtual bool should_determine_duplication_references( - const HierarchyContext *parent_context) const override; - private: USDExporterContext create_usd_export_context(const HierarchyContext *context);