diff --git a/src/Mod/BIM/Arch.py b/src/Mod/BIM/Arch.py index 008daf13f5..b275c17915 100644 --- a/src/Mod/BIM/Arch.py +++ b/src/Mod/BIM/Arch.py @@ -2151,6 +2151,162 @@ def makeWindow( return window +def is_debasable(wall): + """Determines if an Arch Wall can be cleanly converted to a baseless state. + + This function checks if a given wall is a valid candidate for a parametric + "debasing" operation, where its dependency on a Base object is removed and + it becomes driven by its own Length and Placement properties. + + Parameters + ---------- + wall : FreeCAD.DocumentObject + The Arch Wall object to check. + + Returns + ------- + bool + ``True`` if the wall is a valid candidate for debasing, otherwise ``False``. + + Notes + ----- + A wall is considered debasable if its ``Base`` object's final shape consists + of exactly one single, straight edge. This check is generic and works for + any base object that provides a valid ``.Shape`` property, including + ``Draft.Line`` and ``Sketcher::SketchObject`` objects. + """ + import Part + import Draft + + # Ensure the object is actually a wall + if Draft.getType(wall) != "Wall": + return False + + # Check for a valid Base object with a geometric Shape + if not hasattr(wall, "Base") or not wall.Base: + return False + if not hasattr(wall.Base, "Shape") or wall.Base.Shape.isNull(): + return False + + base_shape = wall.Base.Shape + + # The core condition: the final shape must contain exactly one edge. + # This correctly handles Sketches with multiple lines or construction geometry. + if len(base_shape.Edges) != 1: + return False + + # The single edge must be a straight line. + edge = base_shape.Edges[0] + if not isinstance(edge.Curve, (Part.Line, Part.LineSegment)): + return False + + # If all checks pass, the wall is debasable. + return True + + +def debaseWall(wall): + """ + Converts a line-based Arch Wall to be parametrically driven by its own + properties (Length, Width, Height) and Placement, removing its dependency + on a Base object. + + This operation preserves the wall's exact size and global position. + It is only supported for walls based on a single, straight line. + + Returns True on success, False otherwise. + """ + import FreeCAD + + if not is_debasable(wall): + FreeCAD.Console.PrintWarning(f"Wall '{wall.Label}' is not eligible for debasing.\n") + return False + + doc = wall.Document + doc.openTransaction(f"Debase Wall: {wall.Label}") + try: + # Calculate the final state before making any changes + # A wall's final position and orientation are derived from its Base object and its own + # properties like Width and Align. To make the wall independent, this final state must be + # captured. A simple transfer of the Base object's Placement property is unreliable for two + # main reasons: + # - Ambiguity: A Draft.Line's direction is defined by its vertex coordinates, so a line at + # 45° can have a 0° rotation in its Placement. + # - Coordinate Systems: A universal method is needed to handle both Draft objects (defined + # in global coordinates) and Sketch objects (defined in a local coordinate system). + # + # The solution is to use the wall.Proxy.basewires internal attribute. It is non-persistent + # and populated on the fly by calling getExtrusionData(). It contains the baseline edge + # already transformed into the document's global coordinate system. From the vertex + # positions of this globally-aware edge, the new wall's final Placement is calculated. + extrusion_data = wall.Proxy.getExtrusionData(wall) + if not extrusion_data or not hasattr(wall.Proxy, "basewires") or not wall.Proxy.basewires: + raise Exception("Could not retrieve extrusion data to calculate global placement.") + + # In addition to the baseline edge, getExtrusionData() also provides the extrusion vector, + # which is used to determine the wall's vertical orientation. + extrusion_vector = extrusion_data[1] + baseline_edge = wall.Proxy.basewires[0][0] + + # Now determine the wall's rotation inferred from its local axes: + # - The local X axis is along the baseline edge (length). + # - The local Z axis is along the extrusion vector (height). + # - The local Y axis is the cross product of X and Z (width, perpendicular to both the + # - above). + # Once the local axes are known, a FreeCAD.Rotation matrix can be constructed. + z_axis = extrusion_vector.normalize() + x_axis = (baseline_edge.lastVertex().Point - baseline_edge.firstVertex().Point).normalize() + y_axis = z_axis.cross(x_axis).normalize() + final_rotation = FreeCAD.Rotation(x_axis, y_axis, z_axis) + + # This will be the debased wall's local coordinate system origin (0, 0, 0). + # The wall's Align property (Left, Center, Right) determines how the wall's + # Width offsets the final position from the centerline. + centerline_position = baseline_edge.CenterOfMass + align_offset_distance = 0 + if wall.Align == "Left": + align_offset_distance = wall.Width.Value / 2.0 + elif wall.Align == "Right": + align_offset_distance = -wall.Width.Value / 2.0 + + # Convert the offset distance into a vector in the width direction (local Y axis). + align_offset_vector = y_axis * align_offset_distance + final_position = centerline_position - align_offset_vector + + final_placement = FreeCAD.Placement(final_position, final_rotation) + + # Store properties before unlinking + height = wall.Height.Value + length = wall.Length.Value + width = wall.Width.Value + + # 1. Apply the final placement first. + wall.Placement = final_placement + + # 2. Now, remove the base. The recompute triggered by this change + # will already have the correct placement to work with. + wall.Base = None + + # 3. Clear internal caches and set final properties. + if hasattr(wall.Proxy, "connectEdges"): + wall.Proxy.connectEdges = [] + + wall.Height = height + wall.Length = length + wall.Width = width + + # 4. Add an explicit recompute to ensure the final state is settled. + doc.recompute() + + except Exception as e: + doc.abortTransaction() + FreeCAD.Console.PrintError(f"Error debasing wall '{wall.Label}': {e}\n") + return False + finally: + doc.commitTransaction() + + return True + + def _initializeArchObject( objectType, baseClassName=None, diff --git a/src/Mod/BIM/ArchCommands.py b/src/Mod/BIM/ArchCommands.py index cf0469cde6..cfb393e89c 100644 --- a/src/Mod/BIM/ArchCommands.py +++ b/src/Mod/BIM/ArchCommands.py @@ -245,6 +245,15 @@ def removeComponents(objectsList, host=None): for o in objectsList: if o.InList: h = o.InList[0] + + is_base_removal = hasattr(h, "Base") and h.Base == o + has_handler = hasattr(h, "Proxy") and hasattr(h.Proxy, "handleComponentRemoval") + + if is_base_removal and has_handler: + # Dispatch to the object's own smart removal logic and skip the old code path. + h.Proxy.handleComponentRemoval(h, o) + continue + tp = Draft.getType(h) if tp in ["Floor", "Building", "Site", "BuildingPart"]: c = h.Group diff --git a/src/Mod/BIM/ArchComponent.py b/src/Mod/BIM/ArchComponent.py index f6820f5a5a..b61c186dd8 100644 --- a/src/Mod/BIM/ArchComponent.py +++ b/src/Mod/BIM/ArchComponent.py @@ -835,6 +835,13 @@ class Component(ArchIFC.IfcProduct): if o: o.ViewObject.hide() + def handleComponentRemoval(self, obj, subobject): + """ + Default handler for when a component is removed via the Task Panel. + Subclasses can override this to provide special behavior. + """ + removeFromComponent(obj, subobject) + def processSubShapes(self, obj, base, placement=None): """Add Additions and Subtractions to a base shape. @@ -2284,18 +2291,24 @@ class ComponentTaskPanel: self.update() def removeElement(self): - """This method is run as a callback when the user selects the remove button. - - Get the object selected in the tree widget. If there is an object in - the document with the same Name as the selected item in the tree, - remove it from the object being edited, with the removeFromComponent() - function. """ + This method is run as a callback when the user selects the remove button. + It calls a handler on the object's proxy to perform the removal. + """ + element_selected = self.tree.currentItem() + if not element_selected: + return + + element_to_remove = FreeCAD.ActiveDocument.getObject(str(element_selected.toolTip(0))) + + # Call the polymorphic handler on the object's proxy. + # This is generic and works for any Arch object. + if hasattr(self.obj.Proxy, "handleComponentRemoval"): + self.obj.Proxy.handleComponentRemoval(self.obj, element_to_remove) + else: + # Fallback for older proxies that might not have the method + removeFromComponent(self.obj, element_to_remove) - it = self.tree.currentItem() - if it: - comp = FreeCAD.ActiveDocument.getObject(str(it.toolTip(0))) - removeFromComponent(self.obj, comp) self.update() def accept(self): diff --git a/src/Mod/BIM/ArchWall.py b/src/Mod/BIM/ArchWall.py index f139df20d8..f574e26346 100644 --- a/src/Mod/BIM/ArchWall.py +++ b/src/Mod/BIM/ArchWall.py @@ -1520,6 +1520,55 @@ class _Wall(ArchComponent.Component): return (base, extrusion, placement) return None + def handleComponentRemoval(self, obj, subobject): + """ + Overrides the default component removal to implement smart debasing + when the Base object is being removed. + """ + import Arch + from PySide import QtGui + + # Check if the component being removed is this wall's Base + if hasattr(obj, "Base") and obj.Base == subobject: + if Arch.is_debasable(obj): + # This is a valid, single-line wall. Perform a clean debase. + Arch.debaseWall(obj) + else: + # This is a complex wall. Behavior depends on GUI availability. + if FreeCAD.GuiUp: + # --- GUI Path: Warn the user and ask for confirmation. --- + from PySide import QtGui + + msg_box = QtGui.QMessageBox() + msg_box.setWindowTitle(translate("ArchComponent", "Unsupported Base")) + msg_box.setText( + translate( + "ArchComponent", "The base of this wall is not a single straight line." + ) + ) + msg_box.setInformativeText( + translate( + "ArchComponent", + "Removing the base of this complex wall will alter its shape and reset its position.\n\n" + "Do you want to proceed?", + ) + ) + msg_box.setStandardButtons(QtGui.QMessageBox.Yes | QtGui.QMessageBox.Cancel) + msg_box.setDefaultButton(QtGui.QMessageBox.Cancel) + if msg_box.exec_() == QtGui.QMessageBox.Yes: + # User confirmed, perform the standard removal + super(_Wall, self).handleComponentRemoval(obj, subobject) + else: + # --- Headless Path: Do not perform the destructive action. Print a warning. --- + FreeCAD.Console.PrintWarning( + f"Skipping removal of complex base for wall '{obj.Label}'. " + "This interactive action is not supported in headless mode.\n" + ) + else: + # If it's not the base (e.g., an Addition), use the default behavior + # from the parent Component class. + super(_Wall, self).handleComponentRemoval(obj, subobject) + class _ViewProviderWall(ArchComponent.ViewProviderComponent): """The view provider for the wall object. diff --git a/src/Mod/BIM/bimtests/TestArchWall.py b/src/Mod/BIM/bimtests/TestArchWall.py index 30a323f58f..c3e745e3e7 100644 --- a/src/Mod/BIM/bimtests/TestArchWall.py +++ b/src/Mod/BIM/bimtests/TestArchWall.py @@ -118,23 +118,146 @@ class TestArchWall(TestArchBase.TestArchBase): def test_remove_base_from_wall_without_host(self): """ - Tests that removing a wall's base using removeComponents(host=None) - does not crash and successfully unlinks the base. - This is the non-regression test for the 'list' has no attribute 'Base' bug. - https://github.com/FreeCAD/FreeCAD/issues/24532 + Tests that removing a debasable wall's base using removeComponents + successfully unlinks the base. """ self.printTestMessage("Testing removal of a wall's base component...") # 1. Arrange: Create a wall with a base line = Draft.makeLine(App.Vector(0, 0, 0), App.Vector(2000, 0, 0)) + # Ensure the base object's shape is computed, making the wall debasable. + line.recompute() wall = Arch.makeWall(line) + self.document.recompute() # Ensure wall is fully formed self.assertIsNotNone(wall.Base, "Pre-condition failed: Wall should have a base.") + self.assertTrue( + Arch.is_debasable(wall), "Pre-condition failed: The test wall is not debasable." + ) - # 2. Act: Call removeComponents on the base, simulating the failing workflow - # Before the fix, this will raise an AttributeError. - # After the fix, it should complete without error. + # 2. Act: Call removeComponents on the base. + # This will trigger the is_debasable -> True -> debaseWall() path. Arch.removeComponents([wall.Base]) self.document.recompute() - # 3. Assert: The base should now be None + # 3. Assert: The base should now be None because debaseWall was successful. self.assertIsNone(wall.Base, "The wall's Base property was not cleared after removal.") + + def test_is_debasable_with_valid_line_base(self): + """Tests that a wall based on a single Draft.Line is debasable.""" + self.printTestMessage("Checking is_debasable with Draft.Line...") + line = Draft.makeLine(App.Vector(0, 0, 0), App.Vector(1000, 0, 0)) + line.recompute() + wall = Arch.makeWall(line) + self.document.recompute() + self.assertTrue(Arch.is_debasable(wall), "Wall on Draft.Line should be debasable.") + + def test_is_debasable_with_valid_sketch_base(self): + """Tests that a wall based on a Sketch with a single line is debasable.""" + self.printTestMessage("Checking is_debasable with single-line Sketch...") + sketch = self.document.addObject("Sketcher::SketchObject", "SingleLineSketch") + sketch.addGeometry(Part.LineSegment(App.Vector(0, 0, 0), App.Vector(1000, 0, 0))) + self.document.recompute() + wall = Arch.makeWall(sketch) + self.assertTrue(Arch.is_debasable(wall), "Wall on single-line Sketch should be debasable.") + + def test_is_debasable_with_multi_edge_base(self): + """Tests that a wall based on a multi-segment wire is not debasable.""" + self.printTestMessage("Checking is_debasable with multi-segment Wire...") + wire = Draft.makeWire( + [App.Vector(0, 0, 0), App.Vector(1000, 0, 0), App.Vector(1000, 1000, 0)] + ) + wall = Arch.makeWall(wire) + self.assertFalse( + Arch.is_debasable(wall), "Wall on multi-segment wire should not be debasable." + ) + + def test_is_debasable_with_curved_base(self): + """Tests that a wall based on an arc is not debasable.""" + self.printTestMessage("Checking is_debasable with curved base...") + arc = Draft.make_circle(radius=500, startangle=0, endangle=90) + self.document.recompute() + wall = Arch.makeWall(arc) + self.document.recompute() + self.assertFalse(Arch.is_debasable(wall), "Wall on curved base should not be debasable.") + + def test_is_debasable_with_no_base(self): + """Tests that a baseless wall is not debasable.""" + self.printTestMessage("Checking is_debasable with no base...") + wall = Arch.makeWall(length=1000) + self.assertFalse(Arch.is_debasable(wall), "Baseless wall should not be debasable.") + + def test_debase_wall_preserves_global_position(self): + """ + Tests that debaseWall correctly transfers the base's placement to the + wall, preserving its global position and dimensions. + """ + self.printTestMessage("Checking debaseWall preserves global position...") + + # 1. Arrange: Create a rotated and translated line, and a wall from it. + pl = App.Placement(App.Vector(1000, 500, 200), App.Rotation(App.Vector(0, 0, 1), 45)) + line = Draft.makeLine(App.Vector(0, 0, 0), App.Vector(2000, 0, 0)) + line.Placement = pl + line.recompute() # Use object-level recompute + + wall = Arch.makeWall(line, width=200, height=1500, align="Left") + self.document.recompute() + + # Store the wall's original state + original_bb = wall.Shape.BoundBox + original_volume = wall.Shape.Volume + original_length = wall.Length.Value + + # 2. Act: Debase the wall + success = Arch.debaseWall(wall) + self.document.recompute() + + # 3. Assert + self.assertTrue(success, "debaseWall should return True for a valid wall.") + self.assertIsNone(wall.Base, "Wall's Base should be None after debasing.") + + # Core assertions for preserving geometry and placement + self.assertAlmostEqual( + original_volume, + wall.Shape.Volume, + delta=1e-6, + msg="Wall volume should not change after debasing.", + ) + + # Compare individual properties of the BoundBox with a tolerance + final_bb = wall.Shape.BoundBox + self.assertAlmostEqual( + original_bb.XMin, final_bb.XMin, delta=1e-6, msg="Bounding box XMin does not match." + ) + self.assertAlmostEqual( + original_bb.XMax, final_bb.XMax, delta=1e-6, msg="Bounding box XMax does not match." + ) + self.assertAlmostEqual( + original_bb.YMin, final_bb.YMin, delta=1e-6, msg="Bounding box YMin does not match." + ) + self.assertAlmostEqual( + original_bb.YMax, final_bb.YMax, delta=1e-6, msg="Bounding box YMax does not match." + ) + self.assertAlmostEqual( + original_bb.ZMin, final_bb.ZMin, delta=1e-6, msg="Bounding box ZMin does not match." + ) + self.assertAlmostEqual( + original_bb.ZMax, final_bb.ZMax, delta=1e-6, msg="Bounding box ZMax does not match." + ) + + # Check parametric integrity + self.assertAlmostEqual( + wall.Length.Value, + original_length, + delta=1e-6, + msg="Wall's Length property should be preserved.", + ) + + # Verify it remains parametric by changing a property + wall.Height = 2000 + self.document.recompute() + self.assertNotAlmostEqual( + original_volume, + wall.Shape.Volume, + delta=1e-6, + msg="Wall should remain parametric and its volume should change with height.", + )