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

Cleanup: IO: Use blender containers for AbstractHierarchyIterator

Browse Source 
Use Blender Containers in the AbstractHierarchyIterator system. A future
bug fix in the area is looking like it will require another map so it
seems like a good time to replace the old containers before that lands.

The conversion is complicated due to how the prior code leveraged
`operator[]` to both add and update entries; it was never clear which
operation was being performed. This leads to a somewhat confusing mix
of `add_new`, `add`, and `lookup_or_add` calls. The calls in place now
are what was required based on our existing tests.

Additionally, pointer stability guarantees were a little bit different,
affecting the usage of the `graph_children` function the most.

Pull Request: https://projects.blender.org/blender/blender/pulls/134698
This commit is contained in:
Jesse Yurkovich
2025-02-28 05:58:46 +01:00
committed by Jesse Yurkovich
parent 21e7a48f2f
commit 6704647c66
15 changed files with 180 additions and 313 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
source/blender/io/alembic/exporter/abc_hierarchy_iterator.cc
View File

@@ -49,7 +49,7 @@ void ABCHierarchyIterator::update_bounding_box_recursive(Imath::Box3d &bounds,
const HierarchyContext *context)
{
if (context != nullptr) {
AbstractHierarchyWriter *abstract_writer = writers_[context->export_path];
AbstractHierarchyWriter *abstract_writer = writers_.lookup(context->export_path);
ABCAbstractWriter *abc_writer = static_cast<ABCAbstractWriter *>(abstract_writer);
if (abc_writer != nullptr) {
@@ -57,7 +57,12 @@ void ABCHierarchyIterator::update_bounding_box_recursive(Imath::Box3d &bounds,
}
}
for (HierarchyContext *child_context : graph_children(context)) {
ExportChildren *children = graph_children(context);
if (!children) {
return;
}
for (HierarchyContext *child_context : *children) {
update_bounding_box_recursive(bounds, child_context);
}
}
@@ -86,8 +91,8 @@ std::string ABCHierarchyIterator::make_valid_name(const std::string &name) const
return abc_name;
}
AbstractHierarchyIterator::ExportGraph::key_type ABCHierarchyIterator::
determine_graph_index_object(const HierarchyContext *context)
ObjectIdentifier ABCHierarchyIterator::determine_graph_index_object(
const HierarchyContext *context)
{
if (params_.flatten_hierarchy) {
return ObjectIdentifier::for_graph_root();
@@ -96,7 +101,7 @@ AbstractHierarchyIterator::ExportGraph::key_type ABCHierarchyIterator::
return AbstractHierarchyIterator::determine_graph_index_object(context);
}
AbstractHierarchyIterator::ExportGraph::key_type ABCHierarchyIterator::determine_graph_index_dupli(
ObjectIdentifier ABCHierarchyIterator::determine_graph_index_dupli(
const HierarchyContext *context,
const DupliObject *dupli_object,
const DupliParentFinder &dupli_parent_finder)

4
source/blender/io/alembic/exporter/abc_hierarchy_iterator.h
View File

@@ -50,8 +50,8 @@ class ABCHierarchyIterator : public AbstractHierarchyIterator {
protected:
bool mark_as_weak_export(const Object *object) const override;
ExportGraph::key_type determine_graph_index_object(const HierarchyContext *context) override;
AbstractHierarchyIterator::ExportGraph::key_type determine_graph_index_dupli(
ObjectIdentifier determine_graph_index_object(const HierarchyContext *context) override;
ObjectIdentifier determine_graph_index_dupli(
const HierarchyContext *context,
const DupliObject *dupli_object,
const DupliParentFinder &dupli_parent_finder) override;

1
source/blender/io/common/CMakeLists.txt
View File

@@ -49,7 +49,6 @@ target_link_libraries(bf_io_common INTERFACE)
if(WITH_GTESTS)
set(TEST_SRC
intern/abstract_hierarchy_iterator_test.cc
intern/hierarchy_context_order_test.cc
intern/object_identifier_test.cc
intern/string_utils_tests.cc
)

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

@@ -22,10 +22,12 @@
#include "IO_dupli_persistent_id.hh"
#include "BLI_hash.hh"
#include "BLI_map.hh"
#include "BLI_set.hh"
#include "DEG_depsgraph.hh"
#include <map>
#include <set>
#include <string>
struct Depsgraph;
@@ -101,8 +103,6 @@ struct HierarchyContext {
* refers to a different object). */
std::string higher_up_export_path;
bool operator<(const HierarchyContext &other) const;
/* Return a HierarchyContext representing the root of the export hierarchy. */
static const HierarchyContext *root();
@@ -194,9 +194,13 @@ class ObjectIdentifier {
Object *duplicated_by);
bool is_root() const;
uint64_t hash() const
{
return get_default_hash(object, duplicated_by, persistent_id);
}
};
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
@@ -209,16 +213,16 @@ bool operator==(const ObjectIdentifier &obj_ident_a, const ObjectIdentifier &obj
class AbstractHierarchyIterator {
public:
/* Mapping from export path to writer. */
using WriterMap = std::map<std::string, AbstractHierarchyWriter *>;
using WriterMap = blender::Map<std::string, AbstractHierarchyWriter *>;
/* All the children of some object, as per the export hierarchy. */
using ExportChildren = std::set<HierarchyContext *>;
using ExportChildren = blender::Set<HierarchyContext *>;
/* Mapping from an object and its duplicator to the object's export-children. */
using ExportGraph = std::map<ObjectIdentifier, ExportChildren>;
using ExportGraph = blender::Map<ObjectIdentifier, ExportChildren>;
/* 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 = std::map<ID *, std::string>;
using ExportPathMap = blender::Map<ID *, std::string>;
/* IDs of all duplisource objects, used to identify instance prototypes. */
using DupliSources = std::set<ID *>;
using DupliSources = blender::Set<ID *>;
protected:
ExportGraph export_graph_;
@@ -278,7 +282,7 @@ class AbstractHierarchyIterator {
const DupliParentFinder &dupli_parent_finder);
void context_update_for_graph_index(HierarchyContext *context,
const ExportGraph::key_type &graph_index) const;
const ObjectIdentifier &graph_index) const;
void determine_export_paths(const HierarchyContext *parent_context);
bool determine_duplication_references(const HierarchyContext *parent_context,
@@ -328,8 +332,8 @@ 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(
virtual ObjectIdentifier determine_graph_index_object(const HierarchyContext *context);
virtual ObjectIdentifier determine_graph_index_dupli(
const HierarchyContext *context,
const DupliObject *dupli_object,
const DupliParentFinder &dupli_parent_finder);
@@ -364,7 +368,7 @@ class AbstractHierarchyIterator {
}
AbstractHierarchyWriter *get_writer(const std::string &export_path) const;
ExportChildren &graph_children(const HierarchyContext *context);
ExportChildren *graph_children(const HierarchyContext *context);
};
} // namespace blender::io

4
source/blender/io/common/IO_dupli_persistent_id.hh
View File

@@ -6,6 +6,7 @@
#include "BKE_duplilist.hh"
#include <array>
#include <cstdint>
#include <string>
namespace blender::io {
@@ -36,8 +37,9 @@ class PersistentID {
* "3-0", "3-1", etc. for its duplis. */
std::string as_object_name_suffix() const;
uint64_t hash() 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);
private:
void copy_values_from(const PIDArray &persistent_id_values);

8
source/blender/io/common/IO_subdiv_disabler.hh
View File

@@ -5,7 +5,7 @@
#include "DNA_modifier_types.h"
#include <set>
#include "BLI_vector.hh"
struct Depsgraph;
struct ModifierData;
@@ -28,8 +28,10 @@ namespace blender::io {
class SubdivModifierDisabler final {
private:
Depsgraph *depsgraph_;
std::set<ModifierData *> disabled_modifiers_;
std::set<Object *> modified_objects_;
/* TODO: Track the object and its disabled modifier in a single struct and use just 1 Vector. */
blender::Vector<ModifierData *> disabled_modifiers_;
blender::Vector<Object *> modified_objects_;
public:
explicit SubdivModifierDisabler(Depsgraph *depsgraph);

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

@@ -34,19 +34,6 @@ const HierarchyContext *HierarchyContext::root()
return nullptr;
}
bool HierarchyContext::operator<(const HierarchyContext &other) const
{
if (object != other.object) {
return object < other.object;
}
if (duplicator != nullptr && duplicator == other.duplicator) {
/* Only resort to string comparisons when both objects are created by the same duplicator. */
return export_name < other.export_name;
}
return export_parent < other.export_parent;
}
bool HierarchyContext::is_instance() const
{
return !original_export_path.empty();
@@ -177,7 +164,7 @@ AbstractHierarchyIterator::~AbstractHierarchyIterator()
* release_writer() function. By the time this destructor is called, the subclass that implements
* that pure-virtual function is already destructed. */
BLI_assert_msg(
writers_.empty(),
writers_.is_empty(),
"release_writers() should be called before the AbstractHierarchyIterator goes out of scope");
}
@@ -194,8 +181,8 @@ void AbstractHierarchyIterator::iterate_and_write()
void AbstractHierarchyIterator::release_writers()
{
for (WriterMap::value_type it : writers_) {
release_writer(it.second);
for (AbstractHierarchyWriter *writer : writers_.values()) {
release_writer(writer);
}
writers_.clear();
}
@@ -230,8 +217,8 @@ std::string AbstractHierarchyIterator::get_object_data_path(const HierarchyConte
void AbstractHierarchyIterator::debug_print_export_graph(const ExportGraph &graph) const
{
size_t total_graph_size = 0;
for (const ExportGraph::value_type &map_iter : graph) {
const ObjectIdentifier &parent_info = map_iter.first;
for (const auto item : graph.items()) {
const ObjectIdentifier &parent_info = item.key;
const Object *const export_parent = parent_info.object;
const Object *const duplicator = parent_info.duplicated_by;
@@ -245,8 +232,8 @@ void AbstractHierarchyIterator::debug_print_export_graph(const ExportGraph &grap
export_parent == nullptr ? "-null-" : (export_parent->id.name + 2));
}
total_graph_size += map_iter.second.size();
for (HierarchyContext *child_ctx : map_iter.second) {
total_graph_size += item.value.size();
for (HierarchyContext *child_ctx : item.value) {
if (child_ctx->duplicator == nullptr) {
fmt::println(" - {}{}{}",
child_ctx->export_name.c_str(),
@@ -275,8 +262,8 @@ void AbstractHierarchyIterator::export_graph_construct()
/* Add a "null" root node with no children immediately for the case where the top-most node in
* the scene is not being exported and a root node otherwise wouldn't get added. */
ExportGraph::key_type root_node_id = ObjectIdentifier::for_real_object(nullptr);
export_graph_[root_node_id] = ExportChildren();
ObjectIdentifier root_node_id = ObjectIdentifier::for_real_object(nullptr);
export_graph_.add_new(root_node_id, {});
DEGObjectIterSettings deg_iter_settings{};
deg_iter_settings.depsgraph = depsgraph_;
@@ -325,29 +312,27 @@ void AbstractHierarchyIterator::connect_loose_objects()
* These objects will have to be re-attached to some parent object in order to
* fit into the hierarchy. */
ExportGraph loose_objects_graph = export_graph_;
for (const ExportGraph::value_type &map_iter : export_graph_) {
for (const HierarchyContext *child : map_iter.second) {
for (const ExportChildren &children : export_graph_.values()) {
for (const HierarchyContext *child : children) {
/* An object that is marked as a child of another object is not considered 'loose'. */
ObjectIdentifier child_oid = ObjectIdentifier::for_hierarchy_context(child);
loose_objects_graph.erase(child_oid);
loose_objects_graph.remove(child_oid);
}
}
/* The root of the hierarchy is always found, so it's never considered 'loose'. */
loose_objects_graph.erase(ObjectIdentifier::for_graph_root());
loose_objects_graph.remove_contained(ObjectIdentifier::for_graph_root());
/* Iterate over the loose objects and connect them to their export parent. */
for (const ExportGraph::value_type &map_iter : loose_objects_graph) {
const ObjectIdentifier &graph_key = map_iter.first;
for (const ObjectIdentifier &graph_key : loose_objects_graph.keys()) {
Object *object = graph_key.object;
while (true) {
/* Loose objects will all be real objects, as duplicated objects always have
* their duplicator or other exported duplicated object as ancestor. */
ExportGraph::iterator found_parent_iter = export_graph_.find(
ObjectIdentifier::for_real_object(object->parent));
const bool found = export_graph_.contains(ObjectIdentifier::for_real_object(object->parent));
visit_object(object, object->parent, true);
if (found_parent_iter != export_graph_.end()) {
if (found) {
break;
}
/* 'object->parent' will never be nullptr here, as the export graph contains the
@@ -366,17 +351,15 @@ static bool remove_weak_subtrees(const HierarchyContext *context,
bool all_is_weak = context != nullptr && context->weak_export;
const ObjectIdentifier map_key = ObjectIdentifier::for_hierarchy_context(context);
AbstractHierarchyIterator::ExportGraph::const_iterator child_iterator;
child_iterator = input_graph.find(map_key);
if (child_iterator != input_graph.end()) {
for (HierarchyContext *child_context : child_iterator->second) {
const AbstractHierarchyIterator::ExportChildren *children = input_graph.lookup_ptr(map_key);
if (children) {
for (HierarchyContext *child_context : *children) {
bool child_tree_is_weak = remove_weak_subtrees(child_context, clean_graph, input_graph);
all_is_weak &= child_tree_is_weak;
if (child_tree_is_weak) {
/* This subtree is all weak, so we can remove it from the current object's children. */
clean_graph[map_key].erase(child_context);
clean_graph.lookup(map_key).remove(child_context);
delete child_context;
}
}
@@ -384,7 +367,7 @@ static bool remove_weak_subtrees(const HierarchyContext *context,
if (all_is_weak) {
/* This node and all its children are weak, so it can be removed from the export graph. */
clean_graph.erase(map_key);
clean_graph.remove(map_key);
}
return all_is_weak;
@@ -399,8 +382,8 @@ void AbstractHierarchyIterator::export_graph_prune()
void AbstractHierarchyIterator::export_graph_clear()
{
for (ExportGraph::iterator::value_type &it : export_graph_) {
for (HierarchyContext *context : it.second) {
for (const ExportChildren &children : export_graph_.values()) {
for (HierarchyContext *context : children) {
delete context;
}
}
@@ -426,25 +409,23 @@ void AbstractHierarchyIterator::visit_object(Object *object,
copy_m4_m4(context->matrix_world, object->object_to_world().ptr());
ExportGraph::key_type graph_index = determine_graph_index_object(context);
ObjectIdentifier graph_index = determine_graph_index_object(context);
context_update_for_graph_index(context, graph_index);
/* Store this HierarchyContext as child of the export parent. */
export_graph_[graph_index].insert(context);
export_graph_.lookup_or_add(graph_index, {}).add_new(context);
/* Create an empty entry for this object to indicate it is part of the export. This will be used
* by connect_loose_objects(). Having such an "indicator" will make it possible to do an O(log n)
* check on whether an object is part of the export, rather than having to check all objects in
* the map. Note that it's not possible to simply search for (object->parent, nullptr), as the
* object's parent in Blender may not be the same as its export-parent. */
ExportGraph::key_type object_key = ObjectIdentifier::for_real_object(object);
if (export_graph_.find(object_key) == export_graph_.end()) {
export_graph_[object_key] = ExportChildren();
}
ObjectIdentifier object_key = ObjectIdentifier::for_real_object(object);
export_graph_.add(object_key, {});
}
AbstractHierarchyIterator::ExportGraph::key_type AbstractHierarchyIterator::
determine_graph_index_object(const HierarchyContext *context)
ObjectIdentifier AbstractHierarchyIterator::determine_graph_index_object(
const HierarchyContext *context)
{
return ObjectIdentifier::for_real_object(context->export_parent);
}
@@ -471,21 +452,21 @@ void AbstractHierarchyIterator::visit_dupli_object(DupliObject *dupli_object,
context->persistent_id.as_object_name_suffix();
context->export_name = make_valid_name(export_name);
ExportGraph::key_type graph_index = determine_graph_index_dupli(
ObjectIdentifier graph_index = determine_graph_index_dupli(
context, dupli_object, dupli_parent_finder);
context_update_for_graph_index(context, graph_index);
export_graph_[graph_index].insert(context);
export_graph_.lookup_or_add(graph_index, {}).add_new(context);
if (dupli_object->ob) {
this->duplisources_.insert(&dupli_object->ob->id);
this->duplisources_.add(&dupli_object->ob->id);
}
}
AbstractHierarchyIterator::ExportGraph::key_type AbstractHierarchyIterator::
determine_graph_index_dupli(const HierarchyContext *context,
const DupliObject *dupli_object,
const DupliParentFinder &dupli_parent_finder)
ObjectIdentifier AbstractHierarchyIterator::determine_graph_index_dupli(
const HierarchyContext *context,
const DupliObject *dupli_object,
const DupliParentFinder &dupli_parent_finder)
{
const DupliObject *dupli_parent = dupli_parent_finder.find_suitable_export_parent(dupli_object);
@@ -496,7 +477,7 @@ AbstractHierarchyIterator::ExportGraph::key_type AbstractHierarchyIterator::
}
void AbstractHierarchyIterator::context_update_for_graph_index(
HierarchyContext *context, const ExportGraph::key_type &graph_index) const
HierarchyContext *context, const ObjectIdentifier &graph_index) const
{
/* Update the HierarchyContext so that it is consistent with the graph index. */
context->export_parent = graph_index.object;
@@ -514,28 +495,36 @@ void AbstractHierarchyIterator::context_update_for_graph_index(
}
}
AbstractHierarchyIterator::ExportChildren &AbstractHierarchyIterator::graph_children(
AbstractHierarchyIterator::ExportChildren *AbstractHierarchyIterator::graph_children(
const HierarchyContext *context)
{
return export_graph_[ObjectIdentifier::for_hierarchy_context(context)];
/* Note: `graph_children` is called during recursive iteration and MUST NOT change the export
* graph, which would invalidate the iteration. As a result, we cannot add an entry in the
* graph if the incoming `context` is not found. */
return export_graph_.lookup_ptr(ObjectIdentifier::for_hierarchy_context(context));
}
void AbstractHierarchyIterator::determine_export_paths(const HierarchyContext *parent_context)
{
const std::string &parent_export_path = parent_context ? parent_context->export_path : "";
for (HierarchyContext *context : graph_children(parent_context)) {
const ExportChildren *children = graph_children(parent_context);
if (!children) {
return;
}
for (HierarchyContext *context : *children) {
context->export_path = path_concatenate(parent_export_path, context->export_name);
if (context->duplicator == nullptr) {
/* This is an original (i.e. non-instanced) object, so we should keep track of where it was
* exported to, just in case it gets instanced somewhere. */
ID *source_ob = &context->object->id;
duplisource_export_path_[source_ob] = context->export_path;
duplisource_export_path_.add(source_ob, context->export_path);
if (context->object->data != nullptr) {
ID *source_data = static_cast<ID *>(context->object->data);
duplisource_export_path_[source_data] = get_object_data_path(context);
duplisource_export_path_.add(source_data, get_object_data_path(context));
}
}
@@ -546,44 +535,44 @@ void AbstractHierarchyIterator::determine_export_paths(const HierarchyContext *p
bool AbstractHierarchyIterator::determine_duplication_references(
const HierarchyContext *parent_context, const std::string &indent)
{
ExportChildren children = graph_children(parent_context);
const ExportChildren *children = graph_children(parent_context);
if (!children) {
return false;
}
/* 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) {
for (HierarchyContext *context : *children) {
if (context->duplicator != nullptr) {
ID *source_id = &context->object->id;
const ExportPathMap::const_iterator &it = duplisource_export_path_.find(source_id);
if (it == duplisource_export_path_.end()) {
const std::string *source_path = duplisource_export_path_.lookup_ptr(source_id);
if (!source_path) {
/* 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;
duplisource_export_path_.add_new(source_id, context->export_path);
contains_proxy_prototype = true;
}
else {
context->mark_as_instance_of(it->second);
context->mark_as_instance_of(*source_path);
}
if (context->object->data) {
ID *source_data_id = (ID *)context->object->data;
const ExportPathMap::const_iterator &it = duplisource_export_path_.find(source_data_id);
if (it == duplisource_export_path_.end()) {
if (!duplisource_export_path_.contains(source_data_id)) {
/* The original was not found, so mark this instance as "original". */
std::string data_path = get_object_data_path(context);
context->mark_as_not_instanced();
duplisource_export_path_[source_id] = context->export_path;
duplisource_export_path_[source_data_id] = data_path;
duplisource_export_path_.add_overwrite(source_id, context->export_path);
duplisource_export_path_.add_new(source_data_id, data_path);
}
}
}
else {
/* Determine is this context is for an instance prototype. */
ID *id = &context->object->id;
if (duplisources_.find(id) != duplisources_.end()) {
if (duplisources_.contains(id)) {
context->is_duplisource = true;
}
}
@@ -594,7 +583,7 @@ bool AbstractHierarchyIterator::determine_duplication_references(
if (context->is_instance()) {
context->mark_as_not_instanced();
ID *source_id = &context->object->id;
duplisource_export_path_[source_id] = context->export_path;
duplisource_export_path_.add_overwrite(source_id, context->export_path);
}
contains_proxy_prototype = true;
}
@@ -613,7 +602,12 @@ void AbstractHierarchyIterator::make_writers(const HierarchyContext *parent_cont
unit_m4(parent_matrix_inv_world);
}
for (HierarchyContext *context : graph_children(parent_context)) {
const ExportChildren *children = graph_children(parent_context);
if (!children) {
return;
}
for (HierarchyContext *context : *children) {
/* Update the context so that it is correct for this parent-child relation. */
copy_m4_m4(context->parent_matrix_inv_world, parent_matrix_inv_world);
if (parent_context != nullptr) {
@@ -663,9 +657,9 @@ HierarchyContext AbstractHierarchyIterator::context_for_object_data(
data_context.export_path = path_concatenate(data_context.higher_up_export_path,
data_context.export_name);
ExportGraph::key_type object_key = ObjectIdentifier::for_hierarchy_context(&data_context);
ExportGraph::const_iterator iter = export_graph_.find(object_key);
data_context.is_parent = iter != export_graph_.end() ? (iter->second.size() > 0) : false;
const ObjectIdentifier object_key = ObjectIdentifier::for_hierarchy_context(&data_context);
const ExportChildren *children = export_graph_.lookup_ptr(object_key);
data_context.is_parent = children ? (children->size() > 0) : false;
return data_context;
}
@@ -679,7 +673,7 @@ void AbstractHierarchyIterator::make_writer_object_data(const HierarchyContext *
HierarchyContext data_context = context_for_object_data(context);
if (data_context.is_instance()) {
ID *object_data = static_cast<ID *>(context->object->data);
data_context.original_export_path = duplisource_export_path_[object_data];
data_context.original_export_path = duplisource_export_path_.lookup(object_data);
/* If the object is marked as an instance, so should the object data. */
BLI_assert(data_context.is_instance());
@@ -757,12 +751,7 @@ std::string AbstractHierarchyIterator::get_object_data_name(const Object *object
AbstractHierarchyWriter *AbstractHierarchyIterator::get_writer(
const std::string &export_path) const
{
WriterMap::const_iterator it = writers_.find(export_path);
if (it == writers_.end()) {
return nullptr;
}
return it->second;
return writers_.lookup_default(export_path, nullptr);
}
EnsuredWriter AbstractHierarchyIterator::ensure_writer(
@@ -778,7 +767,7 @@ EnsuredWriter AbstractHierarchyIterator::ensure_writer(
return EnsuredWriter::empty();
}
writers_[context->export_path] = writer;
writers_.add_new(context->export_path, writer);
return EnsuredWriter::newly_created(writer);
}

13
source/blender/io/common/intern/abstract_hierarchy_iterator_test.cc
View File

@@ -6,22 +6,21 @@
#include "tests/blendfile_loading_base_test.h"
#include "BKE_scene.hh"
#include "BLI_map.hh"
#include "BLI_path_utils.hh"
#include "BLI_set.hh"
#include "BLO_readfile.hh"
#include "DEG_depsgraph.hh"
#include "DEG_depsgraph_build.hh"
#include "DNA_object_types.h"
#include <map>
#include <set>
namespace blender::io {
namespace {
/* Mapping from ID.name to set of export hierarchy path. Duplicated objects can be exported
* multiple times with different export paths, hence the set. */
using used_writers = std::map<std::string, std::set<std::string>>;
using used_writers = blender::Map<std::string, blender::Set<std::string>>;
class TestHierarchyWriter : public AbstractHierarchyWriter {
public:
@@ -36,13 +35,13 @@ class TestHierarchyWriter : public AbstractHierarchyWriter {
void write(HierarchyContext &context) override
{
const char *id_name = context.object->id.name;
used_writers::mapped_type &writers = writers_map[id_name];
Set<std::string> &writers = writers_map.lookup_or_add(id_name, {});
if (writers.find(context.export_path) != writers.end()) {
if (writers.contains(context.export_path)) {
ADD_FAILURE() << "Unexpectedly found another " << writer_type << " writer for " << id_name
<< " to export to " << context.export_path;
}
writers.insert(context.export_path);
writers.add_new(context.export_path);
}
};

20
source/blender/io/common/intern/dupli_parent_finder.cc
View File

@@ -12,16 +12,16 @@ namespace blender::io {
void DupliParentFinder::insert(const DupliObject *dupli_ob)
{
dupli_set_.insert(dupli_ob->ob);
dupli_set_.add(dupli_ob->ob);
PersistentID dupli_pid(dupli_ob);
pid_to_dupli_[dupli_pid] = dupli_ob;
instancer_pid_to_duplis_[dupli_pid.instancer_pid()].insert(dupli_ob);
pid_to_dupli_.add_new(dupli_pid, dupli_ob);
instancer_pid_to_duplis_.lookup_or_add(dupli_pid.instancer_pid(), {}).add(dupli_ob);
}
bool DupliParentFinder::is_duplicated(const Object *object) const
{
return dupli_set_.find(object) != dupli_set_.end();
return dupli_set_.contains(object);
}
const DupliObject *DupliParentFinder::find_suitable_export_parent(
@@ -45,13 +45,13 @@ const DupliObject *DupliParentFinder::find_duplicated_parent(const DupliObject *
const Object *parent_ob = dupli_ob->ob->parent;
BLI_assert(parent_ob != nullptr);
InstancerPIDToDuplisMap::const_iterator found = instancer_pid_to_duplis_.find(parent_pid);
if (found == instancer_pid_to_duplis_.end()) {
const blender::Set<const DupliObject *> *found = instancer_pid_to_duplis_.lookup_ptr(parent_pid);
if (!found) {
/* Unexpected, as there should be at least one entry here, for the dupli_ob itself. */
return nullptr;
}
for (const DupliObject *potential_parent_dupli : found->second) {
for (const DupliObject *potential_parent_dupli : *found) {
if (potential_parent_dupli->ob != parent_ob) {
continue;
}
@@ -69,12 +69,12 @@ const DupliObject *DupliParentFinder::find_instancer(const DupliObject *dupli_ob
PersistentID dupli_pid(dupli_ob);
PersistentID parent_pid = dupli_pid.instancer_pid();
PIDToDupliMap::const_iterator found = pid_to_dupli_.find(parent_pid);
if (found == pid_to_dupli_.end()) {
const DupliObject *const *found = pid_to_dupli_.lookup_ptr(parent_pid);
if (!found) {
return nullptr;
}
const DupliObject *instancer_dupli = found->second;
const DupliObject *instancer_dupli = *found;
return instancer_dupli;
}

11
source/blender/io/common/intern/dupli_parent_finder.hh
View File

@@ -7,8 +7,8 @@
#include "BKE_duplilist.hh"
#include <map>
#include <set>
#include "BLI_map.hh"
#include "BLI_set.hh"
namespace blender::io {
@@ -17,14 +17,15 @@ namespace blender::io {
class DupliParentFinder final {
private:
/* To check whether an Object * is instanced by this duplicator. */
std::set<const Object *> dupli_set_;
blender::Set<const Object *> dupli_set_;
/* To find the DupliObject given its Persistent ID. */
using PIDToDupliMap = std::map<const PersistentID, const DupliObject *>;
using PIDToDupliMap = blender::Map<const PersistentID, const DupliObject *>;
PIDToDupliMap pid_to_dupli_;
/* Mapping from instancer PID to duplis instanced by it. */
using InstancerPIDToDuplisMap = std::map<const PersistentID, std::set<const DupliObject *>>;
using InstancerPIDToDuplisMap =
blender::Map<const PersistentID, blender::Set<const DupliObject *>>;
InstancerPIDToDuplisMap instancer_pid_to_duplis_;
public:

21
source/blender/io/common/intern/dupli_persistent_id.cc
View File

@@ -91,25 +91,16 @@ std::string PersistentID::as_object_name_suffix() const
return fmt::to_string(buf);
}
bool operator<(const PersistentID &persistent_id_a, const PersistentID &persistent_id_b)
uint64_t PersistentID::hash() const
{
const PersistentID::PIDArray &pid_a = persistent_id_a.persistent_id_;
const PersistentID::PIDArray &pid_b = persistent_id_b.persistent_id_;
for (int index = 0; index < PersistentID::array_length_; ++index) {
const int pid_digit_a = pid_a[index];
const int pid_digit_b = pid_b[index];
if (pid_digit_a != pid_digit_b) {
return pid_digit_a < pid_digit_b;
}
if (pid_a[index] == INT_MAX) {
uint64_t hash = 5381;
for (const int value : persistent_id_) {
if (value == INT_MAX) {
break;
}
hash = hash * 33 ^ uint64_t(value);
}
/* Both Persistent IDs were equal, so not less-than. */
return false;
return hash;
}
bool operator==(const PersistentID &persistent_id_a, const PersistentID &persistent_id_b)

111
source/blender/io/common/intern/hierarchy_context_order_test.cc
View File

@@ -1,111 +0,0 @@
/* SPDX-FileCopyrightText: 2019 Blender Authors
*
* SPDX-License-Identifier: GPL-2.0-or-later */
#include "IO_abstract_hierarchy_iterator.h"
#include "testing/testing.h"
#include "BLI_utildefines.h"
namespace blender::io {
namespace {
Object *fake_pointer(int value)
{
return static_cast<Object *>(POINTER_FROM_INT(value));
}
} // namespace
class HierarchyContextOrderTest : public testing::Test {};
TEST_F(HierarchyContextOrderTest, ObjectPointerTest)
{
HierarchyContext ctx_a = {nullptr};
ctx_a.object = fake_pointer(1);
ctx_a.duplicator = nullptr;
HierarchyContext ctx_b = {nullptr};
ctx_b.object = fake_pointer(2);
ctx_b.duplicator = nullptr;
EXPECT_LT(ctx_a, ctx_b);
EXPECT_FALSE(ctx_b < ctx_a);
EXPECT_FALSE(ctx_a < ctx_a);
}
TEST_F(HierarchyContextOrderTest, DuplicatorPointerTest)
{
HierarchyContext ctx_a = {nullptr};
ctx_a.object = fake_pointer(1);
ctx_a.duplicator = fake_pointer(1);
ctx_a.export_name = "A";
HierarchyContext ctx_b = {nullptr};
ctx_b.object = fake_pointer(1);
ctx_b.duplicator = fake_pointer(1);
ctx_b.export_name = "B";
EXPECT_LT(ctx_a, ctx_b);
EXPECT_FALSE(ctx_b < ctx_a);
EXPECT_FALSE(ctx_a < ctx_a);
}
TEST_F(HierarchyContextOrderTest, ExportParentTest)
{
HierarchyContext ctx_a = {nullptr};
ctx_a.object = fake_pointer(1);
ctx_a.export_parent = fake_pointer(1);
HierarchyContext ctx_b = {nullptr};
ctx_b.object = fake_pointer(1);
ctx_b.export_parent = fake_pointer(2);
EXPECT_LT(ctx_a, ctx_b);
EXPECT_FALSE(ctx_b < ctx_a);
EXPECT_FALSE(ctx_a < ctx_a);
}
TEST_F(HierarchyContextOrderTest, TransitiveTest)
{
HierarchyContext ctx_a = {nullptr};
ctx_a.object = fake_pointer(1);
ctx_a.export_parent = fake_pointer(1);
ctx_a.duplicator = nullptr;
ctx_a.export_name = "A";
HierarchyContext ctx_b = {nullptr};
ctx_b.object = fake_pointer(2);
ctx_b.export_parent = nullptr;
ctx_b.duplicator = fake_pointer(1);
ctx_b.export_name = "B";
HierarchyContext ctx_c = {nullptr};
ctx_c.object = fake_pointer(2);
ctx_c.export_parent = fake_pointer(2);
ctx_c.duplicator = fake_pointer(1);
ctx_c.export_name = "C";
HierarchyContext ctx_d = {nullptr};
ctx_d.object = fake_pointer(2);
ctx_d.export_parent = fake_pointer(3);
ctx_d.duplicator = nullptr;
ctx_d.export_name = "D";
EXPECT_LT(ctx_a, ctx_b);
EXPECT_LT(ctx_a, ctx_c);
EXPECT_LT(ctx_a, ctx_d);
EXPECT_LT(ctx_b, ctx_c);
EXPECT_LT(ctx_b, ctx_d);
EXPECT_LT(ctx_c, ctx_d);
EXPECT_FALSE(ctx_b < ctx_a);
EXPECT_FALSE(ctx_c < ctx_a);
EXPECT_FALSE(ctx_d < ctx_a);
EXPECT_FALSE(ctx_c < ctx_b);
EXPECT_FALSE(ctx_d < ctx_b);
EXPECT_FALSE(ctx_d < ctx_c);
}
} // namespace blender::io

28
source/blender/io/common/intern/object_identifier.cc
View File

@@ -3,6 +3,8 @@
* SPDX-License-Identifier: GPL-2.0-or-later */
#include "IO_abstract_hierarchy_iterator.h"
#include "BLI_assert.h"
#include "BKE_duplilist.hh"
namespace blender::io {
@@ -12,6 +14,11 @@ ObjectIdentifier::ObjectIdentifier(Object *object,
const PersistentID &persistent_id)
: object(object), duplicated_by(duplicated_by), persistent_id(persistent_id)
{
/* Class invariants:
* If duplicated_by is null, persistent_id must be default.
* If duplicated_by is not null, persistent_id must not be default. */
BLI_assert(duplicated_by == nullptr ? persistent_id == PersistentID() :
!(persistent_id == PersistentID()));
}
ObjectIdentifier ObjectIdentifier::for_real_object(Object *object)
@@ -46,25 +53,6 @@ bool ObjectIdentifier::is_root() const
return object == nullptr;
}
bool operator<(const ObjectIdentifier &obj_ident_a, const ObjectIdentifier &obj_ident_b)
{
if (obj_ident_a.object != obj_ident_b.object) {
return obj_ident_a.object < obj_ident_b.object;
}
if (obj_ident_a.duplicated_by != obj_ident_b.duplicated_by) {
return obj_ident_a.duplicated_by < obj_ident_b.duplicated_by;
}
if (obj_ident_a.duplicated_by == nullptr) {
/* Both are real objects, no need to check the persistent ID. */
return false;
}
/* Same object, both are duplicated, use the persistent IDs to determine order. */
return obj_ident_a.persistent_id < obj_ident_b.persistent_id;
}
bool operator==(const ObjectIdentifier &obj_ident_a, const ObjectIdentifier &obj_ident_b)
{
if (obj_ident_a.object != obj_ident_b.object) {
@@ -73,6 +61,8 @@ bool operator==(const ObjectIdentifier &obj_ident_a, const ObjectIdentifier &obj
if (obj_ident_a.duplicated_by != obj_ident_b.duplicated_by) {
return false;
}
/* Return early if we know the expensive persistent_id check won't be necessary. */
if (obj_ident_a.duplicated_by == nullptr) {
return true;
}

58
source/blender/io/common/intern/object_identifier_test.cc
View File

@@ -67,18 +67,14 @@ TEST_F(ObjectIdentifierOrderTest, graph_root)
ObjectIdentifier id_root_1 = ObjectIdentifier::for_graph_root();
ObjectIdentifier id_root_2 = ObjectIdentifier::for_graph_root();
EXPECT_TRUE(id_root_1 == id_root_2);
EXPECT_FALSE(id_root_1 < id_root_2);
EXPECT_FALSE(id_root_2 < id_root_1);
EXPECT_TRUE(id_root_1.hash() == id_root_2.hash());
ObjectIdentifier id_a = ObjectIdentifier::for_real_object(fake_pointer(1));
EXPECT_FALSE(id_root_1 == id_a);
EXPECT_TRUE(id_root_1 < id_a);
EXPECT_FALSE(id_a < id_root_1);
ObjectIdentifier id_accidental_root = ObjectIdentifier::for_real_object(nullptr);
EXPECT_TRUE(id_root_1 == id_accidental_root);
EXPECT_FALSE(id_root_1 < id_accidental_root);
EXPECT_FALSE(id_accidental_root < id_root_1);
EXPECT_TRUE(id_root_1.hash() == id_accidental_root.hash());
}
TEST_F(ObjectIdentifierOrderTest, real_objects)
@@ -86,7 +82,10 @@ TEST_F(ObjectIdentifierOrderTest, real_objects)
ObjectIdentifier id_a = ObjectIdentifier::for_real_object(fake_pointer(1));
ObjectIdentifier id_b = ObjectIdentifier::for_real_object(fake_pointer(2));
EXPECT_FALSE(id_a == id_b);
EXPECT_TRUE(id_a < id_b);
ObjectIdentifier id_c = ObjectIdentifier::for_real_object(fake_pointer(1));
EXPECT_TRUE(id_a == id_c);
EXPECT_TRUE(id_a.hash() == id_c.hash());
}
TEST_F(ObjectIdentifierOrderTest, duplicated_objects)
@@ -94,19 +93,17 @@ TEST_F(ObjectIdentifierOrderTest, duplicated_objects)
ObjectIdentifier id_real_a = ObjectIdentifier::for_real_object(fake_pointer(1));
TestObjectIdentifier id_dupli_a(fake_pointer(1), fake_pointer(2), TestPersistentID(0));
TestObjectIdentifier id_dupli_b(fake_pointer(1), fake_pointer(3), TestPersistentID(0));
TestObjectIdentifier id_same_dupli_a(fake_pointer(1), fake_pointer(2), TestPersistentID(0));
TestObjectIdentifier id_different_dupli_b(fake_pointer(1), fake_pointer(3), TestPersistentID(1));
EXPECT_FALSE(id_real_a == id_dupli_a);
EXPECT_FALSE(id_dupli_a == id_dupli_b);
EXPECT_TRUE(id_real_a < id_dupli_a);
EXPECT_TRUE(id_real_a < id_dupli_b);
EXPECT_TRUE(id_dupli_a < id_dupli_b);
EXPECT_TRUE(id_dupli_a < id_different_dupli_b);
EXPECT_FALSE(id_dupli_b == id_different_dupli_b);
EXPECT_FALSE(id_dupli_a == id_different_dupli_b);
EXPECT_TRUE(id_dupli_b < id_different_dupli_b);
EXPECT_FALSE(id_different_dupli_b < id_dupli_b);
EXPECT_TRUE(id_dupli_a == id_same_dupli_a);
EXPECT_TRUE(id_dupli_a.hash() == id_same_dupli_a.hash());
}
TEST_F(ObjectIdentifierOrderTest, behavior_as_map_keys)
@@ -119,19 +116,19 @@ TEST_F(ObjectIdentifierOrderTest, behavior_as_map_keys)
AbstractHierarchyIterator::ExportGraph graph;
/* This inserts the keys with default values. */
graph[id_root];
graph[id_real_a];
graph[id_dupli_a];
graph[id_dupli_b];
graph[id_another_root];
graph.add_new(id_root, {});
graph.add_new(id_real_a, {});
graph.add_new(id_dupli_a, {});
graph.add_new(id_dupli_b, {});
graph.add(id_another_root, {});
EXPECT_EQ(4, graph.size());
graph.erase(id_another_root);
graph.remove_contained(id_another_root);
EXPECT_EQ(3, graph.size());
TestObjectIdentifier id_another_dupli_b(fake_pointer(1), fake_pointer(3), TestPersistentID(0));
graph.erase(id_another_dupli_b);
graph.remove_contained(id_another_dupli_b);
EXPECT_EQ(2, graph.size());
}
@@ -145,11 +142,11 @@ TEST_F(ObjectIdentifierOrderTest, map_copy_and_update)
AbstractHierarchyIterator::ExportGraph graph;
/* This inserts the keys with default values. */
graph[id_root];
graph[id_real_a];
graph[id_dupli_a];
graph[id_dupli_b];
graph[id_dupli_c];
graph.add_new(id_root, {});
graph.add_new(id_real_a, {});
graph.add_new(id_dupli_a, {});
graph.add_new(id_dupli_b, {});
graph.add_new(id_dupli_c, {});
EXPECT_EQ(5, graph.size());
AbstractHierarchyIterator::ExportGraph graph_copy = graph;
@@ -161,11 +158,11 @@ TEST_F(ObjectIdentifierOrderTest, map_copy_and_update)
ctx1.object = fake_pointer(1);
ctx2.object = fake_pointer(2);
graph_copy[id_root].insert(&ctx1);
EXPECT_EQ(0, graph[id_root].size());
graph_copy.lookup(id_root).add_new(&ctx1);
EXPECT_EQ(0, graph.lookup(id_root).size());
/* Deleting a key in the copy should not update the original. */
graph_copy.erase(id_dupli_c);
graph_copy.remove_contained(id_dupli_c);
EXPECT_EQ(4, graph_copy.size());
EXPECT_EQ(5, graph.size());
}
@@ -188,12 +185,11 @@ TEST_F(PersistentIDTest, instancer_id)
PersistentID expect_instancer_id = TestPersistentID(327);
EXPECT_EQ(expect_instancer_id, child_id.instancer_pid());
EXPECT_EQ(expect_instancer_id.hash(), child_id.instancer_pid().hash());
PersistentID empty_id;
EXPECT_EQ(empty_id, child_id.instancer_pid().instancer_pid());
EXPECT_LT(child_id, expect_instancer_id);
EXPECT_LT(expect_instancer_id, empty_id);
EXPECT_EQ(empty_id.hash(), child_id.instancer_pid().instancer_pid().hash());
}
TEST_F(PersistentIDTest, as_object_name_suffix)

4
source/blender/io/common/intern/subdiv_disabler.cc
View File

@@ -103,7 +103,7 @@ void SubdivModifierDisabler::disable_modifiers()
* moving to a different frame is also going to be faster, so in the end this is probably
* a good thing to do. */
disable_modifier(mod);
modified_objects_.insert(object);
modified_objects_.append(object);
DEG_id_tag_update(&object->id, ID_RECALC_GEOMETRY);
}
}
@@ -111,7 +111,7 @@ void SubdivModifierDisabler::disable_modifiers()
void SubdivModifierDisabler::disable_modifier(ModifierData *mod)
{
mod->mode |= eModifierMode_DisableTemporary;
disabled_modifiers_.insert(mod);
disabled_modifiers_.append(mod);
}
} // namespace blender::io