griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix #124263: Generate unique names during Alembic and USD export

Browse Source 
While object names in Blender are already unique, the names themselves
may be "unsafe" for use in the various file formats. During processing
we make the names "safe". However, we did not guarantee that these new
safe names were themselves unique wrt each other. Consider object names
"Test 1" and "Test-1" which both become "Test_1" after being made safe.
These will collide during export; only 1 object would be exported and
it's undefined which object's data would "win".

To rectify this we add another name map to the hierarchy iterator which
is then used to handle collisions as they happen. The map is per-
hierarchy meaning that a name can appear more than once as long as its
under a different hierarchy. E.g.
- `/root/A/X` and another `/root/B/X` is OK
- `/root/A/X` and another `/root/A/X` is NOT OK

Pull Request: https://projects.blender.org/blender/blender/pulls/135418
This commit is contained in:
Jesse Yurkovich
2025-03-21 21:29:08 +01:00
committed by Jesse Yurkovich
parent 321dbb0115
commit 01ddb320dd
4 changed files with 133 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
source/blender/io/common/IO_abstract_hierarchy_iterator.h
View File

@@ -221,6 +221,9 @@ class AbstractHierarchyIterator {
/* 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. */
using ExportPathMap = blender::Map<ID *, std::string>;
/* Mapping from ID name to a set of names logically residing "under" it. Used for unique
* name generation. */
using ExportUsedNameMap = blender::Map<std::string, blender::Set<std::string>>;
/* IDs of all duplisource objects, used to identify instance prototypes. */
using DupliSources = blender::Set<ID *>;
@@ -232,6 +235,7 @@ class AbstractHierarchyIterator {
WriterMap writers_;
ExportSubset export_subset_;
DupliSources duplisources_;
ExportUsedNameMap used_names_;
public:
explicit AbstractHierarchyIterator(Main *bmain, Depsgraph *depsgraph);
@@ -258,6 +262,9 @@ class AbstractHierarchyIterator {
* This base implementation is a no-op; override in a concrete subclass. */
virtual std::string make_valid_name(const std::string &name) const;
virtual std::string make_unique_name(const std::string &original_name,
Set<std::string> &used_names);
/* Return the name of this ID datablock that is valid for the exported file format. Overriding is
* only necessary if make_valid_name(id->name+2) is not suitable for the exported file format.
* NULL-safe: when `id == nullptr` this returns an empty string. */
@@ -298,7 +305,8 @@ class AbstractHierarchyIterator {
HierarchyContext context_for_object_data(const HierarchyContext *object_context) const;
/* Convenience wrappers around get_id_name(). */
std::string get_object_name(const Object *object) const;
std::string get_object_name(const Object *object);
std::string get_object_name(const Object *object, const Object *parent);
std::string get_object_data_name(const Object *object) const;
using create_writer_func =

33
source/blender/io/common/intern/abstract_hierarchy_iterator.cc
View File

@@ -17,6 +17,8 @@
#include "BLI_assert.h"
#include "BLI_listbase.h"
#include "BLI_math_matrix.h"
#include "BLI_set.hh"
#include "BLI_string_utils.hh"
#include "DNA_ID.h"
#include "DNA_layer_types.h"
@@ -206,6 +208,22 @@ std::string AbstractHierarchyIterator::get_id_name(const ID *id) const
return make_valid_name(std::string(id->name + 2));
}
std::string AbstractHierarchyIterator::make_unique_name(const std::string &original_name,
Set<std::string> &used_names)
{
if (original_name.empty()) {
return "";
}
std::string name = BLI_uniquename_cb(
[&](const StringRef check_name) { return used_names.contains_as(check_name); },
'_',
make_valid_name(original_name));
used_names.add_new(name);
return name;
}
std::string AbstractHierarchyIterator::get_object_data_path(const HierarchyContext *context) const
{
BLI_assert(!context->export_path.empty());
@@ -388,6 +406,7 @@ void AbstractHierarchyIterator::export_graph_clear()
}
}
export_graph_.clear();
used_names_.clear_and_keep_capacity();
}
void AbstractHierarchyIterator::visit_object(Object *object,
@@ -397,7 +416,7 @@ void AbstractHierarchyIterator::visit_object(Object *object,
HierarchyContext *context = new HierarchyContext();
context->object = object;
context->is_object_data_context = false;
context->export_name = get_object_name(object);
context->export_name = get_object_name(object, export_parent);
context->export_parent = export_parent;
context->duplicator = nullptr;
context->weak_export = weak_export;
@@ -450,7 +469,9 @@ void AbstractHierarchyIterator::visit_dupli_object(DupliObject *dupli_object,
/* Construct export name for the dupli-instance. */
std::string export_name = get_object_name(context->object) + "-" +
context->persistent_id.as_object_name_suffix();
context->export_name = make_valid_name(export_name);
Set<std::string> &used_names = used_names_.lookup_or_add(duplicator->id.name, {});
context->export_name = make_unique_name(make_valid_name(export_name), used_names);
ObjectIdentifier graph_index = determine_graph_index_dupli(
context, dupli_object, dupli_parent_finder);
@@ -737,11 +758,17 @@ void AbstractHierarchyIterator::make_writers_particle_systems(
}
}
std::string AbstractHierarchyIterator::get_object_name(const Object *object) const
std::string AbstractHierarchyIterator::get_object_name(const Object *object)
{
return get_id_name(&object->id);
}
std::string AbstractHierarchyIterator::get_object_name(const Object *object, const Object *parent)
{
Set<std::string> &used_names = used_names_.lookup_or_add(parent ? parent->id.name : "", {});
return make_unique_name(object->id.name + 2, used_names);
}
std::string AbstractHierarchyIterator::get_object_data_name(const Object *object) const
{
ID *object_data = static_cast<ID *>(object->data);

2
tests/data

Submodule tests/data updated: 51833b78c1...90bc81396b

104
tests/python/bl_usd_export_test.py
View File

@@ -1258,15 +1258,11 @@ class USDExportTest(AbstractUSDTest):
("/root/Camera/Camera", "Camera"),
("/root/env_light", "DomeLight")
)
def key(el):
return el[0]
expected = tuple(sorted(expected, key=key))
expected = tuple(sorted(expected, key=lambda pair: pair[0]))
stage = Usd.Stage.Open(str(test_path))
actual = ((str(p.GetPath()), p.GetTypeName()) for p in stage.Traverse())
actual = tuple(sorted(actual, key=key))
actual = tuple(sorted(actual, key=lambda pair: pair[0]))
self.assertTupleEqual(expected, actual)
@@ -1311,14 +1307,11 @@ class USDExportTest(AbstractUSDTest):
("/root/env_light", "DomeLight")
)
def key(el):
return el[0]
expected = tuple(sorted(expected, key=key))
expected = tuple(sorted(expected, key=lambda pair: pair[0]))
stage = Usd.Stage.Open(str(test_path))
actual = ((str(p.GetPath()), p.GetTypeName()) for p in stage.Traverse())
actual = tuple(sorted(actual, key=key))
actual = tuple(sorted(actual, key=lambda pair: pair[0]))
self.assertTupleEqual(expected, actual)
@@ -1490,6 +1483,95 @@ class USDExportTest(AbstractUSDTest):
self.assertTrue(tex_path.exists(),
f"Exported texture {tex_path} doesn't exist")
def test_naming_collision_hierarchy(self):
"""Validate that naming collisions during export are handled correctly"""
bpy.ops.wm.open_mainfile(filepath=str(self.testdir / "usd_hierarchy_collision.blend"))
export_path = self.tempdir / "usd_hierarchy_collision.usda"
self.export_and_validate(filepath=str(export_path))
expected = (
('/root', 'Xform'),
('/root/Empty', 'Xform'),
('/root/Empty/Par_002', 'Xform'),
('/root/Empty/Par_002/Par_1', 'Mesh'),
('/root/Empty/Par_003', 'Xform'),
('/root/Empty/Par_003/Par_1', 'Mesh'),
('/root/Empty/Par_004', 'Xform'),
('/root/Empty/Par_004/Par_002', 'Mesh'),
('/root/Empty/Par_1', 'Xform'),
('/root/Empty/Par_1/Par_1', 'Mesh'),
('/root/Level1', 'Xform'),
('/root/Level1/Level2', 'Xform'),
('/root/Level1/Level2/Par2_002', 'Xform'),
('/root/Level1/Level2/Par2_002/Par2_002', 'Mesh'),
('/root/Level1/Level2/Par2_1', 'Xform'),
('/root/Level1/Level2/Par2_1/Par2_1', 'Mesh'),
('/root/Level1/Par2_002', 'Xform'),
('/root/Level1/Par2_002/Par2_1', 'Mesh'),
('/root/Level1/Par2_1', 'Xform'),
('/root/Level1/Par2_1/Par2_1', 'Mesh'),
('/root/Test_002', 'Xform'),
('/root/Test_002/Test_1', 'Mesh'),
('/root/Test_003', 'Xform'),
('/root/Test_003/Test_1', 'Mesh'),
('/root/Test_004', 'Xform'),
('/root/Test_004/Test_002', 'Mesh'),
('/root/Test_1', 'Xform'),
('/root/Test_1/Test_1', 'Mesh'),
('/root/env_light', 'DomeLight'),
('/root/xSource_002', 'Xform'),
('/root/xSource_002/Dup_002', 'Xform'),
('/root/xSource_002/Dup_002/Dup_002', 'Mesh'),
('/root/xSource_002/Dup_002_0', 'Xform'),
('/root/xSource_002/Dup_002_0/Dup_002', 'Mesh'),
('/root/xSource_002/Dup_002_1', 'Xform'),
('/root/xSource_002/Dup_002_1/Dup_002', 'Mesh'),
('/root/xSource_002/Dup_002_2', 'Xform'),
('/root/xSource_002/Dup_002_2/Dup_002', 'Mesh'),
('/root/xSource_002/Dup_002_3', 'Xform'),
('/root/xSource_002/Dup_002_3/Dup_002', 'Mesh'),
('/root/xSource_002/Dup_1', 'Xform'),
('/root/xSource_002/Dup_1/Dup_1', 'Mesh'),
('/root/xSource_002/Dup_1_0', 'Xform'),
('/root/xSource_002/Dup_1_0/Dup_1', 'Mesh'),
('/root/xSource_002/Dup_1_1', 'Xform'),
('/root/xSource_002/Dup_1_1/Dup_1', 'Mesh'),
('/root/xSource_002/Dup_1_2', 'Xform'),
('/root/xSource_002/Dup_1_2/Dup_1', 'Mesh'),
('/root/xSource_002/Dup_1_3', 'Xform'),
('/root/xSource_002/Dup_1_3/Dup_1', 'Mesh'),
('/root/xSource_002/xSource_1', 'Mesh'),
('/root/xSource_1', 'Xform'),
('/root/xSource_1/Dup_002', 'Xform'),
('/root/xSource_1/Dup_002/Dup_1', 'Mesh'),
('/root/xSource_1/Dup_1', 'Xform'),
('/root/xSource_1/Dup_1/Dup_1', 'Mesh'),
('/root/xSource_1/Dup_1_0', 'Xform'),
('/root/xSource_1/Dup_1_0/Dup_1', 'Mesh'),
('/root/xSource_1/Dup_1_001', 'Xform'),
('/root/xSource_1/Dup_1_001/Dup_1', 'Mesh'),
('/root/xSource_1/Dup_1_002', 'Xform'),
('/root/xSource_1/Dup_1_002/Dup_1', 'Mesh'),
('/root/xSource_1/Dup_1_003', 'Xform'),
('/root/xSource_1/Dup_1_003/Dup_1', 'Mesh'),
('/root/xSource_1/Dup_1_004', 'Xform'),
('/root/xSource_1/Dup_1_004/Dup_1', 'Mesh'),
('/root/xSource_1/Dup_1_1', 'Xform'),
('/root/xSource_1/Dup_1_1/Dup_1', 'Mesh'),
('/root/xSource_1/Dup_1_2', 'Xform'),
('/root/xSource_1/Dup_1_2/Dup_1', 'Mesh'),
('/root/xSource_1/Dup_1_3', 'Xform'),
('/root/xSource_1/Dup_1_3/Dup_1', 'Mesh'),
('/root/xSource_1/xSource_1', 'Mesh')
)
expected = tuple(sorted(expected, key=lambda pair: pair[0]))
stage = Usd.Stage.Open(str(export_path))
actual = ((str(p.GetPath()), p.GetTypeName()) for p in stage.Traverse())
actual = tuple(sorted(actual, key=lambda pair: pair[0]))
self.assertTupleEqual(expected, actual)
class USDHookBase:
instructions = {}