From 4a81f72f3227adcbfcbea1897189e15baed0ed9a Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 28 Aug 2026 10:37:19 +0200 Subject: [PATCH] Address quadratic inverse handling --- src/ifcparse/parse.cpp | 100 +++++++++++++++++- .../tests/test_ifcopenshell_parse.cpp | 51 +++++++++ 2 files changed, 149 insertions(+), 2 deletions(-) diff --git a/src/ifcparse/parse.cpp b/src/ifcparse/parse.cpp index 188bce0668..52b663c88a 100644 --- a/src/ifcparse/parse.cpp +++ b/src/ifcparse/parse.cpp @@ -42,6 +42,7 @@ #include #include #include +#include // Apple clang's libc++ has no floating-point std::from_chars overload (it's // =deleted), so on macOS doubles are parsed via strtod_l with a cached "C" @@ -1605,6 +1606,67 @@ class apply_individual_instance_visitor { }; }; +namespace { +struct inverse_attribute_difference { + std::vector removed; + std::vector added; + size_t intersection_size = 0; +}; + +inverse_attribute_difference compute_inverse_attribute_difference( + const std::vector& current, + const std::vector& replacement) { + inverse_attribute_difference result; + const auto common_size = std::min(current.size(), replacement.size()); + const auto mismatch = std::mismatch(current.begin(), current.begin() + common_size, replacement.begin()); + if (mismatch.first == current.begin() + common_size) { + result.intersection_size = common_size; + result.removed.insert(result.removed.end(), current.begin() + common_size, current.end()); + result.added.insert(result.added.end(), replacement.begin() + common_size, replacement.end()); + return result; + } + + std::unordered_map remaining_current; + remaining_current.reserve(current.size()); + for (const auto& instance : current) { + ++remaining_current[instance]; + } + + result.added.reserve(replacement.size()); + for (const auto& instance : replacement) { + auto it = remaining_current.find(instance); + if (it != remaining_current.end() && it->second != 0) { + --it->second; + ++result.intersection_size; + } else { + result.added.push_back(instance); + } + } + + result.removed.reserve(current.size()); + for (const auto& instance : current) { + auto it = remaining_current.find(instance); + if (it != remaining_current.end() && it->second != 0) { + --it->second; + result.removed.push_back(instance); + } + } + return result; +} + +bool use_inverse_attribute_difference(const inverse_attribute_difference& difference) { + const auto changed_count = difference.removed.size() + difference.added.size(); + // Use the delta path when it avoids at least half of the inverse-index mutations. + return difference.intersection_size != 0 && changed_count <= difference.intersection_size * 2; +} + +bool can_have_significant_inverse_attribute_intersection(size_t current_size, size_t replacement_size) { + const auto smaller_size = std::min(current_size, replacement_size); + const auto larger_size = std::max(current_size, replacement_size); + return larger_size <= smaller_size * 3; +} +} // namespace + template typename std::enable_if< (!std::is_base_of_v || std::is_same_v), @@ -1629,6 +1691,18 @@ express::base::set_attribute_value(size_t i, const T& t) { } auto current_attribute = get_attribute_value(i); + inverse_attribute_difference inverse_difference; + bool update_inverses_incrementally = false; + std::vector current_instances; + if constexpr (std::is_same_v>) { + if (current_attribute.type() == ifcopenshell::Argument_AGGREGATE_OF_ENTITY_INSTANCE && + can_have_significant_inverse_attribute_intersection(current_attribute.size(), t.size())) { + current_instances = (std::vector)current_attribute; + inverse_difference = compute_inverse_attribute_difference(current_instances, t); + update_inverses_incrementally = use_inverse_attribute_difference(inverse_difference); + } + } + // Deregister old attribute guid in file guid map. if (i == 0 && (file()->ifcroot_type() != nullptr) && this->declaration().is(*file()->ifcroot_type())) { try { @@ -1645,7 +1719,20 @@ express::base::set_attribute_value(size_t i, const T& t) { } } - if constexpr (std::is_same_v || std::is_same_v> || std::is_same_v>> || std::is_same_v) { + if constexpr (std::is_same_v>) { + unregister_inverse_visitor visitor(*file(), *this); + if (update_inverses_incrementally) { + for (const auto& instance : inverse_difference.removed) { + visitor(instance, (int)i); + } + } else if (!current_instances.empty()) { + for (const auto& instance : current_instances) { + visitor(instance, (int)i); + } + } else { + apply_individual_instance_visitor(current_attribute, (int)i).apply(visitor); + } + } else if constexpr (std::is_same_v || std::is_same_v>> || std::is_same_v) { // Deregister inverse indices in file unregister_inverse_visitor visitor(*file(), *this); apply_individual_instance_visitor(current_attribute, (int)i).apply(visitor); @@ -1665,7 +1752,16 @@ express::base::set_attribute_value(size_t i, const T& t) { auto new_attribute = get_attribute_value(i); // Register inverse indices in file - if constexpr (std::is_same_v || std::is_same_v> || std::is_same_v>>) { + if constexpr (std::is_same_v>) { + register_inverse_visitor visitor(*file(), *this); + if (update_inverses_incrementally) { + for (const auto& instance : inverse_difference.added) { + visitor(instance, (int)i); + } + } else { + apply_individual_instance_visitor(new_attribute, (int)i).apply(visitor); + } + } else if constexpr (std::is_same_v || std::is_same_v>>) { register_inverse_visitor visitor(*file(), *this); apply_individual_instance_visitor(new_attribute, (int)i).apply(visitor); } diff --git a/src/ifcparse/tests/test_ifcopenshell_parse.cpp b/src/ifcparse/tests/test_ifcopenshell_parse.cpp index 760d3aca3a..4fe391b41e 100644 --- a/src/ifcparse/tests/test_ifcopenshell_parse.cpp +++ b/src/ifcparse/tests/test_ifcopenshell_parse.cpp @@ -45,3 +45,54 @@ TEST_CASE("Bypassed entity types include their subtypes", "[ifcparse]") { CHECK(file.instances_by_type("IfcRepresentationItem").empty()); CHECK(file.instances_by_type("IfcCartesianPoint").empty()); } + +TEST_CASE("Aggregate inverse updates preserve reference multiplicity", "[ifcparse]") { + ifcopenshell::file file(ifcopenshell::schema_by_name("IFC4")); + const auto* segment_declaration = file.schema()->declaration_by_name("IfcCompositeCurveSegment"); + auto curve = file.create(file.schema()->declaration_by_name("IfcCompositeCurve")); + auto segment_a = file.create(segment_declaration); + auto segment_b = file.create(segment_declaration); + auto segment_c = file.create(segment_declaration); + auto segment_d = file.create(segment_declaration); + const auto inverse_count = [&file](const express::base& instance) { + return file.instances_by_reference(instance.id()).size(); + }; + + curve.set_attribute_value(0, std::vector{segment_a, segment_a, segment_b, segment_c}); + CHECK(inverse_count(segment_a) == 2); + CHECK(inverse_count(segment_b) == 1); + CHECK(inverse_count(segment_c) == 1); + CHECK(inverse_count(segment_d) == 0); + + curve.set_attribute_value(0, std::vector{segment_a, segment_a, segment_b, segment_c, segment_d}); + CHECK(inverse_count(segment_a) == 2); + CHECK(inverse_count(segment_b) == 1); + CHECK(inverse_count(segment_c) == 1); + CHECK(inverse_count(segment_d) == 1); + + curve.set_attribute_value(0, std::vector{segment_a, segment_a, segment_b, segment_c}); + CHECK(inverse_count(segment_a) == 2); + CHECK(inverse_count(segment_b) == 1); + CHECK(inverse_count(segment_c) == 1); + CHECK(inverse_count(segment_d) == 0); + + const std::vector reordered{segment_c, segment_a, segment_b, segment_a}; + curve.set_attribute_value(0, reordered); + CHECK((std::vector)curve.get_attribute_value(0) == reordered); + CHECK(inverse_count(segment_a) == 2); + CHECK(inverse_count(segment_b) == 1); + CHECK(inverse_count(segment_c) == 1); + CHECK(inverse_count(segment_d) == 0); + + curve.set_attribute_value(0, std::vector{segment_a, segment_b, segment_b, segment_d}); + CHECK(inverse_count(segment_a) == 1); + CHECK(inverse_count(segment_b) == 2); + CHECK(inverse_count(segment_c) == 0); + CHECK(inverse_count(segment_d) == 1); + + curve.set_attribute_value(0, std::vector{segment_c, segment_c, segment_d}); + CHECK(inverse_count(segment_a) == 0); + CHECK(inverse_count(segment_b) == 0); + CHECK(inverse_count(segment_c) == 2); + CHECK(inverse_count(segment_d) == 1); +}