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.
2026-08-11 10:06:47 +00:00 · 2026-06-14 10:56:07 +02:00
parent bb8681a954
commit 55428a0878
8 changed files with 243 additions and 15 deletions
@@ -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()
@@ -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
@@ -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):
@@ -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():
@@ -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()
@@ -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 <http://www.gnu.org/licenses/>.
+#
+# 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()
@@ -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
 # ----------------------------------------------------------------------------
@@ -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()