Merge branch 'master' into bugfix/path-invalid-base-geometry-robustness

src/Mod/Draft/draftguitools/gui_arcs.py

@@ -68,7 +68,7 @@ def GetResources(self):
 
     def Activated(self):
         """Execute when the command is called."""
-        super(Arc, self).Activated(name=translate("draft","Arc")
+        super(Arc, self).Activated(name=translate("draft","Arc"))
         if self.ui:
             self.step = 0
             self.center = None

src/Mod/Draft/draftguitools/gui_beziers.py

@@ -235,7 +235,7 @@ def __init__(self):
 
     def GetResources(self):
         """Set icon, menu and tooltip."""
-        _menu = 
+        _menu = ""
         _tip = ()
 
         return {'Pixmap': 'Draft_CubicBezCurve',

src/Mod/Draft/draftguitools/gui_groups.py

@@ -163,7 +163,10 @@ def GetResources(self):
 
         d = {'Pixmap': 'Draft_SelectGroup',
              'MenuText': QT_TRANSLATE_NOOP("Draft_SelectGroup","Select group"),
-             'ToolTip': QT_TRANSLATE_NOOP("Draft_SelectGroup","If the selection is a group, it selects all objects that are inside this group, including those in nested sub-groups.\n\nIf the selection is a simple object inside a group, it will select the "brother" objects, that is,\nthose that are at the same level as this object, including the upper group that contains them all.")}
+             'ToolTip': QT_TRANSLATE_NOOP("Draft_SelectGroup","If the selection is a group, it selects all objects that are inside this group, "
+                                                              "including those in nested sub-groups.\n\nIf the selection is a simple object "
+                                                              "inside a group, it will select the \"brother\" objects, that is,\nthose that are "
+                                                              "at the same level as this object, including the upper group that contains them all.")}
         return d
 
     def Activated(self):

src/Mod/Draft/draftguitools/gui_shape2dview.py

@@ -54,7 +54,7 @@ class Shape2DView(gui_base_original.Modifier):
 
     def GetResources(self):
         """Set icon, menu and tooltip."""
-        _menu = 
+        _menu = ""
         _tip = ()
 
         return {'Pixmap': 'Draft_2DShapeView',

src/Mod/Draft/draftguitools/gui_shapestrings.py

@@ -62,7 +62,7 @@ class ShapeString(gui_base_original.Creator):
 
     def GetResources(self):
         """Set icon, menu and tooltip."""
-        _menu = 
+        _menu = ""
         _tip = ()
 
         d = {'Pixmap': 'Draft_ShapeString',

src/Mod/Draft/draftguitools/gui_snaps.py

@@ -567,7 +567,7 @@ def __init__(self):
 
     def GetResources(self):
         """Set icon, menu and tooltip."""
-        _tip = 
+        _tip = ""
 
         return {'Pixmap': 'Draft_Snap',
                 'MenuText': QT_TRANSLATE_NOOP("Draft_ShowSnapBar","Show snap toolbar"),

src/Mod/Draft/draftguitools/gui_subelements.py

@@ -54,7 +54,7 @@ def __init__(self):
 
     def GetResources(self):
         """Set icon, menu and tooltip."""
-        _menu = 
+        _menu = ""
         _tip = ()
 
         return {'Pixmap': 'Draft_SubelementHighlight',

src/Mod/Draft/drafttaskpanels/task_circulararray.py

@@ -297,7 +297,7 @@ def create_object(self):
                      "App.ActiveDocument.recompute()"]
 
         # We commit the command list through the parent command
-        self.source_command.commit(translate("draft","Circular array", _cmd_list)
+        self.source_command.commit(translate("draft","Circular array", _cmd_list))
 
     def get_distances(self):
         """Get the distance parameters from the widgets."""

src/Mod/Draft/drafttaskpanels/task_polararray.py

@@ -92,7 +92,7 @@ def __init__(self):
         pix = QtGui.QPixmap(svg)
         icon = QtGui.QIcon.fromTheme(icon_name, QtGui.QIcon(svg))
         self.form.setWindowIcon(icon)
-        self.form.setWindowTitle(translate("draft","Polar array")
+        self.form.setWindowTitle(translate("draft","Polar array"))
 
         self.form.label_icon.setPixmap(pix.scaled(32, 32))
 

src/Mod/Sketcher/App/Sketch.cpp

@@ -138,7 +138,9 @@ bool Sketch::analyseBlockedGeometry( const std::vector<Part::Geometry *> &intern
                                      std::vector<bool> &onlyblockedGeometry,
                                      std::vector<int> &blockedGeoIds) const
 {
-    bool isSomethingBlocked = false;
+    // To understand this function read the documentation in Sketch.h
+    // It is important that "onlyblockedGeometry" ONLY identifies blocked geometry
+    // that is not affected by any other driving constraint
     bool doesBlockAffectOtherConstraints = false;
 
     int geoindex = 0;
@@ -152,7 +154,7 @@ bool Sketch::analyseBlockedGeometry( const std::vector<Part::Geometry *> &intern
                 // is block driving
                 if( c->Type == Sketcher::Block && c->isDriving && c->First == geoindex)
                     blockisDriving = true;
-
+                // We have another driving constraint (which may be InternalAlignment)
                 if( c->Type != Sketcher::Block && c->isDriving &&
                     (c->First == geoindex || c->Second == geoindex || c->Third == geoindex) )
                     blockOnly = false;
@@ -161,12 +163,10 @@ bool Sketch::analyseBlockedGeometry( const std::vector<Part::Geometry *> &intern
             if(blockisDriving) {
                 if(blockOnly) {
                     onlyblockedGeometry[geoindex] = true; // we pre-fix this geometry
-                    isSomethingBlocked = true;
                 }
                 else {
                     // we will have to pos-analyse the first diagnose result for these geometries
                     // in order to avoid redundant constraints
-                    isSomethingBlocked = true;
                     doesBlockAffectOtherConstraints = true;
                     blockedGeoIds.push_back(geoindex);
                 }
@@ -176,39 +176,6 @@ bool Sketch::analyseBlockedGeometry( const std::vector<Part::Geometry *> &intern
         geoindex++;
     }
 
-    if(isSomethingBlocked) {
-
-        // look for internal geometry linked IAs
-        for(auto c : constraintList) {
-            if(c->Type == InternalAlignment) {
-
-                auto geoit = std::find(blockedGeoIds.begin(),blockedGeoIds.end(),c->Second);
-
-                if(geoit != blockedGeoIds.end() || onlyblockedGeometry[c->Second]) { // internal alignment geometry found, add to list
-                    // check if pre-fix or post-analyses
-                    bool blockAffectedOnly = true;
-
-                    for(auto ic : constraintList) {
-                        // there is another driving constraint
-                        if( ic->Type != Sketcher::Block && ic->isDriving &&
-                            (ic->First == c->First || ic->Second == c->First || ic->Third == c->First))
-                            blockAffectedOnly = false;
-                    }
-
-                    if(blockAffectedOnly) {
-                        onlyblockedGeometry[c->Second] = true; // we pre-fix this geometry
-                    }
-                    else {
-                        // we will have to post-analyse the first diagnose result for these geometries
-                        // in order to avoid redundant constraints
-                        doesBlockAffectOtherConstraints = true;
-                        blockedGeoIds.push_back(*geoit);
-                    }
-                }
-            }
-        }
-    }
-
     return doesBlockAffectOtherConstraints;
 }
 
@@ -247,19 +214,23 @@ int Sketch::setUpSketch(const std::vector<Part::Geometry *> &GeoList,
 
     Base::Console().Log("\nOnlyBlocked GeoIds:");
     size_t i = 0;
+    bool found = false;
     for(; i < onlyBlockedGeometry.size(); i++) {
-        if(onlyBlockedGeometry[i])
+        if(onlyBlockedGeometry[i]) {
             Base::Console().Log("\n  GeoId=%d", i);
+            found = true;
+        }
     }
-    if( i == 0)
+    if(found)
         Base::Console().Log("\n  None");
 
     Base::Console().Log("\nNotOnlyBlocked GeoIds:");
     i = 0;
     for(; i < blockedGeoIds.size(); i++)
         Base::Console().Log("\n  GeoId=%d", blockedGeoIds[i]);
-    if( i == 0)
+    if(i == 0)
         Base::Console().Log("\n  None");
+    Base::Console().Log("\n");
 #endif //DEBUG_BLOCK_CONSTRAINT
 
     addGeometry(intGeoList,onlyBlockedGeometry);

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);
-
-                        ExternalGeo.push_back(ellipse);
+                    // 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
+
+                    // 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;
                     }
-                    else {
 
-                        // 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));
 
-                        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);
 
-                        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;
 
-                            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);
                         }
                     }
                 }