From 462ec49297c1eecaaea02f52e4e563d2fc986328 Mon Sep 17 00:00:00 2001 From: wmayer Date: Sun, 27 Dec 2015 11:44:47 +0100 Subject: [PATCH] + fixes #0001956: FreeCAD 0.14.370x hangs when attempting to edit sketch containing ellipse --- src/Mod/Sketcher/App/Constraint.h | 9 +++- .../Sketcher/App/PropertyConstraintList.cpp | 9 +++- src/Mod/Sketcher/App/SketchObject.cpp | 44 +++++++++++++++++++ src/Mod/Sketcher/App/SketchObject.h | 12 +++++ src/Mod/Sketcher/Gui/ViewProviderSketch.cpp | 3 +- 5 files changed, 73 insertions(+), 4 deletions(-) diff --git a/src/Mod/Sketcher/App/Constraint.h b/src/Mod/Sketcher/App/Constraint.h index 0a4bb5efa8..90f18b9f6b 100644 --- a/src/Mod/Sketcher/App/Constraint.h +++ b/src/Mod/Sketcher/App/Constraint.h @@ -31,7 +31,11 @@ namespace Sketcher { - +/*! + Important note: New constraint types must be always added at the end but before 'NumConstraintTypes'. + This is mandatory in order to keep the handling of constraint types upward compatible which means that + this program version ignores later introduced constraint types when reading them from a project file. + */ enum ConstraintType { None = 0, Coincident = 1, @@ -49,7 +53,8 @@ enum ConstraintType { PointOnObject = 13, Symmetric = 14, InternalAlignment = 15, - SnellsLaw = 16 + SnellsLaw = 16, + NumConstraintTypes // must be the last item! }; enum InternalAlignmentType { diff --git a/src/Mod/Sketcher/App/PropertyConstraintList.cpp b/src/Mod/Sketcher/App/PropertyConstraintList.cpp index aa132121cb..15fcaaedac 100644 --- a/src/Mod/Sketcher/App/PropertyConstraintList.cpp +++ b/src/Mod/Sketcher/App/PropertyConstraintList.cpp @@ -288,7 +288,14 @@ void PropertyConstraintList::Restore(Base::XMLReader &reader) for (int i = 0; i < count; i++) { Constraint *newC = new Constraint(); newC->Restore(reader); - values.push_back(newC); + // To keep upward compatibility ignore unknown constraint types + if (newC->Type < Sketcher::NumConstraintTypes) { + values.push_back(newC); + } + else { + // reading a new constraint type which this version cannot handle + delete newC; + } } reader.readEndElement("ConstraintList"); diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 9fb5b3090f..7b2704a667 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -511,6 +511,41 @@ void SketchObject::acceptGeometry() rebuildVertexIndex(); } +bool SketchObject::isSupportedGeometry(const Part::Geometry *geo) const +{ + if (geo->getTypeId() == Part::GeomPoint::getClassTypeId() || + geo->getTypeId() == Part::GeomCircle::getClassTypeId() || + geo->getTypeId() == Part::GeomEllipse::getClassTypeId() || + geo->getTypeId() == Part::GeomArcOfCircle::getClassTypeId() || + geo->getTypeId() == Part::GeomArcOfEllipse::getClassTypeId() || + geo->getTypeId() == Part::GeomLineSegment::getClassTypeId()) { + return true; + } + if (geo->getTypeId() == Part::GeomTrimmedCurve::getClassTypeId()) { + Handle_Geom_TrimmedCurve trim = Handle_Geom_TrimmedCurve::DownCast(geo->handle()); + Handle_Geom_Circle circle = Handle_Geom_Circle::DownCast(trim->BasisCurve()); + Handle_Geom_Ellipse ellipse = Handle_Geom_Ellipse::DownCast(trim->BasisCurve()); + if (!circle.IsNull() || !ellipse.IsNull()) { + return true; + } + } + return false; +} + +std::vector SketchObject::supportedGeometry(const std::vector &geoList) const +{ + std::vector supportedGeoList; + supportedGeoList.reserve(geoList.size()); + // read-in geometry that the sketcher cannot handle + for (std::vector::const_iterator it = geoList.begin(); it != geoList.end(); ++it) { + if (isSupportedGeometry(*it)) { + supportedGeoList.push_back(*it); + } + } + + return supportedGeoList; +} + int SketchObject::addGeometry(const std::vector &geoList, bool construction/*=false*/) { const std::vector< Part::Geometry * > &vals = getInternalGeometry(); @@ -3751,6 +3786,15 @@ void SketchObject::Restore(XMLReader &reader) void SketchObject::onChanged(const App::Property* prop) { + if (isRestoring() && prop == &Geometry) { + std::vector geom = Geometry.getValues(); + std::vector supportedGeom = supportedGeometry(geom); + // To keep upward compatibility ignore unsupported geometry types + if (supportedGeom.size() != geom.size()) { + Geometry.setValues(supportedGeom); + return; + } + } if (prop == &Geometry || prop == &Constraints) { Constraints.checkGeometry(getCompleteGeometry()); } diff --git a/src/Mod/Sketcher/App/SketchObject.h b/src/Mod/Sketcher/App/SketchObject.h index 7efcfa3cff..931978cf54 100644 --- a/src/Mod/Sketcher/App/SketchObject.h +++ b/src/Mod/Sketcher/App/SketchObject.h @@ -72,6 +72,12 @@ public: */ bool noRecomputes; + /*! + \brief Returns true if the sketcher supports the given geometry + \param geo - the geometry + \retval bool - true if the geometry is supported + */ + bool isSupportedGeometry(const Part::Geometry *geo) const; /// add unspecified geometry int addGeometry(const Part::Geometry *geo, bool construction=false); /// add unspecified geometry @@ -264,6 +270,12 @@ protected: void constraintsRenamed(const std::map &renamed); void constraintsRemoved(const std::set &removed); + /*! + \brief Returns a list of supported geometries from the input list + \param geoList - the geometry list + \retval list - the supported geometry list + */ + std::vector supportedGeometry(const std::vector &geoList) const; private: std::vector ExternalGeo; diff --git a/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp b/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp index 287ad0746e..0653250d5c 100644 --- a/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp +++ b/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp @@ -2309,7 +2309,7 @@ void ViewProviderSketch::updateColor(void) bool hasMaterial = false; SoMaterial *m = 0; - if (!hasDatumLabel && type != Sketcher::Coincident && type !=InternalAlignment) { + if (!hasDatumLabel && type != Sketcher::Coincident && type != Sketcher::InternalAlignment) { hasMaterial = true; m = dynamic_cast(s->getChild(CONSTRAINT_SEPARATOR_INDEX_MATERIAL_OR_DATUMLABEL)); } @@ -3187,6 +3187,7 @@ Restart: if ((*it)->Type != edit->vConstrType[i]) { // clearing the type vector will force a rebuild of the visual nodes edit->vConstrType.clear(); + //TODO: The 'goto' here is unsafe as it can happen that we cause an endless loop (see bug #0001956). goto Restart; } try{//because calculateNormalAtPoint, used in there, can throw