From 44c868caed494506dcbb1ec3a2437bf816e31569 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Thu, 12 Aug 2021 13:35:45 +0200 Subject: [PATCH] #1531 Breadth-first search and maintain insertion order on remove_deep() --- src/ifcopenshell-python/ifcopenshell/file.py | 12 ++- .../ifcopenshell/util/element.py | 2 +- src/ifcparse/IfcFile.h | 19 ++++- src/ifcparse/IfcParse.cpp | 73 ++++++++++++++++--- src/ifcparse/IfcParse.h | 2 + 5 files changed, 93 insertions(+), 15 deletions(-) diff --git a/src/ifcopenshell-python/ifcopenshell/file.py b/src/ifcopenshell-python/ifcopenshell/file.py index e4935a611d..a43157c105 100644 --- a/src/ifcopenshell-python/ifcopenshell/file.py +++ b/src/ifcopenshell-python/ifcopenshell/file.py @@ -359,19 +359,27 @@ class file(object): return [entity_instance(e, self) for e in self.wrapped_data.by_type(type)] return [entity_instance(e, self) for e in self.wrapped_data.by_type_excl_subtypes(type)] - def traverse(self, inst, max_levels=None): + def traverse(self, inst, max_levels=None, breadth_first=False): """Get a list of all referenced instances for a particular instance including itself :param inst: The entity instance to get all sub instances :type inst: ifcopenshell.entity_instance.entity_instance :param max_levels: How far deep to recursively fetch sub instances. None or -1 means infinite. :type max_levels: None|int + :param breadth_first: Whether to use breadth-first search, the default is depth-first. + :type max_levels: bool :returns: A list of ifcopenshell.entity_instance.entity_instance objects :rtype: list """ if max_levels is None: max_levels = -1 - return [entity_instance(e, self) for e in self.wrapped_data.traverse(inst.wrapped_data, max_levels)] + + if breadth_first: + fn = self.wrapped_data.traverse_breadth_first + else: + fn = self.wrapped_data.traverse + + return [entity_instance(e, self) for e in fn(inst.wrapped_data, max_levels)] def get_inverse(self, inst): """Return a list of entities that reference this entity diff --git a/src/ifcopenshell-python/ifcopenshell/util/element.py b/src/ifcopenshell-python/ifcopenshell/util/element.py index c7d13cf8fb..5404494b16 100644 --- a/src/ifcopenshell-python/ifcopenshell/util/element.py +++ b/src/ifcopenshell-python/ifcopenshell/util/element.py @@ -104,7 +104,7 @@ def has_element_reference(value, element): def remove_deep(ifc_file, element): # @todo maybe some sort of try-finally mechanism. ifc_file.batch() - subgraph = list(ifc_file.traverse(element)) + subgraph = list(ifc_file.traverse(element, breadth_first=True)) subgraph_set = set(subgraph) for ref in subgraph[::-1]: if ref.id() and len(set(ifc_file.get_inverse(ref)) - subgraph_set) == 0: diff --git a/src/ifcparse/IfcFile.h b/src/ifcparse/IfcFile.h index f22a042e1b..50c6897b75 100644 --- a/src/ifcparse/IfcFile.h +++ b/src/ifcparse/IfcFile.h @@ -24,6 +24,10 @@ #include #include #include +#include +#include +#include +#include #include "ifc_parse_api.h" @@ -132,7 +136,16 @@ private: void build_inverses_(IfcUtil::IfcBaseClass*); - std::set batch_deletion_ids_; + typedef boost::multi_index_container< + int, + boost::multi_index::indexed_by< + boost::multi_index::sequenced<>, + boost::multi_index::ordered_unique< + boost::multi_index::identity + > + > + > batch_deletion_ids_t; + batch_deletion_ids_t batch_deletion_ids_; bool batch_mode_ = false; void process_deletion_(); @@ -220,6 +233,10 @@ public: /// in the first function argument. IfcEntityList::ptr traverse(IfcUtil::IfcBaseClass* instance, int max_level=-1); + /// Same as traverse() but maintains topological order by using a + /// breadth-first search + IfcEntityList::ptr traverse_breadth_first(IfcUtil::IfcBaseClass* instance, int max_level = -1); + IfcEntityList::ptr getInverse(int instance_id, const IfcParse::declaration* type, int attribute_index); /// Marks entity as modified so that potential cache for it is invalidated. diff --git a/src/ifcparse/IfcParse.cpp b/src/ifcparse/IfcParse.cpp index 833d3d6273..f9b873ec97 100644 --- a/src/ifcparse/IfcParse.cpp +++ b/src/ifcparse/IfcParse.cpp @@ -1506,15 +1506,52 @@ void IfcFile::recalculate_id_counter() { MaxId = (unsigned int)k; } +class traversal_recorder { + IfcEntityList::ptr list_; + std::map instances_by_level_; + int mode_; + +public: + traversal_recorder(int mode) : mode_(mode) { + if (mode == 0) { + list_.reset(new IfcEntityList); + } + }; + + void push_back(int level, IfcUtil::IfcBaseClass* instance) { + if (mode_ == 0) { + list_->push(instance); + } else { + auto& l = instances_by_level_[level]; + if (!l) { + l.reset(new IfcEntityList); + } + l->push(instance); + } + } + + IfcEntityList::ptr get_list() const { + if (mode_ == 0) { + return list_; + } else { + IfcEntityList::ptr l(new IfcEntityList); + for (auto& p : instances_by_level_) { + l->push(p.second); + } + return l; + } + } +}; + class traversal_visitor { private: std::set& visited_; - IfcEntityList::ptr& list_; + traversal_recorder& list_; int level_; int max_level_; public: - traversal_visitor(std::set& visited, IfcEntityList::ptr& list, int level, int max_level) + traversal_visitor(std::set& visited, traversal_recorder& list, int level, int max_level) : visited_(visited) , list_(list) , level_(level) @@ -1524,12 +1561,12 @@ public: void operator()(IfcUtil::IfcBaseClass* inst); }; -void traverse_(IfcUtil::IfcBaseClass* instance, std::set& visited, IfcEntityList::ptr list, int level, int max_level) { +void traverse_(IfcUtil::IfcBaseClass* instance, std::set& visited, traversal_recorder& list, int level, int max_level) { if (visited.find(instance) != visited.end()) { return; } visited.insert(instance); - list->push(instance); + list.push_back(level, instance); if (level >= max_level && max_level > 0) return; @@ -1543,9 +1580,18 @@ void traversal_visitor::operator()(IfcUtil::IfcBaseClass* inst) { IfcEntityList::ptr IfcParse::traverse(IfcUtil::IfcBaseClass* instance, int max_level) { std::set visited; - IfcEntityList::ptr return_value(new IfcEntityList); - traverse_(instance, visited, return_value, 0, max_level); - return return_value; + traversal_recorder r(0); + traverse_(instance, visited, r, 0, max_level); + return r.get_list(); +} + +// I'm cheating this isn't breadth-first, but rather we record visited instances +// keeping track of their rank and return a list ordered by rank. Is this equivalent? +IfcEntityList::ptr IfcParse::traverse_breadth_first(IfcUtil::IfcBaseClass* instance, int max_level) { + std::set visited; + traversal_recorder r(1); + traverse_(instance, visited, r, 0, max_level); + return r.get_list(); } /// @note: for backwards compatibility @@ -1553,6 +1599,11 @@ IfcEntityList::ptr IfcFile::traverse(IfcUtil::IfcBaseClass* instance, int max_le return IfcParse::traverse(instance, max_level); } +/// @note: for backwards compatibility +IfcEntityList::ptr IfcFile::traverse_breadth_first(IfcUtil::IfcBaseClass* instance, int max_level) { + return IfcParse::traverse_breadth_first(instance, max_level); +} + void IfcFile::mark_entity_as_modified(int /*id*/) { by_ref_cached_.clear(); @@ -1825,7 +1876,7 @@ void IfcFile::removeEntity(IfcUtil::IfcBaseClass* entity) { throw IfcParse::IfcException("Instance not part of this file"); } - batch_deletion_ids_.insert(id); + batch_deletion_ids_.push_back(id); if (!batch_mode_) { process_deletion_(); @@ -1834,7 +1885,7 @@ void IfcFile::removeEntity(IfcUtil::IfcBaseClass* entity) { void IfcFile::process_deletion_() { - for (auto& id : batch_deletion_ids_) { + for (auto& id : batch_deletion_ids_.get<0>()) { auto entity = instance_by_id(id); IfcEntityList::ptr references = instances_by_reference(id); @@ -1980,10 +2031,10 @@ void IfcFile::process_deletion_() { if (batch_mode_) { for (auto it = byref.begin(); it != byref.end();) { - bool do_delete = batch_deletion_ids_.find(it->first) != batch_deletion_ids_.end(); + bool do_delete = batch_deletion_ids_.get<1>().find(it->first) != batch_deletion_ids_.get<1>().end(); if (!do_delete) { it->second.erase(std::remove_if(it->second.begin(), it->second.end(), [this](int x) { - return batch_deletion_ids_.find(x) != batch_deletion_ids_.end(); + return batch_deletion_ids_.get<1>().find(x) != batch_deletion_ids_.get<1>().end(); }), it->second.end()); do_delete = it->second.empty(); } diff --git a/src/ifcparse/IfcParse.h b/src/ifcparse/IfcParse.h index 0403cd56fb..b6cc855861 100644 --- a/src/ifcparse/IfcParse.h +++ b/src/ifcparse/IfcParse.h @@ -269,6 +269,8 @@ namespace IfcParse { IFC_PARSE_API IfcEntityInstanceData* read(unsigned int i, IfcFile* t, boost::optional offset = boost::none); IFC_PARSE_API IfcEntityList::ptr traverse(IfcUtil::IfcBaseClass* instance, int max_level = -1); + + IFC_PARSE_API IfcEntityList::ptr traverse_breadth_first(IfcUtil::IfcBaseClass* instance, int max_level = -1); } IFC_PARSE_API std::ostream& operator<< (std::ostream& os, const IfcParse::IfcFile& f);