From e0a1577ae0a5e74d057c4a9c1336b29038ee5fae Mon Sep 17 00:00:00 2001 From: DesertSpringsCivil Date: Tue, 10 Feb 2026 00:05:54 -0700 Subject: [PATCH] Fix in-place alignment editing and curve visualization - Add clear_layout_segments API to remove segments while preserving alignment ID - Modify exit_pi_edit_mode to edit segments in-place instead of delete+recreate - Fix curve visualization by using create_shape for segment vertices - Fix evaluate_segment validation to handle negative-length curve segments This prevents "Active alignment no longer exists" errors when editing PIs and properly renders circular arcs regardless of turn direction. Co-Authored-By: Claude Opus 4.5 --- .../bonsai/bim/module/alignment/operator.py | 51 ++-- src/bonsai/bonsai/core/alignment.py | 58 ++--- src/bonsai/bonsai/tool/alignment.py | 68 +++--- .../ifcopenshell/api/alignment/__init__.py | 2 + .../api/alignment/clear_layout_segments.py | 223 ++++++++++++++++++ .../ifcopenshell/api/alignment/util.py | 14 +- 6 files changed, 313 insertions(+), 103 deletions(-) create mode 100644 src/ifcopenshell-python/ifcopenshell/api/alignment/clear_layout_segments.py diff --git a/src/bonsai/bonsai/bim/module/alignment/operator.py b/src/bonsai/bonsai/bim/module/alignment/operator.py index ed5259efe8..0ff41ba597 100644 --- a/src/bonsai/bonsai/bim/module/alignment/operator.py +++ b/src/bonsai/bonsai/bim/module/alignment/operator.py @@ -981,15 +981,15 @@ class SAIKEI_OT_recalculate_pis(Operator): return True def execute(self, context): + import ifcopenshell.api.alignment as align_api + ifc = tool.Ifc.get() props = context.scene.SaikeiAlignmentProperties # Recalculate geometry in UI properties recalculate_pi_geometry(props) - # If there's an active alignment, recreate it with updated data - # We recreate the entire alignment because modifying segments in place - # can leave the IFC layout in an inconsistent state + # If there's an active alignment, update it in-place if props.active_alignment_id != 0: alignment = get_alignment_by_id(ifc, props.active_alignment_id) if alignment is None: @@ -998,37 +998,34 @@ class SAIKEI_OT_recalculate_pis(Operator): self.report({"WARNING"}, "Active alignment no longer exists. Reference cleared.") return {"FINISHED"} if alignment: - # Save the alignment name - alignment_name = alignment.Name or props.new_alignment_name - - # Remove the entire alignment hierarchy (Blender objects) - tool.Alignment.remove_alignment_hierarchy(alignment) - - # Remove the IFC alignment entity entirely - ifcopenshell.api.run("root.remove_product", ifc, product=alignment) + # Get horizontal layout for in-place editing + h_layout = align_api.get_horizontal_layout(alignment) + if h_layout is None: + self.report({"ERROR"}, "Alignment has no horizontal layout") + return {"CANCELLED"} # Collect updated PI data hpoints = [(pi.x, pi.y) for pi in props.pis] radii = [pi.radius for pi in props.pis[1:-1]] - # Create a fresh alignment with the same name - # Use safe wrapper to validate/cleanup before creating - new_alignment = tool.Alignment.safe_create_alignment_by_pi_method( - ifc, - name=alignment_name, - hpoints=hpoints, - radii=radii, - start_station=props.start_station, + # Remove Blender visualization for segments (not the whole hierarchy) + tool.Alignment.remove_layout_segment_objects(h_layout) + + # Clear existing IFC segments (preserves layout and zero-length terminator) + align_api.clear_layout_segments(ifc, h_layout) + + # Add new segments with updated PI positions + align_api.layout_horizontal_alignment_by_pi_method( + ifc, h_layout, hpoints, radii ) - # Create Blender hierarchy for the new alignment - tool.Alignment.create_hierarchy_for_alignment(new_alignment) + # Refresh Blender visualization for new segments + layout_obj = tool.Ifc.get_object(h_layout) + if layout_obj: + tool.Alignment.create_objects_for_layout_segments(h_layout, layout_obj) - # Update the active alignment ID to reference the new entity - props.active_alignment_id = new_alignment.id() - props.active_alignment_name = alignment_name - - self.report({"INFO"}, f"Updated alignment '{alignment_name}' with {len(hpoints)} PIs") + # Alignment ID stays the same - no need to update props.active_alignment_id + self.report({"INFO"}, f"Updated alignment '{alignment.Name}' with {len(hpoints)} PIs") return {"FINISHED"} # No active alignment - just report geometry recalculation @@ -1510,7 +1507,7 @@ class SAIKEI_OT_enter_pi_edit_mode(Operator): core.exit_pi_edit_mode( tool.Ifc, tool.Alignment, self._alignment_id, apply=True ) - self.report({"INFO"}, "PI changes applied - alignment regenerated") + self.report({"INFO"}, "PI changes applied - alignment updated") else: # Just cleanup without regenerating core.exit_pi_edit_mode( diff --git a/src/bonsai/bonsai/core/alignment.py b/src/bonsai/bonsai/core/alignment.py index b41d2e3226..a17f1c47f1 100644 --- a/src/bonsai/bonsai/core/alignment.py +++ b/src/bonsai/bonsai/core/alignment.py @@ -207,22 +207,26 @@ def exit_pi_edit_mode( 1. If apply=True: - Collect new PI positions from empties - Validate the new configuration - - Regenerate alignment segments + - Update alignment segments in-place (preserves alignment ID) 2. Always: - Remove temporary EMPTY objects - Return success status + This function modifies the alignment segments in-place rather than + deleting and recreating the alignment. This preserves the alignment's + IFC entity ID, preventing stale reference issues. + Args: ifc_tool: The IFC tool class alignment_tool: The Alignment tool class alignment_id: The IFC ID of the alignment being edited - apply: If True, regenerate alignment with new PI positions + apply: If True, update alignment with new PI positions Returns: True if successful Raises: - ValueError: If alignment doesn't exist or regeneration fails + ValueError: If alignment doesn't exist or update fails """ import ifcopenshell import ifcopenshell.api.alignment as align_api @@ -248,45 +252,29 @@ def exit_pi_edit_mode( if len(hpoints) < 2: raise ValueError("At least 2 PIs are required") - # Get alignment metadata for recreation - alignment_name = alignment.Name or "Alignment" + # Get horizontal layout - required for in-place editing + h_layout = align_api.get_horizontal_layout(alignment) + if h_layout is None: + raise ValueError("Alignment has no horizontal layout") - # Get start station from existing alignment (or default) - start_station = 0.0 - try: - h_layout = align_api.get_horizontal_layout(alignment) - if h_layout: - # Try to get existing start station from referent - for rel in getattr(alignment, "IsNestedBy", []) or []: - for child in rel.RelatedObjects or []: - if child.is_a("IfcReferent"): - pos_el = child.ObjectPlacement - if pos_el and hasattr(pos_el, "PlacementRelTo"): - # Extract station value if available - pass - except Exception: - pass # Use default start_station - - # Remove empties BEFORE deleting alignment (they're parented to it) + # Remove empties before modifying segments alignment_tool.remove_pi_edit_empties(alignment_id) - # Remove old alignment hierarchy from Blender - alignment_tool.remove_alignment_hierarchy(alignment) + # Remove Blender visualization for segments (not the whole hierarchy) + alignment_tool.remove_layout_segment_objects(h_layout) - # Delete old IFC alignment - ifcopenshell.api.run("root.remove_product", ifc_file, product=alignment) + # Clear existing IFC segments (preserves layout and zero-length terminator) + align_api.clear_layout_segments(ifc_file, h_layout) - # Create new alignment with updated PI positions - new_alignment = alignment_tool.safe_create_alignment_by_pi_method( - ifc_file, - name=alignment_name, - hpoints=hpoints, - radii=radii, - start_station=start_station, + # Add new segments with updated PI positions + align_api.layout_horizontal_alignment_by_pi_method( + ifc_file, h_layout, hpoints, radii ) - # Create new Blender hierarchy - alignment_tool.create_hierarchy_for_alignment(new_alignment) + # Refresh Blender visualization for new segments + layout_obj = ifc_tool.get_object(h_layout) + if layout_obj: + alignment_tool.create_objects_for_layout_segments(h_layout, layout_obj) return True else: diff --git a/src/bonsai/bonsai/tool/alignment.py b/src/bonsai/bonsai/tool/alignment.py index b9c5d200b7..10a3803563 100644 --- a/src/bonsai/bonsai/tool/alignment.py +++ b/src/bonsai/bonsai/tool/alignment.py @@ -257,9 +257,9 @@ class Alignment: ) -> Optional[List[Tuple[float, float, float]]]: """Get vertices for a single alignment segment using IfcOpenShell's geometry engine. - Uses the proven IfcOpenShell geometry engine to evaluate points along - the segment, supporting all segment types (LINE, CIRCULARARC, CLOTHOID, - spirals, etc.). + Uses the proven IfcOpenShell C++ geometry engine (create_shape) to generate + vertices, supporting all segment types (LINE, CIRCULARARC, CLOTHOID, + spirals, etc.) including negative-length curve segments. Args: segment: The IfcAlignmentSegment entity @@ -270,7 +270,7 @@ class Alignment: or None if geometry cannot be generated """ import ifcopenshell.api.alignment as align_api - from ifcopenshell.api.alignment import util as align_util + import ifcopenshell.geom import ifcopenshell.util.unit import numpy as np @@ -302,47 +302,37 @@ class Alignment: if curve_segment is None: continue - # Get segment length + # Use create_shape to generate vertices - the same proven approach as generate_vertices() + # This handles negative-length curve segments correctly at the C++ level try: - seg_length = abs(curve_segment.SegmentLength.wrappedValue) - except (AttributeError, TypeError): + s = ifcopenshell.geom.settings() try: - seg_length = abs(float(curve_segment.SegmentLength)) - except: + s.set("piecewise-step-type", 0) # step-size is maximum step size + except RuntimeError: + pass + try: + s.set("piecewise-step-size", distance_interval) + except RuntimeError: + pass + + shape = ifcopenshell.geom.create_shape(s, curve_segment) + verts = shape.verts + + if len(verts) == 0: continue - if seg_length < 1e-6: - continue - - # Calculate number of sample points - num_points = max(int(seg_length / distance_interval) + 1, 2) - - # Sample points along the segment using IfcOpenShell's geometry engine - for i in range(num_points): - # Calculate distance along segment (don't exceed segment length) - if num_points > 1: - dist_along = min(i * distance_interval, seg_length) - # Ensure we get the last point exactly at segment end - if i == num_points - 1: - dist_along = seg_length - else: - dist_along = 0.0 - - try: - # Use IfcOpenShell's evaluate_segment to get transform matrix - transform_matrix = align_util.evaluate_segment(curve_segment, dist_along) - - # Extract position from 4x4 matrix - # Matrix is transposed by util.py, so translation is in row 3 - x = float(transform_matrix[3, 0]) / unit_scale - y = float(transform_matrix[3, 1]) / unit_scale - z = float(transform_matrix[3, 2]) / unit_scale - + # Reshape to (N, 3) array and apply unit scale + vertices_array = np.array(verts).reshape((-1, 3)) + for v in vertices_array: + # create_shape returns values already in file units, apply scale + x = float(v[0]) / unit_scale + y = float(v[1]) / unit_scale + z = float(v[2]) / unit_scale all_vertices.append((x, y, z)) - except Exception as e: - # If evaluation fails at this point, skip it - continue + except Exception as e: + print(f"[Alignment] create_shape failed for curve segment: {e}") + continue if len(all_vertices) < 2: return None diff --git a/src/ifcopenshell-python/ifcopenshell/api/alignment/__init__.py b/src/ifcopenshell-python/ifcopenshell/api/alignment/__init__.py index 2662015313..e2542360e1 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/alignment/__init__.py +++ b/src/ifcopenshell-python/ifcopenshell/api/alignment/__init__.py @@ -51,6 +51,7 @@ from ._get_segment_start_point_label import register_referent_name_callback from .add_stationing_referent import add_stationing_referent from .add_vertical_layout import add_vertical_layout from .add_zero_length_segment import add_zero_length_segment +from .clear_layout_segments import clear_layout_segments from .create import create from .create_as_offset_curve import create_as_offset_curve from .create_as_polyline import create_as_polyline @@ -93,6 +94,7 @@ __all__ = [ "add_stationing_referent", "add_vertical_layout", "add_zero_length_segment", + "clear_layout_segments", "create", "create_as_offset_curve", "create_as_polyline", diff --git a/src/ifcopenshell-python/ifcopenshell/api/alignment/clear_layout_segments.py b/src/ifcopenshell-python/ifcopenshell/api/alignment/clear_layout_segments.py new file mode 100644 index 0000000000..43e8875d63 --- /dev/null +++ b/src/ifcopenshell-python/ifcopenshell/api/alignment/clear_layout_segments.py @@ -0,0 +1,223 @@ +# IfcOpenShell - IFC toolkit and geometry engine +# Copyright (C) 2025 Thomas Krijnen +# +# This file is part of IfcOpenShell. +# +# IfcOpenShell is free software: you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# IfcOpenShell 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 Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with IfcOpenShell. If not, see . + +import ifcopenshell +import ifcopenshell.api.alignment +import ifcopenshell.api.nest +import ifcopenshell.util.element +from ifcopenshell import entity_instance + + +def _is_zero_length_segment(segment: entity_instance) -> bool: + """Check if segment is a zero-length terminator.""" + dp = segment.DesignParameters + if dp.is_a("IfcAlignmentHorizontalSegment"): + return dp.SegmentLength == 0.0 + elif dp.is_a("IfcAlignmentVerticalSegment"): + return dp.HorizontalLength == 0.0 + elif dp.is_a("IfcAlignmentCantSegment"): + return dp.HorizontalLength == 0.0 + return False + + +def clear_layout_segments(file: ifcopenshell.file, layout: entity_instance) -> None: + """ + Clear all segments from a layout while preserving the layout entity + and zero-length terminator. After calling this, use + layout_horizontal_alignment_by_pi_method() to add new segments. + + This function removes: + - All real (non-zero-length) IfcAlignmentSegment entities from the layout + - Their associated IfcCurveSegment entities from the geometric representation + - Referents positioned on the removed segments + + It preserves: + - The layout entity (IfcAlignmentHorizontal, IfcAlignmentVertical, or IfcAlignmentCant) + - The zero-length terminator segment (required by IFC spec) + - The alignment's main stationing referent + + :param file: The IFC file + :param layout: An IfcAlignmentHorizontal, IfcAlignmentVertical, or IfcAlignmentCant + + Example: + + .. code:: python + + alignment = model.by_type("IfcAlignment")[0] + h_layout = ifcopenshell.api.alignment.get_horizontal_layout(alignment) + + # Clear existing segments + ifcopenshell.api.alignment.clear_layout_segments(model, h_layout) + + # Add new segments with updated PI positions + ifcopenshell.api.alignment.layout_horizontal_alignment_by_pi_method( + model, h_layout, new_hpoints, new_radii + ) + """ + expected_types = ["IfcAlignmentHorizontal", "IfcAlignmentVertical", "IfcAlignmentCant"] + if layout.is_a() not in expected_types: + raise TypeError( + f"Expected entity type to be one of {expected_types}, instead received {layout.is_a()}" + ) + + # Get the geometric curve for this layout + curve = ifcopenshell.api.alignment.get_layout_curve(layout) + + # Get all segments from the layout + segments = ifcopenshell.api.alignment.get_layout_segments(layout) + + if not segments: + return # Nothing to clear + + # Identify segments to remove (all except zero-length terminator) + zero_length_segment = None + segments_to_remove = [] + + for segment in segments: + if _is_zero_length_segment(segment): + zero_length_segment = segment + else: + segments_to_remove.append(segment) + + if not segments_to_remove: + return # Only zero-length terminator exists, nothing to clear + + # Collect curve segments to remove before removing alignment segments + # (we need the nesting relationship to find mapped segments) + curve_segments_to_remove = [] + for segment in segments_to_remove: + try: + mapped = ifcopenshell.api.alignment.get_mapped_segments(segment) + for cs in mapped: + if cs is not None: + curve_segments_to_remove.append(cs) + except (IndexError, AttributeError): + # Segment might not have curve representation yet + pass + + # Remove referents positioned on segments being removed + for segment in segments_to_remove: + # Check for referents positioned relative to this segment + if hasattr(segment, "PositionedRelativeTo") and segment.PositionedRelativeTo: + for rel_pos in segment.PositionedRelativeTo: + referent = rel_pos.RelatingPositioningElement + if referent and referent.is_a("IfcReferent"): + # Remove the referent + ifcopenshell.api.run("root.remove_product", file, product=referent) + + # Remove segments from nesting relationship + ifcopenshell.api.nest.unassign_object(file, related_objects=segments_to_remove) + + # Remove segment entities + for segment in segments_to_remove: + # Remove design parameters + dp = segment.DesignParameters + if dp: + # Remove StartPoint if it exists + if hasattr(dp, "StartPoint") and dp.StartPoint: + file.remove(dp.StartPoint) + file.remove(dp) + + # Remove the segment entity itself + file.remove(segment) + + # Clear curve segments from the geometric representation + if curve and curve.Segments: + # Keep only the zero-length curve segment (last one) + if ifcopenshell.api.alignment.has_zero_length_segment(curve): + zero_length_curve_seg = curve.Segments[-1] + # Update curve to only contain zero-length segment + curve.Segments = (zero_length_curve_seg,) + else: + # No zero-length segment in curve, clear all + curve.Segments = () + + # Clean up removed curve segment entities + for cs in curve_segments_to_remove: + try: + # Remove the curve segment's parent curve and placement + if hasattr(cs, "ParentCurve") and cs.ParentCurve: + parent_curve = cs.ParentCurve + # Check if parent curve is used elsewhere + if file.get_total_inverses(parent_curve) <= 1: + # Remove placement if exists + if hasattr(parent_curve, "Position") and parent_curve.Position: + pos = parent_curve.Position + if hasattr(pos, "Location") and pos.Location: + if file.get_total_inverses(pos.Location) <= 1: + file.remove(pos.Location) + if hasattr(pos, "RefDirection") and pos.RefDirection: + if file.get_total_inverses(pos.RefDirection) <= 1: + file.remove(pos.RefDirection) + if file.get_total_inverses(pos) <= 1: + file.remove(pos) + file.remove(parent_curve) + + # Remove placement on curve segment + if hasattr(cs, "Placement") and cs.Placement: + placement = cs.Placement + if hasattr(placement, "Location") and placement.Location: + if file.get_total_inverses(placement.Location) <= 1: + file.remove(placement.Location) + if hasattr(placement, "RefDirection") and placement.RefDirection: + if file.get_total_inverses(placement.RefDirection) <= 1: + file.remove(placement.RefDirection) + if file.get_total_inverses(placement) <= 1: + file.remove(placement) + + # Remove the curve segment itself + file.remove(cs) + except Exception: + # Entity may have already been removed + pass + + # Reset zero-length terminator to origin position + if zero_length_segment: + dp = zero_length_segment.DesignParameters + if dp.is_a("IfcAlignmentHorizontalSegment"): + # Reset StartPoint to origin + if dp.StartPoint: + dp.StartPoint.Coordinates = (0.0, 0.0) + dp.StartDirection = 0.0 + elif dp.is_a("IfcAlignmentVerticalSegment"): + dp.StartDistAlong = 0.0 + dp.StartHeight = 0.0 + dp.StartGradient = 0.0 + dp.EndGradient = 0.0 + elif dp.is_a("IfcAlignmentCantSegment"): + dp.StartDistAlong = 0.0 + dp.StartCantLeft = 0.0 + dp.StartCantRight = 0.0 + + # Update the zero-length segment's referent + if hasattr(zero_length_segment, "PositionedRelativeTo") and zero_length_segment.PositionedRelativeTo: + for rel_pos in zero_length_segment.PositionedRelativeTo: + referent = rel_pos.RelatingPositioningElement + if referent and referent.is_a("IfcReferent"): + # Update referent position to origin + if hasattr(referent, "ObjectPlacement") and referent.ObjectPlacement: + placement = referent.ObjectPlacement + if hasattr(placement, "RelativePlacement") and placement.RelativePlacement: + rel_place = placement.RelativePlacement + if hasattr(rel_place, "Location") and rel_place.Location: + if hasattr(rel_place.Location, "DistanceAlong"): + rel_place.Location.DistanceAlong.wrappedValue = 0.0 + if hasattr(placement, "CartesianPosition") and placement.CartesianPosition: + cart_pos = placement.CartesianPosition + if hasattr(cart_pos, "Location") and cart_pos.Location: + cart_pos.Location.Coordinates = (0.0, 0.0, 0.0) diff --git a/src/ifcopenshell-python/ifcopenshell/api/alignment/util.py b/src/ifcopenshell-python/ifcopenshell/api/alignment/util.py index a9e6e3f2e6..c82abef50a 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/alignment/util.py +++ b/src/ifcopenshell-python/ifcopenshell/api/alignment/util.py @@ -60,8 +60,18 @@ def evaluate_segment(segment: entity_instance, dist_along: float) -> np.ndarray: segment_type = segment.is_a().upper() if not segment_type in supported_segment_types: raise NotImplementedError(f"Expected entity type 'IFCCURVESEGMENT', got '{segment_type}") - if dist_along > segment.SegmentLength: - raise ValueError(f"Provided value {dist_along=} is beyond the end of the segment ({segment.SegmentLength}).") + + # Validate dist_along is within segment bounds + # SegmentLength can be negative (indicates curve direction), so we need to handle both cases + seg_len = segment.SegmentLength.wrappedValue if hasattr(segment.SegmentLength, 'wrappedValue') else segment.SegmentLength + if seg_len >= 0: + # Positive length: valid range is 0 to seg_len + if dist_along < 0 or dist_along > seg_len: + raise ValueError(f"Provided value {dist_along=} is beyond the end of the segment ({segment.SegmentLength}).") + else: + # Negative length: valid range is seg_len to 0 + if dist_along > 0 or dist_along < seg_len: + raise ValueError(f"Provided value {dist_along=} is beyond the end of the segment ({segment.SegmentLength}).") s = ifcopenshell.geom.settings() function_item = ifcopenshell_wrapper.map_shape(s, segment.wrapped_data)