From 0da210b732ae6f32f276486feacff641193e9065 Mon Sep 17 00:00:00 2001
From: Andrej730 <azhilenkov@gmail.com>
Date: Wed, 22 Nov 2023 18:50:09 +0500
Subject: [PATCH] edit_object_placement optimization for objects with
 subchildren #4035

1) We were iterating over same subchildren 2 or more times.
2) There is no need to change subchildren position at all since their parent position is already adjusted.

Explained here in detail - https://github.com/IfcOpenShell/IfcOpenShell/issues/4035#issuecomment-1822824352
---
 .../api/geometry/edit_object_placement.py     |  5 ++-
 .../geometry/test_edit_object_placement.py    | 41 +++++++++++++++++++
 2 files changed, 44 insertions(+), 2 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 2dcf22662b..7e2270a901 100644
--- a/src/ifcopenshell-python/ifcopenshell/api/geometry/edit_object_placement.py
+++ b/src/ifcopenshell-python/ifcopenshell/api/geometry/edit_object_placement.py
@@ -100,6 +100,8 @@ class Usecase:
         if not placement:
             return []
         results = []
+        # NOTE: we ignore subchildren as we already adjust position for their parent
+        # therefore they're not present in `results` and `should_transform_children` should be `True`
         for referenced_placement in placement.ReferencedByPlacements:
             matrix = ifcopenshell.util.placement.get_local_placement(referenced_placement)
             for obj in referenced_placement.PlacesObject:
@@ -111,8 +113,7 @@ class Usecase:
                     # Feature elements affect the geometry of their parent, and
                     # so logically should always move with the parent.
                     continue
-                results.append({"product": obj, "matrix": matrix, "is_si": False, "should_transform_children": False})
-            results.extend(self.get_children_settings(referenced_placement))
+                results.append({"product": obj, "matrix": matrix, "is_si": False, "should_transform_children": True})
         return results
 
     def get_relative_placement(self, placement_rel_to):
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 3e2d08627b..d840aec181 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
@@ -582,6 +582,47 @@ class TestEditObjectPlacement(test.bootstrap.IFC4):
         with pytest.raises(RuntimeError):
             self.file.by_id(previous_placement_id)
 
+    def test_changing_placements_without_affecting_children_doesnt_affect_subchildren(self):
+        def np_matrix_translation(translation):
+            (m := numpy.eye(4))[:3, 3] = translation
+            return m
+
+        ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcProject")
+        ifcopenshell.api.run("unit.assign_unit", self.file)
+
+        building = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcBuilding")
+        storey = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcBuildingStorey")
+        ifcopenshell.api.run("aggregate.assign_object", self.file, product=storey, relating_object=building)
+        wall = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall")
+        ifcopenshell.api.run("spatial.assign_container", self.file, product=wall, relating_structure=storey)
+
+        matrix = np_matrix_translation((1, 1, 1))
+        submatrix = np_matrix_translation((1, 2, 3))
+        building_placement_id = ifcopenshell.api.run(
+            "geometry.edit_object_placement", self.file, product=building, matrix=matrix.copy(), is_si=False
+        ).id()
+        storey_placement_id = ifcopenshell.api.run(
+            "geometry.edit_object_placement", self.file, product=storey, matrix=matrix.copy(), is_si=False
+        ).id()
+        wall_placement_id = ifcopenshell.api.run(
+            "geometry.edit_object_placement", self.file, product=wall, matrix=matrix.copy(), is_si=False
+        ).id()
+        ifcopenshell.api.run(
+            "geometry.edit_object_placement",
+            self.file,
+            product=building,
+            matrix=submatrix.copy(),
+            is_si=False,
+        )
+        # product and it's children have their placement rebuilt
+        with pytest.raises(RuntimeError):
+            self.file.by_id(building_placement_id)
+        with pytest.raises(RuntimeError):
+            self.file.by_id(storey_placement_id)
+        # subchildren are unaffected, exception is not raised
+        self.file.by_id(wall_placement_id)
+        assert numpy.array_equal(ifcopenshell.util.placement.get_local_placement(wall.ObjectPlacement), matrix)
+
 
 class TestEditObjectPlacementIFC2X3(test.bootstrap.IFC2X3):
     def test_changing_placements_relative_to_a_distribution_element(self):