Modeling Algorithms - Add Clone() function for adapters

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Tue Jul 20 18:44:23 2021 +0300

    0032485: Modeling Algorithms - Add Clone() function for adapters

Author: Author
Date: Tue Jul 13 17:17:51 2021 +0300

    from patch

Author: Author
Date: Mon Jul 19 13:17:28 2021 +0300

    OSD_ThreadPool::Launcher instead OSD_Parallel::For

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Jul 23 19:15:28 2021 +0300

    0032485: More ShallowCopy()

Author: Author
Date: Mon Jul 26 12:34:30 2021 +0300

    set isMultiThread = true

Author: Author
Date: Tue Jul 20 18:44:23 2021 +0300

    0032485: Modeling Algorithms - Add Clone() function for adapters

Author: Author
Date: Tue Jul 13 17:17:51 2021 +0300

    from patch

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Tue Jul 27 18:45:16 2021 +0300

    0032485: More ShallowCopy()+

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Tue Jul 20 18:44:23 2021 +0300

    0032485: Modeling Algorithms - Add Clone() function for adapters

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

[revision removed]

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

[revision removed]

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

[revision removed]

Please review the code in branch CR32485_2
yesterday's tests results (not available now):
http://jenkins-test-occt/view/CR32485_2-master-Author/view/COMPARE/
has only one fail - 'bugs modalg_5 bug24012'
but now it has been fixed

+Handle(Adaptor3d_Curve) Adaptor3d_Curve::ShallowCopy() const
+{
+  throw Standard_NotImplemented("Adaptor3d_Curve::ShallowCopy");
+}

I think that the stab implemented by base class in hierarchy of Adaptors is a legacy of old C++/CDL code.
Could you try declaring new interface (and try also updating old ones in these classes) as pure virtual methods?

Or there are real use cases, where sub-classes are supposed to not implement these methods?

src/GeomAdaptor/GeomAdaptor_SurfaceOfLinearExtrusion.cxx
src/GeomAdaptor/GeomAdaptor_SurfaceOfRevolution.cxx

  aCopy->mySurfaceCache   = mySurfaceCache;

Cache must not be copied.

src/Geom2dEvaluator/Geom2dEvaluator_OffsetCurve.cxx
src/GeomEvaluator/GeomEvaluator_OffsetCurve.cxx
src/GeomEvaluator/GeomEvaluator_OffsetSurface.cxx
src/GeomEvaluator/GeomEvaluator_SurfaceOfExtrusion.cxx
src/GeomEvaluator/GeomEvaluator_SurfaceOfRevolution.cxx
You should select what constructor to call depending on the condition "myBaseAdaptor.IsNull()".

src/ProjLib/ProjLib_CompProjectedCurve.cxx
myTabInt is rewritten in the methods NbIntervals/Intervals, so it must not be shared.

src/BRepAdaptor/BRepAdaptor_Curve.cxx

  aCopy->myCurve = myCurve;

src/BRepAdaptor/BRepAdaptor_Surface.cxx

  aCopy->mySurf = mySurf;

In such way the cache of curve/surface adaptor will be shared, you should avoid it. It is better to create a shallow copy, and then save the value under its handle, as you are doing in BRepAdaptor_CompCurve.

src/BRepAdaptor/BRepAdaptor_Curve2d.cxx

  aCopy->myCurveCache = myCurveCache;

Cache must not be copied.

src/ChFiDS/ChFiDS_ElSpine.cxx
Do not change type of the field "curve". Instead do the same as in BRepAdaptor_Curve.

In the commit message, add a sentence that it will allow using copies of the same adapter in multi-thread calculations.

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Tue Jul 20 18:44:23 2021 +0300

    0032485: Modeling Algorithms - Add Clone() function for adapters
    
    Provide ShallowCopy() functions for adapters & evaluators of curves, 2d curves and surfaces. This will allow using copies of the same adapter in multi-thread calculations.

Kirill, unfortunately, we cannot declare functions in the Adapter3d_Curve class as pure virtual because, in particular, the OffsetCurve() function is implemented only for the GeomAdaptor_Curve and BRepAdaptor_Curve child classes.

OffsetCurve() could be declared as non-virtual as exception or we may implement it in descendants.

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

[revision removed]

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

[revision removed]

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

[revision removed]

> OffsetCurve() could be declared as non-virtual as exception or we may implement it in descendants.

Kirill, we have not implemented errors in Products after I tried switch virtual functions to pure.
http://jenkins-test-occt.nnov.opencascade.com/job/CR32485_3-master-Author-Products-MacOS-opt-compile/1/parsed_console/

remarks fixed in branch CR32485_3 exclude switch to pure virtual functions as it causes problems in Products

> Kirill, we have not implemented errors in Products after I tried switch virtual functions to pure.
I'm a little confused what is the problem updating Products - I guess there would be a couple of classes there, not something requiring considerable efforts.

But lets Mikhail to decide.

There are several adaptors in products having not implemented virtual functions.

HelixGeom_HelixCurve: ShallowCopy(), D3(), GetType()
AcisOther_AdaptorIntCur: ShallowCopy(), D3(), GetType(), Resolution()
AcisData_CurveLawAdaptor: ShallowCopy(), D3(), Resolution(), IsPeriodic(), Period()

There are different algorithms using adaptors, and each algorithm applies different requirements on the presence of virtual functions in adaptor. If we make all virtual functions "pure" then we shall need to implement them in all adaptors.
What are the benefits of this improvement? I see no one, only disadvantages, as some user applications may also suffer from this change.

> What are the benefits of this improvement?
The benefit is to clearly define which adaptor methods are mandatory to be implemented to be used within main OCCT algorithms.
So that mandatory ones like D0/D1 are declared as poor virtual methods, while optional could have default implementation throwing exception.

But if there are no methods in adaptors required to be useful, then OK, lets drop this idea.

There are different OCCT algorithms, which of them to consider 'main'? GCPnts, Extrema, Approx, LProp, and many others, each of them use its own set of adaptor functions.

We could gather the list of functions common for all algorithms. But if we do so, we do not clearly define all mandatory functions needed for all algorithms.

If we make pure virtual all functions that are used in all algorithms, then if the user wants to implement an adaptor for some particular algorithm we force him to implement a set of functions that are not needed for that algorithm.

> There are different OCCT algorithms...
OK, got it.

tests results^
http://jenkins-test-occt.nnov.opencascade.com/view/CR32485_3-master-Author/view/COMPARE/

For integration:
occt - CR32485_3
products - none

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: 17495.94000000042 / 17441.55000000031 [+0.31%]
Products
Total CPU difference: 11565.440000000113 / 11534.270000000102 [+0.27%]
Windows-64-VC14:
OCCT
Total CPU difference: 19307.140625 / 19200.421875 [+0.56%]
Products
Total CPU difference: 12917.859375 / 12874.53125 [+0.34%]


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]

Branch [archived branch] has been deleted by Participant.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]