From 17d5bca2ec3aa296228da4b67782819b5b6e9293 Mon Sep 17 00:00:00 2001 From: Chris Date: Mon, 16 Feb 2026 10:32:08 -0600 Subject: [PATCH] part: fixes #27365 mirror place copy at correct location (#27370) --- src/Mod/Part/App/FeatureMirroring.cpp | 40 ++++++++- src/Mod/Part/CMakeLists.txt | 1 + src/Mod/Part/TestPartApp.py | 1 + src/Mod/Part/parttests/TestPartMirror.py | 100 +++++++++++++++++++++++ 4 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 src/Mod/Part/parttests/TestPartMirror.py diff --git a/src/Mod/Part/App/FeatureMirroring.cpp b/src/Mod/Part/App/FeatureMirroring.cpp index 2727f6a765..f37f02e5a9 100644 --- a/src/Mod/Part/App/FeatureMirroring.cpp +++ b/src/Mod/Part/App/FeatureMirroring.cpp @@ -309,12 +309,48 @@ App::DocumentObjectExecReturn* Mirroring::execute() Base::Vector3d norm = Normal.getValue(); try { + // get shape without transform + auto shape = Feature::getTopoShape(link, ShapeOption::ResolveLink); + + // manually apply placement via setPlacement() before mirroring + if (link->isDerivedFrom(App::GeoFeature::getClassTypeId())) { + App::GeoFeature* geo = static_cast(link); + Base::Placement placement = geo->Placement.getValue(); + + if (!placement.isIdentity()) { + // Convert Placement to gp_Trsf + gp_Trsf trsf; + Base::Matrix4D mat = placement.toMatrix(); + trsf.SetValues( + mat[0][0], + mat[0][1], + mat[0][2], + mat[0][3], + mat[1][0], + mat[1][1], + mat[1][2], + mat[1][3], + mat[2][0], + mat[2][1], + mat[2][2], + mat[2][3] + ); + + // actually transform the geometry (copy=true to create new shape) + BRepBuilderAPI_Transform mkTrf(shape.getShape(), trsf, Standard_True); + shape = TopoShape(mkTrf.Shape()); + } + } + gp_Ax2 ax2(gp_Pnt(base.x, base.y, base.z), gp_Dir(norm.x, norm.y, norm.z)); - auto shape = Feature::getTopoShape(link, ShapeOption::ResolveLink | ShapeOption::Transform); + if (shape.isNull()) { Standard_Failure::Raise("Cannot mirror empty shape"); } - this->Shape.setValue(TopoShape(0).makeElementMirror(shape, ax2)); + + auto mirrored = TopoShape(0).makeElementMirror(shape, ax2); + + this->Shape.setValue(mirrored); copyMaterial(link); return Part::Feature::execute(); diff --git a/src/Mod/Part/CMakeLists.txt b/src/Mod/Part/CMakeLists.txt index 0fe3463832..8dda0a1710 100644 --- a/src/Mod/Part/CMakeLists.txt +++ b/src/Mod/Part/CMakeLists.txt @@ -79,6 +79,7 @@ set(Part_tests parttests/ColorTransparencyTest.py parttests/TopoShapeTest.py parttests/TestTangentMode3-0.21.FCStd + parttests/TestPartMirror.py ) add_custom_target(PartScripts ALL SOURCES diff --git a/src/Mod/Part/TestPartApp.py b/src/Mod/Part/TestPartApp.py index 58fe87a578..ee3e983fa8 100644 --- a/src/Mod/Part/TestPartApp.py +++ b/src/Mod/Part/TestPartApp.py @@ -35,6 +35,7 @@ from parttests.Geom2d_tests import Geom2dTests from parttests.regression_tests import RegressionTests from parttests.TopoShapeListTest import TopoShapeListTest from parttests.TopoShapeTest import TopoShapeTest +from parttests.TestPartMirror import TestPartMirroringRegression # --------------------------------------------------------------------------- diff --git a/src/Mod/Part/parttests/TestPartMirror.py b/src/Mod/Part/parttests/TestPartMirror.py new file mode 100644 index 0000000000..8b0d9f7a3c --- /dev/null +++ b/src/Mod/Part/parttests/TestPartMirror.py @@ -0,0 +1,100 @@ +""" +this test will FAIL on current main branch (with PR #26963) and PASS after the fix. +current main at: https://github.com/FreeCAD/FreeCAD/tree/24f0c8e2c321bb202410feae84eb58779ba8e8d2 +""" + +import unittest +import FreeCAD as App +import Part +import Draft + + +class TestPartMirroringRegression(unittest.TestCase): + """Regression test for GitHub issue #27365. + + Part::Mirroring with Draft Clone produces incorrect results after PR #26963. + The mirror position shifts on recompute when the source has non-identity Placement. + """ + + def setUp(self): + self.doc = App.newDocument("TestMirrorRegression") + + def tearDown(self): + App.closeDocument(self.doc.Name) + + def testMirroringWithDraftCloneStability(self): + """Test that Part::Mirroring position is stable across recomputes. + + This test reproduces issue #27365: mirroring a Draft Clone that has + placement, and causes the mirror position to be incorrect and/or shift + on recompute. + + This test FAILS on current main (after PR #26963) and should pass after a fix is applied. + """ + # create sketch at origin + sketch = self.doc.addObject("Sketcher::SketchObject", "Sketch") + sketch.addGeometry(Part.LineSegment(App.Vector(0, 0, 0), App.Vector(10, 0, 0)), False) + sketch.addGeometry(Part.LineSegment(App.Vector(10, 0, 0), App.Vector(10, 10, 0)), False) + sketch.addGeometry(Part.LineSegment(App.Vector(10, 10, 0), App.Vector(0, 10, 0)), False) + sketch.addGeometry(Part.LineSegment(App.Vector(0, 10, 0), App.Vector(0, 0, 0)), False) + self.doc.recompute() + + # create draft clone at X=30 with 90° rotation around Z + clone = Draft.make_clone(sketch) + clone.Placement = App.Placement(App.Vector(30, 0, 0), App.Rotation(App.Vector(0, 0, 1), 90)) + self.doc.recompute() + + # verify clone is positioned correctly + clone_bbox = clone.Shape.BoundBox + clone_center_x = (clone_bbox.XMin + clone_bbox.XMax) / 2 + self.assertAlmostEqual( + clone_center_x, + 30.0, + delta=5.0, + msg=f"clone should be centered around X=30, but is at X={clone_center_x:.2f}", + ) + + # mirror across YZ plane (X=0, normal in +X direction) + mirror = self.doc.addObject("Part::Mirroring", "Mirror") + mirror.Source = clone + mirror.Base = App.Vector(0, 0, 0) + mirror.Normal = App.Vector(1, 0, 0) + self.doc.recompute() + + # get initial mirror position + initial_bbox = mirror.Shape.BoundBox + initial_center_x = (initial_bbox.XMin + initial_bbox.XMax) / 2 + + # mirror should be on opposite side of plane from clone + # clone is at X≈30, plane at X=0, so mirror should be at X≈-30 + self.assertLess( + initial_center_x, + -20.0, + msg=f"mirror should be at X≈-30 (opposite side of X=0 from clone at X≈30), " + f"but is at X={initial_center_x:.2f}", + ) + + # position must be stable across recompute + # this is the core bug, ie. position shifts on recompute + for i in range(5): + self.doc.recompute() + + final_bbox = mirror.Shape.BoundBox + final_center_x = (final_bbox.XMin + final_bbox.XMax) / 2 + + # position should not change (within tolerance) + self.assertAlmostEqual( + initial_center_x, + final_center_x, + places=3, + msg=f"mirror position shifted on recompute: " + f"X={initial_center_x:.6f} -> X={final_center_x:.6f}", + ) + + +# for standalone execution +if __name__ == "__main__": + suite = unittest.TestSuite() + suite.addTest(unittest.TestLoader().loadTestsFromTestCase(TestPartMirroringRegression)) + runner = unittest.TextTestRunner() + runner.run(suite)