mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-06 07:51:47 +00:00
Fix fillet partner missing from wall unjoin gizmo
GizmoWallUnjoinSingle.poll accepts fillet-corner walls via the looser tool.Parametric.is_path_connectable_wall predicate (fillet corners have no LAYER2 usage by IFC spec, but they still participate in IfcRelConnectsPathElements). The partner filter inside _iter_path_connections used the stricter tool.Blender.Modifier.is_wall (LAYER2-only), so adjacent LAYER2 walls silently dropped their fillet-corner partners from the connection list — the unjoin icon appeared when the fillet wall itself was selected but not on either of its LAYER2 neighbours. Switch the partner filter to is_path_connectable_wall so host and partner predicates match. Add a regression test for the fillet case and an AST forward-compat guard pinning the predicate symbol so a future "tidy the imports" can't silently re-introduce the asymmetry. Generated with the assistance of an AI coding tool.
This commit is contained in:
@@ -2490,18 +2490,18 @@ def _iter_path_connections(
|
||||
if not rel.is_a("IfcRelConnectsPathElements"):
|
||||
continue
|
||||
other = rel.RelatedElement
|
||||
# `Modifier.is_wall(None)` raises on `None.is_a(...)` — guard before the
|
||||
# predicate runs. Malformed / partial IFC files can leave a rel's element
|
||||
# ref unset, and the gizmo loop must survive a stray None rather than
|
||||
# crashing the per-frame `position_gizmos`.
|
||||
if other is None or not tool.Blender.Modifier.is_wall(other):
|
||||
# Malformed / partial IFC files can leave a rel's element ref unset.
|
||||
# The partner predicate calls `.is_a(...)` on its argument, so a None
|
||||
# would raise mid-frame and silently break the gizmo group — guard
|
||||
# before the predicate runs.
|
||||
if other is None or not tool.Parametric.is_path_connectable_wall(other):
|
||||
continue
|
||||
out.append((other, rel.RelatingConnectionType, rel.RelatedConnectionType))
|
||||
for rel in getattr(elem, "ConnectedFrom", []):
|
||||
if not rel.is_a("IfcRelConnectsPathElements"):
|
||||
continue
|
||||
other = rel.RelatingElement
|
||||
if other is None or not tool.Blender.Modifier.is_wall(other):
|
||||
if other is None or not tool.Parametric.is_path_connectable_wall(other):
|
||||
continue
|
||||
out.append((other, rel.RelatedConnectionType, rel.RelatingConnectionType))
|
||||
return out
|
||||
|
||||
@@ -202,11 +202,11 @@ def _make_path_rel(relating, related, relating_ct, related_ct, kind="IfcRelConne
|
||||
)
|
||||
|
||||
|
||||
def _run_iter_path_connections(elem, *, is_wall_predicate=lambda _e: True):
|
||||
def _run_iter_path_connections(elem, *, partner_predicate=lambda _e: True):
|
||||
from bonsai import tool
|
||||
from bonsai.bim.module.model.wall import _iter_path_connections
|
||||
|
||||
with patch.object(tool.Blender.Modifier, "is_wall", side_effect=is_wall_predicate):
|
||||
with patch.object(tool.Parametric, "is_path_connectable_wall", side_effect=partner_predicate):
|
||||
return _iter_path_connections(elem)
|
||||
|
||||
|
||||
@@ -260,14 +260,30 @@ def test_iter_path_connections_skips_non_wall_partners():
|
||||
relating=self_elem, related=non_wall_partner, relating_ct="ATEND", related_ct="ATSTART"
|
||||
)
|
||||
elem = SimpleNamespace(ConnectedTo=[rel_wall, rel_non_wall], ConnectedFrom=[])
|
||||
result = _run_iter_path_connections(elem, is_wall_predicate=lambda e: e is wall_partner)
|
||||
result = _run_iter_path_connections(elem, partner_predicate=lambda e: e is wall_partner)
|
||||
assert result == [(wall_partner, "ATEND", "ATSTART")]
|
||||
|
||||
|
||||
def test_iter_path_connections_includes_fillet_corner_partner():
|
||||
# Fillet-corner walls carry no LAYER2 usage but are still valid path
|
||||
# partners. The enumeration must use the same predicate the gizmo group's
|
||||
# poll uses for the host wall — otherwise the corner is silently dropped
|
||||
# from the neighbour's connection list and looks unconnected from the
|
||||
# LAYER2 wall's perspective.
|
||||
self_elem = object()
|
||||
fillet_partner = object()
|
||||
rel = _make_path_rel(
|
||||
relating=self_elem, related=fillet_partner, relating_ct="ATEND", related_ct="ATSTART"
|
||||
)
|
||||
elem = SimpleNamespace(ConnectedTo=[rel], ConnectedFrom=[])
|
||||
result = _run_iter_path_connections(elem, partner_predicate=lambda e: e is fillet_partner)
|
||||
assert result == [(fillet_partner, "ATEND", "ATSTART")]
|
||||
|
||||
|
||||
def test_iter_path_connections_tolerates_none_partner_refs():
|
||||
# Malformed / partial IFC files can leave a rel's element ref unset.
|
||||
# Without a None guard, `Modifier.is_wall(None)` would raise on
|
||||
# `None.is_a(...)` mid-frame and silently break the gizmo group.
|
||||
# Without a None guard, the partner predicate would receive None and
|
||||
# raise on `.is_a(...)` mid-frame, silently breaking the gizmo group.
|
||||
self_elem = object()
|
||||
other = object()
|
||||
rel_none = _make_path_rel(relating=self_elem, related=None, relating_ct="ATEND", related_ct="ATSTART")
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
# 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 <http://www.gnu.org/licenses/>.
|
||||
#
|
||||
# This file was generated with the assistance of an AI coding tool.
|
||||
|
||||
"""Forward-compat AST contracts for wall gizmo internals.
|
||||
|
||||
Pins structural invariants that no per-call-site behavioural test can catch
|
||||
on its own: the kind of "someone tidied the imports" regression that leaves
|
||||
tests green but silently changes runtime semantics. Each contract names the
|
||||
invariant it pins so a future revert tells the contributor exactly what the
|
||||
rule is."""
|
||||
|
||||
import ast
|
||||
import inspect
|
||||
|
||||
import pytest
|
||||
|
||||
pytestmark = pytest.mark.wall
|
||||
|
||||
|
||||
def test_iter_path_connections_uses_path_connectable_predicate():
|
||||
"""The partner filter must consult the looser ``is_path_connectable_wall``
|
||||
predicate, matching the host-side predicate used by the gizmo group's
|
||||
poll. Strict ``is_wall`` rejects fillet-corner walls (which have no
|
||||
LAYER2 usage by IFC spec), so a regression to ``is_wall`` would silently
|
||||
drop fillet partners from the connection list — visible to the user as
|
||||
"the corner looks unconnected from the adjacent wall's selection.\""""
|
||||
from bonsai.bim.module.model.wall import _iter_path_connections
|
||||
|
||||
source = inspect.getsource(_iter_path_connections)
|
||||
tree = ast.parse(source)
|
||||
attr_names = {node.attr for node in ast.walk(tree) if isinstance(node, ast.Attribute)}
|
||||
|
||||
assert "is_path_connectable_wall" in attr_names, (
|
||||
"_iter_path_connections must filter partners with is_path_connectable_wall — "
|
||||
"the same predicate the gizmo group's poll uses on the host wall. "
|
||||
"Symmetry between host and partner predicates is required for fillet "
|
||||
"corners (no LAYER2 usage) to surface as connected from their LAYER2 "
|
||||
"neighbours' perspective."
|
||||
)
|
||||
assert "is_wall" not in attr_names, (
|
||||
"_iter_path_connections must NOT call .is_wall on partner elements — "
|
||||
"that strict predicate drops fillet-corner walls. Use "
|
||||
"is_path_connectable_wall instead."
|
||||
)
|
||||
Reference in New Issue
Block a user