From d1d1e1d4a2080d9518eb92844653bf4fa3f521f5 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Wed, 10 Jun 2026 20:40:33 +0200 Subject: [PATCH 1/4] Add railing parametric edit + schematic preview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port gizmos-8088's railing gizmo block to v0.8.0: - _RailingEditMixin (PathPreservingEditMixin specialisation) + EnableEditingRailing / CancelEditingRailing / FinishEditingRailing edit triad - CycleRailingType (2-value type cycler) + ToggleRailingUseManualSupports one-shot + EditRailingTerminalType - FlipRailingPathOrder + EnableEditingRailingPath / CancelEditingRailingPath / FinishEditingRailingPath path-edit operators (mutually exclusive with the schematic frame) - GizmoRailingSchematic (BaseSchematicGizmoGroup specialisation) — axonometric schematic frame with per-attribute dimension gizmos for FRAMELESS_PANEL + WALL_MOUNTED_HANDRAIL railing types; hover-on-attr highlights the schematic edges tagged with the matching feature Tests: test_railing_lifecycle.py (280 LOC) + test_railing_schematic.py (272 LOC). Drops the per-feature GizmoPreferences{Door,Window,Stair,Wall,Roof, Railing} PropertyGroups that the source commit added to bim/ui.py — that finer-grained per-attribute toggle model was deliberately collapsed to flat per-feature bools in the PR5b prefs sweep, and GizmoRailingSchematic gates on the flat ``prefs.gizmos.railing`` bool via ``gizmo_pref_name`` so no functionality is lost. Generated with the assistance of an AI coding tool. --- .../bonsai/bim/module/model/__init__.py | 4 + src/bonsai/bonsai/bim/module/model/railing.py | 720 +++++++++++++++++- .../module/model/test_railing_lifecycle.py | 280 +++++++ .../module/model/test_railing_schematic.py | 272 +++++++ 4 files changed, 1266 insertions(+), 10 deletions(-) create mode 100644 src/bonsai/test/bim/module/model/test_railing_lifecycle.py create mode 100644 src/bonsai/test/bim/module/model/test_railing_schematic.py diff --git a/src/bonsai/bonsai/bim/module/model/__init__.py b/src/bonsai/bonsai/bim/module/model/__init__.py index b30fb17896..edac430309 100644 --- a/src/bonsai/bonsai/bim/module/model/__init__.py +++ b/src/bonsai/bonsai/bim/module/model/__init__.py @@ -246,9 +246,13 @@ classes = ( railing.CopyRailingParameters, railing.AddRailing, railing.CancelEditingRailing, + railing.CycleRailingType, + railing.EditRailingTerminalType, railing.FinishEditingRailing, railing.FlipRailingPathOrder, railing.EnableEditingRailing, + railing.GizmoRailingSchematic, + railing.ToggleRailingUseManualSupports, railing.CancelEditingRailingPath, railing.FinishEditingRailingPath, railing.EnableEditingRailingPath, diff --git a/src/bonsai/bonsai/bim/module/model/railing.py b/src/bonsai/bonsai/bim/module/model/railing.py index 6f3697d51d..072b7d562d 100644 --- a/src/bonsai/bonsai/bim/module/model/railing.py +++ b/src/bonsai/bonsai/bim/module/model/railing.py @@ -18,7 +18,8 @@ import json -from typing import Any +import math +from typing import Any, get_args import bmesh import bpy @@ -27,14 +28,20 @@ import ifcopenshell.api.geometry import ifcopenshell.api.pset import ifcopenshell.util.representation import ifcopenshell.util.unit -from mathutils import Vector +from mathutils import Matrix, Vector import bonsai.core.geometry import bonsai.core.root import bonsai.tool as tool +from bonsai.bim.module.drawing import gizmos as gizmo +from bonsai.bim.module.drawing.gizmos import DimensionGizmoConfig +from bonsai.bim.module.model import prop from bonsai.bim.module.model.data import RailingData, refresh from bonsai.bim.module.model.decorator import ProfileDecorator from bonsai.bim.parametric_lifecycle import PathPreservingEditMixin +from bonsai.tool.cad import WELD_TOLERANCE + +V_ = tool.Blender.V_ # reference: # https://ifc43-docs.standards.buildingsmart.org/IFC/RELEASE/IFC4x3/HTML/lexical/IfcRailing.htm @@ -125,6 +132,56 @@ def update_bbim_railing_pset(element: ifcopenshell.entity_instance, railing_data ifcopenshell.api.pset.edit_pset(tool.Ifc.get(), pset=pset, properties={"Data": railing_data}) +def generate_wall_mounted_handrail_preview( + obj: bpy.types.Object, + props: "BIMRailingProperties", + path_data: dict[str, Any], + si_conversion: float, +) -> None: + """Viewport-only WALL_MOUNTED_HANDRAIL preview: rebuild ``obj.data`` from the same + geometry helper the IFC representation builder uses, without writing any IFC.""" + railing_path = [Vector(v) * si_conversion for v in path_data["verts"]] + looped_path = path_data["edges"][-1][-1] == path_data["edges"][0][0] + + geom = ifcopenshell.api.geometry.compute_wall_mounted_handrail_geometry( + railing_path=railing_path, + support_spacing=props.support_spacing, + railing_diameter=props.railing_diameter, + clear_width=props.clear_width, + height=props.height, + use_manual_supports=props.use_manual_supports, + terminal_type=props.terminal_type, + looped_path=looped_path, + unit_scale=1.0, # props are already SI; bypass the IFC project-units conversion + ) + + bm = tool.Blender.get_bmesh_for_mesh(obj.data, clean=True) + + tool.Cad.sweep_disk_along_polyline( + bm, + [Vector(p) for p in geom.handrail_polyline], + geom.handrail_radius, + arc_indices=geom.handrail_arc_point_indices, + ) + + for support in geom.supports: + tool.Cad.sweep_disk_along_polyline( + bm, + [Vector(p) for p in support.arc_polyline], + support.arc_radius, + ) + tool.Cad.add_disk_extrusion( + bm, + Vector(support.disk_position), + support.disk_radius, + support.disk_depth, + support.disk_z_rotation, + ) + + bmesh.ops.recalc_face_normals(bm, faces=bm.faces[:]) + tool.Blender.apply_bmesh(obj.data, bm) + + def update_railing_modifier_bmesh(context: bpy.types.Context) -> None: """before using should make sure that Data contains up-to-date information. If BBIM Pset just changed should call refresh() before updating bmesh @@ -140,6 +197,13 @@ def update_railing_modifier_bmesh(context: bpy.types.Context) -> None: path_data = RailingData.data["path_data"] si_conversion = ifcopenshell.util.unit.calculate_unit_scale(tool.Ifc.get()) + + # WALL_MOUNTED_HANDRAIL renders the preview from the compute helper; IFC stays + # untouched until Finish Editing rebuilds the representation. + if not props.is_editing_path and props.railing_type == "WALL_MOUNTED_HANDRAIL": + generate_wall_mounted_handrail_preview(obj, props, path_data, si_conversion) + return + # need to make sure we support edit mode # since users will probably be in edit mode when they'll be changing railing path bm = tool.Blender.get_bmesh_for_mesh(obj.data, clean=True) @@ -165,8 +229,6 @@ def update_railing_modifier_bmesh(context: bpy.types.Context) -> None: thickness = props.thickness spacing = props.spacing - # spacing - # split each edge in 3 segments by 0.5 * spacing by x-y plane main_edges = bm.edges[:] for main_edge in main_edges: bm_split_edge_at_offset(main_edge, spacing) @@ -211,7 +273,7 @@ def update_railing_modifier_bmesh(context: bpy.types.Context) -> None: bmesh.ops.dissolve_edges(bm, edges=edges_to_dissolve) bmesh.ops.dissolve_verts(bm, verts=verts_to_dissolve) # to remove unnecessary verts in 0 spacing case - bmesh.ops.remove_doubles(bm, verts=bm.verts, dist=0.0001) + bmesh.ops.remove_doubles(bm, verts=bm.verts, dist=WELD_TOLERANCE) bmesh.ops.recalc_face_normals(bm, faces=bm.faces[:]) @@ -271,8 +333,8 @@ def get_path_data(obj: bpy.types.Object) -> dict[str, Any]: segments.append((i - 1, 0)) break - # skip path verts if they just go vertical to avoid errors - if (v.co.xy - prev_v.co.xy).length <= 0.0001: + # Vertical-only segments project to a degenerate XY edge; skip to avoid divide-by-zero downstream. + if (v.co.xy - prev_v.co.xy).length <= WELD_TOLERANCE: continue points.append(v.co) @@ -407,9 +469,8 @@ class CopyRailingParameters(bpy.types.Operator, tool.Ifc.Operator): class _RailingEditMixin(PathPreservingEditMixin): - """Type-specific hooks for railing parametric-edit operators. Single-object - (active_object). ``path_data`` is preserved through the edit; the separate - ``Enable/Finish/CancelEditingRailingPath`` operators handle path editing.""" + """Single-object (active_object) railing-edit hooks; path_data is preserved + through the edit (path editing is a separate operator family).""" pset_name = "BBIM_Railing" @@ -439,6 +500,66 @@ class _RailingEditMixin(PathPreservingEditMixin): def _update_modifier_bmesh(cls, obj: bpy.types.Object, context: bpy.types.Context) -> None: update_railing_modifier_bmesh(context) + @classmethod + def _finish_one(cls, obj: bpy.types.Object, context: bpy.types.Context) -> None: + """Skip the IFC commit when the draft matches the stored pset (no-op edit).""" + resolved = cls._resolve(obj) + if resolved is None: + return + element, props = resolved + pset_data = tool.Model.get_modeling_bbim_pset_data(obj, cls.pset_name) + stored = pset_data["data_dict"] + path_data = stored["path_data"] + draft = props.get_general_kwargs(convert_to_project_units=True) + draft["path_data"] = path_data + + if draft == stored: + props.is_editing = False + return + + cls._update_pset(element, draft) + cls._update_modifier_ifc_data(obj, context) + props.is_editing = False + + @classmethod + def _cancel_one(cls, obj: bpy.types.Object, context: bpy.types.Context) -> None: + """WALL_MOUNTED_HANDRAIL switches the representation back to Body on cancel + (the cylinder preview is lower-poly than the committed swept-disk solid). + Skip the switch when the draft matches the stored pset.""" + resolved = cls._resolve(obj) + if resolved is None: + return + _element, props = resolved + 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) + + if nothing_changed: + props.is_editing = False + return + + if props.railing_type == "WALL_MOUNTED_HANDRAIL": + element = tool.Ifc.get_entity(obj) + assert element + body = ifcopenshell.util.representation.get_representation(element, "Model", "Body", "MODEL_VIEW") + if body: + bonsai.core.geometry.switch_representation( + tool.Ifc, + tool.Geometry, + obj=obj, + representation=body, + ) + else: + cls._update_modifier_bmesh(obj, context) + + props.is_editing = False + class EnableEditingRailing(_RailingEditMixin, bpy.types.Operator, tool.Ifc.Operator): bl_idname = "bim.enable_editing_railing" @@ -467,6 +588,575 @@ class FinishEditingRailing(_RailingEditMixin, bpy.types.Operator, tool.Ifc.Opera return self._finish_targets(context) +class CycleRailingType(bpy.types.Operator, tool.Ifc.Operator, gizmo.CycleTypeMixin): + """Cycle railing_type (FRAMELESS_PANEL ↔ WALL_MOUNTED_HANDRAIL). Shift+click reverses.""" + + bl_idname = "bim.cycle_railing_type" + bl_label = "Cycle Railing Type" + bl_options = {"REGISTER", "UNDO"} + + element_checker = tool.Blender.Modifier.is_railing + props_getter = tool.Model.get_railing_props + type_literal = tool.Model.RailingType + type_attr = "railing_type" + + def _execute(self, context: bpy.types.Context) -> set[str]: + return self._cycle_type(context) + + +class ToggleRailingUseManualSupports(bpy.types.Operator): + """Flip use_manual_supports on the active WALL_MOUNTED_HANDRAIL railing. + + No-op unless a parametric edit is active and the railing is wall-mounted. + """ + + bl_idname = "bim.toggle_railing_use_manual_supports" + bl_label = "Toggle Railing Manual Supports" + bl_description = "Switch between automatic support spacing and manual per-vertex placement" + bl_options = {"REGISTER", "UNDO"} + + def execute(self, context): + obj = context.active_object + if not obj: + return {"CANCELLED"} + props = tool.Model.get_railing_props(obj) + if not props.is_editing or props.railing_type != "WALL_MOUNTED_HANDRAIL": + return {"CANCELLED"} + props.use_manual_supports = not props.use_manual_supports + return {"FINISHED"} + + +class EditRailingTerminalType(bpy.types.Operator): + """Popup menu for terminal_type; writes the picked value via a HIDDEN string property.""" + + bl_idname = "bim.edit_railing_terminal_type" + bl_label = "Choose Railing Terminal Type" + bl_description = "Pick the cap geometry applied at the rail ends" + bl_options = {"REGISTER", "UNDO"} + + terminal_type: bpy.props.StringProperty(name="Terminal Type", default="", options={"HIDDEN", "SKIP_SAVE"}) + + def invoke(self, context: bpy.types.Context, event: bpy.types.Event) -> set[str]: + obj = context.active_object + if not obj: + return {"CANCELLED"} + props = tool.Model.get_railing_props(obj) + if not props.is_editing or props.railing_type != "WALL_MOUNTED_HANDRAIL": + return {"CANCELLED"} + + choices = [v for v in get_args(prop.CapType)] + + def draw(menu_self, _menu_context): + layout = menu_self.layout + for v in choices: + op = layout.operator(self.bl_idname, text=v) + op.terminal_type = v + + context.window_manager.popup_menu(draw, title="Terminal Type", icon="MOD_LATTICE") + return {"FINISHED"} + + def execute(self, context: bpy.types.Context) -> set[str]: + # Re-open the popup if called without a value (e.g. from the command palette). + if not self.terminal_type: + return self.invoke(context, None) # type: ignore[arg-type] + obj = context.active_object + if not obj: + return {"CANCELLED"} + props = tool.Model.get_railing_props(obj) + if self.terminal_type not in get_args(prop.CapType): + self.report({"ERROR"}, f"Unknown terminal_type: {self.terminal_type!r}") + return {"CANCELLED"} + props.terminal_type = self.terminal_type # type: ignore[assignment] + return {"FINISHED"} + + +def _format_attr_distance(attr_name: str): + """text_formatter that renders the named property as a distance, ignoring the + dimension's visible-length argument (which is fixed for schematic gizmos).""" + return lambda p, _v: tool.Unit.format_distance(getattr(p, attr_name)) + + +class GizmoRailingSchematic(bpy.types.GizmoGroup, gizmo.BaseSchematicGizmoGroup): + """Schematic-frame parametric editor for railings. Mutually exclusive with path-edit mode.""" + + bl_idname = "OBJECT_GGT_bim_railing_edition" + bl_label = "Railing Editing Gizmo" + bl_space_type = "VIEW_3D" + bl_region_type = "WINDOW" + bl_options = {"3D", "PERSISTENT"} + + enable_editing_operator = "bim.enable_editing_railing" + finish_editing_operator = "bim.finish_editing_railing" + cancel_editing_operator = "bim.cancel_editing_railing" + cycle_type_operator = "bim.cycle_railing_type" + + props_getter = tool.Model.get_railing_props + gizmo_pref_name = "railing" + + # Schematic-local layout. +X → screen RIGHT, +Y → screen UP, +Z → toward viewer + # (post billboard rotation). Each dimension is anchored alongside the feature it + # measures so the label, not the bar length, carries the value. + SCHEMATIC_MESH_HEIGHT_FRAC = 0.9 # Mesh top edge in schematic-local +Y + SCHEMATIC_MESH_WIDTH_FRAC = 0.7 # Mesh side edges in schematic-local ±X + SCHEMATIC_MESH_RAIL_Y_FRAC = SCHEMATIC_MESH_HEIGHT_FRAC / 2 # WALL_MOUNTED_HANDRAIL rail centreline + SCHEMATIC_MESH_DEPTH_FRAC = 0.06 # Panel depth — small so the schematic reads as slabs not boxes + # WALL_MOUNTED_HANDRAIL dimensions — fractions of schematic_box_size so they + # scale with the host group's box size. + SCHEMATIC_RAIL_RADIUS_FRAC = 0.05 + SCHEMATIC_RAIL_CLEAR_FRAC = 0.5 # Stylised — wider than real-world for visible bracket arm + SCHEMATIC_RAIL_INSET_FRAC = 0.08 # Wall extends past the outermost support on both sides + + @classmethod + def schematic_rail_radius(cls) -> float: + return cls.schematic_box_size * cls.SCHEMATIC_RAIL_RADIUS_FRAC + + @classmethod + def schematic_rail_clear(cls) -> float: + return cls.schematic_box_size * cls.SCHEMATIC_RAIL_CLEAR_FRAC + + # Axonometric 3/4 view: +Z projects down-and-left so the depth axis + # is visibly separated from the back face. Without the X tilt, panel + # thickness (schematic-local Z) collapses to a near-horizontal bar. + schematic_view_rotation = Matrix.Rotation(math.radians(20), 4, "X") @ Matrix.Rotation(math.radians(-25), 4, "Y") + + # Hover a dimension → highlight the schematic edges tagged with the matching feature. + # Tags are written by the mesh builders. "spacing" is empty space (no edges) so it's + # absent from this map and gracefully no-ops on hover. + schematic_attr_to_feature = { + "height": "panel_height", + "thickness": "panel_thickness", + "railing_diameter": "rail_tube", + "clear_width": "bracket", + "support_spacing": "bracket", + } + + schematic_dimension_props = [ + # ── FRAMELESS_PANEL ───────────────────────────────────────────── + DimensionGizmoConfig( + attr_name="height", + axis=(0, 1, 0), + min_value=0.01, + # Gated to FRAMELESS_PANEL: in WALL_MOUNTED_HANDRAIL, height only + # feeds TO_FLOOR / TO_END_POST_AND_FLOOR terminals so dragging it + # is a no-op under the default "180" terminal. + visibility_condition=lambda p: p.railing_type == "FRAMELESS_PANEL", + matrix_position=lambda p: Vector((-GizmoRailingSchematic.SCHEMATIC_MESH_WIDTH_FRAC / 2 - 0.08, 0.0, 0.0)), + schematic_visible_length=SCHEMATIC_MESH_HEIGHT_FRAC, + text_formatter=_format_attr_distance("height"), + ), + DimensionGizmoConfig( + attr_name="thickness", + axis=(0, 0, 1), # panel depth — projects to a true depth direction under the 3/4 tilt + min_value=0.005, + visibility_condition=lambda p: p.railing_type == "FRAMELESS_PANEL", + matrix_position=lambda p: Vector( + ( + ( + -GizmoRailingSchematic.SCHEMATIC_MESH_WIDTH_FRAC / 2 + - GizmoRailingSchematic.SCHEMATIC_MESH_GAP_HALF_WIDTH + ) + / 2, + GizmoRailingSchematic.SCHEMATIC_MESH_HEIGHT_FRAC + 0.05, + -GizmoRailingSchematic.SCHEMATIC_MESH_DEPTH_FRAC / 2, + ) + ), + schematic_visible_length=0.4, # longer than default to survive depth foreshortening + text_formatter=_format_attr_distance("thickness"), + ), + DimensionGizmoConfig( + attr_name="spacing", + axis=(1, 0, 0), + min_value=0.0, # zero-spacing collapses the picket gap into a single continuous panel + visibility_condition=lambda p: p.railing_type == "FRAMELESS_PANEL", + matrix_position=lambda p: Vector((0.0, -0.1, 0.0)), + text_formatter=_format_attr_distance("spacing"), + ), + # ── WALL_MOUNTED_HANDRAIL ────────────────────────────────────── + DimensionGizmoConfig( + attr_name="railing_diameter", + axis=(0, 1, 0), + min_value=0.001, + visibility_condition=lambda p: p.railing_type == "WALL_MOUNTED_HANDRAIL", + matrix_position=lambda p: Vector( + ( + -GizmoRailingSchematic.SCHEMATIC_MESH_WIDTH_FRAC / 2 - 0.05, + GizmoRailingSchematic.SCHEMATIC_MESH_RAIL_Y_FRAC - 0.09, + GizmoRailingSchematic.schematic_rail_clear(), + ) + ), + text_formatter=_format_attr_distance("railing_diameter"), + ), + DimensionGizmoConfig( + attr_name="clear_width", + axis=(0, 0, 1), # +Z is the wall-to-rail perpendicular axis under the 3/4 tilt + min_value=0.001, + visibility_condition=lambda p: p.railing_type == "WALL_MOUNTED_HANDRAIL", + matrix_position=lambda p: Vector( + ( + 0.0, + GizmoRailingSchematic.SCHEMATIC_MESH_RAIL_Y_FRAC, + 0.0, + ) + ), + schematic_visible_length=0.36, # 2× default so the call-out survives depth projection + text_formatter=_format_attr_distance("clear_width"), + ), + DimensionGizmoConfig( + attr_name="support_spacing", + axis=(1, 0, 0), + min_value=0.05, + visibility_condition=lambda p: (p.railing_type == "WALL_MOUNTED_HANDRAIL" and not p.use_manual_supports), + matrix_position=lambda p: Vector( + ( + -GizmoRailingSchematic.SCHEMATIC_MESH_WIDTH_FRAC / 2 + + GizmoRailingSchematic.SCHEMATIC_RAIL_INSET_FRAC, + -0.18, + 0.0, + ) + ), + # Bare names (not Gizmo…SCHEMATIC_…) because the class is still under construction here. + schematic_visible_length=SCHEMATIC_MESH_WIDTH_FRAC - 2 * SCHEMATIC_RAIL_INSET_FRAC, + text_formatter=_format_attr_distance("support_spacing"), + ), + ] + + @classmethod + def is_element_type(cls, element: ifcopenshell.entity_instance) -> bool: + return tool.Blender.Modifier.is_railing(element) + + @classmethod + def schematic_cache_key(cls, props) -> tuple: + """Cache the schematic mesh by ``railing_type`` — proportions are fixed + per type, so the bmesh build runs at most twice across a session + (once for ``FRAMELESS_PANEL``, once for ``WALL_MOUNTED_HANDRAIL``) + rather than once per draw call.""" + return (props.railing_type,) + + def setup_element_specific_gizmos(self, context: bpy.types.Context) -> None: + """Create the WALL_MOUNTED_HANDRAIL-only affordances on the schematic. + + Two static lock glyphs (open/closed) for toggling + ``use_manual_supports``: instantiate both and let the per-frame state + query pick which one to show. State-aware icons use a static pair + rather than a single dynamic gizmo to avoid ``prop_path`` resolution + in the render path. + + Plus a cycle-glyph at the rail end that opens the ``terminal_type`` + popup when clicked. + """ + default_color, highlight_color = self.get_decoration_colors() + + for slot in ("lock_open_gizmo", "lock_closed_gizmo"): + bl_idname = "VIEW3D_GT_lock_open" if slot == "lock_open_gizmo" else "VIEW3D_GT_lock_closed" + gz = self.gizmos.new(bl_idname) + gz.color = default_color + gz.color_highlight = highlight_color + gz.use_draw_scale = False + gz.alpha = 0.8 + gz.target_set_operator("bim.toggle_railing_use_manual_supports") + setattr(self, slot, gz) + + self.terminal_gizmo = self.gizmos.new("VIEW3D_GT_cycle") + self.terminal_gizmo.color = default_color + self.terminal_gizmo.color_highlight = highlight_color + self.terminal_gizmo.use_draw_scale = False + self.terminal_gizmo.alpha = 0.8 + self.terminal_gizmo.target_set_operator("bim.edit_railing_terminal_type") + + def _refresh_element_specific(self, context: bpy.types.Context, mw: "Matrix", props) -> None: + """Position and gate the WALL_MOUNTED_HANDRAIL-only gizmos. + + - Lock glyphs: only WALL_MOUNTED_HANDRAIL while editing. Show + ``lock_open`` when ``use_manual_supports`` is True, the closed + padlock when False ("auto-spacing is locked to support_spacing"). + - Terminal gizmo: same gating, positioned just past the right rail + end so it reads as "configure the rail's end cap". + """ + super()._refresh_element_specific(context, mw, props) + + # ``draw_prepare`` can fire on a freshly recreated GizmoGroup instance + # before ``setup_element_specific_gizmos`` has populated the lock / + # terminal attributes (Blender 5.x recreates per-region groups on + # reload). Bail out cheaply; the next refresh after setup completes + # will reposition them correctly. + if not hasattr(self, "lock_open_gizmo"): + return + + # Single gate for all WALL_MOUNTED_HANDRAIL extras. + active = props.is_editing and not props.is_editing_path and props.railing_type == "WALL_MOUNTED_HANDRAIL" + + if not active: + self.lock_open_gizmo.hide = True + self.lock_closed_gizmo.hide = True + self.terminal_gizmo.hide = True + return + + billboard_rot = self._frame_billboard_rot + view_rotation = self.schematic_view_rotation + anchor = self._compute_schematic_anchor(props, mw, billboard_rot) + + # ── Lock glyphs for use_manual_supports ────────────────────────── + # Sit just above the wall's bottom line, near the centre of the + # schematic — visually grouped with the dimension it controls + # (support_spacing) without overlapping the arrow tail below. + is_manual = bool(props.use_manual_supports) + self.lock_open_gizmo.hide = not is_manual + self.lock_closed_gizmo.hide = is_manual + lock_local = Vector((0.0, 0.05, 0.0)) + lock_world = anchor + billboard_rot @ view_rotation @ lock_local + lock_matrix = gizmo.billboarded_at(lock_world, billboard_rot, 0.09) + self.lock_open_gizmo.matrix_basis = lock_matrix + self.lock_closed_gizmo.matrix_basis = lock_matrix + + # ── Terminal-type popup gizmo at the right rail end ────────────── + # Pushed well past the right wall edge so the icon doesn't crowd + # the wall outline or the bracket attach point. At rail height and + # rail depth so it reads as "attached to the rail terminal". + self.terminal_gizmo.hide = False + terminal_local = Vector( + ( + self.SCHEMATIC_MESH_WIDTH_FRAC / 2 + 0.25, + self.SCHEMATIC_MESH_RAIL_Y_FRAC, + self.schematic_rail_clear(), + ) + ) + terminal_world = anchor + billboard_rot @ view_rotation @ terminal_local + self.terminal_gizmo.matrix_basis = gizmo.billboarded_at(terminal_world, billboard_rot, 0.18) + + def update_editing_gizmos(self, context: bpy.types.Context, mw: "Matrix", props: "BIMRailingProperties") -> None: + """Hide the pen gizmo while polyline path-edit is active; reposition the cycle icon. + + The base class shows the pen gizmo whenever ``is_editing`` is False, + which is the case during path-edit too. Allowing the user to click + through into parametric edit while the polyline mesh is open in EDIT + mode mixes two distinct editing states and leaves a stale draft if + they cancel out — block the entry point instead. The operator itself + is intentionally not guarded (callers via scripting can still invoke + it); this is the UX-level enforcement. + + The cycle icon defaults to the editing icon row (next to validate / + cancel) via the parent's positioning. We move it to just above the + schematic mesh so it reads as "cycle the railing type *shown here*" + — associated with the preview the user is interacting with, not a + generic editing button at the bottom of the schematic. + """ + super().update_editing_gizmos(context, mw, props) + if props.is_editing_path: + self.pen_gizmo.hide = True + + if props.is_editing and not props.is_editing_path: + billboard_rot = self._frame_billboard_rot + view_rotation = self.schematic_view_rotation + anchor = self._compute_schematic_anchor(props, mw, billboard_rot) + # Comfortably above the mesh top edge so the icon doesn't crowd + # the ``thickness`` / ``clear_width`` dimension callouts that + # already sit just above the panel/wall. + cycle_local = Vector((0.0, self.SCHEMATIC_MESH_HEIGHT_FRAC + 0.25, 0.0)) + world_pos = anchor + billboard_rot @ view_rotation @ cycle_local + # 30% smaller than the editing-icon-row default (0.30 → 0.21): + # the cycle is a tertiary affordance compared to pen/validate/cancel. + self.cycle_gizmo.matrix_basis = gizmo.billboarded_at(world_pos, billboard_rot, 0.21) + + @classmethod + def build_schematic_mesh(cls, props) -> "bmesh.types.BMesh": + """Build a wireframe preview of the railing in schematic-local coordinates. + + FRAMELESS_PANEL renders as a box whose proportions track the bound + properties (height / thickness / spacing); WALL_MOUNTED_HANDRAIL + renders as a horizontal tube with two L-shaped supports whose + proportions track railing_diameter / clear_width / support_spacing. + Both are scaled to fit inside ``[-schematic_box_size, +schematic_box_size]`` + on each axis so the schematic reads the same regardless of absolute + property values. + + The mesh is decorative — clicks land on the labeled sliders, not on + the preview geometry. See ``BaseSchematicGizmoGroup`` for the + draw-handler lifecycle. + """ + bm = bmesh.new() + if props.railing_type == "FRAMELESS_PANEL": + cls._build_frameless_panel_schematic(bm, props) + else: + cls._build_wall_mounted_handrail_schematic(bm, props) + return bm + + # Schematic-local half-width of the visible gap between the two panel boxes. + # Conveys the "spacing" semantic at a glance — the user sees two pickets + # separated by air, with the spacing dimension emerging from that gap. + SCHEMATIC_MESH_GAP_HALF_WIDTH = 0.05 + + @classmethod + def _build_frameless_panel_schematic(cls, bm: "bmesh.types.BMesh", props) -> None: + """Stylised panel: two wireframe boxes with a visible gap between them. + + The box edges sit at the ``SCHEMATIC_MESH_*_FRAC`` positions + (matching where the dimension gizmos anchor), so each dimension line + visually starts at the geometry feature it measures. Internal + proportions are stable across drags — the actual values are shown + through the dimension labels, while the schematic communicates + which feature each label refers to. The gap between the two boxes + (set by ``SCHEMATIC_MESH_GAP_HALF_WIDTH``) gives the "spacing" + dimension a real visual referent. + + Edges are tagged on a string layer so hover-highlight can colour + the geometric feature being measured: vertical edges → height, + depth edges → thickness. The X-aligned edges along the panel + width are untagged (they don't correspond to a single dimension). + """ + hw = cls.SCHEMATIC_MESH_WIDTH_FRAC / 2 + hd = cls.SCHEMATIC_MESH_DEPTH_FRAC / 2 + h_top = cls.SCHEMATIC_MESH_HEIGHT_FRAC + gap = cls.SCHEMATIC_MESH_GAP_HALF_WIDTH + + layer_name = cls.SCHEMATIC_FEATURE_LAYER_NAME + feat_layer = bm.edges.layers.string.get(layer_name) or bm.edges.layers.string.new(layer_name) + + # Edge index → feature tag for one box. Order matches the (a, b) + # tuple order below: bottom ring (4) + top ring (4) + verticals (4). + edge_tags_per_box = ( + b"", # (0,1) bottom-back, X-aligned + b"panel_thickness", # (1,2) bottom-right, Z-aligned + b"", # (2,3) bottom-front, X-aligned + b"panel_thickness", # (3,0) bottom-left, Z-aligned + b"", # (4,5) top-back, X-aligned + b"panel_thickness", # (5,6) top-right, Z-aligned + b"", # (6,7) top-front, X-aligned + b"panel_thickness", # (7,4) top-left, Z-aligned + b"panel_height", # (0,4) vertical back-left + b"panel_height", # (1,5) vertical back-right + b"panel_height", # (2,6) vertical front-right + b"panel_height", # (3,7) vertical front-left + ) + + # Build two separate wireframe boxes — one on each side of the central + # gap. The boxes share the same Y range (0..h_top) and Z range (±hd) + # but split the X range so the gap from -gap to +gap stays empty. + for x_left, x_right in ((-hw, -gap), (gap, hw)): + corners = [ + bm.verts.new((x_left, 0.0, -hd)), + bm.verts.new((x_right, 0.0, -hd)), + bm.verts.new((x_right, 0.0, hd)), + bm.verts.new((x_left, 0.0, hd)), + bm.verts.new((x_left, h_top, -hd)), + bm.verts.new((x_right, h_top, -hd)), + bm.verts.new((x_right, h_top, hd)), + bm.verts.new((x_left, h_top, hd)), + ] + for tag, (a, b) in zip( + edge_tags_per_box, + ( + (0, 1), + (1, 2), + (2, 3), + (3, 0), # bottom ring + (4, 5), + (5, 6), + (6, 7), + (7, 4), # top ring + (0, 4), + (1, 5), + (2, 6), + (3, 7), # vertical edges + ), + ): + edge = bm.edges.new((corners[a], corners[b])) + if tag: + edge[feat_layer] = tag + + @classmethod + def _build_wall_mounted_handrail_schematic(cls, bm: "bmesh.types.BMesh", props) -> None: + """Stylised wall-mounted handrail: wall outline, hex tube, two L-brackets. + + Three visual elements convey "rail mounted on a wall": + + - **Wall outline** — a wireframe rectangle in the YZ plane at ``z=0``, + extending slightly past the rail ends so the wall reads as a + surface the rail is *attached to* rather than a coincident frame. + - **Handrail tube** — a hexagonal cross-section extruded along ±X + at ``z=+clear_s`` (in front of the wall), at ``y=rail_y``. + - **L-shaped brackets** at each rail end — from the rail centreline + drop a short distance, then run perpendicular back to the wall + plane. Mirrors the standard wall-mount bracket geometry: a + horizontal arm holding the rail off the wall, a vertical drop + attaching to the rail. + + Like ``_build_frameless_panel_schematic``, the schematic uses fixed + proportions so the dimension gizmos' anchor points stay aligned + with the geometry features regardless of property values. + """ + half_len = cls.SCHEMATIC_MESH_WIDTH_FRAC / 2 + wall_top = cls.SCHEMATIC_MESH_HEIGHT_FRAC + rail_y = cls.SCHEMATIC_MESH_RAIL_Y_FRAC # rail sits at half wall height + radius_s = cls.schematic_rail_radius() + clear_s = cls.schematic_rail_clear() + + layer_name = cls.SCHEMATIC_FEATURE_LAYER_NAME + feat_layer = bm.edges.layers.string.get(layer_name) or bm.edges.layers.string.new(layer_name) + + # ── Wall outline (rectangle at z=0, slightly wider than the rail) ── + # Spans the full schematic height; the rail attaches in the middle, + # so the wall reads as "continuing past the rail above and below". + # Wall edges stay untagged — they're background context, not a + # feature any dimension measures. + wall_extra = 0.08 + wall_x_left = -half_len - wall_extra + wall_x_right = half_len + wall_extra + wall_corners = [ + bm.verts.new((wall_x_left, 0.0, 0.0)), + bm.verts.new((wall_x_right, 0.0, 0.0)), + bm.verts.new((wall_x_right, wall_top, 0.0)), + bm.verts.new((wall_x_left, wall_top, 0.0)), + ] + for a, b in ((0, 1), (1, 2), (2, 3), (3, 0)): + bm.edges.new((wall_corners[a], wall_corners[b])) + + # ── Handrail tube (hex cross-section in YZ, extruded along X) ────── + # Centred on the rail centreline at (±(half_len - rail_inset), + # rail_y, +clear_s) — in front of the wall plane at z=0. The tube + # is shorter than the wall so the wall visibly extends past it on + # both sides; the L-brackets sit at the tube ends, so the leftmost + # bracket no longer coincides with the wall's left edge. + rail_inset = cls.SCHEMATIC_RAIL_INSET_FRAC + rail_x_left = -half_len + rail_inset + rail_x_right = half_len - rail_inset + segments = 6 + ring_left, ring_right = [], [] + for i in range(segments): + theta = 2 * math.pi * i / segments + dy = math.cos(theta) * radius_s + dz = math.sin(theta) * radius_s + ring_left.append(bm.verts.new((rail_x_left, rail_y + dy, clear_s + dz))) + ring_right.append(bm.verts.new((rail_x_right, rail_y + dy, clear_s + dz))) + # All hex-tube edges tagged "rail_tube" so they highlight together + # when the railing_diameter dimension is hovered. + for i in range(segments): + j = (i + 1) % segments + e_left = bm.edges.new((ring_left[i], ring_left[j])) + e_right = bm.edges.new((ring_right[i], ring_right[j])) + e_axial = bm.edges.new((ring_left[i], ring_right[i])) + e_left[feat_layer] = b"rail_tube" + e_right[feat_layer] = b"rail_tube" + e_axial[feat_layer] = b"rail_tube" + + # ── L-brackets at each rail end (rail → drop → wall) ─────────────── + # Bracket attach points follow the rail ends, so they're pulled + # inward by ``rail_inset`` from the wall edges. From the rail + # centreline, drop ``bracket_drop`` in Y, then run perpendicular + # back to the wall plane (z=0). The L shape reads as a wall-mount + # bracket under the 3/4 tilt. Both bracket segments tagged + # "bracket" so they highlight when clear_width OR support_spacing + # is hovered (both dimensions measure features of the supports). + bracket_drop = 0.06 + for x in (rail_x_left, rail_x_right): + v_rail = bm.verts.new((x, rail_y, clear_s)) + v_corner = bm.verts.new((x, rail_y - bracket_drop, clear_s)) + v_wall = bm.verts.new((x, rail_y - bracket_drop, 0.0)) + e1 = bm.edges.new((v_rail, v_corner)) + e2 = bm.edges.new((v_corner, v_wall)) + e1[feat_layer] = b"bracket" + e2[feat_layer] = b"bracket" + + class FlipRailingPathOrder(bpy.types.Operator, tool.Ifc.Operator): bl_idname = "bim.flip_railing_path_order" bl_label = "Flip Railing Path Order" @@ -510,6 +1200,16 @@ class EnableEditingRailingPath(bpy.types.Operator, tool.Ifc.Operator): [o.select_set(False) for o in context.selected_objects if o != obj] assert obj props = tool.Model.get_railing_props(obj) + + # Auto-commit any in-progress parametric draft before switching to + # path-edit. ``set_props_kwargs_from_ifc_data`` a few lines below + # overwrites props with the pset's stored values — without committing + # first, anything the user dragged on a dimension gizmo (height, + # diameter, …) would be silently discarded the moment path-edit + # starts. + if props.is_editing: + tool.Parametric.commit_object_draft(obj, "bim.finish_editing_railing") + data = tool.Model.get_modeling_bbim_pset_data(obj, "BBIM_Railing")["data_dict"] # required since we could load pset from .ifc and BIMRoofProperties won't be set props.set_props_kwargs_from_ifc_data(data) diff --git a/src/bonsai/test/bim/module/model/test_railing_lifecycle.py b/src/bonsai/test/bim/module/model/test_railing_lifecycle.py new file mode 100644 index 0000000000..bcc7005bf9 --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_railing_lifecycle.py @@ -0,0 +1,280 @@ +# 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. + +"""Unit coverage for the ``_RailingEditMixin`` lifecycle overrides. + +The generic ``PathPreservingEditMixin`` lifecycle is tested in +``test_parametric_lifecycle.py``. This file pins the **railing-specific +overrides** that subclass it: + +- ``_RailingEditMixin._finish_one`` short-circuit: when the draft equals + the stored pset, ``_update_pset`` and ``_update_modifier_ifc_data`` are + skipped so an Enable → Finish-without-changes cycle creates no new + ``IfcShapeRepresentation``. +- ``_RailingEditMixin._cancel_one`` short-circuit: same logic guards the + expensive ``bonsai.core.geometry.switch_representation`` call (which + re-tessellates the swept-disk solid) when nothing actually changed. +- ``_RailingEditMixin._cancel_one`` WALL_MOUNTED_HANDRAIL branch: when + changes WERE made, the cancel reloads the IFC body via + ``switch_representation`` instead of running ``update_modifier_bmesh`` + (which would leave the low-poly cylinder-segment preview on screen). +""" + +from unittest import mock + +import pytest + +from test.bim.conftest import _FakePropsBase +from test.bim.conftest import make_lifecycle_obj as _make_obj + +pytestmark = pytest.mark.model + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +class _FakeRailingProps(_FakePropsBase): + """Stand-in for ``BIMRailingProperties`` — adds ``railing_type`` on top of + the shared parametric-edit contract. Starts in ``is_editing=True`` because + the railing-specific overrides under test only fire on Finish / Cancel, + not on Enable.""" + + def __init__(self, railing_type: str = "WALL_MOUNTED_HANDRAIL", general: dict | None = None): + super().__init__(general=general if general is not None else {"railing_type": railing_type, "height": 1.0}) + self.railing_type = railing_type + self.is_editing = True + + +@pytest.fixture +def patched_railing(): + """Patch the railing module's external references for unit testing. + + ``_RailingEditMixin`` calls ``tool.Model.get_modeling_bbim_pset_data``, + ``tool.Ifc.get_entity``, ``ifcopenshell.util.representation.get_representation``, + and ``bonsai.core.geometry.switch_representation`` — each looked up + through the railing module's own bindings, so we patch them there. + + Uses ``mock.patch.object`` with a direct module reference rather than + the dotted-string form: ``mock.patch("bonsai.bim.module.model.railing.bonsai")`` + needs ``pkgutil.resolve_name`` to traverse ``bonsai → bim → module → …``, + which fails at the ``bonsai.bim`` step until that subpackage has been + imported elsewhere. The direct-object form sidesteps the resolution. + + Returns a dict for tests to seed return values and assert call sites. + """ + from bonsai.bim.module.model import railing + + with ( + mock.patch.object(railing, "tool") as mock_tool, + mock.patch.object(railing, "ifcopenshell") as mock_ifc, + mock.patch.object(railing, "bonsai") as mock_bonsai, + ): + # _resolve will be overridden on the test subclass below so the + # parametric_lifecycle.tool patch isn't needed. + mock_tool.Ifc.get_entity.return_value = mock.Mock(name="entity") + yield {"tool": mock_tool, "ifcopenshell": mock_ifc, "bonsai": mock_bonsai} + + +def _railing_test_subclass(props): + """Build a ``_RailingEditMixin`` subclass that bypasses ``_resolve``. + + The base ``_resolve`` reads ``tool.Ifc.get_entity`` from + ``parametric_lifecycle.tool`` (a separate import from the railing + module's ``tool``). Overriding it here keeps the test patches local + to the railing module and the hook closures local to the test.""" + from bonsai.bim.module.model.railing import _RailingEditMixin + + test_element = mock.Mock(name="ifc_element") + + class _TestRailingMixin(_RailingEditMixin): + pset_updates: mock.MagicMock = mock.MagicMock(name="_update_pset") + ifc_data_updates: mock.MagicMock = mock.MagicMock(name="_update_modifier_ifc_data") + bmesh_updates: mock.MagicMock = mock.MagicMock(name="_update_modifier_bmesh") + + @classmethod + def _resolve(cls, obj): + return test_element, props + + @classmethod + def _update_pset(cls, element, data): + cls.pset_updates(element, data) + + @classmethod + def _update_modifier_ifc_data(cls, obj, context): + cls.ifc_data_updates(obj, context) + + @classmethod + def _update_modifier_bmesh(cls, obj, context): + cls.bmesh_updates(obj, context) + + # The base _post_load_data JSON-serialises path_data; bypass that + # here so the round-trip stays a plain dict and tests can compare + # by reference / equality without re-parsing. + @classmethod + def _post_load_data(cls, data): + return dict(data) + + return _TestRailingMixin, test_element + + +# --------------------------------------------------------------------------- +# _RailingEditMixin._finish_one +# --------------------------------------------------------------------------- + + +def test_finish_one_short_circuits_when_draft_matches_stored(patched_railing): + """Enable → Finish without any property edit must NOT write to IFC. + + Without this, every "open Edit, click Validate immediately" cycle + would create a fresh ``IfcShapeRepresentation``, pollute the file's + representation list, and burn an undo entry — the user-visible + regression that motivated the short-circuit. + """ + stored = {"railing_type": "WALL_MOUNTED_HANDRAIL", "height": 1.0} + props = _FakeRailingProps(general=dict(stored)) + obj = _make_obj(props) + patched_railing["tool"].Model.get_modeling_bbim_pset_data.return_value = { + "data_dict": {**stored, "path_data": {"verts": [], "edges": []}}, + } + + cls, _element = _railing_test_subclass(props) + cls._finish_one(obj, mock.Mock(name="context")) + + assert props.is_editing is False, "is_editing must still flip even on no-op" + cls.pset_updates.assert_not_called() + cls.ifc_data_updates.assert_not_called() + + +def test_finish_one_writes_when_draft_differs(patched_railing): + """The complement of the short-circuit: a real property change must + flow through to ``_update_pset`` + ``_update_modifier_ifc_data``.""" + stored = {"railing_type": "WALL_MOUNTED_HANDRAIL", "height": 1.0} + # Draft height differs: simulating a user edit. + props = _FakeRailingProps(general={"railing_type": "WALL_MOUNTED_HANDRAIL", "height": 1.5}) + obj = _make_obj(props) + patched_railing["tool"].Model.get_modeling_bbim_pset_data.return_value = { + "data_dict": {**stored, "path_data": {"verts": [], "edges": []}}, + } + + cls, element = _railing_test_subclass(props) + cls._finish_one(obj, mock.Mock(name="context")) + + assert props.is_editing is False + cls.pset_updates.assert_called_once() + # The pset must receive the DRAFT data, not the stored data — that's the + # whole point of Finish committing the user's edits. + written = cls.pset_updates.call_args[0][1] + assert written["height"] == 1.5 + cls.ifc_data_updates.assert_called_once_with(obj, mock.ANY) + + +# --------------------------------------------------------------------------- +# _RailingEditMixin._cancel_one +# --------------------------------------------------------------------------- + + +def test_cancel_one_short_circuits_when_draft_matches_stored(patched_railing): + """Cancel-without-changes is asymmetrically expensive without this guard: + ``switch_representation`` re-tessellates the IfcSweptDiskSolid and is + visibly slow on a long handrail. When nothing changed, the mesh on + screen is still the committed IFC representation (the preview only + builds on a property change) — skip the reload entirely. + """ + stored = {"railing_type": "WALL_MOUNTED_HANDRAIL", "height": 1.0} + props = _FakeRailingProps(general=dict(stored)) + obj = _make_obj(props) + patched_railing["tool"].Model.get_modeling_bbim_pset_data.return_value = { + "data_dict": {**stored, "path_data": {"verts": [], "edges": []}}, + } + + cls, _element = _railing_test_subclass(props) + cls._cancel_one(obj, mock.Mock(name="context")) + + assert props.is_editing is False + patched_railing["bonsai"].core.geometry.switch_representation.assert_not_called() + cls.bmesh_updates.assert_not_called() + + +def test_cancel_one_wall_mounted_handrail_switches_representation(patched_railing): + """Cancel after a real edit on a WALL_MOUNTED_HANDRAIL must reload the + committed Body representation (high-poly, IFC-derived) rather than + re-running the low-poly bmesh preview — that preview is a viewport-only + approximation and would persist visibly after Cancel without this. + """ + stored = {"railing_type": "WALL_MOUNTED_HANDRAIL", "height": 1.0} + # Differs → not a no-op → cancel must take the real branch. + props = _FakeRailingProps( + railing_type="WALL_MOUNTED_HANDRAIL", + general={"railing_type": "WALL_MOUNTED_HANDRAIL", "height": 1.5}, + ) + obj = _make_obj(props) + patched_railing["tool"].Model.get_modeling_bbim_pset_data.return_value = { + "data_dict": {**stored, "path_data": {"verts": [], "edges": []}}, + } + body_repr = mock.Mock(name="body_representation") + patched_railing["ifcopenshell"].util.representation.get_representation.return_value = body_repr + + cls, _element = _railing_test_subclass(props) + cls._cancel_one(obj, mock.Mock(name="context")) + + assert props.is_editing is False + # Must call switch_representation with the Body representation; must NOT + # call _update_modifier_bmesh (that's the FRAMELESS branch). + patched_railing["bonsai"].core.geometry.switch_representation.assert_called_once() + kwargs = patched_railing["bonsai"].core.geometry.switch_representation.call_args.kwargs + assert kwargs["obj"] is obj + assert kwargs["representation"] is body_repr + cls.bmesh_updates.assert_not_called() + + +def test_cancel_one_frameless_panel_runs_bmesh_preview(patched_railing): + """FRAMELESS_PANEL's bmesh IS the canonical mesh — there's no IFC + swept-disk solid to reload. Cancel must run the bmesh rebuild instead + of switch_representation, which would no-op or worse.""" + stored = {"railing_type": "FRAMELESS_PANEL", "height": 1.0, "thickness": 0.05} + props = _FakeRailingProps( + railing_type="FRAMELESS_PANEL", + general={"railing_type": "FRAMELESS_PANEL", "height": 1.0, "thickness": 0.08}, + ) + obj = _make_obj(props) + patched_railing["tool"].Model.get_modeling_bbim_pset_data.return_value = { + "data_dict": {**stored, "path_data": {"verts": [], "edges": []}}, + } + + cls, _element = _railing_test_subclass(props) + cls._cancel_one(obj, mock.Mock(name="context")) + + assert props.is_editing is False + cls.bmesh_updates.assert_called_once_with(obj, mock.ANY) + patched_railing["bonsai"].core.geometry.switch_representation.assert_not_called() + + +# --------------------------------------------------------------------------- +# _get_railing_path_anchor: tests removed. +# +# The schematic-redesign branch replaced ``GizmoRailingEdition`` with +# ``GizmoRailingSchematic``, which anchors via the schematic frame rather +# than the polyline's first vertex. ``_get_railing_path_anchor`` was the +# helper for the old anchor strategy and has been deleted along with the +# old gizmo group. If schematic-mode gains a similar path-derived helper, +# new tests should land here. +# --------------------------------------------------------------------------- diff --git a/src/bonsai/test/bim/module/model/test_railing_schematic.py b/src/bonsai/test/bim/module/model/test_railing_schematic.py new file mode 100644 index 0000000000..270df8db78 --- /dev/null +++ b/src/bonsai/test/bim/module/model/test_railing_schematic.py @@ -0,0 +1,272 @@ +# 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. + +import types +from types import SimpleNamespace + +import bmesh +import bpy +import pytest + +from bonsai import tool +from bonsai.bim.module.drawing.gizmos import ( + BaseSchematicGizmoGroup, + DimensionGizmoConfig, +) +from bonsai.bim.module.model.railing import GizmoRailingSchematic + +pytestmark = pytest.mark.railing + + +@pytest.fixture(autouse=True) +def _require_real_bpy(): + """Skip the file when ``bpy`` is mocked or absent. + + Without this guard, mis-routed test runs (e.g. ``pytest test/bim/...`` + invoked outside Blender) crash at module-collection time on the chain of + ``bonsai.tool`` imports below, instead of producing a clean ``skipped``. + """ + if not isinstance(bpy, types.ModuleType) or hasattr(bpy, "_mock_name"): + pytest.skip("requires real Blender (bpy is mocked or absent)") + + +# ── Class shape ────────────────────────────────────────────────────────────── + + +def test_railing_schematic_inherits_base(): + """GizmoRailingSchematic plugs into the schematic framework, not the + in-place dimension framework. If a future refactor breaks this lineage + the schematic-specific machinery (sliders, draw handler) silently goes + dormant.""" + assert issubclass(GizmoRailingSchematic, BaseSchematicGizmoGroup) + + +def test_railing_schematic_bl_idname_preserved(): + """``OBJECT_GGT_bim_railing_edition`` is the user-facing identifier and + is referenced by keymaps and persistence. Preserve it across the class + rename — see the migration note in the class docstring.""" + assert GizmoRailingSchematic.bl_idname == "OBJECT_GGT_bim_railing_edition" + + +def test_railing_schematic_props_getter_pairing(): + """``gizmo_pref_name = "railing"`` and ``props_getter = tool.Model.get_railing_props`` + are the pairing test_parametric_registry depends on. If either drifts, + the addon-preferences gizmo toggle silently stops controlling this group.""" + assert GizmoRailingSchematic.gizmo_pref_name == "railing" + assert GizmoRailingSchematic.props_getter == tool.Model.get_railing_props + + +def test_railing_schematic_disables_in_place_dimension_props(): + """The schematic owns the value-input surface — no in-place dimensions on the actual geometry.""" + assert GizmoRailingSchematic.dimension_gizmo_props == [] + + +# ── Dimension configuration ───────────────────────────────────────────────── + + +def test_railing_schematic_has_six_dimensions(): + """One dimension per parametric property — three for each railing_type.""" + assert len(GizmoRailingSchematic.schematic_dimension_props) == 6 + + +def test_railing_schematic_dimension_attr_names_complete(): + """The six bound attributes match the parametric properties that + ``update_railing_modifier_bmesh`` reads when regenerating the live preview.""" + attr_names = {c.attr_name for c in GizmoRailingSchematic.schematic_dimension_props} + assert attr_names == { + "height", + "thickness", + "spacing", + "railing_diameter", + "clear_width", + "support_spacing", + } + + +def test_railing_schematic_dimensions_are_dimension_configs(): + """The dimension-line aesthetic depends on ``DimensionGizmoConfig`` (with + arrows + label), not the abstract slider widget.""" + for config in GizmoRailingSchematic.schematic_dimension_props: + assert isinstance(config, DimensionGizmoConfig) + + +def test_railing_schematic_dimensions_have_text_formatters(): + """Each dimension must format the label from the actual property value, + not from the visually-scaled value the gizmo's getter returns. Without a + formatter the label would show the schematic-scaled length, which is + meaningless to the user.""" + for config in GizmoRailingSchematic.schematic_dimension_props: + assert config.text_formatter is not None, f"{config.attr_name} missing text_formatter" + + +@pytest.mark.parametrize( + "attr_name,railing_type,expected", + [ + ("height", "FRAMELESS_PANEL", True), + ("height", "WALL_MOUNTED_HANDRAIL", False), + ("thickness", "FRAMELESS_PANEL", True), + ("spacing", "FRAMELESS_PANEL", True), + ("railing_diameter", "WALL_MOUNTED_HANDRAIL", True), + ("railing_diameter", "FRAMELESS_PANEL", False), + ("clear_width", "WALL_MOUNTED_HANDRAIL", True), + ], +) +def test_railing_schematic_dimension_visibility_gated_by_railing_type(attr_name, railing_type, expected): + """The two railing types are mutually exclusive — height/thickness/spacing + belong to FRAMELESS_PANEL; railing_diameter/clear_width/support_spacing + belong to WALL_MOUNTED_HANDRAIL. The visibility lambdas enforce that.""" + config = next(c for c in GizmoRailingSchematic.schematic_dimension_props if c.attr_name == attr_name) + props = SimpleNamespace(railing_type=railing_type, use_manual_supports=False) + assert config.visibility_condition(props) is expected + + +def test_railing_schematic_support_spacing_hidden_for_manual_supports(): + """``support_spacing`` only drives auto-positioned supports — when the + user has switched to manual supports the dimension should disappear.""" + config = next(c for c in GizmoRailingSchematic.schematic_dimension_props if c.attr_name == "support_spacing") + auto = SimpleNamespace(railing_type="WALL_MOUNTED_HANDRAIL", use_manual_supports=False) + manual = SimpleNamespace(railing_type="WALL_MOUNTED_HANDRAIL", use_manual_supports=True) + assert config.visibility_condition(auto) is True + assert config.visibility_condition(manual) is False + + +# ── Fixed-length tag rendering ───────────────────────────────────────────── + + +def test_schematic_dim_visible_length_is_constant(): + """Every schematic dimension tag renders at the same width — the bar is a + UI affordance, not a proportional measurement. The constant ratio keeps + tiny (5 mm thickness) and huge (5 m height) values equally clickable; the + real value lives in the dimension label. + + Regression guard: if value-proportional scaling is reintroduced, this + contract breaks silently — small dimensions start collapsing into stacked + arrows again. + """ + cls = GizmoRailingSchematic + ratio = cls.SCHEMATIC_DIM_VISIBLE_LENGTH_RATIO + assert ratio > 0 + assert ratio <= 1.0 # bar must fit within the schematic box + + +def test_schematic_no_compute_schematic_scale_override(): + """The constant-length design has no need for a scale factor. If a + subclass redefines ``_compute_schematic_scale``, it indicates the + scale-based proportional sizing was reintroduced — which is the design + we deliberately stepped away from.""" + assert "_compute_schematic_scale" not in GizmoRailingSchematic.__dict__ + + +# ── Path-edit guard ───────────────────────────────────────────────────────── + + +def test_update_editing_gizmos_override_defined_on_subclass(): + """``GizmoRailingSchematic`` must own the override that hides the pen + icon during path-edit. The parent's version shows the pen whenever + ``is_editing`` is False, which includes path-edit; that would let the + user open two editing modes at once.""" + assert "update_editing_gizmos" in GizmoRailingSchematic.__dict__ + + +# ── Schematic mesh building ───────────────────────────────────────────────── + + +def test_build_schematic_mesh_frameless_panel_returns_bmesh_with_edges(): + """FRAMELESS_PANEL renders as two separated wireframe boxes — 8 corners + per box × 2 = 16 verts; 12 edges per box × 2 = 24 edges. The visible + gap between the two boxes is the "spacing" semantic made literal. + + The mesh proportions are fixed (independent of property values) so the + dimension gizmos can anchor to known feature positions; the property + values are shown through dimension labels, not the mesh size.""" + props = SimpleNamespace( + railing_type="FRAMELESS_PANEL", + height=1.0, + thickness=0.05, + spacing=0.5, + ) + bm = GizmoRailingSchematic.build_schematic_mesh(props) + try: + assert isinstance(bm, bmesh.types.BMesh) + assert len(bm.verts) == 16 + assert len(bm.edges) == 24 + finally: + bm.free() + + +def test_build_schematic_mesh_wall_mounted_handrail_returns_bmesh_with_edges(): + """WALL_MOUNTED_HANDRAIL renders as three visual elements: + + - **Wall outline** — 4 corner verts, 4 edges (rectangle at z=0). + - **Hex tube** — 12 verts (6 per ring × 2 ends), 18 edges + (6 left ring + 6 right ring + 6 axial). + - **L-brackets** at each rail end — 3 verts per bracket (rail centre, + corner, wall attach) × 2 brackets = 6 verts; 2 edges per bracket + (rail→corner, corner→wall) × 2 = 4 edges. + + Total: 22 verts, 26 edges. + """ + props = SimpleNamespace( + railing_type="WALL_MOUNTED_HANDRAIL", + railing_diameter=0.05, + clear_width=0.04, + support_spacing=1.0, + ) + bm = GizmoRailingSchematic.build_schematic_mesh(props) + try: + assert isinstance(bm, bmesh.types.BMesh) + assert len(bm.verts) == 22 + assert len(bm.edges) == 26 + finally: + bm.free() + + +def test_build_schematic_mesh_proportions_independent_of_props(): + """The mesh uses fixed proportions so dimension gizmo anchor points stay + aligned with the geometry — extreme prop ratios don't change the mesh.""" + small = SimpleNamespace(railing_type="FRAMELESS_PANEL", height=0.01, thickness=0.005, spacing=0.05) + large = SimpleNamespace(railing_type="FRAMELESS_PANEL", height=10.0, thickness=0.5, spacing=2.0) + bm_small = GizmoRailingSchematic.build_schematic_mesh(small) + bm_large = GizmoRailingSchematic.build_schematic_mesh(large) + try: + # Same vert count regardless of prop magnitude. + assert len(bm_small.verts) == len(bm_large.verts) + # Same bounding box in each axis (within floating-point noise). + for axis in range(3): + small_coords = [v.co[axis] for v in bm_small.verts] + large_coords = [v.co[axis] for v in bm_large.verts] + assert min(small_coords) == pytest.approx(min(large_coords)) + assert max(small_coords) == pytest.approx(max(large_coords)) + finally: + bm_small.free() + bm_large.free() + + +def test_build_schematic_mesh_panel_top_matches_height_frac(): + """The panel's top edge sits at exactly ``SCHEMATIC_MESH_HEIGHT_FRAC``, + which is also where the ``thickness`` dimension anchors above the box. + If this drifts, the dimension labels float disconnected from the mesh.""" + props = SimpleNamespace(railing_type="FRAMELESS_PANEL", height=1.0, thickness=0.05, spacing=0.3) + bm = GizmoRailingSchematic.build_schematic_mesh(props) + try: + max_y = max(v.co.y for v in bm.verts) + assert max_y == pytest.approx(GizmoRailingSchematic.SCHEMATIC_MESH_HEIGHT_FRAC) + finally: + bm.free() From 05c9df74f9e1015e57ee5a3e51cbbb535d6f48de Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 11 Jun 2026 20:49:27 +0200 Subject: [PATCH 2/4] Migrate railing terminal type to PickType menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switches the IfcRailingType terminal-type selector from cycle-on-click to a popup menu of all terminal-type literals — 5+ values trip the §2.8 menu-pick threshold. Updates classes registration; removes EditRailingTerminalType in favour of PickRailingTerminalType which inherits PickTypeMixin. Adapts the cherry-pick from db016d881 to post-PR5 framework state: - Imports CycleTypeMixin / PickTypeMixin / PathPreservingEditMixin from bim.parametric_lifecycle (PR5 moved them off gizmos.py). - Routes is_railing through tool.Parametric (predicates moved off tool.Blender.Modifier between PR3-PR5). Skips the parametric_lifecycle.py framework refactor the source commit shipped — HEAD has the more-evolved post-PR5 framework that already covers it. Adds the _FakePropsBase + make_lifecycle_obj test helpers to test/bim/conftest.py so the new test_railing_lifecycle.py can exercise the edit triad without a real bpy.types.Object. Brings the test_railing_schematic.py marker in line with the rest of the model lane. Generated with the assistance of an AI coding tool. --- .../bonsai/bim/module/model/__init__.py | 2 +- src/bonsai/bonsai/bim/module/model/railing.py | 153 ++++++------------ src/bonsai/test/bim/conftest.py | 41 +++++ .../module/model/test_railing_lifecycle.py | 144 ++++++++++------- .../module/model/test_railing_schematic.py | 8 +- 5 files changed, 172 insertions(+), 176 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/model/__init__.py b/src/bonsai/bonsai/bim/module/model/__init__.py index edac430309..08806ad8fb 100644 --- a/src/bonsai/bonsai/bim/module/model/__init__.py +++ b/src/bonsai/bonsai/bim/module/model/__init__.py @@ -247,8 +247,8 @@ classes = ( railing.AddRailing, railing.CancelEditingRailing, railing.CycleRailingType, - railing.EditRailingTerminalType, railing.FinishEditingRailing, + railing.PickRailingTerminalType, railing.FlipRailingPathOrder, railing.EnableEditingRailing, railing.GizmoRailingSchematic, diff --git a/src/bonsai/bonsai/bim/module/model/railing.py b/src/bonsai/bonsai/bim/module/model/railing.py index 072b7d562d..825cc339ac 100644 --- a/src/bonsai/bonsai/bim/module/model/railing.py +++ b/src/bonsai/bonsai/bim/module/model/railing.py @@ -19,7 +19,7 @@ import json import math -from typing import Any, get_args +from typing import Any import bmesh import bpy @@ -38,7 +38,11 @@ from bonsai.bim.module.drawing.gizmos import DimensionGizmoConfig from bonsai.bim.module.model import prop from bonsai.bim.module.model.data import RailingData, refresh from bonsai.bim.module.model.decorator import ProfileDecorator -from bonsai.bim.parametric_lifecycle import PathPreservingEditMixin +from bonsai.bim.parametric_lifecycle import ( + CycleTypeMixin, + PathPreservingEditMixin, + PickTypeMixin, +) from bonsai.tool.cad import WELD_TOLERANCE V_ = tool.Blender.V_ @@ -497,53 +501,9 @@ class _RailingEditMixin(PathPreservingEditMixin): update_railing_modifier_ifc_data(context) @classmethod - def _update_modifier_bmesh(cls, obj: bpy.types.Object, context: bpy.types.Context) -> None: - update_railing_modifier_bmesh(context) - - @classmethod - def _finish_one(cls, obj: bpy.types.Object, context: bpy.types.Context) -> None: - """Skip the IFC commit when the draft matches the stored pset (no-op edit).""" - resolved = cls._resolve(obj) - if resolved is None: - return - element, props = resolved - pset_data = tool.Model.get_modeling_bbim_pset_data(obj, cls.pset_name) - stored = pset_data["data_dict"] - path_data = stored["path_data"] - draft = props.get_general_kwargs(convert_to_project_units=True) - draft["path_data"] = path_data - - if draft == stored: - props.is_editing = False - return - - cls._update_pset(element, draft) - cls._update_modifier_ifc_data(obj, context) - props.is_editing = False - - @classmethod - def _cancel_one(cls, obj: bpy.types.Object, context: bpy.types.Context) -> None: - """WALL_MOUNTED_HANDRAIL switches the representation back to Body on cancel - (the cylinder preview is lower-poly than the committed swept-disk solid). - Skip the switch when the draft matches the stored pset.""" - resolved = cls._resolve(obj) - if resolved is None: - return - _element, props = resolved - 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) - - if nothing_changed: - props.is_editing = False - return - + def _restore_viewport_after_cancel(cls, obj: bpy.types.Object, context: bpy.types.Context) -> None: + """WALL_MOUNTED_HANDRAIL reloads the committed Body; others rebuild the preview bmesh.""" + props = tool.Model.get_railing_props(obj) if props.railing_type == "WALL_MOUNTED_HANDRAIL": element = tool.Ifc.get_entity(obj) assert element @@ -555,10 +515,8 @@ class _RailingEditMixin(PathPreservingEditMixin): obj=obj, representation=body, ) - else: - cls._update_modifier_bmesh(obj, context) - - props.is_editing = False + return + update_railing_modifier_bmesh(context) class EnableEditingRailing(_RailingEditMixin, bpy.types.Operator, tool.Ifc.Operator): @@ -588,14 +546,14 @@ class FinishEditingRailing(_RailingEditMixin, bpy.types.Operator, tool.Ifc.Opera return self._finish_targets(context) -class CycleRailingType(bpy.types.Operator, tool.Ifc.Operator, gizmo.CycleTypeMixin): +class CycleRailingType(bpy.types.Operator, tool.Ifc.Operator, CycleTypeMixin): """Cycle railing_type (FRAMELESS_PANEL ↔ WALL_MOUNTED_HANDRAIL). Shift+click reverses.""" bl_idname = "bim.cycle_railing_type" bl_label = "Cycle Railing Type" bl_options = {"REGISTER", "UNDO"} - element_checker = tool.Blender.Modifier.is_railing + element_checker = tool.Parametric.is_railing props_getter = tool.Model.get_railing_props type_literal = tool.Model.RailingType type_attr = "railing_type" @@ -616,58 +574,42 @@ class ToggleRailingUseManualSupports(bpy.types.Operator): bl_options = {"REGISTER", "UNDO"} def execute(self, context): - obj = context.active_object - if not obj: - return {"CANCELLED"} - props = tool.Model.get_railing_props(obj) - if not props.is_editing or props.railing_type != "WALL_MOUNTED_HANDRAIL": + resolved = tool.Model.resolve_active_props_for_edit( + context, + tool.Model.get_railing_props, + subtype=("railing_type", "WALL_MOUNTED_HANDRAIL"), + ) + if resolved is None: return {"CANCELLED"} + _obj, props = resolved props.use_manual_supports = not props.use_manual_supports return {"FINISHED"} -class EditRailingTerminalType(bpy.types.Operator): - """Popup menu for terminal_type; writes the picked value via a HIDDEN string property.""" +class PickRailingTerminalType(bpy.types.Operator, tool.Ifc.Operator, PickTypeMixin): + """Pick ``terminal_type`` for the active WALL_MOUNTED_HANDRAIL railing.""" - bl_idname = "bim.edit_railing_terminal_type" - bl_label = "Choose Railing Terminal Type" + bl_idname = "bim.pick_railing_terminal_type" + bl_label = "Pick Railing Terminal Type" bl_description = "Pick the cap geometry applied at the rail ends" bl_options = {"REGISTER", "UNDO"} - terminal_type: bpy.props.StringProperty(name="Terminal Type", default="", options={"HIDDEN", "SKIP_SAVE"}) + skip_element_check = True + props_getter = tool.Model.get_railing_props + type_literal = prop.CapType + type_attr = "terminal_type" - def invoke(self, context: bpy.types.Context, event: bpy.types.Event) -> set[str]: - obj = context.active_object - if not obj: + def _execute(self, context: bpy.types.Context) -> set[str]: + if ( + tool.Model.resolve_active_props_for_edit( + context, + tool.Model.get_railing_props, + subtype=("railing_type", "WALL_MOUNTED_HANDRAIL"), + ) + is None + ): return {"CANCELLED"} - props = tool.Model.get_railing_props(obj) - if not props.is_editing or props.railing_type != "WALL_MOUNTED_HANDRAIL": - return {"CANCELLED"} - - choices = [v for v in get_args(prop.CapType)] - - def draw(menu_self, _menu_context): - layout = menu_self.layout - for v in choices: - op = layout.operator(self.bl_idname, text=v) - op.terminal_type = v - - context.window_manager.popup_menu(draw, title="Terminal Type", icon="MOD_LATTICE") - return {"FINISHED"} - - def execute(self, context: bpy.types.Context) -> set[str]: - # Re-open the popup if called without a value (e.g. from the command palette). - if not self.terminal_type: - return self.invoke(context, None) # type: ignore[arg-type] - obj = context.active_object - if not obj: - return {"CANCELLED"} - props = tool.Model.get_railing_props(obj) - if self.terminal_type not in get_args(prop.CapType): - self.report({"ERROR"}, f"Unknown terminal_type: {self.terminal_type!r}") - return {"CANCELLED"} - props.terminal_type = self.terminal_type # type: ignore[assignment] - return {"FINISHED"} + return self._pick_type(context) def _format_attr_distance(attr_name: str): @@ -822,7 +764,7 @@ class GizmoRailingSchematic(bpy.types.GizmoGroup, gizmo.BaseSchematicGizmoGroup) @classmethod def is_element_type(cls, element: ifcopenshell.entity_instance) -> bool: - return tool.Blender.Modifier.is_railing(element) + return tool.Parametric.is_railing(element) @classmethod def schematic_cache_key(cls, props) -> tuple: @@ -846,22 +788,17 @@ class GizmoRailingSchematic(bpy.types.GizmoGroup, gizmo.BaseSchematicGizmoGroup) """ default_color, highlight_color = self.get_decoration_colors() - for slot in ("lock_open_gizmo", "lock_closed_gizmo"): - bl_idname = "VIEW3D_GT_lock_open" if slot == "lock_open_gizmo" else "VIEW3D_GT_lock_closed" - gz = self.gizmos.new(bl_idname) - gz.color = default_color - gz.color_highlight = highlight_color - gz.use_draw_scale = False - gz.alpha = 0.8 - gz.target_set_operator("bim.toggle_railing_use_manual_supports") - setattr(self, slot, gz) + self.lock_open_gizmo, self.lock_closed_gizmo = self.create_icon_gizmo_lock_pair( + "bim.toggle_railing_use_manual_supports", + open_color=default_color, + ) - self.terminal_gizmo = self.gizmos.new("VIEW3D_GT_cycle") + self.terminal_gizmo = self.gizmos.new("VIEW3D_GT_menu") self.terminal_gizmo.color = default_color self.terminal_gizmo.color_highlight = highlight_color self.terminal_gizmo.use_draw_scale = False self.terminal_gizmo.alpha = 0.8 - self.terminal_gizmo.target_set_operator("bim.edit_railing_terminal_type") + self.terminal_gizmo.target_set_operator("bim.pick_railing_terminal_type") def _refresh_element_specific(self, context: bpy.types.Context, mw: "Matrix", props) -> None: """Position and gate the WALL_MOUNTED_HANDRAIL-only gizmos. diff --git a/src/bonsai/test/bim/conftest.py b/src/bonsai/test/bim/conftest.py index 2d69fe415a..a957b479ba 100644 --- a/src/bonsai/test/bim/conftest.py +++ b/src/bonsai/test/bim/conftest.py @@ -1,5 +1,46 @@ import pytest + +class _FakePropsBase: + """Base for parametric-edit PropertyGroup stand-ins used in lifecycle tests. + + The parametric-edit lifecycle mixins read/write a common contract: + ``is_editing`` (bool), ``last_kwargs`` (dict | None — capture of the last + data written via ``set_props_kwargs_from_ifc_data``), + ``set_props_kwargs_from_ifc_data(data)``, and + ``get_general_kwargs(convert_to_project_units=True)``. Per-type stand-ins + (door, railing, roof) subclass this and add their own kwargs accessors + and per-type fields.""" + + def __init__(self, general: dict | None = None): + self.is_editing = False + self.last_kwargs: dict | None = None + self.general = dict(general) if general is not None else {} + + def set_props_kwargs_from_ifc_data(self, data): + self.last_kwargs = dict(data) + + def get_general_kwargs(self, convert_to_project_units=True): + return dict(self.general) + + +def make_lifecycle_obj(props, *, name="obj"): + """Build a ``bpy.types.Object`` stand-in for parametric-lifecycle tests. + + The mixin code under test reads ``obj.props`` (the PropertyGroup + stand-in) and ``obj.name`` (used in error reports). ``spec=bpy.types.Object`` + catches typo'd attribute access at test time. ``bpy`` is imported inside + the function so this conftest stays importable when bpy is absent.""" + from unittest import mock + + import bpy + + obj = mock.Mock(spec=bpy.types.Object, name=name) + obj.props = props + obj.name = name + return obj + + # pytest by default doesn't print steps and where it failed. Let's fix that. diff --git a/src/bonsai/test/bim/module/model/test_railing_lifecycle.py b/src/bonsai/test/bim/module/model/test_railing_lifecycle.py index bcc7005bf9..0aad3534b7 100644 --- a/src/bonsai/test/bim/module/model/test_railing_lifecycle.py +++ b/src/bonsai/test/bim/module/model/test_railing_lifecycle.py @@ -18,23 +18,24 @@ # # This file was generated with the assistance of an AI coding tool. -"""Unit coverage for the ``_RailingEditMixin`` lifecycle overrides. +"""Unit coverage for the ``_RailingEditMixin`` overrides and the lifecycle +behaviour railing inherits from ``PathPreservingEditMixin``. -The generic ``PathPreservingEditMixin`` lifecycle is tested in -``test_parametric_lifecycle.py``. This file pins the **railing-specific -overrides** that subclass it: +The parent short-circuit (skip the IFC commit / viewport rebuild when the +draft is identical to the stored pset) lives in +``PathPreservingEditMixin``; the tests below verify railing's subclass +honours that contract by inheritance, then pin the railing-specific +viewport-restore dispatch: -- ``_RailingEditMixin._finish_one`` short-circuit: when the draft equals - the stored pset, ``_update_pset`` and ``_update_modifier_ifc_data`` are - skipped so an Enable → Finish-without-changes cycle creates no new - ``IfcShapeRepresentation``. -- ``_RailingEditMixin._cancel_one`` short-circuit: same logic guards the - expensive ``bonsai.core.geometry.switch_representation`` call (which - re-tessellates the swept-disk solid) when nothing actually changed. -- ``_RailingEditMixin._cancel_one`` WALL_MOUNTED_HANDRAIL branch: when - changes WERE made, the cancel reloads the IFC body via - ``switch_representation`` instead of running ``update_modifier_bmesh`` - (which would leave the low-poly cylinder-segment preview on screen). +- Finish / Cancel no-op short-circuit: inherited from the parent — verified + here because railing was the original consumer that motivated the + optimisation. +- ``_RailingEditMixin._restore_viewport_after_cancel`` dispatch: WALL_MOUNTED_HANDRAIL + reloads the high-poly Body representation via ``switch_representation``; + FRAMELESS_PANEL rebuilds the bmesh preview via + ``update_railing_modifier_bmesh``. This is the per-type branch that used + to live in ``_cancel_one`` and now lives in the viewport-restore hook the + parent's ``_cancel_one`` calls. """ from unittest import mock @@ -68,10 +69,16 @@ class _FakeRailingProps(_FakePropsBase): def patched_railing(): """Patch the railing module's external references for unit testing. - ``_RailingEditMixin`` calls ``tool.Model.get_modeling_bbim_pset_data``, - ``tool.Ifc.get_entity``, ``ifcopenshell.util.representation.get_representation``, - and ``bonsai.core.geometry.switch_representation`` — each looked up - through the railing module's own bindings, so we patch them there. + ``_RailingEditMixin`` and the parent lifecycle reach for + ``tool.Model.get_modeling_bbim_pset_data``, ``tool.Ifc.get_entity``, + ``ifcopenshell.util.representation.get_representation``, + ``bonsai.core.geometry.switch_representation``, and the module-level + ``update_railing_modifier_bmesh`` — each looked up through the railing + module's own bindings, so we patch them there. + + ``parametric_lifecycle.tool`` is patched separately so the parent's + ``_resolve`` and ``_cancel_one`` can read ``tool.Model.get_modeling_bbim_pset_data`` + without falling through to the real Blender bindings. Uses ``mock.patch.object`` with a direct module reference rather than the dotted-string form: ``mock.patch("bonsai.bim.module.model.railing.bonsai")`` @@ -81,17 +88,28 @@ def patched_railing(): Returns a dict for tests to seed return values and assert call sites. """ + from bonsai.bim import parametric_lifecycle from bonsai.bim.module.model import railing with ( mock.patch.object(railing, "tool") as mock_tool, mock.patch.object(railing, "ifcopenshell") as mock_ifc, mock.patch.object(railing, "bonsai") as mock_bonsai, + mock.patch.object(railing, "update_railing_modifier_bmesh") as mock_update_bmesh, + mock.patch.object(parametric_lifecycle, "tool") as mock_pl_tool, ): # _resolve will be overridden on the test subclass below so the - # parametric_lifecycle.tool patch isn't needed. + # parametric_lifecycle.tool patch isn't needed for that path, but the + # parent's _cancel_one / _finish_one still call + # tool.Model.get_modeling_bbim_pset_data and would otherwise miss. mock_tool.Ifc.get_entity.return_value = mock.Mock(name="entity") - yield {"tool": mock_tool, "ifcopenshell": mock_ifc, "bonsai": mock_bonsai} + yield { + "tool": mock_tool, + "ifcopenshell": mock_ifc, + "bonsai": mock_bonsai, + "update_bmesh": mock_update_bmesh, + "pl_tool": mock_pl_tool, + } def _railing_test_subclass(props): @@ -108,7 +126,7 @@ def _railing_test_subclass(props): class _TestRailingMixin(_RailingEditMixin): pset_updates: mock.MagicMock = mock.MagicMock(name="_update_pset") ifc_data_updates: mock.MagicMock = mock.MagicMock(name="_update_modifier_ifc_data") - bmesh_updates: mock.MagicMock = mock.MagicMock(name="_update_modifier_bmesh") + bmesh_updates: mock.MagicMock = mock.MagicMock(name="_restore_viewport_after_cancel") @classmethod def _resolve(cls, obj): @@ -123,7 +141,7 @@ def _railing_test_subclass(props): cls.ifc_data_updates(obj, context) @classmethod - def _update_modifier_bmesh(cls, obj, context): + def _restore_viewport_after_cancel(cls, obj, context): cls.bmesh_updates(obj, context) # The base _post_load_data JSON-serialises path_data; bypass that @@ -148,11 +166,14 @@ def test_finish_one_short_circuits_when_draft_matches_stored(patched_railing): would create a fresh ``IfcShapeRepresentation``, pollute the file's representation list, and burn an undo entry — the user-visible regression that motivated the short-circuit. + + Behaviour now inherited from ``PathPreservingEditMixin``; railing keeps + the coverage as the original consumer of the contract. """ stored = {"railing_type": "WALL_MOUNTED_HANDRAIL", "height": 1.0} props = _FakeRailingProps(general=dict(stored)) obj = _make_obj(props) - patched_railing["tool"].Model.get_modeling_bbim_pset_data.return_value = { + patched_railing["pl_tool"].Model.get_modeling_bbim_pset_data.return_value = { "data_dict": {**stored, "path_data": {"verts": [], "edges": []}}, } @@ -171,7 +192,7 @@ def test_finish_one_writes_when_draft_differs(patched_railing): # Draft height differs: simulating a user edit. props = _FakeRailingProps(general={"railing_type": "WALL_MOUNTED_HANDRAIL", "height": 1.5}) obj = _make_obj(props) - patched_railing["tool"].Model.get_modeling_bbim_pset_data.return_value = { + patched_railing["pl_tool"].Model.get_modeling_bbim_pset_data.return_value = { "data_dict": {**stored, "path_data": {"verts": [], "edges": []}}, } @@ -198,11 +219,14 @@ def test_cancel_one_short_circuits_when_draft_matches_stored(patched_railing): visibly slow on a long handrail. When nothing changed, the mesh on screen is still the committed IFC representation (the preview only builds on a property change) — skip the reload entirely. + + Behaviour now inherited from ``PathPreservingEditMixin``; railing keeps + the coverage as the original consumer of the contract. """ stored = {"railing_type": "WALL_MOUNTED_HANDRAIL", "height": 1.0} props = _FakeRailingProps(general=dict(stored)) obj = _make_obj(props) - patched_railing["tool"].Model.get_modeling_bbim_pset_data.return_value = { + patched_railing["pl_tool"].Model.get_modeling_bbim_pset_data.return_value = { "data_dict": {**stored, "path_data": {"verts": [], "edges": []}}, } @@ -211,60 +235,56 @@ def test_cancel_one_short_circuits_when_draft_matches_stored(patched_railing): assert props.is_editing is False patched_railing["bonsai"].core.geometry.switch_representation.assert_not_called() + patched_railing["update_bmesh"].assert_not_called() cls.bmesh_updates.assert_not_called() -def test_cancel_one_wall_mounted_handrail_switches_representation(patched_railing): - """Cancel after a real edit on a WALL_MOUNTED_HANDRAIL must reload the - committed Body representation (high-poly, IFC-derived) rather than - re-running the low-poly bmesh preview — that preview is a viewport-only - approximation and would persist visibly after Cancel without this. - """ - stored = {"railing_type": "WALL_MOUNTED_HANDRAIL", "height": 1.0} - # Differs → not a no-op → cancel must take the real branch. - props = _FakeRailingProps( - railing_type="WALL_MOUNTED_HANDRAIL", - general={"railing_type": "WALL_MOUNTED_HANDRAIL", "height": 1.5}, - ) +# --------------------------------------------------------------------------- +# _RailingEditMixin._restore_viewport_after_cancel — per-type viewport-restore dispatch +# +# The parent's _cancel_one calls cls._restore_viewport_after_cancel whenever +# the draft differs from the stored pset. Railing's override branches on +# railing_type so WALL_MOUNTED_HANDRAIL reloads the high-poly Body +# representation rather than rebuilding the low-poly cylinder-segment preview. +# --------------------------------------------------------------------------- + + +def test_restore_viewport_wall_mounted_handrail_switches_representation(patched_railing): + """WALL_MOUNTED_HANDRAIL restore must call ``switch_representation`` with + the Body representation — the preview is viewport-only (low-poly cylinder) + and would persist visibly without the reload.""" + from bonsai.bim.module.model.railing import _RailingEditMixin + + props = _FakeRailingProps(railing_type="WALL_MOUNTED_HANDRAIL") obj = _make_obj(props) - patched_railing["tool"].Model.get_modeling_bbim_pset_data.return_value = { - "data_dict": {**stored, "path_data": {"verts": [], "edges": []}}, - } + patched_railing["tool"].Model.get_railing_props.return_value = props body_repr = mock.Mock(name="body_representation") patched_railing["ifcopenshell"].util.representation.get_representation.return_value = body_repr - cls, _element = _railing_test_subclass(props) - cls._cancel_one(obj, mock.Mock(name="context")) + _RailingEditMixin._restore_viewport_after_cancel(obj, mock.Mock(name="context")) - assert props.is_editing is False - # Must call switch_representation with the Body representation; must NOT - # call _update_modifier_bmesh (that's the FRAMELESS branch). patched_railing["bonsai"].core.geometry.switch_representation.assert_called_once() kwargs = patched_railing["bonsai"].core.geometry.switch_representation.call_args.kwargs assert kwargs["obj"] is obj assert kwargs["representation"] is body_repr - cls.bmesh_updates.assert_not_called() + # Must NOT fall through to the FRAMELESS bmesh-rebuild path. + patched_railing["update_bmesh"].assert_not_called() -def test_cancel_one_frameless_panel_runs_bmesh_preview(patched_railing): +def test_restore_viewport_frameless_panel_calls_module_bmesh_rebuild(patched_railing): """FRAMELESS_PANEL's bmesh IS the canonical mesh — there's no IFC - swept-disk solid to reload. Cancel must run the bmesh rebuild instead - of switch_representation, which would no-op or worse.""" - stored = {"railing_type": "FRAMELESS_PANEL", "height": 1.0, "thickness": 0.05} - props = _FakeRailingProps( - railing_type="FRAMELESS_PANEL", - general={"railing_type": "FRAMELESS_PANEL", "height": 1.0, "thickness": 0.08}, - ) + swept-disk solid to reload. The restore must delegate to the module-level + ``update_railing_modifier_bmesh`` rebuilder rather than swap representations.""" + from bonsai.bim.module.model.railing import _RailingEditMixin + + props = _FakeRailingProps(railing_type="FRAMELESS_PANEL") obj = _make_obj(props) - patched_railing["tool"].Model.get_modeling_bbim_pset_data.return_value = { - "data_dict": {**stored, "path_data": {"verts": [], "edges": []}}, - } + patched_railing["tool"].Model.get_railing_props.return_value = props + ctx = mock.Mock(name="context") - cls, _element = _railing_test_subclass(props) - cls._cancel_one(obj, mock.Mock(name="context")) + _RailingEditMixin._restore_viewport_after_cancel(obj, ctx) - assert props.is_editing is False - cls.bmesh_updates.assert_called_once_with(obj, mock.ANY) + patched_railing["update_bmesh"].assert_called_once_with(ctx) patched_railing["bonsai"].core.geometry.switch_representation.assert_not_called() diff --git a/src/bonsai/test/bim/module/model/test_railing_schematic.py b/src/bonsai/test/bim/module/model/test_railing_schematic.py index 270df8db78..7ba12a7a70 100644 --- a/src/bonsai/test/bim/module/model/test_railing_schematic.py +++ b/src/bonsai/test/bim/module/model/test_railing_schematic.py @@ -32,7 +32,7 @@ from bonsai.bim.module.drawing.gizmos import ( ) from bonsai.bim.module.model.railing import GizmoRailingSchematic -pytestmark = pytest.mark.railing +pytestmark = pytest.mark.model @pytest.fixture(autouse=True) @@ -167,10 +167,8 @@ def test_schematic_dim_visible_length_is_constant(): def test_schematic_no_compute_schematic_scale_override(): - """The constant-length design has no need for a scale factor. If a - subclass redefines ``_compute_schematic_scale``, it indicates the - scale-based proportional sizing was reintroduced — which is the design - we deliberately stepped away from.""" + """The constant-length schematic must not reintroduce scale-based + proportional sizing via a ``_compute_schematic_scale`` override.""" assert "_compute_schematic_scale" not in GizmoRailingSchematic.__dict__ From 92aa890add29edafac0cd2e91ca311536bbdd266 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Thu, 11 Jun 2026 20:49:41 +0200 Subject: [PATCH 3/4] Refresh railing preview on every gizmo edit update_railing skipped the bmesh rebuild for WALL_MOUNTED_HANDRAIL railings because the only mesh source available at the time mutated IFC. The viewport-only preview helper that lands with the parametric gizmo work (generate_wall_mounted_handrail_preview) sidesteps IFC entirely, so the WALL_MOUNTED_HANDRAIL branch can join the FRAMELESS_PANEL path and trigger update_railing_modifier_bmesh on every property write. Gizmo drag now repaints the viewport in real time instead of waiting for Finish Editing. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/prop.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/model/prop.py b/src/bonsai/bonsai/bim/module/model/prop.py index b9709c067a..ebafafda6d 100644 --- a/src/bonsai/bonsai/bim/module/model/prop.py +++ b/src/bonsai/bonsai/bim/module/model/prop.py @@ -228,11 +228,7 @@ def update_wall_offset_baseline(self: "BIMWallProperties", context: bpy.types.Co def update_railing(self: "BIMRailingProperties", context: bpy.types.Context) -> None: """Regenerate railing mesh when property changes.""" if self.is_editing: - # Only FRAMELESS_PANEL can update live via bmesh. - # WALL_MOUNTED_HANDRAIL geometry is generated from IFC representation, - # so it only updates on "Finish Editing" to avoid modifying IFC during preview. - if self.railing_type == "FRAMELESS_PANEL": - _get_updater("railing", "update_railing_modifier_bmesh")(context) + _get_updater("railing", "update_railing_modifier_bmesh")(context) def update_roof(self: "BIMRoofProperties", context: bpy.types.Context) -> None: From 2f067187eebe1bbe51ba82ec050bae3044adcc49 Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Mon, 15 Jun 2026 10:08:50 +0200 Subject: [PATCH 4/4] Disable snap on schematic gizmos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Schematic dimensions float in billboarded viewport space; their labels carry the value, not the bar length. Snapping the dragged tip to scene vertices produces nonsensical value jumps when the mouse crosses unrelated meshes. Add an opt-out flag on the parametric gizmo group base and override it on the schematic base — every schematic subclass inherits no-snap behaviour, and in-place parametric gizmos (door, window, wall, stair, roof, mep) keep the existing Ctrl-toggleable snap because the default stays True. GizmoDimension.invoke also forces tool_settings.use_snap = False for schematic gizmos so the header magnet visibly switches off for the drag's duration. The existing exit path restores the user's previous setting on release. Generated with the assistance of an AI coding tool. --- .../bonsai/bim/module/drawing/gizmos.py | 38 ++++++++++++++++--- .../test/bim/module/drawing/test_gizmos.py | 22 ++++++++++- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/drawing/gizmos.py b/src/bonsai/bonsai/bim/module/drawing/gizmos.py index 3c03e9db49..6028ac055d 100644 --- a/src/bonsai/bonsai/bim/module/drawing/gizmos.py +++ b/src/bonsai/bonsai/bim/module/drawing/gizmos.py @@ -4877,7 +4877,14 @@ class GizmoDimension(GizmoMovable): self.init_value = click_distance - if self.initial_snap_state and self.active_obj: + # Schematic gizmos opt out of dimension snap. Force the header + # indicator to ``off`` for the drag's duration so the user sees the + # state matches behaviour; ``exit`` restores ``initial_snap_state``. + # Skipping the snap cache here also avoids the per-drag mesh probe. + snap_supported = getattr(self.gizmo_group, "snap_enabled_on_dimensions", True) + if not snap_supported: + context.scene.tool_settings.use_snap = False + elif self.initial_snap_state and self.active_obj: build_snap_cache(context, self.active_obj) self._snap_cache_built = True @@ -4919,11 +4926,18 @@ class GizmoDimension(GizmoMovable): if not region or not rv3d: return {"RUNNING_MODAL"} - tool_settings.use_snap = not self.initial_snap_state if event.ctrl else self.initial_snap_state + # Group-level opt-out: schematic gizmos float in viewport space, so + # global-snap-to-scene-vertices would produce spurious value jumps. + # The fallback (``True``) covers any gizmo whose group is not a + # ``BaseParametricGizmoGroup``. + snap_supported = getattr(self.gizmo_group, "snap_enabled_on_dimensions", True) - if tool_settings.use_snap and not self._snap_cache_built and self.active_obj: - build_snap_cache(context, self.active_obj) - self._snap_cache_built = True + if snap_supported: + tool_settings.use_snap = not self.initial_snap_state if event.ctrl else self.initial_snap_state + + if tool_settings.use_snap and not self._snap_cache_built and self.active_obj: + build_snap_cache(context, self.active_obj) + self._snap_cache_built = True current_coord = (event.mouse_region_x, event.mouse_region_y) @@ -4947,7 +4961,7 @@ class GizmoDimension(GizmoMovable): delta = (current_3d - self.start_location).dot(axis_direction) - if tool_settings.use_snap and self.active_obj: + if snap_supported and tool_settings.use_snap and self.active_obj: # Snap the dimension tip (not mouse position) to target # Calculate where the dimension tip would be with current delta # The tip is at: gizmo_origin + axis * (init_value + delta) @@ -5320,6 +5334,13 @@ class BaseParametricGizmoGroup: # Pre-computed flip matrix for negative value handling (180° rotation around Z) FLIP_MATRIX = Matrix.Rotation(math.pi, 4, "Z") + # Default: dimension drags respect Blender's global snap (Ctrl-toggleable + # during drag). Subclasses whose dimensions float in viewport space rather + # than aligning to real-world geometry should override to ``False`` — + # snapping to scene vertices in that case produces spurious value jumps + # as the mouse crosses unrelated meshes. + snap_enabled_on_dimensions: bool = True + # === Icon Gizmo Layout (meters) === # Icons are positioned in a horizontal row above the element: # [Validate] [Cancel] [Cycle] @@ -6580,6 +6601,11 @@ class BaseSchematicGizmoGroup(BaseParametricGizmoGroup): # list and become no-ops. The schematic equivalents below take their place. dimension_gizmo_props: list[DimensionGizmoConfig] = [] + # Schematic dimensions float in billboarded viewport space, not aligned to + # real-world geometry. Snapping the dragged tip to scene vertices would + # produce nonsensical value jumps as the mouse crosses unrelated meshes. + snap_enabled_on_dimensions: bool = False + # Declarative dimension configuration consumed by ``setup_schematic_dimensions`` # and ``update_schematic_dimensions``. Each config produces one # ``BIM_GT_gizmo_dimension`` instance positioned at a schematic-local diff --git a/src/bonsai/test/bim/module/drawing/test_gizmos.py b/src/bonsai/test/bim/module/drawing/test_gizmos.py index cc781cd118..18f640549b 100644 --- a/src/bonsai/test/bim/module/drawing/test_gizmos.py +++ b/src/bonsai/test/bim/module/drawing/test_gizmos.py @@ -24,7 +24,11 @@ from types import SimpleNamespace import bpy import pytest -from bonsai.bim.module.drawing.gizmos import DimensionGizmoConfig +from bonsai.bim.module.drawing.gizmos import ( + BaseParametricGizmoGroup, + BaseSchematicGizmoGroup, + DimensionGizmoConfig, +) pytestmark = pytest.mark.drawing @@ -52,3 +56,19 @@ def test_text_formatter_receives_props_and_value(): config = DimensionGizmoConfig(attr_name="length", axis=(1, 0, 0), text_formatter=formatter) props = SimpleNamespace(label="L") assert config.text_formatter(props, 3.14) == "L=3.14" + + +def test_parametric_base_enables_dimension_snap_by_default(): + """In-place parametric gizmos align to real-world geometry, so dragging + must respect the global snap toggle (Ctrl-flip during drag) — same + contract every door / window / wall / stair / roof / mep dimension + has shipped with.""" + assert BaseParametricGizmoGroup.snap_enabled_on_dimensions is True + + +def test_schematic_base_disables_dimension_snap(): + """Schematic dimensions float in viewport space; snapping the dragged + tip to scene vertices would produce spurious value jumps as the + mouse crosses unrelated geometry. The opt-out lives on the base so + every schematic subclass inherits it without per-class wiring.""" + assert BaseSchematicGizmoGroup.snap_enabled_on_dimensions is False