From 95fcaccfd807e321383be684b70a1e87f42915fe Mon Sep 17 00:00:00 2001 From: Abdullah Tahiri Date: Sat, 13 Feb 2021 15:12:28 +0100 Subject: [PATCH] Sketcher: Fix external geometry ellipse projection in parallel plane ==================================================================== When the ellipse to be projected and the sketch plane are parallel, the original code by shermelin provided for a translation of the original ellipse, which would be the best solution if it weren't because the Sketcher, internally, works under the assumption of a normal vector to the sketcher plane being (0,0,1). If the original ellipse is parallel to the sketch plane, but the sketch plane is not the XY plane, the copy and translation would result in a ellipse not in the XY plane of the Sketcher. Then the sketcher internals will not properly consider its dimensions. The solution applied here is to default to the general method for non-parallel planes. It solves: https://forum.freecadweb.org/viewtopic.php?f=3&t=55284#p477522 --- src/Mod/Sketcher/App/SketchObject.cpp | 140 +++++++++++++------------- 1 file changed, 70 insertions(+), 70 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 81ff77e0c5..ed8e0afda8 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -6591,86 +6591,86 @@ void SketchObject::rebuildExternalGeometry(void) gp_Dir origAxisMinorDir = elipsOrig.YAxis().Direction(); gp_Vec origAxisMinor = elipsOrig.MinorRadius() * gp_Vec(origAxisMinorDir); - if (sketchPlane.Position().Direction().IsParallel(elipsOrig.Position().Direction(), Precision::Angular())) { - elipsDest = elipsOrig.Translated(origCenter, destCenter); - Handle(Geom_Ellipse) curve = new Geom_Ellipse(elipsDest); - Part::GeomEllipse* ellipse = new Part::GeomEllipse(); - ellipse->setHandle(curve); - GeometryFacade::setConstruction(ellipse, true); + // Here, it used to be a test for parallel direction between the sketchplane and the elipsOrig, in which + // the original ellipse would be copied and translated to the new position. + // The problem with that approach is that for the sketcher the normal vector is always (0,0,1). If the original + // ellipse was not on the XY plane, the copy will not be either. Then, the dimensions would be wrong because of the + // different major axis direction (which is not projected on the XY plane). So here, we default to the more general + // ellipse construction algorithm. + // + // Doing that solves: + // https://forum.freecadweb.org/viewtopic.php?f=3&t=55284#p477522 - ExternalGeo.push_back(ellipse); + // GENERAL ELLIPSE CONSTRUCTION ALGORITHM + // + // look for major axis of projected ellipse + // + // t is the parameter along the origin ellipse + // OM(t) = origCenter + // + majorRadius * cos(t) * origAxisMajorDir + // + minorRadius * sin(t) * origAxisMinorDir + gp_Vec2d PA = ProjVecOnPlane_UV(origAxisMajor, sketchPlane); + gp_Vec2d PB = ProjVecOnPlane_UV(origAxisMinor, sketchPlane); + double t_max = 2.0 * PA.Dot(PB) / (PA.SquareMagnitude() - PB.SquareMagnitude()); + t_max = 0.5 * atan(t_max); // gives new major axis is most cases, but not all + double t_min = t_max + 0.5 * M_PI; + + // ON_max = OM(t_max) gives the point, which projected on the sketch plane, + // becomes the apoapse of the pojected ellipse. + gp_Vec ON_max = origAxisMajor * cos(t_max) + origAxisMinor * sin(t_max); + gp_Vec ON_min = origAxisMajor * cos(t_min) + origAxisMinor * sin(t_min); + gp_Vec destAxisMajor = ProjVecOnPlane_UVN(ON_max, sketchPlane); + gp_Vec destAxisMinor = ProjVecOnPlane_UVN(ON_min, sketchPlane); + + double RDest = destAxisMajor.Magnitude(); + double rDest = destAxisMinor.Magnitude(); + + if (RDest < rDest) { + double rTmp = rDest; + rDest = RDest; + RDest = rTmp; + gp_Vec axisTmp = destAxisMajor; + destAxisMajor = destAxisMinor; + destAxisMinor = axisTmp; + } + + double sens = sketchAx3.Direction().Dot(elipsOrig.Position().Direction()); + gp_Ax2 destCurveAx2(destCenter, + gp_Dir(0, 0, sens > 0.0 ? 1.0 : -1.0), + gp_Dir(destAxisMajor)); + + if ((RDest - rDest) < (double) Precision::Confusion()) { // projection is a circle + Handle(Geom_Circle) curve = new Geom_Circle(destCurveAx2, 0.5 * (rDest + RDest)); + Part::GeomCircle* circle = new Part::GeomCircle(); + circle->setHandle(curve); + GeometryFacade::setConstruction(circle, true); + + ExternalGeo.push_back(circle); } else { + if (sketchPlane.Position().Direction().IsNormal(elipsOrig.Position().Direction(), Precision::Angular())) { + gp_Vec start = gp_Vec(destCenter.XYZ()) + destAxisMajor; + gp_Vec end = gp_Vec(destCenter.XYZ()) - destAxisMajor; - // look for major axis of projected ellipse - // - // t is the parameter along the origin ellipse - // OM(t) = origCenter - // + majorRadius * cos(t) * origAxisMajorDir - // + minorRadius * sin(t) * origAxisMinorDir - gp_Vec2d PA = ProjVecOnPlane_UV(origAxisMajor, sketchPlane); - gp_Vec2d PB = ProjVecOnPlane_UV(origAxisMinor, sketchPlane); - double t_max = 2.0 * PA.Dot(PB) / (PA.SquareMagnitude() - PB.SquareMagnitude()); - t_max = 0.5 * atan(t_max); // gives new major axis is most cases, but not all - double t_min = t_max + 0.5 * M_PI; - - // ON_max = OM(t_max) gives the point, which projected on the sketch plane, - // becomes the apoapse of the pojected ellipse. - gp_Vec ON_max = origAxisMajor * cos(t_max) + origAxisMinor * sin(t_max); - gp_Vec ON_min = origAxisMajor * cos(t_min) + origAxisMinor * sin(t_min); - gp_Vec destAxisMajor = ProjVecOnPlane_UVN(ON_max, sketchPlane); - gp_Vec destAxisMinor = ProjVecOnPlane_UVN(ON_min, sketchPlane); - - double RDest = destAxisMajor.Magnitude(); - double rDest = destAxisMinor.Magnitude(); - - if (RDest < rDest) { - double rTmp = rDest; - rDest = RDest; - RDest = rTmp; - gp_Vec axisTmp = destAxisMajor; - destAxisMajor = destAxisMinor; - destAxisMinor = axisTmp; - } - - double sens = sketchAx3.Direction().Dot(elipsOrig.Position().Direction()); - gp_Ax2 destCurveAx2(destCenter, - gp_Dir(0, 0, sens > 0.0 ? 1.0 : -1.0), - gp_Dir(destAxisMajor)); - - if ((RDest - rDest) < (double) Precision::Confusion()) { // projection is a circle - Handle(Geom_Circle) curve = new Geom_Circle(destCurveAx2, 0.5 * (rDest + RDest)); - Part::GeomCircle* circle = new Part::GeomCircle(); - circle->setHandle(curve); - GeometryFacade::setConstruction(circle, true); - - ExternalGeo.push_back(circle); + Part::GeomLineSegment * projectedSegment = new Part::GeomLineSegment(); + projectedSegment->setPoints(Base::Vector3d(start.X(), start.Y(), start.Z()), + Base::Vector3d(end.X(), end.Y(), end.Z())); + GeometryFacade::setConstruction(projectedSegment, true); + ExternalGeo.push_back(projectedSegment); } else { - if (sketchPlane.Position().Direction().IsNormal(elipsOrig.Position().Direction(), Precision::Angular())) { - gp_Vec start = gp_Vec(destCenter.XYZ()) + destAxisMajor; - gp_Vec end = gp_Vec(destCenter.XYZ()) - destAxisMajor; - Part::GeomLineSegment * projectedSegment = new Part::GeomLineSegment(); - projectedSegment->setPoints(Base::Vector3d(start.X(), start.Y(), start.Z()), - Base::Vector3d(end.X(), end.Y(), end.Z())); - GeometryFacade::setConstruction(projectedSegment, true); - ExternalGeo.push_back(projectedSegment); - } - else { - - elipsDest.SetPosition(destCurveAx2); - elipsDest.SetMajorRadius(destAxisMajor.Magnitude()); - elipsDest.SetMinorRadius(destAxisMinor.Magnitude()); + elipsDest.SetPosition(destCurveAx2); + elipsDest.SetMajorRadius(destAxisMajor.Magnitude()); + elipsDest.SetMinorRadius(destAxisMinor.Magnitude()); - Handle(Geom_Ellipse) curve = new Geom_Ellipse(elipsDest); - Part::GeomEllipse* ellipse = new Part::GeomEllipse(); - ellipse->setHandle(curve); - GeometryFacade::setConstruction(ellipse, true); + Handle(Geom_Ellipse) curve = new Geom_Ellipse(elipsDest); + Part::GeomEllipse* ellipse = new Part::GeomEllipse(); + ellipse->setHandle(curve); + GeometryFacade::setConstruction(ellipse, true); - ExternalGeo.push_back(ellipse); - } + ExternalGeo.push_back(ellipse); } } }