kindred/create
1
1
Fork 0
Code Issues 53 Pull Requests Actions Packages Projects 9 Releases 2 Wiki Activity

BIM: Smart removal of wall bases (#24550)

Browse Source 
* BIM: Implement smart base removal for Walls

Previously, removing the Base object from an Arch Wall would cause the
wall to reset its position to the document origin and could lead to
unintended geometric changes for complex walls.

This commit introduces a "smart debasing" mechanism integrated into the
Component Task Panel's "Remove" button:

- For walls based on a single straight line, the operation now preserves
  the wall's global position and parametric `Length`, making it an
  independent object.
- For walls with complex bases (multi-segment, curved), a warning dialog
  is now presented to the user, explaining the consequences (shape
  alteration and position reset) before allowing the operation to
  proceed.

This is supported by new API functions `Arch.is_debasable()` and
`Arch.debaseWall()`, which contain the core logic for the feature.

Fixes: https://github.com/FreeCAD/FreeCAD/issues/24453

* BIM: Move wall debasing logic into ArchWall proxy

The logic for handling the removal of a wall's base object was previously
implemented directly within the generic `ComponentTaskPanel` in
`ArchComponent.py`. This created a tight coupling, forcing the generic
component UI to have specific knowledge about the `ArchWall` type.

This commit refactors the implementation to follow a more object-oriented
and polymorphic design:

1.  A new overridable method, `handleComponentRemoval(subobject)`, has been
    added to the base `ArchComponent` proxy class. Its default implementation
    maintains the standard removal behavior.

2.  The `_Wall` proxy class in `ArchWall.py` now overrides this method. All
    wall-specific debasing logic, including the eligibility check and the
    user-facing warning dialog, now resides entirely within this override.

3.  The `ComponentTaskPanel.removeElement` method has been simplified. It is
    now a generic dispatcher that calls `handleComponentRemoval` on the
    proxy of the object being edited, with no specific knowledge of object types.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* BIM: Correct user warning

The operation can indeed be undone.

Co-authored-by: Roy-043 <70520633+Roy-043@users.noreply.github.com>

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Roy-043 <70520633+Roy-043@users.noreply.github.com>
This commit is contained in:
Furgo
2025-11-18 14:58:59 +01:00
committed by GitHub
parent 7980d82207
commit 0b0d1ccc12
5 changed files with 368 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

156
src/Mod/BIM/Arch.py
View File

@@ -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,

9
src/Mod/BIM/ArchCommands.py
View File

@@ -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

33
src/Mod/BIM/ArchComponent.py
View File

@@ -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):

49
src/Mod/BIM/ArchWall.py
View File

@@ -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.

139
src/Mod/BIM/bimtests/TestArchWall.py
View File

@@ -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.",
)