diff --git a/src/Mod/Part/App/AttachExtension.cpp b/src/Mod/Part/App/AttachExtension.cpp
index a9051e64ac..0736dd56ff 100644
--- a/src/Mod/Part/App/AttachExtension.cpp
+++ b/src/Mod/Part/App/AttachExtension.cpp
@@ -26,6 +26,12 @@
#include
#include
+#include
+#include
+#include
+#include
+#include
+
#include "AttachExtension.h"
#include "AttachExtensionPy.h"
@@ -479,6 +485,162 @@ bool AttachExtension::extensionHandleChangedPropertyName(Base::XMLReader& reader
return App::DocumentObjectExtension::extensionHandleChangedPropertyName(reader, TypeName, PropName);
}
+void AttachExtension::handleLegacyTangentPlaneOrientation()
+{
+ // check attachment mode
+ if (_props.attacher->mapMode != mmTangentPlane) {
+ return;
+ }
+
+ // check stored document program version (applies to v1.0 and earlier only)
+ int major, minor;
+ if (sscanf(getExtendedObject()->getDocument()->getProgramVersion(), "%d.%d", &major, &minor) != 2) {
+ return;
+ }
+ if (major > 1 || (major == 1 && minor > 0)) {
+ return;
+ }
+
+ // check for an App::Plane support object exists
+ App::GeoFeature* geof = nullptr;
+ for (auto obj : this->AttachmentSupport.linkedObjects()) {
+ if (obj->isDerivedFrom()) {
+ geof = freecad_cast(obj);
+ break;
+ }
+ }
+ if (!geof) {
+ return;
+ }
+
+ /*
+ * determine which global axis (X/Y/Z) is closest to the plane normal
+ */
+
+ // extract the plane normal
+ Base::Vector3d norm;
+ geof->Placement.getValue().getRotation().multVec(Base::Vector3d(0.0, 0.0, 1.0), norm);
+ gp_Dir normal(norm.x, norm.y, norm.z);
+
+ // determine the dominant global axis
+ std::array cosXYZ = {
+ fabs(normal.Dot(gp_Dir(1, 0, 0))), // normal dot global X axis
+ fabs(normal.Dot(gp_Dir(0, 1, 0))), // normal dot global Y axis
+ fabs(normal.Dot(gp_Dir(0, 0, 1))) // normal dot global Z axis
+ };
+ std::size_t axis = std::distance(cosXYZ.begin(), std::ranges::max_element(cosXYZ));
+
+ // if Z is dominant, no changes needed
+ if (axis == 2) {
+ return;
+ }
+
+ /*
+ * correct attachment offset position and rotation based on dominant axis
+ */
+
+ App::DocumentObject* owner = getExtendedObject();
+ Base::Console().message("Converting attachment offset of %s\n",
+ owner->getNameInDocument());
+
+ try {
+ // extract current attachment offset values
+ Base::Placement placement = this->AttachmentOffset.getValue();
+ Base::Vector3d position = placement.getPosition();
+ Base::Rotation rotation = placement.getRotation();
+ double yaw, pitch, roll;
+ rotation.getYawPitchRoll(yaw, pitch, roll);
+
+ // extract existing expressions
+ auto buildOID = [owner](const std::vector& path) {
+ App::ObjectIdentifier oid(owner, true);
+ for (const auto& name : path) {
+ oid.addComponent(App::ObjectIdentifier::SimpleComponent(name));
+ }
+ return oid;
+ };
+ App::ObjectIdentifier oidX = buildOID ({"AttachmentOffset", "Base", "x"});
+ App::ObjectIdentifier oidY = buildOID ({"AttachmentOffset", "Base", "y"});
+ App::ObjectIdentifier oidYaw = buildOID ({"AttachmentOffset", "Rotation", "Yaw"});
+ const App::Expression* exprX = nullptr;
+ const App::Expression* exprY = nullptr;
+ const App::Expression* exprYaw = nullptr;
+ for (const auto& [oid, expr] : owner->ExpressionEngine.getExpressions()) {
+ if (oid == oidX) {
+ exprX = expr;
+ } else if (oid == oidY) {
+ exprY = expr;
+ } else if (oid == oidYaw) {
+ exprYaw = expr;
+ }
+ }
+
+ // convert placement and expressions according to the dominant axis
+ App::Expression* newExprX = nullptr;
+ App::Expression* newExprY = nullptr;
+ App::Expression* newExprYaw = nullptr;
+ if (axis == 0) { // normal mostly X
+ // values
+ std::swap(position.x, position.y);
+ position.x = -position.x;
+ rotation.setYawPitchRoll(yaw + 90, pitch, roll);
+
+ // expressions
+ if (exprX) {
+ newExprY = App::ExpressionParser::parse(owner, exprX->toString().c_str());
+ }
+ if (exprY) {
+ std::string expr = "-1 * (" + exprY->toString() + ")";
+ newExprX = App::ExpressionParser::parse(owner, expr.c_str());
+ }
+ if (exprYaw) {
+ std::string expr = "(" + exprYaw->toString() + ") + 90 deg";
+ newExprYaw = App::ExpressionParser::parse(owner, expr.c_str());
+ }
+ }
+ else if (axis == 1) { // normal mostly Y
+ // values
+ std::swap(position.x, position.y);
+ position.y = -position.y;
+ rotation.setYawPitchRoll(yaw - 90, pitch, roll);
+
+ // expressions
+ if (exprX) {
+ std::string expr = "-1 * (" + exprX->toString() + ")";
+ newExprY = App::ExpressionParser::parse(owner, expr.c_str());
+ }
+ if (exprY) {
+ newExprX = App::ExpressionParser::parse(owner, exprY->toString().c_str());
+ }
+ if (exprYaw) {
+ std::string expr = "(" + exprYaw->toString() + ") - 90 deg";
+ newExprYaw = App::ExpressionParser::parse(owner, expr.c_str());
+ }
+ }
+ else {
+ // should not happen
+ return;
+ }
+
+ // store updated placement and expressions back to the document object
+
+ // expressions
+ owner->ExpressionEngine.setValue(oidX, App::ExpressionPtr(newExprX));
+ owner->ExpressionEngine.setValue(oidY, App::ExpressionPtr(newExprY));
+ owner->ExpressionEngine.setValue(oidYaw, App::ExpressionPtr(newExprYaw));
+
+ // values
+ placement.setPosition(position);
+ placement.setRotation(rotation);
+ this->AttachmentOffset.setValue(placement);
+
+ } catch (const Base::Exception& e) {
+ Base::Console().error("Error converting legacy attachment offset of %s: %s\n",
+ owner->getNameInDocument(),
+ e.what());
+ }
+}
+
void AttachExtension::onExtendedDocumentRestored()
{
try {
@@ -494,6 +656,10 @@ void AttachExtension::onExtendedDocumentRestored()
restoreAttacherEngine(this);
+ if (_props.attacher->mapMode == mmTangentPlane) {
+ handleLegacyTangentPlaneOrientation();
+ }
+
bool bAttached = positionBySupport();
updateSinglePropertyStatus(bAttached);
diff --git a/src/Mod/Part/App/AttachExtension.h b/src/Mod/Part/App/AttachExtension.h
index a7f9aa8e68..6969260e2a 100644
--- a/src/Mod/Part/App/AttachExtension.h
+++ b/src/Mod/Part/App/AttachExtension.h
@@ -147,6 +147,8 @@ protected:
App::PropertyPlacement& getPlacement() const;
void initBase(bool force);
+ void handleLegacyTangentPlaneOrientation();
+
public:
void updateAttacherVals(bool base = false) const;
// This update both _props and _baseProps if base = false
diff --git a/src/Mod/Part/CMakeLists.txt b/src/Mod/Part/CMakeLists.txt
index 293eb07660..0fe3463832 100644
--- a/src/Mod/Part/CMakeLists.txt
+++ b/src/Mod/Part/CMakeLists.txt
@@ -78,6 +78,7 @@ set(Part_tests
parttests/ColorPerFaceTest.py
parttests/ColorTransparencyTest.py
parttests/TopoShapeTest.py
+ parttests/TestTangentMode3-0.21.FCStd
)
add_custom_target(PartScripts ALL SOURCES
diff --git a/src/Mod/Part/parttests/TestTangentMode3-0.21.FCStd b/src/Mod/Part/parttests/TestTangentMode3-0.21.FCStd
new file mode 100644
index 0000000000..05ac4ae832
Binary files /dev/null and b/src/Mod/Part/parttests/TestTangentMode3-0.21.FCStd differ
diff --git a/src/Mod/Part/parttests/regression_tests.py b/src/Mod/Part/parttests/regression_tests.py
index 80cee9ec0c..d0589abb97 100644
--- a/src/Mod/Part/parttests/regression_tests.py
+++ b/src/Mod/Part/parttests/regression_tests.py
@@ -3,6 +3,10 @@
import FreeCAD
from FreeCAD import Vector, Base, newDocument, closeDocument
import Part
+
+import math
+import os
+
if "BUILD_SKETCHER" in FreeCAD.__cmake__:
import Sketcher
@@ -123,6 +127,65 @@ class RegressionTests(unittest.TestCase):
cube.recompute()
assert cube.Placement.Base.isEqual(v1,1e-6)
+ def test_TangentMode3_Issue24254(self):
+ """In FreeCAD 1.1 we changed the behavior of the mmTangentPlane, but added code to handle old files
+ that relied upon the original tangent plane behavior. This test ensures that old files open with the expected
+ tangent plane setup."""
+
+ location = os.path.dirname(os.path.realpath(__file__))
+ FreeCAD.openDocument(os.path.join(location,"TestTangentMode3-0.21.FCStd"), True)
+
+ def check_plane(name, expected_pos, expected_normal, expected_xaxis):
+ obj = FreeCAD.ActiveDocument.getObject(name)
+ if not obj:
+ FreeCAD.Console.PrintError(f"{name}: object not found\n")
+ return
+
+ pos = obj.Placement.Base
+ rot = obj.Placement.Rotation
+ normal = rot.multVec(FreeCAD.Vector(0,0,1))
+ xaxis = rot.multVec(FreeCAD.Vector(1,0,0))
+
+ # position
+ self.assertAlmostEqual((pos - expected_pos).Length, 0)
+
+ # normal
+ self.assertAlmostEqual(normal.getAngle(expected_normal), 0)
+
+ # tangent x-axis
+ self.assertAlmostEqual(xaxis.getAngle(expected_xaxis), 0)
+
+ r = 10.0
+ rad30 = math.radians(30)
+ cos30 = math.cos(rad30)
+ sin30 = math.sin(rad30)
+
+ expected_planes = [
+ ("Sketch001",
+ FreeCAD.Vector(-1 * r, 0, 0), # position
+ FreeCAD.Vector(1, 0, 0), # normal
+ FreeCAD.Vector(0, -1, 0)), # tangent x-axis
+ ("Sketch002",
+ FreeCAD.Vector(sin30 * r,
+ cos30 * cos30 * (-r),
+ cos30 * sin30 * r),
+ FreeCAD.Vector(sin30,
+ cos30 * cos30 * (-1),
+ cos30 * sin30),
+ FreeCAD.Vector(0, sin30, cos30)),
+ ("Sketch003",
+ FreeCAD.Vector(cos30 * cos30 * r,
+ sin30 * r,
+ cos30 * sin30 * r),
+ FreeCAD.Vector(cos30 * cos30,
+ sin30,
+ cos30 * sin30),
+ FreeCAD.Vector(sin30, 0, -cos30)),
+ ]
+
+ for name, pos, normal, xaxis in expected_planes:
+ check_plane(name, pos, normal, xaxis)
+
def tearDown(self):
"""Clean up our test, optionally preserving the test document"""
# This flag allows doing something like this: