MantisBT - Community
View Issue Details
0028880Community[OCCT] OCCT:Modeling Algorithmspublic2017-06-30 11:552020-09-11 17:51
eryar 
bugmaster 
normalfeature 
verifiedfixed 
WindowsVC++ 201564 bit
[OCCT] 7.1.0 
[OCCT] 7.5.0 
bugs/moddata_1/buc60854
0028880: Modeling Algorithms - add missing BRepFeat_SplitShape::Right() getter
There is a Left() to get the left shapes of the split,
I think it should have a Right() to get the other parts of the split shape.
N/A
No tags attached.
Issue History
2017-06-30 11:55eryarNew Issue
2017-06-30 11:55eryarAssigned To => msv
2017-07-03 10:09msvNote Added: 0067927
2017-07-03 10:09msvAssigned Tomsv => eryar
2017-07-03 10:09msvStatusnew => feedback
2017-07-03 11:26eryarNote Added: 0067930
2017-07-03 12:50msvNote Added: 0067939
2017-07-03 12:50msvAssigned Toeryar => msv
2017-07-03 12:50msvStatusfeedback => assigned
2020-02-13 17:26utverdovAssigned Tomsv => antonavt
2020-03-02 12:17gitNote Added: 0090757
2020-03-02 12:19antonavtAssigned Toantonavt => msv
2020-03-02 15:55kgvSummaryAdd Right function for BRepFeat_SplitShape => Modeling Algorithms - add missing BRepFeat_SplitShape::Right() getter
2020-03-02 15:55kgvDescription Updatedbug_revision_view_page.php?rev_id=22621#r22621
2020-03-02 17:14msvNote Added: 0090778
2020-03-02 17:14msvAssigned Tomsv => eryar
2020-03-02 17:14msvAssigned Toeryar => antonavt
2020-03-03 22:59gitNote Added: 0090805
2020-03-03 23:01antonavtAssigned Toantonavt => eryar
2020-03-03 23:01antonavtAssigned Toeryar => msv
2020-03-04 10:18msvNote Added: 0090806
2020-03-04 10:19msvStatusassigned => resolved
2020-03-04 10:19msvSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=22628#r22628
2020-03-05 10:04msvNote Added: 0090827
2020-03-05 10:04msvAssigned Tomsv => antonavt
2020-03-05 10:04msvStatusresolved => assigned
2020-03-06 13:02gitNote Added: 0090848
2020-03-06 13:11antonavtNote Added: 0090849
2020-03-06 13:11antonavtAssigned Toantonavt => msv
2020-03-06 13:11antonavtStatusassigned => resolved
2020-03-10 09:44msvNote Added: 0090877
2020-03-10 09:44msvAssigned Tomsv => antonavt
2020-03-10 09:44msvStatusresolved => assigned
2020-03-10 15:13gitNote Added: 0090881
2020-03-10 15:15antonavtAssigned Toantonavt => msv
2020-03-10 15:15antonavtStatusassigned => resolved
2020-03-10 17:51msvNote Added: 0090884
2020-03-10 17:51msvAssigned Tomsv => antonavt
2020-03-10 17:51msvStatusresolved => assigned
2020-03-11 06:31gitNote Added: 0090889
2020-03-11 06:32antonavtAssigned Toantonavt => msv
2020-03-11 06:32antonavtStatusassigned => resolved
2020-03-11 10:12msvNote Added: 0090892
2020-03-11 10:12msvAssigned Tomsv => antonavt
2020-03-11 10:12msvStatusresolved => assigned
2020-03-18 08:37gitNote Added: 0090996
2020-03-18 08:38antonavtAssigned Toantonavt => msv
2020-03-18 08:38antonavtStatusassigned => resolved
2020-03-18 09:08kgvNote Added: 0090998
2020-03-18 09:08kgvNote Edited: 0090998bug_revision_view_page.php?bugnote_id=90998#r22678
2020-03-18 10:18msvNote Added: 0091007
2020-03-18 10:57msvNote Added: 0091009
2020-03-18 10:58msvAssigned Tomsv => antonavt
2020-03-18 10:58msvStatusresolved => assigned
2020-03-18 16:42antonavtNote Added: 0091037
2020-03-18 16:44antonavtNote Deleted: 0091037
2020-03-19 11:23gitNote Added: 0091045
2020-03-19 11:24antonavtAssigned Toantonavt => msv
2020-03-19 11:24antonavtStatusassigned => resolved
2020-03-23 10:20gitNote Added: 0091188
2020-03-23 10:25msvNote Added: 0091189
2020-03-23 12:03msvNote Added: 0091192
2020-03-23 12:03msvAssigned Tomsv => bugmaster
2020-03-23 12:03msvStatusresolved => reviewed
2020-03-26 12:44bugmasterNote Added: 0091262
2020-03-26 12:44bugmasterStatusreviewed => tested
2020-03-26 12:48bugmasterTest case number => bugs/moddata_1/buc60854
2020-03-28 13:36bugmasterChangeset attached => occt master 68064d7b
2020-03-28 13:36bugmasterStatustested => verified
2020-03-28 13:36bugmasterResolutionopen => fixed
2020-03-28 13:46gitNote Added: 0091327
2020-03-28 13:47gitNote Added: 0091330
2020-03-28 13:47gitNote Added: 0091331
2020-03-28 13:47gitNote Added: 0091332
2020-09-11 17:51abvTarget Version => 7.5.0

