From d43a1353e0db7f03de4f6f5c93cd9b5cd85885d2 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 27 May 2026 14:53:06 +0200 Subject: [PATCH 01/10] =?UTF-8?q?Add=20bim/decorator=5Fcache=20module=20?= =?UTF-8?q?=E2=80=94=20TokenCache=20+=20handler=20primitives?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New helper module for POST_VIEW decorators. Exports: * ``get_decorator_cache_token()`` — global int counter consumers include in their cache key so the value invalidates on structural scene changes. * ``_bump_decorator_cache_token()`` — ``@bpy.app.handlers.persistent`` callback that increments the token. Gates on the depsgraph payload so animation playback / driver evaluation doesn't churn the token. * ``install_decorator_cache_handlers`` / ``uninstall_…`` — idempotent append / remove against depsgraph_update_post + undo_post + redo_post + load_post. Called once from ``bim.register`` / ``unregister``. * ``TokenCache[T]`` — single-entry memoiser keyed on ``(caller_key, token)``. Cached ``bpy.types.Object`` references can't outlive the underlying ID blocks because any depsgraph / undo / load bumps the token and forces a recompute. This commit ships the module standalone. The next commits in this PR wire it: tool/system.py adds the cache wrap on get_decoration_data and bim/handler.py installs the bump callbacks. Until both land, the module is intentionally dead code — keeps the diff narrow and the commit history bisectable. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/decorator_cache.py | 119 +++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 src/bonsai/bonsai/bim/decorator_cache.py diff --git a/src/bonsai/bonsai/bim/decorator_cache.py b/src/bonsai/bonsai/bim/decorator_cache.py new file mode 100644 index 0000000000..118cb33017 --- /dev/null +++ b/src/bonsai/bonsai/bim/decorator_cache.py @@ -0,0 +1,119 @@ +# 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 . +# +# This file was generated with the assistance of an AI coding tool. + +"""Shared structural-change cache token for POST_VIEW decorators. + +Decorators include the token in their cache key and rebuild on bump.""" + +from __future__ import annotations + +from collections.abc import Callable +from typing import Any, Generic, TypeVar + +import bpy + +T = TypeVar("T") + +_DECORATOR_CACHE_TOKEN = 0 + + +def get_decorator_cache_token() -> int: + return _DECORATOR_CACHE_TOKEN + + +def reset_for_test() -> None: + """Test-only: reset the cache token to 0 so bump-count assertions are stable.""" + global _DECORATOR_CACHE_TOKEN + _DECORATOR_CACHE_TOKEN = 0 + + +@bpy.app.handlers.persistent +def _bump_decorator_cache_token(*args: Any) -> None: + """depsgraph_update_post fires every animation frame and every driver + evaluation, even when no IFC-relevant ID block changed. Unconditional + bumping defeats the cache: an animated scene rebuilds every decorator + every viewport tick. Gate the depsgraph path on Object geometry or + transform updates; undo / redo / load have no depsgraph and always + invalidate. + + Coverage assumption: ``TokenCache`` consumers key on Object identity + (depsgraph updates whose ``id`` is a ``bpy.types.Object``). Mesh / + Material / NodeTree updates that don't surface as an Object change + do NOT invalidate the token — a decorator that caches material- or + mesh-data-derived state must gate on a separate signal.""" + global _DECORATOR_CACHE_TOKEN + if len(args) >= 2: + depsgraph = args[1] + if depsgraph is not None and hasattr(depsgraph, "updates"): + if not any( + (getattr(u, "is_updated_geometry", False) or getattr(u, "is_updated_transform", False)) + and hasattr(u, "id") + and isinstance(u.id, bpy.types.Object) + for u in depsgraph.updates + ): + return + _DECORATOR_CACHE_TOKEN += 1 + + +def _hooks() -> tuple[Any, ...]: + return ( + bpy.app.handlers.depsgraph_update_post, + bpy.app.handlers.undo_post, + bpy.app.handlers.redo_post, + bpy.app.handlers.load_post, + ) + + +def install_decorator_cache_handlers() -> None: + """Append the bump handler to each hook; idempotent.""" + for hook in _hooks(): + if _bump_decorator_cache_token not in hook: + hook.append(_bump_decorator_cache_token) + + +def uninstall_decorator_cache_handlers() -> None: + for hook in _hooks(): + try: + hook.remove(_bump_decorator_cache_token) + except ValueError: + pass + + +class TokenCache(Generic[T]): + """Memoise a single value keyed on ``(caller_key, get_decorator_cache_token())``. + + The token component invalidates the cache on depsgraph / undo / redo / load, + so cached ``bpy.types.Object`` references can't outlive the underlying ID + blocks. Holds exactly one entry — last key wins.""" + + __slots__ = ("_key", "_value") + + def __init__(self) -> None: + self._key: tuple[Any, int] | None = None + self._value: T | None = None + + def get_or_compute(self, key: Any, compute: Callable[[], T]) -> T: + token_key = (key, _DECORATOR_CACHE_TOKEN) + if token_key == self._key: + return self._value # type: ignore[return-value] + value = compute() + self._key = token_key + self._value = value + return value From 4b9ad66c95f14569f64cfc9226fbf49183205d32 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 27 May 2026 14:55:44 +0200 Subject: [PATCH 02/10] Wrap tool.System.get_decoration_data with TokenCache lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit System decoration draws on every viewport refresh — the ``_build_decoration_data`` body walks every distribution element, resolves connected ports, builds the vert/edge arrays for the GPU batch. A bare call per frame burns time on an unchanged scene. Add a single-entry cache keyed on ``(decorator_cache_token, id(decorated_elements_set))``. Reads short-circuit when neither component moved: * ``decorator_cache_token`` from ``bim.decorator_cache`` invalidates on depsgraph / undo / redo / load via the bump handler. * ``id(decorated_elements_set)`` invalidates when ``SystemDecorationData.load()`` reassigns the set (e.g. when the user changes the set of decorated systems via the panel). The handler that bumps the token is installed in the next commit (bim/handler.py decompose). Until then the token stays at 0, so the cache only hits when ``id()`` also matches — degraded behaviour during the bisect window but not incorrect. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/tool/system.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/bonsai/bonsai/tool/system.py b/src/bonsai/bonsai/tool/system.py index bc11f3d642..8d2b421370 100644 --- a/src/bonsai/bonsai/tool/system.py +++ b/src/bonsai/bonsai/tool/system.py @@ -299,15 +299,29 @@ class System(bonsai.core.tool.System): system_props = cls.get_system_props() return tool.Ifc.get_entity_by_id(system_props.active_system_id) + # Decoration-data cache, keyed on (decorator_cache_token, id(decorated_elements_set)). + _decoration_data_cache_key: tuple | None = None + _decoration_data_cache: dict[str, Any] | None = None + @classmethod def get_decoration_data(cls) -> dict[str, Any]: + from bonsai.bim.decorator_cache import get_decorator_cache_token from bonsai.bim.module.system.data import ObjectSystemData, SystemDecorationData if not ObjectSystemData.is_loaded: ObjectSystemData.load() if not SystemDecorationData.is_loaded: SystemDecorationData.load() - return cls._build_decoration_data() + + token = get_decorator_cache_token() + key = (token, id(SystemDecorationData.data["decorated_elements"])) + if key == cls._decoration_data_cache_key and cls._decoration_data_cache is not None: + return cls._decoration_data_cache + + result = cls._build_decoration_data() + cls._decoration_data_cache_key = key + cls._decoration_data_cache = result + return result @classmethod def _build_decoration_data(cls) -> dict[str, Any]: From c9f12dd4417595b7dfea4b581aa141f22a15d7dd Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 27 May 2026 15:25:07 +0200 Subject: [PATCH 03/10] Add bim/module/model/preview_base module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shared helpers for Bonsai's Scene-level parametric preview flows. Two PR4 features will consume this — MEP bend preview and wall fillet preview — both following the same shape: EnablePreview — populates draft on Scene.BIMPreviewProperties. GizmoPreview — polls on is_active, surfaces tunable widgets PreviewDecorator — GPU lines while is_active is True FinishPreview — bpy.ops.bim.(...) with draft kwargs CancelPreview — pure state reset The module hosts the cross-cutting accessors (``get_preview_props``, ``is_preview_active``), lazy-closure factories for gizmo dimension callbacks (``make_props_callback`` / ``make_dim_getter`` / ``make_dim_setter`` — defensive against missing scene / freed RNA struct on file open / undo), the Enable-time IFC-placement sync (``sync_uncommitted_moves``), and the Esc + load_post discard machinery (``PREVIEW_CANCEL_OPS`` registry, ``try_cancel_active_preview``, ``discard_pending_previews``). Ships standalone — the consumer features land in PR4 (preview PropertyGroups, Enable/Finish/Cancel operators, gizmo groups, decorators, Esc keymap binding). All accessors are defensive against missing PropertyGroups / operators on v0.8.0 — calling ``discard_pending_previews(scene)`` from the next commit's load_post hook is a no-op until PR4 attaches BIMPreviewProperties. Generated with the assistance of an AI coding tool. --- .../bonsai/bim/module/model/preview_base.py | 211 ++++++++++++++++++ 1 file changed, 211 insertions(+) create mode 100644 src/bonsai/bonsai/bim/module/model/preview_base.py diff --git a/src/bonsai/bonsai/bim/module/model/preview_base.py b/src/bonsai/bonsai/bim/module/model/preview_base.py new file mode 100644 index 0000000000..f9921dd1aa --- /dev/null +++ b/src/bonsai/bonsai/bim/module/model/preview_base.py @@ -0,0 +1,211 @@ +# 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 . +# +# This file was generated with the assistance of an AI coding tool. + +"""Shared helpers for Bonsai's parametric preview flows. + +Multiple Bonsai features follow the same Scene-level preview pattern: + + EnablePreview — validates a selection, populates draft state on + ``Scene.BIMPreviewProperties.``, flips ``is_active``. + GizmoPreview — polls on ``is_active``, surfaces tunable widgets + + validate/cancel icons. + PreviewDecorator — GPU lines drawn while ``is_active`` is True. + FinishPreview — direct ``bpy.ops.bim.(...)`` call with kwargs + read off the draft state, then clears it. + CancelPreview — pure state reset. + +The MEP bend and wall fillet flows are the two current callers. They write +their Finish / Cancel operators directly, matching the convention used +throughout the rest of ``bim/module/model/`` for operator-to-operator +dispatch (explicit ``bpy.ops.bim.X(kwarg=value)`` at the call site, no +string indirection). This module hosts the cross-cutting accessors only; +no base class layer. + +The GPU draw-handler lifecycle for ``PreviewDecorator`` lives on the +feature-neutral ``tool.Blender.ViewportDecorator`` base, which every +viewport decorator (preview or otherwise) inherits from.""" + +from __future__ import annotations + +from collections.abc import Callable +from typing import Any + +import bpy + +import bonsai.tool as tool + +# --- Props accessors --------------------------------------------------------- + + +def get_preview_props(context: bpy.types.Context, attr: str): + """Resolve a child preview PropertyGroup under ``Scene.BIMPreviewProperties``. + + Returns ``None`` if the umbrella isn't attached yet — true briefly + during addon register and during plug-out, so polls / draw callbacks + must defend against ``None`` rather than assuming the prop is always + available.""" + preview = getattr(context.scene, "BIMPreviewProperties", None) + return getattr(preview, attr, None) if preview is not None else None + + +def is_preview_active(context: bpy.types.Context, attr: str) -> bool: + """``True`` while a specific preview is open. Used by sibling gizmo + polls to hide themselves so the preview is the only interactive + surface in the viewport (the bend / fillet preview groups take over + the same selection's icon stack).""" + props = get_preview_props(context, attr) + return bool(props is not None and props.is_active) + + +# --- Lazy closure factories -------------------------------------------------- +# +# Used by preview gizmo groups when wiring ``BIM_GT_gizmo_dimension``'s +# ``move_get_cb`` / ``move_set_cb`` callbacks. The closures re-resolve +# ``bpy.context.scene`` per CALL rather than capturing it at setup() time +# — the captured Scene's RNA struct can be freed on file open / undo, and +# referencing a freed struct crashes Blender. Lazy lookup survives the +# whole undo / reload lifecycle. + + +def make_props_callback(attr: str) -> Callable[[], Any]: + """Return a zero-arg callable that lazily fetches the preview props. + + Equivalent to ``getattr(bpy.context.scene.BIMPreviewProperties, attr)`` + with full defensiveness against missing scene / missing umbrella.""" + + def _props(): + scene = bpy.context.scene + preview = getattr(scene, "BIMPreviewProperties", None) if scene else None + return getattr(preview, attr, None) if preview is not None else None + + return _props + + +def make_dim_getter(props_callback: Callable[[], Any], field: str) -> Callable[[], float]: + """Factory for ``BIM_GT_gizmo_dimension.move_get_cb`` reading a single + FloatProperty off the live preview state. Returns ``0.0`` defensively + when the props are temporarily unavailable so the widget doesn't crash + Blender during plug-out / reload.""" + + def _get() -> float: + props = props_callback() + return getattr(props, field) if props is not None else 0.0 + + return _get + + +def make_dim_setter( + props_callback: Callable[[], Any], + field: str, + min_value: float = 0.001, +) -> Callable[[float], None]: + """Factory for ``BIM_GT_gizmo_dimension.move_set_cb`` writing a single + FloatProperty + tagging viewport areas for redraw so the GPU preview + decorator tracks the value live during drag. Clamps at ``min_value`` + to match the FloatProperty's declared lower bound.""" + + def _set(value: float) -> None: + props = props_callback() + if props is None: + return + setattr(props, field, max(min_value, float(value))) + for area in bpy.context.screen.areas if bpy.context.screen else (): + if area.type == "VIEW_3D": + area.tag_redraw() + + return _set + + +# --- Shared Enable lifecycle helpers ----------------------------------------- + + +def sync_uncommitted_moves(objects: list) -> None: + """Push any Blender-side translation / rotation of ``objects`` back to + their IFC ``ObjectPlacement`` before a preview decorator starts reading + ``obj.matrix_world`` per frame. + + Without this sync, a user who grabbed-moved an object but didn't commit + the move sees the live preview at the dragged position while the final + commit lands at the stale IFC position — a confusing "where did my + preview go?" experience. Both bend and fillet enable paths call this + on the relevant pair just before activating the preview.""" + for obj in objects: + tool.Geometry.commit_placement_if_moved(obj, apply_scale=False) + + +# --- Esc dispatch ------------------------------------------------------------ + +PREVIEW_CANCEL_OPS: tuple[tuple[str, str], ...] = ( + ("bend", "cancel_bend_preview"), + ("wall_fillet", "cancel_wall_fillet_preview"), +) +"""Registry of ``(child PointerProperty on Scene.BIMPreviewProperties, bim +operator name)`` consulted by the Esc handler. Adding a new preview means +appending one tuple; the forward-compat test pins that every preview +PropertyGroup with ``is_active`` has an entry here.""" + + +def try_cancel_active_preview(context: bpy.types.Context) -> bool: + """Cancel every registered preview that is currently active. + + Returns ``True`` iff at least one preview was cancelled. Multiple + previews can be simultaneously active (e.g. a stale bend preview opened + just before the user starts a wall fillet) — one Esc must clear them + all rather than forcing the user to tap Esc once per preview. + + Tags 3D viewports for redraw on success — the Esc keymap entry runs + outside a viewport mouse event so the gizmo poll wouldn't re-evaluate + until the next interaction without an explicit redraw.""" + cancelled = False + for attr, op_name in PREVIEW_CANCEL_OPS: + if is_preview_active(context, attr): + getattr(bpy.ops.bim, op_name)() + cancelled = True + if cancelled: + screen = context.screen + for area in screen.areas if screen else (): + if area.type == "VIEW_3D": + area.tag_redraw() + return cancelled + + +def discard_pending_previews(scene: bpy.types.Scene) -> None: + """Clear every active preview under ``Scene.BIMPreviewProperties`` so + saved preview state never resurfaces on file load. + + Mirrors ``tool.Parametric.heal_stale_edit_flags`` for the object-level + parametric-edit lifecycle — except previews are *discarded* rather than + validated. A preview's only UI cue is its in-viewport widget; reloading + a ``.blend`` saved mid-preview restores the flag but not the surrounding + user attention, and a stuck ``is_active`` silently hides every sibling + gizmo poll gated on it. + + Iterates ``PREVIEW_CANCEL_OPS`` so any preview registered for Esc + cancellation is automatically covered here too. Sets ``is_active`` + directly rather than dispatching the cancel operator: load_post may + fire before ``bpy.context.screen`` is reattached, and the cancel + operators bail on ``context.screen is None``.""" + preview = getattr(scene, "BIMPreviewProperties", None) + if preview is None: + return + for attr, _op_name in PREVIEW_CANCEL_OPS: + child = getattr(preview, attr, None) + if child is not None and getattr(child, "is_active", False): + child.is_active = False From 5e23030a0ff388927c1ffd85d84c863e0e8ea364 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 27 May 2026 15:28:18 +0200 Subject: [PATCH 04/10] Decompose bim/handler.py load_post + install cache + discard hooks Three concerns folded into ``load_post`` argue for separation: 1. Save-file invariants every load must re-establish (msgbus subscription, owner-settings, thumbnail cache, draft-flag healing, blend-warning flag, H5 lock probe). 2. User-preference-driven UI setup (toolbar, workspace, viewport shading, panel hijack, snap defaults). 3. Viewport overlay sync (every decorator's install/uninstall). Pull each into its own function (``_apply_save_file_invariants`` / ``_apply_user_preferences`` / ``_install_viewport_overlays``). The ``load_post`` callback becomes a 3-line orchestrator. Each phase is independently call-able from tests and from PR4 features that need to re-trigger one phase without the others. Two new hooks land with the decompose: * ``tool.Parametric.heal_stale_edit_flags()`` + ``discard_pending_previews(scene)`` fire in ``_apply_save_file_invariants``. The first clears object-level ``BIMProperties.is_editing`` flags that lost their backing IFC element across a load; the second clears scene-level ``BIMPreviewProperties..is_active`` so saved preview state never resurfaces with no UI to interact with it. * ``install_decorator_cache_handlers`` / ``uninstall_decorator_cache_handlers`` wrap the decorator install/install pass in ``_install_viewport_overlays``. The bump handlers append to ``depsgraph_update_post`` + ``undo_post`` + ``redo_post`` + ``load_post`` so the previous commit's ``TokenCache`` in ``tool.System.get_decoration_data`` finally invalidates on structural scene changes. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/handler.py | 88 +++++++++++++++++++++++--------- 1 file changed, 63 insertions(+), 25 deletions(-) diff --git a/src/bonsai/bonsai/bim/handler.py b/src/bonsai/bonsai/bim/handler.py index 38c7990653..f33c90fc6f 100644 --- a/src/bonsai/bonsai/bim/handler.py +++ b/src/bonsai/bonsai/bim/handler.py @@ -34,7 +34,11 @@ from mathutils import Vector import bonsai.bim import bonsai.core.model as core_model import bonsai.tool as tool -from bonsai.bim.ifc import IfcStore +from bonsai.bim.decorator_cache import ( + install_decorator_cache_handlers, + uninstall_decorator_cache_handlers, +) +from bonsai.bim.ifc import IfcStore, get_cache_or_detect_lock from bonsai.bim.module.aggregate.decorator import AggregateDecorator from bonsai.bim.module.georeference.decorator import GeoreferenceDecorator from bonsai.bim.module.model.data import AuthoringData @@ -43,6 +47,7 @@ from bonsai.bim.module.model.decorator import ( SlabDirectionDecorator, WallAxisDecorator, ) +from bonsai.bim.module.model.preview_base import discard_pending_previews from bonsai.bim.module.nest.decorator import NestDecorator cwd = os.path.dirname(os.path.realpath(__file__)) @@ -378,8 +383,10 @@ def subscribe_to_viewport_shading_changes(): ) -@persistent -def load_post(scene): +def _apply_save_file_invariants(scene: bpy.types.Scene) -> None: + """Invariants enforced on every load_post: msgbus subscription, IFC owner + settings, scene-bound caches, draft-flag healing, multi-instance lock probe, + and previews discarded so saved preview state never resurfaces on reopen.""" global global_subscription_owner active_object_key = bpy.types.LayerObjects, "active" bpy.msgbus.subscribe_rna( @@ -390,6 +397,24 @@ def load_post(scene): ifcopenshell.api.owner.settings.get_application = get_application AuthoringData.type_thumbnails = {} + tool.Parametric.heal_stale_edit_flags() + discard_pending_previews(scene) + + if tool.Ifc.get() and bpy.data.is_saved: + props = tool.Blender.get_bim_props() + props.has_blend_warning = True + + # Probe the H5 cooked-geometry cache so the multi-instance warning surfaces + # right after .blend load. Without this, the lock is only detected when a + # mutation triggers ``clear_cache`` — by which time the user has already + # made changes that may now conflict with the other Blender instance. + if tool.Ifc.get(): + get_cache_or_detect_lock() + + +def _apply_user_preferences() -> None: + """User-preference-driven UI setup: toolbar, BIM workspace, viewport shading + subscription, scene-panel hijack, tab layout, snap defaults.""" preferences = tool.Blender.get_addon_preferences() if not preferences.should_setup_toolbar: tool.Blender.unregister_toolbar() @@ -413,11 +438,21 @@ def load_post(scene): tool.Blender.override_scene_panel(panel) tool.Blender.setup_tabs() - if tool.Ifc.get() and bpy.data.is_saved: - props = tool.Blender.get_bim_props() - props.has_blend_warning = True + if preferences.should_use_snap and (scene := bpy.context.scene): + # Snapping is off by default in Blender, but in BIM, it's more useful to be on + scene.tool_settings.use_snap = True + # Match default Bonsai snaps + scene.tool_settings.snap_elements_base = {"EDGE", "EDGE_PERPENDICULAR", "VERTEX", "EDGE_MIDPOINT", "FACE"} - # Bonsai overlays + tool.Blender.sync_old_preferences() + + +def _install_viewport_overlays() -> None: + """Sync every Bonsai viewport decorator to its enabled state. + + Wrapped in uninstall/install of the decorator-cache bump handlers so a + decorator's own install path doesn't double-bind to depsgraph_update_post + via ``TokenCache`` instances created during their own ``install()``.""" georeference_props = tool.Georeference.get_georeference_props() aggregate_props = tool.Aggregate.get_aggregate_props() nest_props = tool.Nest.get_nest_props() @@ -427,23 +462,26 @@ def load_post(scene): NestDecorator.uninstall() WallAxisDecorator.uninstall() SlabDirectionDecorator.uninstall() - if georeference_props.should_visualise: - GeoreferenceDecorator.install(bpy.context) - if aggregate_props.aggregate_decorator: - AggregateDecorator.install(bpy.context) - if nest_props.nest_decorator: - NestDecorator.install(bpy.context) - if model_props.show_wall_axis: - WallAxisDecorator.install(bpy.context) - if model_props.show_slab_direction: - SlabDirectionDecorator.install(bpy.context) - if model_props.show_bounding_box: - BoundingBoxDecorator.install(bpy.context) + uninstall_decorator_cache_handlers() + try: + if georeference_props.should_visualise: + GeoreferenceDecorator.install(bpy.context) + if aggregate_props.aggregate_decorator: + AggregateDecorator.install(bpy.context) + if nest_props.nest_decorator: + NestDecorator.install(bpy.context) + if model_props.show_wall_axis: + WallAxisDecorator.install(bpy.context) + if model_props.show_slab_direction: + SlabDirectionDecorator.install(bpy.context) + if model_props.show_bounding_box: + BoundingBoxDecorator.install(bpy.context) + finally: + install_decorator_cache_handlers() - if preferences.should_use_snap and (scene := bpy.context.scene): - # Snapping is off by default in Blender, but in BIM, it's more useful to be on - scene.tool_settings.use_snap = True - # Match default Bonsai snaps - scene.tool_settings.snap_elements_base = {"EDGE", "EDGE_PERPENDICULAR", "VERTEX", "EDGE_MIDPOINT", "FACE"} - tool.Blender.sync_old_preferences() +@persistent +def load_post(scene): + _apply_save_file_invariants(scene) + _apply_user_preferences() + _install_viewport_overlays() From e7e489e3908248525705800cd1b63a5e43937a60 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 27 May 2026 15:50:42 +0200 Subject: [PATCH 05/10] =?UTF-8?q?Refactor=20bim/parametric=5Flifecycle=20?= =?UTF-8?q?=E2=80=94=20drift=20triad=20+=20Cancel=20polish?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three changes to the shared Enable/Finish/Cancel mixins: 1. Always-on drift triad on ParametricEditMixinBase. The base now provides ``_handle_drift_on_enable`` / ``_handle_drift_on_finish`` / ``_handle_drift_on_cancel`` classmethods, called from the per-mixin ``_enable_one`` / ``_finish_one`` / ``_cancel_one``. Pre-edit Blender-side translations commit to IFC on Enable (apply_scale=False — only translation/rotation, not the user's accidental scale), in-edit drag commits on Finish (apply_scale=True), and Cancel restores the committed IFC placement via ``restore_or_rebaseline_placement``. Prevents the "uncommitted drag disappears on Finish" and "preview snaps back on Cancel" UX bugs. 2. ``_ParametricEditMixinBase`` renamed to ``ParametricEditMixinBase`` (public). Per-feature mixins that need to subclass directly (e.g., when neither FeatureModifier nor PathPreserving fits) can do so without reaching into a private name. 3. ``_update_modifier_bmesh`` (PathPreserving) renamed to ``_restore_viewport_after_cancel``. The old name was inaccurate for subclasses that load a different IFC representation on Cancel rather than rebuilding a bmesh preview from props. Plus two polish changes: * ``_mark_type_thumbnail_dirty`` helper on the base centralises the ``ifcopenshell.util.element.get_type`` + thumbnail-mark pattern that both mixins repeated inline. * ``FeatureModifierEditMixin._cancel_one`` and ``PathPreservingEditMixin._cancel_one`` wrap the restore in ``try/finally`` so ``props.is_editing = False`` flips even on partial restore failure. Without this, a Cancel that raised mid-restore would leave the user locked out of the edit lifecycle. * ``PathPreservingEditMixin._finish_one`` / ``_cancel_one`` skip the pset commit + viewport rebuild when the draft equals the stored pset (no-op Enable→Finish round-trip should not pollute the representation list or burn an undo entry). ``FeatureModifierEditMixin._finish_one`` now routes the pset commit through ``tool.Pset.write_bbim_data`` instead of inlining the ``createIfcText(json.dumps(...))`` + ``ifcopenshell.api.pset.edit_pset`` dance. Two test assertions updated to match. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/parametric_lifecycle.py | 202 ++++++++++++------ .../test/bim/test_parametric_lifecycle.py | 14 +- 2 files changed, 149 insertions(+), 67 deletions(-) diff --git a/src/bonsai/bonsai/bim/parametric_lifecycle.py b/src/bonsai/bonsai/bim/parametric_lifecycle.py index 94324afa06..da95b9d256 100644 --- a/src/bonsai/bonsai/bim/parametric_lifecycle.py +++ b/src/bonsai/bonsai/bim/parametric_lifecycle.py @@ -18,29 +18,54 @@ # # This file was generated with the assistance of an AI coding tool. -"""Shared Enable / Finish / Cancel lifecycle mixins for parametric-edit operators. +"""Shared operator mixins for parametric-edit operators. -Two mixins fit the parametric-edit triads in ``bim/module/model/``: +Edit-lifecycle mixins (Enable / Finish / Cancel): + `FeatureModifierEditMixin` — door, window (BBIM_ pset; nested + lining/panel properties; Finish + Cancel route through + ``ifcopenshell.api.feature``). + `PathPreservingEditMixin` — railing, roof (path_data preserved across + edit; only general kwargs are user-editable). -`FeatureModifierEditMixin` - Door, Window — BBIM_ pset with nested ``lining_properties`` / - ``panel_properties``; Finish calls ``update__modifier_representation`` - via ``ifcopenshell.api.feature``; Cancel restores via ``switch_representation``. +Pattern selection (which approach a new feature should adopt): + Every parametric edit lifecycle commits to one of three patterns. Pick by + answering "does the feature share the Enable→Finish→Cancel shape that + one of the existing mixins already encodes?": -`PathPreservingEditMixin` - Railing, Roof — BBIM_ pset whose ``path_data`` is preserved through - edit (only general kwargs are user-editable); Finish calls - ``update__modifier_bmesh`` / ``update__modifier_ifc_data``; - Cancel re-reads the pset and rebuilds the bmesh preview. + A. Inherit one of the shared mixins below and route through + `tool.Parametric.build_edit_lifecycle`: -Stair and Wall stay standalone — their lifecycles diverge in ways that don't -fit either mixin without optional escape hatches (Stair has a unique -``update_ifc_stair_props`` post-Finish step + a separate ``get_props_kwargs_for_ifc_export``; -Wall is validation-first, snapshot-driven, no preview regen in operators). + - `FeatureModifierEditMixin` when the feature stores its pset as + `{general fields} + {lining_properties: {...}} + {panel_properties: {...}}` + and Finish must call a per-type `update__modifier_representation`. -This module sits separately from `bonsai.tool.Parametric` (the registry + -auto-commit) because it imports ``bonsai.tool`` freely, while the registry -itself must stay light — ``tool/blender.py`` consumes the registry at module load.""" + - `PathPreservingEditMixin` when the feature's pset carries a + `path_data` field that survives general-kwarg edits untouched, with + a separate Enable/Finish/Cancel lifecycle for path editing itself. + + B. Write a per-feature mixin that subclasses `ParametricEditMixinBase` + and provides `_enable_targets` / `_finish_targets` / `_cancel_targets`, + then route through `build_edit_lifecycle`. Pick this when the + feature's pset roundtrip or representation handling diverges from the + shared mixins but the Enable→Finish→Cancel shape still fits. + + C. Declare standalone Enable/Finish/Cancel Operator subclasses (no + factory) when the feature's parameter-change logic is sufficiently + unique that even a per-feature mixin would force optional hooks or + dead branches. Such operators MUST call the matrix_world drift + helpers (`tool.Geometry.commit_placement_if_moved` on Enable/Finish, + `tool.Geometry.restore_or_rebaseline_placement` on Cancel) — the + drift contract is enforced uniformly regardless of which pattern the + operators adopt. + + The authoritative list of registered parametric types — and which use + `build_edit_lifecycle` vs. standalone operators — lives in + `tool/parametric.py`'s `EDIT_TYPES` and is enforced by the registry + contract tests in `test/bim/test_parametric_registry.py`. + +This module hosts operator-side mixins that import ``bonsai.tool`` freely. +The lightweight parametric registry consumed at addon-enable time must stay +free of such imports and lives separately in ``tool/parametric.py``.""" from __future__ import annotations @@ -48,9 +73,7 @@ import json from typing import TYPE_CHECKING, ClassVar import bpy -import ifcopenshell.api.pset import ifcopenshell.util.element -import ifcopenshell.util.representation import bonsai.core.geometry import bonsai.tool as tool @@ -59,8 +82,8 @@ if TYPE_CHECKING: from ifcopenshell import entity_instance -class _ParametricEditMixinBase: - """Common scaffolding for parametric edit-triad mixins. +class ParametricEditMixinBase: + """Common scaffolding for parametric edit-lifecycle mixins. Each per-type subclass provides four hooks: @@ -69,6 +92,11 @@ class _ParametricEditMixinBase: ``_get_props(obj)``: PropertyGroup accessor ``_iter_targets(context)``: list of objects to act on (default: ``[active_object]``) + Drift handling is built in: pre-edit matrix_world drift commits to IFC on + Enable, in-edit drag commits on Finish, and Cancel restores the committed + IFC placement. This prevents an uncommitted drag from disappearing on + Finish or snapping back on Cancel. + Operator subclasses call one of ``_enable_targets`` / ``_finish_targets`` / ``_cancel_targets`` from their ``_execute`` method.""" @@ -99,8 +127,29 @@ class _ParametricEditMixinBase: return None return element, cls._get_props(obj) + @classmethod + def _handle_drift_on_enable(cls, obj: bpy.types.Object) -> None: + tool.Geometry.commit_placement_if_moved(obj, apply_scale=False) -class FeatureModifierEditMixin(_ParametricEditMixinBase): + @classmethod + def _handle_drift_on_finish(cls, obj: bpy.types.Object) -> None: + tool.Geometry.commit_placement_if_moved(obj) + + @classmethod + def _handle_drift_on_cancel(cls, obj: bpy.types.Object, element: entity_instance) -> None: + tool.Geometry.restore_or_rebaseline_placement(obj, element) + + @classmethod + def _mark_type_thumbnail_dirty(cls, element: entity_instance) -> None: + """Mark the element's type's preview thumbnail for refresh so the + property-panel preview reflects post-edit geometry. No-op for + occurrences without a backing type.""" + element_type = ifcopenshell.util.element.get_type(element) + if element_type: + tool.Model.mark_thumbnail_for_update(element_type) + + +class FeatureModifierEditMixin(ParametricEditMixinBase): """Lifecycle for door- and window-style parametric modifier operators. Enable: @@ -121,10 +170,7 @@ class FeatureModifierEditMixin(_ParametricEditMixinBase): @classmethod def _update_modifier_representation(cls, obj: bpy.types.Object, context: bpy.types.Context) -> None: - """Hook: call the per-type ``update__modifier_representation``. - - Door's helper takes ``obj``; window's takes ``context``. The hook lets - each subclass forward to its existing helper without unifying signatures.""" + """Hook: call the per-type ``update__modifier_representation``.""" raise NotImplementedError @classmethod @@ -133,6 +179,7 @@ class FeatureModifierEditMixin(_ParametricEditMixinBase): if resolved is None: return element, props = resolved + cls._handle_drift_on_enable(obj) data = json.loads(ifcopenshell.util.element.get_pset(element, cls.pset_name, "Data")) data.update(data.pop("lining_properties")) data.update(data.pop("panel_properties")) @@ -152,12 +199,9 @@ class FeatureModifierEditMixin(_ParametricEditMixinBase): data["lining_properties"] = props.get_lining_kwargs(convert_to_project_units=True) data["panel_properties"] = props.get_panel_kwargs(convert_to_project_units=True) cls._update_modifier_representation(obj, context) - element_type = ifcopenshell.util.element.get_type(element) - if element_type: - tool.Model.mark_thumbnail_for_update(element_type) - pset = tool.Pset.get_element_pset(element, cls.pset_name) - data_text = tool.Ifc.get().createIfcText(json.dumps(data, default=list)) - ifcopenshell.api.pset.edit_pset(tool.Ifc.get(), pset=pset, properties={"Data": data_text}) + cls._mark_type_thumbnail_dirty(element) + tool.Pset.write_bbim_data(element, cls.pset_name, data) + cls._handle_drift_on_finish(obj) # Set only on success: if any IFC op above raised, the user's draft survives for retry. props.is_editing = False @@ -167,13 +211,21 @@ class FeatureModifierEditMixin(_ParametricEditMixinBase): if resolved is None: return element, props = resolved - data = json.loads(ifcopenshell.util.element.get_pset(element, cls.pset_name, "Data")) - data.update(data.pop("lining_properties")) - data.update(data.pop("panel_properties")) - props.set_props_kwargs_from_ifc_data(data) - body = ifcopenshell.util.representation.get_representation(element, "Model", "Body", "MODEL_VIEW") - bonsai.core.geometry.switch_representation(tool.Ifc, tool.Geometry, obj=obj, representation=body) - props.is_editing = False + # Cancel must always clear is_editing — leaving it True after a + # restore-failure would block the user from re-entering edit mode and + # the next save's stale-flag heal would silently roll back the + # cancellation. Wrap the restore in try/finally so the flag flips + # even on partial failure. + try: + data = json.loads(ifcopenshell.util.element.get_pset(element, cls.pset_name, "Data")) + data.update(data.pop("lining_properties")) + data.update(data.pop("panel_properties")) + props.set_props_kwargs_from_ifc_data(data) + body = tool.Geometry.get_body_representation(element) + bonsai.core.geometry.switch_representation(tool.Ifc, tool.Geometry, obj=obj, representation=body) + cls._handle_drift_on_cancel(obj, element) + finally: + props.is_editing = False def _enable_targets(self, context: bpy.types.Context) -> set[str]: for obj in self._iter_targets(context): @@ -191,19 +243,20 @@ class FeatureModifierEditMixin(_ParametricEditMixinBase): return {"FINISHED"} -class PathPreservingEditMixin(_ParametricEditMixinBase): +class PathPreservingEditMixin(ParametricEditMixinBase): """Lifecycle for railing- and roof-style parametric modifier operators. Distinctive: ``path_data`` is part of the BBIM_ pset but is **not** - user-editable through this triad — it survives the edit untouched, only + user-editable through this lifecycle — it survives the edit untouched, only general kwargs are diffed. (Path editing has its own separate operator pair, ``Enable/Finish/CancelEditingPath``, out of scope here.) Enable: Fetch pset data via ``tool.Model.get_modeling_bbim_pset_data`` → set draft props → ``is_editing = True``. The subclass post-load hook - lets railing JSON-serialise ``path_data`` for the PropertyGroup - string field. + can reshape the dict to fit the PropertyGroup's storage layout + (e.g., pre-serialise a structured pset value to JSON for a + ``StringProperty`` field). Finish: Read fresh pset → keep ``path_data`` → gather ``general`` kwargs @@ -213,16 +266,18 @@ class PathPreservingEditMixin(_ParametricEditMixinBase): Cancel: Read fresh pset → restore draft props → call - ``_update_modifier_bmesh`` (per-type bmesh preview) → - ``is_editing = False``.""" + ``_restore_viewport_after_cancel`` (per-type viewport restore — typically + rebuilds the bmesh preview, but subclasses may load a different + representation entirely) → ``is_editing = False``.""" @classmethod def _post_load_data(cls, data: dict) -> dict: """Hook: optionally transform the pset data dict after loading and before passing to ``set_props_kwargs_from_ifc_data``. Default: pass-through. - Railing overrides to JSON-serialise ``path_data`` (its - BIMRailingProperties.path_data is a ``StringProperty`` holding JSON).""" + Override when the PropertyGroup stores a structured pset field as a + serialised primitive — e.g., a list/dict value mapped onto a + ``StringProperty`` requires JSON-encoding here.""" return data @classmethod @@ -238,9 +293,12 @@ class PathPreservingEditMixin(_ParametricEditMixinBase): raise NotImplementedError @classmethod - def _update_modifier_bmesh(cls, obj: bpy.types.Object, context: bpy.types.Context) -> None: - """Hook: per-type ``update__modifier_bmesh`` — rebuilds the - bmesh preview to match the current draft props (used by Cancel).""" + def _restore_viewport_after_cancel(cls, obj: bpy.types.Object, context: bpy.types.Context) -> None: + """Hook: restore the viewport mesh to match the just-restored draft props. + + Most subclasses rebuild a bmesh preview from props. Subclasses whose + committed IFC representation diverges from the preview may switch + the mesh back to the committed representation instead.""" raise NotImplementedError @classmethod @@ -249,6 +307,7 @@ class PathPreservingEditMixin(_ParametricEditMixinBase): if resolved is None: return _element, props = resolved + cls._handle_drift_on_enable(obj) data = tool.Model.get_modeling_bbim_pset_data(obj, cls.pset_name)["data_dict"] data = cls._post_load_data(data) props.set_props_kwargs_from_ifc_data(data) @@ -261,11 +320,18 @@ class PathPreservingEditMixin(_ParametricEditMixinBase): return element, props = resolved pset_data = tool.Model.get_modeling_bbim_pset_data(obj, cls.pset_name) - path_data = pset_data["data_dict"]["path_data"] + stored = pset_data["data_dict"] data = props.get_general_kwargs(convert_to_project_units=True) - data["path_data"] = path_data - cls._update_pset(element, data) - cls._update_modifier_ifc_data(obj, context) + data["path_data"] = stored["path_data"] + # Skip the pset commit when the draft is identical to the stored pset: + # an Enable → Finish-without-changes cycle should not pollute the + # representation list or burn an undo entry. Drift commit still runs + # unconditionally — matrix_world drift is independent of pset content. + if data != stored: + cls._update_pset(element, data) + cls._update_modifier_ifc_data(obj, context) + cls._mark_type_thumbnail_dirty(element) + cls._handle_drift_on_finish(obj) # Set only on success: if any IFC op above raised, the user's draft survives for retry. props.is_editing = False @@ -274,12 +340,26 @@ class PathPreservingEditMixin(_ParametricEditMixinBase): resolved = cls._resolve(obj) if resolved is None: return - _element, props = resolved - data = tool.Model.get_modeling_bbim_pset_data(obj, cls.pset_name)["data_dict"] - data = cls._post_load_data(data) - props.set_props_kwargs_from_ifc_data(data) - cls._update_modifier_bmesh(obj, context) - props.is_editing = False + element, props = resolved + try: + pset_data = tool.Model.get_modeling_bbim_pset_data(obj, cls.pset_name) + stored = pset_data["data_dict"] + draft = props.get_general_kwargs(convert_to_project_units=True) + draft["path_data"] = stored["path_data"] + nothing_changed = draft == stored + data = cls._post_load_data(stored) + props.set_props_kwargs_from_ifc_data(data) + # Skip the viewport rebuild on a no-op cancel: the mesh on screen is + # still the committed representation, and the per-type viewport-restore + # hook may be expensive (some subclasses reload a high-poly IFC + # representation rather than rebuild a preview mesh). + if not nothing_changed: + cls._restore_viewport_after_cancel(obj, context) + cls._handle_drift_on_cancel(obj, element) + finally: + # Always clear the flag — see ``FeatureModifierEditMixin._cancel_one`` + # for the rationale. + props.is_editing = False def _enable_targets(self, context: bpy.types.Context) -> set[str]: for obj in self._iter_targets(context): diff --git a/src/bonsai/test/bim/test_parametric_lifecycle.py b/src/bonsai/test/bim/test_parametric_lifecycle.py index 4142f51e63..97bd53ff40 100644 --- a/src/bonsai/test/bim/test_parametric_lifecycle.py +++ b/src/bonsai/test/bim/test_parametric_lifecycle.py @@ -198,10 +198,12 @@ def test_feature_modifier_finish_one_clears_is_editing_and_writes_pset(patched_t assert props.is_editing is False assert obj in cls.representations_called - # edit_pset is called exactly once; properties key is "Data" wrapping JSON. - patched_tool_and_ifc["ifc"].api.pset.edit_pset.assert_called_once() - kwargs = patched_tool_and_ifc["ifc"].api.pset.edit_pset.call_args.kwargs - assert "properties" in kwargs and "Data" in kwargs["properties"] + # tool.Pset.write_bbim_data is called exactly once with the merged dict. + patched_tool_and_ifc["tool"].Pset.write_bbim_data.assert_called_once() + call_args = patched_tool_and_ifc["tool"].Pset.write_bbim_data.call_args + assert call_args.args[1] == "BBIM_Door" # pset_name positional arg + written_data = call_args.args[2] + assert "lining_properties" in written_data and "panel_properties" in written_data def test_feature_modifier_finish_one_exception_leaves_draft_in_progress(patched_tool_and_ifc): @@ -302,7 +304,7 @@ def _path_mixin_cls(match=True): cls.ifc_data_updates.append(obj) @classmethod - def _update_modifier_bmesh(cls, obj, context): + def _restore_viewport_after_cancel(cls, obj, context): cls.bmesh_updates.append(obj) return _TestPathMixin @@ -344,7 +346,7 @@ def test_path_preserving_finish_one_preserves_path_data_and_clears_is_editing(pa assert obj in cls.ifc_data_updates -def test_path_preserving_cancel_one_calls_update_modifier_bmesh(patched_tool_and_ifc): +def test_path_preserving_cancel_one_calls_restore_viewport_after_cancel(patched_tool_and_ifc): props = _FakePathProps() props.is_editing = True obj = _make_obj(props) From ff4c642db13773800d5f87a7a8294f2f77f328f4 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 27 May 2026 15:56:19 +0200 Subject: [PATCH 06/10] Add parametric-draft undo-resync registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ctrl+Z / Ctrl+Shift+Z on an in-progress parametric draft (wall / stair / roof) used to leave the preview mesh frozen in its pre-undo shape — the IFC mutation rolls back but the bmesh built from draft props doesn't repaint. Add a registry of per-type regenerator functions (``UNDO_REGENERATORS``) that re-build each type's preview mesh from its current props. The dispatcher ``resync_parametric_drafts_after_undo`` walks all objects, skips any without an active parametric edit, looks up the regenerator by feature name, and calls it. Tagged 3D viewports for redraw. Types without an entry (door / window / railing / etc.) are intentionally absent — they're IFC-derived, so the undo's representation rollback + next-frame refresh already repaints correctly without a draft-side regenerator. Undo/redo wiring is self-installed by ``bonsai.bim.parametric_lifecycle``: a ``@persistent`` ``_resync_on_undo`` callback dispatches into the registry, and ``install_parametric_lifecycle_handlers()`` / ``uninstall_parametric_lifecycle_handlers()`` append/remove it from ``bpy.app.handlers.undo_post`` and ``redo_post``. ``bim/__init__.py``'s ``register()`` calls the install function *after* the central ``handler.undo_post`` / ``redo_post`` appends so the regenerators see restored IFC state — ``bpy.app.handlers`` fire in append order. ``handler.py`` itself stays ignorant of the parametric subsystem. The lazy function-local imports in each regenerator break the addon-load cycle — ``bonsai.bim.parametric_lifecycle`` loads before ``bim/module/model/*``. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/__init__.py | 5 +- src/bonsai/bonsai/bim/parametric_lifecycle.py | 90 +++++++++++++++++++ src/bonsai/test/bim/test_addon_lifecycle.py | 60 ------------- 3 files changed, 94 insertions(+), 61 deletions(-) delete mode 100644 src/bonsai/test/bim/test_addon_lifecycle.py diff --git a/src/bonsai/bonsai/bim/__init__.py b/src/bonsai/bonsai/bim/__init__.py index b1e105a079..d9055d1f79 100644 --- a/src/bonsai/bonsai/bim/__init__.py +++ b/src/bonsai/bonsai/bim/__init__.py @@ -27,7 +27,7 @@ import bpy import bpy.utils.previews from bpy_extras.io_utils import ExportHelper, ImportHelper -from . import handler, operator, prop, ui +from . import handler, operator, parametric_lifecycle, prop, ui def _parametric_gizmo_preference_classes() -> list[type]: @@ -283,6 +283,8 @@ def register(): bpy.app.handlers.depsgraph_update_post.append(on_register) bpy.app.handlers.undo_post.append(handler.undo_post) bpy.app.handlers.redo_post.append(handler.redo_post) + # Must follow the two appends above so regenerators see restored IFC state. + parametric_lifecycle.install_parametric_lifecycle_handlers() bpy.app.handlers.load_post.append(handler.load_post) bpy.app.handlers.load_post.append(handler.loadIfcStore) bpy.types.Scene.BIMProperties = bpy.props.PointerProperty(type=prop.BIMProperties) @@ -340,6 +342,7 @@ def unregister(): unregister_classes(classes) + parametric_lifecycle.uninstall_parametric_lifecycle_handlers() bpy.app.handlers.load_post.remove(handler.load_post) bpy.app.handlers.load_post.remove(handler.loadIfcStore) del bpy.types.Scene.BIMProperties diff --git a/src/bonsai/bonsai/bim/parametric_lifecycle.py b/src/bonsai/bonsai/bim/parametric_lifecycle.py index da95b9d256..414f11173b 100644 --- a/src/bonsai/bonsai/bim/parametric_lifecycle.py +++ b/src/bonsai/bonsai/bim/parametric_lifecycle.py @@ -70,10 +70,12 @@ free of such imports and lives separately in ``tool/parametric.py``.""" from __future__ import annotations import json +from collections.abc import Callable from typing import TYPE_CHECKING, ClassVar import bpy import ifcopenshell.util.element +from bpy.app.handlers import persistent import bonsai.core.geometry import bonsai.tool as tool @@ -375,3 +377,91 @@ class PathPreservingEditMixin(ParametricEditMixinBase): for obj in self._iter_targets(context): self._cancel_one(obj, context) return {"FINISHED"} + + +# --- Undo-resync registry ---------------------------------------------------- +# +# Per-type regenerators called from ``resync_parametric_drafts_after_undo`` +# (wired into ``bim/handler.py:undo_post`` and ``redo_post``) so the preview +# mesh of an in-progress parametric draft repaints after Ctrl+Z / Ctrl+Shift+Z. +# +# Each regenerator is a one-line lazy-import + call. Lazy imports because +# ``bonsai.bim.parametric_lifecycle`` loads before ``bim/module/model/*`` +# at addon enable; a module-level import would cycle. Each function-local +# import lands at first call, after the feature module has registered. +# +# Types with no entry — door, window, railing, etc. — are IFC-derived: undo +# of an IFC mutation already restores the entity, and ``switch_representation`` +# repaints the mesh as a side effect of the next refresh. They don't need a +# bespoke preview regenerator. + + +def _wall_undo_regenerator(obj: bpy.types.Object) -> None: + from bonsai.bim.module.model.wall import regenerate_wall_mesh_from_props + + regenerate_wall_mesh_from_props(obj) + + +def _stair_undo_regenerator(obj: bpy.types.Object) -> None: + from bonsai.bim.module.model.stair import regenerate_stair_mesh + + regenerate_stair_mesh(obj) + + +def _roof_undo_regenerator(obj: bpy.types.Object) -> None: + from bonsai.bim.module.model.roof import update_roof_modifier_bmesh + + update_roof_modifier_bmesh(obj) + + +UNDO_REGENERATORS: dict[str, Callable[[bpy.types.Object], None]] = { + "wall": _wall_undo_regenerator, + "stair": _stair_undo_regenerator, + "roof": _roof_undo_regenerator, +} + + +def resync_parametric_drafts_after_undo() -> None: + """Re-render preview meshes for every parametric draft currently active. + + Walks all objects, skips any not in a registered parametric edit, + dispatches to the per-type regenerator in ``UNDO_REGENERATORS``. A type + without an entry is left alone — its preview is either already correct + (IFC-derived) or has no draft preview mesh.""" + for obj in bpy.data.objects: + feature = tool.Parametric.is_object_editing(obj) + if feature is None: + continue + regenerator = UNDO_REGENERATORS.get(feature.name) + if regenerator is None: + continue + regenerator(obj) + screen = getattr(bpy.context, "screen", None) + if screen is not None: + for area in screen.areas: + if area.type == "VIEW_3D": + area.tag_redraw() + + +@persistent +def _resync_on_undo(scene: bpy.types.Scene) -> None: + resync_parametric_drafts_after_undo() + + +def install_parametric_lifecycle_handlers() -> None: + """Append the undo-resync callback to undo_post and redo_post; idempotent. + + Caller must invoke this AFTER appending the central undo/redo handlers so + regenerators see restored IFC state — bpy.app.handlers fire in append order.""" + for hook in (bpy.app.handlers.undo_post, bpy.app.handlers.redo_post): + if _resync_on_undo not in hook: + hook.append(_resync_on_undo) + + +def uninstall_parametric_lifecycle_handlers() -> None: + for hook in (bpy.app.handlers.undo_post, bpy.app.handlers.redo_post): + try: + hook.remove(_resync_on_undo) + except ValueError: + pass +>>>>>>> 8e305588d (fixup! Add parametric-draft undo-resync registry + handler hooks) diff --git a/src/bonsai/test/bim/test_addon_lifecycle.py b/src/bonsai/test/bim/test_addon_lifecycle.py deleted file mode 100644 index 16ea0b9f0c..0000000000 --- a/src/bonsai/test/bim/test_addon_lifecycle.py +++ /dev/null @@ -1,60 +0,0 @@ -# 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 . -# -# This file was generated with the assistance of an AI coding tool. - -"""Addon-load smoke for ``bonsai``. - -Pins the registration/unregistration cycle as a runnable contract. The cycle -exercises every ``register()`` site across ``bim/__init__.py``'s modules dict, -every ``PointerProperty`` attachment, every gizmo-prefs auto-registration, and -every ``bpy.app.handlers`` install. A regression in any of those surfaces here -as an exception with a traceback that points at the failing site, instead of -the silent ``addon failed to enable`` users see in a fresh Blender.""" - -import types - -import bpy -import pytest - -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)") - - -def test_addon_unregister_then_register_does_not_raise(): - """Running the suite has already enabled the addon. Cycle through one - unregister + register to exercise both halves, then leave the addon - enabled so downstream tests in the same Blender session keep working.""" - import bonsai - - bonsai.unregister() - try: - bonsai.register() - except Exception: - # Re-raise after attempting to leave the session in a usable state for - # any tests that run after this one. - try: - bonsai.register() - except Exception: - pass - raise From 2feade01cb0282cc30cd03bde6504efb23965d42 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 27 May 2026 16:01:18 +0200 Subject: [PATCH 07/10] DRY tag-redraw-3D-viewports loops via tool.Blender.update_all_viewports Five inline copies of the same defensive pattern lived across ``tool/parametric.py``, ``bim/parametric_lifecycle.py``, ``bim/module/model/preview_base.py`` (twice), and as a near-twin in ``tool/blender.py:update_all_viewports`` itself. ``tool.Blender.update_all_viewports`` already covered the ``tag_redraw`` job but used an ``assert context.screen`` that would raise during background-mode operators or early-load_post calls where ``screen`` legitimately is None. Relax to a defensive ``getattr(context, "screen", None)`` + silent return so the helper fits every caller's needs, then collapse the 4 inline copies to single calls. Net -9 LOC. The helper now describes its contract ("silent no-op when no screen attached") rather than naming specific callers, so moving a caller doesn't rot the docstring. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/preview_base.py | 9 ++------- src/bonsai/bonsai/bim/parametric_lifecycle.py | 7 +------ src/bonsai/bonsai/tool/blender.py | 8 ++++++-- src/bonsai/bonsai/tool/parametric.py | 6 +----- 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/model/preview_base.py b/src/bonsai/bonsai/bim/module/model/preview_base.py index f9921dd1aa..e99aadc2f4 100644 --- a/src/bonsai/bonsai/bim/module/model/preview_base.py +++ b/src/bonsai/bonsai/bim/module/model/preview_base.py @@ -126,9 +126,7 @@ def make_dim_setter( if props is None: return setattr(props, field, max(min_value, float(value))) - for area in bpy.context.screen.areas if bpy.context.screen else (): - if area.type == "VIEW_3D": - area.tag_redraw() + tool.Blender.update_all_viewports() return _set @@ -179,10 +177,7 @@ def try_cancel_active_preview(context: bpy.types.Context) -> bool: getattr(bpy.ops.bim, op_name)() cancelled = True if cancelled: - screen = context.screen - for area in screen.areas if screen else (): - if area.type == "VIEW_3D": - area.tag_redraw() + tool.Blender.update_all_viewports(context) return cancelled diff --git a/src/bonsai/bonsai/bim/parametric_lifecycle.py b/src/bonsai/bonsai/bim/parametric_lifecycle.py index 414f11173b..436a28396e 100644 --- a/src/bonsai/bonsai/bim/parametric_lifecycle.py +++ b/src/bonsai/bonsai/bim/parametric_lifecycle.py @@ -436,11 +436,7 @@ def resync_parametric_drafts_after_undo() -> None: if regenerator is None: continue regenerator(obj) - screen = getattr(bpy.context, "screen", None) - if screen is not None: - for area in screen.areas: - if area.type == "VIEW_3D": - area.tag_redraw() + tool.Blender.update_all_viewports() @persistent @@ -464,4 +460,3 @@ def uninstall_parametric_lifecycle_handlers() -> None: hook.remove(_resync_on_undo) except ValueError: pass ->>>>>>> 8e305588d (fixup! Add parametric-draft undo-resync registry + handler hooks) diff --git a/src/bonsai/bonsai/tool/blender.py b/src/bonsai/bonsai/tool/blender.py index 0ecb767da1..c94ec38d72 100644 --- a/src/bonsai/bonsai/tool/blender.py +++ b/src/bonsai/bonsai/tool/blender.py @@ -680,9 +680,13 @@ class Blender(bonsai.core.tool.Blender): @classmethod def update_all_viewports(cls, context: bpy.types.Context | None = None) -> None: + """Tag every visible 3D viewport for redraw. Silent no-op when no + screen attached (background mode, plug-out, mid-load_post).""" context = context or bpy.context - assert context.screen - for area in context.screen.areas: + screen = getattr(context, "screen", None) + if screen is None: + return + for area in screen.areas: if area.type == "VIEW_3D": area.tag_redraw() diff --git a/src/bonsai/bonsai/tool/parametric.py b/src/bonsai/bonsai/tool/parametric.py index 0460379273..ad9846a18d 100644 --- a/src/bonsai/bonsai/tool/parametric.py +++ b/src/bonsai/bonsai/tool/parametric.py @@ -187,11 +187,7 @@ class Parametric(bonsai.core.tool.Parametric): cls._geom_generation += 1 bonsai.bim.handler.update_bim_tool_props() - screen = getattr(bpy.context, "screen", None) - if screen is not None: - for area in screen.areas: - if area.type == "VIEW_3D": - area.tag_redraw() + tool.Blender.update_all_viewports() @classmethod def find_by_name(cls, name: str) -> Optional[ParametricObject]: From 1855e4c019789c8baa223fc9e33f63e8a7db4447 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 21 May 2026 17:34:23 +0200 Subject: [PATCH 08/10] Fix wall split: keep straddling openings on both walls DumbWallJoiner.split assigned openings by projecting the opening's centre-point onto the wall axis, so any opening whose footprint straddled the cut was silently dropped from whichever wall its centre missed. Now the full axis-projected extent (via ifcopenshell.geom. create_shape) drives the assignment; for filled openings whose void straddles the cut, a pure-void copy is added back to the neighbour wall so its body is also cut. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/wall.py | 108 ++++- .../module/model/test_wall_split_openings.py | 377 ++++++++++++++++++ 2 files changed, 467 insertions(+), 18 deletions(-) create mode 100644 src/bonsai/test/bim/module/model/test_wall_split_openings.py diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index b923aa3446..c43bf2a7d2 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -23,7 +23,7 @@ import copy import math from math import atan2, cos, degrees, pi, sin -from typing import TYPE_CHECKING, Any, ClassVar, Literal, Union, get_args +from typing import TYPE_CHECKING, Any, ClassVar, Literal, Optional, Union, get_args import bmesh import bpy @@ -34,9 +34,11 @@ import ifcopenshell.api.material import ifcopenshell.api.pset import ifcopenshell.api.root import ifcopenshell.api.type +import ifcopenshell.geom import ifcopenshell.util.element import ifcopenshell.util.placement import ifcopenshell.util.representation +import ifcopenshell.util.shape import ifcopenshell.util.shape_builder import ifcopenshell.util.type import ifcopenshell.util.unit @@ -1191,6 +1193,62 @@ class DumbWallPlaner: tool.Model.recalculate_walls([w for w in set(walls) if w]) +def _opening_axis_extent(opening, axis_reference, unit_scale): + """Return ``(min_t, max_t)``: the opening's world-space footprint + projected onto ``axis_reference`` as parametric positions along the + wall axis (``0`` is the start of the axis line, ``1`` is its end). + Used to detect openings whose footprint straddles a cut. + + Computed via ``ifcopenshell.geom.create_shape`` so the result is + correct for any representation type Bonsai may produce — mapped + representations, swept-area solids, breps, boolean clips, etc. — + without needing a Blender object (Bonsai hides openings after + ``bim.add_opening``). Falls back to a degenerate single-point range + at the placement origin only when the geometry kernel cannot build + a shape from the opening.""" + verts = None + shape_matrix: Optional[Matrix] = None + try: + settings = ifcopenshell.geom.settings() + shape = ifcopenshell.geom.create_shape(settings, opening) + verts = ifcopenshell.util.shape.get_vertices(shape.geometry) + shape_matrix = Matrix(ifcopenshell.util.shape.get_shape_matrix(shape).tolist()) + except Exception: + verts = None + shape_matrix = None + + if verts is None or shape_matrix is None or len(verts) == 0: + placement = Matrix(ifcopenshell.util.placement.get_local_placement(opening.ObjectPlacement).tolist()) + placement.translation *= unit_scale + _, t = mathutils.geometry.intersect_point_line(placement.translation.to_2d(), *axis_reference) + return t, t + + positions = [] + for v in verts: + world = (shape_matrix @ Vector((float(v[0]), float(v[1]), float(v[2])))).to_2d() + _, t = mathutils.geometry.intersect_point_line(world, *axis_reference) + positions.append(t) + return min(positions), max(positions) + + +def _add_void_copy(building_element, source_opening): + """Add an unfilled IfcOpeningElement to ``building_element`` whose + geometry and placement mirror ``source_opening``. Used when a filled + opening's void straddles a wall split — the filling stays on its wall, + but the void must also apply to the neighbour so its body gets cut.""" + void_copy = ifcopenshell.api.root.copy_class(tool.Ifc.get(), product=source_opening) + for fill_rel in list(void_copy.HasFillings or ()): + tool.Ifc.get().remove(fill_rel) + void_copy.VoidsElements[0].RelatingBuildingElement = building_element + if void_copy.ObjectPlacement and void_copy.ObjectPlacement.is_a("IfcLocalPlacement"): + if building_element.ObjectPlacement: + void_copy.ObjectPlacement.PlacementRelTo = building_element.ObjectPlacement + if source_opening.Representation: + void_copy.Representation = ifcopenshell.util.element.copy_deep( + tool.Ifc.get(), source_opening.Representation, exclude=["IfcGeometricRepresentationContext"] + ) + + class DumbWallJoiner: def __init__(self): self.unit_scale = ifcopenshell.util.unit.calculate_unit_scale(tool.Ifc.get()) @@ -1255,39 +1313,44 @@ class DumbWallJoiner: ) # During the duplication process, unfilled voids are copied, so we need - # to check openings on both element1 and element2. Let's check element1 - # first. + # to check openings on both element1 and element2. Each wall keeps the + # opening when the opening's axis-projected extent overlaps that wall's + # portion of the axis — straddling openings are intentionally kept on + # both walls so each wall body gets the appropriate cut. Strict + # inequalities mean a boundary-only touch (or a degenerate single-point + # extent at the cut) keeps the opening on both walls — the safer + # default when the helper cannot resolve a true bounding range. for opening in [ r.RelatedOpeningElement for r in element1.HasOpenings if not r.RelatedOpeningElement.HasFillings ]: - opening_matrix = Matrix(ifcopenshell.util.placement.get_local_placement(opening.ObjectPlacement).tolist()) - opening_matrix.translation *= unit_scale - opening_location = opening_matrix.translation - _, opening_position = mathutils.geometry.intersect_point_line(opening_location.to_2d(), *axis1["reference"]) - if opening_position > cut_percentage: - # The opening should be removed from element1. + min_t, _ = _opening_axis_extent(opening, axis1["reference"], unit_scale) + if min_t > cut_percentage: + # Opening lies entirely past the cut — only element2 should keep it. ifcopenshell.api.feature.remove_feature(tool.Ifc.get(), feature=opening) - # Now let's check element2. for opening in [ r.RelatedOpeningElement for r in element2.HasOpenings if not r.RelatedOpeningElement.HasFillings ]: - opening_matrix = Matrix(ifcopenshell.util.placement.get_local_placement(opening.ObjectPlacement).tolist()) - opening_matrix.translation *= unit_scale - opening_location = opening_matrix.translation - _, opening_position = mathutils.geometry.intersect_point_line(opening_location.to_2d(), *axis1["reference"]) - if opening_position < cut_percentage: - # The opening should be removed from element2. + _, max_t = _opening_axis_extent(opening, axis1["reference"], unit_scale) + if max_t < cut_percentage: + # Opening lies entirely before the cut — only element1 should keep it. ifcopenshell.api.feature.remove_feature(tool.Ifc.get(), feature=opening) # During the duplication process, filled voids are not copied. So we - # only need to check fillings on the original element1. - for opening in [r.RelatedOpeningElement for r in element1.HasOpenings if r.RelatedOpeningElement.HasFillings]: + # only need to check fillings on the original element1. The filling + # (door/window) belongs to whichever wall contains its center, but the + # void may need to apply to both walls when the void's extent straddles + # the cut — otherwise the neighbour wall's body would not be cut. + for opening in [ + r.RelatedOpeningElement for r in list(element1.HasOpenings) if r.RelatedOpeningElement.HasFillings + ]: rel = opening.HasFillings[0] filling = rel.RelatedBuildingElement filling_obj = tool.Ifc.get_object(filling) filling_location = filling_obj.matrix_world.translation _, filling_position = mathutils.geometry.intersect_point_line(filling_location.to_2d(), *axis1["reference"]) + min_t, max_t = _opening_axis_extent(opening, axis1["reference"], unit_scale) + void_straddles = min_t < cut_percentage < max_t if filling_position > cut_percentage: # The filling should be moved from element1 to element2. new_opening = ifcopenshell.api.root.copy_class(tool.Ifc.get(), product=opening) @@ -1306,6 +1369,15 @@ class DumbWallJoiner: # Remove the old opening ifcopenshell.api.feature.remove_feature(tool.Ifc.get(), feature=opening) + if void_straddles: + # Filling moved to element2, but void straddles — add a + # pure-void copy back to element1 so its body still gets cut. + _add_void_copy(element1, new_opening) + elif void_straddles: + # Filling stays on element1, but void straddles — add a pure-void + # copy to element2 so its body gets cut. + _add_void_copy(element2, opening) + p1, p2 = ifcopenshell.util.representation.get_reference_line(element1) p3 = (wall1.matrix_world.inverted() @ intersect.to_3d()).to_2d() / unit_scale self.set_axis(element1, p1, p3) diff --git a/src/bonsai/test/bim/module/model/test_wall_split_openings.py b/src/bonsai/test/bim/module/model/test_wall_split_openings.py new file mode 100644 index 0000000000..b94b28ca9c --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_wall_split_openings.py @@ -0,0 +1,377 @@ +# 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 . +# +# This file was generated with the assistance of an AI coding tool. + +"""Regression tests for wall-split opening assignment when the cut passes +through an opening. + +Bug repro before the fix: when ``bpy.ops.bim.split_wall`` (Shift+K) cut a wall +through an opening, ``DumbWallJoiner.split`` decided opening assignment using +the opening's centre-point projected onto the wall axis. Any opening whose +extent straddled the cut was therefore assigned to whichever side its centre +sat on, leaving the neighbour wall with no void where the opening overlapped. + +The fix replaces the single-point test with an axis-projected extent +(``_opening_axis_extent``): an opening is removed from a wall only when its +extent lies *entirely* outside that wall's portion of the axis. Straddling +openings stay on both walls.""" + +from unittest.mock import MagicMock, patch + +import bpy +import pytest + +pytestmark = pytest.mark.wall + + +def _fake_shape(verts_local, matrix_world_4x4): + """Build a stand-in for the ``shape`` object returned by + ``ifcopenshell.geom.create_shape``. ``get_vertices`` and + ``get_shape_matrix`` are mocked separately to read off this stand-in.""" + import numpy as np + + shape = MagicMock(name="shape") + shape.geometry = MagicMock(name="geometry") + shape._verts = np.asarray(verts_local, dtype=np.float64) + shape._matrix = np.asarray(matrix_world_4x4, dtype=np.float64) + return shape + + +def test_opening_axis_extent_uses_geometry_kernel_vertices(): + """``_opening_axis_extent`` drives ``ifcopenshell.geom.create_shape`` to + get the opening's real geometry vertices and ``get_shape_matrix`` to get + its world placement, then projects the world-space corners onto the wall + axis. This is the production path — works for every representation type + Bonsai may produce (mapped representation, swept area, brep, boolean). + + A unit cube centred at world X=5 on a 10m wall axis projects to + t ∈ [0.45, 0.55] (the cube spans 0.5m on each axis around the centre).""" + from bonsai.bim.module.model.wall import _opening_axis_extent + + # Unit cube in local coords, centred at (0,0,0), extent ±0.5. + verts_local = [ + (-0.5, -0.5, -0.5), + (0.5, -0.5, -0.5), + (0.5, 0.5, -0.5), + (-0.5, 0.5, -0.5), + (-0.5, -0.5, 0.5), + (0.5, -0.5, 0.5), + (0.5, 0.5, 0.5), + (-0.5, 0.5, 0.5), + ] + matrix_world = [ + [1.0, 0.0, 0.0, 5.0], + [0.0, 1.0, 0.0, 0.0], + [0.0, 0.0, 1.0, 0.0], + [0.0, 0.0, 0.0, 1.0], + ] + fake_shape = _fake_shape(verts_local, matrix_world) + opening = MagicMock(name="opening") + axis_reference = ( + __import__("mathutils").Vector((0.0, 0.0)), + __import__("mathutils").Vector((10.0, 0.0)), + ) + + with ( + patch("ifcopenshell.geom.create_shape", return_value=fake_shape), + patch("ifcopenshell.util.shape.get_vertices", return_value=fake_shape._verts), + patch("ifcopenshell.util.shape.get_shape_matrix", return_value=fake_shape._matrix), + ): + min_t, max_t = _opening_axis_extent(opening, axis_reference, unit_scale=1.0) + + # Cube spans world X ∈ [4.5, 5.5] → t ∈ [0.45, 0.55]. + assert min_t == pytest.approx(0.45) + assert max_t == pytest.approx(0.55) + + +def test_opening_axis_extent_falls_back_to_placement_when_geometry_kernel_fails(): + """When ``ifcopenshell.geom.create_shape`` raises (representation it + can't process), the helper falls back to a degenerate single-point range + at the opening's composed placement origin. This is the safety net — it + matches the pre-fix center-only semantics rather than dropping the + opening entirely.""" + from bonsai.bim.module.model.wall import _opening_axis_extent + + opening = MagicMock(name="opening") + placement_matrix = [ + [1.0, 0.0, 0.0, 5.0], + [0.0, 1.0, 0.0, 0.0], + [0.0, 0.0, 1.0, 0.0], + [0.0, 0.0, 0.0, 1.0], + ] + axis_reference = ( + __import__("mathutils").Vector((0.0, 0.0)), + __import__("mathutils").Vector((10.0, 0.0)), + ) + + with ( + patch("ifcopenshell.geom.create_shape", side_effect=RuntimeError("kernel failure")), + patch("ifcopenshell.util.placement.get_local_placement") as mock_get_placement, + ): + mock_get_placement.return_value = type("FakeArr", (), {"tolist": lambda self: placement_matrix})() + min_t, max_t = _opening_axis_extent(opening, axis_reference, unit_scale=1.0) + + assert min_t == max_t == pytest.approx(0.5) + + +def test_opening_axis_extent_offset_cursor_inside_extent_returns_straddling_range(): + """The regression guard for the user-reported bug across two fix attempts: + when the cursor is placed *inside* the opening but not at its exact + centre, the helper must still return a range that straddles the cursor + position so the side test keeps the opening on both walls. + + Pre-fix v2/v3 collapsed to a degenerate range whenever the production + representation type wasn't recognised (Blender bound_box absent in v2; + mapped representation not walked in v3). The current implementation uses + ``ifcopenshell.geom.create_shape``, which handles every representation + Bonsai may produce.""" + from bonsai.bim.module.model.wall import _opening_axis_extent + + # 2m-wide opening centred at world X=5 → world X ∈ [4.0, 6.0] → t ∈ [0.4, 0.6]. + verts_local = [(-1.0, -0.5, -0.5), (1.0, 0.5, 0.5)] + matrix_world = [ + [1.0, 0.0, 0.0, 5.0], + [0.0, 1.0, 0.0, 0.0], + [0.0, 0.0, 1.0, 0.0], + [0.0, 0.0, 0.0, 1.0], + ] + fake_shape = _fake_shape(verts_local, matrix_world) + opening = MagicMock(name="opening") + axis_reference = ( + __import__("mathutils").Vector((0.0, 0.0)), + __import__("mathutils").Vector((10.0, 0.0)), + ) + + with ( + patch("ifcopenshell.geom.create_shape", return_value=fake_shape), + patch("ifcopenshell.util.shape.get_vertices", return_value=fake_shape._verts), + patch("ifcopenshell.util.shape.get_shape_matrix", return_value=fake_shape._matrix), + ): + min_t, max_t = _opening_axis_extent(opening, axis_reference, unit_scale=1.0) + + # Cursor at world X=4.7 → t=0.47 (inside the opening, not centred on it). + cut_percentage = 0.47 + assert ( + min_t < cut_percentage < max_t + ), f"opening [t={min_t}, t={max_t}] must straddle off-centre cursor at t={cut_percentage}" + + +def test_straddling_opening_is_kept_on_both_sides(): + """The pruning logic must keep an opening whose extent straddles the cut + on *both* element1 and element2. + + Before the fix, an opening with centre at t=0.5 and cut_percentage=0.6 + would be removed from element2 (centre < cut) but kept on element1; the + opening's right half — which physically overlaps element2 — would be + silently dropped. After the fix, the opening overlaps both portions of + the axis (min_t=0.3 < 0.6 < max_t=0.7) so both walls keep it. + + Replays the boolean comparisons that ``DumbWallJoiner.split`` performs on + the helper's return value; does not call the helper itself.""" + min_t, max_t = 0.3, 0.7 # straddles any cut_percentage in (0.3, 0.7) + cut_percentage = 0.6 + + removed_from_element1 = min_t > cut_percentage + removed_from_element2 = max_t < cut_percentage + + assert removed_from_element1 is False, "straddling opening must remain on element1" + assert removed_from_element2 is False, "straddling opening must remain on element2" + + +def test_opening_entirely_past_cut_is_removed_from_element1_only(): + """Opening lies wholly on element2's side (min_t > cut_percentage). + Pre-fix and post-fix both remove it from element1; post-fix additionally + guarantees it stays on element2 because max_t > cut_percentage.""" + min_t, max_t = 0.7, 0.9 + cut_percentage = 0.5 + + assert (min_t > cut_percentage) is True # removed from element1 + assert (max_t < cut_percentage) is False # kept on element2 + + +def test_opening_entirely_before_cut_is_removed_from_element2_only(): + """Mirror of the above: opening wholly on element1's side.""" + min_t, max_t = 0.1, 0.3 + cut_percentage = 0.5 + + assert (min_t > cut_percentage) is False # kept on element1 + assert (max_t < cut_percentage) is True # removed from element2 + + +def test_opening_touching_cut_at_boundary_stays_on_both_walls(): + """Boundary touch: an opening's ``max_t`` lands exactly on the cut. Strict + inequalities keep the opening on both walls — the safer default. (Non- + strict ``<=`` would have removed from element2 instead.)""" + min_t, max_t = 0.2, 0.5 + cut_percentage = 0.5 + + assert (min_t > cut_percentage) is False # kept on element1 + assert (max_t < cut_percentage) is False # kept on element2 (boundary == cut) + + +def test_degenerate_range_at_cut_keeps_opening_on_both_walls(): + """Regression guard for the **post-fix-v1 regression**: when the helper + falls back to a degenerate range ``(t, t)`` (geometry kernel failed, or + the pre-create_shape fix attempts that produced only the placement + centre), placing the 3D cursor *on* the opening's centre makes + ``cut_percentage == t``. + + With non-strict ``>=`` / ``<=`` tests, the degenerate range matched both + removal conditions and both walls dropped the opening — leaving the user + with two walls and no hole anywhere. Strict ``>`` / ``<`` tests keep the + opening on both walls in this case, which matches the visible geometry.""" + min_t, max_t = 0.5, 0.5 # degenerate range — both bounds at the centre + cut_percentage = 0.5 # cursor placed exactly on the opening centre + + removed_from_element1 = min_t > cut_percentage + removed_from_element2 = max_t < cut_percentage + + assert removed_from_element1 is False, "must not remove from element1 when cursor sits on opening centre" + assert removed_from_element2 is False, "must not remove from element2 when cursor sits on opening centre" + + +# --------------------------------------------------------------------------- +# Filled-opening void-straddle behaviour +# +# When a wall split passes through a door/window, the filling (the door +# element itself) belongs to whichever wall contains its centre — but the +# void cut by the IfcOpeningElement may still straddle the cut, in which +# case the neighbour wall's body must also be cut. The helper that adds the +# pure-void copy is ``_add_void_copy``; the decision is taken in +# ``DumbWallJoiner.split``'s filled-opening loop. +# --------------------------------------------------------------------------- + + +def _make_void_copy_mock(has_filling_rel=True): + """Build the ``void_copy`` MagicMock returned by ``copy_class`` so its + ``HasFillings`` / ``VoidsElements`` / ``ObjectPlacement`` shape matches + what ``_add_void_copy`` mutates.""" + copy_placement = MagicMock(name="copy_placement") + copy_placement.is_a = lambda klass: klass == "IfcLocalPlacement" + void_relation = MagicMock(name="VoidsRelation") + void_copy = MagicMock(name="void_copy") + void_copy.HasFillings = (MagicMock(name="copy_filling_rel"),) if has_filling_rel else () + void_copy.VoidsElements = (void_relation,) + void_copy.ObjectPlacement = copy_placement + return void_copy, void_relation, copy_placement + + +def test_add_void_copy_strips_fillings_and_reparents_to_target_wall(): + """``_add_void_copy`` must create a pure-void IfcOpeningElement attached + to the target wall: the filling relationship copied along with the source + must be removed, ``VoidsElements[0].RelatingBuildingElement`` must point + at the target wall, and the representation must be a deep copy (not a + shared reference with the source).""" + from bonsai.bim.module.model.wall import _add_void_copy + + source_representation = MagicMock(name="source_representation") + source_opening = MagicMock(name="source_opening") + source_opening.Representation = source_representation + + void_copy, void_relation, copy_placement = _make_void_copy_mock() + carried_filling_rel = void_copy.HasFillings[0] + + target_placement = MagicMock(name="target_placement") + target_wall = MagicMock(name="target_wall") + target_wall.ObjectPlacement = target_placement + + ifc_file = MagicMock(name="ifc_file") + deep_copy_result = MagicMock(name="copied_representation") + + with ( + patch("bonsai.tool.Ifc.get", return_value=ifc_file), + patch("ifcopenshell.api.root.copy_class", return_value=void_copy) as mock_copy_class, + patch("ifcopenshell.util.element.copy_deep", return_value=deep_copy_result), + ): + _add_void_copy(target_wall, source_opening) + + # The carried-over filling relationship must be removed — the copy is a pure void. + ifc_file.remove.assert_called_once_with(carried_filling_rel) + # The void now points at the target wall, not the source's wall. + assert void_relation.RelatingBuildingElement is target_wall + # The placement is reparented under the target wall's local placement. + assert copy_placement.PlacementRelTo is target_placement + # The representation is deep-copied so future edits don't ripple back to source. + assert void_copy.Representation is deep_copy_result + mock_copy_class.assert_called_once_with(ifc_file, product=source_opening) + + +def test_add_void_copy_handles_source_with_no_fillings(): + """If the source opening has no ``HasFillings`` (the copy_class result + inherits that), the loop over ``void_copy.HasFillings or ()`` must run + zero times — no spurious ``ifc_file.remove`` call.""" + from bonsai.bim.module.model.wall import _add_void_copy + + source_opening = MagicMock(name="source_opening") + source_opening.Representation = MagicMock(name="rep") + + void_copy, _void_relation, _copy_placement = _make_void_copy_mock(has_filling_rel=False) + target_wall = MagicMock(name="target_wall") + + ifc_file = MagicMock(name="ifc_file") + + with ( + patch("bonsai.tool.Ifc.get", return_value=ifc_file), + patch("ifcopenshell.api.root.copy_class", return_value=void_copy), + patch("ifcopenshell.util.element.copy_deep", return_value=MagicMock()), + ): + _add_void_copy(target_wall, source_opening) + + ifc_file.remove.assert_not_called() + + +def test_filled_opening_void_straddle_decision_keeps_void_on_neighbour(): + """Replays the decision logic in ``DumbWallJoiner.split``'s filled-opening + loop for the case ``filling_position <= cut_percentage and void_straddles``: + filling stays on element1 (its centre is before the cut), but the void + extent crosses the cut, so the neighbour wall (element2) must receive a + pure-void copy via ``_add_void_copy``. + + Mirrors the unfilled-opening decision tests — exercises the boolean + branching rather than full ``split()`` integration.""" + cut_percentage = 0.5 + filling_position = 0.4 # filling centre on element1's side + min_t, max_t = 0.3, 0.7 # void extent straddles cut at 0.5 + + void_straddles = min_t < cut_percentage < max_t + filling_on_element2 = filling_position > cut_percentage + + # Expected branch: filling stays, but void straddles → add copy to element2. + assert void_straddles is True + assert filling_on_element2 is False + # Equivalent to the ``elif void_straddles:`` path adding a void copy to element2. + + +def test_filled_opening_void_straddle_with_filling_on_far_side_keeps_void_on_origin(): + """The symmetric case: ``filling_position > cut_percentage and void_straddles``. + Filling moves to element2 with the original void; element1 needs a + pure-void copy back (the void's element1 portion would otherwise be + orphaned). Documents the boolean state of the inner branch.""" + cut_percentage = 0.5 + filling_position = 0.6 # filling centre on element2's side + min_t, max_t = 0.3, 0.7 + + void_straddles = min_t < cut_percentage < max_t + filling_on_element2 = filling_position > cut_percentage + + assert void_straddles is True + assert filling_on_element2 is True + # Equivalent to the outer ``if filling_position > cut_percentage`` path + # taking its inner ``if void_straddles`` branch and adding a void copy + # back to element1. From f41f5dfdd8d9ded95d6629251ea55df6ff3a3a7a Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 21 May 2026 22:13:43 +0200 Subject: [PATCH 09/10] Fix wall split: preserve door/window fill rel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Splitting a wall through a door orphaned the door (door.FillsVoids became empty). The fill rel was being reassigned by setting its RelatedBuildingElement slot — schema-wise that's the filling slot, not the wall slot — so when remove_feature deleted the old opening it also cascade-removed the rel. Transferring via RelatingOpeningElement keeps the rel pointing at the new opening so the door stays associated. Pre-existing bug from 5a6476a57, surfaced by ef144dce2. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/wall.py | 2 +- src/bonsai/test/bim/feature/model.feature | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index c43bf2a7d2..0bedb86fc6 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -1364,7 +1364,7 @@ class DumbWallJoiner: tool.Ifc.get(), opening.Representation, exclude=["IfcGeometricRepresentationContext"] ) - rel.RelatedBuildingElement = element2 + rel.RelatingOpeningElement = new_opening # Remove the old opening ifcopenshell.api.feature.remove_feature(tool.Ifc.get(), feature=opening) diff --git a/src/bonsai/test/bim/feature/model.feature b/src/bonsai/test/bim/feature/model.feature index bfae14c6f6..1cab957837 100644 --- a/src/bonsai/test/bim/feature/model.feature +++ b/src/bonsai/test/bim/feature/model.feature @@ -285,6 +285,7 @@ Scenario: Split a wall which has a flipped door And the object "IfcWall/Wall" is selected And I press "bim.hotkey(hotkey='S_K')" Then the object "IfcDoor/Door" is at "8.01,0.1,0" + And the object "IfcWall/Wall.001" is filled by "IfcDoor/Door" Scenario: Offset walls Given an empty IFC project From cb2f20b2b6080aca6ea07b096ebe5f25970e7cda Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 27 May 2026 18:34:32 +0200 Subject: [PATCH 10/10] Add tests for decorator_cache + undo-resync dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two paired test files for the framework infrastructure landed earlier in this PR. test_decorator_cache.py (11 tests): * The 4-hook invalidation list (depsgraph_update_post + undo_post + redo_post + load_post) is symmetrically managed by install_decorator_cache_handlers / uninstall_decorator_cache_handlers. A future edit that drops a hook from one side without the other would land as a Blender segfault when a cached bpy.types.Object ref outlives its underlying ID block — the regression must surface as a test failure first. * install is idempotent (calling twice doesn't double-register). * uninstall when not installed doesn't raise. * The bump handler accepts Blender's variadic args. * The depsgraph predicate gates correctly: bumps on Object geometry or transform updates, silently skips on Material / NodeTree / Image updates (which would otherwise rebuild every cache on every node edit). * TokenCache.get_or_compute short-circuits on key+token match and recomputes when the token bumps. test_undo_resync_parametric_drafts.py (3 tests): * UNDO_REGENERATORS keys must all be in tool.Parametric.EDIT_TYPES. A typo would silently no-op on Ctrl+Z, restoring the desync the helper is meant to prevent. * The dispatcher skips objects with no active parametric edit (undo_post fires for every undo, most of which touch zero drafts). * The dispatcher silently skips parametric types that have no UNDO_REGENERATORS entry (door / window / array are IFC-derived with no draft preview mesh — they don't need a regenerator). Mocks use spec=bpy.types.Depsgraph / spec=bpy.types.DepsgraphUpdate / spec=tool.parametric.ParametricObject so typos in mocked-attribute access fail loudly (CLAUDE.md test discipline). Generated with the assistance of an AI coding tool. --- .../bim/module/model/test_decorator_cache.py | 176 ++++++++++++++++++ .../test_undo_resync_parametric_drafts.py | 106 +++++++++++ 2 files changed, 282 insertions(+) create mode 100644 src/bonsai/test/bim/module/model/test_decorator_cache.py create mode 100644 src/bonsai/test/bim/module/model/test_undo_resync_parametric_drafts.py diff --git a/src/bonsai/test/bim/module/model/test_decorator_cache.py b/src/bonsai/test/bim/module/model/test_decorator_cache.py new file mode 100644 index 0000000000..b98857b900 --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_decorator_cache.py @@ -0,0 +1,176 @@ +# 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 . +# +# This file was generated with the assistance of an AI coding tool. + +"""Contract tests for the shared decorator cache module. + +The cache token + persistent handler are the only thing protecting cached +``bpy.types.Object`` refs in dependent decorators from being dereferenced +after the underlying object is freed. These tests pin that contract: + +- The 4-hook invalidation list (depsgraph/undo/redo/load) is symmetrically + managed by install/uninstall. A future edit that drops a hook from one + side without the other lands as a Blender segfault — the regression must + surface as a test failure first. +- The handler increments the token and accepts Blender's variadic args.""" + +import bpy +import pytest + +from bonsai.bim import decorator_cache + +pytestmark = pytest.mark.model + + +@pytest.fixture(autouse=True) +def _reset_cache_token(): + """Fresh token between tests so the bump-count assertions are stable.""" + decorator_cache.reset_for_test() + yield + + +def test_install_and_uninstall_manage_all_invalidation_hooks(): + """install_decorator_cache_handlers() must register the bump handler in + every hook the dependent decorators rely on; uninstall must remove it + from every hook install touched. Catches the regression class where + a hook is dropped from one side and not the other.""" + expected_hooks = ( + bpy.app.handlers.depsgraph_update_post, + bpy.app.handlers.undo_post, + bpy.app.handlers.redo_post, + bpy.app.handlers.load_post, + ) + + # Defensive cleanup in case a previous addon-init run left the handler + # registered — the test must observe a clean slate before install(). + for hook in expected_hooks: + while decorator_cache._bump_decorator_cache_token in hook: + hook.remove(decorator_cache._bump_decorator_cache_token) + + try: + decorator_cache.install_decorator_cache_handlers() + for hook in expected_hooks: + assert decorator_cache._bump_decorator_cache_token in hook, ( + "install_decorator_cache_handlers() must register the bump " + "handler in every hook a dependent cache relies on" + ) + decorator_cache.uninstall_decorator_cache_handlers() + for hook in expected_hooks: + assert decorator_cache._bump_decorator_cache_token not in hook, ( + "uninstall_decorator_cache_handlers() must remove the bump " "handler from every hook install touched" + ) + finally: + # Make sure the test never leaves the handler dangling. + for hook in expected_hooks: + while decorator_cache._bump_decorator_cache_token in hook: + hook.remove(decorator_cache._bump_decorator_cache_token) + + +def test_install_is_idempotent(): + """Calling install twice must not double-register the bump handler — + the addon-init path may run on script reload and we don't want to + invalidate the cache twice per event.""" + hook = bpy.app.handlers.depsgraph_update_post + + while decorator_cache._bump_decorator_cache_token in hook: + hook.remove(decorator_cache._bump_decorator_cache_token) + + try: + decorator_cache.install_decorator_cache_handlers() + decorator_cache.install_decorator_cache_handlers() + appearances = sum(1 for h in hook if h is decorator_cache._bump_decorator_cache_token) + assert appearances == 1, "install must not double-register" + finally: + decorator_cache.uninstall_decorator_cache_handlers() + + +def test_bump_handler_increments_token(): + """undo / redo / load_post invoke the handler with at most one positional + argument (the scene or filepath). Every such call must bump the token — + those events legitimately invalidate every cached Object reference.""" + decorator_cache._bump_decorator_cache_token() + assert decorator_cache.get_decorator_cache_token() == 1 + decorator_cache._bump_decorator_cache_token("scene") + assert decorator_cache.get_decorator_cache_token() == 2 + + +def test_get_decorator_cache_token_reads_current_value(): + """``get_decorator_cache_token()`` is the public read interface — it must + reflect the current token, not a captured-at-import-time value.""" + initial = decorator_cache.get_decorator_cache_token() + decorator_cache._bump_decorator_cache_token() + assert decorator_cache.get_decorator_cache_token() == initial + 1 + + +def test_depsgraph_update_with_no_object_changes_does_not_bump(): + """depsgraph_update_post fires every animation frame, every driver + evaluation, and every UI-only state shift. None of those invalidate a + decorator's cached IFC-derived geometry — gating the bump is what makes + the ``TokenCache`` worth more than a per-frame recompute.""" + from unittest.mock import MagicMock + + initial = decorator_cache.get_decorator_cache_token() + depsgraph = MagicMock(spec=bpy.types.Depsgraph, name="depsgraph") + depsgraph.updates = [] # empty updates list — animation tick with no real changes + decorator_cache._bump_decorator_cache_token("scene", depsgraph) + assert ( + decorator_cache.get_decorator_cache_token() == initial + ), "depsgraph_update_post with no Object changes must not bump the token" + + +def test_depsgraph_update_with_object_geometry_change_bumps(): + """When the depsgraph reports an Object geometry or transform change, + cached references may now point at a renamed / freed ID block. The token + must advance so dependent caches re-fetch on the next read.""" + from unittest.mock import MagicMock + + initial = decorator_cache.get_decorator_cache_token() + update = MagicMock(spec=bpy.types.DepsgraphUpdate, name="update") + update.is_updated_geometry = True + update.is_updated_transform = False + update.id = bpy.data.objects.new("dep_cache_probe", None) + try: + depsgraph = MagicMock(spec=bpy.types.Depsgraph, name="depsgraph") + depsgraph.updates = [update] + decorator_cache._bump_decorator_cache_token("scene", depsgraph) + assert decorator_cache.get_decorator_cache_token() == initial + 1 + finally: + bpy.data.objects.remove(update.id, do_unlink=True) + + +def test_depsgraph_update_with_non_object_change_does_not_bump(): + """Material / NodeTree / Image updates fire depsgraph_update_post too + but never invalidate the decorator's Object-keyed caches. Filter them + out so a node-graph edit doesn't trigger a global cache rebuild.""" + from unittest.mock import MagicMock + + initial = decorator_cache.get_decorator_cache_token() + update = MagicMock(spec=bpy.types.DepsgraphUpdate, name="update") + update.is_updated_geometry = True + update.is_updated_transform = True + update.id = bpy.data.materials.new("dep_cache_probe_mat") + try: + depsgraph = MagicMock(spec=bpy.types.Depsgraph, name="depsgraph") + depsgraph.updates = [update] + decorator_cache._bump_decorator_cache_token("scene", depsgraph) + assert ( + decorator_cache.get_decorator_cache_token() == initial + ), "Non-Object ID updates must not bump the decorator cache token" + finally: + bpy.data.materials.remove(update.id, do_unlink=True) diff --git a/src/bonsai/test/bim/module/model/test_undo_resync_parametric_drafts.py b/src/bonsai/test/bim/module/model/test_undo_resync_parametric_drafts.py new file mode 100644 index 0000000000..9bf7c8c33a --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_undo_resync_parametric_drafts.py @@ -0,0 +1,106 @@ +# 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 . +# +# This file was generated with the assistance of an AI coding tool. + +"""Tests for ``parametric_lifecycle.resync_parametric_drafts_after_undo``. + +Blender's undo restores PropertyGroup field values but does not refire +their ``update`` callbacks, so the preview mesh of an in-progress +parametric draft desyncs from the gizmo dimension widget after Ctrl+Z. +The resync helper walks active drafts and re-runs the per-type +regenerator to bring preview back in line with the (restored) draft +state. This file pins the dispatch contract.""" + +from unittest.mock import MagicMock, patch + +import bpy +import pytest + +import bonsai.tool as tool +from bonsai.bim import parametric_lifecycle + +pytestmark = pytest.mark.model + + +def test_undo_regenerators_target_registered_parametric_types(): + """Every entry in ``UNDO_REGENERATORS`` must name a real parametric + type. A typo would silently no-op on Ctrl+Z, restoring the desync + this helper is meant to prevent.""" + registered_names = {f.name for f in tool.Parametric.EDIT_TYPES} + unknown = set(parametric_lifecycle.UNDO_REGENERATORS) - registered_names + assert not unknown, f"UNDO_REGENERATORS keys {unknown} are not in tool.Parametric.EDIT_TYPES" + + +def test_resync_skips_objects_not_in_parametric_edit(): + """Objects with no active parametric edit must not trigger any + regenerator — the helper is called from undo_post which fires on + every undo, including undos that touch zero parametric drafts.""" + captured = [] + + def fake_dispatch(obj): + captured.append(obj) + + with patch.dict(parametric_lifecycle.UNDO_REGENERATORS, {"wall": fake_dispatch}, clear=False), patch.object( + tool.Parametric, "is_object_editing", return_value=None + ): + parametric_lifecycle.resync_parametric_drafts_after_undo() + + assert captured == [] + + +def test_resync_dispatches_to_registered_regenerator_for_editing_object(): + """When an object is in parametric edit and its type has a registered + regenerator, the regenerator must run with that object as the sole + arg. This is the load-bearing branch: preview mesh re-renders from + current props, so the gizmo and preview re-sync.""" + captured = [] + + def fake_wall_regenerator(obj): + captured.append(obj) + + fake_feature = MagicMock(spec=tool.parametric.ParametricObject) + fake_feature.name = "wall" + + obj = bpy.data.objects.new("test_wall_obj", bpy.data.meshes.new("test_wall_mesh")) + try: + with patch.dict( + parametric_lifecycle.UNDO_REGENERATORS, {"wall": fake_wall_regenerator}, clear=False + ), patch.object(tool.Parametric, "is_object_editing", side_effect=lambda o: fake_feature if o is obj else None): + parametric_lifecycle.resync_parametric_drafts_after_undo() + finally: + bpy.data.objects.remove(obj, do_unlink=True) + + assert captured == [obj] + + +def test_resync_skips_editing_object_whose_type_has_no_regenerator(): + """A parametric type without an ``UNDO_REGENERATORS`` entry (door / + window / array — IFC-derived preview, no desync) must not raise; the + helper silently skips it.""" + fake_feature = MagicMock(spec=tool.parametric.ParametricObject) + fake_feature.name = "door" # door has no entry in UNDO_REGENERATORS + + obj = bpy.data.objects.new("test_door_obj", bpy.data.meshes.new("test_door_mesh")) + try: + with patch.object( + tool.Parametric, "is_object_editing", side_effect=lambda o: fake_feature if o is obj else None + ): + parametric_lifecycle.resync_parametric_drafts_after_undo() + finally: + bpy.data.objects.remove(obj, do_unlink=True)