From 562cac277c38a3eb7e56de4f0754d76c53f3fa6c Mon Sep 17 00:00:00 2001 From: Furgo <148809153+furgo16@users.noreply.github.com> Date: Sun, 15 Feb 2026 17:35:26 +0100 Subject: [PATCH] BIM: fix coordinate normalization and area calculation for generic ArchComponents (#27322) * BIM: extend ArchComponent test suite Adds several unit tests to TestArchComponent.py to validate the coordinate system logic and CSG behavior of generic Arch Components. Included tests cover: - Double-transformation/displacement bugs when using moved base objects. - Correct alignment of CSG additions/subtractions in world space. - Consistency of Vertical and Horizontal area calculations for rotated components. - Automatic geometry spreading (arraying) via the Axis property. - Cleanup behavior ensuring the shape is cleared when the Base link is removed. * BIM: fix double displacement bug and area calculation in ArchComponent Rewrites ArchComponent.Component.execute to resolve placement conflicts between the Document Object and its internal TopoShape. Changes: - Reset base_shape.Placement to Identity immediately after copying from the source. This strips inherited placements from the Base object, ensuring the geometry is local before processing. - Pass the object's placement to processSubShapes. This allows additions and subtractions to be correctly localized to the component's local origin. - Replaces direct Shape assignment with self.applyShape(). This fixes area calculations by ensuring the AreaCalculator receives the correct placement context, and enables automatic geometry spreading via the Axis property. Fixes: https://github.com/FreeCAD/FreeCAD/issues/24580 * BIM: Refine base validation in ArchComponent to prevent stale shapes Updates the validation logic in ArchComponent.execute() to correctly handle the removal of a base object. Changes: - The check on self.ensureBase(obj) now explicitly looks for False. Because ensureBase returns None when no base is present, the previous logic would return early and leave the old shape behind. - This change allows the method to fall through to the shape-clearing logic, ensuring that unlinking a base object correctly removes its geometry from the 3D view. - Added a TODO note to ensureBase() regarding its undocumented triple-state return (True, False, None) for future cleanup. --- src/Mod/BIM/ArchComponent.py | 34 ++++- src/Mod/BIM/bimtests/TestArchComponent.py | 171 ++++++++++++++++++++++ 2 files changed, 199 insertions(+), 6 deletions(-) diff --git a/src/Mod/BIM/ArchComponent.py b/src/Mod/BIM/ArchComponent.py index dabcbbcba5..8ea4b9ef83 100644 --- a/src/Mod/BIM/ArchComponent.py +++ b/src/Mod/BIM/ArchComponent.py @@ -391,16 +391,36 @@ class Component(ArchIFC.IfcProduct): obj: The component object. """ + import Part if self.clone(obj): return - if not self.ensureBase(obj): + if self.ensureBase(obj) is False: + # This will fall through if the Component object has no base, allowing the base shapeto + # be cleared return - if obj.Base: - shape = self.spread(obj, obj.Base.Shape) - if obj.Additions or obj.Subtractions: - shape = self.processSubShapes(obj, shape) - obj.Shape = shape + + # Only proceed if a Base object is linked and contains valid geometry. + if obj.Base and hasattr(obj.Base, "Shape") and not obj.Base.Shape.isNull(): + # Create a standalone shape as a deep copy of the base geometry, to avoid modifying + # the original source. + base_shape = Part.Shape(obj.Base.Shape) + + # Reset the shape's internal placement to Identity. This strips the placement + # inherited from the Base object, ensuring the geometry is centered at (0,0,0) for + # Boolean operations in processSubShapes. This also prevents the shape's placement from + # overwriting the Component's own Placement property during assignment in applyShape. + base_shape.Placement = FreeCAD.Placement() + + # Localize the CSG shapes: pass the object's placement to processSubShapes, so that the + # placements of any additions and subtractions are also localized to the local origin of + # the Arch Component. + final_shape = self.processSubShapes(obj, base_shape, obj.Placement) + self.applyShape(obj, final_shape, obj.Placement, allownosolid=True) + else: + # Clear the shape if the base has been removed. This avoids leaving a stale shape that + # is not updated when the base is removed. + obj.Shape = Part.Shape() def dumps(self): return None @@ -1267,6 +1287,8 @@ class Component(ArchIFC.IfcProduct): def ensureBase(self, obj): """Returns False if the object has a Base but of the wrong type. Either returns True""" + # TODO: this method has a third undocumented state: None, which is returned if the object + # has no Base. This should either be fixed if unintended, or documented if intended. if getattr(obj, "Base", None): if obj.Base.isDerivedFrom("Part::Feature"): diff --git a/src/Mod/BIM/bimtests/TestArchComponent.py b/src/Mod/BIM/bimtests/TestArchComponent.py index 6704761b1a..4c298a7547 100644 --- a/src/Mod/BIM/bimtests/TestArchComponent.py +++ b/src/Mod/BIM/bimtests/TestArchComponent.py @@ -547,3 +547,174 @@ class TestArchComponent(TestArchBase.TestArchBase): f"Got: {area_actual:.3f} m²" ), ) + + def test_rotated_component_area(self): + """Verify AreaCalculator respects Placement for generic ArchComponents.""" + self.printTestMessage("ArchComponent rotated area calculation...") + + # Create a horizontal slab (1 m x 1 m x 0.01 m) + # Area of one large face = 1.0 m2 + box = self.document.addObject("Part::Box", "HorizontalSlab") + box.Length = 1000.0 + box.Width = 1000.0 + box.Height = 10.0 + self.document.recompute() + + # Wrap in a generic Component and rotate 90 degrees around the X axis to turn it into a + # vertical panel. + comp = Arch.makeComponent(box) + comp.Placement.Rotation = App.Rotation(App.Vector(1, 0, 0), 90) + self.document.recompute() + + # Check the VerticalArea property. + # FreeCAD sums all vertical faces (front + back + vertical side edges). + # Expected: (1.0 * 1.0)*2 + (1.0 * 0.01)*2 = 2.02 m2 + expected_v_area = 2.02 + actual_v_area = comp.VerticalArea.getValueAs("m^2").Value + + self.assertAlmostEqual( + actual_v_area, + expected_v_area, + places=2, + msg=f"Vertical area calculation failed for rotated component. Got {actual_v_area}", + ) + + def test_moved_component_subtraction(self): + """Verify subtractions align correctly for moved/rotated components.""" + self.printTestMessage("moved component boolean subtraction...") + + # Create a base component and move it 5 meters away + base_box = self.document.addObject("Part::Box", "BaseBox") + base_box.Length = base_box.Width = base_box.Height = 100.0 + comp = Arch.makeComponent(base_box) + comp.Placement.Base = App.Vector(5000.0, 0, 0) + self.document.recompute() + + initial_volume = comp.Shape.Volume # Should be 1,000,000 mm^3 + + # Create a "Cutter" box at the same global 5-meter position + cutter = self.document.addObject("Part::Box", "CutterBox") + cutter.Length = cutter.Width = cutter.Height = 100.0 + cutter.Placement.Base = App.Vector(5000.0, 0, 0) + self.document.recompute() + + # Add the cutter to subtractions. + # processSubShapes must inverse-transform the cutter into the component's local space. + comp.Subtractions = [cutter] + self.document.recompute() + + # Assert: If the fix works, the volumes overlap perfectly and result is 0. + # If the fix fails, the cutter is ignored because it looks for the box at the origin. + final_volume = comp.Shape.Volume + self.assertLess( + final_volume, + initial_volume, + "Subtraction failed. The global cutter did not intersect the moved local shape.", + ) + self.assertAlmostEqual(final_volume, 0.0, places=5) + + def test_apply_shape_spread(self): + """Ensure generic components handle spreading (automatic arraying) via the Axis property.""" + self.printTestMessage("applyShape spread logic (generic component)...") + + # Create base geometry at identity (0,0,0) + box = self.document.addObject("Part::Box", "SpreadBase") + box.Length = box.Width = box.Height = 100.0 + self.document.recompute() + + # Create a generic Arch Component + comp = Arch.makeComponent(box) + + # Create an Axis system with 2 points (at 0 and 2000mm) + axis = Arch.makeAxis(num=2, size=2000) + self.document.recompute() + + # Link the axis to the component + comp.Axis = axis + self.document.recompute() + + # Verify that the resulting shape contains 2 instances (solids) + # This confirms that the execute() loop correctly processes the Axis property. + self.assertEqual( + len(comp.Shape.Solids), + 2, + "Generic Arch Component failed to spread geometry to Axis points.", + ) + + def test_component_double_transformation(self): + """Test that Arch Components do not suffer from double-transformation.""" + self.printTestMessage("ArchComponent placement and coordinate integrity...") + + # Scenario 1: translation and vertex check + with self.subTest(case="Translation Only"): + base_box = self.document.addObject("Part::Box", "BaseBoxTrans") + base_box.Length = base_box.Width = base_box.Height = 1000.0 + + # Move the box 10 meters away. Raw vertices are at 0, Shape.Placement is at 10 m. + base_box.Placement.Base = App.Vector(10000, 0, 0) + self.document.recompute() + + comp = Arch.makeComponent(base_box, name="TestTrans") + self.document.recompute() + + # The component object should match the base object's placement + self.assertEqual(comp.Placement.Base.x, 10000.0) + + # Verification of localization: + # Visual Center = Object.Placement (10000) + Shape.Center (500) = 10500. + # If the bug were present (double transform), it would be 20500. + actual_center_x = comp.Shape.BoundBox.Center.x + self.assertAlmostEqual( + actual_center_x, + 10500.0, + places=3, + msg="Double transformation detected! Object is offset twice.", + ) + + # Scenario 2: CSG alignment (Additions) + with self.subTest(case="CSG Alignment"): + # Base box (1m cube) moved to 5m + base_box_csg = self.document.addObject("Part::Box", "BaseBoxCSG") + base_box_csg.Length = base_box_csg.Width = base_box_csg.Height = 1000.0 + base_box_csg.Placement.Base = App.Vector(5000, 0, 0) + + comp_csg = Arch.makeComponent(base_box_csg, name="TestCSG") + self.document.recompute() + + # Addition box (identical 1m cube) at exactly the same global location (5m) + # They should overlap perfectly. + add_box = self.document.addObject("Part::Box", "AdditionBox") + add_box.Length = add_box.Width = add_box.Height = 1000.0 + add_box.Placement.Base = App.Vector(5000, 0, 0) + if App.GuiUp: + add_box.ViewObject.hide() + + comp_csg.Additions = [add_box] + self.document.recompute() + + # If sanitized, they overlap perfectly: Total Volume = 1,000,000,000 mm3 + # If not sanitized, they would be 5m apart: Volume = 2,000,000,000 mm3 + self.assertAlmostEqual( + comp_csg.Shape.Volume, + 1000000000.0, + delta=100.0, + msg="CSG pieces did not align. Base shape likely retained the offset.", + ) + + def test_component_base_removal_cleanup(self): + """Test that removing the Base property clears the component shape.""" + self.printTestMessage("ArchComponent Base Removal Cleanup...") + + box = self.document.addObject("Part::Box", "StaleTestBox") + comp = Arch.makeComponent(box) + self.document.recompute() + + self.assertFalse(comp.Shape.isNull(), "Component should have a shape initially.") + + # Trigger the 'else' block + comp.Base = None + self.document.recompute() + + self.assertTrue( + comp.Shape.isNull(), "Component retained a stale shape after its Base was removed." + )