Notes
(0067927)
msv   
2017-07-03 10:09   
Right() is simply realized by getting all faces from the result except those returned by Left(). Usually this is implemented on application side. Do you really insist on implementation of this method in the algorithm?
(0067930)
eryar   
2017-07-03 11:26   
Hi msv,
Yes, I really insist on implementation the 'Right()' method.

When using the split algorithm, User want to do some operation on the splitted two parts.

Meanwhile there have classify algorithm in the BRepFeat_SplitShape, so it is more straightforward and efficient to provide both Left() and Right() to get the splitting two parts of the splitted shape.

Best Regards,
(0067939)
msv   
2017-07-03 12:50   
OK, we will make this development as we have free resources. If it is important and urgent for you then you can contribute by speeding up development via support contact.
(0090757)
git   
2020-03-02 12:17   
Branch CR28880 has been created by antonavt.

SHA-1: 1697c4525bc4777647494e011530ae4d26ab9b48


Detailed log of new commits:

Author: antonavt
Date: Mon Mar 2 12:16:14 2020 +0300

    Added Right() to BRepFeat_SplitShape
(0090778)
msv   
2020-03-02 17:14   
Remarks:
- Implementation of Right() is not optimal. Its complexity is O(n^2), because the method Contains() of the List class makes iterations to find the goal. Consider using a Map.
- The calculation of myRight must be cached not to compute it each time the method is called. Also, this cache is to be reset on initialization with another shape.
(0090805)
git   
2020-03-03 22:59   
Branch CR28880 has been updated by antonavt.

SHA-1: 1f6fbfd89e0d51cf9c79eb82cd109435178398b1


Detailed log of new commits:

Author: antonavt
Date: Tue Mar 3 22:42:13 2020 +0300

    0028880: Modeling Algorithms - add missing BRepFeat_SplitShape::Right() getter
    
    - A list is replaced by a map in the method "Contains";
    - Added method "setMyRight" calculating myRight and called from Build().

Author: antonavt
Date: Tue Mar 3 21:55:10 2020 +0300

    0028880: Modeling Algorithms - add missing BRepFeat_SplitShape::Right() getter \n\n -tratata

(0090806)
msv   
2020-03-04 10:18   
Dear Anton, please change status to Resolved when put a bug to review stage.
(0090827)
msv   
2020-03-05 10:04   
New remarks follow.

