Fix add_boolean removing second operands from unrelated representations

add_boolean was removing second operands from ALL IfcShapeRepresentations
that referenced them, which could corrupt unrelated shapes and leave
representations with empty Items (bug #7803).

The API no longer modifies Items — callers manage this explicitly.
validate_type and Bonsai's AddBoolean operator now handle their own
item removal scoped to the correct representation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Dion Moult
2026-03-21 18:21:41 +11:00
parent bcfad8d96d
commit fca258fb07
5 changed files with 38 additions and 37 deletions
@@ -540,6 +540,16 @@ class AddBoolean(Operator, tool.Ifc.Operator):
booleans = ifcopenshell.api.geometry.add_boolean(tool.Ifc.get(), first_item, second_items, props.operator)
rep_obj = tool.Geometry.get_geometry_props().representation_obj
if booleans:
# Users typically select two top-level items and expect the
# operand to be absorbed into the boolean, not remain as a
# standalone item alongside it.
representation = tool.Geometry.get_active_representation(rep_obj)
representation = ifcopenshell.util.representation.resolve_representation(representation)
second_items_set = set(second_items)
new_items = [i for i in representation.Items if i not in second_items_set]
if new_items:
representation.Items = new_items
rep_element = tool.Ifc.get_entity(rep_obj)
tool.Model.mark_manual_booleans(rep_element, booleans)
tool.Geometry.reload_representation(rep_obj)
@@ -31,17 +31,6 @@ def add_boolean(
) -> list[ifcopenshell.entity_instance]:
"""Adds a boolean operation to two or more representation items
If an IfcBooleanOperand is part of the top level items in an
IfcShapeRepresentation, it will be removed from that level whilst being
added to the IfcBooleanResult. This is because it is generally intuitive
that an item is either participating in a boolean operation, or being an
item in its own right, but not both.
However, if an IfcBooleanOperand is part of another boolean operation
already, it will not be removed from the existing operation. A new
operation will be created, and therefore it will participate in two
operations.
This function protects against recursive booleans.
After a boolean operation is made, since the items of
@@ -101,9 +90,6 @@ def add_boolean(
booleans = []
for second_item in second_items:
for inverse in file.get_inverse(second_item):
if inverse.is_a("IfcShapeRepresentation"):
inverse.Items = list(set(inverse.Items) - {second_item})
if first.is_a("IfcTesselatedFaceSet"):
first.Closed = True # For now, trust the user to do the right thing.
if second_item.is_a("IfcTesselatedFaceSet"):
@@ -83,6 +83,7 @@ def validate_type(
if remaining_items:
ifcopenshell.api.geometry.add_boolean(file, preferred_item, remaining_items, "UNION")
representation.Items = [i for i in representation.Items if i not in remaining_items]
representation.RepresentationType = ifcopenshell.util.representation.guess_type(representation.Items)
if representation.RepresentationType == "CSG":
@@ -42,7 +42,7 @@ class TestAddBoolean(test.bootstrap.IFC4):
assert boolean.FirstOperand == first
assert boolean.SecondOperand == second
assert boolean.Operator == "DIFFERENCE"
assert set(rep.Items) == {boolean}
assert set(rep.Items) == {boolean, second}
def test_adding_multiple_booleans_from_three_top_level_items(self):
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
@@ -58,13 +58,14 @@ class TestAddBoolean(test.bootstrap.IFC4):
booleans = ifcopenshell.api.geometry.add_boolean(self.file, first, [second1, second2])
assert len(booleans) == 2
assert len(rep.Items) == 1
assert rep.Items[0].FirstOperand.is_a("IfcBooleanResult")
assert rep.Items[0].SecondOperand == second2
assert rep.Items[0].Operator == "DIFFERENCE"
assert rep.Items[0].FirstOperand.FirstOperand == first
assert rep.Items[0].FirstOperand.SecondOperand == second1
assert rep.Items[0].FirstOperand.Operator == "DIFFERENCE"
final_boolean = booleans[-1]
assert final_boolean.FirstOperand.is_a("IfcBooleanResult")
assert final_boolean.SecondOperand == second2
assert final_boolean.Operator == "DIFFERENCE"
assert final_boolean.FirstOperand.FirstOperand == first
assert final_boolean.FirstOperand.SecondOperand == second1
assert final_boolean.FirstOperand.Operator == "DIFFERENCE"
assert set(rep.Items) == {final_boolean, second1, second2}
def test_adding_a_boolean_to_an_existing_operand_from_a_top_level_item(self):
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
@@ -78,14 +79,16 @@ class TestAddBoolean(test.bootstrap.IFC4):
second2 = builder.block()
rep = builder.get_representation(body, [first, second1])
booleans = ifcopenshell.api.geometry.add_boolean(self.file, first, [second1])
# second1 stays in Items, add second2 as well
rep.Items = list(rep.Items) + [second2]
booleans = ifcopenshell.api.geometry.add_boolean(self.file, first, [second2])
assert len(booleans) == 1
assert len(rep.Items) == 1
assert rep.Items[0].FirstOperand.is_a("IfcBooleanResult")
assert rep.Items[0].SecondOperand == second2
assert rep.Items[0].FirstOperand.FirstOperand == first
assert rep.Items[0].FirstOperand.SecondOperand == second1
final_boolean = booleans[0]
assert final_boolean.FirstOperand.is_a("IfcBooleanResult")
assert final_boolean.SecondOperand == second2
assert final_boolean.FirstOperand.FirstOperand == first
assert final_boolean.FirstOperand.SecondOperand == second1
assert set(rep.Items) == {final_boolean, second1, second2}
def test_adding_a_boolean_to_an_existing_operand_from_another_operand(self):
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
@@ -104,7 +107,7 @@ class TestAddBoolean(test.bootstrap.IFC4):
booleans = ifcopenshell.api.geometry.add_boolean(self.file, first1, [second2])
assert len(booleans) == 1
assert len(rep.Items) == 2
assert len(rep.Items) == 4
assert self.file.get_total_inverses(first1) == 1
result = next(iter(self.file.get_inverse(first1)))
@@ -132,14 +135,15 @@ class TestAddBoolean(test.bootstrap.IFC4):
rep = builder.get_representation(body, [first, second])
ifcopenshell.api.geometry.add_boolean(self.file, first, [second])
ifcopenshell.api.geometry.add_boolean(self.file, first, [second])
assert len(rep.Items) == 1
assert rep.Items[0].FirstOperand == first
assert rep.Items[0].SecondOperand == second
assert set(rep.Items) == {self.file.by_type("IfcBooleanResult")[0], second}
boolean = self.file.by_type("IfcBooleanResult")[0]
assert boolean.FirstOperand == first
assert boolean.SecondOperand == second
ifcopenshell.api.geometry.add_boolean(self.file, second, [second])
ifcopenshell.api.geometry.add_boolean(self.file, second, [first])
assert len(rep.Items) == 1
assert rep.Items[0].FirstOperand == first
assert rep.Items[0].SecondOperand == second
assert set(rep.Items) == {boolean, second}
assert boolean.FirstOperand == first
assert boolean.SecondOperand == second
assert len(self.file.by_type("IfcBooleanResult")) == 1
@@ -76,7 +76,7 @@ class TestValidateType(test.bootstrap.IFC4):
booleans = ifcopenshell.api.geometry.add_boolean(self.file, first, [second1])
assert len(booleans) == 1
assert len(rep.Items) == 3
assert len(rep.Items) == 4
assert ifcopenshell.api.geometry.validate_type(self.file, rep) is True
assert len(rep.Items) == 1
assert rep.RepresentationType == "CSG"
@@ -96,9 +96,9 @@ class TestValidateType(test.bootstrap.IFC4):
booleans = ifcopenshell.api.geometry.add_boolean(self.file, first, [second1])
assert len(booleans) == 1
assert len(rep.Items) == 2
assert len(rep.Items) == 3 # boolean replaced first, but second1 stays in Items
assert ifcopenshell.api.geometry.validate_type(self.file, rep) is False
assert len(rep.Items) == 2
assert len(rep.Items) == 2 # validate_type unioned second1 into the boolean
assert rep.RepresentationType is None