mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-09-20 15:08:51 +00:00
Bonsai: fix Apply Opening crash on non-fillings
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.
This commit is contained in:
@@ -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")
|
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):
|
def _resolve_active_host(context: bpy.types.Context, n_selected: int):
|
||||||
"""Shared poll prologue: gizmo gate + selection cardinality + active-in-
|
"""Shared poll prologue: gizmo gate + selection cardinality + active-in-
|
||||||
selected + IFC entity lookup + supported-host predicate. Returns the
|
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):
|
class GizmoHostAddOpening(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMixin):
|
||||||
"""Activates when a host element (wall / slab / roof) is the active object
|
"""Activates when exactly two objects are selected and one is a fillable
|
||||||
and exactly one other selected object is *not* itself a host.
|
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
|
Selection-order independent: the host role is identified by class, not
|
||||||
projected location on the host. A click dispatches ``bim.add_opening``,
|
by active state. The "+" icon anchors on the host's surface regardless
|
||||||
which handles any element exposing the ``HasOpenings`` inverse.
|
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
|
Per-frame positioning keeps the icon facing the camera as the viewport
|
||||||
orbits."""
|
orbits."""
|
||||||
@@ -90,22 +104,29 @@ class GizmoHostAddOpening(bpy.types.GizmoGroup, _WallGeomCachedBillboardingMixin
|
|||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def poll(cls, context: bpy.types.Context) -> bool:
|
def poll(cls, context: bpy.types.Context) -> bool:
|
||||||
element = _resolve_active_host(context, n_selected=2)
|
if not _wall_gizmo_poll_gate(context):
|
||||||
if element is None:
|
|
||||||
return False
|
return False
|
||||||
# The operator itself filters on HasOpenings, but checking here keeps
|
selected = list(tool.Blender.get_selected_objects())
|
||||||
# the icon from appearing on host classes that can't accept openings
|
if len(selected) != 2:
|
||||||
# in the active IFC schema.
|
|
||||||
if not hasattr(element, "HasOpenings"):
|
|
||||||
return False
|
return False
|
||||||
active = context.active_object
|
active = context.active_object
|
||||||
other = next(o for o in tool.Blender.get_selected_objects() if o is not active)
|
if active is None or active not in selected:
|
||||||
# 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)):
|
|
||||||
return False
|
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:
|
def setup(self, context: bpy.types.Context) -> None:
|
||||||
default_color, highlight_color = self.get_decoration_colors()
|
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:
|
def position_gizmos(self, context: bpy.types.Context) -> None:
|
||||||
host_obj = context.active_object
|
selected = list(tool.Blender.get_selected_objects())
|
||||||
if not host_obj:
|
if len(selected) != 2:
|
||||||
return
|
return
|
||||||
selected = tool.Blender.get_selected_objects()
|
a, b = selected[0], selected[1]
|
||||||
other = next((o for o in selected if o is not host_obj), None)
|
a_element = tool.Ifc.get_entity(a)
|
||||||
if not other:
|
b_element = tool.Ifc.get_entity(b)
|
||||||
return
|
if is_supported_host(a_element):
|
||||||
element = tool.Ifc.get_entity(host_obj)
|
host_obj, host_element, other = a, a_element, b
|
||||||
if not element:
|
elif is_supported_host(b_element):
|
||||||
|
host_obj, host_element, other = b, b_element, a
|
||||||
|
else:
|
||||||
return
|
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)
|
world_pos = wall_anchor(context, self, host_obj, other)
|
||||||
else:
|
else:
|
||||||
world_pos = layer3_anchor(host_obj, other)
|
world_pos = layer3_anchor(host_obj, other)
|
||||||
|
|||||||
@@ -34,11 +34,13 @@ class AddOpening(bpy.types.Operator, tool.Ifc.Operator):
|
|||||||
bl_label = "Apply Opening"
|
bl_label = "Apply Opening"
|
||||||
bl_options = {"REGISTER", "UNDO"}
|
bl_options = {"REGISTER", "UNDO"}
|
||||||
bl_description = (
|
bl_description = (
|
||||||
"Apply opening objects to an Element.\n\n"
|
"Cuts openings in a wall, slab, or roof using selected shape objects — "
|
||||||
"The Element and the openings to be applied should be selected. The order of selection is not important.\n"
|
"doors, windows, existing openings, or plain (non-IFC) meshes. "
|
||||||
"Opening can be just a Blender mesh object.\n\n"
|
"Selection order doesn't matter.\n\n"
|
||||||
"Shift+click: keep the filling at its current matrix_world — skip the wall-axis snap "
|
"Doors and windows also fill the opening. Other IFC classes are currently "
|
||||||
"and the rl1/rl2 Z-elevation default that the regular click applies."
|
"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
|
# 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.")
|
self.report({"INFO"}, "You can't add an opening to another opening.")
|
||||||
continue
|
continue
|
||||||
elif not element1.is_a("IfcOpeningElement") and not element2.is_a("IfcOpeningElement"):
|
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 element1.is_a("IfcWindow") or element1.is_a("IfcDoor"): # Add a fill to an element.
|
||||||
obj1, obj2 = obj2, obj1
|
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(
|
FilledOpeningGenerator().generate(
|
||||||
obj2,
|
obj2,
|
||||||
obj1,
|
obj1,
|
||||||
|
|||||||
@@ -51,6 +51,10 @@ _IFC_CLASS_BY_KIND = {
|
|||||||
"slab": "IfcSlab",
|
"slab": "IfcSlab",
|
||||||
"roof": "IfcRoof",
|
"roof": "IfcRoof",
|
||||||
"plain": "IfcDiscreteAccessory",
|
"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
|
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):
|
def test_poll_rejects_active_host_without_has_openings(patched_tool):
|
||||||
# Real-world equivalent: an IFC class that the active schema strips
|
# Real-world equivalent: an IFC class that the active schema strips
|
||||||
# ``HasOpenings`` from (e.g., a non-element subtype). The active sentinel
|
# ``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)
|
icon = SimpleNamespace(matrix_basis=None, hide=True)
|
||||||
self_stub = SimpleNamespace(add_opening_icon=icon)
|
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:
|
with contextlib.ExitStack() as stack:
|
||||||
stack.enter_context(
|
stack.enter_context(
|
||||||
patched_tool(
|
patched_tool(
|
||||||
selected_list=selected,
|
selected_list=selected,
|
||||||
entity=host_element,
|
entity=lambda o: entity_map.get(id(o)),
|
||||||
modifier_predicates={"is_path_connectable_wall": is_path_connectable_wall},
|
modifier_predicates={"is_path_connectable_wall": is_path_connectable_wall},
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user