From f6590d8be25680d74a204145f17f0b1480e3a2eb Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Fri, 12 Jun 2026 08:02:50 +0200 Subject: [PATCH 01/12] Add tool.Wall slab-connection helpers + tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four classmethods enable the new wall-slab connection gizmo work: - iter_wall_slab_connections(wall): yields (slab, rel) tuples for every IfcRelConnectsElements(TOP) on wall.ConnectedFrom — the rel kind extend_walls_to_underside creates. - iter_slab_wall_connections(slab): mirror, walks slab.ConnectedTo so a slab-side gizmo can enumerate every wall clipped to its underside. - find_wall_slab_rel(wall, slab): locates the specific rel between a wall + slab pair so a disconnect operator knows what to remove. - wall_slab_connection_location_world(wall_obj, slab_obj): returns the world-space icon anchor — wall axis midpoint X/Y lifted to the slab's mesh-bbox underside Z. Approximate (uses slab bbox vs reconstructing the slab's clip plane) but adequate for icon placement on a wall whose top meets the slab; returns None when the wall has no IFC Axis representation. Tests (11) pin the rel-shape contract (class + Description=="TOP", non-TOP and non-IfcRelConnectsElements rels skipped, None relating defensively skipped) plus the icon-anchor math (axis-mid lifted to slab-bbox bottom; None for axisless walls). Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/tool/wall.py | 56 +++++ .../model/test_wall_slab_connections.py | 211 ++++++++++++++++++ 2 files changed, 267 insertions(+) create mode 100644 src/bonsai/test/bim/module/model/test_wall_slab_connections.py diff --git a/src/bonsai/bonsai/tool/wall.py b/src/bonsai/bonsai/tool/wall.py index c982b15371..6f471e0c8a 100644 --- a/src/bonsai/bonsai/tool/wall.py +++ b/src/bonsai/bonsai/tool/wall.py @@ -242,6 +242,62 @@ class Wall(bonsai.core.tool.Wall): local_p2 = Vector((p2[0] * unit_scale, p2[1] * unit_scale, 0.0)) return obj.matrix_world @ local_p1, obj.matrix_world @ local_p2 + @classmethod + def iter_wall_slab_connections(cls, wall: ifcopenshell.entity_instance): + """Yield ``(slab, rel)`` tuples for every ``IfcRelConnectsElements(TOP)`` + connecting a slab to this wall — the rel kind ``extend_walls_to_underside`` + creates. Walks ``wall.ConnectedFrom`` because the slab is the relating + side of the TOP rel.""" + for rel in getattr(wall, "ConnectedFrom", []) or (): + if not rel.is_a("IfcRelConnectsElements") or rel.Description != "TOP": + continue + slab = rel.RelatingElement + if slab is None: + continue + yield slab, rel + + @classmethod + def iter_slab_wall_connections(cls, slab: ifcopenshell.entity_instance): + """Yield ``(wall, rel)`` tuples for every wall clipped to this slab's + underside. Mirror of ``iter_wall_slab_connections`` from the slab side + — walks ``slab.ConnectedTo``.""" + for rel in getattr(slab, "ConnectedTo", []) or (): + if not rel.is_a("IfcRelConnectsElements") or rel.Description != "TOP": + continue + wall = rel.RelatedElement + if wall is None: + continue + yield wall, rel + + @classmethod + def find_wall_slab_rel( + cls, wall: ifcopenshell.entity_instance, slab: ifcopenshell.entity_instance + ) -> ifcopenshell.entity_instance | None: + """Return the single ``IfcRelConnectsElements(TOP)`` between ``wall`` + and ``slab``, or ``None`` if none exists. Used by the disconnect + operator to find the specific rel to remove.""" + for s, rel in cls.iter_wall_slab_connections(wall): + if s == slab: + return rel + return None + + @classmethod + def wall_slab_connection_location_world( + cls, wall_obj: bpy.types.Object, slab_obj: bpy.types.Object + ) -> Vector | None: + """World-space point where a wall is clipped by a slab — the wall's + axis midpoint lifted to the slab's underside Z. Approximate: uses the + slab's mesh bbox bottom in world space rather than reconstructing the + slab's clip plane. Adequate for icon placement on a wall whose top + meets the slab; returns ``None`` when the wall has no reference line.""" + ref = cls.get_world_reference_line(wall_obj) + if ref is None: + return None + axis_mid_world = (ref[0] + ref[1]) * 0.5 + slab_bottom_local_z = min(c[2] for c in slab_obj.bound_box) + slab_bottom_world_z = (slab_obj.matrix_world @ Vector((0.0, 0.0, slab_bottom_local_z))).z + return Vector((axis_mid_world.x, axis_mid_world.y, slab_bottom_world_z)) + @classmethod def walk_connected_walls( cls, diff --git a/src/bonsai/test/bim/module/model/test_wall_slab_connections.py b/src/bonsai/test/bim/module/model/test_wall_slab_connections.py new file mode 100644 index 0000000000..5bd9704e3c --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_wall_slab_connections.py @@ -0,0 +1,211 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Behaviour tests for the wall-slab connection helpers on tool.Wall. + +Pins the rel-shape contract (IfcRelConnectsElements with Description=="TOP") +the underside-extension feature creates, and the icon placement contract the +new wall-slab connection gizmo group reads.""" + +from unittest.mock import Mock, patch + +import pytest +from mathutils import Matrix, Vector + +import bonsai.tool as tool + +pytestmark = pytest.mark.model + + +def _rel(klass: str = "IfcRelConnectsElements", description: str = "TOP", relating=None, related=None): + rel = Mock() + rel.is_a = lambda c: c == klass + rel.Description = description + rel.RelatingElement = relating + rel.RelatedElement = related + return rel + + +def _wall_with_rels(*rels) -> Mock: + wall = Mock() + wall.ConnectedFrom = list(rels) + return wall + + +def _slab_with_rels(*rels) -> Mock: + slab = Mock() + slab.ConnectedTo = list(rels) + return slab + + +# --------------------------------------------------------------------------- +# iter_wall_slab_connections — yields (slab, rel) for TOP rels +# --------------------------------------------------------------------------- + + +def test_iter_wall_slab_connections_yields_top_rels(): + slab_a = Mock(name="slab_a") + slab_b = Mock(name="slab_b") + wall = _wall_with_rels( + _rel(relating=slab_a), + _rel(relating=slab_b), + ) + + result = list(tool.Wall.iter_wall_slab_connections(wall)) + + assert result == [(slab_a, wall.ConnectedFrom[0]), (slab_b, wall.ConnectedFrom[1])] + + +def test_iter_wall_slab_connections_skips_non_top_description(): + """Only TOP-described rels count; BOTTOM / SIDE / arbitrary strings are + skipped so other RelConnectsElements semantics aren't confused with the + underside-extension contract.""" + slab = Mock() + wall = _wall_with_rels( + _rel(description="BOTTOM", relating=slab), + _rel(description="TOP", relating=slab), + ) + + result = list(tool.Wall.iter_wall_slab_connections(wall)) + + assert len(result) == 1 + assert result[0][0] is slab + + +def test_iter_wall_slab_connections_skips_non_connectselements_rels(): + """Path-connections to other walls show up on ConnectedFrom too — the + helper must filter on rel class, not just presence.""" + slab = Mock() + wall = _wall_with_rels( + _rel(klass="IfcRelConnectsPathElements", relating=slab), + _rel(klass="IfcRelConnectsElements", relating=slab), + ) + + result = list(tool.Wall.iter_wall_slab_connections(wall)) + + assert len(result) == 1 + + +def test_iter_wall_slab_connections_handles_none_relating(): + """A malformed rel with RelatingElement=None is skipped rather than + raising — defensive against partially-loaded IFC files.""" + wall = _wall_with_rels(_rel(relating=None)) + + result = list(tool.Wall.iter_wall_slab_connections(wall)) + + assert result == [] + + +def test_iter_wall_slab_connections_empty_when_no_connectedfrom(): + wall = Mock() + wall.ConnectedFrom = [] + + assert list(tool.Wall.iter_wall_slab_connections(wall)) == [] + + +# --------------------------------------------------------------------------- +# iter_slab_wall_connections — mirror, walks slab.ConnectedTo +# --------------------------------------------------------------------------- + + +def test_iter_slab_wall_connections_yields_top_rels(): + wall_a = Mock() + wall_b = Mock() + slab = _slab_with_rels( + _rel(related=wall_a), + _rel(related=wall_b), + ) + + result = list(tool.Wall.iter_slab_wall_connections(slab)) + + assert [w for w, _ in result] == [wall_a, wall_b] + + +def test_iter_slab_wall_connections_skips_non_top(): + wall = Mock() + slab = _slab_with_rels( + _rel(description="BOTTOM", related=wall), + _rel(description="TOP", related=wall), + ) + + result = list(tool.Wall.iter_slab_wall_connections(slab)) + + assert len(result) == 1 + + +# --------------------------------------------------------------------------- +# find_wall_slab_rel — locate specific rel between wall + slab +# --------------------------------------------------------------------------- + + +def test_find_wall_slab_rel_returns_match(): + slab_a = Mock(name="slab_a") + slab_b = Mock(name="slab_b") + rel_a = _rel(relating=slab_a) + rel_b = _rel(relating=slab_b) + wall = _wall_with_rels(rel_a, rel_b) + + assert tool.Wall.find_wall_slab_rel(wall, slab_b) is rel_b + + +def test_find_wall_slab_rel_returns_none_when_unconnected(): + slab_a = Mock(name="slab_a") + other_slab = Mock(name="other_slab") + wall = _wall_with_rels(_rel(relating=slab_a)) + + assert tool.Wall.find_wall_slab_rel(wall, other_slab) is None + + +# --------------------------------------------------------------------------- +# wall_slab_connection_location_world — icon anchor point +# --------------------------------------------------------------------------- + + +def test_wall_slab_connection_location_lifts_axis_mid_to_slab_underside(): + """The icon sits at the wall's axis midpoint X/Y lifted to the slab's + underside Z so it reads as a marker on the slab cut line.""" + wall_obj = Mock() + slab_obj = Mock() + slab_obj.matrix_world = Matrix.Translation(Vector((0.0, 0.0, 3.0))) + slab_obj.bound_box = [ + (-1.0, -1.0, 0.0), + (1.0, -1.0, 0.0), + (-1.0, 1.0, 0.0), + (1.0, 1.0, 0.0), + (-1.0, -1.0, 0.2), + (1.0, -1.0, 0.2), + (-1.0, 1.0, 0.2), + (1.0, 1.0, 0.2), + ] + + ref_line = (Vector((1.0, 0.0, 0.0)), Vector((3.0, 0.0, 0.0))) + with patch.object(tool.Wall, "get_world_reference_line", return_value=ref_line): + loc = tool.Wall.wall_slab_connection_location_world(wall_obj, slab_obj) + + assert loc == Vector((2.0, 0.0, 3.0)) + + +def test_wall_slab_connection_location_returns_none_for_axisless_wall(): + """A wall without an IFC Axis representation has no reference line; the + helper returns None so callers can skip rather than guess a location.""" + wall_obj = Mock() + slab_obj = Mock() + with patch.object(tool.Wall, "get_world_reference_line", return_value=None): + assert tool.Wall.wall_slab_connection_location_world(wall_obj, slab_obj) is None From b0eb55cc3897471ab7f2fa2a631e66c50221c84f Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Fri, 12 Jun 2026 08:19:04 +0200 Subject: [PATCH 02/12] Add bim.disconnect_wall_slab operator Counterpart to UnjoinWallPathConnection on the wall-slab side: takes a wall + slab GlobalId pair, locates the IfcRelConnectsElements(TOP) between them via tool.Wall.find_wall_slab_rel, removes it via ifcopenshell.api.geometry.disconnect_element, then re-runs core.regenerate_wall_to_underside so the wall re-clips against any remaining connected slabs (the disconnected slab is excluded naturally because the helper walks tool.Model.get_connected_slab_objs which filters by the rel set). Defensive reports replace silent CANCELLED on three error paths the UI can hit when the gizmo dispatches against stale state: unknown GlobalIds, wall entity without a Blender object, no rel found between the resolved pair. Tests cover all four control flows (happy path + three error paths) plus a registration smoke that catches a forgotten classes-tuple update. A follow-up commit will retrofit this + UnjoinWallPathConnection + the MEP port disconnects through a unified bim.disconnect_elements dispatcher with a small connection-type registry; that lands as a separate single-concern commit so the typed operator can be reviewed first. Generated with the assistance of an AI coding tool. --- .../bonsai/bim/module/model/__init__.py | 1 + src/bonsai/bonsai/bim/module/model/wall.py | 51 ++++++ .../module/model/test_disconnect_wall_slab.py | 164 ++++++++++++++++++ 3 files changed, 216 insertions(+) create mode 100644 src/bonsai/test/bim/module/model/test_disconnect_wall_slab.py diff --git a/src/bonsai/bonsai/bim/module/model/__init__.py b/src/bonsai/bonsai/bim/module/model/__init__.py index 08806ad8fb..35922acf5e 100644 --- a/src/bonsai/bonsai/bim/module/model/__init__.py +++ b/src/bonsai/bonsai/bim/module/model/__init__.py @@ -120,6 +120,7 @@ classes = ( wall.RotateWall90, wall.SplitWall, wall.SplitWallAtCursor, + wall.DisconnectWallSlab, wall.UnjoinWallPathConnection, wall.UnjoinWalls, wall.EnableWallFilletPreview, diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index dd9d21697c..e11f89940b 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -349,6 +349,57 @@ class UnjoinWallPathConnection(_CommitWallDraftsFirstMixin, bpy.types.Operator, _resync_walls_after_mutation([active, other]) +class DisconnectWallSlab(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Operator): + """Disconnect the wall from one specific underside slab — counterpart to + UnjoinWallPathConnection on the wall-slab side. Both endpoints are + identified by IFC GlobalId so the dispatch survives rename / undo / save. + + Drops the IfcRelConnectsElements(TOP) rel + all underside booleans on the + wall, then re-runs regenerate_wall_to_underside which re-clips the wall + to whatever slabs remain connected. The all-booleans-then-regenerate + approach is safe with HEAD's flat BBIM_Boolean pset (no per-slab id + storage); switches to a per-slab boolean removal when PR #8147's + dict-with-slab-guid pset migration lands.""" + + bl_idname = "bim.disconnect_wall_slab" + bl_label = "Disconnect Wall From Slab" + bl_description = "Remove the TOP connection between a wall and one slab and re-clip the wall to remaining slabs" + bl_options = {"REGISTER", "UNDO"} + + wall_guid: bpy.props.StringProperty(name="Wall GlobalId") + slab_guid: bpy.props.StringProperty(name="Slab GlobalId") + + @classmethod + def poll(cls, context): + if not tool.Model.has_selected_ifc_objects(): + cls.poll_message_set("No IFC objects selected.") + return False + if _poll_reject_array_children(cls): + return False + return True + + def _perform(self, context): + ifc_file = tool.Ifc.get() + try: + wall = ifc_file.by_guid(self.wall_guid) if self.wall_guid else None + slab = ifc_file.by_guid(self.slab_guid) if self.slab_guid else None + except RuntimeError: + wall = slab = None + if wall is None or slab is None: + self.report({"ERROR"}, "Could not resolve wall and slab from supplied GlobalIds.") + return + wall_obj = tool.Ifc.get_object(wall) + if wall_obj is None: + self.report({"ERROR"}, "Wall has no Blender object.") + return + rel = tool.Wall.find_wall_slab_rel(wall, slab) + if rel is None: + self.report({"ERROR"}, "No TOP connection between this wall and slab.") + return + ifcopenshell.api.geometry.disconnect_element(ifc_file, relating_element=slab, related_element=wall) + core.regenerate_wall_to_underside(tool.Ifc, tool.Geometry, tool.Model, [wall_obj]) + + class ExtendWallsToUnderside(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Operator): bl_idname = "bim.extend_walls_to_underside" bl_label = "Extend Walls To Underside" diff --git a/src/bonsai/test/bim/module/model/test_disconnect_wall_slab.py b/src/bonsai/test/bim/module/model/test_disconnect_wall_slab.py new file mode 100644 index 0000000000..5c342be644 --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_disconnect_wall_slab.py @@ -0,0 +1,164 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Behaviour tests for ``bim.disconnect_wall_slab``. + +Pins the dispatch contract: resolves the wall + slab from GlobalIds, finds the +specific ``IfcRelConnectsElements(TOP)`` rel, removes it via the IFC API, then +delegates to ``core.regenerate_wall_to_underside`` to re-clip the wall against +any remaining slab connections.""" + +from unittest.mock import MagicMock, Mock, patch + +import pytest + +pytestmark = pytest.mark.model + + +def _make_op(*, wall_guid="WALL-GUID", slab_guid="SLAB-GUID"): + op = Mock() + op.wall_guid = wall_guid + op.slab_guid = slab_guid + op.report = Mock() + return op + + +def _ifc_file_with(*, walls: dict | None = None, slabs: dict | None = None): + ifc = MagicMock(name="ifc_file") + walls = walls or {} + slabs = slabs or {} + + def _by_guid(guid): + if guid in walls: + return walls[guid] + if guid in slabs: + return slabs[guid] + raise RuntimeError(f"no entity with guid {guid}") + + ifc.by_guid.side_effect = _by_guid + return ifc + + +def test_disconnect_removes_rel_then_regenerates(): + """Happy path: resolve both endpoints, find rel, call disconnect_element, + then regenerate so remaining slabs re-clip cleanly.""" + from bonsai.bim.module.model.wall import DisconnectWallSlab + + wall = Mock(name="wall") + slab = Mock(name="slab") + rel = Mock(name="rel") + wall_obj = Mock(name="wall_obj") + ifc_file = _ifc_file_with(walls={"WALL-GUID": wall}, slabs={"SLAB-GUID": slab}) + op = _make_op() + + with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( + "bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=wall_obj + ), patch("bonsai.bim.module.model.wall.tool.Wall.find_wall_slab_rel", return_value=rel), patch( + "bonsai.bim.module.model.wall.ifcopenshell.api.geometry.disconnect_element" + ) as disconnect, patch( + "bonsai.bim.module.model.wall.core.regenerate_wall_to_underside" + ) as regen: + DisconnectWallSlab._perform(op, context=MagicMock()) + + disconnect.assert_called_once_with(ifc_file, relating_element=slab, related_element=wall) + regen.assert_called_once() + args, _ = regen.call_args + assert args[3] == [wall_obj] + op.report.assert_not_called() + + +def test_disconnect_reports_when_guids_unknown(): + """Stale UI state can dispatch with guids no longer in the file — surface + an ERROR rather than crashing on RuntimeError from by_guid.""" + from bonsai.bim.module.model.wall import DisconnectWallSlab + + ifc_file = _ifc_file_with() + op = _make_op(wall_guid="MISSING", slab_guid="ALSO-MISSING") + + with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( + "bonsai.bim.module.model.wall.ifcopenshell.api.geometry.disconnect_element" + ) as disconnect, patch("bonsai.bim.module.model.wall.core.regenerate_wall_to_underside") as regen: + DisconnectWallSlab._perform(op, context=MagicMock()) + + disconnect.assert_not_called() + regen.assert_not_called() + op.report.assert_called_once() + args, _ = op.report.call_args + assert args[0] == {"ERROR"} + + +def test_disconnect_reports_when_rel_missing(): + """find_wall_slab_rel returns None when the rel doesn't exist (UI was + showing a stale icon). Operator reports + skips the mutation.""" + from bonsai.bim.module.model.wall import DisconnectWallSlab + + wall = Mock(name="wall") + slab = Mock(name="slab") + wall_obj = Mock(name="wall_obj") + ifc_file = _ifc_file_with(walls={"WALL-GUID": wall}, slabs={"SLAB-GUID": slab}) + op = _make_op() + + with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( + "bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=wall_obj + ), patch("bonsai.bim.module.model.wall.tool.Wall.find_wall_slab_rel", return_value=None), patch( + "bonsai.bim.module.model.wall.ifcopenshell.api.geometry.disconnect_element" + ) as disconnect, patch( + "bonsai.bim.module.model.wall.core.regenerate_wall_to_underside" + ) as regen: + DisconnectWallSlab._perform(op, context=MagicMock()) + + disconnect.assert_not_called() + regen.assert_not_called() + op.report.assert_called_once() + args, _ = op.report.call_args + assert args[0] == {"ERROR"} + + +def test_disconnect_reports_when_wall_obj_missing(): + """The wall entity exists but has no Blender object — surface ERROR + rather than silently no-op (or crash trying to pass None to regen).""" + from bonsai.bim.module.model.wall import DisconnectWallSlab + + wall = Mock(name="wall") + slab = Mock(name="slab") + ifc_file = _ifc_file_with(walls={"WALL-GUID": wall}, slabs={"SLAB-GUID": slab}) + op = _make_op() + + with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( + "bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=None + ), patch("bonsai.bim.module.model.wall.ifcopenshell.api.geometry.disconnect_element") as disconnect, patch( + "bonsai.bim.module.model.wall.core.regenerate_wall_to_underside" + ) as regen: + DisconnectWallSlab._perform(op, context=MagicMock()) + + disconnect.assert_not_called() + regen.assert_not_called() + op.report.assert_called_once() + + +def test_disconnect_operator_is_registered(): + """Catches a forgotten classes-tuple update — the operator file can be + saved cleanly but the class never reaches Blender's registry without + the __init__.py entry.""" + from bonsai.bim.module import model + + assert any( + getattr(cls, "bl_idname", None) == "bim.disconnect_wall_slab" for cls in model.classes + ), "DisconnectWallSlab is not in the model classes tuple" From a3593ed58b53859bd00b438282dcb7e7b43fb29b Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Fri, 12 Jun 2026 09:06:09 +0200 Subject: [PATCH 03/12] Unify wall disconnect ops via bim.disconnect_elements Single generic dispatcher replaces UnjoinWallPathConnection + DisconnectWallSlab. Takes two GlobalIds, looks up every supported rel between them via tool.Connection.find_rels, dispatches the right cleanup by rel kind: - path (IfcRelConnectsPathElements): remove_connection on every rel in both orientations + recreate both walls + resync drafts. - element-top (IfcRelConnectsElements with Description=="TOP"): disconnect_element + regenerate_wall_to_underside on the wall side via orient_element_top to recover which input is wall vs slab. - element (other IfcRelConnectsElements): plain disconnect_element. tool.Connection lands as a new tool module with two helpers: - find_rels(a, b): every supported rel between two elements, walking both ConnectedTo + ConnectedFrom (catches both authoring orientations and dedups by id). - find_rel(a, b): first-match convenience. - orient_element_top(rel, a, b): recovers (wall, slab) from a TOP rel regardless of which input came first. Updates GizmoWallUnjoinSingle to target bim.disconnect_elements with both element_a_guid + element_b_guid pre-filled per icon. Adds the single registration in tool/__init__.py and the classes-tuple entry in bim/module/model/__init__.py. Drops the two retired classes. Tests cover both cleanup branches (path + element-top), missing endpoints, no-rel-found, and registration smoke. Generated with the assistance of an AI coding tool. --- .../bonsai/bim/module/model/__init__.py | 3 +- src/bonsai/bonsai/bim/module/model/wall.py | 163 ++++------- src/bonsai/bonsai/tool/__init__.py | 1 + src/bonsai/bonsai/tool/connection.py | 113 ++++++++ .../module/model/test_disconnect_elements.py | 265 ++++++++++++++++++ .../module/model/test_disconnect_wall_slab.py | 164 ----------- 6 files changed, 442 insertions(+), 267 deletions(-) create mode 100644 src/bonsai/bonsai/tool/connection.py create mode 100644 src/bonsai/test/bim/module/model/test_disconnect_elements.py delete mode 100644 src/bonsai/test/bim/module/model/test_disconnect_wall_slab.py diff --git a/src/bonsai/bonsai/bim/module/model/__init__.py b/src/bonsai/bonsai/bim/module/model/__init__.py index 35922acf5e..4028520634 100644 --- a/src/bonsai/bonsai/bim/module/model/__init__.py +++ b/src/bonsai/bonsai/bim/module/model/__init__.py @@ -120,8 +120,7 @@ classes = ( wall.RotateWall90, wall.SplitWall, wall.SplitWallAtCursor, - wall.DisconnectWallSlab, - wall.UnjoinWallPathConnection, + wall.DisconnectElements, wall.UnjoinWalls, wall.EnableWallFilletPreview, wall.FinishWallFilletPreview, diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index e11f89940b..1058de7151 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -285,89 +285,29 @@ class UnjoinWalls(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Oper _resync_walls_after_mutation(tool.Blender.get_selected_objects()) -class UnjoinWallPathConnection(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Operator): - """Surgical counterpart to `UnjoinWalls`: disconnect the active wall from one - specific partner wall, leaving the active wall's other connections intact. The - partner is identified by IFC GlobalId — invariant under Blender-object renames, - file save/reload, and the undo stack — set on the operator properties by the - single-wall unjoin gizmo at click time.""" +class DisconnectElements(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Operator): + """Disconnect two IFC elements given their GlobalIds — generic dispatcher + that infers the connection rel kind via tool.Connection.find_rels and runs + the right post-disconnect cleanup: - bl_idname = "bim.unjoin_wall_path_connection" - bl_label = "Unjoin Wall Connection" - bl_description = "Disconnect the active wall from a single specific partner wall" + - ``"path"`` (IfcRelConnectsPathElements) → removes every rel between + the pair (catches both orientations) via remove_connection + recreates + both walls + resyncs drafts. + - ``"element-top"`` (IfcRelConnectsElements with Description=="TOP") → + disconnect_element + regenerate_wall_to_underside on the wall side. + - ``"element"`` (other IfcRelConnectsElements) → disconnect_element only. + + Both endpoints by GlobalId so the dispatch survives rename / undo / save. + Replaces the previous typed UnjoinWallPathConnection + DisconnectWallSlab + operators with one entry-point gizmos and shortcuts can bind to.""" + + bl_idname = "bim.disconnect_elements" + bl_label = "Disconnect Elements" + bl_description = "Remove the connection between two IFC elements identified by GlobalId" bl_options = {"REGISTER", "UNDO"} - other_wall_guid: bpy.props.StringProperty(name="Other Wall GlobalId") - - @classmethod - def poll(cls, context): - if not tool.Model.has_selected_ifc_objects(): - cls.poll_message_set("No IFC objects selected.") - return False - if _poll_reject_array_children(cls): - return False - return True - - def _perform(self, context): - active = tool.Blender.get_active_object(is_selected=True) - if not active: - self.report({"ERROR"}, "Could not resolve walls for surgical unjoin.") - return - elem_active = tool.Ifc.get_entity(active) - if not elem_active: - self.report({"ERROR"}, "Active object is not bound to an IFC entity.") - return - elem_other = None - if self.other_wall_guid: - try: - elem_other = tool.Ifc.get().by_guid(self.other_wall_guid) - except RuntimeError: - elem_other = None - other = tool.Ifc.get_object(elem_other) if elem_other else None - if not elem_other or not other: - self.report({"ERROR"}, "Could not resolve walls for surgical unjoin.") - return - # Walk the inverse graph for the specific IfcRelConnectsPathElements joining - # these two walls and remove only that one. `disconnect_path`'s - # (relating, related) mode only inspects `relating.ConnectedTo`, so a single - # call misses the rel when it was authored with the opposite orientation. - rels = [ - rel - for rel in getattr(elem_active, "ConnectedTo", []) - if rel.is_a("IfcRelConnectsPathElements") and rel.RelatedElement == elem_other - ] + [ - rel - for rel in getattr(elem_active, "ConnectedFrom", []) - if rel.is_a("IfcRelConnectsPathElements") and rel.RelatingElement == elem_other - ] - for rel in rels: - bonsai.core.geometry.remove_connection(tool.Geometry, connection=rel) - # Recreate body+axis on both walls so the mesh state matches the IFC mutation - # and stale miter cuts are dropped. - tool.Model.recreate_wall(elem_active, active) - tool.Model.recreate_wall(elem_other, other) - _resync_walls_after_mutation([active, other]) - - -class DisconnectWallSlab(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Operator): - """Disconnect the wall from one specific underside slab — counterpart to - UnjoinWallPathConnection on the wall-slab side. Both endpoints are - identified by IFC GlobalId so the dispatch survives rename / undo / save. - - Drops the IfcRelConnectsElements(TOP) rel + all underside booleans on the - wall, then re-runs regenerate_wall_to_underside which re-clips the wall - to whatever slabs remain connected. The all-booleans-then-regenerate - approach is safe with HEAD's flat BBIM_Boolean pset (no per-slab id - storage); switches to a per-slab boolean removal when PR #8147's - dict-with-slab-guid pset migration lands.""" - - bl_idname = "bim.disconnect_wall_slab" - bl_label = "Disconnect Wall From Slab" - bl_description = "Remove the TOP connection between a wall and one slab and re-clip the wall to remaining slabs" - bl_options = {"REGISTER", "UNDO"} - - wall_guid: bpy.props.StringProperty(name="Wall GlobalId") - slab_guid: bpy.props.StringProperty(name="Slab GlobalId") + element_a_guid: bpy.props.StringProperty(name="Element A GlobalId") + element_b_guid: bpy.props.StringProperty(name="Element B GlobalId") @classmethod def poll(cls, context): @@ -381,23 +321,42 @@ class DisconnectWallSlab(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.I def _perform(self, context): ifc_file = tool.Ifc.get() try: - wall = ifc_file.by_guid(self.wall_guid) if self.wall_guid else None - slab = ifc_file.by_guid(self.slab_guid) if self.slab_guid else None + elem_a = ifc_file.by_guid(self.element_a_guid) if self.element_a_guid else None + elem_b = ifc_file.by_guid(self.element_b_guid) if self.element_b_guid else None except RuntimeError: - wall = slab = None - if wall is None or slab is None: - self.report({"ERROR"}, "Could not resolve wall and slab from supplied GlobalIds.") + elem_a = elem_b = None + if elem_a is None or elem_b is None: + self.report({"ERROR"}, "Could not resolve elements from supplied GlobalIds.") return - wall_obj = tool.Ifc.get_object(wall) - if wall_obj is None: - self.report({"ERROR"}, "Wall has no Blender object.") + rels = tool.Connection.find_rels(elem_a, elem_b) + if not rels: + self.report({"ERROR"}, "No connection found between elements.") return - rel = tool.Wall.find_wall_slab_rel(wall, slab) - if rel is None: - self.report({"ERROR"}, "No TOP connection between this wall and slab.") - return - ifcopenshell.api.geometry.disconnect_element(ifc_file, relating_element=slab, related_element=wall) - core.regenerate_wall_to_underside(tool.Ifc, tool.Geometry, tool.Model, [wall_obj]) + # All rels between a single pair should share a kind in practice; pick + # the first kind for the cleanup dispatch and remove every rel below. + kind = rels[0][1] + if kind == "path": + for rel, _ in rels: + bonsai.core.geometry.remove_connection(tool.Geometry, connection=rel) + obj_a = tool.Ifc.get_object(elem_a) + obj_b = tool.Ifc.get_object(elem_b) + if obj_a is not None and obj_b is not None: + tool.Model.recreate_wall(elem_a, obj_a) + tool.Model.recreate_wall(elem_b, obj_b) + _resync_walls_after_mutation([obj_a, obj_b]) + elif kind in ("element-top", "element"): + for rel, _ in rels: + wall, slab = tool.Connection.orient_element_top(rel, elem_a, elem_b) + ifcopenshell.api.geometry.disconnect_element( + ifc_file, relating_element=slab, related_element=wall + ) + if kind == "element-top": + # The TOP rel is what extend_walls_to_underside creates; the + # related side is always the wall. + wall = rels[0][0].RelatedElement + wall_obj = tool.Ifc.get_object(wall) + if wall_obj is not None: + core.regenerate_wall_to_underside(tool.Ifc, tool.Geometry, tool.Model, [wall_obj]) class ExtendWallsToUnderside(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Operator): @@ -3912,9 +3871,10 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix by end, plus unlimited ATPATH T-junctions), so a pool of icons is preallocated and hidden on a per-frame basis based on the live connection set. - Each visible icon dispatches `bim.unjoin_wall_path_connection` with the partner - wall's GlobalId set on the bound operator properties, so a click removes only - the single rel under that icon — the other connections on the same wall survive. + Each visible icon dispatches `bim.disconnect_elements` with the active wall + + partner wall GlobalIds set on the bound operator properties, so a click removes + only the single rel under that icon — the other connections on the same wall + survive. Mutually exclusive with `GizmoWallJoinIntersection` via `poll()` (that group requires len(selected) == 2; this one requires 1).""" @@ -3962,11 +3922,11 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix self.unjoin_op_props = [] for _ in range(self.POOL_SIZE): icon = self.setup_icon_gizmo( - "VIEW3D_GT_wall_link_toggle", default_color, highlight_color, "bim.unjoin_wall_path_connection" + "VIEW3D_GT_wall_link_toggle", default_color, highlight_color, "bim.disconnect_elements" ) icon.hide = True self.unjoin_icons.append(icon) - self.unjoin_op_props.append(icon.target_set_operator("bim.unjoin_wall_path_connection")) + self.unjoin_op_props.append(icon.target_set_operator("bim.disconnect_elements")) def position_gizmos(self, context: bpy.types.Context) -> None: # Default: hide every pool slot. The visible-set is rebuilt from the live @@ -4009,12 +3969,13 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix icon = self.unjoin_icons[slot_idx] icon.matrix_basis = gizmo.billboarded_at(location + clearance, billboard_rot, scale=self.ICON_SCALE) icon.hide = False - # Only the partner-GlobalId property is rewritten per frame; the operator + # Only the GlobalId properties are rewritten per frame; the operator # binding itself is the long-lived handle set up at setup() time. GlobalId # (not Blender object name) keeps the binding stable across renames, file # save/reload, and any sit-in-the-undo-stack interlude between dispatch # and execute. - self.unjoin_op_props[slot_idx].other_wall_guid = other_elem.GlobalId + self.unjoin_op_props[slot_idx].element_a_guid = elem.GlobalId + self.unjoin_op_props[slot_idx].element_b_guid = other_elem.GlobalId # Mirror the partner reference onto the icon itself so its draw() # can outline the partner on hover without a Gizmo-side getter on # the bound operator (the API exposes target_set_operator with diff --git a/src/bonsai/bonsai/tool/__init__.py b/src/bonsai/bonsai/tool/__init__.py index 03716236e1..afdec36b84 100644 --- a/src/bonsai/bonsai/tool/__init__.py +++ b/src/bonsai/bonsai/tool/__init__.py @@ -31,6 +31,7 @@ from bonsai.tool.cad import Cad from bonsai.tool.clash import Clash from bonsai.tool.classification import Classification from bonsai.tool.collector import Collector +from bonsai.tool.connection import Connection from bonsai.tool.context import Context from bonsai.tool.cost import Cost from bonsai.tool.covering import Covering diff --git a/src/bonsai/bonsai/tool/connection.py b/src/bonsai/bonsai/tool/connection.py new file mode 100644 index 0000000000..055eb47a68 --- /dev/null +++ b/src/bonsai/bonsai/tool/connection.py @@ -0,0 +1,113 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Generic discovery of the relation linking two IFC elements. + +Used by ``bim.disconnect_elements`` so the operator surface is one operator +per disconnect intent (active vs. partner, identified by GlobalId) rather +than one per rel class. The kind label returned alongside the rel lets the +operator dispatch the right post-disconnect cleanup: + +- ``"path"`` for ``IfcRelConnectsPathElements`` (wall-wall, wall-roof, etc.) +- ``"element-top"`` for ``IfcRelConnectsElements`` with ``Description=="TOP"`` + (the rel kind ``extend_walls_to_underside`` creates) +- ``"element"`` for any other ``IfcRelConnectsElements`` + +Add new rel kinds by extending :py:meth:`Connection.find_rel`. The disconnect +operator's cleanup switch maps each kind to the right post-mutation calls.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + import ifcopenshell + + +class Connection: + @classmethod + def find_rels( + cls, + elem_a: "ifcopenshell.entity_instance", + elem_b: "ifcopenshell.entity_instance", + ) -> "list[tuple[ifcopenshell.entity_instance, str]]": + """Return every supported rel linking ``elem_a`` to ``elem_b`` as a + list of ``(rel, kind)`` tuples. Walks both ``ConnectedTo`` and + ``ConnectedFrom`` because either side of the rel can be the relating + element, and the same pair may carry rels authored with opposite + orientations (``disconnect_path``'s ``(relating, related)`` mode only + inspects ``relating.ConnectedTo``, so a single call would miss the + opposite-orientation rel).""" + rels: list[tuple[ifcopenshell.entity_instance, str]] = [] + seen: set[int] = set() + + def _record(rel, kind): + if rel.id() not in seen: + seen.add(rel.id()) + rels.append((rel, kind)) + + for rel in getattr(elem_a, "ConnectedTo", []) or (): + if rel.is_a("IfcRelConnectsPathElements") and getattr(rel, "RelatedElement", None) == elem_b: + _record(rel, "path") + for rel in getattr(elem_a, "ConnectedFrom", []) or (): + if rel.is_a("IfcRelConnectsPathElements") and getattr(rel, "RelatingElement", None) == elem_b: + _record(rel, "path") + + for rel in getattr(elem_a, "ConnectedFrom", []) or (): + if rel.is_a("IfcRelConnectsElements") and getattr(rel, "RelatingElement", None) == elem_b: + kind = "element-top" if getattr(rel, "Description", None) == "TOP" else "element" + _record(rel, kind) + for rel in getattr(elem_a, "ConnectedTo", []) or (): + if rel.is_a("IfcRelConnectsElements") and getattr(rel, "RelatedElement", None) == elem_b: + kind = "element-top" if getattr(rel, "Description", None) == "TOP" else "element" + _record(rel, kind) + + return rels + + @classmethod + def find_rel( + cls, + elem_a: "ifcopenshell.entity_instance", + elem_b: "ifcopenshell.entity_instance", + ) -> "tuple[ifcopenshell.entity_instance | None, str | None]": + """Return the first ``(rel, kind)`` or ``(None, None)``. Cheaper than + ``find_rels`` when callers only need to know whether a connection + exists or what kind it is.""" + rels = cls.find_rels(elem_a, elem_b) + return rels[0] if rels else (None, None) + + @classmethod + def orient_element_top( + cls, + rel: "ifcopenshell.entity_instance", + elem_a: "ifcopenshell.entity_instance", + elem_b: "ifcopenshell.entity_instance", + ) -> "tuple[ifcopenshell.entity_instance, ifcopenshell.entity_instance]": + """Return ``(wall, slab)`` for an ``IfcRelConnectsElements(TOP)`` rel. + + The ``extend_walls_to_underside`` flow stores slab as the relating + side and wall as related — orientation is recovered by checking + which input matches which rel attribute. Callers pass any two + elements; this resolves which is the wall and which is the slab so + post-disconnect cleanup (regenerate-wall-to-underside) targets the + right object.""" + if getattr(rel, "RelatingElement", None) == elem_a: + return elem_b, elem_a + return elem_a, elem_b diff --git a/src/bonsai/test/bim/module/model/test_disconnect_elements.py b/src/bonsai/test/bim/module/model/test_disconnect_elements.py new file mode 100644 index 0000000000..02464e1625 --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_disconnect_elements.py @@ -0,0 +1,265 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Behaviour tests for the unified ``bim.disconnect_elements`` operator and +``tool.Connection.find_rels`` registry. + +Pin the dispatch contract: rels are found in either orientation; the kind +label drives cleanup (``path`` recreates both walls + resyncs drafts; +``element-top`` runs ``regenerate_wall_to_underside``); missing endpoints +report ERROR rather than crashing.""" + +from unittest.mock import MagicMock, Mock, patch + +import pytest + +import bonsai.tool as tool + +pytestmark = pytest.mark.model + + +def _rel(klass: str, *, relating=None, related=None, description=None, rel_id: int = 0): + rel = Mock() + rel.is_a = lambda c: c == klass + rel.RelatingElement = relating + rel.RelatedElement = related + rel.Description = description + rel.id = lambda: rel_id + return rel + + +def _elem(*, connected_to=(), connected_from=()): + e = Mock() + e.ConnectedTo = list(connected_to) + e.ConnectedFrom = list(connected_from) + e.GlobalId = "GUID" + return e + + +# --------------------------------------------------------------------------- +# tool.Connection.find_rels — registry behaviour +# --------------------------------------------------------------------------- + + +def test_find_rels_returns_path_rel_in_either_orientation(): + """The same wall pair can carry path rels authored with either orientation; + find_rels must catch both.""" + elem_a = _elem() + elem_b = _elem() + rel_ab = _rel("IfcRelConnectsPathElements", related=elem_b, rel_id=1) + rel_ba = _rel("IfcRelConnectsPathElements", relating=elem_b, rel_id=2) + elem_a.ConnectedTo = [rel_ab] + elem_a.ConnectedFrom = [rel_ba] + + rels = tool.Connection.find_rels(elem_a, elem_b) + + assert {r.id() for r, _ in rels} == {1, 2} + assert all(k == "path" for _, k in rels) + + +def test_find_rels_classifies_top_element_rel_specifically(): + """IfcRelConnectsElements with Description=='TOP' is the rel kind + extend_walls_to_underside creates. Tag it ``element-top`` so the + operator can dispatch the regenerate-wall-to-underside cleanup.""" + wall = _elem() + slab = _elem() + rel = _rel("IfcRelConnectsElements", relating=slab, description="TOP", rel_id=1) + wall.ConnectedFrom = [rel] + + rels = tool.Connection.find_rels(wall, slab) + + assert rels == [(rel, "element-top")] + + +def test_find_rels_classifies_non_top_element_rel_generically(): + """Other IfcRelConnectsElements descriptions don't get the TOP-specific + cleanup. Tag as plain ``element`` so the operator just removes the rel.""" + elem_a = _elem() + elem_b = _elem() + rel = _rel("IfcRelConnectsElements", relating=elem_b, description="ATTACHMENT", rel_id=1) + elem_a.ConnectedFrom = [rel] + + rels = tool.Connection.find_rels(elem_a, elem_b) + + assert rels == [(rel, "element")] + + +def test_find_rels_returns_empty_when_disconnected(): + elem_a = _elem() + elem_b = _elem() + assert tool.Connection.find_rels(elem_a, elem_b) == [] + + +def test_find_rels_dedups_by_id(): + """A rel that surfaces on both ConnectedTo and ConnectedFrom (in + pathological IFC files) should not be returned twice.""" + elem_a = _elem() + elem_b = _elem() + rel = _rel("IfcRelConnectsPathElements", related=elem_b, relating=elem_b, rel_id=1) + elem_a.ConnectedTo = [rel] + elem_a.ConnectedFrom = [rel] + + rels = tool.Connection.find_rels(elem_a, elem_b) + + assert len(rels) == 1 + + +# --------------------------------------------------------------------------- +# tool.Connection.find_rel — first-match convenience +# --------------------------------------------------------------------------- + + +def test_find_rel_returns_first_match_or_none_none(): + elem_a = _elem() + elem_b = _elem() + rel = _rel("IfcRelConnectsPathElements", related=elem_b, rel_id=1) + elem_a.ConnectedTo = [rel] + + assert tool.Connection.find_rel(elem_a, elem_b) == (rel, "path") + assert tool.Connection.find_rel(elem_a, _elem()) == (None, None) + + +# --------------------------------------------------------------------------- +# tool.Connection.orient_element_top — wall / slab orientation recovery +# --------------------------------------------------------------------------- + + +def test_orient_element_top_returns_wall_then_slab(): + """The TOP rel stores slab as relating + wall as related; orient_element_top + figures out which input is which regardless of argument order.""" + wall = _elem() + slab = _elem() + rel = _rel("IfcRelConnectsElements", relating=slab, related=wall, description="TOP") + + assert tool.Connection.orient_element_top(rel, wall, slab) == (wall, slab) + assert tool.Connection.orient_element_top(rel, slab, wall) == (wall, slab) + + +# --------------------------------------------------------------------------- +# bim.disconnect_elements — dispatch + cleanup +# --------------------------------------------------------------------------- + + +def _make_op(*, a_guid="A", b_guid="B"): + op = Mock() + op.element_a_guid = a_guid + op.element_b_guid = b_guid + op.report = Mock() + return op + + +def test_disconnect_path_removes_all_rels_then_recreates_walls(): + from bonsai.bim.module.model.wall import DisconnectElements + + elem_a = Mock() + elem_b = Mock() + rel1 = Mock() + rel2 = Mock() + obj_a = Mock() + obj_b = Mock() + + ifc_file = MagicMock() + ifc_file.by_guid.side_effect = lambda g: {"A": elem_a, "B": elem_b}[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=[(rel1, "path"), (rel2, "path")], + ), patch( + "bonsai.bim.module.model.wall.tool.Ifc.get_object", side_effect=lambda e: {elem_a: obj_a, elem_b: obj_b}[e] + ), patch("bonsai.bim.module.model.wall.bonsai.core.geometry.remove_connection") as remove, patch( + "bonsai.bim.module.model.wall.tool.Model.recreate_wall" + ) as recreate, patch("bonsai.bim.module.model.wall._resync_walls_after_mutation") as resync: + DisconnectElements._perform(op, context=MagicMock()) + + assert remove.call_count == 2 + assert recreate.call_count == 2 + resync.assert_called_once_with([obj_a, obj_b]) + op.report.assert_not_called() + + +def test_disconnect_element_top_calls_regenerate(): + from bonsai.bim.module.model.wall import DisconnectElements + + wall = Mock() + slab = Mock() + wall_obj = Mock() + rel = Mock() + rel.RelatedElement = wall + + ifc_file = MagicMock() + ifc_file.by_guid.side_effect = lambda g: {"A": wall, "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.Connection.orient_element_top", return_value=(wall, slab) + ), patch("bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=wall_obj), patch( + "bonsai.bim.module.model.wall.ifcopenshell.api.geometry.disconnect_element" + ) as disc, patch("bonsai.bim.module.model.wall.core.regenerate_wall_to_underside") as regen: + DisconnectElements._perform(op, context=MagicMock()) + + disc.assert_called_once_with(ifc_file, relating_element=slab, related_element=wall) + regen.assert_called_once() + op.report.assert_not_called() + + +def test_disconnect_reports_on_unknown_guids(): + from bonsai.bim.module.model.wall import DisconnectElements + + ifc_file = MagicMock() + ifc_file.by_guid.side_effect = RuntimeError("missing") + op = _make_op(a_guid="MISSING_A", b_guid="MISSING_B") + + with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( + "bonsai.bim.module.model.wall.tool.Connection.find_rels" + ) as find: + DisconnectElements._perform(op, context=MagicMock()) + + find.assert_not_called() + op.report.assert_called_once() + args, _ = op.report.call_args + assert args[0] == {"ERROR"} + + +def test_disconnect_reports_when_no_rel_found(): + from bonsai.bim.module.model.wall import DisconnectElements + + elem_a = Mock() + elem_b = Mock() + ifc_file = MagicMock() + ifc_file.by_guid.side_effect = lambda g: {"A": elem_a, "B": elem_b}[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=[] + ): + DisconnectElements._perform(op, context=MagicMock()) + + op.report.assert_called_once() + + +def test_disconnect_operator_is_registered(): + from bonsai.bim.module import model + + assert any( + getattr(cls, "bl_idname", None) == "bim.disconnect_elements" for cls in model.classes + ), "DisconnectElements is not in the model classes tuple" diff --git a/src/bonsai/test/bim/module/model/test_disconnect_wall_slab.py b/src/bonsai/test/bim/module/model/test_disconnect_wall_slab.py deleted file mode 100644 index 5c342be644..0000000000 --- a/src/bonsai/test/bim/module/model/test_disconnect_wall_slab.py +++ /dev/null @@ -1,164 +0,0 @@ -# Bonsai - OpenBIM Blender Add-on -# Copyright (C) 2026 -# -# This file is part of Bonsai. -# -# Bonsai is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Bonsai is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Bonsai. If not, see . -# -# This file was generated with the assistance of an AI coding tool. - -"""Behaviour tests for ``bim.disconnect_wall_slab``. - -Pins the dispatch contract: resolves the wall + slab from GlobalIds, finds the -specific ``IfcRelConnectsElements(TOP)`` rel, removes it via the IFC API, then -delegates to ``core.regenerate_wall_to_underside`` to re-clip the wall against -any remaining slab connections.""" - -from unittest.mock import MagicMock, Mock, patch - -import pytest - -pytestmark = pytest.mark.model - - -def _make_op(*, wall_guid="WALL-GUID", slab_guid="SLAB-GUID"): - op = Mock() - op.wall_guid = wall_guid - op.slab_guid = slab_guid - op.report = Mock() - return op - - -def _ifc_file_with(*, walls: dict | None = None, slabs: dict | None = None): - ifc = MagicMock(name="ifc_file") - walls = walls or {} - slabs = slabs or {} - - def _by_guid(guid): - if guid in walls: - return walls[guid] - if guid in slabs: - return slabs[guid] - raise RuntimeError(f"no entity with guid {guid}") - - ifc.by_guid.side_effect = _by_guid - return ifc - - -def test_disconnect_removes_rel_then_regenerates(): - """Happy path: resolve both endpoints, find rel, call disconnect_element, - then regenerate so remaining slabs re-clip cleanly.""" - from bonsai.bim.module.model.wall import DisconnectWallSlab - - wall = Mock(name="wall") - slab = Mock(name="slab") - rel = Mock(name="rel") - wall_obj = Mock(name="wall_obj") - ifc_file = _ifc_file_with(walls={"WALL-GUID": wall}, slabs={"SLAB-GUID": slab}) - op = _make_op() - - with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( - "bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=wall_obj - ), patch("bonsai.bim.module.model.wall.tool.Wall.find_wall_slab_rel", return_value=rel), patch( - "bonsai.bim.module.model.wall.ifcopenshell.api.geometry.disconnect_element" - ) as disconnect, patch( - "bonsai.bim.module.model.wall.core.regenerate_wall_to_underside" - ) as regen: - DisconnectWallSlab._perform(op, context=MagicMock()) - - disconnect.assert_called_once_with(ifc_file, relating_element=slab, related_element=wall) - regen.assert_called_once() - args, _ = regen.call_args - assert args[3] == [wall_obj] - op.report.assert_not_called() - - -def test_disconnect_reports_when_guids_unknown(): - """Stale UI state can dispatch with guids no longer in the file — surface - an ERROR rather than crashing on RuntimeError from by_guid.""" - from bonsai.bim.module.model.wall import DisconnectWallSlab - - ifc_file = _ifc_file_with() - op = _make_op(wall_guid="MISSING", slab_guid="ALSO-MISSING") - - with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( - "bonsai.bim.module.model.wall.ifcopenshell.api.geometry.disconnect_element" - ) as disconnect, patch("bonsai.bim.module.model.wall.core.regenerate_wall_to_underside") as regen: - DisconnectWallSlab._perform(op, context=MagicMock()) - - disconnect.assert_not_called() - regen.assert_not_called() - op.report.assert_called_once() - args, _ = op.report.call_args - assert args[0] == {"ERROR"} - - -def test_disconnect_reports_when_rel_missing(): - """find_wall_slab_rel returns None when the rel doesn't exist (UI was - showing a stale icon). Operator reports + skips the mutation.""" - from bonsai.bim.module.model.wall import DisconnectWallSlab - - wall = Mock(name="wall") - slab = Mock(name="slab") - wall_obj = Mock(name="wall_obj") - ifc_file = _ifc_file_with(walls={"WALL-GUID": wall}, slabs={"SLAB-GUID": slab}) - op = _make_op() - - with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( - "bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=wall_obj - ), patch("bonsai.bim.module.model.wall.tool.Wall.find_wall_slab_rel", return_value=None), patch( - "bonsai.bim.module.model.wall.ifcopenshell.api.geometry.disconnect_element" - ) as disconnect, patch( - "bonsai.bim.module.model.wall.core.regenerate_wall_to_underside" - ) as regen: - DisconnectWallSlab._perform(op, context=MagicMock()) - - disconnect.assert_not_called() - regen.assert_not_called() - op.report.assert_called_once() - args, _ = op.report.call_args - assert args[0] == {"ERROR"} - - -def test_disconnect_reports_when_wall_obj_missing(): - """The wall entity exists but has no Blender object — surface ERROR - rather than silently no-op (or crash trying to pass None to regen).""" - from bonsai.bim.module.model.wall import DisconnectWallSlab - - wall = Mock(name="wall") - slab = Mock(name="slab") - ifc_file = _ifc_file_with(walls={"WALL-GUID": wall}, slabs={"SLAB-GUID": slab}) - op = _make_op() - - with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( - "bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=None - ), patch("bonsai.bim.module.model.wall.ifcopenshell.api.geometry.disconnect_element") as disconnect, patch( - "bonsai.bim.module.model.wall.core.regenerate_wall_to_underside" - ) as regen: - DisconnectWallSlab._perform(op, context=MagicMock()) - - disconnect.assert_not_called() - regen.assert_not_called() - op.report.assert_called_once() - - -def test_disconnect_operator_is_registered(): - """Catches a forgotten classes-tuple update — the operator file can be - saved cleanly but the class never reaches Blender's registry without - the __init__.py entry.""" - from bonsai.bim.module import model - - assert any( - getattr(cls, "bl_idname", None) == "bim.disconnect_wall_slab" for cls in model.classes - ), "DisconnectWallSlab is not in the model classes tuple" From c7d5d6c498982dcf3e8729afddb13cd1804c84be Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Fri, 12 Jun 2026 11:05:02 +0200 Subject: [PATCH 04/12] Gate slab disconnect gizmos behind parametric edit lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires slabs into the parametric edit framework (tool.Parametric .EDIT_TYPES) so the wall-slab disconnect UI gets ESC handling, red cancel icon, mutual exclusion with other parametric edits, and per-feature gizmo prefs — all from BaseParametricGizmoGroup — without duplicating the lifecycle. Adds: - ParametricObject("slab") registry entry + tool.Parametric.is_slab predicate (any IfcSlab). - BIMSlabProperties with is_editing flag; PointerProperty wired by the framework's register_object_properties. - bim.enable_editing_slab / bim.finish_editing_slab / bim.cancel_editing_slab operators on tool.Ifc.Operator so they flow through tool.Parametric.run_bim_op cleanly. No IFC mutation — slab edit is a pure UI gate; finish and cancel share the body. - tool.Model.get_slab_props accessor. - GizmoSlabEdition inheriting BaseParametricGizmoGroup with the pen / validate / cancel triad. is_element_type narrows to IfcSlab with at least one wall clipped to its underside. The disconnect-icon group GizmoSlabUnjoinWalls polls behind _slab_connection_gizmo_poll_gate(require_editing=True), which now reads is_editing through tool.Model.get_slab_props. Drops the standalone GizmoSlabConnectionAccess + the setup_pen_cancel_icons helper added earlier in this branch — both superseded by the framework integration. Also folds in the wall + multi-slab gizmo polish requested live: - Wall side: stack the per-slab unjoin icons vertically (up to 5) so multi-slab connections each get a distinct clickable icon; hover-highlight reveals which slab will disconnect. - GizmoPairDisconnect activates when 2 elements with an IfcRelConnectsElements(TOP) rel are selected, with the icon at the wall-slab connection world anchor. - Wall-slab anchor moved from slab clip Z to wall top + WALL_SLAB_CONNECTION_Z_CLEARANCE so the disconnect icon perches above the extend-vertical / slope gizmo instead of overlapping. - Shared _resolve_active_partner_pair helper for 2-selection gizmos; _slab_connection_gizmo_poll_gate added to _REQUIRED_CALLEES + GizmoSlabEdition added to the AST forward-compat allowlist. Build note: wall.py's DisconnectElements._perform imports bonsai.core.connection.disconnect_rel — that core module is being added in a parallel-session commit. Until that lands the addon import will fail. Generated with the assistance of an AI coding tool. --- .../bonsai/bim/module/model/__init__.py | 6 + src/bonsai/bonsai/bim/module/model/prop.py | 15 + src/bonsai/bonsai/bim/module/model/slab.py | 71 ++++ src/bonsai/bonsai/bim/module/model/wall.py | 349 +++++++++++++++--- src/bonsai/bonsai/tool/model.py | 5 + src/bonsai/bonsai/tool/parametric.py | 11 + src/bonsai/bonsai/tool/wall.py | 29 +- ..._wall_array_child_filter_forward_compat.py | 8 +- .../model/test_wall_slab_connections.py | 32 +- 9 files changed, 452 insertions(+), 74 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/model/__init__.py b/src/bonsai/bonsai/bim/module/model/__init__.py index 4028520634..6583144c89 100644 --- a/src/bonsai/bonsai/bim/module/model/__init__.py +++ b/src/bonsai/bonsai/bim/module/model/__init__.py @@ -110,6 +110,8 @@ classes = ( wall.GizmoWallFilletPreview, wall.GizmoWallFilletReedit, wall.GizmoWallFilletToggleOpenings, + wall.GizmoSlabEdition, + wall.GizmoSlabUnjoinWalls, wall.GizmoWallJoinIntersection, wall.GizmoWallLinkToggle, wall.GizmoWallUnjoinSingle, @@ -154,11 +156,14 @@ classes = ( slab.DisableEditingExtrusionProfile, slab.DisableEditingSketchExtrusionProfile, slab.AddSlabFromWall, + slab.CancelEditingSlab, slab.DrawPolylineSlab, slab.EditExtrusionProfile, slab.EditSketchExtrusionProfile, slab.EnableEditingExtrusionProfile, slab.EnableEditingSketchExtrusionProfile, + slab.EnableEditingSlab, + slab.FinishEditingSlab, slab.RecalculateSlab, slab.ResetVertex, slab.SetArcIndex, @@ -185,6 +190,7 @@ classes = ( prop.BIMDoorProperties, prop.BIMRailingProperties, prop.BIMRoofProperties, + prop.BIMSlabProperties, prop.BIMWallProperties, prop.BIMPipeSegmentProperties, prop.BIMDuctSegmentProperties, diff --git a/src/bonsai/bonsai/bim/module/model/prop.py b/src/bonsai/bonsai/bim/module/model/prop.py index ebafafda6d..e2f8a2a9a8 100644 --- a/src/bonsai/bonsai/bim/module/model/prop.py +++ b/src/bonsai/bonsai/bim/module/model/prop.py @@ -1689,6 +1689,21 @@ class BIMRoofProperties(PropertyGroup): setattr(target_props, prop_name, prop_value) +class BIMSlabProperties(PropertyGroup): + """Transient state for the slab disconnect-access gizmo. + + ``is_editing`` flips True when the user clicks the pen icon on a slab + that has wall connections — gating the per-wall disconnect icons in + ``GizmoSlabUnjoinWalls`` so they're hidden until the user opts in. No + IFC draft state lives here: the disconnect operator commits directly, + so this PropertyGroup carries only the UI gate.""" + + is_editing: bpy.props.BoolProperty(name="Slab Edit Active", default=False, options={"SKIP_SAVE"}) + + if TYPE_CHECKING: + is_editing: bool + + class BIMWallProperties(PropertyGroup): """Transient draft state for parametric wall gizmo editing. diff --git a/src/bonsai/bonsai/bim/module/model/slab.py b/src/bonsai/bonsai/bim/module/model/slab.py index 58a353ab28..516dd04233 100644 --- a/src/bonsai/bonsai/bim/module/model/slab.py +++ b/src/bonsai/bonsai/bim/module/model/slab.py @@ -991,3 +991,74 @@ class RecalculateSlab(bpy.types.Operator, tool.Ifc.Operator): tool.Model.recalculate_walls(walls) return {"FINISHED"} + + +class EnableEditingSlab(bpy.types.Operator, tool.Ifc.Operator): + """Open the slab disconnect-access mode. Pure UI toggle: flips + ``obj.BIMSlabProperties.is_editing`` so the per-wall disconnect + gizmos surface on the slab. ``tool.Ifc.Operator`` base because the + parametric framework's universal dispatcher routes through + ``tool.Parametric.run_bim_op``, which only accepts that subclass for + undo-safe lifecycle. No IFC mutation.""" + + bl_idname = "bim.enable_editing_slab" + bl_label = "Edit Slab Connections" + bl_description = "Show disconnect icons for every wall clipped to this slab" + bl_options = {"REGISTER", "UNDO"} + + @classmethod + def poll(cls, context): + obj = context.active_object + if obj is None: + return False + element = tool.Ifc.get_entity(obj) + return element is not None and element.is_a("IfcSlab") + + def _execute(self, context): + context.active_object.BIMSlabProperties.is_editing = True + return {"FINISHED"} + + +class CancelEditingSlab(bpy.types.Operator, tool.Ifc.Operator): + """Close the slab disconnect-access mode.""" + + bl_idname = "bim.cancel_editing_slab" + bl_label = "Close Slab Edit" + bl_description = "Hide the slab disconnect icons" + bl_options = {"REGISTER", "UNDO"} + + @classmethod + def poll(cls, context): + obj = context.active_object + if obj is None: + return False + element = tool.Ifc.get_entity(obj) + return element is not None and element.is_a("IfcSlab") + + def _execute(self, context): + context.active_object.BIMSlabProperties.is_editing = False + return {"FINISHED"} + + +class FinishEditingSlab(bpy.types.Operator, tool.Ifc.Operator): + """Close the slab disconnect-access mode. Same body as Cancel — slab + edit is a pure UI gate with no IFC draft to commit; the framework + requires both ``bim.finish_editing_`` and + ``bim.cancel_editing_`` to exist by name convention.""" + + bl_idname = "bim.finish_editing_slab" + bl_label = "Finish Slab Edit" + bl_description = "Hide the slab disconnect icons" + bl_options = {"REGISTER", "UNDO"} + + @classmethod + def poll(cls, context): + obj = context.active_object + if obj is None: + return False + element = tool.Ifc.get_entity(obj) + return element is not None and element.is_a("IfcSlab") + + def _execute(self, context): + context.active_object.BIMSlabProperties.is_editing = False + return {"FINISHED"} diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index 1058de7151..33443b8fdd 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -48,6 +48,7 @@ import mathutils.geometry import numpy as np from mathutils import Matrix, Vector +import bonsai.core.connection import bonsai.core.geometry import bonsai.core.model as core import bonsai.core.root @@ -108,6 +109,50 @@ def _wall_gizmo_poll_gate(context: bpy.types.Context) -> bool: return True +def _resolve_active_partner_pair( + context: bpy.types.Context, +) -> "tuple[bpy.types.Object, bpy.types.Object, ifcopenshell.entity_instance, ifcopenshell.entity_instance] | None": + """Return ``(active_obj, partner_obj, active_elem, partner_elem)`` for a + selection of exactly two IFC-bound objects with the active one named, + else ``None``. Used by every 2-selection gizmo to skip the standard + "resolve active + partner + IFC entities" preamble.""" + active = tool.Blender.get_active_object(is_selected=True) + if active is None: + return None + selected = list(tool.Blender.get_selected_objects()) + if len(selected) != 2: + return None + partner = next((o for o in selected if o != active), None) + if partner is None: + return None + active_elem = tool.Ifc.get_entity(active) + partner_elem = tool.Ifc.get_entity(partner) + if active_elem is None or partner_elem is None: + return None + return active, partner, active_elem, partner_elem + + +def _slab_connection_gizmo_poll_gate(context: bpy.types.Context, *, require_editing: bool = False) -> bool: + """Shared gate for slab-side connection gizmos: exactly 1 IfcSlab + selected, not an array child, has at least one wall clipped to its + underside. With ``require_editing=True`` additionally requires the + slab's parametric edit lifecycle to be active (pen icon clicked) so + the gizmo only surfaces after explicit opt-in.""" + active = tool.Blender.get_active_object(is_selected=True) + if active is None: + return False + if len(tool.Blender.get_selected_objects()) != 1: + return False + element = tool.Ifc.get_entity(active) + if element is None or not element.is_a("IfcSlab"): + return False + if tool.Blender.Modifier.any_selected_is_array_child(): + return False + if require_editing and not tool.Model.get_slab_props(active).is_editing: + return False + return any(True for _ in tool.Wall.iter_slab_wall_connections(element)) + + def _wall_topology_gizmo_poll_gate(context: bpy.types.Context) -> bool: """Tighter gate for wall topology gizmos (merge / join / extend / unjoin / fillet): base ``_wall_gizmo_poll_gate`` plus an array-child filter. @@ -332,31 +377,27 @@ class DisconnectElements(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.I if not rels: self.report({"ERROR"}, "No connection found between elements.") return - # All rels between a single pair should share a kind in practice; pick - # the first kind for the cleanup dispatch and remove every rel below. - kind = rels[0][1] - if kind == "path": - for rel, _ in rels: - bonsai.core.geometry.remove_connection(tool.Geometry, connection=rel) - obj_a = tool.Ifc.get_object(elem_a) - obj_b = tool.Ifc.get_object(elem_b) - if obj_a is not None and obj_b is not None: - tool.Model.recreate_wall(elem_a, obj_a) - tool.Model.recreate_wall(elem_b, obj_b) - _resync_walls_after_mutation([obj_a, obj_b]) - elif kind in ("element-top", "element"): - for rel, _ in rels: - wall, slab = tool.Connection.orient_element_top(rel, elem_a, elem_b) - ifcopenshell.api.geometry.disconnect_element( - ifc_file, relating_element=slab, related_element=wall - ) - if kind == "element-top": - # The TOP rel is what extend_walls_to_underside creates; the - # related side is always the wall. - wall = rels[0][0].RelatedElement - wall_obj = tool.Ifc.get_object(wall) - if wall_obj is not None: - core.regenerate_wall_to_underside(tool.Ifc, tool.Geometry, tool.Model, [wall_obj]) + path_objs: list[bpy.types.Object] = [] + for rel, kind in rels: + bonsai.core.connection.disconnect_rel( + tool.Ifc, + tool.Geometry, + tool.Model, + tool.Connection, + rel=rel, + kind=kind, + elem=elem_a, + partner=elem_b, + ) + if kind == "path": + obj_a = tool.Ifc.get_object(elem_a) + obj_b = tool.Ifc.get_object(elem_b) + if obj_a is not None and obj_a not in path_objs: + path_objs.append(obj_a) + if obj_b is not None and obj_b not in path_objs: + path_objs.append(obj_b) + if path_objs: + _resync_walls_after_mutation(path_objs) class ExtendWallsToUnderside(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Operator): @@ -3865,16 +3906,17 @@ class GizmoWallLinkToggle(gizmo.GizmoLinkToggle, bpy.types.Gizmo): class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMixin): """Activates when exactly one LAYER2 wall is selected. Surfaces an unjoin icon at - every join location inferred from the wall's IfcRelConnectsPathElements inverse - graph — the single-selection mirror of `GizmoWallJoinIntersection`'s two-wall - unjoin state. A wall may participate in many such rels (up to 1 ATSTART + 1 ATEND - by end, plus unlimited ATPATH T-junctions), so a pool of icons is preallocated - and hidden on a per-frame basis based on the live connection set. + every connection location on the wall — wall-wall path connections via + IfcRelConnectsPathElements + wall-slab underside clips via IfcRelConnectsElements + with Description=="TOP". A wall may participate in many such rels (up to 1 ATSTART + + 1 ATEND by end, plus unlimited ATPATH T-junctions, plus one rel per clipped + slab), so a pool of icons is preallocated and hidden on a per-frame basis based + on the live connection set. Each visible icon dispatches `bim.disconnect_elements` with the active wall + - partner wall GlobalIds set on the bound operator properties, so a click removes - only the single rel under that icon — the other connections on the same wall - survive. + partner element GlobalIds set on the bound operator properties, so a click + removes only the single rel under that icon — the other connections on the + same wall survive. Mutually exclusive with `GizmoWallJoinIntersection` via `poll()` (that group requires len(selected) == 2; this one requires 1).""" @@ -3892,6 +3934,8 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix # creation is forbidden — so the pool must be sized upfront for the worst case. POOL_SIZE = 16 ICON_SCALE = 0.35 + SLAB_STACK_MAX = 5 + SLAB_STACK_OFFSET_Z = 0.5 @classmethod def poll(cls, context: bpy.types.Context) -> bool: @@ -3947,15 +3991,27 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix billboard_rot = gizmo.get_billboard_rotation(context) clearance = gizmo.top_down_clearance(context, billboard_rot) - connections = _get_wall_connections_cached(self, elem) - if len(connections) > self.POOL_SIZE and not getattr(self, "_pool_cap_warned", False): + path_connections = _get_wall_connections_cached(self, elem) + slab_connections = list(tool.Wall.iter_wall_slab_connections(elem)) + slab_overflow = max(0, len(slab_connections) - self.SLAB_STACK_MAX) + if slab_overflow and not getattr(self, "_slab_cap_warned", False): print( - f"[bonsai] GizmoWallUnjoinSingle: wall has {len(connections)} path connections; " + f"[bonsai] GizmoWallUnjoinSingle: wall has {len(slab_connections)} slab " + f"connections; only the first {self.SLAB_STACK_MAX} are shown stacked." + ) + self._slab_cap_warned = True + slab_connections = slab_connections[: self.SLAB_STACK_MAX] + total = len(path_connections) + len(slab_connections) + if total > self.POOL_SIZE and not getattr(self, "_pool_cap_warned", False): + print( + f"[bonsai] GizmoWallUnjoinSingle: wall has {total} connections " + f"({len(path_connections)} path + {len(slab_connections)} slab); " f"only the first {self.POOL_SIZE} unjoin gizmos are shown." ) self._pool_cap_warned = True - for slot_idx, (other_elem, self_ct, other_ct) in enumerate(connections): + slot_idx = 0 + for other_elem, self_ct, other_ct in path_connections: if slot_idx >= self.POOL_SIZE: break other_obj = tool.Ifc.get_object(other_elem) @@ -3966,21 +4022,216 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix continue seg_other = _wall_axis_world_segment_from_geom(other_obj, other_geom) location = tool.Wall.path_connection_location_world(seg_self, self_ct, seg_other, other_ct) + self._bind_unjoin_icon(slot_idx, location + clearance, billboard_rot, elem, other_elem, other_obj) + slot_idx += 1 + + for stack_idx, (slab_elem, _rel) in enumerate(slab_connections): + if slot_idx >= self.POOL_SIZE: + break + slab_obj = tool.Ifc.get_object(slab_elem) + if slab_obj is None: + continue + location = tool.Wall.wall_slab_connection_location_world(wall_obj, slab_obj) + if location is None: + continue + # Stack vertically so each slab gets a distinct clickable icon; + # hover-highlight then shows the user which slab they're about to + # disconnect from. + stacked = location + Vector((0.0, 0.0, stack_idx * self.SLAB_STACK_OFFSET_Z)) + self._bind_unjoin_icon(slot_idx, stacked + clearance, billboard_rot, elem, slab_elem, slab_obj) + slot_idx += 1 + + def _bind_unjoin_icon(self, slot_idx, location, billboard_rot, active_elem, partner_elem, partner_obj): + """Place + bind one pool icon to a (active, partner) GlobalId pair. + + Only the GlobalId properties are rewritten per frame; the operator + binding itself is the long-lived handle set up at setup() time. GlobalId + (not Blender object name) keeps the binding stable across renames, file + save/reload, and any sit-in-the-undo-stack interlude between dispatch + and execute. The partner Blender object is mirrored onto the icon for + its hover-outline draw, since the Gizmo API exposes + ``target_set_operator`` but no symmetric reader.""" + icon = self.unjoin_icons[slot_idx] + icon.matrix_basis = gizmo.billboarded_at(location, billboard_rot, scale=self.ICON_SCALE) + icon.hide = False + self.unjoin_op_props[slot_idx].element_a_guid = active_elem.GlobalId + self.unjoin_op_props[slot_idx].element_b_guid = partner_elem.GlobalId + icon.partner_obj = partner_obj + + +class GizmoSlabUnjoinWalls(bpy.types.GizmoGroup, gizmo.BillboardingGizmoGroupMixin): + """Slab-side mirror of GizmoWallUnjoinSingle: when exactly one IfcSlab is + selected and at least one wall is clipped to its underside, surface an + unjoin icon at each connection point. The icons resolve at the same + world location as the wall-side gizmo (via the symmetric + tool.Wall.wall_slab_connection_location_world) so the same connection + has a single visual marker reachable from either selection. + + Each visible icon dispatches bim.disconnect_elements with the slab + + wall GlobalIds, so a click removes the single rel under that icon and + re-clips the wall to whatever remaining slabs it's connected to.""" + + bl_idname = "OBJECT_GGT_bim_slab_unjoin_walls" + bl_label = "Slab Unjoin Walls Gizmo" + bl_space_type = "VIEW_3D" + bl_region_type = "WINDOW" + bl_options = {"3D", "PERSISTENT"} + + POOL_SIZE = 16 + ICON_SCALE = 0.35 + + @classmethod + def poll(cls, context: bpy.types.Context) -> bool: + return _slab_connection_gizmo_poll_gate(context, require_editing=True) + + def setup(self, context: bpy.types.Context) -> None: + default_color, highlight_color = self.get_decoration_colors() + self.unjoin_icons = [] + self.unjoin_op_props = [] + for _ in range(self.POOL_SIZE): + icon = self.setup_icon_gizmo( + "VIEW3D_GT_wall_link_toggle", default_color, highlight_color, "bim.disconnect_elements" + ) + icon.hide = True + self.unjoin_icons.append(icon) + self.unjoin_op_props.append(icon.target_set_operator("bim.disconnect_elements")) + + def position_gizmos(self, context: bpy.types.Context) -> None: + for icon in self.unjoin_icons: + icon.hide = True + + selected = list(tool.Blender.get_selected_objects()) + if len(selected) != 1: + return + slab_obj = selected[0] + slab_elem = tool.Ifc.get_entity(slab_obj) + if slab_elem is None: + return + + billboard_rot = gizmo.get_billboard_rotation(context) + clearance = gizmo.top_down_clearance(context, billboard_rot) + connections = list(tool.Wall.iter_slab_wall_connections(slab_elem)) + if len(connections) > self.POOL_SIZE and not getattr(self, "_pool_cap_warned", False): + print( + f"[bonsai] GizmoSlabUnjoinWalls: slab has {len(connections)} wall connections; " + f"only the first {self.POOL_SIZE} unjoin gizmos are shown." + ) + self._pool_cap_warned = True + + slot_idx = 0 + for wall_elem, _rel in connections: + if slot_idx >= self.POOL_SIZE: + break + wall_obj = tool.Ifc.get_object(wall_elem) + if wall_obj is None: + continue + location = tool.Wall.wall_slab_connection_location_world(wall_obj, slab_obj) + if location is None: + continue icon = self.unjoin_icons[slot_idx] icon.matrix_basis = gizmo.billboarded_at(location + clearance, billboard_rot, scale=self.ICON_SCALE) icon.hide = False - # Only the GlobalId properties are rewritten per frame; the operator - # binding itself is the long-lived handle set up at setup() time. GlobalId - # (not Blender object name) keeps the binding stable across renames, file - # save/reload, and any sit-in-the-undo-stack interlude between dispatch - # and execute. - self.unjoin_op_props[slot_idx].element_a_guid = elem.GlobalId - self.unjoin_op_props[slot_idx].element_b_guid = other_elem.GlobalId - # Mirror the partner reference onto the icon itself so its draw() - # can outline the partner on hover without a Gizmo-side getter on - # the bound operator (the API exposes target_set_operator with - # no symmetric reader). - icon.partner_obj = other_obj + self.unjoin_op_props[slot_idx].element_a_guid = slab_elem.GlobalId + self.unjoin_op_props[slot_idx].element_b_guid = wall_elem.GlobalId + icon.partner_obj = wall_obj + slot_idx += 1 + + +class GizmoSlabEdition(bpy.types.GizmoGroup, gizmo.BaseParametricGizmoGroup): + """Pen / validate / cancel triad for slab disconnect-access mode. + + Polls on a single IfcSlab with at least one wall clipped to its underside. + Pen routes through the universal ``bim.enable_editing_parametric`` + dispatcher; finish + cancel both clear ``is_editing`` (no IFC mutation — + the framework requires the triad to exist by name convention even for a + pure UI gate). ESC, the red-coloured cancel icon, mutual exclusion with + other active parametric edits, gizmo prefs gating — all handled by the + base class.""" + + bl_idname = "OBJECT_GGT_bim_slab_edition" + bl_label = "Slab Editing Gizmo" + bl_space_type = "VIEW_3D" + bl_region_type = "WINDOW" + bl_options = {"3D", "PERSISTENT"} + + enable_editing_operator = "bim.enable_editing_slab" + finish_editing_operator = "bim.finish_editing_slab" + cancel_editing_operator = "bim.cancel_editing_slab" + cycle_type_operator = "" + + props_getter = tool.Model.get_slab_props + gizmo_pref_name = "slab" + + @classmethod + def is_element_type(cls, element: ifcopenshell.entity_instance) -> bool: + return tool.Parametric.is_slab(element) and any( + True for _ in tool.Wall.iter_slab_wall_connections(element) + ) + + +class GizmoPairDisconnect(bpy.types.GizmoGroup, gizmo.BillboardingGizmoGroupMixin): + """Surfaces a disconnect icon when exactly 2 IFC elements are selected + and they share a supported rel — currently the wall + slab pair joined + by an ``IfcRelConnectsElements(TOP)``. Click dispatches + ``bim.disconnect_elements`` with both GlobalIds. For wall-wall pairs, + ``GizmoWallJoinIntersection``'s unjoin icon already exposes the same + affordance via ``bim.unjoin_walls``.""" + + bl_idname = "OBJECT_GGT_bim_pair_disconnect" + bl_label = "Disconnect Pair Gizmo" + bl_space_type = "VIEW_3D" + bl_region_type = "WINDOW" + bl_options = {"3D", "PERSISTENT"} + + ICON_SCALE = 0.35 + + @classmethod + def poll(cls, context: bpy.types.Context) -> bool: + selected = list(tool.Blender.get_selected_objects()) + if len(selected) != 2: + return False + if tool.Blender.Modifier.any_selected_is_array_child(): + return False + elem_a = tool.Ifc.get_entity(selected[0]) + elem_b = tool.Ifc.get_entity(selected[1]) + if elem_a is None or elem_b is None: + return False + rels = tool.Connection.find_rels(elem_a, elem_b) + return any(kind == "element-top" for _, kind in rels) + + def setup(self, context: bpy.types.Context) -> None: + default_color, highlight_color = self.get_decoration_colors() + self.disconnect_icon = self.setup_icon_gizmo( + "VIEW3D_GT_wall_link_toggle", default_color, highlight_color, "bim.disconnect_elements" + ) + self.disconnect_icon.hide = True + self.disconnect_op = self.disconnect_icon.target_set_operator("bim.disconnect_elements") + + def position_gizmos(self, context: bpy.types.Context) -> None: + self.disconnect_icon.hide = True + pair = _resolve_active_partner_pair(context) + if pair is None: + return + active, partner_obj, active_elem, partner_elem = pair + # Helper expects wall + slab regardless of which the user marked active. + if active_elem.is_a("IfcWall"): + wall_obj, slab_obj = active, partner_obj + elif partner_elem.is_a("IfcWall"): + wall_obj, slab_obj = partner_obj, active + else: + return + location = tool.Wall.wall_slab_connection_location_world(wall_obj, slab_obj) + if location is None: + return + billboard_rot = gizmo.get_billboard_rotation(context) + clearance = gizmo.top_down_clearance(context, billboard_rot) + self.disconnect_icon.matrix_basis = gizmo.billboarded_at( + location + clearance, billboard_rot, scale=self.ICON_SCALE + ) + self.disconnect_icon.hide = False + self.disconnect_op.element_a_guid = active_elem.GlobalId + self.disconnect_op.element_b_guid = partner_elem.GlobalId + self.disconnect_icon.partner_obj = partner_obj class GizmoWallFilletPreview(bpy.types.GizmoGroup, gizmo.BillboardingGizmoGroupMixin): diff --git a/src/bonsai/bonsai/tool/model.py b/src/bonsai/bonsai/tool/model.py index 2ea1678479..d126657e36 100644 --- a/src/bonsai/bonsai/tool/model.py +++ b/src/bonsai/bonsai/tool/model.py @@ -82,6 +82,7 @@ if TYPE_CHECKING: BIMPolylineProperties, BIMRailingProperties, BIMRoofProperties, + BIMSlabProperties, BIMStairProperties, BIMSverchokProperties, BIMWallProperties, @@ -118,6 +119,10 @@ class Model(bonsai.core.tool.Model): def get_railing_props(cls, obj: bpy.types.Object) -> BIMRailingProperties: return obj.BIMRailingProperties # pyright: ignore[reportAttributeAccessIssue] + @classmethod + def get_slab_props(cls, obj: bpy.types.Object) -> BIMSlabProperties: + return obj.BIMSlabProperties # pyright: ignore[reportAttributeAccessIssue] + @classmethod def get_pipe_segment_props(cls, obj: bpy.types.Object) -> BIMPipeSegmentProperties: return obj.BIMPipeSegmentProperties # pyright: ignore[reportAttributeAccessIssue] diff --git a/src/bonsai/bonsai/tool/parametric.py b/src/bonsai/bonsai/tool/parametric.py index 3c3fba66f5..313133f192 100644 --- a/src/bonsai/bonsai/tool/parametric.py +++ b/src/bonsai/bonsai/tool/parametric.py @@ -157,6 +157,7 @@ class Parametric(bonsai.core.tool.Parametric): ParametricObject("pipe_segment", supports_build_edit_lifecycle=True), ParametricObject("duct_segment", supports_build_edit_lifecycle=True), ParametricObject("wall"), + ParametricObject("slab"), ] # Annotations for the uppercase constants populated from ``EDIT_TYPES`` by @@ -171,6 +172,7 @@ class Parametric(bonsai.core.tool.Parametric): PIPE_SEGMENT: ClassVar[ParametricObject] DUCT_SEGMENT: ClassVar[ParametricObject] WALL: ClassVar[ParametricObject] + SLAB: ClassVar[ParametricObject] _geom_generation: int = 0 @@ -459,6 +461,15 @@ class Parametric(bonsai.core.tool.Parametric): return False return tool.Pset.get_element_pset(element, "BBIM_Stair") is not None + @classmethod + def is_slab(cls, element: entity_instance) -> bool: + """``True`` for any ``IfcSlab``. The slab edit lifecycle only gates + the connection-disconnect UI — no IFC mutation — so we don't narrow + further (e.g. by checking for wall connections). Per-gizmo polls + layer the "has wall connections" check on top via + ``tool.Wall.iter_slab_wall_connections``.""" + return element is not None and element.is_a("IfcSlab") + @classmethod def is_wall(cls, element: entity_instance) -> bool: """A wall is editable by the parametric gizmo if it is an IfcWall with LAYER2 usage. diff --git a/src/bonsai/bonsai/tool/wall.py b/src/bonsai/bonsai/tool/wall.py index 6f471e0c8a..b3c850e790 100644 --- a/src/bonsai/bonsai/tool/wall.py +++ b/src/bonsai/bonsai/tool/wall.py @@ -281,22 +281,35 @@ class Wall(bonsai.core.tool.Wall): return rel return None + WALL_SLAB_CONNECTION_Z_CLEARANCE = 0.5 + """Lift above the wall top so the disconnect icon sits above the + extend-vertical / slope gizmo and reads as "the thing above the wall = + the slab connection".""" + @classmethod def wall_slab_connection_location_world( cls, wall_obj: bpy.types.Object, slab_obj: bpy.types.Object ) -> Vector | None: - """World-space point where a wall is clipped by a slab — the wall's - axis midpoint lifted to the slab's underside Z. Approximate: uses the - slab's mesh bbox bottom in world space rather than reconstructing the - slab's clip plane. Adequate for icon placement on a wall whose top - meets the slab; returns ``None`` when the wall has no reference line.""" + """World-space anchor for the wall-slab disconnect icon. + + X / Y come from the wall axis midpoint (so the icon sits in the + middle of the wall horizontally); Z is the wall's top in world space + plus ``WALL_SLAB_CONNECTION_Z_CLEARANCE`` so the icon perches above + the slope gizmo. The slab-side gizmo calls this with the same + arguments so both sides of the same connection render a single + visual marker. ``slab_obj`` is kept on the signature for the + symmetric call shape; the helper's body no longer reads from it. + Returns ``None`` when the wall has no reference line.""" ref = cls.get_world_reference_line(wall_obj) if ref is None: return None axis_mid_world = (ref[0] + ref[1]) * 0.5 - slab_bottom_local_z = min(c[2] for c in slab_obj.bound_box) - slab_bottom_world_z = (slab_obj.matrix_world @ Vector((0.0, 0.0, slab_bottom_local_z))).z - return Vector((axis_mid_world.x, axis_mid_world.y, slab_bottom_world_z)) + if wall_obj.bound_box: + wall_top_local_z = max(c[2] for c in wall_obj.bound_box) + wall_top_world_z = (wall_obj.matrix_world @ Vector((0.0, 0.0, wall_top_local_z))).z + else: + wall_top_world_z = axis_mid_world.z + return Vector((axis_mid_world.x, axis_mid_world.y, wall_top_world_z + cls.WALL_SLAB_CONNECTION_Z_CLEARANCE)) @classmethod def walk_connected_walls( diff --git a/src/bonsai/test/bim/module/model/test_wall_array_child_filter_forward_compat.py b/src/bonsai/test/bim/module/model/test_wall_array_child_filter_forward_compat.py index 6c1b367b7c..6e4a7fdca9 100644 --- a/src/bonsai/test/bim/module/model/test_wall_array_child_filter_forward_compat.py +++ b/src/bonsai/test/bim/module/model/test_wall_array_child_filter_forward_compat.py @@ -26,6 +26,8 @@ Allow-list (gizmos intentionally outside the rule): - ``GizmoWallEdition`` — single-object parametric edit gizmo. Its base parametric poll already filters array children. +- ``GizmoSlabEdition`` — same as ``GizmoWallEdition`` (inherits + ``BaseParametricGizmoGroup`` whose poll filters array children). - ``GizmoWallFilletPreview`` — the preview-owner whose poll must fire WHILE its own preview is active; routing it through the topology gate would self-block it. @@ -47,9 +49,11 @@ pytestmark = pytest.mark.model # Wall gizmo groups intentionally outside the rule. Add a new entry only # with the in-code reasoning above. -_ALLOWLIST = frozenset({"GizmoWallEdition", "GizmoWallFilletPreview"}) +_ALLOWLIST = frozenset({"GizmoSlabEdition", "GizmoWallEdition", "GizmoWallFilletPreview"}) -_REQUIRED_CALLEES = frozenset({"_wall_topology_gizmo_poll_gate", "any_selected_is_array_child"}) +_REQUIRED_CALLEES = frozenset( + {"_wall_topology_gizmo_poll_gate", "_slab_connection_gizmo_poll_gate", "any_selected_is_array_child"} +) def _wall_module_source(): diff --git a/src/bonsai/test/bim/module/model/test_wall_slab_connections.py b/src/bonsai/test/bim/module/model/test_wall_slab_connections.py index 5bd9704e3c..5fff484943 100644 --- a/src/bonsai/test/bim/module/model/test_wall_slab_connections.py +++ b/src/bonsai/test/bim/module/model/test_wall_slab_connections.py @@ -178,28 +178,30 @@ def test_find_wall_slab_rel_returns_none_when_unconnected(): # --------------------------------------------------------------------------- -def test_wall_slab_connection_location_lifts_axis_mid_to_slab_underside(): - """The icon sits at the wall's axis midpoint X/Y lifted to the slab's - underside Z so it reads as a marker on the slab cut line.""" +def test_wall_slab_connection_location_perches_above_wall_top(): + """Icon X/Y comes from the wall axis midpoint; Z from the wall's mesh + bbox top in world space plus WALL_SLAB_CONNECTION_Z_CLEARANCE so the + icon sits above the extend-vertical / slope gizmo at the wall top.""" wall_obj = Mock() - slab_obj = Mock() - slab_obj.matrix_world = Matrix.Translation(Vector((0.0, 0.0, 3.0))) - slab_obj.bound_box = [ - (-1.0, -1.0, 0.0), - (1.0, -1.0, 0.0), - (-1.0, 1.0, 0.0), - (1.0, 1.0, 0.0), - (-1.0, -1.0, 0.2), - (1.0, -1.0, 0.2), - (-1.0, 1.0, 0.2), - (1.0, 1.0, 0.2), + wall_obj.matrix_world = Matrix.Identity(4) + wall_obj.bound_box = [ + (-0.1, -0.1, 0.0), + (0.1, -0.1, 0.0), + (-0.1, 0.1, 0.0), + (0.1, 0.1, 0.0), + (-0.1, -0.1, 3.0), + (0.1, -0.1, 3.0), + (-0.1, 0.1, 3.0), + (0.1, 0.1, 3.0), ] + slab_obj = Mock() ref_line = (Vector((1.0, 0.0, 0.0)), Vector((3.0, 0.0, 0.0))) with patch.object(tool.Wall, "get_world_reference_line", return_value=ref_line): loc = tool.Wall.wall_slab_connection_location_world(wall_obj, slab_obj) - assert loc == Vector((2.0, 0.0, 3.0)) + expected_z = 3.0 + tool.Wall.WALL_SLAB_CONNECTION_Z_CLEARANCE + assert loc == Vector((2.0, 0.0, expected_z)) def test_wall_slab_connection_location_returns_none_for_axisless_wall(): From bb8681a9545c22d174d0b6f3a2ac7e6c388a0663 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Fri, 12 Jun 2026 11:11:07 +0200 Subject: [PATCH 05/12] Cascade connection cleanup on element delete Deleting a slab that was connected to a wall via IfcRelConnectsElements(TOP) left the wall holding orphan IfcBooleanResult items + a stale BBIM_Boolean pset. The disconnect operator already runs the right cleanup; element delete just never invoked it. Extract the per-kind cleanup into core.connection.disconnect_rel so the operator (bim.disconnect_elements) and a new cascade in tool.Geometry.delete_ifc_object share one dispatch table. Adding a future rel kind to tool.Connection.find_rels now flows into both call sites automatically; an AST forward-compat guard enforces coverage. Other adjustments: - regenerate_wall_to_underside zero-slab branch now removes stale clip booleans instead of silently skipping, so disconnecting the last TOP slab also reverts the wall correctly. - duplicate_ifc_objects (Shift+D) calls strip_underside_booleans on copied walls so the duplicate doesn't carry over the source's slab trim, then reloads the body representation when something was stripped so the viewport reflects the change without waiting on Shift+G. - batch_being_deleted_ids threads through OverrideDelete so the cascade can suppress partner-side regenerate when both endpoints are queued for deletion in the same batch. This file was generated with the assistance of an AI coding tool. --- .../bonsai/bim/module/geometry/operator.py | 12 +- src/bonsai/bonsai/core/connection.py | 99 ++++++++ src/bonsai/bonsai/core/model.py | 17 +- src/bonsai/bonsai/core/tool.py | 10 + src/bonsai/bonsai/tool/connection.py | 37 +++ src/bonsai/bonsai/tool/geometry.py | 41 +++- src/bonsai/bonsai/tool/model.py | 42 ++++ .../module/model/test_disconnect_elements.py | 168 +++++++++++--- src/bonsai/test/core/bootstrap.py | 7 + src/bonsai/test/core/test_connection.py | 218 ++++++++++++++++++ .../tool/test_connection_forward_compat.py | 123 ++++++++++ 11 files changed, 742 insertions(+), 32 deletions(-) create mode 100644 src/bonsai/bonsai/core/connection.py create mode 100644 src/bonsai/test/core/test_connection.py create mode 100644 src/bonsai/test/tool/test_connection_forward_compat.py diff --git a/src/bonsai/bonsai/bim/module/geometry/operator.py b/src/bonsai/bonsai/bim/module/geometry/operator.py index 05a7b43625..e21668679b 100644 --- a/src/bonsai/bonsai/bim/module/geometry/operator.py +++ b/src/bonsai/bonsai/bim/module/geometry/operator.py @@ -890,6 +890,16 @@ class OverrideDelete(bpy.types.Operator): # Track aggregates before deleting their parts aggregates_to_check = self.track_aggregates(objects_to_remove) + # Snapshot the set of IFC entity ids being deleted in this batch so the + # connection-rel cascade inside `delete_ifc_object` can suppress + # partner-side regenerate when the partner is also about to vanish. + batch_being_deleted_ids: set[int] = set() + for obj in objects_to_remove: + if not tool.Blender.is_valid_data_block(obj): + continue + if (entity := tool.Ifc.get_entity(obj)) is not None: + batch_being_deleted_ids.add(entity.id()) + clear_active_object = True for i, obj in enumerate(objects_to_remove, 1): @@ -931,7 +941,7 @@ class OverrideDelete(bpy.types.Operator): if tool.Drawing.is_auto_annotation(element): self.report({"INFO"}, "References cannot be deleted. Exclude the referenced element instead.") continue - tool.Geometry.delete_ifc_object(obj) + tool.Geometry.delete_ifc_object(obj, batch_being_deleted_ids=batch_being_deleted_ids) elif tool.Geometry.is_representation_item(obj): tool.Geometry.delete_ifc_item(obj) else: diff --git a/src/bonsai/bonsai/core/connection.py b/src/bonsai/bonsai/core/connection.py new file mode 100644 index 0000000000..71a2bb871c --- /dev/null +++ b/src/bonsai/bonsai/core/connection.py @@ -0,0 +1,99 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Shared post-disconnect cleanup dispatch. + +Used by both ``bim.disconnect_elements`` (explicit user disconnect) and the +connection cascade in ``tool.Geometry.delete_ifc_object`` (implicit +disconnect-on-delete). Each rel kind returned by +:py:meth:`bonsai.tool.connection.Connection.find_rels` / +:py:meth:`find_rels_for_element` maps to a single arm here, so adding a new +rel kind means extending one dispatch table — both call sites benefit +automatically and the AST forward-compat guard enforces coverage. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import bonsai.core.geometry +from bonsai.core.model import regenerate_wall_to_underside + +if TYPE_CHECKING: + import bpy + import ifcopenshell + + import bonsai.tool as tool + + +def disconnect_rel( + ifc: "type[tool.Ifc]", + geometry: "type[tool.Geometry]", + model: "type[tool.Model]", + connection: "type[tool.Connection]", + rel: "ifcopenshell.entity_instance", + kind: str, + elem: "ifcopenshell.entity_instance", + partner: "ifcopenshell.entity_instance", + skip_elem_recreate: bool = False, + skip_partner_recreate: bool = False, +) -> None: + """Run the post-disconnect cleanup for one rel. + + ``elem`` and ``partner`` are the two endpoints. The ``skip_*_recreate`` + flags suppress per-side regenerate / recreate work — used by the + cascade-on-delete to avoid re-extruding entities that are about to be + removed by ``remove_product``. For the disconnect operator (where neither + endpoint is being deleted), both flags stay False and the full cleanup + runs on both sides. + """ + if kind == "path": + bonsai.core.geometry.remove_connection(geometry, connection=rel) + if not skip_elem_recreate: + elem_obj = ifc.get_object(elem) + if elem_obj is not None: + model.recreate_wall(elem, elem_obj) + if not skip_partner_recreate: + partner_obj = ifc.get_object(partner) + if partner_obj is not None: + model.recreate_wall(partner, partner_obj) + elif kind == "element-top": + wall, _slab = connection.orient_element_top(rel, elem, partner) + ifc.run( + "geometry.disconnect_element", + relating_element=rel.RelatingElement, + related_element=rel.RelatedElement, + ) + # Skip the wall-side regenerate when the wall is itself being deleted — + # either it's the elem of this cascade pass, or it's the partner that + # was queued earlier in the same batch. + if (wall is elem and skip_elem_recreate) or (wall is partner and skip_partner_recreate): + return + wall_obj = ifc.get_object(wall) + if wall_obj is not None: + regenerate_wall_to_underside(ifc, geometry, model, [wall_obj]) + elif kind == "element": + ifc.run( + "geometry.disconnect_element", + relating_element=rel.RelatingElement, + related_element=rel.RelatedElement, + ) + else: + raise ValueError(f"Unknown rel kind: {kind!r}") diff --git a/src/bonsai/bonsai/core/model.py b/src/bonsai/bonsai/core/model.py index 874675ea7f..30b1c9b515 100644 --- a/src/bonsai/bonsai/core/model.py +++ b/src/bonsai/bonsai/core/model.py @@ -167,12 +167,22 @@ def regenerate_wall_to_underside( model: type[tool.Model], wall_objs: list[bpy.types.Object], ) -> None: - """Re-clip walls to their connected underside objects after the slab has moved.""" + """Re-clip walls to their connected underside objects after the slab has moved. + + When a wall has no remaining slab connections — the case reached after the + last TOP rel is severed (via disconnect or via cascade-on-slab-delete) — the + stale trim booleans are cleaned up so the wall reverts to its pre-clip + extrusion instead of holding orphan ``IfcBooleanResult`` items and a dead + ``BBIM_Boolean`` pset. + """ clipped_objs = [] + reverted_objs = [] for obj in wall_objs: wall = ifc.get_entity(obj) slab_objs = model.get_connected_slab_objs(wall) if not slab_objs: + model.remove_wall_to_underside_booleans(wall) + reverted_objs.append(obj) continue if ifc.is_moved(obj): geometry.run_edit_object_placement(obj=obj) @@ -185,8 +195,9 @@ def regenerate_wall_to_underside( if clip: model.clip_wall_to_slab(wall, clip) clipped_objs.append(obj) - if clipped_objs: - model.reload_body_representation(clipped_objs) + refresh_objs = clipped_objs + reverted_objs + if refresh_objs: + model.reload_body_representation(refresh_objs) def extend_wall_to_slab( diff --git a/src/bonsai/bonsai/core/tool.py b/src/bonsai/bonsai/core/tool.py index f48165bd4c..91003174ad 100644 --- a/src/bonsai/bonsai/core/tool.py +++ b/src/bonsai/bonsai/core/tool.py @@ -195,6 +195,14 @@ class Collector: def assign(cls, obj, should_clean_users_collection=False): pass +@interface +class Connection: + def find_rel(cls, elem_a, elem_b): pass + def find_rels(cls, elem_a, elem_b): pass + def find_rels_for_element(cls, elem): pass + def orient_element_top(cls, rel, elem_a, elem_b): pass + + @interface class Context: def clear_context(cls): pass @@ -694,11 +702,13 @@ class Model: def load_openings(cls, openings): pass def purge_scene_openings(cls): pass def recalculate_walls(cls, objs): pass + def recreate_wall(cls, element, obj): pass def regenerate_array(cls, parent, data): pass def regenerate_profile(cls, obj): pass def regenerate_slab(cls, obj): pass def reload_body_representation(cls, obj_or_objects): pass def remove_wall_to_underside_booleans(cls, wall): pass + def strip_underside_booleans(cls, wall): pass def replace_object_ifc_representation(cls, ifc_file, ifc_context, obj, new_representation): pass diff --git a/src/bonsai/bonsai/tool/connection.py b/src/bonsai/bonsai/tool/connection.py index 055eb47a68..4ec154b573 100644 --- a/src/bonsai/bonsai/tool/connection.py +++ b/src/bonsai/bonsai/tool/connection.py @@ -93,6 +93,43 @@ class Connection: rels = cls.find_rels(elem_a, elem_b) return rels[0] if rels else (None, None) + @classmethod + def find_rels_for_element( + cls, + elem: "ifcopenshell.entity_instance", + ) -> "list[tuple[ifcopenshell.entity_instance, str, ifcopenshell.entity_instance]]": + """Return every supported rel touching ``elem`` as ``(rel, kind, partner)`` + triples. ``partner`` is the *other* element on the rel — the side cascade + cleanup must operate on when ``elem`` is being deleted. + + Mirrors :py:meth:`find_rels`'s kind taxonomy. The single-element entry + point lets the cascade-on-delete in ``tool.Geometry.delete_ifc_object`` + enumerate everything the disconnect operator would handle pairwise. + """ + result: list[tuple["ifcopenshell.entity_instance", str, "ifcopenshell.entity_instance"]] = [] + seen: set[int] = set() + + def _record(rel, kind, partner): + if partner is None or rel.id() in seen: + return + seen.add(rel.id()) + result.append((rel, kind, partner)) + + for rel in getattr(elem, "ConnectedTo", []) or (): + if rel.is_a("IfcRelConnectsPathElements"): + _record(rel, "path", getattr(rel, "RelatedElement", None)) + elif rel.is_a("IfcRelConnectsElements"): + kind = "element-top" if getattr(rel, "Description", None) == "TOP" else "element" + _record(rel, kind, getattr(rel, "RelatedElement", None)) + for rel in getattr(elem, "ConnectedFrom", []) or (): + if rel.is_a("IfcRelConnectsPathElements"): + _record(rel, "path", getattr(rel, "RelatingElement", None)) + elif rel.is_a("IfcRelConnectsElements"): + kind = "element-top" if getattr(rel, "Description", None) == "TOP" else "element" + _record(rel, kind, getattr(rel, "RelatingElement", None)) + + return result + @classmethod def orient_element_top( cls, diff --git a/src/bonsai/bonsai/tool/geometry.py b/src/bonsai/bonsai/tool/geometry.py index e0b719f065..e7de9c1f6a 100644 --- a/src/bonsai/bonsai/tool/geometry.py +++ b/src/bonsai/bonsai/tool/geometry.py @@ -65,6 +65,7 @@ from typing_extensions import TypeIs import bonsai.bim.helper import bonsai.bim.import_ifc +import bonsai.core.connection import bonsai.core.drawing import bonsai.core.geometry import bonsai.core.root @@ -271,12 +272,38 @@ class Geometry(bonsai.core.tool.Geometry): bpy.data.objects.remove(obj) @classmethod - def delete_ifc_object(cls, obj: bpy.types.Object) -> None: + def delete_ifc_object( + cls, + obj: bpy.types.Object, + batch_being_deleted_ids: Optional[set[int]] = None, + ) -> None: ifc_file = tool.Ifc.get() element = tool.Ifc.get_entity(obj) if not element: return - elif element.is_a("IfcAnnotation"): + # Cascade connection-rel teardown — symmetric to bim.disconnect_elements. + # When a slab connected to a wall via IfcRelConnectsElements(TOP) is deleted, + # the wall's trim booleans + BBIM_Boolean pset would otherwise be orphaned. + # skip_elem_recreate is always True here because we're inside delete: the + # element is about to vanish, so re-extruding it would be wasted work. + # skip_partner_recreate fires only when the partner is also queued in the + # same OverrideDelete batch. + if element.is_a("IfcRoot"): + skip_ids = batch_being_deleted_ids or set() + for rel, kind, partner in tool.Connection.find_rels_for_element(element): + bonsai.core.connection.disconnect_rel( + tool.Ifc, + tool.Geometry, + tool.Model, + tool.Connection, + rel=rel, + kind=kind, + elem=element, + partner=partner, + skip_elem_recreate=True, + skip_partner_recreate=(partner.id() in skip_ids), + ) + if element.is_a("IfcAnnotation"): if element.ObjectType == "DRAWING": return bonsai.core.drawing.remove_drawing(tool.Ifc, tool.Drawing, drawing=element) elif tool.Drawing.is_auto_annotation(element): @@ -2348,6 +2375,16 @@ class Geometry(bonsai.core.tool.Geometry): old_to_new[element] = [new] if new.is_a("IfcRelSpaceBoundary"): tool.Boundary.decorate_boundary(new_obj) + # Slab-trim booleans (from extend_walls_to_underside) belong to + # the source wall's connection, not the copy. Strip them so the + # duplicate reverts to its pre-clip extrusion — mirrors the way + # filling rels are dropped while manual booleans persist on copy. + # Reload the body when something was stripped so the viewport + # immediately shows the unclipped geometry; otherwise the user + # sees a stale mesh until they Shift+G, which is easy to miss. + if new.is_a("IfcWall"): + if tool.Model.strip_underside_booleans(new): + tool.Model.reload_body_representation(new_obj) # Remap Blender parent relationships for duplicated objects for old_obj_name, new_obj_name in old_obj_name_to_new_obj_name.items(): diff --git a/src/bonsai/bonsai/tool/model.py b/src/bonsai/bonsai/tool/model.py index d126657e36..8882d0c65f 100644 --- a/src/bonsai/bonsai/tool/model.py +++ b/src/bonsai/bonsai/tool/model.py @@ -909,6 +909,48 @@ class Model(bonsai.core.tool.Model): """Return True if element has an IfcRelConnectsElements(TOP) relationship.""" return any(rel.is_a("IfcRelConnectsElements") and rel.Description == "TOP" for rel in element.ConnectedFrom) + @classmethod + def strip_underside_booleans(cls, wall: ifcopenshell.entity_instance) -> bool: + """Remove slab-trim ``IfcBooleanResult`` items from a wall's body chain. + + Returns ``True`` if any boolean was removed, so the caller knows whether + a Blender-side body reload is needed to surface the geometry change. + + Hook for the duplicate path (Shift+D): the source wall's clip booleans + don't make sense on a copy pulled away from the slab. Booleans whose + ``SecondOperand.is_a("IfcTessellatedFaceSet")`` are removed — same + imprecise discriminator the rest of the wall-to-underside machinery + uses (manual cuts authored from tessellated meshes would also be + stripped, but most manual cuts use ``IfcExtrudedAreaSolid`` / CSG + primitives and are unaffected). + + Cannot reuse ``remove_wall_to_underside_booleans`` here because the + duplicate's ``BBIM_Boolean.Data`` holds the source wall's stale ids — + ``get_manual_booleans`` returns empty on the copy and the helper + early-returns. The duplicate hook works directly off the chain. + """ + representation = tool.Geometry.get_body_representation(wall) + if not representation: + return False + chain = cls.get_booleans(wall, representation) + to_remove = [ + b for b in chain if (sec := b.SecondOperand) is not None and sec.is_a("IfcTessellatedFaceSet") + ] + for b in to_remove: + tool.Geometry.remove_representation_item(b.SecondOperand, wall) + # Sweep the now-stale BBIM_Boolean entries on the copy (their ids point + # at booleans that were never in this wall's chain — they survived the + # ifcopenshell deep copy as JSON text in the pset payload). + pset_data = ifcopenshell.util.element.get_pset(wall, "BBIM_Boolean") + if pset_data: + representation = tool.Geometry.get_body_representation(wall) + chain_ids = {b.id() for b in cls.get_booleans(wall, representation)} if representation else set() + stored_ids = set(json.loads(pset_data["Data"])) + stale_ids = stored_ids - chain_ids + if stale_ids: + cls.unmark_manual_booleans(wall, list(stale_ids)) + return bool(to_remove) + @classmethod def remove_wall_to_underside_booleans(cls, wall: ifcopenshell.entity_instance) -> None: """Remove all IfcBooleanResult items previously added by extend_walls_to_underside.""" diff --git a/src/bonsai/test/bim/module/model/test_disconnect_elements.py b/src/bonsai/test/bim/module/model/test_disconnect_elements.py index 02464e1625..9eb37481de 100644 --- a/src/bonsai/test/bim/module/model/test_disconnect_elements.py +++ b/src/bonsai/test/bim/module/model/test_disconnect_elements.py @@ -121,6 +121,52 @@ def test_find_rels_dedups_by_id(): assert len(rels) == 1 +# --------------------------------------------------------------------------- +# tool.Connection.find_rels_for_element — single-element entry point +# --------------------------------------------------------------------------- + + +def test_find_rels_for_element_returns_kind_and_partner_per_rel(): + """Cascade-on-delete needs every rel touching one element plus the partner + element on the other side of each rel — that's the cleanup target.""" + elem = _elem() + partner_a = _elem() + partner_b = _elem() + rel_path = _rel("IfcRelConnectsPathElements", related=partner_a, rel_id=1) + rel_top = _rel("IfcRelConnectsElements", relating=partner_b, description="TOP", rel_id=2) + elem.ConnectedTo = [rel_path] + elem.ConnectedFrom = [rel_top] + + result = tool.Connection.find_rels_for_element(elem) + + assert (rel_path, "path", partner_a) in result + assert (rel_top, "element-top", partner_b) in result + assert len(result) == 2 + + +def test_find_rels_for_element_dedups_by_rel_id(): + elem = _elem() + partner = _elem() + rel = _rel("IfcRelConnectsPathElements", related=partner, relating=partner, rel_id=1) + elem.ConnectedTo = [rel] + elem.ConnectedFrom = [rel] + + result = tool.Connection.find_rels_for_element(elem) + + assert len(result) == 1 + + +def test_find_rels_for_element_skips_rels_without_partner(): + """Defensive: a malformed rel missing the opposite-side attribute should not + crash — record nothing for it rather than emit a (rel, kind, None) triple + that would later trip a None-deref in the dispatch.""" + elem = _elem() + bad = _rel("IfcRelConnectsPathElements", related=None, rel_id=1) + elem.ConnectedTo = [bad] + + assert tool.Connection.find_rels_for_element(elem) == [] + + # --------------------------------------------------------------------------- # tool.Connection.find_rel — first-match convenience # --------------------------------------------------------------------------- @@ -165,15 +211,16 @@ def _make_op(*, a_guid="A", b_guid="B"): return op -def test_disconnect_path_removes_all_rels_then_recreates_walls(): +def test_disconnect_dispatches_one_call_per_rel(): + """Operator forwards every rel returned by find_rels to disconnect_rel, + in order — the operator is a thin wrapper; per-kind cleanup logic lives + in core.connection.disconnect_rel and is tested separately.""" from bonsai.bim.module.model.wall import DisconnectElements elem_a = Mock() elem_b = Mock() rel1 = Mock() rel2 = Mock() - obj_a = Mock() - obj_b = Mock() ifc_file = MagicMock() ifc_file.by_guid.side_effect = lambda g: {"A": elem_a, "B": elem_b}[g] @@ -181,45 +228,114 @@ def test_disconnect_path_removes_all_rels_then_recreates_walls(): 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=[(rel1, "path"), (rel2, "path")], - ), patch( - "bonsai.bim.module.model.wall.tool.Ifc.get_object", side_effect=lambda e: {elem_a: obj_a, elem_b: obj_b}[e] - ), patch("bonsai.bim.module.model.wall.bonsai.core.geometry.remove_connection") as remove, patch( - "bonsai.bim.module.model.wall.tool.Model.recreate_wall" - ) as recreate, patch("bonsai.bim.module.model.wall._resync_walls_after_mutation") as resync: + 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"): DisconnectElements._perform(op, context=MagicMock()) - assert remove.call_count == 2 - assert recreate.call_count == 2 - resync.assert_called_once_with([obj_a, obj_b]) + assert dispatch.call_count == 2 + # Both rels dispatch with elem=elem_a, partner=elem_b regardless of orientation + # — orient_element_top inside disconnect_rel recovers the wall/slab roles. + for call, expected_rel, expected_kind in zip( + dispatch.call_args_list, [rel1, rel2], ["path", "element-top"] + ): + kw = call.kwargs + assert kw["rel"] is expected_rel + assert kw["kind"] == expected_kind + assert kw["elem"] is elem_a + assert kw["partner"] is elem_b op.report.assert_not_called() -def test_disconnect_element_top_calls_regenerate(): +def test_disconnect_resyncs_path_objs_once_for_path_kind(): + """For path rels the operator collects both endpoint objects and resyncs + drafts once at the end — a Blender-side concern that doesn't belong in + the core dispatch.""" from bonsai.bim.module.model.wall import DisconnectElements - wall = Mock() - slab = Mock() - wall_obj = Mock() + elem_a = Mock() + elem_b = Mock() + obj_a = Mock() + obj_b = Mock() rel = Mock() - rel.RelatedElement = wall ifc_file = MagicMock() - ifc_file.by_guid.side_effect = lambda g: {"A": wall, "B": slab}[g] + ifc_file.by_guid.side_effect = lambda g: {"A": elem_a, "B": elem_b}[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.Ifc.get_object", + 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: + DisconnectElements._perform(op, context=MagicMock()) + + resync.assert_called_once_with([obj_a, obj_b]) + + +def test_disconnect_skips_resync_for_non_path_kind(): + """element-top / element kinds don't need wall-draft resync — that's a + path-specific concern (DumbWallJoiner geometry refresh).""" + from bonsai.bim.module.model.wall import DisconnectElements + + elem_a = Mock() + elem_b = Mock() + rel = Mock() + + ifc_file = MagicMock() + ifc_file.by_guid.side_effect = lambda g: {"A": elem_a, "B": elem_b}[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.Connection.orient_element_top", return_value=(wall, slab) - ), patch("bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=wall_obj), patch( - "bonsai.bim.module.model.wall.ifcopenshell.api.geometry.disconnect_element" - ) as disc, patch("bonsai.bim.module.model.wall.core.regenerate_wall_to_underside") as regen: + ), 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: DisconnectElements._perform(op, context=MagicMock()) - disc.assert_called_once_with(ifc_file, relating_element=slab, related_element=wall) - regen.assert_called_once() - op.report.assert_not_called() + resync.assert_not_called() + + +def test_disconnect_gizmo_direction_symmetry(): + """The wall-selected gizmo dispatches with element_a=wall, element_b=slab. + The slab-selected gizmo dispatches with element_a=slab, element_b=wall. + Both routes hit disconnect_rel with the same (rel, kind) pair — orientation + recovery happens inside the dispatch, not at the operator layer.""" + from bonsai.bim.module.model.wall import DisconnectElements + + wall = Mock(name="wall") + slab = Mock(name="slab") + rel = Mock() + + ifc_file = MagicMock() + op = _make_op() + + def _run_with_guids(a, b): + ifc_file.by_guid.side_effect = lambda g: {a: wall if a == "WALL" else slab, b: slab if b == "SLAB" else wall}[g] + op.element_a_guid = a + op.element_b_guid = b + 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.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()) + return dispatch.call_args.kwargs + + wall_first = _run_with_guids("WALL", "SLAB") + slab_first = _run_with_guids("SLAB", "WALL") + + # disconnect_rel sees (rel, "element-top") in both runs; elem/partner swap + # by argument order but orient_element_top inside disconnect_rel resolves + # the wall/slab roles symmetrically. + assert wall_first["rel"] is rel and slab_first["rel"] is rel + assert wall_first["kind"] == slab_first["kind"] == "element-top" + assert {wall_first["elem"], wall_first["partner"]} == {wall, slab} + assert {slab_first["elem"], slab_first["partner"]} == {wall, slab} def test_disconnect_reports_on_unknown_guids(): diff --git a/src/bonsai/test/core/bootstrap.py b/src/bonsai/test/core/bootstrap.py index cd8371e1c3..6715fe8a94 100644 --- a/src/bonsai/test/core/bootstrap.py +++ b/src/bonsai/test/core/bootstrap.py @@ -60,6 +60,13 @@ def collector(): prophet.verify() +@pytest.fixture +def connection(): + prophet = Prophecy(bonsai.core.tool.Connection) + yield prophet + prophet.verify() + + @pytest.fixture def context(): prophet = Prophecy(bonsai.core.tool.Context) diff --git a/src/bonsai/test/core/test_connection.py b/src/bonsai/test/core/test_connection.py new file mode 100644 index 0000000000..5b09bc4dfa --- /dev/null +++ b/src/bonsai/test/core/test_connection.py @@ -0,0 +1,218 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Dispatch tests for ``core.connection.disconnect_rel``. + +The dispatch is the single source of truth for per-kind cleanup shared by the +explicit ``bim.disconnect_elements`` operator and the implicit cascade in +``tool.Geometry.delete_ifc_object``. Each kind has one test that pins which +helpers must be called; the AST forward-compat guard in +``test_connection_forward_compat.py`` then asserts the dispatch table covers +every kind ``Connection.find_rels`` can emit. + +Uses ``unittest.mock`` directly (rather than the Prophecy fixtures) because +the dispatch passes IFC rel entities with attribute access (``rel.RelatingElement``) +that Prophecy's JSON call recorder can't serialize. +""" + +from types import SimpleNamespace +from unittest.mock import Mock, patch + +import pytest + +import bonsai.core.connection as subject + + +def _rel(relating="slab", related="wall"): + return SimpleNamespace(RelatingElement=relating, RelatedElement=related) + + +def _ifc_with_objects(mapping): + ifc = Mock() + ifc.get_object.side_effect = lambda e: mapping.get(e) + ifc.run = Mock() + return ifc + + +class TestDisconnectRelPath: + def test_removes_connection_and_recreates_both_walls(self): + ifc = _ifc_with_objects({"elem_a": "obj_a", "elem_b": "obj_b"}) + geometry = Mock() + model = Mock() + connection = Mock() + + with patch("bonsai.core.connection.bonsai.core.geometry.remove_connection") as remove: + subject.disconnect_rel( + ifc, geometry, model, connection, + rel="rel", kind="path", elem="elem_a", partner="elem_b", + ) + + remove.assert_called_once_with(geometry, connection="rel") + model.recreate_wall.assert_any_call("elem_a", "obj_a") + model.recreate_wall.assert_any_call("elem_b", "obj_b") + assert model.recreate_wall.call_count == 2 + + def test_skip_elem_recreate_suppresses_elem_side(self): + """Cascade case: elem is being deleted — don't recreate it.""" + ifc = _ifc_with_objects({"elem": "elem_obj", "partner": "partner_obj"}) + geometry = Mock() + model = Mock() + connection = Mock() + + with patch("bonsai.core.connection.bonsai.core.geometry.remove_connection"): + subject.disconnect_rel( + ifc, geometry, model, connection, + rel="rel", kind="path", elem="elem", partner="partner", + skip_elem_recreate=True, + ) + + model.recreate_wall.assert_called_once_with("partner", "partner_obj") + + def test_skip_partner_recreate_suppresses_partner_side(self): + ifc = _ifc_with_objects({"elem": "elem_obj", "partner": "partner_obj"}) + geometry = Mock() + model = Mock() + connection = Mock() + + with patch("bonsai.core.connection.bonsai.core.geometry.remove_connection"): + subject.disconnect_rel( + ifc, geometry, model, connection, + rel="rel", kind="path", elem="elem", partner="partner", + skip_partner_recreate=True, + ) + + model.recreate_wall.assert_called_once_with("elem", "elem_obj") + + def test_both_skips_means_only_remove_rel(self): + ifc = _ifc_with_objects({}) + geometry = Mock() + model = Mock() + connection = Mock() + + with patch("bonsai.core.connection.bonsai.core.geometry.remove_connection") as remove: + subject.disconnect_rel( + ifc, geometry, model, connection, + rel="rel", kind="path", elem="elem", partner="partner", + skip_elem_recreate=True, + skip_partner_recreate=True, + ) + + remove.assert_called_once() + model.recreate_wall.assert_not_called() + + +class TestDisconnectRelElementTop: + def test_disconnects_then_regenerates_wall(self): + """Operator case (no skip flags): both sides survive, so the wall gets + re-clipped against currently-connected slabs.""" + rel = _rel() + ifc = _ifc_with_objects({"wall": "wall_obj"}) + geometry = Mock() + model = Mock() + connection = Mock() + connection.orient_element_top.return_value = ("wall", "slab") + + with patch("bonsai.core.connection.regenerate_wall_to_underside") as regen: + subject.disconnect_rel( + ifc, geometry, model, connection, + rel=rel, kind="element-top", elem="elem", partner="partner", + ) + + ifc.run.assert_called_once_with( + "geometry.disconnect_element", relating_element="slab", related_element="wall" + ) + regen.assert_called_once_with(ifc, geometry, model, ["wall_obj"]) + + def test_slab_delete_cascade_still_regenerates_wall(self): + """When slab is being deleted (elem=slab), wall survives and must + re-clip against remaining connections — the cascade's main purpose.""" + rel = _rel() + ifc = _ifc_with_objects({"wall": "wall_obj"}) + connection = Mock() + connection.orient_element_top.return_value = ("wall", "slab") + + with patch("bonsai.core.connection.regenerate_wall_to_underside") as regen: + subject.disconnect_rel( + ifc, Mock(), Mock(), connection, + rel=rel, kind="element-top", elem="slab", partner="wall", + skip_elem_recreate=True, # slab is being deleted + ) + + regen.assert_called_once() + + def test_wall_delete_cascade_skips_wall_regen(self): + """When the wall itself is being deleted, regenerating its body moments + before remove_product wipes it is wasted work — skip.""" + rel = _rel() + ifc = _ifc_with_objects({"wall": "wall_obj"}) + connection = Mock() + connection.orient_element_top.return_value = ("wall", "slab") + + with patch("bonsai.core.connection.regenerate_wall_to_underside") as regen: + subject.disconnect_rel( + ifc, Mock(), Mock(), connection, + rel=rel, kind="element-top", elem="wall", partner="slab", + skip_elem_recreate=True, # wall is being deleted + ) + + regen.assert_not_called() + ifc.run.assert_called_once() # rel still removed + + def test_both_in_batch_skips_wall_regen(self): + """Batch delete of both endpoints, processing slab first: partner (wall) + also queued for deletion → skip wall regen.""" + rel = _rel() + ifc = _ifc_with_objects({"wall": "wall_obj"}) + connection = Mock() + connection.orient_element_top.return_value = ("wall", "slab") + + with patch("bonsai.core.connection.regenerate_wall_to_underside") as regen: + subject.disconnect_rel( + ifc, Mock(), Mock(), connection, + rel=rel, kind="element-top", elem="slab", partner="wall", + skip_elem_recreate=True, + skip_partner_recreate=True, # wall also in batch + ) + + regen.assert_not_called() + + +class TestDisconnectRelElement: + def test_just_removes_the_rel(self): + rel = _rel(relating="A", related="B") + ifc = Mock() + + subject.disconnect_rel( + ifc, Mock(), Mock(), Mock(), + rel=rel, kind="element", elem="elem_a", partner="elem_b", + ) + + ifc.run.assert_called_once_with( + "geometry.disconnect_element", relating_element="A", related_element="B" + ) + + +class TestDisconnectRelUnknownKind: + def test_raises_value_error(self): + with pytest.raises(ValueError, match="Unknown rel kind"): + subject.disconnect_rel( + Mock(), Mock(), Mock(), Mock(), + rel="rel", kind="bogus", elem="a", partner="b", + ) diff --git a/src/bonsai/test/tool/test_connection_forward_compat.py b/src/bonsai/test/tool/test_connection_forward_compat.py new file mode 100644 index 0000000000..af830353b3 --- /dev/null +++ b/src/bonsai/test/tool/test_connection_forward_compat.py @@ -0,0 +1,123 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Forward-compat AST contract: ``core.connection.disconnect_rel`` must have a +branch for every rel ``kind`` emitted by ``tool.connection.Connection`` lookups. + +Adding a new rel kind (e.g. ``"void"``, ``"fill"``, ``"interferes"``) to +``find_rels`` / ``find_rels_for_element`` without extending ``disconnect_rel`` +would silently regress the disconnect operator and the cascade-on-delete: a new +kind would reach the dispatch, hit the ``raise ValueError("Unknown rel kind")`` +fallback, and either crash the operator or leave the cascade half-done. This +guard makes the symmetry mandatory at test time.""" + +import ast +from pathlib import Path + +import pytest + +pytestmark = pytest.mark.model + + +BONSAI_ROOT = Path(__file__).parent.parent.parent / "bonsai" +TOOL_CONNECTION = BONSAI_ROOT / "tool" / "connection.py" +CORE_CONNECTION = BONSAI_ROOT / "core" / "connection.py" + + +def _find_function(tree: ast.Module, name: str) -> ast.FunctionDef: + for node in ast.walk(tree): + if isinstance(node, ast.FunctionDef) and node.name == name: + return node + raise AssertionError(f"Function {name!r} not found") + + +def _find_method(tree: ast.Module, class_name: str, method_name: str) -> ast.FunctionDef: + for node in ast.walk(tree): + if isinstance(node, ast.ClassDef) and node.name == class_name: + for child in node.body: + if isinstance(child, ast.FunctionDef) and child.name == method_name: + return child + raise AssertionError(f"Method {class_name}.{method_name} not found") + + +def _kinds_emitted_by(method: ast.FunctionDef) -> set[str]: + """Extract every kind label this method emits. + + Looks at exactly two narrow patterns to avoid false positives from + docstrings or type-annotation strings: + + - ``_record(rel, "", …)`` — positional string at index 1, the + conventional emit shape in ``find_rels`` / ``find_rels_for_element``. + - ``kind = "" if … else ""`` and chained variants — string + literals on either branch of an ``ast.IfExp`` assigned to ``kind``. + """ + kinds: set[str] = set() + for node in ast.walk(method): + if isinstance(node, ast.Call): + func = node.func + if isinstance(func, ast.Name) and func.id == "_record" and len(node.args) >= 2: + arg = node.args[1] + if isinstance(arg, ast.Constant) and isinstance(arg.value, str): + kinds.add(arg.value) + elif isinstance(arg, ast.IfExp): + for branch in (arg.body, arg.orelse): + if isinstance(branch, ast.Constant) and isinstance(branch.value, str): + kinds.add(branch.value) + elif isinstance(node, ast.Assign): + targets = [t for t in node.targets if isinstance(t, ast.Name) and t.id == "kind"] + if not targets or not isinstance(node.value, ast.IfExp): + continue + for branch in (node.value.body, node.value.orelse): + if isinstance(branch, ast.Constant) and isinstance(branch.value, str): + kinds.add(branch.value) + return kinds + + +def _kind_branches_in_disconnect_rel(tree: ast.Module) -> set[str]: + """Return every kind matched by ``disconnect_rel``'s ``kind == "…"`` branches.""" + fn = _find_function(tree, "disconnect_rel") + kinds: set[str] = set() + for node in ast.walk(fn): + if isinstance(node, ast.Compare) and len(node.ops) == 1 and isinstance(node.ops[0], ast.Eq): + left = node.left + right = node.comparators[0] + if isinstance(left, ast.Name) and left.id == "kind": + if isinstance(right, ast.Constant) and isinstance(right.value, str): + kinds.add(right.value) + return kinds + + +def test_disconnect_rel_handles_every_kind_emitted_by_connection_lookups() -> None: + tool_tree = ast.parse(TOOL_CONNECTION.read_text(encoding="utf-8")) + core_tree = ast.parse(CORE_CONNECTION.read_text(encoding="utf-8")) + + emitted = _kinds_emitted_by(_find_method(tool_tree, "Connection", "find_rels")) | _kinds_emitted_by( + _find_method(tool_tree, "Connection", "find_rels_for_element") + ) + handled = _kind_branches_in_disconnect_rel(core_tree) + + assert emitted, "Sanity check: no kinds extracted — emit pattern may have changed" + + missing = emitted - handled + assert not missing, ( + f"core.connection.disconnect_rel is missing branches for kinds {missing}. " + f"Every kind returned by Connection.find_rels / find_rels_for_element " + f"must have a matching if/elif branch in the dispatch." + ) From 55428a0878739ceb289ab6ef6bb2e6886a41f220 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Sun, 14 Jun 2026 10:56:07 +0200 Subject: [PATCH 06/12] Add wall regen helper, fillet underside, bug sweep Wall body rebuild + slab underside re-clip are now unified behind tool.Model.regenerate_wall and called from split / merge / extend operators. Fillet corner walls accept extend-to-underside (poll + operator partition switched to is_path_connectable_wall) and surface the wall-unjoin gizmo without the parametric-edit gate, since fillets cannot enter that lifecycle. DumbWallJoiner.split strips the duplicate's inherited slab-trim booleans up front so wall2 lands at the cut point. regenerate_fillet_corner_wall re-clips after the body rewrite so a prior extend-to-slab survives neighbour recalcs. Drive-by bug sweep: tuple typo in hotkey_S_G's IfcSpace check, defensive .get() in draw_regen_operations for partial AuthoringData loads, and a try/except in get_active_representation matching the existing convention for stale mesh ifc_definition_ids after a representation rebuild. Tests cover the regenerate_wall branching, the get_active_representation stale-id contract, and the GizmoWallExtendVertically fillet acceptance. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/data.py | 4 + src/bonsai/bonsai/bim/module/model/wall.py | 51 +++++++++-- .../bonsai/bim/module/model/workspace.py | 8 +- src/bonsai/bonsai/tool/geometry.py | 13 ++- src/bonsai/bonsai/tool/model.py | 13 +++ .../bim/module/model/test_regenerate_wall.py | 85 +++++++++++++++++++ .../test/bim/module/model/test_wall_gizmos.py | 53 ++++++++++-- src/bonsai/test/tool/test_geometry.py | 31 +++++++ 8 files changed, 243 insertions(+), 15 deletions(-) create mode 100644 src/bonsai/test/bim/module/model/test_regenerate_wall.py diff --git a/src/bonsai/bonsai/bim/module/model/data.py b/src/bonsai/bonsai/bim/module/model/data.py index 10553f1bed..36cde4eb0d 100644 --- a/src/bonsai/bonsai/bim/module/model/data.py +++ b/src/bonsai/bonsai/bim/module/model/data.py @@ -51,6 +51,10 @@ class AuthoringData: @classmethod def load(cls, ifc_element_type: Optional[str] = None): + # ``is_loaded`` is set first as a recursion guard: one of the data + # computations evaluates a PropertyGroup enum's ``items`` callback, + # which re-enters this method. Without the guard, load recurses to + # RecursionError. cls.is_loaded = True cls.props = tool.Model.get_model_props() cls.data["default_container"] = cls.default_container() diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index 33443b8fdd..1941c6fd6e 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -291,6 +291,15 @@ def _resync_walls_after_mutation(objs: Iterable["bpy.types.Object | None"]) -> N _maybe_resync_wall_props_from_ifc(obj) +def _regenerate_walls(objs: "Iterable[bpy.types.Object | None]") -> None: + """Rebuild every wall in ``objs`` from current IFC state — extrusion, + openings, and any underside slab clip — so the caller doesn't carry + feature-specific dispatch.""" + for obj in objs: + if obj is not None: + tool.Model.regenerate_wall(obj) + + class _CommitWallDraftsFirstMixin: """Operator mixin that flushes any in-progress wall parametric drafts in the current selection before delegating to the subclass's ``_perform``. @@ -420,7 +429,7 @@ class ExtendWallsToUnderside(_CommitWallDraftsFirstMixin, bpy.types.Operator, to element = tool.Ifc.get_entity(obj) if not element: continue - if tool.Model.get_usage_type(element) == "LAYER2": + if tool.Parametric.is_path_connectable_wall(element): walls.append(obj) else: slabs.append(obj) @@ -441,7 +450,7 @@ class RegenerateWallToUnderside(bpy.types.Operator, tool.Ifc.Operator): wall_objs = [ obj for obj in tool.Blender.get_selected_objects() - if (element := tool.Ifc.get_entity(obj)) and tool.Model.get_usage_type(element) == "LAYER2" + if (element := tool.Ifc.get_entity(obj)) and tool.Parametric.is_path_connectable_wall(element) ] if wall_objs: core.regenerate_wall_to_underside(tool.Ifc, tool.Geometry, tool.Model, wall_objs) @@ -693,9 +702,14 @@ class SplitWall(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Operat def _perform(self, context): selected_objs = tool.Model.get_selected_mesh_objects() + post_split_walls: list[bpy.types.Object] = [] for obj in selected_objs: - DumbWallJoiner().split(obj, context.scene.cursor.location) - _resync_walls_after_mutation(selected_objs) + new_obj = DumbWallJoiner().split(obj, context.scene.cursor.location) + post_split_walls.append(obj) + if new_obj is not None and new_obj not in post_split_walls: + post_split_walls.append(new_obj) + _resync_walls_after_mutation(post_split_walls) + _regenerate_walls(post_split_walls) return {"FINISHED"} @@ -730,6 +744,7 @@ class MergeWall(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Operat surviving_obj = next(o for o in selected_objs if o != active_obj) DumbWallJoiner().merge(surviving_obj, active_obj) _maybe_resync_wall_props_from_ifc(surviving_obj) + _regenerate_walls([surviving_obj]) return {"FINISHED"} @@ -1514,7 +1529,7 @@ class DumbWallJoiner: body = copy.deepcopy(axis1["reference"]) tool.Model.recreate_wall(element1, wall1) - def split(self, wall1: bpy.types.Object, target: Vector) -> None: + def split(self, wall1: bpy.types.Object, target: Vector) -> "bpy.types.Object | None": unit_scale = ifcopenshell.util.unit.calculate_unit_scale(tool.Ifc.get()) element1 = tool.Ifc.get_entity(wall1) @@ -1535,6 +1550,13 @@ class DumbWallJoiner: wall2 = self.duplicate_wall(wall1) element2 = tool.Ifc.get_entity(wall2) + # The duplicate inherits wall1's slab-trim boolean chain (copied by + # copy_class) but ``BBIM_Boolean.Data`` carries wall1's stale ids, so + # ``get_manual_booleans(element2)`` returns empty and the regenerator + # rebuilds wall2's body without those clips. Strip them up front so + # wall2 starts clean before the axis + placement reshape. + tool.Model.strip_underside_booleans(element2) + # Get the ATEND connection from wall1 to use it in wall2 relating_element = None connections = element1.ConnectedTo @@ -1634,6 +1656,7 @@ class DumbWallJoiner: tool.Model.recreate_wall(element1, wall1) tool.Model.recreate_wall(element2, wall2) + return wall2 def flip(self, wall1: bpy.types.Object) -> None: if tool.Ifc.is_moved(wall1): @@ -2509,7 +2532,9 @@ class ExtendWallToCursor(bpy.types.Operator, tool.Ifc.Operator): tool.Model, context.scene.cursor.location, ) - _resync_walls_after_mutation(tool.Blender.get_selected_objects()) + affected = list(tool.Blender.get_selected_objects()) + _resync_walls_after_mutation(affected) + _regenerate_walls(affected) return {"FINISHED"} @@ -2542,6 +2567,7 @@ class ExtendWallHeightToCursor(bpy.types.Operator, tool.Ifc.Operator): with bpy.context.temp_override(active_object=obj, selected_objects=[obj]): bpy.ops.bim.change_extrusion_depth(depth=new_height) _maybe_resync_wall_props_from_ifc(obj) + _regenerate_walls([obj]) return {"FINISHED"} @@ -3168,6 +3194,12 @@ def regenerate_fillet_corner_wall(element: ifcopenshell.entity_instance, obj: bp # the banana body. If a neighbour moved, the new placement follows; if # neither moved, the new matrix equals the old within floating-point noise. _apply_fillet_corner_geometry(ifc_file, obj, geom, wall_a_obj) + # The body rebuild swaps the wall's representation, so any prior underside + # clip is gone. Re-clip from the surviving TOP rels so an extend-to-slab + # applied to a fillet wall isn't silently wiped on the next neighbour + # recalc, ChangeExtrusionDepth, or split / merge call site. + if tool.Model.has_underside_connection(element): + core.regenerate_wall_to_underside(tool.Ifc, tool.Geometry, tool.Model, [obj]) class EnableWallFilletPreview(bpy.types.Operator): @@ -3640,7 +3672,7 @@ class GizmoWallExtendVertically(bpy.types.GizmoGroup, _WallGeomCachedBillboardin return False other = next(o for o in selected if o is not active) other_element = tool.Ifc.get_entity(other) - if not other_element or tool.Model.get_usage_type(other_element) != "LAYER2": + if not other_element or not tool.Parametric.is_path_connectable_wall(other_element): return False return True @@ -3950,6 +3982,11 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix element = tool.Ifc.get_entity(active) if not element or not tool.Parametric.is_path_connectable_wall(element): return False + # Fillet-corner walls have no LAYER2 usage and cannot enter the + # parametric edit lifecycle, so the ``is_editing`` gate is bypassed + # for them — otherwise their connection icons would never surface. + if tool.Parametric.is_fillet_corner_wall(element): + return True props = tool.Model.get_wall_props(active) if not props.is_editing: return False diff --git a/src/bonsai/bonsai/bim/module/model/workspace.py b/src/bonsai/bonsai/bim/module/model/workspace.py index e44dabd916..abe4f45113 100644 --- a/src/bonsai/bonsai/bim/module/model/workspace.py +++ b/src/bonsai/bonsai/bim/module/model/workspace.py @@ -963,7 +963,11 @@ class EditObjectUI: @classmethod def draw_regen_operations(cls, row, ui_context): - if AuthoringData.data["is_regenable_element"]: + # ``AuthoringData.load`` flips ``is_loaded`` at entry as a recursion + # guard, so a partial load (any computation along the way raising) + # leaves the tail keys unset. ``.get()`` keeps the header draw alive + # until the underlying failure is investigated. + if AuthoringData.data.get("is_regenable_element"): row = cls.layout.row(align=True) if ui_context != "TOOL_HEADER" else row add_layout_hotkey_operator(row, "Regen", "S_G", "Recalculate Element Geometry", ui_context) @@ -1317,7 +1321,7 @@ class Hotkey(bpy.types.Operator, tool.Ifc.Operator): bpy.ops.bim.recalculate_profile() elif self.active_class in ("IfcWindow", "IfcWindowStandardCase", "IfcDoor", "IfcDoorStandardCase"): bpy.ops.bim.recalculate_fill() - elif self.active_class in ("IfcSpace"): + elif self.active_class in ("IfcSpace",): bpy.ops.bim.generate_space() def hotkey_S_M(self): diff --git a/src/bonsai/bonsai/tool/geometry.py b/src/bonsai/bonsai/tool/geometry.py index e7de9c1f6a..14e4c5cfcc 100644 --- a/src/bonsai/bonsai/tool/geometry.py +++ b/src/bonsai/bonsai/tool/geometry.py @@ -668,7 +668,13 @@ class Geometry(bonsai.core.tool.Geometry): and isinstance(data, Geometry.TYPES_WITH_MESH_PROPERTIES) and (ifc_id := tool.Geometry.get_mesh_props(data).ifc_definition_id) ): - return tool.Ifc.get().by_id(ifc_id) + try: + return tool.Ifc.get().by_id(ifc_id) + except RuntimeError: + # Stale id: a representation rebuild freed the old entity + # while obj.data still tracks its id. Treated as "no active + # representation" — same contract as a mesh with id 0. + return None @classmethod def get_data_representation(cls, data: bpy.types.ID) -> ifcopenshell.entity_instance | None: @@ -2385,6 +2391,11 @@ class Geometry(bonsai.core.tool.Geometry): if new.is_a("IfcWall"): if tool.Model.strip_underside_booleans(new): tool.Model.reload_body_representation(new_obj) + # HasOpenings rels don't follow object duplication, so + # the duplicate's body must rebuild to match its current + # opening set. + else: + tool.Model.regenerate_wall(new_obj) # Remap Blender parent relationships for duplicated objects for old_obj_name, new_obj_name in old_obj_name_to_new_obj_name.items(): diff --git a/src/bonsai/bonsai/tool/model.py b/src/bonsai/bonsai/tool/model.py index 8882d0c65f..87d8fd4210 100644 --- a/src/bonsai/bonsai/tool/model.py +++ b/src/bonsai/bonsai/tool/model.py @@ -3071,6 +3071,19 @@ class Model(bonsai.core.tool.Model): obj.matrix_world = tool.Loader.apply_blender_offset_to_matrix_world(obj, matrix) tool.Geometry.record_object_position(obj) + @classmethod + def regenerate_wall(cls, obj: bpy.types.Object) -> None: + """Rebuild a wall's body from current IFC state: extrusion + openings + first, then re-clip to any surviving ``IfcRelConnectsElements(TOP)`` + slab. Safe on walls with no openings and no slab connection — both + steps no-op against their preconditions.""" + element = tool.Ifc.get_entity(obj) + if element is None: + return + cls.recreate_wall(element, obj) + if cls.has_underside_connection(element): + bonsai.core.model.regenerate_wall_to_underside(tool.Ifc, tool.Geometry, cls, [obj]) + @classmethod def recalculate_walls(cls, walls: list[bpy.types.Object]) -> None: queue: set[tuple[ifcopenshell.entity_instance, bpy.types.Object]] = set() diff --git a/src/bonsai/test/bim/module/model/test_regenerate_wall.py b/src/bonsai/test/bim/module/model/test_regenerate_wall.py new file mode 100644 index 0000000000..68ca4bcdf9 --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_regenerate_wall.py @@ -0,0 +1,85 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Pins the branching contract of ``tool.Model.regenerate_wall``. + +The body rebuild always runs (extrusion + openings); the slab re-clip only +runs when an ``IfcRelConnectsElements(TOP)`` rel survives. A wall without +either feature still completes without crashing.""" + +from unittest.mock import Mock, patch + +import pytest + +import bonsai.tool as tool + +pytestmark = pytest.mark.model + + +def test_regenerate_wall_rebuilds_body_and_reclips_when_connected(): + """Wall with a TOP connection: body rebuilt first, then re-clipped.""" + element = Mock() + obj = Mock() + + with patch("bonsai.tool.model.tool.Ifc.get_entity", return_value=element), patch.object( + tool.Model, "recreate_wall" + ) as recreate, patch.object(tool.Model, "has_underside_connection", return_value=True), patch( + "bonsai.tool.model.bonsai.core.model.regenerate_wall_to_underside" + ) as regen: + tool.Model.regenerate_wall(obj) + + recreate.assert_called_once_with(element, obj) + regen.assert_called_once() + args, _ = regen.call_args + assert args[3] == [obj] + + +def test_regenerate_wall_skips_reclip_when_no_top_rel(): + """Wall without a TOP connection: body rebuilt; re-clip skipped.""" + element = Mock() + obj = Mock() + + with patch("bonsai.tool.model.tool.Ifc.get_entity", return_value=element), patch.object( + tool.Model, "recreate_wall" + ) as recreate, patch.object(tool.Model, "has_underside_connection", return_value=False), patch( + "bonsai.tool.model.bonsai.core.model.regenerate_wall_to_underside" + ) as regen: + tool.Model.regenerate_wall(obj) + + recreate.assert_called_once_with(element, obj) + regen.assert_not_called() + + +def test_regenerate_wall_noops_when_obj_has_no_ifc_entity(): + """Non-IFC objects (e.g. a freshly created Blender mesh before + `tool.Ifc.run("root.create_entity")` runs) return None from get_entity; + the helper must return without touching the body or any rels.""" + obj = Mock() + + with patch("bonsai.tool.model.tool.Ifc.get_entity", return_value=None), patch.object( + tool.Model, "recreate_wall" + ) as recreate, patch.object(tool.Model, "has_underside_connection") as has_top, patch( + "bonsai.tool.model.bonsai.core.model.regenerate_wall_to_underside" + ) as regen: + tool.Model.regenerate_wall(obj) + + recreate.assert_not_called() + has_top.assert_not_called() + regen.assert_not_called() diff --git a/src/bonsai/test/bim/module/model/test_wall_gizmos.py b/src/bonsai/test/bim/module/model/test_wall_gizmos.py index c3b97e9466..4bdd78ed4a 100644 --- a/src/bonsai/test/bim/module/model/test_wall_gizmos.py +++ b/src/bonsai/test/bim/module/model/test_wall_gizmos.py @@ -50,12 +50,15 @@ def _make_context(active, selected): return SimpleNamespace(active_object=active, selected_objects=list(selected)) -def _patch_tools(prefs_on, selected, active_element, other_element, active_usage, other_usage): +def _patch_tools( + prefs_on, selected, active_element, other_element, active_usage, other_usage, other_is_path_connectable=None +): """Return a stack of patches that simulate one selection / IFC state for poll(). ``prefs.gizmos.draw_gizmos_in_3d_viewport`` is the top-level toggle. The - selection set, the IFC entity lookup, and the usage-type lookup are stubbed - so the test only depends on the predicate ordering in poll().""" + selection set, the IFC entity lookup, the usage-type lookup, and the + path-connectable-wall predicate are stubbed so the test only depends on + the predicate ordering in poll().""" prefs = SimpleNamespace(gizmos=SimpleNamespace(draw_gizmos_in_3d_viewport=prefs_on)) entity_map = {} @@ -67,12 +70,18 @@ def _patch_tools(prefs_on, selected, active_element, other_element, active_usage usage_map[id(active_element)] = active_usage usage_map[id(other_element)] = other_usage + if other_is_path_connectable is None: + other_is_path_connectable = other_usage == "LAYER2" + def get_entity(obj): return entity_map.get(id(obj)) def get_usage_type(element): return usage_map.get(id(element)) + def is_path_connectable_wall(element): + return element is other_element and other_is_path_connectable + from bonsai import tool return [ @@ -80,6 +89,7 @@ def _patch_tools(prefs_on, selected, active_element, other_element, active_usage patch.object(tool.Blender, "get_selected_objects", return_value=set(selected)), patch.object(tool.Ifc, "get_entity", side_effect=get_entity), patch.object(tool.Model, "get_usage_type", side_effect=get_usage_type), + patch.object(tool.Parametric, "is_path_connectable_wall", side_effect=is_path_connectable_wall), # The array-child filter is pinned by its own test file; stub it here # so these poll tests stay focused on the count / layer-usage gates # and don't have to scaffold the memoization cache key. @@ -87,7 +97,15 @@ def _patch_tools(prefs_on, selected, active_element, other_element, active_usage ] -def _run_poll(prefs_on, active_is_in_selected, len_override, active_usage, other_usage, active_has_entity=True): +def _run_poll( + prefs_on, + active_is_in_selected, + len_override, + active_usage, + other_usage, + active_has_entity=True, + other_is_path_connectable=None, +): from bonsai.bim.module.model.wall import GizmoWallExtendVertically slab_obj = _Obj("slab") @@ -103,7 +121,15 @@ def _run_poll(prefs_on, active_is_in_selected, len_override, active_usage, other slab_element = object() if active_has_entity else None wall_element = object() - patches = _patch_tools(prefs_on, selected, slab_element, wall_element, active_usage, other_usage) + patches = _patch_tools( + prefs_on, + selected, + slab_element, + wall_element, + active_usage, + other_usage, + other_is_path_connectable=other_is_path_connectable, + ) for p in patches: p.start() try: @@ -189,6 +215,23 @@ def test_poll_rejects_when_other_is_not_layer2_wall(): ) +def test_poll_accepts_fillet_corner_wall_partner(): + # Fillet-corner walls carry no LAYER2 usage by spec but the extend-to- + # underside operator handles them just like a parametric LAYER2 wall — + # the gizmo must surface for the slab + fillet-corner selection too. + assert ( + _run_poll( + prefs_on=True, + active_is_in_selected=True, + len_override=None, + active_usage="LAYER3", + other_usage=None, + other_is_path_connectable=True, + ) + is True + ) + + # ---------------------------------------------------------------------------- # _iter_path_connections — IfcRelConnectsPathElements inverse-graph walk # ---------------------------------------------------------------------------- diff --git a/src/bonsai/test/tool/test_geometry.py b/src/bonsai/test/tool/test_geometry.py index 424e1fd876..5683ce03de 100644 --- a/src/bonsai/test/tool/test_geometry.py +++ b/src/bonsai/test/tool/test_geometry.py @@ -135,6 +135,37 @@ class TestGetRepresentationData(NewFile): assert subject.get_representation_data(representation) == data +class TestGetActiveRepresentation(NewFile): + def test_returns_representation_for_live_id(self): + ifc = ifcopenshell.file() + tool.Ifc.set(ifc) + representation = ifc.createIfcShapeRepresentation() + mesh = bpy.data.meshes.new("Mesh") + obj = bpy.data.objects.new("Object", mesh) + tool.Geometry.get_mesh_props(mesh).ifc_definition_id = representation.id() + assert subject.get_active_representation(obj) == representation + + def test_returns_none_when_mesh_has_no_id(self): + ifc = ifcopenshell.file() + tool.Ifc.set(ifc) + obj = bpy.data.objects.new("Object", bpy.data.meshes.new("Mesh")) + assert subject.get_active_representation(obj) is None + + def test_returns_none_when_id_is_stale(self): + """A representation rebuild can free the old entity while obj.data + still tracks its id. Returning ``None`` keeps every UI redraw alive + instead of spamming ``RuntimeError`` from the by_id lookup.""" + ifc = ifcopenshell.file() + tool.Ifc.set(ifc) + representation = ifc.createIfcShapeRepresentation() + mesh = bpy.data.meshes.new("Mesh") + obj = bpy.data.objects.new("Object", mesh) + stale_id = representation.id() + tool.Geometry.get_mesh_props(mesh).ifc_definition_id = stale_id + ifc.remove(representation) + assert subject.get_active_representation(obj) is None + + class TestGetRepresentationId(NewFile): def test_run(self): ifc = ifcopenshell.file() From 3f8d7165e545581627f91c806ced3b3e5ce13c69 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Mon, 15 Jun 2026 10:57:07 +0200 Subject: [PATCH 07/12] Preserve openings on wall merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DumbWallJoiner.merge cascade-deletes element2's HasOpenings via delete_ifc_object, which previously dropped every IfcOpeningElement (and any IfcDoor / IfcWindow filling) hosted by the discarded wall. Re-host each void rel onto the survivor BEFORE the delete fires, and re-apply the opening's captured world matrix via edit_object_placement so the void doesn't drift when the two walls have different placements — a PlacementRelTo swap alone would fail this when origins differ along the shared axis. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/wall.py | 19 ++ .../module/model/test_wall_merge_openings.py | 192 ++++++++++++++++++ 2 files changed, 211 insertions(+) create mode 100644 src/bonsai/test/bim/module/model/test_wall_merge_openings.py diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index 1941c6fd6e..83946d4c0d 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -1736,6 +1736,25 @@ class DumbWallJoiner: related_connection=rel.RelatedConnectionType, ) + # Re-host openings from the discarded wall to the survivor before + # ``delete_ifc_object`` cascade-removes element2's voids and any + # filling that depends on them. ``edit_object_placement`` preserves + # the opening's world position when element1 and element2 have + # different placements — a ``PlacementRelTo`` swap alone would + # shift the opening as the relative offset changes. + ifc_file = tool.Ifc.get() + for rel in list(element2.HasOpenings): + opening = rel.RelatedOpeningElement + world_matrix = ifcopenshell.util.placement.get_local_placement(opening.ObjectPlacement) + rel.RelatingBuildingElement = element1 + ifcopenshell.api.geometry.edit_object_placement( + ifc_file, + product=opening, + matrix=world_matrix, + is_si=False, + should_transform_children=False, + ) + tool.Model.recreate_wall(element1, wall1) tool.Geometry.delete_ifc_object(wall2) diff --git a/src/bonsai/test/bim/module/model/test_wall_merge_openings.py b/src/bonsai/test/bim/module/model/test_wall_merge_openings.py new file mode 100644 index 0000000000..ab90ed9a60 --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_wall_merge_openings.py @@ -0,0 +1,192 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Pins the contract that ``DumbWallJoiner.merge`` re-hosts openings from +the discarded wall to the survivor before the cascade delete tears down +``element2.HasOpenings`` and any filling that references them. + +``edit_object_placement`` preserves the opening's world position when the +two walls have different placements — a ``PlacementRelTo`` swap alone +would shift the opening as the relative offset changes.""" + +from unittest.mock import MagicMock, Mock, patch + +import numpy as np +import pytest + +pytestmark = pytest.mark.wall + + +def _opening_rel(opening_id: int, placement_matrix: np.ndarray): + """Build a stub ``IfcRelVoidsElement`` carrying an opening with a known + placement. ``RelatingBuildingElement`` is settable so the test can + observe the re-host.""" + opening = Mock(name=f"opening_{opening_id}") + opening.id.return_value = opening_id + opening.ObjectPlacement = Mock(name=f"opening_placement_{opening_id}") + rel = Mock(name=f"voids_rel_{opening_id}") + rel.RelatedOpeningElement = opening + rel.RelatingBuildingElement = None + return rel, opening, placement_matrix + + +def _merge_inputs(*, has_openings): + """Stage the minimum wall1 + wall2 + element1 + element2 surface that + ``DumbWallJoiner.merge`` reads. The reference lines and placements are + rigged so the collinearity guard passes and execution reaches the + opening-migration loop.""" + wall1 = Mock(name="wall1") + wall2 = Mock(name="wall2") + element1 = Mock(name="element1") + element2 = Mock(name="element2") + element1.ObjectPlacement = Mock(name="elem1_placement") + element2.ObjectPlacement = Mock(name="elem2_placement") + element1.ConnectedTo = [] + element1.ConnectedFrom = [] + element2.ConnectedTo = [] + element2.ConnectedFrom = [] + element2.HasOpenings = list(has_openings) + return wall1, wall2, element1, element2 + + +def _run_merge(wall1, wall2, element1, element2, opening_matrices, captured_edit_calls): + """Invoke ``DumbWallJoiner().merge`` against the staged inputs with + every heavy IFC / Blender side effect patched out. ``opening_matrices`` + maps an opening id to its captured world matrix; ``captured_edit_calls`` + is appended to whenever ``edit_object_placement`` fires.""" + from bonsai.bim.module.model.wall import DumbWallJoiner + + def fake_get_local_placement(placement): + for rel in element2.HasOpenings: + if rel.RelatedOpeningElement.ObjectPlacement is placement: + return opening_matrices[rel.RelatedOpeningElement.id()] + return np.eye(4) + + def fake_get_entity(obj): + return {wall1: element1, wall2: element2}[obj] + + def fake_edit_object_placement(ifc_file, *, product, matrix, is_si, should_transform_children): + captured_edit_calls.append( + { + "product": product, + "matrix": matrix, + "is_si": is_si, + "should_transform_children": should_transform_children, + } + ) + + 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]) + + 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", + side_effect=fake_get_local_placement, + ), + patch( + "bonsai.bim.module.model.wall.ifcopenshell.api.geometry.edit_object_placement", + side_effect=fake_edit_object_placement, + ), + patch("bonsai.bim.module.model.wall.tool.Model.recreate_wall"), + patch("bonsai.bim.module.model.wall.tool.Geometry.delete_ifc_object") as delete_ifc_object, + patch("bonsai.bim.module.model.wall.DumbWallJoiner.set_axis"), + ): + DumbWallJoiner().merge(wall1, wall2) + return delete_ifc_object + + +def test_merge_rehosts_each_opening_to_survivor(): + """Every void rel on the discarded wall is rebound to the survivor so + the cascade delete doesn't take them down with element2.""" + matrix_a = np.eye(4) + matrix_a[0, 3] = 1.0 + matrix_b = np.eye(4) + matrix_b[0, 3] = 3.0 + rel_a, opening_a, _ = _opening_rel(opening_id=101, placement_matrix=matrix_a) + rel_b, opening_b, _ = _opening_rel(opening_id=102, placement_matrix=matrix_b) + wall1, wall2, element1, element2 = _merge_inputs(has_openings=[rel_a, rel_b]) + + _run_merge( + wall1, + wall2, + element1, + element2, + opening_matrices={101: matrix_a, 102: matrix_b}, + captured_edit_calls=[], + ) + + assert rel_a.RelatingBuildingElement is element1 + assert rel_b.RelatingBuildingElement is element1 + + +def test_merge_preserves_opening_world_placement(): + """``edit_object_placement`` re-applies the opening's pre-merge world + matrix so the void doesn't drift when the two walls have different + placements — the regression a ``PlacementRelTo`` swap alone would + fail.""" + matrix = np.eye(4) + matrix[:3, 3] = (2.5, 0.0, 0.0) + rel, opening, _ = _opening_rel(opening_id=42, placement_matrix=matrix) + wall1, wall2, element1, element2 = _merge_inputs(has_openings=[rel]) + captured: list[dict] = [] + + _run_merge( + wall1, + wall2, + element1, + element2, + opening_matrices={42: matrix}, + captured_edit_calls=captured, + ) + + edit_calls_for_opening = [call for call in captured if call["product"] is opening] + assert len(edit_calls_for_opening) == 1 + np.testing.assert_allclose(edit_calls_for_opening[0]["matrix"], matrix, atol=1e-9) + assert edit_calls_for_opening[0]["should_transform_children"] is False + + +def test_merge_rehosts_before_delete(): + """Order matters: ``delete_ifc_object`` cascades through + ``element2.HasOpenings`` and would destroy the void if it ran before + the re-host. Assert the survivor was rebound before delete fires.""" + matrix = np.eye(4) + rel, opening, _ = _opening_rel(opening_id=7, placement_matrix=matrix) + wall1, wall2, element1, element2 = _merge_inputs(has_openings=[rel]) + + delete_ifc_object = _run_merge( + wall1, + wall2, + element1, + element2, + opening_matrices={7: matrix}, + captured_edit_calls=[], + ) + + assert rel.RelatingBuildingElement is element1 + delete_ifc_object.assert_called_once_with(wall2) From 7dcf415ef144d3a1a4834939121886249003ab32 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Mon, 15 Jun 2026 11:06:32 +0200 Subject: [PATCH 08/12] Resync wall props after dimension mutation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ChangeExtrusionDepth, ChangeExtrusionXAngle, and ChangeLayerLength mutate IFC extrusion / axis but never re-prime BIMWallProperties from the post-mutation state. Gizmo icons that position from props.height then sit at the pre-mutation elevation even though the wall mesh shows the new one — visible asymmetry against the workspace header H field which redraws live. Add the existing _resync_walls_after_mutation call to each operator's epilogue. _maybe_resync_wall_props_from_ifc already skips non-walls and walls in edit mode, so calling on the raw selection list is safe. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/wall.py | 3 + .../test_wall_props_resync_on_dim_change.py | 57 +++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 src/bonsai/test/bim/module/model/test_wall_props_resync_on_dim_change.py diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index 83946d4c0d..bc54335c91 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -811,6 +811,7 @@ class ChangeExtrusionDepth(bpy.types.Operator, tool.Ifc.Operator): if layer2_objs: tool.Model.recalculate_walls(layer2_objs) + _resync_walls_after_mutation(layer2_objs) return {"FINISHED"} @@ -926,6 +927,7 @@ class ChangeExtrusionXAngle(bpy.types.Operator, tool.Ifc.Operator): if layer2_objs: tool.Model.recalculate_walls(layer2_objs) + _resync_walls_after_mutation(layer2_objs) return {"FINISHED"} @@ -948,6 +950,7 @@ class ChangeLayerLength(bpy.types.Operator, tool.Ifc.Operator): selected_objs = tool.Model.get_selected_mesh_ifc_objects() for obj in selected_objs: joiner.set_length(obj, self.length) + _resync_walls_after_mutation(selected_objs) class OffsetWalls(bpy.types.Operator, tool.Ifc.Operator): diff --git a/src/bonsai/test/bim/module/model/test_wall_props_resync_on_dim_change.py b/src/bonsai/test/bim/module/model/test_wall_props_resync_on_dim_change.py new file mode 100644 index 0000000000..f9f05fb936 --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_wall_props_resync_on_dim_change.py @@ -0,0 +1,57 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Pins the contract that the three dimension-mutating wall operators — +``bim.change_extrusion_depth``, ``bim.change_extrusion_x_angle``, +``bim.change_layer_length`` — re-prime ``BIMWallProperties`` from the +post-mutation IFC at the end of ``_execute``. + +Without the resync, ``props.height`` / ``props.length`` / ``props.x_angle`` +stay at their pre-mutation values; gizmo icons that position from +``props.height`` then sit at the old elevation even though the wall mesh +shows the new one.""" + +import inspect + +import pytest + +pytestmark = pytest.mark.wall + + +def _execute_source(operator_cls): + return inspect.getsource(operator_cls._execute) + + +def test_change_extrusion_depth_resyncs_wall_props(): + from bonsai.bim.module.model.wall import ChangeExtrusionDepth + + assert "_resync_walls_after_mutation" in _execute_source(ChangeExtrusionDepth) + + +def test_change_extrusion_x_angle_resyncs_wall_props(): + from bonsai.bim.module.model.wall import ChangeExtrusionXAngle + + assert "_resync_walls_after_mutation" in _execute_source(ChangeExtrusionXAngle) + + +def test_change_layer_length_resyncs_wall_props(): + from bonsai.bim.module.model.wall import ChangeLayerLength + + assert "_resync_walls_after_mutation" in _execute_source(ChangeLayerLength) From bbe437adc847b8d34013fd12f8d39443d9ebe15f Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Mon, 15 Jun 2026 11:41:01 +0200 Subject: [PATCH 09/12] Fix wall-split filled-opening classification + void copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in DumbWallJoiner.split's filled-opening branch: 1. Side classification read filling_obj.matrix_world.translation — flip-fragile because flip_object rotates the filler 180° + translates so the bbox stays visually in place, moving the door origin to the opposite bbox corner. A flipped door centred over the cut could be classified on the wrong side. Switch to the opening's axis-projected midpoint, which the unfilled-opening loop already uses. 2. When the void straddles the cut and the filling moves to element2, the void copy for element1 was taken from the rebound new_opening whose PlacementRelTo had been swapped to element2 — the new void on element1 then sat in element2's local frame. Reorder so the copy reads from the original opening (still hosted by element1) before remove_feature destroys it. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/wall.py | 26 +++++--- .../model/test_wall_split_filled_opening.py | 66 +++++++++++++++++++ 2 files changed, 82 insertions(+), 10 deletions(-) create mode 100644 src/bonsai/test/bim/module/model/test_wall_split_filled_opening.py diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index bc54335c91..428c999274 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -1619,13 +1619,16 @@ class DumbWallJoiner: r.RelatedOpeningElement for r in list(element1.HasOpenings) if r.RelatedOpeningElement.HasFillings ]: rel = opening.HasFillings[0] - filling = rel.RelatedBuildingElement - filling_obj = tool.Ifc.get_object(filling) - filling_location = filling_obj.matrix_world.translation - _, filling_position = mathutils.geometry.intersect_point_line(filling_location.to_2d(), *axis_world_2d) min_t, max_t = _opening_axis_extent(opening, axis_world_2d, unit_scale) + # Use the opening's axis-projected midpoint to classify the side. + # The filling's ``matrix_world.translation`` is flip-fragile — + # ``flip_object`` rotates 180° + translates so the bbox stays + # visually in place, moving the door origin to the opposite + # corner, which would mis-classify a flipped door centred over + # the cut. + opening_midpoint = (min_t + max_t) / 2 void_straddles = min_t < cut_percentage < max_t - if filling_position > cut_percentage: + if opening_midpoint > cut_percentage: # The filling should be moved from element1 to element2. new_opening = ifcopenshell.api.root.copy_class(tool.Ifc.get(), product=opening) new_opening.VoidsElements[0].RelatingBuildingElement = element2 @@ -1640,13 +1643,16 @@ class DumbWallJoiner: rel.RelatingOpeningElement = new_opening - # Remove the old opening - ifcopenshell.api.feature.remove_feature(tool.Ifc.get(), feature=opening) - if void_straddles: # Filling moved to element2, but void straddles — add a - # pure-void copy back to element1 so its body still gets cut. - _add_void_copy(element1, new_opening) + # pure-void copy back to element1. Read from the original + # ``opening`` whose ObjectPlacement still references + # element1; ``new_opening`` was rebound to element2 and + # would copy element2's frame instead. + _add_void_copy(element1, opening) + + # Remove the old opening + ifcopenshell.api.feature.remove_feature(tool.Ifc.get(), feature=opening) elif void_straddles: # Filling stays on element1, but void straddles — add a pure-void # copy to element2 so its body gets cut. diff --git a/src/bonsai/test/bim/module/model/test_wall_split_filled_opening.py b/src/bonsai/test/bim/module/model/test_wall_split_filled_opening.py new file mode 100644 index 0000000000..9bbfcbb88a --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_wall_split_filled_opening.py @@ -0,0 +1,66 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Pins two contracts in ``DumbWallJoiner.split``'s filled-opening branch: + +1. Side classification reads the opening's axis-projected midpoint, not + the filling's ``matrix_world.translation``. The filling origin is + flip-fragile — ``flip_object`` rotates the filler 180° + translates so + the bbox stays visually in place, which would mis-classify a flipped + door centred over the cut. +2. When the void straddles the cut and the filling moves to element2, + the void copy for element1 is taken from the ORIGINAL opening (whose + ``ObjectPlacement`` still references element1), not the rebound + ``new_opening`` (whose ``PlacementRelTo`` was swapped to element2).""" + +import inspect + +import pytest + +pytestmark = pytest.mark.wall + + +def _split_source(): + from bonsai.bim.module.model.wall import DumbWallJoiner + + return inspect.getsource(DumbWallJoiner.split) + + +def test_side_classification_uses_opening_midpoint_not_filling_origin(): + source = _split_source() + assert "opening_midpoint" in source + # The pre-fix code projected the filling's world translation onto the + # axis to classify; that path must be gone. + assert "filling_obj.matrix_world.translation" not in source + + +def test_void_copy_reads_from_original_opening_before_remove(): + source = _split_source() + # Locate the "filling moves to element2" branch via the opening + # midpoint check; the void-copy and the trailing remove_feature both + # live inside this branch, after the prior unfilled-opening loops. + branch_start = source.index("if opening_midpoint > cut_percentage:") + branch = source[branch_start:] + add_idx = branch.index("_add_void_copy(element1, opening)") + remove_idx = branch.index("feature.remove_feature(tool.Ifc.get(), feature=opening)") + # Read-from-original is the whole point — the rebound ``new_opening`` + # references element2's frame and would shift the void to element1's + # origin in element2's local coords. + assert add_idx < remove_idx From 6119f0045e58eae83d2f71efff9177574fad0682 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Mon, 15 Jun 2026 12:37:24 +0200 Subject: [PATCH 10/12] Swap merge convention to active-is-survivor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bim.merge_wall now consumes the non-active selection into the active one — matching Blender's OBJECT_OT_join (Ctrl+J) and MESH_OT_merge "at last" convention. The wall the user clicks last absorbs the other; users following Blender muscle-memory get the result they expect. DumbWallJoiner.merge is already structurally asymmetric (wall1 = survivor); only the caller in MergeWall._perform needed flipping. Audit confirmed the previous call site was the sole caller of DumbWallJoiner.merge in production code. Drive-by tidies on adjacent code: collapse two over-length comprehensions under black's 120-char budget, and switch ``any(True for _ in gen)`` to ``any(gen)`` since the iterable yields tuples that are always truthy. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/wall.py | 19 ++-- src/bonsai/bonsai/tool/model.py | 4 +- .../model/test_merge_wall_convention.py | 87 +++++++++++++++++++ 3 files changed, 97 insertions(+), 13 deletions(-) create mode 100644 src/bonsai/test/bim/module/model/test_merge_wall_convention.py diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index 428c999274..5ab2233a08 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -150,7 +150,7 @@ def _slab_connection_gizmo_poll_gate(context: bpy.types.Context, *, require_edit return False if require_editing and not tool.Model.get_slab_props(active).is_editing: return False - return any(True for _ in tool.Wall.iter_slab_wall_connections(element)) + return any(tool.Wall.iter_slab_wall_connections(element)) def _wall_topology_gizmo_poll_gate(context: bpy.types.Context) -> bool: @@ -739,12 +739,13 @@ class MergeWall(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Operat active_obj = context.active_object assert active_obj selected_objs = tool.Model.get_selected_mesh_objects() - # The merge deletes the second argument when the walls are collinear; - # only the first survives, so the resync targets the non-active wall. - surviving_obj = next(o for o in selected_objs if o != active_obj) - DumbWallJoiner().merge(surviving_obj, active_obj) - _maybe_resync_wall_props_from_ifc(surviving_obj) - _regenerate_walls([surviving_obj]) + # Active-is-survivor — matches Blender's Ctrl+J / "merge at last" + # convention so the wall a user clicks last absorbs the other. + # DumbWallJoiner.merge deletes its second argument. + other_obj = next(o for o in selected_objs if o != active_obj) + DumbWallJoiner().merge(active_obj, other_obj) + _maybe_resync_wall_props_from_ifc(active_obj) + _regenerate_walls([active_obj]) return {"FINISHED"} @@ -4229,9 +4230,7 @@ class GizmoSlabEdition(bpy.types.GizmoGroup, gizmo.BaseParametricGizmoGroup): @classmethod def is_element_type(cls, element: ifcopenshell.entity_instance) -> bool: - return tool.Parametric.is_slab(element) and any( - True for _ in tool.Wall.iter_slab_wall_connections(element) - ) + return tool.Parametric.is_slab(element) and any(tool.Wall.iter_slab_wall_connections(element)) class GizmoPairDisconnect(bpy.types.GizmoGroup, gizmo.BillboardingGizmoGroupMixin): diff --git a/src/bonsai/bonsai/tool/model.py b/src/bonsai/bonsai/tool/model.py index 87d8fd4210..bc6f49063b 100644 --- a/src/bonsai/bonsai/tool/model.py +++ b/src/bonsai/bonsai/tool/model.py @@ -933,9 +933,7 @@ class Model(bonsai.core.tool.Model): if not representation: return False chain = cls.get_booleans(wall, representation) - to_remove = [ - b for b in chain if (sec := b.SecondOperand) is not None and sec.is_a("IfcTessellatedFaceSet") - ] + to_remove = [b for b in chain if (sec := b.SecondOperand) is not None and sec.is_a("IfcTessellatedFaceSet")] for b in to_remove: tool.Geometry.remove_representation_item(b.SecondOperand, wall) # Sweep the now-stale BBIM_Boolean entries on the copy (their ids point diff --git a/src/bonsai/test/bim/module/model/test_merge_wall_convention.py b/src/bonsai/test/bim/module/model/test_merge_wall_convention.py new file mode 100644 index 0000000000..6447b8541b --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_merge_wall_convention.py @@ -0,0 +1,87 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Pins the active-is-survivor merge convention. + +``bim.merge_wall`` must consume the non-active selection into the active +one — matching Blender's ``OBJECT_OT_join`` / ``MESH_OT_merge`` "at +last" convention. Users following Ctrl+J muscle-memory click the +surviving wall last; the operator must align with that expectation.""" + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest + +pytestmark = pytest.mark.wall + + +def _run_perform(active, other): + """Invoke ``MergeWall._perform`` as an unbound function with the + two wall stubs in the selection, patching the heavy IFC / Blender + side effects. Returns the ``(merger_arg_1, merger_arg_2)`` actually + passed to ``DumbWallJoiner.merge``.""" + from bonsai.bim.module.model.wall import MergeWall + + context = SimpleNamespace(active_object=active) + captured_call = {} + + def _capture_merge(self, a, b): + captured_call["wall1"] = a + captured_call["wall2"] = b + + fake_self = SimpleNamespace() + + with ( + patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=MagicMock(name="ifc_file")), + patch("bonsai.bim.module.model.wall.tool.Model.get_selected_mesh_objects", return_value=[active, other]), + patch("bonsai.bim.module.model.wall.DumbWallJoiner.__init__", return_value=None), + patch("bonsai.bim.module.model.wall.DumbWallJoiner.merge", new=_capture_merge), + patch("bonsai.bim.module.model.wall._maybe_resync_wall_props_from_ifc"), + patch("bonsai.bim.module.model.wall._regenerate_walls") as regen_walls, + ): + result = MergeWall._perform(fake_self, context) + + return captured_call, regen_walls, result + + +def test_active_wall_is_passed_as_survivor_to_merge(): + """The first argument to ``DumbWallJoiner.merge`` is the survivor; + the active object must occupy that slot so the wall the user clicked + last absorbs the other.""" + active = SimpleNamespace(name="active") + other = SimpleNamespace(name="other") + + captured, _regen, _ = _run_perform(active, other) + + assert captured["wall1"] is active + assert captured["wall2"] is other + + +def test_post_merge_resync_targets_active_not_consumed(): + """After the merge ``_regenerate_walls`` rebuilds the survivor's + body. Targeting the consumed wall would crash on a freed ``bpy_struct``; + the survivor (active) is the only valid target.""" + active = SimpleNamespace(name="active") + other = SimpleNamespace(name="other") + + _, regen_walls, _ = _run_perform(active, other) + + regen_walls.assert_called_once_with([active]) From 13c89ace83cf4ae754c92c9cf078a454683bcc9f Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Mon, 15 Jun 2026 14:32:29 +0200 Subject: [PATCH 11/12] Fix merge crash + surface/lock fillet preview connections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/bonsai/bonsai/bim/module/model/wall.py | 116 ++++++++++++++---- .../module/model/test_disconnect_elements.py | 72 ++++++++++- .../module/model/test_wall_merge_openings.py | 84 +++++++++++++ 3 files changed, 242 insertions(+), 30 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index 5ab2233a08..b7f287c0d6 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -386,6 +386,20 @@ class DisconnectElements(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.I if not rels: self.report({"ERROR"}, "No connection found between elements.") return + # The fillet corner's join with its source walls defines the fillet's + # identity — unjoining there would tear down the chord axis reference + # without rebuilding the source walls' miter cuts. Deleting the corner + # wall is the supported teardown, which cascades back to the source + # walls via the connection-cleanup handler. + either_is_fillet = tool.Parametric.is_fillet_corner_wall( + elem_a + ) or tool.Parametric.is_fillet_corner_wall(elem_b) + if either_is_fillet and any(k == "path" for _, k in rels): + self.report( + {"INFO"}, + "Fillet wall path connections can't be unjoined — delete the fillet wall element to remove the corner.", + ) + return path_objs: list[bpy.types.Object] = [] for rel, kind in rels: bonsai.core.connection.disconnect_rel( @@ -740,8 +754,8 @@ class MergeWall(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.Ifc.Operat assert active_obj selected_objs = tool.Model.get_selected_mesh_objects() # Active-is-survivor — matches Blender's Ctrl+J / "merge at last" - # convention so the wall a user clicks last absorbs the other. - # DumbWallJoiner.merge deletes its second argument. + # convention. The first argument survives, the second is consumed, + # so the active wall ends up absorbing the other. other_obj = next(o for o in selected_objs if o != active_obj) DumbWallJoiner().merge(active_obj, other_obj) _maybe_resync_wall_props_from_ifc(active_obj) @@ -1623,10 +1637,10 @@ class DumbWallJoiner: min_t, max_t = _opening_axis_extent(opening, axis_world_2d, unit_scale) # Use the opening's axis-projected midpoint to classify the side. # The filling's ``matrix_world.translation`` is flip-fragile — - # ``flip_object`` rotates 180° + translates so the bbox stays - # visually in place, moving the door origin to the opposite - # corner, which would mis-classify a flipped door centred over - # the cut. + # flipping rotates the filler 180° + translates so the bbox + # stays visually in place, moving the door origin to the + # opposite corner, which would mis-classify a flipped door + # centred over the cut. opening_midpoint = (min_t + max_t) / 2 void_straddles = min_t < cut_percentage < max_t if opening_midpoint > cut_percentage: @@ -1722,7 +1736,14 @@ class DumbWallJoiner: p2[0] = max(x_ordinates) self.set_axis(element1, p1, p2) + # ConnectedTo / ConnectedFrom carry both ``IfcRelConnectsPathElements`` + # (the wall-wall joins this loop migrates) and + # ``IfcRelConnectsElements`` (the slab underside clip). Only the + # path rels expose ``RelatingConnectionType`` / ``RelatedConnectionType``; + # the element rels die with element2 via the trailing cascade delete. for rel in element2.ConnectedTo: + if not rel.is_a("IfcRelConnectsPathElements"): + continue ifcopenshell.api.geometry.disconnect_path( tool.Ifc.get(), element=element1, connection_type=rel.RelatingConnectionType ) @@ -1735,6 +1756,8 @@ class DumbWallJoiner: ) for rel in element2.ConnectedFrom: + if not rel.is_a("IfcRelConnectsPathElements"): + continue ifcopenshell.api.geometry.disconnect_path( tool.Ifc.get(), element=element1, connection_type=rel.RelatedConnectionType ) @@ -1747,23 +1770,24 @@ class DumbWallJoiner: ) # Re-host openings from the discarded wall to the survivor before - # ``delete_ifc_object`` cascade-removes element2's voids and any - # filling that depends on them. ``edit_object_placement`` preserves - # the opening's world position when element1 and element2 have + # the cascade delete tears down element2's voids and any filling + # that depends on them. ``edit_object_placement`` preserves the + # opening's world position when element1 and element2 have # different placements — a ``PlacementRelTo`` swap alone would # shift the opening as the relative offset changes. ifc_file = tool.Ifc.get() for rel in list(element2.HasOpenings): opening = rel.RelatedOpeningElement - world_matrix = ifcopenshell.util.placement.get_local_placement(opening.ObjectPlacement) rel.RelatingBuildingElement = element1 - ifcopenshell.api.geometry.edit_object_placement( - ifc_file, - product=opening, - matrix=world_matrix, - is_si=False, - should_transform_children=False, - ) + if opening.ObjectPlacement: + world_matrix = ifcopenshell.util.placement.get_local_placement(opening.ObjectPlacement) + ifcopenshell.api.geometry.edit_object_placement( + ifc_file, + product=opening, + matrix=world_matrix, + is_si=False, + should_transform_children=False, + ) tool.Model.recreate_wall(element1, wall1) @@ -3348,6 +3372,19 @@ class CancelWallFilletPreview(bpy.types.Operator): props = preview_base.get_preview_props(context, "wall_fillet") if props is None or not props.is_active: return {"CANCELLED"} + # Clear the corner's edit flag so the connection disconnect gizmos + # disappear in lockstep with the radius preview when the user + # cancels. The id read happens BEFORE clear_preview_state wipes it. + corner_id = props.editing_corner_id + if corner_id: + ifc_file = tool.Ifc.get() + if ifc_file is not None: + try: + corner_obj = tool.Ifc.get_object(ifc_file.by_id(corner_id)) + except RuntimeError: + corner_obj = None + if corner_obj is not None: + tool.Model.get_wall_props(corner_obj).is_editing = False preview_base.clear_preview_state(props) return {"FINISHED"} @@ -3421,6 +3458,11 @@ class EnableWallFilletPreviewFromCorner(bpy.types.Operator): props.radius = float(radius) props.editing_corner_id = corner_elem.id() props.is_active = True + # Flag the corner as "in edit mode" so the wall-side connection + # disconnect gizmos surface in parallel with the fillet preview — + # one pen-icon click enters BOTH radius retune AND connection + # inspection. + tool.Model.get_wall_props(corner_obj).is_editing = True return {"FINISHED"} @@ -3997,10 +4039,23 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix ICON_SCALE = 0.35 SLAB_STACK_MAX = 5 SLAB_STACK_OFFSET_Z = 0.5 + # Muted gray used for connection icons that are visible (the connection + # exists) but inert (clicking dispatches a no-op + INFO report). Fillet + # corner ↔ source-wall joins use this — disconnecting them would tear + # down the fillet's chord axis reference, so the supported teardown is + # deleting the corner wall instead. + LOCKED_COLOR: ClassVar[tuple[float, float, float]] = (0.5, 0.5, 0.5) @classmethod def poll(cls, context: bpy.types.Context) -> bool: - if not _wall_topology_gizmo_poll_gate(context): + # Bypass the shared topology gate's ``any_preview_active`` block — + # ``BIMWallProperties.is_editing`` is the real gate for this gizmo + # group, and that flag is set both by the regular wall edit lifecycle + # AND by the fillet preview entry (so a fillet corner under preview + # surfaces its connections in parallel with the radius drag). + if not tool.Blender.are_viewport_gizmos_enabled(): + return False + if tool.Blender.Modifier.any_selected_is_array_child(): return False active = tool.Blender.get_active_object(is_selected=True) if active is None: @@ -4011,11 +4066,6 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix element = tool.Ifc.get_entity(active) if not element or not tool.Parametric.is_path_connectable_wall(element): return False - # Fillet-corner walls have no LAYER2 usage and cannot enter the - # parametric edit lifecycle, so the ``is_editing`` gate is bypassed - # for them — otherwise their connection icons would never surface. - if tool.Parametric.is_fillet_corner_wall(element): - return True props = tool.Model.get_wall_props(active) if not props.is_editing: return False @@ -4023,6 +4073,9 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix def setup(self, context: bpy.types.Context) -> None: default_color, highlight_color = self.get_decoration_colors() + # Stashed so per-frame ``_bind_unjoin_icon`` can restore the active + # tone when an icon was muted in a previous frame for fillet lock. + self._default_unjoin_color = default_color # Bind the operator on each pool icon ONCE at setup time and keep the returned # OperatorProperties handles. target_set_operator allocates a fresh handle on # every call, so calling it from position_gizmos (which fires every redraw @@ -4077,6 +4130,7 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix self._pool_cap_warned = True slot_idx = 0 + self_is_fillet = tool.Parametric.is_fillet_corner_wall(elem) for other_elem, self_ct, other_ct in path_connections: if slot_idx >= self.POOL_SIZE: break @@ -4088,7 +4142,10 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix continue seg_other = _wall_axis_world_segment_from_geom(other_obj, other_geom) location = tool.Wall.path_connection_location_world(seg_self, self_ct, seg_other, other_ct) - self._bind_unjoin_icon(slot_idx, location + clearance, billboard_rot, elem, other_elem, other_obj) + is_locked = self_is_fillet or tool.Parametric.is_fillet_corner_wall(other_elem) + self._bind_unjoin_icon( + slot_idx, location + clearance, billboard_rot, elem, other_elem, other_obj, is_locked=is_locked + ) slot_idx += 1 for stack_idx, (slab_elem, _rel) in enumerate(slab_connections): @@ -4107,7 +4164,9 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix self._bind_unjoin_icon(slot_idx, stacked + clearance, billboard_rot, elem, slab_elem, slab_obj) slot_idx += 1 - def _bind_unjoin_icon(self, slot_idx, location, billboard_rot, active_elem, partner_elem, partner_obj): + def _bind_unjoin_icon( + self, slot_idx, location, billboard_rot, active_elem, partner_elem, partner_obj, *, is_locked=False + ): """Place + bind one pool icon to a (active, partner) GlobalId pair. Only the GlobalId properties are rewritten per frame; the operator @@ -4116,10 +4175,15 @@ class GizmoWallUnjoinSingle(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMix save/reload, and any sit-in-the-undo-stack interlude between dispatch and execute. The partner Blender object is mirrored onto the icon for its hover-outline draw, since the Gizmo API exposes - ``target_set_operator`` but no symmetric reader.""" + ``target_set_operator`` but no symmetric reader. + + ``is_locked=True`` (fillet corner involvement) writes a muted color + instead of the active tone; the GUIDs still propagate so the bound + operator can surface a friendly INFO report on click.""" icon = self.unjoin_icons[slot_idx] icon.matrix_basis = gizmo.billboarded_at(location, billboard_rot, scale=self.ICON_SCALE) icon.hide = False + icon.color = self.LOCKED_COLOR if is_locked else self._default_unjoin_color self.unjoin_op_props[slot_idx].element_a_guid = active_elem.GlobalId self.unjoin_op_props[slot_idx].element_b_guid = partner_elem.GlobalId icon.partner_obj = partner_obj diff --git a/src/bonsai/test/bim/module/model/test_disconnect_elements.py b/src/bonsai/test/bim/module/model/test_disconnect_elements.py index 9eb37481de..4aeb7ffbad 100644 --- a/src/bonsai/test/bim/module/model/test_disconnect_elements.py +++ b/src/bonsai/test/bim/module/model/test_disconnect_elements.py @@ -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() diff --git a/src/bonsai/test/bim/module/model/test_wall_merge_openings.py b/src/bonsai/test/bim/module/model/test_wall_merge_openings.py index ab90ed9a60..27f7779357 100644 --- a/src/bonsai/test/bim/module/model/test_wall_merge_openings.py +++ b/src/bonsai/test/bim/module/model/test_wall_merge_openings.py @@ -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" From 7cd7db0c2b079440e7f85c1d0518ec7df9202370 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Mon, 15 Jun 2026 14:56:46 +0200 Subject: [PATCH 12/12] Tidy: black formatting + PR7a test docstrings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps three over-length lines black wanted on the merge-filter + fillet-lock commit (wall.py's ``either_is_fillet`` chain rewraps the right-hand ``or`` operand; test_disconnect_elements.py patch-stacks break each ``patch(`` onto its own continuation line). Adds per-test docstrings to test_wall_props_resync_on_dim_change.py and test_wall_split_filled_opening.py so the contract each pins is visible on grep / on test-run failure output without scrolling to the module-level docstring. Drops a flip_object sibling-symbol mention from the module docstring per CLAUDE.md §4a. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/wall.py | 6 ++-- .../module/model/test_disconnect_elements.py | 32 +++++++++++++------ .../test_wall_props_resync_on_dim_change.py | 7 ++++ .../model/test_wall_split_filled_opening.py | 23 ++++++------- 4 files changed, 44 insertions(+), 24 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index b7f287c0d6..183d9d32e6 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -391,9 +391,9 @@ class DisconnectElements(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.I # without rebuilding the source walls' miter cuts. Deleting the corner # wall is the supported teardown, which cascades back to the source # walls via the connection-cleanup handler. - either_is_fillet = tool.Parametric.is_fillet_corner_wall( - elem_a - ) or tool.Parametric.is_fillet_corner_wall(elem_b) + either_is_fillet = tool.Parametric.is_fillet_corner_wall(elem_a) or tool.Parametric.is_fillet_corner_wall( + elem_b + ) if either_is_fillet and any(k == "path" for _, k in rels): self.report( {"INFO"}, diff --git a/src/bonsai/test/bim/module/model/test_disconnect_elements.py b/src/bonsai/test/bim/module/model/test_disconnect_elements.py index 4aeb7ffbad..da668e604a 100644 --- a/src/bonsai/test/bim/module/model/test_disconnect_elements.py +++ b/src/bonsai/test/bim/module/model/test_disconnect_elements.py @@ -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( + ), 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()) @@ -239,9 +241,7 @@ def test_disconnect_dispatches_one_call_per_rel(): assert dispatch.call_count == 2 # Both rels dispatch with elem=elem_a, partner=elem_b regardless of orientation # — orient_element_top inside disconnect_rel recovers the wall/slab roles. - for call, expected_rel, expected_kind in zip( - dispatch.call_args_list, [rel1, rel2], ["path", "element-top"] - ): + for call, expected_rel, expected_kind in zip(dispatch.call_args_list, [rel1, rel2], ["path", "element-top"]): kw = call.kwargs assert kw["rel"] is expected_rel assert kw["kind"] == expected_kind @@ -271,7 +271,9 @@ def test_disconnect_resyncs_path_objs_once_for_path_kind(): ), patch( "bonsai.bim.module.model.wall.tool.Ifc.get_object", 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( + ), 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.tool.Parametric.is_fillet_corner_wall", return_value=False @@ -298,7 +300,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( + ), 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()) @@ -328,7 +332,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"), patch( + ) 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()) @@ -409,7 +415,9 @@ def test_disconnect_refuses_path_kind_when_either_side_is_fillet(): ), 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: + ), patch( + "bonsai.bim.module.model.wall.bonsai.core.connection.disconnect_rel" + ) as dispatch: DisconnectElements._perform(op, context=MagicMock()) dispatch.assert_not_called() @@ -437,9 +445,13 @@ def test_disconnect_allows_slab_kind_even_when_wall_is_fillet(): ), 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( + ), 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" + ): DisconnectElements._perform(op, context=MagicMock()) dispatch.assert_called_once() diff --git a/src/bonsai/test/bim/module/model/test_wall_props_resync_on_dim_change.py b/src/bonsai/test/bim/module/model/test_wall_props_resync_on_dim_change.py index f9f05fb936..68a95782fd 100644 --- a/src/bonsai/test/bim/module/model/test_wall_props_resync_on_dim_change.py +++ b/src/bonsai/test/bim/module/model/test_wall_props_resync_on_dim_change.py @@ -40,18 +40,25 @@ def _execute_source(operator_cls): def test_change_extrusion_depth_resyncs_wall_props(): + """Height mutation must re-prime ``BIMWallProperties.height`` so + gizmo icons positioned from ``props.height`` track the post-mutation + wall top in the same redraw.""" from bonsai.bim.module.model.wall import ChangeExtrusionDepth assert "_resync_walls_after_mutation" in _execute_source(ChangeExtrusionDepth) def test_change_extrusion_x_angle_resyncs_wall_props(): + """Slope mutation must re-prime ``BIMWallProperties.x_angle`` so + slope-driven gizmo positions track the new angle.""" from bonsai.bim.module.model.wall import ChangeExtrusionXAngle assert "_resync_walls_after_mutation" in _execute_source(ChangeExtrusionXAngle) def test_change_layer_length_resyncs_wall_props(): + """Length mutation must re-prime ``BIMWallProperties.length`` so + horizontal gizmo X positions track the new axis extent.""" from bonsai.bim.module.model.wall import ChangeLayerLength assert "_resync_walls_after_mutation" in _execute_source(ChangeLayerLength) diff --git a/src/bonsai/test/bim/module/model/test_wall_split_filled_opening.py b/src/bonsai/test/bim/module/model/test_wall_split_filled_opening.py index 9bbfcbb88a..499ce2509f 100644 --- a/src/bonsai/test/bim/module/model/test_wall_split_filled_opening.py +++ b/src/bonsai/test/bim/module/model/test_wall_split_filled_opening.py @@ -22,9 +22,9 @@ 1. Side classification reads the opening's axis-projected midpoint, not the filling's ``matrix_world.translation``. The filling origin is - flip-fragile — ``flip_object`` rotates the filler 180° + translates so - the bbox stays visually in place, which would mis-classify a flipped - door centred over the cut. + flip-fragile — flipping rotates the filler 180° + translates so the + bbox stays visually in place, which would mis-classify a flipped door + centred over the cut. 2. When the void straddles the cut and the filling moves to element2, the void copy for element1 is taken from the ORIGINAL opening (whose ``ObjectPlacement`` still references element1), not the rebound @@ -44,23 +44,24 @@ def _split_source(): def test_side_classification_uses_opening_midpoint_not_filling_origin(): + """Side classification must read the opening's axis-projected + midpoint, not the filling's world translation — the latter shifts + under flipping and would mis-classify a flipped door centred over + the cut.""" source = _split_source() assert "opening_midpoint" in source - # The pre-fix code projected the filling's world translation onto the - # axis to classify; that path must be gone. assert "filling_obj.matrix_world.translation" not in source def test_void_copy_reads_from_original_opening_before_remove(): + """When the filling moves to element2 and the void straddles the + cut, element1's pure-void copy must come from the original opening + BEFORE the cleanup that destroys it — the rebound ``new_opening`` + references element2's frame and would shift the void to element1's + origin in element2's local coords.""" source = _split_source() - # Locate the "filling moves to element2" branch via the opening - # midpoint check; the void-copy and the trailing remove_feature both - # live inside this branch, after the prior unfilled-opening loops. branch_start = source.index("if opening_midpoint > cut_percentage:") branch = source[branch_start:] add_idx = branch.index("_add_void_copy(element1, opening)") remove_idx = branch.index("feature.remove_feature(tool.Ifc.get(), feature=opening)") - # Read-from-original is the whole point — the rebound ``new_opening`` - # references element2's frame and would shift the void to element1's - # origin in element2's local coords. assert add_idx < remove_idx