Data Exchange, Step export - writing untrimmed curve

Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: atychini
Date: Mon Feb 7 15:57:38 2022 +0300

    0032817: Data Exchange - Step export - writing untrimmed Curve
    
    Incorrect curve handling. If curve doesn't have trims and vertexes, we handle it such as trimmed curve, but it should be a simple curve. Curves with only one vertex and one trim should handle by another way, such as a curve that has trims but doesn't have vertexes. So I added conditions which handles these cases

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Author,
please review branch CR32817

All tests are OK, see:
http://jenkins-test-08.nnov.opencascade.com/view/CR32817-master-atychini/view/COMPARE/

I have some remarks

+  if(aVFirst.IsNull() && aVLast.IsNull() && Precision::IsInfinite(aFirst) && Precision::IsInfinite(aLast))
+  if (aVFirst.IsNull() && !Precision::IsNegativeInfinite(aFirst))
+  if (aVLast.IsNull() && !Precision::IsPositiveInfinite(aLast))

There are different compression (2e100, 1e100). Please use limitation, that equals with void Bounding Box.

+    GeomToStep_MakeCartesianPoint aGTSMCP(gp_Pnt(aFirst, aFirst, aFirst));
+    GeomToStep_MakeCartesianPoint aGTSMCP(gp_Pnt(aLast, aLast, aLast));

Creation of point is incorrect. You need to project a trim to the curve to get a point.

+    aSGC = GeomToStep_MakeCurve(aC).Value();

Checking for status(isDone) is necessary.

-  Standard_Real First, Last;
-  Handle(Geom_Curve) C = BRep_Tool::Curve(anEdge, First, Last); 
-  if ( ! C.IsNull() ) {
-    if (C->IsKind(STANDARD_TYPE(Geom_TrimmedCurve)))
-      C = Handle(Geom_TrimmedCurve)::DownCast(C)->BasisCurve();
-    GeomToStep_MakeCurve  gtpC(C);

I think, that moving conversion to the base curve with creation is the best way.

+  if ( ! aC.IsNull() ) {
+    if (aC->IsKind(STANDARD_TYPE(Geom_TrimmedCurve)))
+      aC = Handle(Geom_TrimmedCurve)::DownCast(aC)->BasisCurve();
+    GeomToStep_MakeCurve  gtpC(aC);

Code style.

   7 catch {Close D}
   8 XNewDoc D
   9 line l1 0 0 0 0 1 0
  10 mkedge e1 l1
  11 XAddShape D e1
  12 WriteStep D $imagedir/${casename}.stp
  13 file delete $imagedir/${casename}.stp

You need to tests all types of trimmed(create more test cases). Compare the base and written results.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Author,
please review branch CR32817

All tests are OK, see:
http://jenkins-test-08.nnov.opencascade.com/view/CR32817-master-atychini/view/COMPARE/
http://jenkins-test-08.nnov.opencascade.com/view/CR32817-master-atychini/view/TESTING/job/CR32817-master-atychini-OCCT-Windows-64-VC14-opt-test-restart/1/HTML_20Report/
http://jenkins-test-08.nnov.opencascade.com/view/CR32817-master-atychini/view/TESTING/job/CR32817-master-atychini-OCCT-Windows-64-VC14-opt-test-restart/2/HTML_20Report/
http://jenkins-test-08.nnov.opencascade.com/view/CR32817-master-atychini/view/TESTING/job/CR32817-master-atychini-OCCT-Debian80-64-opt-test-restart/HTML_20Report/

I have some remarks

      aSGC = GeomToStep_MakeCurve(aC).Value();
      if (!aSGC.IsNull())
      {
        theCurveList->Append(aSGC);
        return Standard_True;
      }

GeomToStep_MakeCurve need to be checked for a result (isDone)

  // ---------------------------------------
  // Translate 3D representation of the Edge
  // ---------------------------------------
    
    
  // Handle(Geom_Curve) C = CA.Curve().Curve();

  // UPDATE FMA 26-02-96 
  // General remark : this full code should be deaply reworked
  //                  Too many objects are not used !

Code has been moved in the incorrect place. It can be cleared or rewrite too.
_
Please remove any tabulations in your code

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Author,
please review branch CR32817

All tests are OK, see:
http://jenkins-test-occt.nnov.opencascade.com/view/CR32817-master-atychini/view/COMPARE/

        aSGC = aGTSML.Value();
        aSGC = MakeTrimmedCurve(aGTSML.Value(), aSGCP1, aSGCP2, 0, aLength, Standard_True);
        //				 (anEdge.Orientation() == TopAbs_FORWARD));
      }

    aSGC = MakeTrimmedCurve(aPMSC, aSGCP1, aSGCP2, aTrim1, aTrim2, Standard_True);
    //			     (anEdge.Orientation() == TopAbs_FORWARD));
  }

Please remove the incorrect comment+ tabulations.

Standard_Boolean TopoDSToStep_WireframeBuilder::
  GetTrimmedCurveFromEdge(const TopoDS_Edge& theEdge, 
              const TopoDS_Face& theFace, 
              MoniTool_DataMapOfShapeTransient& theMap, 
              Handle(TColStd_HSequenceOfTransient)& theCurveList) const

Please remove tabs

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Commenter 4,
please integrate:
OCCT: CR32817
PROD: NO

Commenter 3

Please split long comments in git commit description. Normally, one sentence per line.

+  TopExp_Explorer anExp;
+  for (anExp.Init(anEdge, TopAbs_VERTEX); anExp.More(); anExp.Next())
+  {

It is undesired declaring iterator outside of the for() header, when it is unused afterwards.

+    TopoDS_Vertex aVertex = TopoDS::Vertex(anExp.Value());

Redundant shape copy.

+      GeomToStep_MakeCurve aCurveMaker = GeomToStep_MakeCurve(aC);

Redundant assignment.

+  if (theMap.IsBound(anEdge))
+  {
+    aSGC = Handle(StepGeom_Curve)::DownCast ( theMap.Find(anEdge) );

if (Handle(Standard_Transient)* aTransient = theMap.Seek (anEdge))
{
  aSGC = Handle(StepGeom_Curve)::DownCast (*aTransient);
}

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Author,
please review branch CR32817

All tests are OK, see:
http://jenkins-test-occt.nnov.opencascade.com/view/CR32817-master-atychini/view/COMPARE/

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Commenter 4,
please integrate:
OCCT: CR32817
PROD: NO

Combination -
OCCT branch : [archived branch]
master SHA - [revision removed]
[revision removed]
Products branch : [archived branch] SHA - [revision removed]
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 18487.460000000385 / 18395.730000000258 [+0.50%]
Products
Total CPU difference: 11709.300000000132 / 11708.150000000112 [+0.01%]
Windows-64-VC14:
OCCT
Total CPU difference: 20412.546875 / 20350.609375 [+0.30%]
Products
Total CPU difference: 13181.25 / 13154.5625 [+0.20%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

Branch [archived branch] has been deleted by Participant.

[revision removed]