diff --git a/src/Mod/Assembly/App/AssemblyUtils.cpp b/src/Mod/Assembly/App/AssemblyUtils.cpp index 6ea4902727..21e70455f2 100644 --- a/src/Mod/Assembly/App/AssemblyUtils.cpp +++ b/src/Mod/Assembly/App/AssemblyUtils.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -197,6 +198,120 @@ double getEdgeRadius(const App::DocumentObject* obj, const std::string& elt) return sf.GetType() == GeomAbs_Circle ? sf.Circle().Radius() : 0.0; } +/// Determine whether \a obj represents a planar datum when referenced with an +/// empty element type (bare sub-name ending with "."). +/// +/// Covers three independent class hierarchies: +/// 1. App::Plane (origin planes, Part::DatumPlane) +/// 2. Part::Datum (PartDesign::Plane — not derived from App::Plane) +/// 3. Any Part::Feature whose whole-object shape is a single planar face +/// (e.g. Part::Plane primitive referenced without an element) +static bool isDatumPlane(const App::DocumentObject* obj) +{ + if (!obj) { + return false; + } + + // Origin planes and Part::DatumPlane (both inherit App::Plane). + if (obj->isDerivedFrom()) { + return true; + } + + // PartDesign datum objects inherit Part::Datum but NOT App::Plane. + // Part::Datum is also the base for PartDesign::Line and PartDesign::Point, + // so inspect the shape to confirm it is actually planar. + if (obj->isDerivedFrom()) { + auto* feat = static_cast(obj); + const auto& shape = feat->Shape.getShape().getShape(); + if (shape.IsNull()) { + return false; + } + TopExp_Explorer ex(shape, TopAbs_FACE); + if (ex.More()) { + BRepAdaptor_Surface sf(TopoDS::Face(ex.Current())); + return sf.GetType() == GeomAbs_Plane; + } + return false; + } + + // Fallback for any Part::Feature (e.g. Part::Plane primitive) referenced + // bare — if its shape is a single planar face, treat it as a datum plane. + if (auto* feat = dynamic_cast(obj)) { + const auto& shape = feat->Shape.getShape().getShape(); + if (shape.IsNull()) { + return false; + } + TopExp_Explorer ex(shape, TopAbs_FACE); + if (!ex.More()) { + return false; + } + BRepAdaptor_Surface sf(TopoDS::Face(ex.Current())); + if (sf.GetType() != GeomAbs_Plane) { + return false; + } + ex.Next(); + // Only treat as datum if there is exactly one face — a multi-face + // solid referenced bare is ambiguous and should not be classified. + return !ex.More(); + } + + return false; +} + +/// Same idea for datum lines (App::Line, PartDesign::Line, etc.). +static bool isDatumLine(const App::DocumentObject* obj) +{ + if (!obj) { + return false; + } + + if (obj->isDerivedFrom()) { + return true; + } + + if (obj->isDerivedFrom()) { + auto* feat = static_cast(obj); + const auto& shape = feat->Shape.getShape().getShape(); + if (shape.IsNull()) { + return false; + } + TopExp_Explorer ex(shape, TopAbs_EDGE); + if (ex.More()) { + BRepAdaptor_Curve cv(TopoDS::Edge(ex.Current())); + return cv.GetType() == GeomAbs_Line; + } + return false; + } + + return false; +} + +/// Same idea for datum points (App::Point, PartDesign::Point, etc.). +static bool isDatumPoint(const App::DocumentObject* obj) +{ + if (!obj) { + return false; + } + + if (obj->isDerivedFrom()) { + return true; + } + + if (obj->isDerivedFrom()) { + auto* feat = static_cast(obj); + const auto& shape = feat->Shape.getShape().getShape(); + if (shape.IsNull()) { + return false; + } + // A datum point has a vertex but no edges or faces. + TopExp_Explorer exE(shape, TopAbs_EDGE); + TopExp_Explorer exV(shape, TopAbs_VERTEX); + return !exE.More() && exV.More(); + } + + return false; +} + DistanceType getDistanceType(App::DocumentObject* joint) { if (!joint) { @@ -210,54 +325,177 @@ DistanceType getDistanceType(App::DocumentObject* joint) auto* obj1 = getLinkedObjFromRef(joint, "Reference1"); auto* obj2 = getLinkedObjFromRef(joint, "Reference2"); - // Datum planes (App::Plane) have empty element types because their - // sub-name ends with "." and yields no Face/Edge/Vertex element. - // Detect them here and classify before the main geometry chain, - // which cannot handle the empty element type. - const bool datum1 = type1.empty() && obj1 && obj1->isDerivedFrom(); - const bool datum2 = type2.empty() && obj2 && obj2->isDerivedFrom(); + // Datum objects referenced bare have empty element types (sub-name + // ends with "."). PartDesign datums referenced through a body can + // also produce non-standard element types like "Plane" (from a + // sub-name such as "Body.DatumPlane.Plane" — Part::Datum::getSubObject + // ignores the trailing element, but splitSubName still extracts it). + // + // Detect these before the main geometry chain, which only handles + // the standard Face/Edge/Vertex element types. + // + // isDatumPlane/Line/Point cover all three independent hierarchies: + // - App::Plane / App::Line / App::Point (origin datums) + // - Part::Datum subclasses (PartDesign datums) + // - Part::Feature with single-face shape (Part::Plane primitive, bare ref) + auto isNonGeomElement = [](const std::string& t) { + return t != "Face" && t != "Edge" && t != "Vertex"; + }; + const bool datumPlane1 = isNonGeomElement(type1) && isDatumPlane(obj1); + const bool datumPlane2 = isNonGeomElement(type2) && isDatumPlane(obj2); + const bool datumLine1 = isNonGeomElement(type1) && !datumPlane1 && isDatumLine(obj1); + const bool datumLine2 = isNonGeomElement(type2) && !datumPlane2 && isDatumLine(obj2); + const bool datumPoint1 = isNonGeomElement(type1) && !datumPlane1 && !datumLine1 && isDatumPoint(obj1); + const bool datumPoint2 = isNonGeomElement(type2) && !datumPlane2 && !datumLine2 && isDatumPoint(obj2); + const bool datum1 = datumPlane1 || datumLine1 || datumPoint1; + const bool datum2 = datumPlane2 || datumLine2 || datumPoint2; if (datum1 || datum2) { - if (datum1 && datum2) { + // Map each datum side to a synthetic element type so the same + // classification logic applies regardless of which hierarchy + // the object comes from. + auto syntheticType = [](bool isPlane, bool isLine, bool isPoint, + const std::string& elemType) -> std::string { + if (isPlane) return "Face"; + if (isLine) return "Edge"; + if (isPoint) return "Vertex"; + return elemType; // non-datum side keeps its real type + }; + + const std::string syn1 = syntheticType(datumPlane1, datumLine1, datumPoint1, type1); + const std::string syn2 = syntheticType(datumPlane2, datumLine2, datumPoint2, type2); + + // Both sides are datum planes. + if (datumPlane1 && datumPlane2) { FC_LOG("Assembly : getDistanceType('" << joint->getFullName() << "') — datum+datum → PlanePlane"); return DistanceType::PlanePlane; } - // One side is a datum plane, the other has a real element type. + // One side is a datum plane, the other has a real element type + // (or is another datum kind). // For PointPlane/LinePlane, the solver's PointInPlaneConstraint // reads the plane normal from marker_j (Reference2). Unlike // real Face+Vertex joints (where both Placements carry the // face normal from findPlacement), datum planes only carry // their normal through computeMarkerTransform. So the datum - // must end up on Reference2 for the normal to reach marker_j. + // plane must end up on Reference2 for the normal to reach marker_j. // // For PlanePlane the convention matches the existing Face+Face // path (plane on Reference1). - const auto& otherType = datum1 ? type2 : type1; + if (datumPlane1 || datumPlane2) { + const auto& otherSyn = datumPlane1 ? syn2 : syn1; - if (otherType == "Vertex" || otherType == "Edge") { - // Datum must be on Reference2 (j side). - if (datum1) { - swapJCS(joint); // move datum from Ref1 → Ref2 + if (otherSyn == "Vertex" || otherSyn == "Edge") { + // Datum plane must be on Reference2 (j side). + if (datumPlane1) { + swapJCS(joint); // move datum from Ref1 → Ref2 + } + DistanceType result = (otherSyn == "Vertex") + ? DistanceType::PointPlane : DistanceType::LinePlane; + FC_LOG("Assembly : getDistanceType('" << joint->getFullName() + << "') — datum+" << otherSyn << " → " + << distanceTypeName(result) + << (datumPlane1 ? " (swapped)" : "")); + return result; } - DistanceType result = (otherType == "Vertex") - ? DistanceType::PointPlane : DistanceType::LinePlane; + + // Face + datum plane or datum plane + datum plane → PlanePlane. + // No swap needed: PlanarConstraint is symmetric (uses both + // z_i and z_j), and preserving the original Reference order + // keeps the initial Placement values consistent so the solver + // stays in the correct orientation branch. FC_LOG("Assembly : getDistanceType('" << joint->getFullName() - << "') — datum+" << otherType << " → " - << distanceTypeName(result) - << (datum1 ? " (swapped)" : "")); - return result; + << "') — datum+" << otherSyn << " → PlanePlane"); + return DistanceType::PlanePlane; } - // Face + datum or unknown + datum → PlanePlane. - // No swap needed: PlanarConstraint is symmetric (uses both - // z_i and z_j), and preserving the original Reference order - // keeps the initial Placement values consistent so the solver - // stays in the correct orientation branch. - FC_LOG("Assembly : getDistanceType('" << joint->getFullName() - << "') — datum+" << otherType << " → PlanePlane"); - return DistanceType::PlanePlane; + // Datum line or datum point paired with a real element type. + // Map to the appropriate pair using synthetic types and fall + // through to the main geometry chain below. The synthetic + // types ("Edge", "Vertex") will match the existing if-else + // branches — but those branches call isEdgeType/isFaceType on + // the object, which requires a real sub-element name. For + // datum lines/points the element is empty, so we classify + // directly here. + if (datumLine1 && datumLine2) { + FC_LOG("Assembly : getDistanceType('" << joint->getFullName() + << "') — datumLine+datumLine → LineLine"); + return DistanceType::LineLine; + } + if (datumPoint1 && datumPoint2) { + FC_LOG("Assembly : getDistanceType('" << joint->getFullName() + << "') — datumPoint+datumPoint → PointPoint"); + return DistanceType::PointPoint; + } + if ((datumLine1 && datumPoint2) || (datumPoint1 && datumLine2)) { + if (datumPoint1) { + swapJCS(joint); // line first + } + FC_LOG("Assembly : getDistanceType('" << joint->getFullName() + << "') — datumLine+datumPoint → PointLine"); + return DistanceType::PointLine; + } + + // One datum line/point + one real element type. + if (datumLine1 || datumLine2) { + const auto& otherSyn = datumLine1 ? syn2 : syn1; + if (otherSyn == "Face") { + // Line + Face — need line on Reference2 (edge side). + if (datumLine1) { + swapJCS(joint); + } + // We don't know the face type without inspecting the shape, + // but LinePlane is the most common and safest classification. + FC_LOG("Assembly : getDistanceType('" << joint->getFullName() + << "') — datumLine+Face → LinePlane"); + return DistanceType::LinePlane; + } + if (otherSyn == "Vertex") { + if (datumLine2) { + swapJCS(joint); // line first + } + FC_LOG("Assembly : getDistanceType('" << joint->getFullName() + << "') — datumLine+Vertex → PointLine"); + return DistanceType::PointLine; + } + if (otherSyn == "Edge") { + FC_LOG("Assembly : getDistanceType('" << joint->getFullName() + << "') — datumLine+Edge → LineLine"); + return DistanceType::LineLine; + } + } + if (datumPoint1 || datumPoint2) { + const auto& otherSyn = datumPoint1 ? syn2 : syn1; + if (otherSyn == "Face") { + // Point + Face — face first, point second. + if (!datumPoint2) { + swapJCS(joint); // put face on Ref1 + } + FC_LOG("Assembly : getDistanceType('" << joint->getFullName() + << "') — datumPoint+Face → PointPlane"); + return DistanceType::PointPlane; + } + if (otherSyn == "Edge") { + // Edge first, point second. + if (datumPoint1) { + swapJCS(joint); + } + FC_LOG("Assembly : getDistanceType('" << joint->getFullName() + << "') — datumPoint+Edge → PointLine"); + return DistanceType::PointLine; + } + if (otherSyn == "Vertex") { + FC_LOG("Assembly : getDistanceType('" << joint->getFullName() + << "') — datumPoint+Vertex → PointPoint"); + return DistanceType::PointPoint; + } + } + + // If we get here, it's an unrecognized datum combination. + FC_WARN("Assembly : getDistanceType('" << joint->getFullName() + << "') — unrecognized datum combination (syn1=" + << syn1 << ", syn2=" << syn2 << ")"); } if (type1 == "Vertex" && type2 == "Vertex") { diff --git a/src/Mod/Assembly/AssemblyTests/TestDatumClassification.py b/src/Mod/Assembly/AssemblyTests/TestDatumClassification.py new file mode 100644 index 0000000000..96b95331c7 --- /dev/null +++ b/src/Mod/Assembly/AssemblyTests/TestDatumClassification.py @@ -0,0 +1,266 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# /**************************************************************************** +# * +# Copyright (c) 2025 Kindred Systems * +# * +# This file is part of FreeCAD. * +# * +# FreeCAD is free software: you can redistribute it and/or modify it * +# under the terms of the GNU Lesser General Public License as * +# published by the Free Software Foundation, either version 2.1 of the * +# License, or (at your option) any later version. * +# * +# FreeCAD is distributed in the hope that it will be useful, but * +# WITHOUT ANY WARRANTY; without even the implied warranty of * +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * +# Lesser General Public License for more details. * +# * +# You should have received a copy of the GNU Lesser General Public * +# License along with FreeCAD. If not, see * +# . * +# * +# ***************************************************************************/ + +""" +Tests for datum plane classification in Distance joints. + +Verifies that getDistanceType correctly classifies joints involving datum +planes from all three class hierarchies: + 1. App::Plane — origin planes (XY, XZ, YZ) + 2. PartDesign::Plane — datum planes inside a PartDesign body + 3. Part::Plane — Part workbench plane primitives (bare reference) +""" + +import unittest + +import FreeCAD as App +import JointObject + + +class TestDatumClassification(unittest.TestCase): + """Tests that Distance joints with datum plane references are + classified as PlanePlane (not Other) regardless of the datum + object's class hierarchy.""" + + def setUp(self): + doc_name = self.__class__.__name__ + if App.ActiveDocument: + if App.ActiveDocument.Name != doc_name: + App.newDocument(doc_name) + else: + App.newDocument(doc_name) + App.setActiveDocument(doc_name) + self.doc = App.ActiveDocument + + self.assembly = self.doc.addObject("Assembly::AssemblyObject", "Assembly") + self.jointgroup = self.assembly.newObject("Assembly::JointGroup", "Joints") + + def tearDown(self): + App.closeDocument(self.doc.Name) + + # ── Helpers ───────────────────────────────────────────────────── + + def _make_box(self, x=0, y=0, z=0, size=10): + box = self.assembly.newObject("Part::Box", "Box") + box.Length = size + box.Width = size + box.Height = size + box.Placement = App.Placement(App.Vector(x, y, z), App.Rotation()) + return box + + def _make_joint(self, joint_type, ref1, ref2): + joint = self.jointgroup.newObject("App::FeaturePython", "Joint") + JointObject.Joint(joint, joint_type) + refs = [ + [ref1[0], ref1[1]], + [ref2[0], ref2[1]], + ] + joint.Proxy.setJointConnectors(joint, refs) + return joint + + def _make_pd_body_with_datum_plane(self, name="Body"): + """Create a PartDesign::Body with a datum plane inside the assembly.""" + body = self.assembly.newObject("PartDesign::Body", name) + datum = body.newObject("PartDesign::Plane", "DatumPlane") + self.doc.recompute() + return body, datum + + def _make_part_plane(self, name="PartPlane"): + """Create a Part::Plane primitive inside the assembly.""" + plane = self.assembly.newObject("Part::Plane", name) + plane.Length = 10 + plane.Width = 10 + self.doc.recompute() + return plane + + # ── Origin plane tests (App::Plane — existing behaviour) ─────── + + def test_origin_plane_face_classified_as_plane_plane(self): + """Distance joint: box Face → origin datum plane → PlanePlane.""" + origin = self.assembly.Origin + xy = origin.getXY() + box = self._make_box(0, 0, 50) + + joint = self._make_joint( + 5, # Distance + [box, ["Face1", "Vertex1"]], + [origin, [xy.Name + ".", xy.Name + "."]], + ) + joint.Distance = 0.0 + + result = self.assembly.solve() + self.assertEqual( + result, + 0, + "Distance joint with origin plane should solve (not produce Other)", + ) + + # ── PartDesign::Plane tests ──────────────────────────────────── + + def test_pd_datum_plane_face_classified_as_plane_plane(self): + """Distance joint: box Face → PartDesign::Plane → PlanePlane.""" + body, datum = self._make_pd_body_with_datum_plane() + box = self._make_box(0, 0, 50) + + # Ground the body so the solver has a fixed reference. + gnd = self.jointgroup.newObject("App::FeaturePython", "GroundedJoint") + JointObject.GroundedJoint(gnd, body) + + # Reference the datum plane with a bare sub-name (ends with "."). + joint = self._make_joint( + 5, # Distance + [box, ["Face1", "Vertex1"]], + [body, [datum.Name + ".", datum.Name + "."]], + ) + joint.Distance = 0.0 + + result = self.assembly.solve() + self.assertNotEqual( + result, + -1, + "Distance joint with PartDesign::Plane should not fail to solve " + "(DistanceType should be PlanePlane, not Other)", + ) + + def test_pd_datum_plane_vertex_classified_as_point_plane(self): + """Distance joint: box Vertex → PartDesign::Plane → PointPlane.""" + body, datum = self._make_pd_body_with_datum_plane() + box = self._make_box(0, 0, 50) + + gnd = self.jointgroup.newObject("App::FeaturePython", "GroundedJoint") + JointObject.GroundedJoint(gnd, body) + + joint = self._make_joint( + 5, # Distance + [box, ["Vertex1", "Vertex1"]], + [body, [datum.Name + ".", datum.Name + "."]], + ) + joint.Distance = 0.0 + + result = self.assembly.solve() + self.assertNotEqual( + result, + -1, + "Distance joint vertex → PartDesign::Plane should not fail " + "(DistanceType should be PointPlane, not Other)", + ) + + def test_two_pd_datum_planes_classified_as_plane_plane(self): + """Distance joint: PartDesign::Plane → PartDesign::Plane → PlanePlane.""" + body1, datum1 = self._make_pd_body_with_datum_plane("Body1") + body2, datum2 = self._make_pd_body_with_datum_plane("Body2") + + gnd = self.jointgroup.newObject("App::FeaturePython", "GroundedJoint") + JointObject.GroundedJoint(gnd, body1) + + joint = self._make_joint( + 5, # Distance + [body1, [datum1.Name + ".", datum1.Name + "."]], + [body2, [datum2.Name + ".", datum2.Name + "."]], + ) + joint.Distance = 0.0 + + result = self.assembly.solve() + self.assertNotEqual( + result, + -1, + "Distance joint PartDesign::Plane → PartDesign::Plane should not fail " + "(DistanceType should be PlanePlane, not Other)", + ) + + # ── Part::Plane tests (primitive, bare reference) ────────────── + + def test_part_plane_bare_ref_face_classified_as_plane_plane(self): + """Distance joint: box Face → Part::Plane (bare ref) → PlanePlane.""" + plane = self._make_part_plane() + box = self._make_box(0, 0, 50) + + gnd = self.jointgroup.newObject("App::FeaturePython", "GroundedJoint") + JointObject.GroundedJoint(gnd, plane) + + # Bare reference to Part::Plane (sub-name ends with "."). + joint = self._make_joint( + 5, # Distance + [box, ["Face1", "Vertex1"]], + [plane, [".", "."]], + ) + joint.Distance = 0.0 + + result = self.assembly.solve() + self.assertNotEqual( + result, + -1, + "Distance joint with Part::Plane (bare ref) should not fail " + "(DistanceType should be PlanePlane, not Other)", + ) + + def test_part_plane_with_face1_classified_as_plane_plane(self): + """Distance joint: box Face → Part::Plane Face1 → PlanePlane. + + When Part::Plane is referenced with an explicit Face1 element, + it should enter the normal Face+Face classification path.""" + plane = self._make_part_plane() + box = self._make_box(0, 0, 50) + + gnd = self.jointgroup.newObject("App::FeaturePython", "GroundedJoint") + JointObject.GroundedJoint(gnd, plane) + + joint = self._make_joint( + 5, # Distance + [box, ["Face1", "Vertex1"]], + [plane, ["Face1", "Vertex1"]], + ) + joint.Distance = 0.0 + + result = self.assembly.solve() + self.assertNotEqual( + result, + -1, + "Distance joint with Part::Plane Face1 should solve normally", + ) + + # ── Cross-hierarchy tests ────────────────────────────────────── + + def test_origin_plane_and_pd_datum_classified_as_plane_plane(self): + """Distance joint: origin App::Plane → PartDesign::Plane → PlanePlane.""" + origin = self.assembly.Origin + xy = origin.getXY() + body, datum = self._make_pd_body_with_datum_plane() + + gnd = self.jointgroup.newObject("App::FeaturePython", "GroundedJoint") + JointObject.GroundedJoint(gnd, body) + + joint = self._make_joint( + 5, # Distance + [origin, [xy.Name + ".", xy.Name + "."]], + [body, [datum.Name + ".", datum.Name + "."]], + ) + joint.Distance = 0.0 + + result = self.assembly.solve() + self.assertNotEqual( + result, + -1, + "Distance joint origin plane → PartDesign::Plane should not fail " + "(DistanceType should be PlanePlane, not Other)", + ) diff --git a/src/Mod/Assembly/CMakeLists.txt b/src/Mod/Assembly/CMakeLists.txt index 80d1c8efc5..a4468a51f7 100644 --- a/src/Mod/Assembly/CMakeLists.txt +++ b/src/Mod/Assembly/CMakeLists.txt @@ -61,6 +61,7 @@ SET(AssemblyTests_SRCS AssemblyTests/TestKindredSolverIntegration.py AssemblyTests/TestKCSolvePy.py AssemblyTests/TestAssemblyOriginPlanes.py + AssemblyTests/TestDatumClassification.py AssemblyTests/mocks/__init__.py AssemblyTests/mocks/MockGui.py )