mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-09-22 00:32:38 +00:00
Fix merge crash + surface/lock fillet preview connections
DumbWallJoiner.merge previously crashed on walls with a slab underside
clip because the ConnectedTo / ConnectedFrom migration loops assumed
every rel was an IfcRelConnectsPathElements. The slab's
IfcRelConnectsElements(TOP) rel has no RelatingConnectionType /
RelatedConnectionType and raised AttributeError mid-migration. Filter
on rel class; the slab rel dies with element2 via the trailing
delete_ifc_object cascade.
The fillet preview pen icon now also flips the corner's
BIMWallProperties.is_editing so the connection-disconnect gizmos
surface in parallel with the radius drag. CancelWallFilletPreview
clears the flag before tearing the preview state down so both UIs
hide together. GizmoWallUnjoinSingle.poll inlines the viewport +
array-child guards from the topology gate so the gizmo can show
during preview — its own is_editing check is the real gate.
Fillet-to-source-wall path connection icons render in a muted gray
(LOCKED_COLOR) instead of the active disconnect tone, and the
bim.disconnect_elements operator early-returns with an INFO report
("Fillet wall path connections can't be unjoined — delete the fillet
wall element to remove the corner.") when either side resolves to a
fillet corner. The slab clip rel kind stays disconnect-able since
its identity is separate from the fillet's chord-axis reference.
Drive-by /improve polish on adjacent wall.py code: 3 comment tightenings
dropping sibling-symbol names + a defensive ``if opening.ObjectPlacement:``
guard in the merge opening migration matching the pattern used elsewhere
in the same file.
Generated with the assistance of an AI coding tool.
This commit is contained in:
@@ -231,7 +231,9 @@ def test_disconnect_dispatches_one_call_per_rel():
|
||||
return_value=[(rel1, "path"), (rel2, "element-top")],
|
||||
), patch("bonsai.bim.module.model.wall.bonsai.core.connection.disconnect_rel") as dispatch, patch(
|
||||
"bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=Mock()
|
||||
), patch("bonsai.bim.module.model.wall._resync_walls_after_mutation"):
|
||||
), patch("bonsai.bim.module.model.wall._resync_walls_after_mutation"), patch(
|
||||
"bonsai.bim.module.model.wall.tool.Parametric.is_fillet_corner_wall", return_value=False
|
||||
):
|
||||
DisconnectElements._perform(op, context=MagicMock())
|
||||
|
||||
assert dispatch.call_count == 2
|
||||
@@ -271,7 +273,9 @@ def test_disconnect_resyncs_path_objs_once_for_path_kind():
|
||||
side_effect=lambda e: {elem_a: obj_a, elem_b: obj_b}[e],
|
||||
), patch("bonsai.bim.module.model.wall.bonsai.core.connection.disconnect_rel"), patch(
|
||||
"bonsai.bim.module.model.wall._resync_walls_after_mutation"
|
||||
) as resync:
|
||||
) as resync, patch(
|
||||
"bonsai.bim.module.model.wall.tool.Parametric.is_fillet_corner_wall", return_value=False
|
||||
):
|
||||
DisconnectElements._perform(op, context=MagicMock())
|
||||
|
||||
resync.assert_called_once_with([obj_a, obj_b])
|
||||
@@ -294,7 +298,9 @@ def test_disconnect_skips_resync_for_non_path_kind():
|
||||
"bonsai.bim.module.model.wall.tool.Connection.find_rels", return_value=[(rel, "element-top")]
|
||||
), patch("bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=Mock()), patch(
|
||||
"bonsai.bim.module.model.wall.bonsai.core.connection.disconnect_rel"
|
||||
), patch("bonsai.bim.module.model.wall._resync_walls_after_mutation") as resync:
|
||||
), patch("bonsai.bim.module.model.wall._resync_walls_after_mutation") as resync, patch(
|
||||
"bonsai.bim.module.model.wall.tool.Parametric.is_fillet_corner_wall", return_value=False
|
||||
):
|
||||
DisconnectElements._perform(op, context=MagicMock())
|
||||
|
||||
resync.assert_not_called()
|
||||
@@ -322,7 +328,9 @@ def test_disconnect_gizmo_direction_symmetry():
|
||||
"bonsai.bim.module.model.wall.tool.Connection.find_rels", return_value=[(rel, "element-top")]
|
||||
), patch("bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=Mock()), patch(
|
||||
"bonsai.bim.module.model.wall.bonsai.core.connection.disconnect_rel"
|
||||
) as dispatch, patch("bonsai.bim.module.model.wall._resync_walls_after_mutation"):
|
||||
) as dispatch, patch("bonsai.bim.module.model.wall._resync_walls_after_mutation"), patch(
|
||||
"bonsai.bim.module.model.wall.tool.Parametric.is_fillet_corner_wall", return_value=False
|
||||
):
|
||||
DisconnectElements._perform(op, context=MagicMock())
|
||||
return dispatch.call_args.kwargs
|
||||
|
||||
@@ -379,3 +387,59 @@ def test_disconnect_operator_is_registered():
|
||||
assert any(
|
||||
getattr(cls, "bl_idname", None) == "bim.disconnect_elements" for cls in model.classes
|
||||
), "DisconnectElements is not in the model classes tuple"
|
||||
|
||||
|
||||
def test_disconnect_refuses_path_kind_when_either_side_is_fillet():
|
||||
"""The fillet corner's join with its source walls defines its identity
|
||||
— unjoining there would tear down the chord axis reference. The
|
||||
operator reports an INFO directing the user to delete the corner
|
||||
wall and skips the dispatch entirely."""
|
||||
from bonsai.bim.module.model.wall import DisconnectElements
|
||||
|
||||
fillet = Mock(name="fillet_corner")
|
||||
wall = Mock(name="source_wall")
|
||||
rel = Mock()
|
||||
|
||||
ifc_file = MagicMock()
|
||||
ifc_file.by_guid.side_effect = lambda g: {"A": fillet, "B": wall}[g]
|
||||
op = _make_op()
|
||||
|
||||
with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch(
|
||||
"bonsai.bim.module.model.wall.tool.Connection.find_rels", return_value=[(rel, "path")]
|
||||
), patch(
|
||||
"bonsai.bim.module.model.wall.tool.Parametric.is_fillet_corner_wall",
|
||||
side_effect=lambda e: e is fillet,
|
||||
), patch("bonsai.bim.module.model.wall.bonsai.core.connection.disconnect_rel") as dispatch:
|
||||
DisconnectElements._perform(op, context=MagicMock())
|
||||
|
||||
dispatch.assert_not_called()
|
||||
op.report.assert_called_once()
|
||||
args, _ = op.report.call_args
|
||||
assert args[0] == {"INFO"}
|
||||
|
||||
|
||||
def test_disconnect_allows_slab_kind_even_when_wall_is_fillet():
|
||||
"""The fillet ↔ slab underside clip is a different relationship from
|
||||
the fillet ↔ source-wall path join. Slab disconnect must remain
|
||||
available while the corner is in preview."""
|
||||
from bonsai.bim.module.model.wall import DisconnectElements
|
||||
|
||||
fillet = Mock(name="fillet_corner")
|
||||
slab = Mock(name="slab")
|
||||
rel = Mock()
|
||||
|
||||
ifc_file = MagicMock()
|
||||
ifc_file.by_guid.side_effect = lambda g: {"A": fillet, "B": slab}[g]
|
||||
op = _make_op()
|
||||
|
||||
with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch(
|
||||
"bonsai.bim.module.model.wall.tool.Connection.find_rels", return_value=[(rel, "element-top")]
|
||||
), patch(
|
||||
"bonsai.bim.module.model.wall.tool.Parametric.is_fillet_corner_wall",
|
||||
side_effect=lambda e: e is fillet,
|
||||
), patch("bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=Mock()), patch(
|
||||
"bonsai.bim.module.model.wall.bonsai.core.connection.disconnect_rel"
|
||||
) as dispatch, patch("bonsai.bim.module.model.wall._resync_walls_after_mutation"):
|
||||
DisconnectElements._perform(op, context=MagicMock())
|
||||
|
||||
dispatch.assert_called_once()
|
||||
|
||||
@@ -190,3 +190,87 @@ def test_merge_rehosts_before_delete():
|
||||
|
||||
assert rel.RelatingBuildingElement is element1
|
||||
delete_ifc_object.assert_called_once_with(wall2)
|
||||
|
||||
|
||||
def test_merge_skips_non_path_connection_rels():
|
||||
"""``ConnectedTo`` / ``ConnectedFrom`` carry both
|
||||
``IfcRelConnectsPathElements`` (wall-wall joins) AND
|
||||
``IfcRelConnectsElements`` (slab underside clips). Only the path rels
|
||||
expose ``RelatingConnectionType`` / ``RelatedConnectionType``;
|
||||
accessing those attributes on an element rel raises ``AttributeError``.
|
||||
The migration loop must filter on the rel class so a wall with a slab
|
||||
clip can still be merged."""
|
||||
from bonsai.bim.module.model.wall import DumbWallJoiner
|
||||
|
||||
wall1, wall2, element1, element2 = _merge_inputs(has_openings=[])
|
||||
|
||||
path_rel = Mock(name="path_rel")
|
||||
path_rel.is_a = lambda c: c == "IfcRelConnectsPathElements"
|
||||
path_rel.RelatingElement = Mock(name="rel_relating")
|
||||
path_rel.RelatedElement = Mock(name="rel_related")
|
||||
path_rel.RelatingConnectionType = "ATSTART"
|
||||
path_rel.RelatedConnectionType = "ATEND"
|
||||
|
||||
slab_rel = Mock(name="slab_rel")
|
||||
slab_rel.is_a = lambda c: c == "IfcRelConnectsElements"
|
||||
slab_rel.Description = "TOP"
|
||||
# ``RelatedConnectionType`` is what the merge loop reads from
|
||||
# ``ConnectedFrom``; the real ``IfcRelConnectsElements`` schema has
|
||||
# no such attribute, so wire the stub to raise like ifcopenshell does.
|
||||
type(slab_rel).RelatedConnectionType = property(
|
||||
lambda self: (_ for _ in ()).throw(AttributeError("RelatedConnectionType"))
|
||||
)
|
||||
type(slab_rel).RelatingConnectionType = property(
|
||||
lambda self: (_ for _ in ()).throw(AttributeError("RelatingConnectionType"))
|
||||
)
|
||||
element2.ConnectedFrom = [slab_rel, path_rel]
|
||||
|
||||
captured_disconnects = []
|
||||
captured_connects = []
|
||||
|
||||
def fake_disconnect_path(*args, **kwargs):
|
||||
captured_disconnects.append(kwargs)
|
||||
|
||||
def fake_connect_path(*args, **kwargs):
|
||||
captured_connects.append(kwargs)
|
||||
|
||||
p1 = np.array([0.0, 0.0])
|
||||
p2 = np.array([5.0, 0.0])
|
||||
p3 = np.array([5.0, 0.0])
|
||||
p4 = np.array([10.0, 0.0])
|
||||
|
||||
def fake_get_entity(obj):
|
||||
return {wall1: element1, wall2: element2}[obj]
|
||||
|
||||
with (
|
||||
patch("bonsai.bim.module.model.wall.tool.Ifc.is_moved", return_value=False),
|
||||
patch("bonsai.bim.module.model.wall.tool.Ifc.get_entity", side_effect=fake_get_entity),
|
||||
patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=MagicMock(name="ifc_file")),
|
||||
patch(
|
||||
"bonsai.bim.module.model.wall.ifcopenshell.util.representation.get_reference_line",
|
||||
side_effect=lambda elem: (p1, p2) if elem is element1 else (p3, p4),
|
||||
),
|
||||
patch(
|
||||
"bonsai.bim.module.model.wall.ifcopenshell.util.placement.get_local_placement",
|
||||
return_value=np.eye(4),
|
||||
),
|
||||
patch(
|
||||
"bonsai.bim.module.model.wall.ifcopenshell.api.geometry.disconnect_path",
|
||||
side_effect=fake_disconnect_path,
|
||||
),
|
||||
patch(
|
||||
"bonsai.bim.module.model.wall.ifcopenshell.api.geometry.connect_path",
|
||||
side_effect=fake_connect_path,
|
||||
),
|
||||
patch("bonsai.bim.module.model.wall.tool.Model.recreate_wall"),
|
||||
patch("bonsai.bim.module.model.wall.tool.Geometry.delete_ifc_object"),
|
||||
patch("bonsai.bim.module.model.wall.DumbWallJoiner.set_axis"),
|
||||
):
|
||||
# The bug pre-fix: the slab rel's ``RelatedConnectionType`` access
|
||||
# raised AttributeError and crashed merge. With the filter, this
|
||||
# call must complete cleanly.
|
||||
DumbWallJoiner().merge(wall1, wall2)
|
||||
|
||||
assert len(captured_disconnects) == 1
|
||||
assert len(captured_connects) == 1
|
||||
assert captured_disconnects[0]["connection_type"] == "ATEND"
|
||||
|
||||
Reference in New Issue
Block a user