From 041306c5f003fd0010af3044924173c9a868dd47 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 2 Jul 2026 11:44:33 +0200 Subject: [PATCH] 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 # ---------------------------------------------------------------------------