Please try following the OCCT coding rules available in online doc:
https://dev.opencascade.org/doc/overview/html/occt_dev_guides__coding_rules.html [^]
Legacy code may not follow, but the newly inserted code must follow.

In particular, do not insert tabs (tune your editor to insert spaces). Also, name local variables using 'a' prefix (mapOfLeft -> aMapOfLeft, fac->aFace).

Move setMyRight() method off fields definition section to the protected section.

If the caller is not going to use Right() method then calculations made in setMyRight() are redundant. So, call it from Right() with condition if it is the first call after the latest Build(). Please read my above comment 0028880:0090778.

In commit message, all lines not intended to be put in the final commit must be prefixed by '#'.
(0090848)
git   
2020-03-06 13:02   
Branch CR28880 has been updated by antonavt.

SHA-1: 0622ee3882bb8673fb5d4edff15f81c1b89a179f


Detailed log of new commits:

Author: antonavt
Date: Fri Mar 6 12:53:27 2020 +0300

    0028880: Modeling Algorithms - add missing BRepFeat_SplitShape::Right() getter
    
    - Marqued myRight field as mutable;
    - Moved myRight clearing in Build();
    - Deleted setMyRight() method and moved its code inside Right() method with checking myRight emptiness.

(0090849)
antonavt   
2020-03-06 13:11   
I've done necessary corrections.
(0090877)
msv   
2020-03-10 09:44   
OK, one new small remark. In the line 89:
      const TopoDS_Face& aFace = TopoDS::Face(anExplorer.Current());
the variable aFace does not need to be of type TopoDS_Face. It is enough to have it of base type TopoDS_Shape. And therefore casting is not needed.

And now, to prepare for integration, please rebase the branch on new master and combine all is one commit with the proper message.
(0090881)
git   
2020-03-10 15:13   
Branch CR28880_2 has been created by antonavt.

SHA-1: a29ee42cbf7cb9d83528ef2d7ce1426d7d9ae0ec


Detailed log of new commits:

Author: antonavt
Date: Mon Mar 2 12:16:14 2020 +0300

    0028880: Modeling Algorithms - add missing BRepFeat_SplitShape::Right() getter
    
    Added Right() method to BRepFeat_SplitShape
(0090884)
msv   
2020-03-10 17:51   
You have not rebased new branch on new master. You based on origin/IR-2020-02-28.
origin/master is updated each weekend.

By the way, now, as you don't use TopoDS casting methods, please remove include of TopoDS.hxx
(0090889)
git   
2020-03-11 06:31   
Branch CR28880_3 has been created by antonavt.

SHA-1: a30abfe330684c3318d051e37807d3e70b837f22


Detailed log of new commits:

Author: antonavt
Date: Mon Mar 2 12:16:14 2020 +0300

    0028880: Modeling Algorithms - add missing BRepFeat_SplitShape::Right() getter
    
    Added Right() method to BRepFeat_SplitShape
(0090892)
msv   
2020-03-11 10:12   
OK with the code. It's perfect. And now we need to create a non-regression test case for this issue.

There is a test code in the file QABugs_14.cxx in the method BUC60854 that uses the Left() method of BRepFeat_SplitShape. There is the same-name draw command and the test case using it.

The test case is tests\bugs\moddata_1\buc60854 and can be run with the command "test bugs moddata_1 buc60854". Now it has 'bad' state and it is wrong. I have studied this test script and found out that the result is OK.

I propose to improve this test by making it valid. And, in addition, add in it testing of the new method Right(). For that, please add a new option in the command buc60854 so that to select which method Left() or Right() to use as the result.

All new modifications are to be done in the same branch.
(0090996)
git   
2020-03-18 08:37   
Branch CR28880_3 has been updated by antonavt.

SHA-1: 4e3e5c3dd936b8a67d34e943bd676dc37f0b4eab


Detailed log of new commits:

