Salome HOME
Issue #20513: Filling problem
authorArtem Zhidkov <Artem.Zhidkov@opencascade.com>
Tue, 22 Dec 2020 19:44:54 +0000 (22:44 +0300)
committerArtem Zhidkov <Artem.Zhidkov@opencascade.com>
Tue, 22 Dec 2020 19:44:54 +0000 (22:44 +0300)
* 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
src/BuildPlugin/BuildPlugin_Filling.h
src/BuildPlugin/CMakeLists.txt
src/BuildPlugin/Test/Test20513.py [new file with mode: 0644]
src/GeomAlgoAPI/GeomAlgoAPI_ShapeTools.cpp
src/ModelHighAPI/ModelHighAPI_Tools.cpp
src/SketchPlugin/SketchPlugin_Sketch.cpp

index 457f2eeeef5276bdbed11e82df8ff440b854ac9f..9f669cf710bd620c9267ab69f58be7d9238891a5 100644 (file)
 #include <GeomAlgoAPI_Filling.h>
 #include <GeomAlgoAPI_ShapeTools.h>
 #include <GeomAlgoAPI_Tools.h>
+#include <GeomAlgoAPI_WireBuilder.h>
 
+#include <GeomAPI_Curve.h>
 #include <GeomAPI_Pnt.h>
 #include <GeomAPI_ShapeExplorer.h>
 #include <GeomAPI_Wire.h>
+#include <GeomAPI_WireExplorer.h>
 
 #include <cmath>
 
@@ -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<GeomPointPtr, GeomPointPtr> 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<GeomPointPtr, GeomPointPtr>(aStart, anEnd);
+}
+
+static void edgePoints(const GeomEdgePtr& theEdge, std::list<GeomPointPtr>& 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<GeomPointPtr> anEdge1Points, anEdge2Points;
+  edgePoints(theEdge1, anEdge1Points);
+  edgePoints(theEdge2, anEdge2Points);
+
+  double d1 = 0.0;
+  double d2 = 0.0;
+  std::list<GeomPointPtr>::const_iterator anIt1 = anEdge1Points.begin();
+  std::list<GeomPointPtr>::const_iterator anIt2 = anEdge2Points.begin();
+  std::list<GeomPointPtr>::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<GeomPointPtr, GeomPointPtr> anEdge1Points = edgeBoundaries(theEdge1);
+  std::pair<GeomPointPtr, GeomPointPtr> 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));
+}
index 1d7297309fcc04bfaf28a7107a28d014bf3c61ba..9725c8a33d1d6fc81d51245c63c5ac36b127f64e 100644 (file)
@@ -171,8 +171,7 @@ private:
   void restoreDefaultParameters();
 
 private:
-  std::shared_ptr<GeomAPI_Pnt> myLastEdgeStartPoint;
-  std::shared_ptr<GeomAPI_Pnt> myLastEdgeEndPoint;
+  std::shared_ptr<GeomAPI_Edge> myLastEdge;
 };
 
 #endif
index c8b933b22a6ae569c066a541a44c9e161e3e6bce..4bcf4a001c626395c85db7510c10420cd6380b76 100644 (file)
@@ -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 (file)
index 0000000..11192d9
--- /dev/null
@@ -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())
index 9a51a5ae766a8e156268bb38071eff22344bb365..e1840bc63ee3d0405b2cd26eb4a326fbf0e8dd04 100644 (file)
@@ -1135,7 +1135,8 @@ std::shared_ptr<GeomAPI_Edge> 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));
   }
index 7f959291cb4d815416327ca4a4064d2235b31dec..1e2648cf2305c7badc4f82ad1e32f6e25910cbf9 100644 (file)
@@ -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()) {
index 984293083baf47154dfbad362417e2f1509ef07d..d7efa170f9a61b0f9a22556ba2551e18846f9a05 100644 (file)
@@ -153,6 +153,19 @@ void SketchPlugin_Sketch::execute()
 
 std::shared_ptr<ModelAPI_Feature> 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<SketchPlugin_Feature> aCurSketchFeature =
+      std::dynamic_pointer_cast<SketchPlugin_Feature>(aCurFeature);
+  std::shared_ptr<SketchPlugin_Sketch> aCurSketch =
+      std::dynamic_pointer_cast<SketchPlugin_Sketch>(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<ModelAPI_Feature> 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;
 }