From 55428a0878739ceb289ab6ef6bb2e6886a41f220 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Sun, 14 Jun 2026 10:56:07 +0200 Subject: [PATCH] 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()