diff --git a/src/ifcopenshell-python/ifcopenshell/util/element.py b/src/ifcopenshell-python/ifcopenshell/util/element.py index db0ef219d8..174235f334 100644 --- a/src/ifcopenshell-python/ifcopenshell/util/element.py +++ b/src/ifcopenshell-python/ifcopenshell/util/element.py @@ -1753,27 +1753,21 @@ def remove_deep2( # ifc_file.batch() if not ifc_file: ifc_file = element.file - total_inverses = ifc_file.get_total_inverses(element) - if total_inverses > 0: - - def are_inverses_contained() -> bool: - also_considered_inverses = 0 - - for considered_element in also_consider: - traverse = ifc_file.traverse(considered_element, max_levels=1) - if element in traverse: - also_considered_inverses += 1 - if total_inverses == also_considered_inverses: - return True - return False - - if not are_inverses_contained(): - return + # The start element may only be referenced from also_consider; decided in + # C++ without traversing each considered element. + if not ifc_file._all_inverses_within(element, [e.id() for e in also_consider if e.id()]): + return to_delete: set[ifcopenshell.entity_instance] = set() subgraph = list(ifc_file.traverse(element, breadth_first=True)) subgraph.extend(also_consider) subgraph_set = set(subgraph) + # Which subgraph members are referenced only from inside the subgraph, + # decided once in C++ without materializing any inverse list. Clearing + # large aggregates below only removes references whose source is inside + # the subgraph, so this doesn't change while the loop runs. + subgraph_ids = [e.id() for e in subgraph_set if e.id()] + referenced_only_within = set(ifc_file._ids_referenced_only_within(subgraph_ids)) subelement_queue = [element] # Cache already processed entities to avoid traversing them multiple time. @@ -1787,12 +1781,7 @@ def remove_deep2( subelement_id and subelement_id not in processed_ids and subelement not in do_not_delete - and ( - # 0 or 1 inverses guarantees that the subelement only exists in this subgraph - ifc_file.get_total_inverses(subelement) < 2 - # Alternatively, let's ensure all inverses are within the subgraph - or len(set(ifc_file.get_inverse(subelement)) - subgraph_set) == 0 - ) + and subelement_id in referenced_only_within ): to_delete.add(subelement) subelement_queue.extend(ifc_file.traverse(subelement, max_levels=1)[1:]) diff --git a/src/ifcparse/storage.h b/src/ifcparse/storage.h index 0e41073f06..01aec3846e 100644 --- a/src/ifcparse/storage.h +++ b/src/ifcparse/storage.h @@ -592,6 +592,27 @@ namespace ifcopenshell { return n; } + // True iff pred accepts the source of every live record + // referencing referenced_id. Stops at the first rejection. + template + bool all_sources(uint32_t referenced_id, Pred&& pred) const { + auto range = base_range(referenced_id); + for (auto it = range.first; it != range.second; ++it) { + if (!is_dead(*it) && !pred(it->source_id)) { + return false; + } + } + auto bucket = delta_.find(referenced_id); + if (bucket != delta_.end()) { + for (const auto& record : bucket->second) { + if (!pred(record.source_id)) { + return false; + } + } + } + return true; + } + bool empty() const { return size() == 0; } diff --git a/src/ifcwrap/IfcParseWrapper.i b/src/ifcwrap/IfcParseWrapper.i index 261f3c9da9..b6df107d85 100644 --- a/src/ifcwrap/IfcParseWrapper.i +++ b/src/ifcwrap/IfcParseWrapper.i @@ -148,6 +148,7 @@ PyObject* get_feature(const std::string& x) { #include #include +#include // Atomic IFC/STEP write (issue #4797): serialize to a temporary file next to // the destination, then atomically rename it onto the destination. If the @@ -316,6 +317,46 @@ private: throw ifcopenshell::exception("Only entities with ids are supported for get_total_inverses. Provided entity: '" + e.declaration().name() + "'."); } + // True iff every instance referencing e has an id in ids. Stops at the + // first referencing instance outside the set, without materializing any. + bool _all_inverses_within(const express::base& e, const std::vector& ids) { + auto e_ = e.as(); + if (!e_) { + throw ifcopenshell::exception("Only entities with ids are supported for _all_inverses_within. Provided entity: '" + e.declaration().name() + "'."); + } + const std::unordered_set allowed(ids.begin(), ids.end()); + const auto referenced_id = (uint32_t)e_.id(); + return std::visit([referenced_id, &allowed](auto& x) -> bool { + if constexpr (std::is_same_v, ifcopenshell::impl::in_memory_file_storage>) { + return x.byref_excl_.all_sources(referenced_id, [&allowed](uint32_t source_id) { + return allowed.count(source_id) != 0; + }); + } else { + throw ifcopenshell::exception("_all_inverses_within is only implemented for in-memory storage"); + } + }, $self->storage_); + } + + // The subset of ids whose every referencing instance is itself in ids: + // one crossing in, one crossing out, early exit per id in C++. + std::vector _ids_referenced_only_within(const std::vector& ids) { + const std::unordered_set allowed(ids.begin(), ids.end()); + const auto within = [&allowed](uint32_t source_id) { return allowed.count(source_id) != 0; }; + std::vector contained; + std::visit([&](auto& x) { + if constexpr (std::is_same_v, ifcopenshell::impl::in_memory_file_storage>) { + for (int id : ids) { + if (x.byref_excl_.all_sources((uint32_t)id, within)) { + contained.push_back(id); + } + } + } else { + throw ifcopenshell::exception("_ids_referenced_only_within is only implemented for in-memory storage"); + } + }, $self->storage_); + return contained; + } + void _write(const std::string& fn) { // Atomic write: serialize to a temp file next to the target, then // atomically rename it into place, so an interrupted write can never