Author: antonavt
Date: Wed Mar 18 08:35:19 2020 +0300

    Merge branch 'master' of git.dev.opencascade.org:occt into CR28880

Author: antonavt
Date: Wed Mar 18 08:24:45 2020 +0300

    0028880: Modeling Algorithms - add missing BRepFeat_SplitShape::Right() getter
    
    - Modified test case buc60854. Now its state is "OK";
    - Added an option in command buc60854 which allows to select method Left() or Right();
    - Modified method BUC60854 in QABugs_14.cxx.

(0090998)
kgv   
2020-03-18 09:08   
> Merge branch 'master' of into CR28880
Please prepare branch without merges.

(0091007)
msv   
2020-03-18 10:18   
Dear Anton,
We maintain master branch being a set of flat commits without merge loops. So, when you actualize your branch on the current master please do 'git rebase' instead of 'git pull'.
(0091009)
msv   
2020-03-18 10:57   
Please update the help of the command BUC60854 (line 1059), considering new option.

Update the test case buc60854 with adding check of Right() function and check its result.
(0091045)
git   
2020-03-19 11:23   
Branch CR28880_4 has been created by antonavt.

SHA-1: 4ab08aed4c1f12bb26209a6f22a2870fb2bf7a56


Detailed log of new commits:

Author: antonavt
Date: Mon Mar 2 12:16:14 2020 +0300

    0028880: Modeling Algorithms - add missing BRepFeat_SplitShape::Right() getter
    
    - Added Right() method to BRepFeat_SplitShape;
    - Added an option in command buc60854, which allows to select Left() or Right() method;
    - Modified method BUC60854 in QABugs_14.cxx;
    - Added new option to the help of command buc60854;
    - Modified test case buc60854. Now its state is "OK";
    - Added check Right() in buc60854 test case.
(0091188)
git   
2020-03-23 10:20   
Branch CR28880_4 has been updated forcibly by msv.

SHA-1: 7bcb535cefdd35eca2cabea006bfdbc41716a1bc
(0091189)
msv   
2020-03-23 10:25   
I have rebased on master and made 2 minor corrections.
Tests are running
http://jenkins-test-12.nnov.opencascade.com/view/CR28880-master-msv/view/COMPARE/. [^]
(0091192)
msv   
2020-03-23 12:03   
For integration:
OCCT - CR28880_4
Products - none
(0091262)
bugmaster   
2020-03-26 12:44   
Combination -
OCCT branch : WEEK-13
master SHA - 89180f98222651faa3f1cffe9f6d5a9abae8a4e8
fe4497f3246e6bc1ced97ac331c148f0809ded15
Products branch : WEEK-13 SHA - f10b867b449ebfa55e0a3c8cb276ae511f9cf7f2
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: 16843.850000000195 / 16846.320000000138 [-0.01%]
Products
Total CPU difference: 11298.030000000097 / 11306.210000000074 [-0.07%]
Windows-64-VC14:
OCCT
Total CPU difference: 18288.453125 / 18268.796875 [+0.11%]
Products
Total CPU difference: 13126.15625 / 13110.609375 [+0.12%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0091327)
git   
2020-03-28 13:46   
Branch CR28880_4 has been deleted by inv.

SHA-1: 7bcb535cefdd35eca2cabea006bfdbc41716a1bc
(0091330)
git   
2020-03-28 13:47   
Branch CR28880_3 has been deleted by inv.

SHA-1: 4e3e5c3dd936b8a67d34e943bd676dc37f0b4eab
(0091331)
git   
2020-03-28 13:47   
Branch CR28880_2 has been deleted by inv.

SHA-1: a29ee42cbf7cb9d83528ef2d7ce1426d7d9ae0ec
(0091332)
git   
2020-03-28 13:47   
Branch CR28880 has been deleted by inv.

SHA-1: 0622ee3882bb8673fb5d4edff15f81c1b89a179f