From fdf9970685227899ca992b84b3a03a695dacd876 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 2 Jul 2026 09:02:02 +0200 Subject: [PATCH] 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}, ) )