Glassvision/IfcOpenShell
1
0
Fork 0
mirror of https://github.com/IfcOpenShell/IfcOpenShell.git synced 2026-08-09 17:31:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix #5954. Special automatic movement of features shouldn't then also move children of those features.

Browse Source 
If you move a wall, and that wall has features (e.g. openings), it's
desirable to also move those openings (because they are invisible). This
is a special exception to `should_transform_children` because the
definition of the feature (opening) is inherently tied to the parent
(wall).

What wasn't considered is that this would typically then also move
subchildren of the features (e.g. fills like doors). I'm surprised
nobody caught this earlier.

I did also consider another approach where if you move a wall, it moves
all unfilled openings, and if you move a door which fills a opening, it
moves the opening too. Intuitively it sounds nice, but it doesn't work
because:

 - Openings can have multiple fillings. If you move all fillings, they
all fight to move the openings.
 - All logic about children goes one way: a placement may have child
placements relative to it. This breaks the convention (if moving a door
instead moves its opening) which can make brains explode.
 - It starts to conflate rules about relative / referenced placements
with spatial decomposition. We assume all IFCs are valid and follows the
convention of relative placement but we cannot guarantee this. This also
leads to brain explosion.
This commit is contained in:
Dion Moult
2025-01-13 15:44:52 +11:00
parent 316b9554d4
commit 8cf7057abc
2 changed files with 22 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ifcopenshell-python/ifcopenshell/api/geometry/edit_object_placement.py
+9 -1
View File
@@ -130,7 +130,15 @@ class Usecase:
continue
elif obj.is_a("IfcFeatureElement"):
# Feature elements affect the geometry of their parent, and
# so logically should always move with the parent.
# so logically should always move with the parent. However,
# subchildren shouldn't move.
placement2 = obj.ObjectPlacement
for referenced_placement2 in placement2.ReferencedByPlacements:
matrix2 = ifcopenshell.util.placement.get_local_placement(referenced_placement2)
for obj2 in referenced_placement2.PlacesObject:
results.append(
{"product": obj2, "matrix": matrix2, "is_si": False, "should_transform_children": True}
)
continue
results.append({"product": obj, "matrix": matrix, "is_si": False, "should_transform_children": True})
return results
src/ifcopenshell-python/test/api/geometry/test_edit_object_placement.py
+13 -1
View File
@@ -545,11 +545,12 @@ class TestEditObjectPlacement(test.bootstrap.IFC4):
with pytest.raises(RuntimeError):
self.file.by_id(previous_placement_id)
def test_changing_placements_always_affecting_child_features_as_a_special_case(self):
def test_changing_placements_always_affecting_child_features_but_not_subchildren_as_a_special_case(self):
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
ifcopenshell.api.unit.assign_unit(self.file)
element = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcWall")
subelement = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcOpeningElement")
subsubelement = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcDoor")
matrix = numpy.eye(4)
matrix[:3, 3] = (1, 1, 1)
@@ -557,16 +558,23 @@ class TestEditObjectPlacement(test.bootstrap.IFC4):
submatrix = numpy.eye(4)
submatrix[:3, 3] = (1, 2, 3)
subsubmatrix = numpy.eye(4)
subsubmatrix[:3, 3] = (7, 8, 9)
shifted_submatrix = numpy.eye(4)
shifted_submatrix[:3, 3] = (1, 3, 5)
ifcopenshell.api.void.add_opening(self.file, opening=subelement, element=element)
ifcopenshell.api.void.add_filling(self.file, opening=subelement, element=subsubelement)
previous_placement_id = ifcopenshell.api.geometry.edit_object_placement(
self.file, product=element, matrix=matrix.copy(), is_si=False
).id()
ifcopenshell.api.geometry.edit_object_placement(
self.file, product=subelement, matrix=submatrix.copy(), is_si=False
)
ifcopenshell.api.geometry.edit_object_placement(
self.file, product=subsubelement, matrix=subsubmatrix.copy(), is_si=False
)
ifcopenshell.api.geometry.edit_object_placement(
self.file,
product=element,
@@ -578,7 +586,11 @@ class TestEditObjectPlacement(test.bootstrap.IFC4):
assert numpy.array_equal(
ifcopenshell.util.placement.get_local_placement(subelement.ObjectPlacement), shifted_submatrix
)
assert numpy.array_equal(
ifcopenshell.util.placement.get_local_placement(subsubelement.ObjectPlacement), subsubmatrix
)
assert subelement.ObjectPlacement.PlacementRelTo == element.ObjectPlacement
assert subsubelement.ObjectPlacement.PlacementRelTo == subelement.ObjectPlacement
# old placement should be removed to avoid orphaned entities
with pytest.raises(RuntimeError):
self.file.by_id(previous_placement_id)