From 54812374805d189ce2dae7332d1fced116abce7b Mon Sep 17 00:00:00 2001 From: Andrej730 Date: Wed, 16 Aug 2023 10:48:42 +0500 Subject: [PATCH] geometry.edit_object_placement was producing orphaned placements It was producing orphaned IfcLocalPlacement if placement was parent to some other one. During edit_object_placement we were replacing all parent references with the new placement but wasn't removing the old one. --- .../api/geometry/edit_object_placement.py | 14 ++-- .../geometry/test_edit_object_placement.py | 83 +++++++------------ 2 files changed, 36 insertions(+), 61 deletions(-) diff --git a/src/ifcopenshell-python/ifcopenshell/api/geometry/edit_object_placement.py b/src/ifcopenshell-python/ifcopenshell/api/geometry/edit_object_placement.py index e12885e1c2..2dcf22662b 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/geometry/edit_object_placement.py +++ b/src/ifcopenshell-python/ifcopenshell/api/geometry/edit_object_placement.py @@ -49,15 +49,14 @@ class Usecase: old_placement = self.settings["product"].ObjectPlacement if old_placement: - inverses = self.file.get_inverse(old_placement) - if len(inverses) == 1: + for inverse in self.file.get_inverse(old_placement): + if inverse.is_a("IfcLocalPlacement"): + ifcopenshell.util.element.replace_attribute(inverse, old_placement, new_placement) + + if self.file.get_total_inverses(old_placement) == 1: self.settings["product"].ObjectPlacement = None old_placement.PlacementRelTo = None - ifcopenshell.util.element.remove_deep2(self.file, old_placement) - else: - for inverse in inverses: - if inverse.is_a("IfcLocalPlacement"): - ifcopenshell.util.element.replace_attribute(inverse, old_placement, new_placement) + ifcopenshell.util.element.remove_deep(self.file, old_placement) new_placement.PlacementRelTo = placement_rel_to self.settings["product"].ObjectPlacement = new_placement @@ -97,7 +96,6 @@ class Usecase: elif getattr(self.settings["product"], "ContainedInStructure", None): return self.settings["product"].ContainedInStructure[0].RelatingStructure.ObjectPlacement - def get_children_settings(self, placement): if not placement: return [] diff --git a/src/ifcopenshell-python/test/api/geometry/test_edit_object_placement.py b/src/ifcopenshell-python/test/api/geometry/test_edit_object_placement.py index cb8d9412f2..3e2d08627b 100644 --- a/src/ifcopenshell-python/test/api/geometry/test_edit_object_placement.py +++ b/src/ifcopenshell-python/test/api/geometry/test_edit_object_placement.py @@ -167,7 +167,6 @@ class TestEditObjectPlacement(test.bootstrap.IFC4): assert numpy.array_equal(ifcopenshell.util.placement.get_local_placement(subelement.ObjectPlacement), matrix2) assert element.ObjectPlacement != subelement.ObjectPlacement - def test_changing_placements_relative_to_a_spatial_container(self): ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcProject") ifcopenshell.api.run("unit.assign_unit", self.file) @@ -515,33 +514,19 @@ class TestEditObjectPlacement(test.bootstrap.IFC4): ifcopenshell.api.run("unit.assign_unit", self.file) element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcChiller") subelement = ifcopenshell.api.run("system.add_port", self.file, element=element) - matrix = numpy.array( - ( - (1.0, 0.0, 0.0, 1.0), - (0.0, 1.0, 0.0, 1.0), - (0.0, 0.0, 1.0, 1.0), - (0.0, 0.0, 0.0, 1.0), - ) - ) - submatrix = numpy.array( - ( - (1.0, 0.0, 0.0, 1.0), - (0.0, 1.0, 0.0, 2.0), - (0.0, 0.0, 1.0, 3.0), - (0.0, 0.0, 0.0, 1.0), - ) - ) - shifted_submatrix = numpy.array( - ( - (1.0, 0.0, 0.0, 1.0), - (0.0, 1.0, 0.0, 3.0), - (0.0, 0.0, 1.0, 5.0), - (0.0, 0.0, 0.0, 1.0), - ) - ) - ifcopenshell.api.run( + + matrix = numpy.eye(4) + matrix[:3, 3] = (1, 1, 1) + + submatrix = numpy.eye(4) + submatrix[:3, 3] = (1, 2, 3) + + shifted_submatrix = numpy.eye(4) + shifted_submatrix[:3, 3] = (1, 3, 5) + + previous_placement_id = ifcopenshell.api.run( "geometry.edit_object_placement", self.file, product=element, matrix=matrix.copy(), is_si=False - ) + ).id() ifcopenshell.api.run( "geometry.edit_object_placement", self.file, product=subelement, matrix=submatrix.copy(), is_si=False ) @@ -556,40 +541,29 @@ class TestEditObjectPlacement(test.bootstrap.IFC4): assert numpy.array_equal(ifcopenshell.util.placement.get_local_placement(element.ObjectPlacement), submatrix) assert numpy.array_equal(ifcopenshell.util.placement.get_local_placement(subelement.ObjectPlacement), shifted_submatrix) assert subelement.ObjectPlacement.PlacementRelTo == element.ObjectPlacement + # old placement should be removed to avoid orphaned entities + with pytest.raises(RuntimeError): + self.file.by_id(previous_placement_id) def test_changing_placements_always_affecting_child_features_as_a_special_case(self): ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcProject") ifcopenshell.api.run("unit.assign_unit", self.file) element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall") subelement = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcOpeningElement") - matrix = numpy.array( - ( - (1.0, 0.0, 0.0, 1.0), - (0.0, 1.0, 0.0, 1.0), - (0.0, 0.0, 1.0, 1.0), - (0.0, 0.0, 0.0, 1.0), - ) - ) - submatrix = numpy.array( - ( - (1.0, 0.0, 0.0, 1.0), - (0.0, 1.0, 0.0, 2.0), - (0.0, 0.0, 1.0, 3.0), - (0.0, 0.0, 0.0, 1.0), - ) - ) - shifted_submatrix = numpy.array( - ( - (1.0, 0.0, 0.0, 1.0), - (0.0, 1.0, 0.0, 3.0), - (0.0, 0.0, 1.0, 5.0), - (0.0, 0.0, 0.0, 1.0), - ) - ) + + matrix = numpy.eye(4) + matrix[:3, 3] = (1, 1, 1) + + submatrix = numpy.eye(4) + submatrix[:3, 3] = (1, 2, 3) + + shifted_submatrix = numpy.eye(4) + shifted_submatrix[:3, 3] = (1, 3, 5) + ifcopenshell.api.run("void.add_opening", self.file, opening=subelement, element=element) - ifcopenshell.api.run( + previous_placement_id = ifcopenshell.api.run( "geometry.edit_object_placement", self.file, product=element, matrix=matrix.copy(), is_si=False - ) + ).id() ifcopenshell.api.run( "geometry.edit_object_placement", self.file, product=subelement, matrix=submatrix.copy(), is_si=False ) @@ -604,6 +578,9 @@ class TestEditObjectPlacement(test.bootstrap.IFC4): assert numpy.array_equal(ifcopenshell.util.placement.get_local_placement(element.ObjectPlacement), submatrix) assert numpy.array_equal(ifcopenshell.util.placement.get_local_placement(subelement.ObjectPlacement), shifted_submatrix) assert subelement.ObjectPlacement.PlacementRelTo == element.ObjectPlacement + # old placement should be removed to avoid orphaned entities + with pytest.raises(RuntimeError): + self.file.by_id(previous_placement_id) class TestEditObjectPlacementIFC2X3(test.bootstrap.IFC2X3):