mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-11 18:16:40 +00:00
Warn on shared-rep parametric edits
A user clicking the pen icon on a typed-product occurrence whose body representation is mapped from its type would silently mutate every sibling occurrence's geometry. Add a confirmation dialog at the pen-icon dispatcher (the single chokepoint every feature routes through) showing the sibling count, with a session-scoped suppress checkbox. The check is read-only: tool.Model.get_sibling_occurrence_count wraps tool.Geometry.get_elements_by_representation against the resolved body rep and subtracts self + type. A forward-compat AST guard pins the dispatcher monopoly so any future feature that binds pen_gizmo directly to a feature-specific enable op fails the test before merge. Generated with the assistance of an AI coding tool.
This commit is contained in:
committed by
Thomas Krijnen
parent
83d4283934
commit
0319763376
@@ -192,6 +192,7 @@ classes = (
|
||||
prop.BIMBendPreviewProperties,
|
||||
prop.BIMWallFilletPreviewProperties,
|
||||
prop.BIMPreviewProperties,
|
||||
prop.BIMParametricEditDialogPrefs,
|
||||
ui.BIM_PT_array,
|
||||
ui.BIM_PT_stair,
|
||||
ui.BIM_PT_wall,
|
||||
@@ -349,6 +350,9 @@ def register():
|
||||
type=prop.BIMExternalParametricGeometryProperties
|
||||
)
|
||||
bpy.types.Scene.BIMPreviewProperties = bpy.props.PointerProperty(type=prop.BIMPreviewProperties)
|
||||
bpy.types.WindowManager.BIMParametricEditDialogPrefs = bpy.props.PointerProperty(
|
||||
type=prop.BIMParametricEditDialogPrefs
|
||||
)
|
||||
|
||||
bpy.types.VIEW3D_MT_add.prepend(ui.add_menu)
|
||||
bpy.app.handlers.load_post.append(handler.load_post)
|
||||
@@ -374,6 +378,7 @@ def unregister():
|
||||
tool.Parametric.unregister_object_properties()
|
||||
del bpy.types.Object.BIMExternalParametricGeometryProperties
|
||||
del bpy.types.Scene.BIMPreviewProperties
|
||||
del bpy.types.WindowManager.BIMParametricEditDialogPrefs
|
||||
|
||||
bpy.app.handlers.load_post.remove(handler.load_post)
|
||||
bpy.types.VIEW3D_MT_add.remove(ui.add_menu)
|
||||
|
||||
@@ -15,6 +15,8 @@
|
||||
#
|
||||
# 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 modified with the assistance of an AI coding tool.
|
||||
|
||||
import json
|
||||
from typing import ClassVar
|
||||
@@ -881,6 +883,36 @@ class EnableEditingParametric(bpy.types.Operator):
|
||||
default="",
|
||||
description="Operator bl_idname to invoke (e.g., 'bim.enable_editing_door').",
|
||||
)
|
||||
sibling_count: bpy.props.IntProperty(default=0, options={"HIDDEN"})
|
||||
|
||||
@staticmethod
|
||||
def should_show_shared_rep_dialog(*, suppress: bool, has_entity: bool, sibling_count: int) -> bool:
|
||||
"""Pure decision for the pre-edit warning. Returns ``True`` only when the
|
||||
edit will silently mutate other elements' geometry AND the user has not
|
||||
opted out of the warning for this session."""
|
||||
if suppress or not has_entity:
|
||||
return False
|
||||
return sibling_count > 0
|
||||
|
||||
def invoke(self, context, event):
|
||||
prefs = getattr(context.window_manager, "BIMParametricEditDialogPrefs", None)
|
||||
suppress = bool(prefs and prefs.suppress_shared_rep_warning)
|
||||
obj = context.active_object
|
||||
element = tool.Ifc.get_entity(obj) if obj else None
|
||||
self.sibling_count = tool.Model.get_sibling_occurrence_count(element) if element is not None else 0
|
||||
if self.should_show_shared_rep_dialog(
|
||||
suppress=suppress, has_entity=element is not None, sibling_count=self.sibling_count
|
||||
):
|
||||
return context.window_manager.invoke_props_dialog(self, width=400)
|
||||
return self.execute(context)
|
||||
|
||||
def draw(self, context):
|
||||
layout = self.layout
|
||||
layout.label(text="Shared geometry", icon="ERROR")
|
||||
layout.label(text=f"Geometry is shared with {self.sibling_count} other element(s).")
|
||||
layout.label(text="Edits will affect them too.")
|
||||
prefs = context.window_manager.BIMParametricEditDialogPrefs
|
||||
layout.prop(prefs, "suppress_shared_rep_warning", text="Don't show this again for this session")
|
||||
|
||||
def execute(self, context):
|
||||
# Malformed ``feature_enable_op`` (missing dot) would otherwise crash
|
||||
|
||||
@@ -2149,3 +2149,23 @@ class BIMPreviewProperties(PropertyGroup):
|
||||
if TYPE_CHECKING:
|
||||
bend: BIMBendPreviewProperties
|
||||
wall_fillet: BIMWallFilletPreviewProperties
|
||||
|
||||
|
||||
class BIMParametricEditDialogPrefs(PropertyGroup):
|
||||
"""Session-scoped flag for the parametric-edit pen-icon dispatcher.
|
||||
|
||||
Attached to ``WindowManager`` so the state lives for one Blender session
|
||||
and resets on restart — the right scope for "don't show this again for
|
||||
this session" toggles."""
|
||||
|
||||
suppress_shared_rep_warning: bpy.props.BoolProperty(
|
||||
name="Suppress shared-representation warning",
|
||||
description=(
|
||||
"When true, the pen-icon dispatcher skips the shared-geometry "
|
||||
"confirmation dialog. Resets on Blender restart."
|
||||
),
|
||||
default=False,
|
||||
)
|
||||
|
||||
if TYPE_CHECKING:
|
||||
suppress_shared_rep_warning: bool
|
||||
|
||||
@@ -372,6 +372,28 @@ class Model(bonsai.core.tool.Model):
|
||||
else:
|
||||
break
|
||||
|
||||
@classmethod
|
||||
def get_sibling_occurrence_count(cls, element: ifcopenshell.entity_instance) -> int:
|
||||
"""Number of *other* products sharing this element's body representation.
|
||||
|
||||
Returns the count of products bound to the same resolved body rep, minus
|
||||
``element`` itself and minus its type (if any). Zero when the element has
|
||||
no body rep, no resolved rep, or no siblings. A non-zero result means a
|
||||
parametric edit on ``element`` will silently mutate other instances'
|
||||
geometry."""
|
||||
body_rep = tool.Geometry.get_body_representation(element)
|
||||
if not body_rep:
|
||||
return 0
|
||||
resolved = ifcopenshell.util.representation.resolve_representation(body_rep)
|
||||
if not resolved:
|
||||
return 0
|
||||
elements = tool.Geometry.get_elements_by_representation(resolved)
|
||||
elements.discard(element)
|
||||
element_type = ifcopenshell.util.element.get_type(element)
|
||||
if element_type is not None:
|
||||
elements.discard(element_type)
|
||||
return len(elements)
|
||||
|
||||
unit_scale: float
|
||||
vertices: list[Vector]
|
||||
edges: list[Sequence[int]]
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
# 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.
|
||||
|
||||
"""Tests for the universal pen-icon dispatcher's pre-edit warning path.
|
||||
|
||||
The dispatcher gates the parametric-edit triad behind a confirmation dialog
|
||||
whenever the active element's body representation is shared with sibling
|
||||
occurrences (typed product + mapped representation). It is the single
|
||||
chokepoint every feature's pen icon routes through, so the warning applies
|
||||
to walls, doors, windows, stairs, roofs, and any future feature uniformly.
|
||||
|
||||
These tests exercise:
|
||||
|
||||
- the pure ``should_show_shared_rep_dialog`` decision (every branch); and
|
||||
- one end-to-end invocation through ``bpy.ops`` to pin the wiring between
|
||||
the decision and ``invoke_props_dialog``."""
|
||||
|
||||
import types
|
||||
|
||||
import bpy
|
||||
import pytest
|
||||
|
||||
from bonsai.bim.module.model.array import EnableEditingParametric
|
||||
|
||||
pytestmark = pytest.mark.model
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _require_real_bpy():
|
||||
if not isinstance(bpy, types.ModuleType) or hasattr(bpy, "_mock_name"):
|
||||
pytest.skip("requires real Blender (bpy is mocked or absent)")
|
||||
|
||||
|
||||
class TestShouldShowSharedRepDialog:
|
||||
"""Exhaustive truth table for the pre-edit-warning decision. Keeping this
|
||||
pure (no bpy, no operator instance) means a future change to the dispatch
|
||||
wiring can't silently flip a branch — the decision is independently pinned."""
|
||||
|
||||
decide = staticmethod(EnableEditingParametric.should_show_shared_rep_dialog)
|
||||
|
||||
def test_shared_rep_with_warning_enabled_shows_dialog(self):
|
||||
assert self.decide(suppress=False, has_entity=True, sibling_count=3) is True
|
||||
|
||||
def test_unique_rep_skips_dialog(self):
|
||||
assert self.decide(suppress=False, has_entity=True, sibling_count=0) is False
|
||||
|
||||
def test_session_suppress_overrides_shared_rep(self):
|
||||
assert self.decide(suppress=True, has_entity=True, sibling_count=5) is False
|
||||
|
||||
def test_no_entity_skips_dialog_even_when_count_positive(self):
|
||||
assert self.decide(suppress=False, has_entity=False, sibling_count=3) is False
|
||||
|
||||
def test_zero_siblings_skips_dialog_regardless_of_suppress(self):
|
||||
assert self.decide(suppress=False, has_entity=True, sibling_count=0) is False
|
||||
assert self.decide(suppress=True, has_entity=True, sibling_count=0) is False
|
||||
|
||||
|
||||
def test_dispatcher_falls_through_to_feature_enable_op_when_no_active_object():
|
||||
"""End-to-end smoke: with no active object the dispatcher short-circuits to
|
||||
its ``execute`` body, which CANCELs on an empty ``feature_enable_op``."""
|
||||
bpy.context.window_manager.BIMParametricEditDialogPrefs.suppress_shared_rep_warning = False
|
||||
try:
|
||||
with bpy.context.temp_override(active_object=None):
|
||||
result = bpy.ops.bim.enable_editing_parametric("INVOKE_DEFAULT", feature_enable_op="")
|
||||
finally:
|
||||
bpy.context.window_manager.BIMParametricEditDialogPrefs.suppress_shared_rep_warning = False
|
||||
assert result == {"CANCELLED"}
|
||||
@@ -0,0 +1,91 @@
|
||||
# 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 contract for the pen-icon dispatcher monopoly.
|
||||
|
||||
Every parametric gizmo group's pen icon must bind to the universal
|
||||
``bim.enable_editing_parametric`` dispatcher rather than the feature's own
|
||||
enable operator. The dispatcher is the single chokepoint where pre-edit
|
||||
checks (shared-representation warning, future safety gates) run; a feature
|
||||
that binds directly bypasses every such check silently."""
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
pytestmark = pytest.mark.drawing
|
||||
|
||||
|
||||
BONSAI_ROOT = Path(__file__).parent.parent.parent / "bonsai"
|
||||
BIM_DIR = BONSAI_ROOT / "bim"
|
||||
DISPATCHER_IDNAME = "bim.enable_editing_parametric"
|
||||
|
||||
|
||||
def _iter_pen_gizmo_target_set_operator_calls(tree: ast.Module):
|
||||
"""Yield each ``ast.Call`` matching ``<receiver>.pen_gizmo.target_set_operator(...)``.
|
||||
Receiver is any attribute access (``self.pen_gizmo``, ``group.pen_gizmo``, etc.)."""
|
||||
for node in ast.walk(tree):
|
||||
if not isinstance(node, ast.Call):
|
||||
continue
|
||||
func = node.func
|
||||
if not isinstance(func, ast.Attribute) or func.attr != "target_set_operator":
|
||||
continue
|
||||
receiver = func.value
|
||||
if not isinstance(receiver, ast.Attribute) or receiver.attr != "pen_gizmo":
|
||||
continue
|
||||
yield node
|
||||
|
||||
|
||||
def test_every_pen_gizmo_binding_routes_through_the_universal_dispatcher() -> None:
|
||||
violations: list[str] = []
|
||||
found_any = False
|
||||
for path in BIM_DIR.rglob("*.py"):
|
||||
try:
|
||||
tree = ast.parse(path.read_text(encoding="utf-8"))
|
||||
except SyntaxError:
|
||||
continue
|
||||
for call in _iter_pen_gizmo_target_set_operator_calls(tree):
|
||||
found_any = True
|
||||
if not call.args:
|
||||
violations.append(f"{path}:{call.lineno} pen_gizmo.target_set_operator() called with no args")
|
||||
continue
|
||||
first_arg = call.args[0]
|
||||
if not isinstance(first_arg, ast.Constant) or not isinstance(first_arg.value, str):
|
||||
violations.append(
|
||||
f"{path}:{call.lineno} pen_gizmo.target_set_operator() first arg is not a string literal"
|
||||
)
|
||||
continue
|
||||
if first_arg.value != DISPATCHER_IDNAME:
|
||||
violations.append(
|
||||
f"{path}:{call.lineno} pen_gizmo.target_set_operator({first_arg.value!r}) "
|
||||
f"bypasses the universal dispatcher"
|
||||
)
|
||||
|
||||
assert found_any, (
|
||||
"No pen_gizmo.target_set_operator(...) calls found anywhere under bim/. "
|
||||
"Either the gizmo-binding pattern has been refactored away (this test "
|
||||
"needs updating) or the search root is wrong."
|
||||
)
|
||||
assert not violations, (
|
||||
"Pen-icon bindings must route through the universal dispatcher "
|
||||
f"({DISPATCHER_IDNAME!r}) so the shared-representation warning and any "
|
||||
"future pre-edit checks apply to every feature. Violations:\n " + "\n ".join(violations)
|
||||
)
|
||||
@@ -934,3 +934,88 @@ class TestOffsetWall(NewFile):
|
||||
usage.DirectionSense = "NEGATIVE"
|
||||
subject.offset_wall(obj, "EXTERIOR")
|
||||
assert usage.OffsetFromReferenceLine == 100
|
||||
|
||||
|
||||
class TestGetSiblingOccurrenceCount(NewFile):
|
||||
"""The pen-icon dispatcher's pre-edit warning depends on this count: zero
|
||||
means the edit is safe (unique geometry), non-zero means the edit will
|
||||
silently mutate other instances sharing the same resolved body rep."""
|
||||
|
||||
def _make_body_subcontext(self, ifc: ifcopenshell.file) -> ifcopenshell.entity_instance:
|
||||
import ifcopenshell.api.context
|
||||
|
||||
ifcopenshell.api.root.create_entity(ifc, ifc_class="IfcProject", name="Project")
|
||||
parent = ifcopenshell.api.context.add_context(ifc, context_type="Model")
|
||||
return ifcopenshell.api.context.add_context(
|
||||
ifc,
|
||||
context_type="Model",
|
||||
context_identifier="Body",
|
||||
target_view="MODEL_VIEW",
|
||||
parent=parent,
|
||||
)
|
||||
|
||||
def _create_wall_with_body_rep(
|
||||
self,
|
||||
ifc: ifcopenshell.file,
|
||||
body_subcontext: ifcopenshell.entity_instance,
|
||||
name: str = "Wall",
|
||||
) -> ifcopenshell.entity_instance:
|
||||
wall = ifcopenshell.api.root.create_entity(ifc, ifc_class="IfcWall", name=name)
|
||||
rep = ifc.createIfcShapeRepresentation(
|
||||
ContextOfItems=body_subcontext,
|
||||
RepresentationIdentifier="Body",
|
||||
RepresentationType="SweptSolid",
|
||||
Items=[ifc.createIfcExtrudedAreaSolid()],
|
||||
)
|
||||
ifcopenshell.api.geometry.assign_representation(ifc, product=wall, representation=rep)
|
||||
return wall
|
||||
|
||||
def test_returns_zero_when_element_has_no_body_representation(self):
|
||||
ifc = ifcopenshell.file()
|
||||
tool.Ifc.set(ifc)
|
||||
wall = ifcopenshell.api.root.create_entity(ifc, ifc_class="IfcWall")
|
||||
assert subject.get_sibling_occurrence_count(wall) == 0
|
||||
|
||||
def test_returns_zero_when_element_has_unique_body_representation(self):
|
||||
ifc = ifcopenshell.file()
|
||||
tool.Ifc.set(ifc)
|
||||
body = self._make_body_subcontext(ifc)
|
||||
wall = self._create_wall_with_body_rep(ifc, body)
|
||||
assert subject.get_sibling_occurrence_count(wall) == 0
|
||||
|
||||
def test_returns_sibling_count_excluding_self_and_type(self):
|
||||
ifc = ifcopenshell.file()
|
||||
tool.Ifc.set(ifc)
|
||||
body = self._make_body_subcontext(ifc)
|
||||
wall_type = ifcopenshell.api.root.create_entity(ifc, ifc_class="IfcWallType", name="WAL01")
|
||||
type_rep = ifc.createIfcShapeRepresentation(
|
||||
ContextOfItems=body,
|
||||
RepresentationIdentifier="Body",
|
||||
RepresentationType="SweptSolid",
|
||||
Items=[ifc.createIfcExtrudedAreaSolid()],
|
||||
)
|
||||
ifcopenshell.api.geometry.assign_representation(ifc, product=wall_type, representation=type_rep)
|
||||
|
||||
occurrences = [ifcopenshell.api.root.create_entity(ifc, ifc_class="IfcWall", name=f"Wall{i}") for i in range(3)]
|
||||
ifcopenshell.api.type.assign_type(ifc, related_objects=occurrences, relating_type=wall_type)
|
||||
|
||||
assert subject.get_sibling_occurrence_count(occurrences[0]) == 2
|
||||
assert subject.get_sibling_occurrence_count(occurrences[1]) == 2
|
||||
assert subject.get_sibling_occurrence_count(occurrences[2]) == 2
|
||||
|
||||
def test_type_with_occurrences_reports_its_occurrence_count(self):
|
||||
ifc = ifcopenshell.file()
|
||||
tool.Ifc.set(ifc)
|
||||
body = self._make_body_subcontext(ifc)
|
||||
wall_type = ifcopenshell.api.root.create_entity(ifc, ifc_class="IfcWallType", name="WAL01")
|
||||
type_rep = ifc.createIfcShapeRepresentation(
|
||||
ContextOfItems=body,
|
||||
RepresentationIdentifier="Body",
|
||||
RepresentationType="SweptSolid",
|
||||
Items=[ifc.createIfcExtrudedAreaSolid()],
|
||||
)
|
||||
ifcopenshell.api.geometry.assign_representation(ifc, product=wall_type, representation=type_rep)
|
||||
occurrences = [ifcopenshell.api.root.create_entity(ifc, ifc_class="IfcWall", name=f"Wall{i}") for i in range(2)]
|
||||
ifcopenshell.api.type.assign_type(ifc, related_objects=occurrences, relating_type=wall_type)
|
||||
|
||||
assert subject.get_sibling_occurrence_count(wall_type) == 2
|
||||
|
||||
Reference in New Issue
Block a user