From fdf9970685227899ca992b84b3a03a695dacd876 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 2 Jul 2026 09:02:02 +0200 Subject: [PATCH 1/6] Bonsai: fix Apply Opening crash on non-fillings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The + gizmo previously appeared whenever a fillable host and any non-host object were selected, so clicking it against an IfcCovering crashed the geometry kernel when the opening generator tried to derive a shape it couldn't build (AttributeError on 'NoneType.wrapped_data'). Tighten the gizmo poll to require the secondary selection to be a class the operator can dispatch on: IfcDoor, IfcWindow, IfcOpeningElement, or a non-IFC mesh. Make the poll selection-order- independent so either click order activates it. Validate the same class set at the operator boundary so keymap or scripted invocations report a clear warning instead of crashing. The narrower Door/Window support in the opening generator is a Bonsai implementation limit, not an IFC schema restriction — IfcRelFillsElement.RelatedBuildingElement is typed as IfcElement and the schema permits any subtype. The tooltip and inline comment on the validation branch note this so a future reader knows the gate is future-work, not schema-mandated. Rewrite the operator's bl_description to end-user-friendly wording that drops the internal terms matrix_world and rl1/rl2. Fixes #8215. Generated with the assistance of an AI coding tool. --- .../module/model/host_add_opening_gizmo.py | 75 ++++++++++++------- src/bonsai/bonsai/bim/module/void/operator.py | 24 ++++-- .../model/test_host_add_opening_gizmo.py | 46 +++++++++++- 3 files changed, 112 insertions(+), 33 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/model/host_add_opening_gizmo.py b/src/bonsai/bonsai/bim/module/model/host_add_opening_gizmo.py index 64863fff22..d30ab29bba 100644 --- a/src/bonsai/bonsai/bim/module/model/host_add_opening_gizmo.py +++ b/src/bonsai/bonsai/bim/module/model/host_add_opening_gizmo.py @@ -52,6 +52,18 @@ def is_supported_host(element) -> bool: return tool.Parametric.is_path_connectable_wall(element) or element.is_a("IfcSlab") or element.is_a("IfcRoof") +def is_supported_filling(element) -> bool: + """Total predicate. A ``None`` element (raw Blender mesh) is accepted + because the apply-opening operator converts unclassified meshes into + ``IfcOpeningElement`` instances. IFC entities are accepted only when + their class is one the operator can dispatch on: ``IfcDoor`` / + ``IfcWindow`` (filled openings) or ``IfcOpeningElement`` (existing + opening reassigned to a new host).""" + if element is None: + return True + return element.is_a("IfcDoor") or element.is_a("IfcWindow") or element.is_a("IfcOpeningElement") + + def _resolve_active_host(context: bpy.types.Context, n_selected: int): """Shared poll prologue: gizmo gate + selection cardinality + active-in- selected + IFC entity lookup + supported-host predicate. Returns the @@ -72,12 +84,14 @@ def _resolve_active_host(context: bpy.types.Context, n_selected: int): class GizmoHostAddOpening(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMixin): - """Activates when a host element (wall / slab / roof) is the active object - and exactly one other selected object is *not* itself a host. + """Activates when exactly two objects are selected and one is a fillable + host (wall / slab / roof) while the other is a valid filling (door / + window / existing opening, or a plain Blender mesh). - Renders a single ``VIEW3D_GT_add_opening`` icon at the void object's - projected location on the host. A click dispatches ``bim.add_opening``, - which handles any element exposing the ``HasOpenings`` inverse. + Selection-order independent: the host role is identified by class, not + by active state. The "+" icon anchors on the host's surface regardless + of which object was clicked first. The dispatched ``bim.add_opening`` + operator also handles either order. Per-frame positioning keeps the icon facing the camera as the viewport orbits.""" @@ -90,22 +104,29 @@ class GizmoHostAddOpening(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMixin @classmethod def poll(cls, context: bpy.types.Context) -> bool: - element = _resolve_active_host(context, n_selected=2) - if element is None: + if not _wall_gizmo_poll_gate(context): return False - # The operator itself filters on HasOpenings, but checking here keeps - # the icon from appearing on host classes that can't accept openings - # in the active IFC schema. - if not hasattr(element, "HasOpenings"): + selected = list(tool.Blender.get_selected_objects()) + if len(selected) != 2: return False active = context.active_object - other = next(o for o in tool.Blender.get_selected_objects() if o is not active) - # Host + host pairings are claimed by host-specific gizmos (wall-join, - # extend-vertical, …) — suppress here so the add-opening icon never - # stacks on top of them. - if is_supported_host(tool.Ifc.get_entity(other)): + if active is None or active not in selected: return False - return True + a_element = tool.Ifc.get_entity(selected[0]) + b_element = tool.Ifc.get_entity(selected[1]) + return cls._is_apply_opening_pair(a_element, b_element) or cls._is_apply_opening_pair(b_element, a_element) + + @staticmethod + def _is_apply_opening_pair(host_element, filling_element) -> bool: + """``host_element`` qualifies as a fillable host AND ``filling_element`` + qualifies as a filling. Used twice with the operands swapped so the + gizmo polls true regardless of which of the two selected objects is + active.""" + if not is_supported_host(host_element): + return False + if not hasattr(host_element, "HasOpenings"): + return False + return is_supported_filling(filling_element) def setup(self, context: bpy.types.Context) -> None: default_color, highlight_color = self.get_decoration_colors() @@ -114,18 +135,20 @@ class GizmoHostAddOpening(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMixin ) def position_gizmos(self, context: bpy.types.Context) -> None: - host_obj = context.active_object - if not host_obj: + selected = list(tool.Blender.get_selected_objects()) + if len(selected) != 2: return - selected = tool.Blender.get_selected_objects() - other = next((o for o in selected if o is not host_obj), None) - if not other: - return - element = tool.Ifc.get_entity(host_obj) - if not element: + a, b = selected[0], selected[1] + a_element = tool.Ifc.get_entity(a) + b_element = tool.Ifc.get_entity(b) + if is_supported_host(a_element): + host_obj, host_element, other = a, a_element, b + elif is_supported_host(b_element): + host_obj, host_element, other = b, b_element, a + else: return - if tool.Parametric.is_path_connectable_wall(element): + if tool.Parametric.is_path_connectable_wall(host_element): world_pos = wall_anchor(context, self, host_obj, other) else: world_pos = layer3_anchor(host_obj, other) diff --git a/src/bonsai/bonsai/bim/module/void/operator.py b/src/bonsai/bonsai/bim/module/void/operator.py index 4f5d18f56c..b001e29f45 100644 --- a/src/bonsai/bonsai/bim/module/void/operator.py +++ b/src/bonsai/bonsai/bim/module/void/operator.py @@ -34,11 +34,13 @@ class AddOpening(bpy.types.Operator, tool.Ifc.Operator): bl_label = "Apply Opening" bl_options = {"REGISTER", "UNDO"} bl_description = ( - "Apply opening objects to an Element.\n\n" - "The Element and the openings to be applied should be selected. The order of selection is not important.\n" - "Opening can be just a Blender mesh object.\n\n" - "Shift+click: keep the filling at its current matrix_world — skip the wall-axis snap " - "and the rl1/rl2 Z-elevation default that the regular click applies." + "Cuts openings in a wall, slab, or roof using selected shape objects — " + "doors, windows, existing openings, or plain (non-IFC) meshes. " + "Selection order doesn't matter.\n\n" + "Doors and windows also fill the opening. Other IFC classes are currently " + "unsupported by the opening generator and get skipped with a warning.\n\n" + "Shift+click: keep each opening at its shape object's current position " + "instead of snapping to the wall." ) # Toggled by ``invoke`` when the user holds SHIFT during a gizmo / hotkey @@ -84,8 +86,20 @@ class AddOpening(bpy.types.Operator, tool.Ifc.Operator): self.report({"INFO"}, "You can't add an opening to another opening.") continue elif not element1.is_a("IfcOpeningElement") and not element2.is_a("IfcOpeningElement"): + # Bonsai currently derives opening geometry only from + # IfcDoor and IfcWindow (via OverallWidth/OverallHeight + # or their type's ELEVATION_VIEW profile). IFC's schema + # permits any IfcElement as a filling; broadening this + # gate is future work in the opening generator, not a + # schema requirement. if element1.is_a("IfcWindow") or element1.is_a("IfcDoor"): # Add a fill to an element. obj1, obj2 = obj2, obj1 + elif not (element2.is_a("IfcWindow") or element2.is_a("IfcDoor")): + self.report( + {"INFO"}, + f"Cannot apply {element2.is_a()} as an opening — Bonsai currently supports only IfcDoor and IfcWindow as parametric fillings.", + ) + continue FilledOpeningGenerator().generate( obj2, obj1, diff --git a/src/bonsai/test/bim/module/model/test_host_add_opening_gizmo.py b/src/bonsai/test/bim/module/model/test_host_add_opening_gizmo.py index 9e42ec34c5..52817c2b05 100644 --- a/src/bonsai/test/bim/module/model/test_host_add_opening_gizmo.py +++ b/src/bonsai/test/bim/module/model/test_host_add_opening_gizmo.py @@ -51,6 +51,10 @@ _IFC_CLASS_BY_KIND = { "slab": "IfcSlab", "roof": "IfcRoof", "plain": "IfcDiscreteAccessory", + "door": "IfcDoor", + "window": "IfcWindow", + "opening": "IfcOpeningElement", + "covering": "IfcCovering", } @@ -168,6 +172,40 @@ def test_poll_rejects_host_host_pairs(active_kind, other_kind, patched_tool): assert _run_poll(patched_tool, active_kind=active_kind, other_kind=other_kind) is False +@pytest.mark.parametrize("filling_kind", ["door", "window", "opening", "mesh"]) +def test_poll_accepts_host_with_supported_filling(filling_kind, patched_tool): + """The apply-opening gizmo must activate when the secondary selection + is a class the operator can dispatch on: ``IfcDoor`` / ``IfcWindow`` + (filled openings), ``IfcOpeningElement`` (existing opening reassigned + to a new host), or a raw Blender mesh (converted to an opening).""" + assert _run_poll(patched_tool, active_kind="wall", other_kind=filling_kind) is True + + +@pytest.mark.parametrize("non_filling_kind", ["covering", "plain"]) +def test_poll_rejects_host_with_non_filling(non_filling_kind, patched_tool): + """An IFC entity whose class the apply-opening operator can't dispatch + on must keep the gizmo hidden — clicking it would otherwise dispatch + the operator on a class whose geometry the opening generator can't + derive, causing a deep traceback in the geometry kernel.""" + assert _run_poll(patched_tool, active_kind="wall", other_kind=non_filling_kind) is False + + +@pytest.mark.parametrize("filling_kind", ["door", "window", "opening", "mesh"]) +def test_poll_accepts_filling_active_with_host_other(filling_kind, patched_tool): + """The poll must be selection-order independent: the icon should appear + whether the user clicked the host first or the filling first. The + operator handles either order, so the gizmo should match.""" + assert _run_poll(patched_tool, active_kind=filling_kind, other_kind="wall") is True + + +@pytest.mark.parametrize("non_filling_kind", ["covering", "plain"]) +def test_poll_rejects_non_filling_active_with_host_other(non_filling_kind, patched_tool): + """The selection-order independence must not loosen the filling + predicate — covering + wall stays rejected regardless of which is + active.""" + assert _run_poll(patched_tool, active_kind=non_filling_kind, other_kind="wall") is False + + def test_poll_rejects_active_host_without_has_openings(patched_tool): # Real-world equivalent: an IFC class that the active schema strips # ``HasOpenings`` from (e.g., a non-element subtype). The active sentinel @@ -265,12 +303,16 @@ def _run_position_layer3_branch( icon = SimpleNamespace(matrix_basis=None, hide=True) self_stub = SimpleNamespace(add_opening_icon=icon) - host_element = object() + # Host identification in the gizmo branches on the entity's class, so + # the sentinel must respond to ``is_a``. The non-host selection has no + # IFC entity (mesh-like) and is accepted as a filling. + host_element = _FakeIfcEntity("IfcSlab") + entity_map = {id(host_obj): host_element, id(other): None} with contextlib.ExitStack() as stack: stack.enter_context( patched_tool( selected_list=selected, - entity=host_element, + entity=lambda o: entity_map.get(id(o)), modifier_predicates={"is_path_connectable_wall": is_path_connectable_wall}, ) ) From 6ee3c7a15f99911e46a6ca5cb726a176f42bc901 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 2 Jul 2026 09:08:09 +0200 Subject: [PATCH 2/6] Bonsai: restore opening regen on recalculate_fill Commit 82dd1d94d switched RecalculateFill from bonsai.core.geometry.switch_representation to the surgical tool.Geometry.recut_host to speed up batched host recuts. The trade-off was intentional for that scope but dropped the implicit opening body refresh that switch_representation used to provide: SHIFT+G on a door whose parametric dimensions had drifted from its opening no longer resized the opening, so the wall recut still hit a stale mapped source. Extract a targeted single-source helper on tool.Model (regenerate_filling_opening_body) that regenerates one filling's mapped opening body via the existing FilledOpeningGenerator and inverse-substitutes the new representation across every filling that shares the mapped source. Refactor the family-wide caller (update_simple_openings, used by the parametric-edit finish path) to delegate to the same helper, deduped by source id so fragmented type families where multiple mapped sources coexist all get refreshed. Call the targeted helper at the top of RecalculateFill._recalculate_fills for each distinct source among the selected fillings. All body- representation lookups go through tool.Geometry.get_body_representation rather than inlining the ("Model", "Body", "MODEL_VIEW") triple. An AST forward-compat guard pins the call site. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/opening.py | 19 ++++ src/bonsai/bonsai/tool/model.py | 94 +++++++++++++------ .../test_recalculate_fill_forward_compat.py | 59 ++++++++++++ 3 files changed, 145 insertions(+), 27 deletions(-) create mode 100644 src/bonsai/test/bim/module/model/test_recalculate_fill_forward_compat.py diff --git a/src/bonsai/bonsai/bim/module/model/opening.py b/src/bonsai/bonsai/bim/module/model/opening.py index 4f0ded743f..d144bacf69 100644 --- a/src/bonsai/bonsai/bim/module/model/opening.py +++ b/src/bonsai/bonsai/bim/module/model/opening.py @@ -608,6 +608,25 @@ class RecalculateFill(bpy.types.Operator, tool.Ifc.Operator): return self._recalculate_fills(context) def _recalculate_fills(self, context): + # Refresh each selected filling's mapped opening source before + # recutting the host. Dedup by source id covers the common shared- + # source case in one rewrite while leaving unrelated sibling sources + # untouched. + seen_source_ids: set[int] = set() + for obj in context.selected_objects: + element = tool.Ifc.get_entity(obj) + if not element or not element.FillsVoids: + continue + opening = element.FillsVoids[0].RelatingOpeningElement + body = tool.Geometry.get_body_representation(opening) + if body is None: + continue + source = tool.Geometry.resolve_mapped_representation(body) + if source.id() in seen_source_ids: + continue + seen_source_ids.add(source.id()) + tool.Model.regenerate_filling_opening_body(element) + for obj in context.selected_objects: element = tool.Ifc.get_entity(obj) if not element or not element.FillsVoids: diff --git a/src/bonsai/bonsai/tool/model.py b/src/bonsai/bonsai/tool/model.py index bf9005768f..ad3af1f59c 100644 --- a/src/bonsai/bonsai/tool/model.py +++ b/src/bonsai/bonsai/tool/model.py @@ -2060,47 +2060,87 @@ class Model(bonsai.core.tool.Model): return (vertices, edges, faces) @classmethod - def update_simple_openings(cls, element: ifcopenshell.entity_instance) -> None: + def regenerate_filling_opening_body(cls, filling: ifcopenshell.entity_instance) -> Optional[bpy.types.Object]: + """Regenerate only the mapped source used by ``filling``'s opening so + it matches ``filling``'s current parametric dimensions. + + Returns the voided host Blender object so the caller can recut it, + or ``None`` if ``filling`` has no opening to refresh. Callers + targeting a single user-selected filling should use this rather than + the family-wide variant to avoid touching unrelated sibling sources.""" from bonsai.bim.module.model.opening import FilledOpeningGenerator - ifc_file = tool.Ifc.get() - fillings = {e: tool.Ifc.get_object(e) for e in tool.Array.get_parametric_propagation_targets(element)} + if not filling.FillsVoids: + return None - voided_objs = set() - has_replaced_opening_representation = False + ifc_file = tool.Ifc.get() + opening = filling.FillsVoids[0].RelatingOpeningElement + voided_obj = tool.Ifc.get_object(opening.VoidsElements[0].RelatingBuildingElement) + if voided_obj is None: + return None + + old_representation = tool.Geometry.get_body_representation(opening) + if old_representation is None: + return voided_obj + old_representation = tool.Geometry.resolve_mapped_representation(old_representation) + + ifcopenshell.api.geometry.unassign_representation(ifc_file, product=opening, representation=old_representation) + + filling_obj = tool.Ifc.get_object(filling) + new_representation = FilledOpeningGenerator().generate_opening_from_filling( + filling, filling_obj, voided_obj.dimensions[1] + ) + + for inverse in ifc_file.get_inverse(old_representation): + ifcopenshell.util.element.replace_attribute(inverse, old_representation, new_representation) + + ifcopenshell.api.geometry.remove_representation(ifc_file, representation=old_representation) + + return voided_obj + + @classmethod + def regenerate_simple_opening_bodies(cls, element: ifcopenshell.entity_instance) -> set: + """Regenerate every distinct mapped opening source within ``element``'s + type-occurrence family so each one matches the family's current + parametric dimensions. + + Most occurrences share a single mapped source — refreshing it once + propagates to every filling via inverse-substitution. Some families, + especially those imported from foreign authoring tools, fragment into + several mapped sources for the same type; dedup is by source id so + every distinct source gets one refresh. Returns the set of Blender + objects whose host representation needs a viewport-level recut + (callers handle the recut themselves).""" + ifc_file = tool.Ifc.get() + fillings = list(tool.Array.get_parametric_propagation_targets(element)) + + voided_objs: set = set() + seen_source_ids: set[int] = set() for filling in fillings: if not filling.FillsVoids: continue opening = filling.FillsVoids[0].RelatingOpeningElement voided_obj = tool.Ifc.get_object(opening.VoidsElements[0].RelatingBuildingElement) - voided_objs.add(voided_obj) + if voided_obj is not None: + voided_objs.add(voided_obj) - # We assume all occurrences of the same element type (e.g. a window) - # will use openings of the same thickness. - # Generator we use by default will create a really thick opening representation - # to make sure it will fit for walls with different thickness. - if has_replaced_opening_representation: + body = tool.Geometry.get_body_representation(opening) + if body is None: continue + source = tool.Geometry.resolve_mapped_representation(body) + if source.id() in seen_source_ids: + continue + seen_source_ids.add(source.id()) - old_representation = ifcopenshell.util.representation.get_representation( - opening, "Model", "Body", "MODEL_VIEW" - ) - old_representation = tool.Geometry.resolve_mapped_representation(old_representation) - ifcopenshell.api.geometry.unassign_representation( - ifc_file, product=opening, representation=old_representation - ) + cls.regenerate_filling_opening_body(filling) - new_representation = FilledOpeningGenerator().generate_opening_from_filling( - filling, fillings[filling], voided_obj.dimensions[1] - ) + return voided_objs - for inverse in ifc_file.get_inverse(old_representation): - ifcopenshell.util.element.replace_attribute(inverse, old_representation, new_representation) - - ifcopenshell.api.geometry.remove_representation(ifc_file, representation=old_representation) - - has_replaced_opening_representation = True + @classmethod + def update_simple_openings(cls, element: ifcopenshell.entity_instance) -> None: + voided_objs = cls.regenerate_simple_opening_bodies(element) + fillings = {e: tool.Ifc.get_object(e) for e in tool.Array.get_parametric_propagation_targets(element)} tool.Model.reload_body_representation(voided_objs) if fillings: diff --git a/src/bonsai/test/bim/module/model/test_recalculate_fill_forward_compat.py b/src/bonsai/test/bim/module/model/test_recalculate_fill_forward_compat.py new file mode 100644 index 0000000000..7c56914226 --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_recalculate_fill_forward_compat.py @@ -0,0 +1,59 @@ +# 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. + +"""AST contract: ``RecalculateFill`` must invoke +``regenerate_simple_opening_bodies`` before recutting hosts. + +Hosts recut with a surgical mesh-only path don't refresh the shared mapped +opening source — so any change to a parametric filling's dimensions stays +invisible at the opening boundary until the body representation is +regenerated. Pinning the call site forces future refactors to keep the +regen step in place.""" + +import ast +from pathlib import Path + +import pytest + +pytestmark = pytest.mark.model + + +def _recalculate_fill_body_source() -> str: + from bonsai.bim.module.model import opening as opening_module + + source = Path(opening_module.__file__).read_text(encoding="utf-8") + tree = ast.parse(source) + for node in ast.walk(tree): + if isinstance(node, ast.ClassDef) and node.name == "RecalculateFill": + for child in node.body: + if isinstance(child, ast.FunctionDef) and child.name == "_recalculate_fills": + return ast.unparse(child) + raise AssertionError("RecalculateFill._recalculate_fills was not found in opening.py") + + +def test_recalculate_fill_regenerates_opening_bodies_before_recut(): + body = _recalculate_fill_body_source() + assert "regenerate_filling_opening_body" in body, ( + "RecalculateFill._recalculate_fills must call " + "tool.Model.regenerate_filling_opening_body for each selected " + "filling before recutting the host. Without that call the host is " + "recut against a stale shared mapped opening source, so changes " + "to filling dimensions never surface." + ) From 9d2de117a93078f2c22b86ee13e16f505f970726 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 2 Jul 2026 09:13:04 +0200 Subject: [PATCH 3/6] Bonsai: sync filling placements on wall regen recalculate_walls commits the wall's own placement to IFC before recreating its geometry but did not touch its fillings. A door moved along the wall's reference line therefore stayed cut at its old position when the user pressed SHIFT+G on the wall, because the wall recut ran against the still-stale opening placement in IFC. Walk each wall's HasOpenings and, for every filling whose Blender matrix_world differs from its committed IFC placement (tool.Ifc.is_moved), commit the filling's placement and propagate the new matrix to the enclosing opening via ifcopenshell.api.geometry.edit_object_placement. The subsequent recreate_wall pass then sees the fresh opening positions and cuts at the right spot. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/tool/model.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/bonsai/bonsai/tool/model.py b/src/bonsai/bonsai/tool/model.py index ad3af1f59c..9babc968ba 100644 --- a/src/bonsai/bonsai/tool/model.py +++ b/src/bonsai/bonsai/tool/model.py @@ -3148,6 +3148,26 @@ class Model(bonsai.core.tool.Model): obj = tool.Ifc.get_object(rel.RelatingElement) tool.Geometry.commit_placement_if_moved(obj) queue.add((rel.RelatingElement, obj)) + + # Sync filling and opening placements so subsequent wall recuts + # operate on the up-to-date opening positions — a filling moved + # along the wall's reference line otherwise stays cut at its old + # spot. + for element, wall in queue: + if not wall: + continue + for rel in getattr(element, "HasOpenings", []) or []: + opening = rel.RelatedOpeningElement + for fill_rel in getattr(opening, "HasFillings", []) or []: + filling = fill_rel.RelatedBuildingElement + filling_obj = tool.Ifc.get_object(filling) + if filling_obj is None or not tool.Ifc.is_moved(filling_obj): + continue + bonsai.core.geometry.edit_object_placement(tool.Ifc, tool.Geometry, tool.Surveyor, obj=filling_obj) + ifcopenshell.api.geometry.edit_object_placement( + tool.Ifc.get(), product=opening, matrix=filling_obj.matrix_world + ) + for element, wall in queue: if not wall: continue From 4d92a64206938405dca4867a3ba6051a4b0fc27f Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 2 Jul 2026 09:18:13 +0200 Subject: [PATCH 4/6] Bonsai: skip sibling refresh on show/hide toggle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EditOpenings.edit_openings unconditionally walked sibling wall sets twice on every processed opening — once by mapped source id via get_similar_openings_building_objs, once by filling type via get_all_building_objects_of_similar_openings — and unioned both into the building_objs recut set. reload_body_representation then hit every one of those walls with a switch_representation call, even for the show/hide toggle path where nothing about the opening changed. Move both sibling-wall unions inside the is_edited / is_moved branch. Pure show/hide (no shape edit, no move) now touches only the wall(s) directly hosting the toggled openings. The edit and move paths still refresh siblings the same as before, since a mapped-source rewrite propagates the new shape to every sharing wall and each one needs a recut. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/opening.py | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/model/opening.py b/src/bonsai/bonsai/bim/module/model/opening.py index d144bacf69..fffa84c7c7 100644 --- a/src/bonsai/bonsai/bim/module/model/opening.py +++ b/src/bonsai/bonsai/bim/module/model/opening.py @@ -977,27 +977,29 @@ class EditOpenings(Operator, tool.Ifc.Operator): for opening_element in opening_elements: opening_obj = tool.Ifc.get_object(opening_element) - similar_openings = bonsai.core.geometry.get_similar_openings(tool.Ifc, opening_element) - similar_openings_building_objs = bonsai.core.geometry.get_similar_openings_building_objs( - tool.Ifc, similar_openings - ) - building_objs.update(similar_openings_building_objs) - if opening_obj: - if tool.Ifc.is_edited(opening_obj): - tool.Geometry.run_geometry_update_representation(obj=opening_obj) - bonsai.core.geometry.edit_similar_opening_placement( - tool.Geometry, opening_element, similar_openings - ) - elif tool.Ifc.is_moved(opening_obj): - bonsai.core.geometry.edit_object_placement(tool.Ifc, tool.Geometry, tool.Surveyor, obj=opening_obj) + opening_edited = tool.Ifc.is_edited(opening_obj) + opening_moved = tool.Ifc.is_moved(opening_obj) + # Sibling walls only need a viewport-level refresh when the + # opening's shape or placement actually changed — a pure + # show/hide toggle leaves them in their existing state. + if opening_edited or opening_moved: + similar_openings = bonsai.core.geometry.get_similar_openings(tool.Ifc, opening_element) + similar_openings_building_objs = bonsai.core.geometry.get_similar_openings_building_objs( + tool.Ifc, similar_openings + ) + building_objs.update(similar_openings_building_objs) + if opening_edited: + tool.Geometry.run_geometry_update_representation(obj=opening_obj) + else: + bonsai.core.geometry.edit_object_placement( + tool.Ifc, tool.Geometry, tool.Surveyor, obj=opening_obj + ) bonsai.core.geometry.edit_similar_opening_placement( tool.Geometry, opening_element, similar_openings ) + building_objs.update(self.get_all_building_objects_of_similar_openings(opening_element)) - building_objs.update( - self.get_all_building_objects_of_similar_openings(opening_element) - ) # NB this has nothing to do with clone similar_opening tool.Ifc.unlink(element=opening_element) if props.representation_obj == opening_obj: props.representation_obj = None From 5fba0026dd7aaea106c532722088a91b042e1349 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 2 Jul 2026 09:19:50 +0200 Subject: [PATCH 5/6] Bonsai: fix ruff import-sort drift in geometry+model ui Both files interleaved bpy.types imports with ifcopenshell.util imports, which ruff's I001 rejects for standard-library / third-party ordering. Running ruff check --fix on the two files reorders them into the isort-canonical shape with no behaviour change. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/geometry/ui.py | 2 +- src/bonsai/bonsai/bim/module/model/ui.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/geometry/ui.py b/src/bonsai/bonsai/bim/module/geometry/ui.py index 212e6ca74b..68e3e0fa47 100644 --- a/src/bonsai/bonsai/bim/module/geometry/ui.py +++ b/src/bonsai/bonsai/bim/module/geometry/ui.py @@ -17,9 +17,9 @@ # along with Bonsai. If not, see . import bpy +import ifcopenshell.util.unit from bpy.types import Menu, Panel, UIList -import ifcopenshell.util.unit import bonsai.bim import bonsai.tool as tool from bonsai.bim.helper import prop_with_search diff --git a/src/bonsai/bonsai/bim/module/model/ui.py b/src/bonsai/bonsai/bim/module/model/ui.py index e2b73dfe3d..13b918e5fd 100644 --- a/src/bonsai/bonsai/bim/module/model/ui.py +++ b/src/bonsai/bonsai/bim/module/model/ui.py @@ -22,9 +22,9 @@ from collections.abc import Iterable from typing import TYPE_CHECKING, Any import bpy +import ifcopenshell.util.unit from bpy.types import Panel -import ifcopenshell.util.unit import bonsai.bim import bonsai.tool as tool from bonsai.bim.helper import prop_with_search From 041306c5f003fd0010af3044924173c9a868dd47 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 2 Jul 2026 11:44:33 +0200 Subject: [PATCH 6/6] Bonsai: extract is_filling_supported + guard aggregate hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fold two related cleanups from post-PR review into one commit: Shared filling predicate — the gizmo poll and AddOpening._add_openings both need to decide whether an IFC entity is a Bonsai-supported filling (IfcDoor / IfcWindow, the classes the opening generator can derive geometry from). Centralise the check in bim.module.model.opening as is_filling_supported so a schema-broadening tomorrow only edits one predicate. The gizmo's own predicate is renamed is_supported_filling_or_opening to reflect its wider domain (also accepts None for raw meshes and IfcOpeningElement for reassignment). Aggregate-host guard — regenerate_filling_opening_body returns the voided host Blender object so callers can recut it. Aggregates have no mesh data; returning them made callers hit switch_representation against a None data-block. Guard on voided_obj.data is None and return None so callers can skip cleanly. Adds a direct position_gizmos test asserting host-at-index-1 (filling active) still anchors on the slab — pins the class-based dispatch's selection-order independence. Generated with the assistance of an AI coding tool. --- .../module/model/host_add_opening_gizmo.py | 21 ++++--- src/bonsai/bonsai/bim/module/model/opening.py | 9 +++ src/bonsai/bonsai/bim/module/void/operator.py | 12 +--- src/bonsai/bonsai/tool/model.py | 7 +-- .../model/test_host_add_opening_gizmo.py | 55 +++++++++++++++++-- 5 files changed, 78 insertions(+), 26 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/model/host_add_opening_gizmo.py b/src/bonsai/bonsai/bim/module/model/host_add_opening_gizmo.py index d30ab29bba..ff0e692bac 100644 --- a/src/bonsai/bonsai/bim/module/model/host_add_opening_gizmo.py +++ b/src/bonsai/bonsai/bim/module/model/host_add_opening_gizmo.py @@ -33,6 +33,7 @@ from mathutils import Vector import bonsai.tool as tool from bonsai.bim.module.drawing import gizmos as gizmo +from bonsai.bim.module.model.opening import is_filling_supported from bonsai.bim.module.model.wall import ( _get_wall_geom_cached, _wall_camera_facing_icon_y, @@ -52,16 +53,18 @@ def is_supported_host(element) -> bool: return tool.Parametric.is_path_connectable_wall(element) or element.is_a("IfcSlab") or element.is_a("IfcRoof") -def is_supported_filling(element) -> bool: - """Total predicate. A ``None`` element (raw Blender mesh) is accepted - because the apply-opening operator converts unclassified meshes into - ``IfcOpeningElement`` instances. IFC entities are accepted only when - their class is one the operator can dispatch on: ``IfcDoor`` / - ``IfcWindow`` (filled openings) or ``IfcOpeningElement`` (existing - opening reassigned to a new host).""" +def is_supported_filling_or_opening(element) -> bool: + """Total predicate for the add-opening gizmo poll. ``None`` (raw Blender + mesh) is accepted because the operator converts unclassified meshes + into ``IfcOpeningElement`` instances. ``IfcOpeningElement`` is accepted + because reassigning an existing opening to a new host is a legal path + through the operator. Otherwise defer to the generator's own + supported-filling predicate.""" if element is None: return True - return element.is_a("IfcDoor") or element.is_a("IfcWindow") or element.is_a("IfcOpeningElement") + if element.is_a("IfcOpeningElement"): + return True + return is_filling_supported(element) def _resolve_active_host(context: bpy.types.Context, n_selected: int): @@ -126,7 +129,7 @@ class GizmoHostAddOpening(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMixin return False if not hasattr(host_element, "HasOpenings"): return False - return is_supported_filling(filling_element) + return is_supported_filling_or_opening(filling_element) def setup(self, context: bpy.types.Context) -> None: default_color, highlight_color = self.get_decoration_colors() diff --git a/src/bonsai/bonsai/bim/module/model/opening.py b/src/bonsai/bonsai/bim/module/model/opening.py index fffa84c7c7..0508883c35 100644 --- a/src/bonsai/bonsai/bim/module/model/opening.py +++ b/src/bonsai/bonsai/bim/module/model/opening.py @@ -240,6 +240,15 @@ def _store_batch_in_cache(cache_key: tuple[int, str], batch: "gpu.types.GPUBatch _batch_cache[cache_key] = (epoch, batch) +def is_filling_supported(element) -> bool: + """True when Bonsai's opening generator can derive an opening from this + element. IFC's schema permits any IfcElement as a filling; Bonsai + currently supports only IfcDoor and IfcWindow because those are the + classes with OverallWidth/OverallHeight attributes (or their types' + ELEVATION_VIEW profiles) that the generator can consume.""" + return element is not None and element.is_a() in ("IfcDoor", "IfcWindow") + + class FilledOpeningGenerator: def generate( self, diff --git a/src/bonsai/bonsai/bim/module/void/operator.py b/src/bonsai/bonsai/bim/module/void/operator.py index b001e29f45..433ea3a06b 100644 --- a/src/bonsai/bonsai/bim/module/void/operator.py +++ b/src/bonsai/bonsai/bim/module/void/operator.py @@ -26,7 +26,7 @@ import bonsai.bim.handler import bonsai.core.geometry import bonsai.core.root import bonsai.tool as tool -from bonsai.bim.module.model.opening import FilledOpeningGenerator +from bonsai.bim.module.model.opening import FilledOpeningGenerator, is_filling_supported class AddOpening(bpy.types.Operator, tool.Ifc.Operator): @@ -86,15 +86,9 @@ class AddOpening(bpy.types.Operator, tool.Ifc.Operator): self.report({"INFO"}, "You can't add an opening to another opening.") continue elif not element1.is_a("IfcOpeningElement") and not element2.is_a("IfcOpeningElement"): - # Bonsai currently derives opening geometry only from - # IfcDoor and IfcWindow (via OverallWidth/OverallHeight - # or their type's ELEVATION_VIEW profile). IFC's schema - # permits any IfcElement as a filling; broadening this - # gate is future work in the opening generator, not a - # schema requirement. - if element1.is_a("IfcWindow") or element1.is_a("IfcDoor"): # Add a fill to an element. + if is_filling_supported(element1): # Add a fill to an element. obj1, obj2 = obj2, obj1 - elif not (element2.is_a("IfcWindow") or element2.is_a("IfcDoor")): + elif not is_filling_supported(element2): self.report( {"INFO"}, f"Cannot apply {element2.is_a()} as an opening — Bonsai currently supports only IfcDoor and IfcWindow as parametric fillings.", diff --git a/src/bonsai/bonsai/tool/model.py b/src/bonsai/bonsai/tool/model.py index 9babc968ba..dee8c218f2 100644 --- a/src/bonsai/bonsai/tool/model.py +++ b/src/bonsai/bonsai/tool/model.py @@ -2065,9 +2065,8 @@ class Model(bonsai.core.tool.Model): it matches ``filling``'s current parametric dimensions. Returns the voided host Blender object so the caller can recut it, - or ``None`` if ``filling`` has no opening to refresh. Callers - targeting a single user-selected filling should use this rather than - the family-wide variant to avoid touching unrelated sibling sources.""" + or ``None`` if ``filling`` has no opening to refresh or the host is + an aggregate (no mesh data to recut against).""" from bonsai.bim.module.model.opening import FilledOpeningGenerator if not filling.FillsVoids: @@ -2076,7 +2075,7 @@ class Model(bonsai.core.tool.Model): ifc_file = tool.Ifc.get() opening = filling.FillsVoids[0].RelatingOpeningElement voided_obj = tool.Ifc.get_object(opening.VoidsElements[0].RelatingBuildingElement) - if voided_obj is None: + if voided_obj is None or voided_obj.data is None: return None old_representation = tool.Geometry.get_body_representation(opening) diff --git a/src/bonsai/test/bim/module/model/test_host_add_opening_gizmo.py b/src/bonsai/test/bim/module/model/test_host_add_opening_gizmo.py index 52817c2b05..86275c660e 100644 --- a/src/bonsai/test/bim/module/model/test_host_add_opening_gizmo.py +++ b/src/bonsai/test/bim/module/model/test_host_add_opening_gizmo.py @@ -61,16 +61,19 @@ _IFC_CLASS_BY_KIND = { class _FakeIfcEntity: """Minimal stand-in for an ``ifcopenshell.entity_instance`` in poll tests. - Provides the two surfaces the gizmo's poll consults: ``is_a(type_name)`` - (used directly by ``is_supported_host`` for slab/roof) and an optional - ``HasOpenings`` attribute (probed by the poll's ``hasattr`` guard).""" + Mirrors ``ifcopenshell.entity_instance.is_a``'s two call shapes: + ``is_a("Foo")`` returns True when the entity's class is ``Foo``, and + ``is_a()`` returns the class name as a string. ``HasOpenings`` is + optional so the poll's ``hasattr`` guard branch is reachable.""" def __init__(self, ifc_class: str, has_openings: bool = True): self._ifc_class = ifc_class if has_openings: self.HasOpenings = () - def is_a(self, type_name: str) -> bool: + def is_a(self, type_name: str | None = None): + if type_name is None: + return self._ifc_class return self._ifc_class == type_name @@ -339,6 +342,50 @@ def test_layer3_branch_always_parks_above_top_face(patched_tool, other_z): assert pos.z == pytest.approx(0.2 + BaseParametricGizmoGroup.ICON_Z_OFFSET) +def test_position_gizmos_identifies_host_by_class_when_selected_second(patched_tool): + """Host role in ``position_gizmos`` is resolved by IFC class, not by + active-object position — so a slab clicked SECOND (filling first, + host active or not) still anchors the icon correctly on the slab. + This pins the selection-order independence of the positioner (the + poll's independence is covered separately by the poll parametrize).""" + from bonsai.bim.module.drawing import gizmos as gizmo_module + from bonsai.bim.module.model.host_add_opening_gizmo import GizmoHostAddOpening + + other = SimpleNamespace(matrix_world=Matrix.Translation(Vector((0.7, 0.4, 1.0)))) + host_obj = SimpleNamespace(matrix_world=Matrix.Identity(4), bound_box=[(0.0, 0.0, 0.0), (0.0, 0.0, 0.2)] * 4) + # Host at index 1; the filling (no IFC entity) sits at index 0 as active. + selected = [other, host_obj] + context = SimpleNamespace(active_object=other) + icon = SimpleNamespace(matrix_basis=None, hide=True) + self_stub = SimpleNamespace(add_opening_icon=icon) + + entity_map = {id(host_obj): _FakeIfcEntity("IfcSlab"), id(other): None} + with contextlib.ExitStack() as stack: + stack.enter_context( + patched_tool( + selected_list=selected, + entity=lambda o: entity_map.get(id(o)), + modifier_predicates={"is_path_connectable_wall": False}, + ) + ) + stack.enter_context(patch.object(gizmo_module, "get_billboard_rotation", return_value=Matrix.Identity(4))) + stack.enter_context( + patch.object( + gizmo_module, "billboarded_at", side_effect=lambda pos, rot, scale=0.5: Matrix.Translation(pos) + ) + ) + GizmoHostAddOpening.position_gizmos(self_stub, context) + + # Icon anchors on the host's top face (slab bound_box top-Z = 0.2) at + # the void's XY — same result as when the host was at index 0. + from bonsai.bim.module.drawing.gizmos import BaseParametricGizmoGroup + + pos = icon.matrix_basis.translation + assert pos.x == pytest.approx(0.7) + assert pos.y == pytest.approx(0.4) + assert pos.z == pytest.approx(0.2 + BaseParametricGizmoGroup.ICON_Z_OFFSET) + + # --------------------------------------------------------------------------- # is_supported_host() — predicate totality # ---------------------------------------------------------------------------