diff --git a/src/Mod/Part/App/TopoShapeExpansion.cpp b/src/Mod/Part/App/TopoShapeExpansion.cpp index 2fcd2a890f..9492107fc3 100644 --- a/src/Mod/Part/App/TopoShapeExpansion.cpp +++ b/src/Mod/Part/App/TopoShapeExpansion.cpp @@ -3990,9 +3990,10 @@ TopoShape& TopoShape::makeElementMirror(const TopoShape& shape, const gp_Ax2& ax } gp_Trsf mat; mat.SetMirror(ax2); - TopLoc_Location loc = shape.getShape().Location(); - gp_Trsf placement = loc.Transformation(); - mat = placement * mat; + // Note: Do NOT extract and pre-multiply the shape's Location/Placement here. + // BRepBuilderAPI_Transform correctly handles shapes with Location already. + // Pre-multiplying would double-apply the placement, causing incorrect results + // for shapes with non-identity Placement. See GitHub issue #20834. BRepBuilderAPI_Transform mkTrf(shape.getShape(), mat); return makeElementShape(mkTrf, shape, op); } diff --git a/src/Mod/Part/parttests/TopoShapeTest.py b/src/Mod/Part/parttests/TopoShapeTest.py index 0cc78f25e7..0ad82fa195 100644 --- a/src/Mod/Part/parttests/TopoShapeTest.py +++ b/src/Mod/Part/parttests/TopoShapeTest.py @@ -607,6 +607,76 @@ class TopoShapeTest(unittest.TestCase, TopoShapeAssertions): if mirror.ElementMapVersion != "": # Should be '4' as of Mar 2023. self.assertEqual(mirror.ElementMapSize, 26) + def testTopoShapeMirrorWithPlacement(self): + """Test that mirror() produces identical results regardless of how the + shape is positioned - via direct coordinates or via Placement. + Regression test for GitHub issue #20834. + + The bug was: when a shape has a non-identity Location (Placement), + the mirror result was incorrect because the placement was being + double-applied in makeElementMirror(). + """ + # Create two identical boxes at the same visual location using different methods: + # Method 1: Box geometry positioned directly at (0, 30, 0), identity Placement + box_direct = Part.makeBox(10, 20, 30, App.Vector(0, 30, 0)) + + # Method 2: Box geometry at origin, then moved via Placement + box_placed = Part.makeBox(10, 20, 30) + box_placed.Placement = App.Placement(App.Vector(0, 30, 0), App.Rotation()) + + # Verify both boxes appear at the same location + self.assertAlmostEqual(box_direct.BoundBox.XMin, box_placed.BoundBox.XMin, places=5) + self.assertAlmostEqual(box_direct.BoundBox.YMin, box_placed.BoundBox.YMin, places=5) + self.assertAlmostEqual(box_direct.BoundBox.ZMin, box_placed.BoundBox.ZMin, places=5) + + # Mirror both across the XZ plane (Y=0) + # A point (x, y, z) mirrors to (x, -y, z) + # So box at Y=30..50 should mirror to Y=-50..-30 + mirror_direct = box_direct.mirror(App.Vector(), App.Vector(0, 1, 0)) + mirror_placed = box_placed.mirror(App.Vector(), App.Vector(0, 1, 0)) + + # The mirrored shapes should have identical bounding boxes + self.assertAlmostEqual( + mirror_direct.BoundBox.XMin, + mirror_placed.BoundBox.XMin, + places=5, + msg="Mirror with Placement produced different XMin", + ) + self.assertAlmostEqual( + mirror_direct.BoundBox.YMin, + mirror_placed.BoundBox.YMin, + places=5, + msg="Mirror with Placement produced different YMin", + ) + self.assertAlmostEqual( + mirror_direct.BoundBox.ZMin, + mirror_placed.BoundBox.ZMin, + places=5, + msg="Mirror with Placement produced different ZMin", + ) + self.assertAlmostEqual( + mirror_direct.BoundBox.XMax, + mirror_placed.BoundBox.XMax, + places=5, + msg="Mirror with Placement produced different XMax", + ) + self.assertAlmostEqual( + mirror_direct.BoundBox.YMax, + mirror_placed.BoundBox.YMax, + places=5, + msg="Mirror with Placement produced different YMax", + ) + self.assertAlmostEqual( + mirror_direct.BoundBox.ZMax, + mirror_placed.BoundBox.ZMax, + places=5, + msg="Mirror with Placement produced different ZMax", + ) + + # Verify the expected mirror result: Y=30..50 mirrors to Y=-50..-30 + self.assertAlmostEqual(mirror_direct.BoundBox.YMin, -50.0, places=5) + self.assertAlmostEqual(mirror_direct.BoundBox.YMax, -30.0, places=5) + def testTopoShapeScale(self): # Act scale = self.doc.Box1.Shape.scaled(2)