Notes
 (0070584) msv 2017-09-15 14:43
 We cannot consider this issue as a bug. Moreover it cannot be considered as a regression. The changes made in the scope of the bug #28724 are conscious. And these changes are described in the Upgrade guide. So, I propose to close this issue.
 (0070588) Roman Lygin 2017-09-15 15:03
 It is up to you, so please decide as you deem appropriate but I find the logic broken. Geometrically-wise, periodicity is a more strict property than closedness, so closedness is a mandatory condition of periodicity. The curve can be closed but not periodic. But if the curve is periodic then it is closed. The fix breaks this geometrical principle and results in periodic but open curve, which is weird. #28724 is not publicly accessible so no clues what's in there. Release Notes just barely mentions the fix, no background/justification either.
 (0070595) msv 2017-09-15 16:04
 You can read commit message corresponding to the fix of 28724. Also, you can read upgrade guide, as I told. Periodicity has no common with closeness. Geometry is considered periodic if by adding the period to the parameter we obtain the same 3D point. And it is the case for a part of circle.
 (0070654) Roman Lygin 2017-09-18 17:43
 (0070666) nbv 2017-09-19 10:41    (edited on: 2017-09-19 10:42)
 Dear Roman, Thank you for your links to the User guides of other CAD-systems. However, I would like to draw your attention to the following: 1. As it has already been said by Mikhail (MSV) “Geometry is considered periodic if by adding the period to the parameter we obtain the same 3D point.” (please see the message 0029115:0070595). 2. All questions in the domain. I.e. if any circle is defined in the interval [PI/2, 3*PI/2] then we have the following situation. Let us consider some parameter (e.g. PI) associated with some 3D-point. If we add or subtract the period (2*PI) we will obtain the point which is out of the domain [PI/2, 3*PI/2]. By the same reason, this arc of circle cannot be considered as periodic, in fact, in spite of new 3D-point being the same as for previous parameter (PI). 3. Remark in item 2 is excepted by the fact that the current version of OCCT allows extending surfaces/curves out of their true domain. Yes, it leads to some problems. Therefore, the issue #23675 (P-curves of a face are out of the domain of the face) has been created in order to forbid to work out of domain (this bug is private and has not been integrated yet). However, the current version of OCCT allows it. Therefore, the point with parameter (-PI) on the circle (see item 2) can exist. Therefore, even above described circle can be considered as periodic. So, despite this question being contentious, new behavior (entered in frame of the issue #28724) DOES NOT contradict the main OCCT-conception. ------ Now, let me give a few words about the fix #28724. All reasons are given in the commit message: “The main reason of the regression is that the Extrema algorithm finds the truth extrema point but cannot adjust it to the range of given circle. It is connected with the fact that Geom(2d)Adaptor_Curve::IsPeriodic() method returns false for given circle because adaptor contains a piece of the circle which is not closed.” I.e. the extrema point was found but cannot be adjusted to the true circle domain. After the fix, it is adjusted correctly. ----- We agree that the fix leads to global changes in customer code. Therefore, we have written corresponding notes in Upgrade guide. And I regret but corresponding codes in customer applications shall be rewritten. New example of the code can be taken from changes in “GeomFill_NSections.cxx” made in frame of the issue #28724. I.e. Use ```const Standard_Real aParF = AC1.FirstParameter(); const Standard_Real aParL = AC1.LastParameter(); const Standard_Real aPeriod = AC1.IsPeriodic() ? AC1.Period() : 0.0; if ((aPeriod != 0.0) && (Abs(aParL - aParF - aPeriod) < Precision::PConfusion())) ``` Instead of ```if(AC1.IsPeriodic()) ```
 (0070711) nbv 2017-09-20 13:42    (edited on: 2017-09-20 13:46)
 After discussions, we decide 1. To return previous behavior of adaptors. 2. To add in Adaptor*-classes new methods (e.g. IsBasisPeriodic() or AdjustPeriodic()). 3. To use new methods in algorithms where analogical problem exists. Additionally, new synthetic cases should be created to cover base low-level algorithms (such as, all extremas, intersector, classifier).
 (0070716) Roman Lygin 2017-09-20 18:18
 (0070722) msv 2017-09-21 02:08
 I agree, please proceed. However, if you add IsBasisPeriodic method you will be forced to add also the method BasisPeriod(), and may be others. I would prefer adding AdjustPeriodic method, which will do nothing in general case.
 (0070724) azv 2017-09-21 08:03
 Dear Mikhail, Nikolay, I'm very confused by the proposed solution. What is a purpose to implement such strange behavior? In my opinion, methods like AdjustPeriodic() are applicable only if the adaptor returns true by IsPeriodic() methods. Unfortunately, in case of half circle (for example, in the range [2*pi, 3*pi]) IsPeriodic() should return false, thus there is no case to adjust. Method IsBasisPeriodic() is more natural, but it brokes a role of adaptor to hide original curve from the user. Moreover, two methods IsPeriodic() and IsBasisPeriodic() will disorient a user which one should be used. I also think that the algorithms (Extrema, for example) should take care of the parametric range of the curve. I mean, that the Extrema algorithms for the case above (half circle in the range [2*pi, 3*pi]) have to return parametric value in the given range, but not in a real domain of underlying circle [0, 2*pi].
 (0070727) msv 2017-09-21 09:40
 Dear Artem, I understand your confusion. However, Extrema has only instance of adapter to make decision about actions needed to complete its job, and we should provide it with convenient interface to do this job, and at the same time to minimize code duplication. For that, it is natural to insert the method of adjusting periodicity in the adapter. To avoid confusion with IsPeriodic returning false, we can name the new method something like TryPutPointInside. If you have another idea, please share it.
 (0070728) nbv 2017-09-21 09:46
 Dear Artem, Discussion about "IsPeriodic" is impasse (it can be continued infinitely). I think, all questions will be solved by the detailed documentation about new methods. So, these new methods will be able to be added and used in necessary algorithms. > I also think that the algorithms (Extrema, for example) should take care of the parametric range of the curve. I mean, that the Extrema algorithms for the case above (half circle in the range [2*pi, 3*pi]) have to return parametric value in the given range, but not in a real domain of underlying circle [0, 2*pi]. Unfortunately, low-level extrema (which finds all solutions itself) works neither curve nor adaptor. It works with gp_circle. So, it cannot return the point in range different from [0,2*pi]. All adjustments are made in higher level. Moreover, what about other algorithms? If we try to intersect periodic B-spline defined in "shifted" range with another B-spline. In the current version of OCCT, all points of found Walking-line will be adjusted. However, will it be done if both arguments are considered as not periodic? So, we must insert correspond piece of code in all algorithms and support it by the synthetic cases.
 (0070729) azv 2017-09-21 10:14
 nbv >> Unfortunately, low-level extrema (which finds all solutions itself) works neither curve nor adaptor. It works with gp_circle. So, it cannot return the point in range different from [0,2*pi]. All adjustments are made in higher level. That's what I tried to say above. Adaptor should not take care about the parameter of the point, the Extrema algorithm should do it. nbv >> Moreover, what about other algorithms? If we try to intersect periodic B-spline defined in "shifted" range with another B-spline. In the current version of OCCT, all points of found Walking-line will be adjusted. However, will it be done if both arguments are considered as not periodic? So, we must insert correspond piece of code in all algorithms and support it by the synthetic cases. Sorry, but your proposal leads to changing this code too. What are the benefits of your approach?
 (0070730) nbv 2017-09-21 10:24
 > That's what I tried to say above. Adaptor should not take care about the parameter of the point, the Extrema algorithm should do it. This solution leads to copy-paste the same code fragment in different places of OCCT. So, it is better to create some global function (e.g. as method of Adaptor* class) and to call it from these places. The main idea is that the new function must be global and available for main OCCT-algorithms. Therefore, Adaptor is a good place for it. > Sorry, but your proposal leads to changing this code too. What are the benefits of your approach? New solution has not any benefits. However, in the current MASTER, all points will be adjusted correctly.
 (0071309) abv 2017-10-09 14:22
 Note discussion of apparently the same subject here: https://dev.opencascade.org/index.php?q=node/1155 [^]
 (0074668) git 2018-03-19 18:37
 Branch CR29115 has been created by nbv. SHA-1: 023408bad3fef54bff393bea083214544bac987d Detailed log of new commits: Author: nbv Date: Wed Feb 21 17:31:37 2018 +0300     0029115: [Regression] GeomAdaptor mistakenly reports non-periodic curves to be periodic          1. Section "Concept of periodicity applied in OCCT-algorithms" has been added in Doxigen-documentation          2. Method IsPeriodic() of classes Geom(2d)_TrimmedCurve has changed its behavior. Please see the documentation about correspond methods.          3. Methods IsUPeriodic() and IsVPeriodic() has changed their behavior.          4. Methods Geom_SurfaceOfLinearExtrusion::UPeriod() and Geom_SurfaceOfRevolution::VPeriod() has changed their behavior.          5. Method GeomLib::IsClosed(...) has been added in order to check closure of 3D/2D curve with some tolerance.          6. Methods ShapeAnalysis_Curve::IsPeriodic(...), BRepBlend_HCurve2dTool::IsPeriodic(...), Contap_HCurve2dTool::IsPeriodic(...), IntPatch_HCurve2dTool::IsPeriodic(...), BRepBlend_HCurveTool::IsPeriodic(...) have been removed.          7. Method             void BOPTools_AlgoTools2D::AdjustPCurveOnFace (const TopoDS_Face& theF,                                                       const Handle(Geom_Curve)& theC3D,                                                       const Handle(Geom2d_Curve)& theC2D,                                                       Handle(Geom2d_Curve)& theC2DA,                                                       const Handle(IntTools_Context)& theContext = Handle(IntTools_Context)());          has been removed.          8. The following classes have become abstract (some methods have become pure virtual): Adaptor2d_Curve2d, Adaptor3d_Curve, Adaptor3d_Surface.          9. Methods ChFiDS_ElSpine::IsClosed() GeomFill_SnglrFunc::IsClosed(), ProjLib_CompProjectedCurve::IsClosed(), ProjLib_CompProjectedCurve::IsPeriodic(), ProjLib_CompProjectedCurve::Period() have been created.          10. Methods GeomLib::AllowExtend(...), GeomLib::AllowExtendUParameter(...) and GeomLib::AllowExtendVParameter(...) have been created.          11. Interface of Geom_ConicalSurface::Apex(...) method has been corrected insignificantly.          12. Method IntTools_Tools::IsClosed(...) has been removed.
 (0074690) git 2018-03-20 10:58
 Branch CR29115 has been updated forcibly by nbv. SHA-1: 6d183b787422b4f46d221d933f7e638c5c348e87
 (0074778) git 2018-03-22 19:34
 Branch CR29115 has been updated forcibly by nbv. SHA-1: f7d79f5f580ec4c4c6835473390a0ce7ec6f7fc0
 (0074796) git 2018-03-23 10:14
 Branch CR29115 has been updated forcibly by nbv. SHA-1: 61a11f04a19fe1b891b550aaec86aebeba6cef77
 (0074909) git 2018-03-26 10:53
 Branch CR29115 has been updated forcibly by nbv. SHA-1: 8f3a6b85efa1584dc5134d02228b314caac75777
 (0074974) git 2018-03-27 17:26
 (0074984) nbv 2018-03-28 10:04
 Dear Mikhail, Please review CR29115_1 branch for OCCT and CR29115prod branch for OCCT-products. Test results are here: http://jenkins-test-11.nnov.opencascade.com:8080/view/CR29115-CR29115prod_NBV/. [^] There are some remarks: 1. About the messages 0029115:0071309 and 0029115:0070711. Currently, we allow trimmed curve/surface and adaptors being periodic if they are based on periodic curves. Any trim boundaries are not taken into account. This behavior has been documented. 2. Differences in images are found: a. IMAGE sat doc_1 B8: B8.png differs    IMAGE sat doc_1 F4: F4.png differs    IMAGE sat load B3: B3.png differs    IMAGE sat doc_2 A3: A3.png differs    IMAGE sat doc_2 A1: A1.png differs    IMAGE sat doc_2 A4: A4.png differs    IMAGE sat doc_2 A2: A2.png differs    ... Seam-edge has been rotated. It is not regressions/improvements because seam-edges can take any locations. b. IMAGE boolean bopfuse_complex J6: J6.png differs    IMAGE boolean bfuse_complex M1: M1.png differs    IMAGE boolean bfuse_complex K6: K6.png differs    IMAGE boolean bcut_complex H8: H8.png differs    IMAGE heal split_closed_faces F4: F4.png differs    IMAGE heal split_continuity_advanced ZF1: ZF1.png differs    IMAGE heal surface_to_bspline F6: F6.png differs    ... The reason of this behavior is explained in the issue #23675: P-curves of a face are out of the domain of the face. DBRep_IsoBuilder processes periodic and not periodic faces (which are out of the surface's domain) differently. So, the structure of the shape has not been changed. We just have another set of iso-lines. In my opinion, it is not a regression. Moreover, the new result is looked better. c. IMAGE omf advanced_meshcut Z6: Z6.png differs    IMAGE omf advanced_meshfuse ZB6: ZB6.png differs    IMAGE omf advanced_meshfuse V5: V5.png differs     Meshing algorithm works differently for periodic and not-periodic faces. However, it is not a regression/improvement. Other differences are explained in the commit message. 3. I kindly recommend you to start reviewing with documentation. It is really describes and explains all made changes.
 (0075097) msv 2018-04-02 11:28
 Remarks to documentation: dox/user_guides/modeling_data/modeling_data.md - 790: must be "of their curves" - 798: "In OCCT, a surface is set in parametric form" - 810: "The methods analogical to GeomLib::AllowExtend(...) that can be applied to a surface are called..." - 856: must be "of their surfaces" - 858: We must also introduce the methods IsPeriodic(), IsUPeriodic(), IsVPeriodic(). Otherwise it is unclear from the text how to avoid calling Period() in non-periodic cases. - 862: "the behavior of these methods"
 (0075099) msv 2018-04-02 12:16
 src/Adaptor2d/Adaptor2d_Curve2d.hxx - 89: extra "//!" - Making the methods pure virtual is non consistent here: 1) their implementation in cxx is not removed; 2) due to behavior of all other methods. src/Adaptor2d/Adaptor2d_HCurve2d.hxx - 79: extra "//!" src/Adaptor3d/Adaptor3d_Curve.hxx - 91: extra "//!" - Making the methods pure virtual is non consistent with behavior of all other methods. src/Adaptor3d/Adaptor3d_Surface.hxx - Making the methods pure virtual is non consistent with behavior of all other methods. To be continued.
 (0075109) git 2018-04-02 15:55
 (0075110) nbv 2018-04-02 15:55
 Dear Mikhail, Please review CR29115_2 branch
 (0075117) msv 2018-04-02 16:52
 src/GeomLib/GeomLib.hxx - 35: double declaration of Geom_Curve - 38: unused declaration of Adaptor3d_Surface - 60,61: unused includes - 201: "defines" => "determines" - 223: why this method takes non-handle curve unlike IsClosed()? Make uniform parameters. - The name AllowExtend is not exact. The better name is IsTrimAllowed. The same is for methods IsUTrimAllowed, IsVTrimAllowed. src/GeomLib/GeomLib.cxx - 2760: make protection against infinite FirstVParameter. - 2790: make protection against infinite FirstUParameter. - 2792: make protection against null iso curve. - 2838: decrease zone of invalidity instead of increasing it. To be continued.
 (0075120) git 2018-04-02 17:56
 (0075123) git 2018-04-02 18:23
 Branch CR29115_3 has been updated forcibly by nbv. SHA-1: 171d6639631ad634c40ab659910839d00342aa46
 (0075136) msv 2018-04-03 09:59
 (0075137) msv 2018-04-03 10:20
 dox/user_guides/modeling_data/modeling_data.md - 800: "any surface always has" Put in commit message a pair of words about the main changes (that adaptors and trimmed curves periodicity is always got from the basis geometry).
 (0075138) abv 2018-04-03 11:23
 (0075148) git 2018-04-03 13:45
 (0075160) git 2018-04-04 10:31
 Branch CR29115_4 has been updated forcibly by nbv. SHA-1: a6f2fad6fa96f4d91b0a2655688e8906b410aa12
 (0075174) git 2018-04-04 19:32
 (0075182) git 2018-04-05 12:27
 Branch CR29115_5 has been updated forcibly by nbv. SHA-1: 0fc7e7d114e86ebe97365e143e0987d14c010919
 (0075192) git 2018-04-05 18:53
 Branch CR29115_5 has been updated forcibly by nbv. SHA-1: 5181bc771e9fdffe22f70cda1ad71b0bb455f7d1
 (0075205) git 2018-04-06 09:56
 Branch CR29115_5 has been updated forcibly by nbv. SHA-1: 6b226e703de9b52f8a1f90d682fe62d3571806ba
 (0075392) git 2018-04-12 10:28
 Branch CR29115_5 has been updated forcibly by nbv. SHA-1: d06060c900f7a862615cc2518e41e4c1043229cc
 (0075394) nbv 2018-04-12 10:31
 Branches CR29115_5 and CR29115prod have been rebased on the current MASTER.
 (0075423) git 2018-04-12 14:53
 Branch CR29115_5 has been updated forcibly by nbv. SHA-1: 651607e9c47e0bc48a1d6c237b3455d973d0dc46
 (0075465) git 2018-04-16 15:59
 Branch CR29115_5 has been updated forcibly by nbv. SHA-1: 5ed5cf59f24f22244c7daefa667bd4f0d75c95c0
 (0076383) git 2018-05-29 09:18
 (0076387) nbv 2018-05-29 11:05
 Branches have been rebased on the current MASTER and retested now.
 (0076806) git 2018-06-18 09:48
 Branch CR29115_6 has been updated forcibly by nbv. SHA-1: 4d663e6dc4c68cfd28e063bb42f6c19590228242
 (0076810) nbv 2018-06-18 13:34
 Branches have been rebased on the current MASTER and retested now.
 (0077646) git 2018-07-16 10:34
 Branch CR29115_6 has been updated forcibly by nbv. SHA-1: 6847963ebb002ba87d1fea2a7aa66522f57e2ae9
 (0077651) git 2018-07-16 12:17
 (0077782) git 2018-07-18 14:03
 Branch CR29115_6 has been updated forcibly by nbv. SHA-1: 3676e97e869ca88b821f96fefb5a50130c576e60
 (0077952) git 2018-07-24 09:24
 Branch CR29115_6 has been updated forcibly by nbv. SHA-1: 4f1bd132304e5caf4fc5597f6e5bbe36ec209f4c
 (0077959) nbv 2018-07-24 13:09
 Branches CR29115_6 and CR29115prod have been rebased on the current MASTER and retested.
 (0078199) git 2018-07-30 17:35
 Branch CR29115_6 has been updated forcibly by nbv. SHA-1: 63ce78068a3f97075ea96b3fb3f4460dd1f7b926
 (0078579) git 2018-08-13 10:44
 Branch CR29115_6 has been updated forcibly by nbv. SHA-1: acc5cc2043c99a702baaf233ca3b977871f00303
 (0078792) git 2018-08-27 10:57
 Branch CR29115_6 has been updated forcibly by nbv. SHA-1: 0656b833886893b27e8dfe94ecb0e09b0f5b9a9a
 (0079076) git 2018-09-10 09:34
 Branch CR29115_6 has been updated forcibly by nbv. SHA-1: c740222a44402034618afafc4cd633a8c9afc3fc
 (0079971) git 2018-10-15 17:30
 Branch CR29115_6 has been updated forcibly by nbv. SHA-1: 9a19a14be8464a014aa9bc9174e8aa8e99270bd7
 (0080443) git 2018-10-29 11:42
 Branch CR29115_6 has been updated forcibly by nbv. SHA-1: adcec72255517254f560481b0fa6c14737ab239a
 (0080802) git 2018-11-07 10:13
 Branch CR29115_6 has been updated forcibly by nbv. SHA-1: 44763777981692698c5375f2c99f590db922734f
 (0081445) git 2018-12-11 09:26
 Branch CR29115_6 has been updated forcibly by nbv. SHA-1: 0d4f55c2aee1b21942224c76f0ec418196e6f1dc