From 99bd16c981c93efc3bf5168b40e7844041f23174 Mon Sep 17 00:00:00 2001 From: Artem Zhidkov Date: Tue, 22 Dec 2020 22:44:54 +0300 Subject: [PATCH] Issue #20513: Filling problem * Restore current feature after update of a sketch from Python API. * Improve Filling feature to update correctly the orientation of closed wires. * Workaround to avoid crash when converting wire to a single edge. --- src/BuildPlugin/BuildPlugin_Filling.cpp | 153 ++++++++++++++++----- src/BuildPlugin/BuildPlugin_Filling.h | 3 +- src/BuildPlugin/CMakeLists.txt | 1 + src/BuildPlugin/Test/Test20513.py | 97 +++++++++++++ src/GeomAlgoAPI/GeomAlgoAPI_ShapeTools.cpp | 3 +- src/ModelHighAPI/ModelHighAPI_Tools.cpp | 2 +- src/SketchPlugin/SketchPlugin_Sketch.cpp | 19 ++- 7 files changed, 239 insertions(+), 39 deletions(-) create mode 100644 src/BuildPlugin/Test/Test20513.py diff --git a/src/BuildPlugin/BuildPlugin_Filling.cpp b/src/BuildPlugin/BuildPlugin_Filling.cpp index 457f2eeee..9f669cf71 100644 --- a/src/BuildPlugin/BuildPlugin_Filling.cpp +++ b/src/BuildPlugin/BuildPlugin_Filling.cpp @@ -30,10 +30,13 @@ #include #include #include +#include +#include #include #include #include +#include #include @@ -48,6 +51,15 @@ struct FillingParameters bool isApprox; }; +static bool isReverseClosedCurve(const GeomEdgePtr& theEdge1, + const GeomEdgePtr& theEdge2); +static bool isReverseOpenedCurve(const GeomEdgePtr& theEdge1, + const GeomEdgePtr& theEdge2, + const double theTolerance); +static void shiftStartPoint(GeomWirePtr& theWire, + const GeomEdgePtr& theRefEdge, + const double theTolerance); + //================================================================================================= BuildPlugin_Filling::BuildPlugin_Filling() @@ -104,15 +116,11 @@ void BuildPlugin_Filling::execute() for(int anIndex = 0; anIndex < aSelectionList->size(); ++anIndex) { AttributeSelectionPtr aSelection = aSelectionList->value(anIndex); GeomEdgePtr anEdge = toEdge(aSelection->value(), aParameters.method); - if (!anEdge) { - myLastEdgeStartPoint = GeomPointPtr(); - myLastEdgeEndPoint = GeomPointPtr(); + if (!anEdge) return; - } aFilling->add(anEdge); } - myLastEdgeStartPoint = GeomPointPtr(); - myLastEdgeEndPoint = GeomPointPtr(); + myLastEdge = GeomEdgePtr(); // build result aFilling->build(aParameters.isApprox); @@ -148,15 +156,20 @@ void BuildPlugin_Filling::attributeChanged(const std::string& theID) //================================================================================================= GeomEdgePtr BuildPlugin_Filling::toEdge(const GeomShapePtr& theShape, const std::string& theMethod) { + static const double TOLERANCE = 1.e-7; + GeomEdgePtr anEdge; switch (theShape->shapeType()) { case GeomAPI_Shape::EDGE: anEdge = GeomEdgePtr(new GeomAPI_Edge(GeomAlgoAPI_Copy(theShape).shape())); break; - case GeomAPI_Shape::WIRE: - anEdge = GeomAlgoAPI_ShapeTools::wireToEdge( - GeomWirePtr(new GeomAPI_Wire(theShape))); + case GeomAPI_Shape::WIRE: { + GeomWirePtr aWire(new GeomAPI_Wire(theShape)); + if (myLastEdge && theMethod == Method::AUTO_CORRECT_ORIENTATION()) + shiftStartPoint(aWire, myLastEdge, TOLERANCE); + anEdge = GeomAlgoAPI_ShapeTools::wireToEdge(aWire); break; + } default: break; } @@ -171,34 +184,18 @@ GeomEdgePtr BuildPlugin_Filling::toEdge(const GeomShapePtr& theShape, const std: // correct edge orientation according to filling method if (theMethod == Method::AUTO_CORRECT_ORIENTATION()) { // check the distance to previous edge boundaries, reverse edge if necessary - GeomPointPtr aStartPnt = anEdge->firstPoint(); - GeomPointPtr aEndPnt = anEdge->lastPoint(); - if (anEdge->orientation() == GeomAPI_Shape::REVERSED) { - aStartPnt = anEdge->lastPoint(); - aEndPnt = anEdge->firstPoint(); - } bool isReverse = false; - if (myLastEdgeStartPoint) { - double d1 = myLastEdgeStartPoint->distance(aStartPnt) - + myLastEdgeEndPoint->distance(aEndPnt); - double d2 = myLastEdgeStartPoint->distance(aEndPnt) - + myLastEdgeEndPoint->distance(aStartPnt); - if (fabs(d1 - d2) < 1.e-7) { - // undefined case => check distance to start point only - d1 = myLastEdgeStartPoint->distance(aStartPnt); - d2 = myLastEdgeStartPoint->distance(aEndPnt); - } - isReverse = d2 < d1; + if (myLastEdge) { + if (myLastEdge->firstPoint()->distance(myLastEdge->lastPoint()) < TOLERANCE && + anEdge->firstPoint()->distance(anEdge->lastPoint()) < TOLERANCE) + isReverse = isReverseClosedCurve(myLastEdge, anEdge); + else + isReverse = isReverseOpenedCurve(myLastEdge, anEdge, TOLERANCE); } - if (isReverse) { + myLastEdge = anEdge; + if (isReverse) anEdge->reverse(); - myLastEdgeStartPoint = aEndPnt; - myLastEdgeEndPoint = aStartPnt; - } else { - myLastEdgeStartPoint = aStartPnt; - myLastEdgeEndPoint = aEndPnt; - } } else if (theMethod == Method::USE_CURVE_INFORMATION()) { // make all edges FORWARD to avoid reversing the curves by GeomAlgoAPI_Filling algorithm @@ -218,3 +215,93 @@ void BuildPlugin_Filling::restoreDefaultParameters() real(TOLERANCE_3D_ID())->setValue(TOLERANCE_3D_DEFAULT()); boolean(APPROXIMATION_ID())->setValue(APPROXIMATION_DEFAULT()); } + + +//============ Auxiliary functions ======================================================== + +static std::pair edgeBoundaries(const GeomEdgePtr& theEdge) +{ + GeomPointPtr aStart = theEdge->firstPoint(); + GeomPointPtr anEnd = theEdge->lastPoint(); + if (theEdge->orientation() == GeomAPI_Shape::REVERSED) + std::swap(aStart, anEnd); + return std::pair(aStart, anEnd); +} + +static void edgePoints(const GeomEdgePtr& theEdge, std::list& thePoints) +{ + GeomAPI_Curve aCurve(theEdge); + static const int aNbSegments = 10; + double aStart = aCurve.startParam(); + double aEnd = aCurve.endParam(); + for (int i = 0; i <= aNbSegments; ++i) + thePoints.push_back(aCurve.getPoint(aStart * (1.0 - (double)i / aNbSegments) + + aEnd * (double)i / aNbSegments )); + if (theEdge->orientation() == GeomAPI_Shape::REVERSED) + thePoints.reverse(); +} + +bool isReverseClosedCurve(const GeomEdgePtr& theEdge1, + const GeomEdgePtr& theEdge2) +{ + std::list anEdge1Points, anEdge2Points; + edgePoints(theEdge1, anEdge1Points); + edgePoints(theEdge2, anEdge2Points); + + double d1 = 0.0; + double d2 = 0.0; + std::list::const_iterator anIt1 = anEdge1Points.begin(); + std::list::const_iterator anIt2 = anEdge2Points.begin(); + std::list::const_reverse_iterator anIt2Rev = anEdge2Points.rbegin(); + for (; anIt1 != anEdge1Points.end(); ++anIt1, ++anIt2, ++anIt2Rev) { + d1 += (*anIt1)->distance(*anIt2); + d2 += (*anIt1)->distance(*anIt2Rev); + } + return d2 < d1; +} + +bool isReverseOpenedCurve(const GeomEdgePtr& theEdge1, + const GeomEdgePtr& theEdge2, + const double theTolerance) +{ + std::pair anEdge1Points = edgeBoundaries(theEdge1); + std::pair anEdge2Points = edgeBoundaries(theEdge2); + double d1 = anEdge1Points.first->distance(anEdge2Points.first) + + anEdge1Points.second->distance(anEdge2Points.second); + double d2 = anEdge1Points.first->distance(anEdge2Points.second) + + anEdge1Points.second->distance(anEdge2Points.first); + if (fabs(d1 - d2) < theTolerance) { + // undefined case => check distance to start point only + d1 = anEdge1Points.first->distance(anEdge2Points.first); + d2 = anEdge1Points.first->distance(anEdge2Points.second); + } + return d2 < d1; +} + +void shiftStartPoint(GeomWirePtr& theWire, const GeomEdgePtr& theRefEdge, const double theTolerance) +{ + if (!theWire->isClosed()) { + GeomVertexPtr aV1, aV2; + GeomAlgoAPI_ShapeTools::findBounds(theWire, aV1, aV2); + if (aV1->point()->distance(aV2->point()) > theTolerance) + return; + } + + // find closest vertex on the wire to the start point on the edge + GeomPointPtr aFirstRefPnt = theRefEdge->firstPoint(); + ListOfShape aBegin, aEnd; + double aMinDist = 1.e100; + for (GeomAPI_WireExplorer anExp(theWire); anExp.more(); anExp.next()) { + double aDist = anExp.currentVertex()->point()->distance(aFirstRefPnt); + if (aDist < aMinDist) { + aMinDist = aDist; + aEnd.insert(aEnd.end(), aBegin.begin(), aBegin.end()); + aBegin.clear(); + } + aBegin.push_back(anExp.current()); + } + aBegin.insert(aBegin.end(), aEnd.begin(), aEnd.end()); + + GeomShapePtr aShape = GeomAlgoAPI_WireBuilder::wire(aBegin); + theWire.reset(new GeomAPI_Wire(aShape)); +} diff --git a/src/BuildPlugin/BuildPlugin_Filling.h b/src/BuildPlugin/BuildPlugin_Filling.h index 1d7297309..9725c8a33 100644 --- a/src/BuildPlugin/BuildPlugin_Filling.h +++ b/src/BuildPlugin/BuildPlugin_Filling.h @@ -171,8 +171,7 @@ private: void restoreDefaultParameters(); private: - std::shared_ptr myLastEdgeStartPoint; - std::shared_ptr myLastEdgeEndPoint; + std::shared_ptr myLastEdge; }; #endif diff --git a/src/BuildPlugin/CMakeLists.txt b/src/BuildPlugin/CMakeLists.txt index c8b933b22..4bcf4a001 100644 --- a/src/BuildPlugin/CMakeLists.txt +++ b/src/BuildPlugin/CMakeLists.txt @@ -160,4 +160,5 @@ ADD_UNIT_TESTS(TestVertex.py Test3271.py Test19056.py Test20469.py + Test20513.py ) diff --git a/src/BuildPlugin/Test/Test20513.py b/src/BuildPlugin/Test/Test20513.py new file mode 100644 index 000000000..11192d91d --- /dev/null +++ b/src/BuildPlugin/Test/Test20513.py @@ -0,0 +1,97 @@ +# Copyright (C) 2020 CEA/DEN, EDF R&D +# +# This library 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. +# +# This library 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 this library; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +# +# See http://www.salome-platform.org/ or email : webmaster.salome@opencascade.com +# + +from SketchAPI import * + +from salome.shaper import model + +model.begin() +partSet = model.moduleDocument() + +### Create Part +Part_1 = model.addPart(partSet) +Part_1_doc = Part_1.document() + +### Create Sketch +Sketch_1 = model.addSketch(Part_1_doc, model.defaultPlane("XOY")) + +### Create SketchProjection +SketchProjection_1 = Sketch_1.addProjection(model.selection("VERTEX", "PartSet/Origin"), False) +SketchPoint_1 = SketchProjection_1.createdFeature() + +### Create SketchBSpline +SketchBSpline_1_poles = [(0, 0), + (64.69989977297864, 3.820449445888325), + (63.227018959685, 42.02181716036146), + (19.7442263567145, 66.94290132940253), + (0, 0) + ] +SketchBSpline_1 = Sketch_1.addSpline(poles = SketchBSpline_1_poles) +[SketchPoint_2, SketchPoint_3, SketchPoint_4, SketchPoint_5, SketchPoint_6] = SketchBSpline_1.controlPoles(auxiliary = [0, 1, 2, 3, 4]) +[SketchLine_1, SketchLine_2, SketchLine_3, SketchLine_4] = SketchBSpline_1.controlPolygon(auxiliary = [0, 1, 2, 3]) +Sketch_1.setCoincident(SketchAPI_Point(SketchPoint_2).coordinates(), SketchPoint_1.result()) +Sketch_1.setCoincident(SketchAPI_Point(SketchPoint_6).coordinates(), SketchPoint_1.result()) +model.do() + +### Create Plane +Plane_4 = model.addPlane(Part_1_doc, model.selection("FACE", "Sketch_1/Face-SketchBSpline_1f"), 10, False) + +### Create Sketch +Sketch_2 = model.addSketch(Part_1_doc, model.selection("FACE", "Plane_1")) + +### Create SketchProjection +SketchProjection_2 = Sketch_2.addProjection(model.selection("VERTEX", "PartSet/Origin"), False) +SketchPoint_7 = SketchProjection_2.createdFeature() + +### Create SketchBSpline +SketchBSpline_2_poles = [(0, 0), + (8.839087148173476, 45.21301708610518), + (53.18579907801947, 39.22586299994072), + (47.99568029170977, 4.506542817571567), + (0, 0) + ] +SketchBSpline_2 = Sketch_2.addSpline(poles = SketchBSpline_2_poles) +[SketchPoint_8, SketchPoint_9, SketchPoint_10, SketchPoint_11, SketchPoint_12] = SketchBSpline_2.controlPoles(auxiliary = [0, 1, 2, 3, 4]) +[SketchLine_5, SketchLine_6, SketchLine_7, SketchLine_8] = SketchBSpline_2.controlPolygon(auxiliary = [0, 1, 2, 3]) +Sketch_2.setCoincident(SketchAPI_Point(SketchPoint_8).coordinates(), SketchPoint_7.result()) +Sketch_2.setCoincident(SketchAPI_Point(SketchPoint_12).coordinates(), SketchPoint_7.result()) +model.do() + +### Create Wire +Wire_1 = model.addWire(Part_1_doc, [model.selection("EDGE", "Sketch_1/SketchBSpline_1")], False) + +### Create Wire +Wire_2 = model.addWire(Part_1_doc, [model.selection("EDGE", "Sketch_2/SketchBSpline_2")], False) + +### Create Filling +Filling_1 = model.addFilling(Part_1_doc, [model.selection("WIRE", "Wire_1_1"), model.selection("WIRE", "Wire_2_1")]) + +model.end() + +from GeomAPI import * + +model.testNbResults(Filling_1, 1) +model.testNbSubResults(Filling_1, [0]) +model.testNbSubShapes(Filling_1, GeomAPI_Shape.SOLID, [0]) +model.testNbSubShapes(Filling_1, GeomAPI_Shape.FACE, [1]) +model.testNbSubShapes(Filling_1, GeomAPI_Shape.EDGE, [4]) +model.testNbSubShapes(Filling_1, GeomAPI_Shape.VERTEX, [8]) +model.testResultsVolumes(Filling_1, [1872.0403629667237]) + +assert(model.checkPythonDump()) diff --git a/src/GeomAlgoAPI/GeomAlgoAPI_ShapeTools.cpp b/src/GeomAlgoAPI/GeomAlgoAPI_ShapeTools.cpp index 9a51a5ae7..e1840bc63 100644 --- a/src/GeomAlgoAPI/GeomAlgoAPI_ShapeTools.cpp +++ b/src/GeomAlgoAPI/GeomAlgoAPI_ShapeTools.cpp @@ -1135,7 +1135,8 @@ std::shared_ptr GeomAlgoAPI_ShapeTools::wireToEdge( // on the curve and second is placed at the start, this workaround copies second curve to avoid // treating these edges as a single curve by setting trim parameters. aWire = fixParametricGaps(aWire); - TopoDS_Edge aNewEdge = BRepAlgo::ConcatenateWireC0(aWire); + aWire = BRepAlgo::ConcatenateWire(aWire, GeomAbs_G1); // join smooth parts of wire + TopoDS_Edge aNewEdge = BRepAlgo::ConcatenateWireC0(aWire); // join C0 parts of wire anEdge = GeomEdgePtr(new GeomAPI_Edge); anEdge->setImpl(new TopoDS_Edge(aNewEdge)); } diff --git a/src/ModelHighAPI/ModelHighAPI_Tools.cpp b/src/ModelHighAPI/ModelHighAPI_Tools.cpp index 7f959291c..1e2648cf2 100644 --- a/src/ModelHighAPI/ModelHighAPI_Tools.cpp +++ b/src/ModelHighAPI/ModelHighAPI_Tools.cpp @@ -359,7 +359,7 @@ GeomAPI_Shape::ShapeType getShapeType(const ModelHighAPI_Selection& theSelection case ModelHighAPI_Selection::VT_ResultSubShapePair: { ResultSubShapePair aPair = theSelection.resultSubShapePair(); GeomShapePtr aShape = aPair.second; - if(!aShape.get()) { + if(!aShape.get() && aPair.first.get()) { aShape = aPair.first->shape(); } if(!aShape.get()) { diff --git a/src/SketchPlugin/SketchPlugin_Sketch.cpp b/src/SketchPlugin/SketchPlugin_Sketch.cpp index 984293083..d7efa170f 100644 --- a/src/SketchPlugin/SketchPlugin_Sketch.cpp +++ b/src/SketchPlugin/SketchPlugin_Sketch.cpp @@ -153,6 +153,19 @@ void SketchPlugin_Sketch::execute() std::shared_ptr SketchPlugin_Sketch::addFeature(std::string theID) { + // It is necessary to keep and restore the current feature in the document, + // if the sketch is updated from Python API. Because in that case, the current feature + // may be a non-sketch feature, so it is required to set it back, after adding a sketch feature, + // to keep the sequence of non-sketch features within the document. + FeaturePtr aCurFeature = document()->currentFeature(false); + std::shared_ptr aCurSketchFeature = + std::dynamic_pointer_cast(aCurFeature); + std::shared_ptr aCurSketch = + std::dynamic_pointer_cast(aCurFeature); + if ((aCurSketch && aCurSketch.get() == this) || + (aCurSketchFeature && aCurSketchFeature->sketch() == this)) + aCurFeature = FeaturePtr(); // no need to restore feature if it is from the current sketch + // Set last feature of the sketch as current feature. // Reason: Changing of parameter through Python API may lead to creation of new features // (e.g. changing number of copies in MultiRotation). If the sketch is not the last @@ -177,8 +190,10 @@ std::shared_ptr SketchPlugin_Sketch::addFeature(std::string th aSketchFeature->setSketch(this); data()->reflist(SketchPlugin_Sketch::FEATURES_ID())->append(aNew); } - // set as current also after it becomes sub to set correctly enabled for other sketch subs - document()->setCurrentFeature(aNew, false); + + // set as current also after it becomes sub to set correctly enabled for other sketch subs + // or restore the previous current feature + document()->setCurrentFeature(aCurFeature ? aCurFeature : aNew, false); return aNew; } -- 2.39.2