View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0028456Open CASCADEOCCT:Modeling Algorithmspublic2017-02-14 18:592017-09-29 16:25
ReporterisnAssigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version7.1.0 
Target Version7.2.0Fixed in Version7.2.0 
Summary0028456: BRepBuilderAPI_MakeFace modifies the input shape
DescriptionBRepBuilderAPI_MakeFace modifies the input wire: updates the tolerances of wire's edges/vertices or/and updates flags/ranges.
It's proposed to add mutable flag for BRepBuilderAPI_MakeShape which defines if input shape can be modified during make-face operation or some of subshapes from input shape should be empty-copied to avoid modifying.
Steps To Reproducebox a 1 1 1
explode a w
setflags a_1 locked
mkplane result a_1

...
also test cases: mkface mkplane A1 A2 (with additional setflags <inp_sh> locked before mkplane operation)
TagsNo tags attached.
Test case numberheal same_parameter_locked; heal update_tolerance_locked

Relationships

related to 0027166 assignedmsv Non-destructive principle in algorithms 

Activities

git

2017-02-17 15:15

administrator   ~0063887

Branch CR28456 has been created by isn.

SHA-1: 29f72a0e4eaf3d1975d888d5a3e060ad0fb44ee5


Detailed log of new commits:

Author: isn
Date: Tue Feb 14 18:42:54 2017 +0300

    0028456: BRepBuilderAPI_MakeFace modifies the input shape
    
    (draft version)
    1) New 'MutableInput' flag have been added
    2) BRepLib::UpdateTolerances() now supports non-mutable input (finished, but seems to be unoptimal)
    3) BRepLib::SameParameter() also supports non-mutable, but this code unfinished yet

git

2017-02-28 20:11

administrator   ~0064041

Branch CR28456_1 has been created by isn.

SHA-1: 28db31477208ac8566456c113a85e3716efac4f7


Detailed log of new commits:

Author: isn
Date: Tue Feb 14 18:42:54 2017 +0300

    0028456: BRepBuilderAPI_MakeFace modifies the input shape
    
    1) New 'MutableInput' flag have been added
    2) BRepLib::UpdateTolerances() & BRepLib::SameParameter() now supports non-mutable input
    3) new "-mi" option have been added to mkplane command

msv

2017-03-09 11:55

developer   ~0064189

Remarks:

Commit message:
1) have => has
2) supports => support
3) have => has
Point that you added new flag in the ancestor algorithm, root of all others. Point the default value.

src\BRepBuilderAPI\BRepBuilderAPI_MakeShape.hxx
- Point the default value of the flag.
- 69: use const for argument.
- It is better to make myMutableInput private, and methods IsMutableInput and SetMutableInput non-virtual (so that to safely call from constructor).

src\BRepBuilderAPI\BRepBuilderAPI_MakeFace.hxx
- 120: wrap the long line.

src\BRepBuilderAPI\BRepBuilderAPI_MakeFace.cxx
- 269: use the method SetMutableInput.

src\BRepLib\BRepLib_MakeFace.cxx
- 269: move into the scope where it is used. Follow coding rules when naming new variables (prefix 'a', see https://dev.opencascade.org/doc/overview/html/occt_dev_guides__coding_rules.html#occt_coding_rules_2_3). What is ECSH? Can you drop a line in comments?
- 273: what is wrong about forced?
- 277: not clear question

src\BRepLib\BRepLib.hxx
- Follow coding rules for naming of arguments (prefix 'the').
- 141, 149: add description
- 142,165,174: use const for non-modifiable arguments.
- 156-159: absence of punctuation makes hard to understand the text. Also, use
tags to mark the end of line.
"and the new shape will be returned." => "In this case the new shape will be returned."
- 161: "This" => "These".

src\BRepLib\BRepLib.cxx
- use BRepTools_ReShape instead of data map for mapping old->new shape.
- 1201: no such method in the class.
- Please add standard comment header before each method.
- 757: mismatch name
- 834: what kind of checks do you mean?
- 897-899: use BRep_Builder::UpdateFace
- 949-950, 1618: I don't see why do you separate vertices, edges and faces.
- 1784: not clear question

src\BRepTest\BRepTest_SurfaceCommands.cxx
- 185: yes, add usage
- 196: historical reasons, let's leave here as it is.
- Change option name to "-nmi". Character '-' here has no meaning of negation, and "-mi" can be confusing.

git

2017-03-16 18:11

administrator   ~0064388

Branch CR28456_1 has been updated by isn.

SHA-1: f4feee43a535e63ea6bc633814ef3c93e723b685


Detailed log of new commits:

Author: isn
Date: Fri Mar 10 15:19:23 2017 +0300

    corrections

git

2017-03-17 17:51

administrator   ~0064409

Branch CR28456_1 has been updated forcibly by isn.

SHA-1: d0a816dd3742bdfa5ea50e376d4cfc13a08d2f08

msv

2017-03-21 11:15

developer   ~0064573

Remarks:

- As we agreed, changes in MakeShape, MakeFace and BRepTest_SurfaceCommands.cxx are to be dropped.

src\BRepTools\BRepTools_ReShape.hxx
- Add description of the method IsNewShape.
- What is the meaning of the name 'mySMap'? May be to rename it something like myNewShapes?

src\BRepLib\BRepLib.hxx
- 140: why 'AnEdge'? Please read coding rules. 'theEdge' is correct!
- add description of SameParameter at 142
- 149: remove the empty line
- 158: extra '.' at the end.
- update description of the method SameParameter at 164.
- add description of UpdateTolerances at 174
- remove argument isMutableInput from public methods. Methods with reshaper must write all changes in reshaper only. In implementation, make the current methods static and call them from API methods. It is better in implementation accept pointer to reshaper, in order to not create an instance when it is not needed. Null pointer means 'mutable input'. In method SameParameter make reshaper last argument.

src\BRepLib\BRepLib.cxx
- 912: missing header
- 1178: again, no such method in the class. How is it compiled?
- 1590: missing header

git

2017-03-31 15:09

administrator   ~0064832

Branch CR28456_1 has been updated by isn.

SHA-1: b419b5dd67175d34311c2659cb14fd5211efd00d


Detailed log of new commits:

Author: isn
Date: Fri Mar 31 14:27:35 2017 +0300

    corrections

git

2017-04-03 14:43

administrator   ~0064861

Branch CR28456_1 has been updated forcibly by isn.

SHA-1: ae5422b13d4d8c2782b45dae7ec3fc6e1cde3b91

git

2017-04-03 15:37

administrator   ~0064863

Branch CR28456_1 has been updated forcibly by isn.

SHA-1: 01f0e13443230ac722ac1ede44293c2340055f64

git

2017-04-03 16:14

administrator   ~0064864

Branch CR28456_1 has been updated forcibly by isn.

SHA-1: e4bb9b12b7eade926f939085901bc4a43f7e1124

msv

2017-04-03 16:59

developer   ~0064870

src/BRepTools/BRepTools_ReShape.hxx
- 142: 'have' -> 'has', and put point at end.

src/BRepLib/BRepLib.hxx
- 29-30: remove unused includes
- 146-150: use punctuation and capital letters to distinct sentences (otherwise doxygen will generate a mess).

src\BRepLib\BRepLib.cxx
- 996: use InternalUpdateTolerances here.

- Squash into one commit and put in the new branch.

msv

2017-04-03 16:59

developer   ~0064871

And please rebase on last master.

git

2017-04-05 14:47

administrator   ~0064936

Branch CR28456_2 has been created by isn.

SHA-1: d4dc3d145dc2f69eeb21d1c7cb5bf864e00d04bc


Detailed log of new commits:

Author: isn
Date: Tue Feb 14 18:42:54 2017 +0300

    0028456: BRepBuilderAPI_MakeFace modifies the input shape
    
    1) BRepLib::UpdateTolerances(..) & BRepLib::SameParameter(..) functions now support non-mutable input feature. reshaper is used to store modified copies of subshapes of original (input) shape(s) as substitutions.
    2) IsNewShape(..) method has been added to BRepTools_ReShape to check if the given shape has been recorded as a value

git

2017-04-05 21:31

administrator   ~0064953

Branch CR28456_2 has been updated forcibly by isn.

SHA-1: 9421eaead8adb2c34501e3be2d85b3ea69b9adf8

msv

2017-04-06 10:59

developer   ~0064965

- Add possibility in the commands sameparameter/fsameparameter to use the new safe API.
- Create several test cases for the new API of sameparameter in the category heal/same_parameter.
- Add information about new method of reshaper IsNewShape() in its documentation here: https://dev.opencascade.org/doc/overview/html/occt_user_guides__shape_healing.html#occt_shg_5_1

git

2017-04-10 17:54

administrator   ~0065109

Branch CR28456_2 has been updated by isn.

SHA-1: 6f244fcbdc60068869f57ca78dda6c46a5b5a33e


Detailed log of new commits:

Author: isn
Date: Mon Apr 10 17:51:02 2017 +0300

    update of tests/docs

msv

2017-04-11 10:06

developer   ~0065115

src\BRepTest\BRepTest_BasicCommands.cxx
- 330,367: there will be warning on other platforms; include sub-expressions in parentheses.

git

2017-04-11 15:38

administrator   ~0065142

Branch CR28456_2 has been updated by isn.

SHA-1: 1d39ab3455018ce1654ed6ba0825a9514c4d4028


Detailed log of new commits:

Author: isn
Date: Tue Apr 11 15:38:16 2017 +0300

    remark

msv

2017-04-11 16:33

developer   ~0065145

Reviewed.

apv

2017-04-12 10:16

tester   ~0065166

Dear BugMaster,

Branch CR28456_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: 1d39ab3455018ce1654ed6ba0825a9514c4d4028

Number of compiler warnings:
occt component:
   Linux: 6 (0 on master)
   Windows: 0 (0 on master)
   MasOS: 1 (0 on master)
products component:
   Linux: 64
   Windows: 0
   MacOS: 1210
New warnings have been detected during OCCT component building
on Linux:
http://jenkins-test-05.nnov.opencascade.com/view/CR28456_2-master/job/CR28456_2-master-OCCT-Debian70-64-opt-compile/1/warnings17Result/new/
on MacOS:
http://jenkins-test-05.nnov.opencascade.com/view/CR28456_2-master/job/CR28456_2-master-OCCT-MacOS-opt-compile/1/warnings7Result/

Regressions/Differences:
http://occt-tests/CR28456_2-master-OCCT/Debian70-64/summary.html
http://occt-tests/CR28456_2-master-OCCT/Windows-64-VC10/summary.html
bugs modalg_1 buc60896

Testing cases:
heal same_parameter_locked - OK
http://occt-tests/CR28456_2-master-OCCT/Debian70-64/summary.html#heal-same_parameter_locked
http://occt-tests/CR28456_2-master-OCCT/Windows-64-VC10/summary.html#heal-same_parameter_locked
heal update_tolerance_locked - OK
http://occt-tests/CR28456_2-master-OCCT/Debian70-64/summary.html#heal-update_tolerance_locked
http://occt-tests/CR28456_2-master-OCCT/Windows-64-VC10/summary.html#heal-update_tolerance_locked

Testing on Linux:
Total MEMORY difference: 91551894 / 91388139 [+0.18%]
Total CPU difference: 19598.810000000238 / 19613.190000000242 [-0.07%]

Testing on Windows:
Total MEMORY difference: 57863693 / 57855806 [+0.01%]
Total CPU difference: 18351.318035898534 / 18063.40259029849 [+1.59%]

apv

2017-04-12 10:18

tester   ~0065167

Dear Ilya,

Branch CR28456_2 has been rejected due to:
- additional warnings
- regressions/difference/improvements

git

2017-04-12 14:38

administrator   ~0065178

Branch CR28456_2 has been updated by isn.

SHA-1: 70eefcd674fc5da961f19d1356aedd936bceabf0


Detailed log of new commits:

Author: isn
Date: Wed Apr 12 14:38:26 2017 +0300

    elimination of warnings; corrections of test

isn

2017-04-12 14:52

developer   ~0065179

Last edited: 2017-04-12 14:55

 Dear Alexey,

>>New warnings have been detected during OCCT component building
Seems to be fixed now.

>>Test bugs vis bug5682
the new changes should not affect this test case...

>>Test bugs modalg_1 buc60896
I guess this script looks somewhat incorrect.
This line:
sameparameter result tol 1.e-2
uses additional (i.e. optional) tolerance value. However, 'tol' is not a tcl value in this context. Before the patch, 'sameparamter' command ignores this 'tol' word along with the next value (1.e-2, for ex.). Thus this draw-command uses a default tolerance value (1.e-7). So I think that the 'tol' word should be dropped. I correct the test case according to this.

Please compile&test the new commit.

apv

2017-04-13 11:06

tester   ~0065195

Dear BugMaster,

Branch CR28456_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: 70eefcd674fc5da961f19d1356aedd936bceabf0

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 0 (0 on master)
   MasOS: 0 (0 on master)
products component:
    Linux: 64
    Windows: 0
    MacOS: 1188

Regressions/Differences:
Not detected

Testing cases:
heal same_parameter_locked - OK
http://occt-tests/CR28456_2-master-OCCT/Debian70-64/summary.html#heal-same_parameter_locked
http://occt-tests/CR28456_2-master-OCCT/Windows-64-VC10/summary.html#heal-same_parameter_locked
heal update_tolerance_locked - OK
http://occt-tests/CR28456_2-master-OCCT/Debian70-64/summary.html#heal-update_tolerance_locked
http://occt-tests/CR28456_2-master-OCCT/Windows-64-VC10/summary.html#heal-update_tolerance_locked

Testing on Linux:
Total MEMORY difference: 91345181 / 91384662 [-0.04%]
Total CPU difference: 19708.93000000035 / 19613.25000000024 [+0.49%]

Testing on Windows:
Total MEMORY difference: 57862638 / 57855806 [+0.01%]
Total CPU difference: 18352.659644498486 / 18063.40259029849 [+1.60%]

git

2017-05-12 11:35

administrator   ~0065912

Branch CR28456 has been deleted by kgv.

SHA-1: 29f72a0e4eaf3d1975d888d5a3e060ad0fb44ee5

git

2017-05-12 11:35

administrator   ~0065913

Branch CR28456_1 has been deleted by kgv.

SHA-1: e4bb9b12b7eade926f939085901bc4a43f7e1124

git

2017-05-12 11:40

administrator   ~0065978

Branch CR28456_2 has been deleted by kgv.

SHA-1: 70eefcd674fc5da961f19d1356aedd936bceabf0

Related Changesets

occt: master b60e8432

2017-02-14 15:42:54

isn


Committer: bugmaster Details Diff		 0028456: BRepBuilderAPI_MakeFace modifies the input shape

1) BRepLib::UpdateTolerances(..) & BRepLib::SameParameter(..) functions now support non-mutable input feature. reshaper is used to store modified copies of subshapes of original (input) shape(s) as substitutions.
2) IsNewShape(..) method has been added to BRepTools_ReShape to check if the given shape has been recorded as a value		 Affected Issues
0028456
mod - dox/dev_guides/tests/tests.md Diff File
mod - dox/user_guides/shape_healing/shape_healing.md Diff File
mod - src/BRepLib/BRepLib.cxx Diff File
mod - src/BRepLib/BRepLib.hxx Diff File
mod - src/BRepTest/BRepTest_BasicCommands.cxx Diff File
mod - src/BRepTools/BRepTools_ReShape.cxx Diff File
mod - src/BRepTools/BRepTools_ReShape.hxx Diff File
mod - tests/bugs/modalg_1/buc60896 Diff File
mod - tests/heal/grids.list Diff File
add - tests/heal/same_parameter_locked/A1 Diff File
add - tests/heal/same_parameter_locked/A2 Diff File
add - tests/heal/same_parameter_locked/A3 Diff File
add - tests/heal/same_parameter_locked/A4 Diff File
add - tests/heal/same_parameter_locked/A5 Diff File
add - tests/heal/same_parameter_locked/A6 Diff File
add - tests/heal/same_parameter_locked/A7 Diff File
add - tests/heal/same_parameter_locked/end Diff File
add - tests/heal/update_tolerance_locked/A1 Diff File
add - tests/heal/update_tolerance_locked/A2 Diff File
add - tests/heal/update_tolerance_locked/A3 Diff File
add - tests/heal/update_tolerance_locked/A4 Diff File
add - tests/heal/update_tolerance_locked/end Diff File

Issue History

Date Modified Username Field Change
2017-02-14 18:59 isn New Issue
2017-02-14 18:59 isn Assigned To => isn
2017-02-14 19:00 isn Relationship added related to 0027166
2017-02-14 19:01 isn Steps to Reproduce Updated
2017-02-15 09:16 msv Status new => assigned
2017-02-15 09:16 msv Product Version => 7.1.0
2017-02-15 09:16 msv Summary BRepBuilderAPI_MakeFace modifies modifies the input shape => BRepBuilderAPI_MakeFace modifies the input shape
2017-02-17 15:15 git Note Added: 0063887
2017-02-28 20:11 git Note Added: 0064041
2017-03-01 11:56 isn Assigned To isn =>
2017-03-01 11:57 isn Assigned To => msv
2017-03-01 11:57 isn Status assigned => resolved
2017-03-09 11:55 msv Note Added: 0064189
2017-03-09 11:55 msv Assigned To msv => isn
2017-03-09 11:55 msv Status resolved => assigned
2017-03-16 18:11 git Note Added: 0064388
2017-03-17 17:51 git Note Added: 0064409
2017-03-17 19:59 isn Assigned To isn => msv
2017-03-17 19:59 isn Status assigned => feedback
2017-03-21 11:15 msv Note Added: 0064573
2017-03-21 11:15 msv Assigned To msv => isn
2017-03-21 11:15 msv Status feedback => assigned
2017-03-31 15:09 git Note Added: 0064832
2017-04-03 14:43 git Note Added: 0064861
2017-04-03 15:37 git Note Added: 0064863
2017-04-03 16:14 git Note Added: 0064864
2017-04-03 16:14 isn Assigned To isn => msv
2017-04-03 16:14 isn Status assigned => resolved
2017-04-03 16:59 msv Note Added: 0064870
2017-04-03 16:59 msv Assigned To msv => isn
2017-04-03 16:59 msv Status resolved => assigned
2017-04-03 16:59 msv Note Added: 0064871
2017-04-05 14:47 git Note Added: 0064936
2017-04-05 21:31 git Note Added: 0064953
2017-04-05 21:32 isn Assigned To isn => msv
2017-04-05 21:32 isn Status assigned => resolved
2017-04-06 10:59 msv Note Added: 0064965
2017-04-06 10:59 msv Assigned To msv => isn
2017-04-06 10:59 msv Status resolved => assigned
2017-04-10 17:54 git Note Added: 0065109
2017-04-10 17:57 isn Assigned To isn => msv
2017-04-10 17:57 isn Status assigned => resolved
2017-04-11 10:06 msv Note Added: 0065115
2017-04-11 10:06 msv Assigned To msv => isn
2017-04-11 10:06 msv Status resolved => assigned
2017-04-11 15:38 git Note Added: 0065142
2017-04-11 15:39 isn Assigned To isn => msv
2017-04-11 15:39 isn Status assigned => resolved
2017-04-11 16:33 msv Note Added: 0065145
2017-04-11 16:33 msv Assigned To msv => bugmaster
2017-04-11 16:33 msv Status resolved => reviewed
2017-04-11 16:35 apv Assigned To bugmaster => apv
2017-04-11 16:46 apv Test case number => heal same_parameter_locked; heal update_tolerance_locked
2017-04-12 10:16 apv Note Added: 0065166
2017-04-12 10:16 apv Assigned To apv => isn
2017-04-12 10:16 apv Status reviewed => assigned
2017-04-12 10:18 apv Note Added: 0065167
2017-04-12 14:38 git Note Added: 0065178
2017-04-12 14:52 isn Note Added: 0065179
2017-04-12 14:55 isn Note Edited: 0065179
2017-04-12 14:55 isn Assigned To isn => apv
2017-04-12 14:55 isn Status assigned => feedback
2017-04-13 11:06 apv Note Added: 0065195
2017-04-13 11:06 apv Assigned To apv => bugmaster
2017-04-13 11:06 apv Status feedback => tested
2017-04-14 14:28 bugmaster Changeset attached => occt master b60e8432
2017-04-14 14:28 bugmaster Status tested => verified
2017-04-14 14:28 bugmaster Resolution open => fixed
2017-05-12 11:35 git Note Added: 0065912
2017-05-12 11:35 git Note Added: 0065913
2017-05-12 11:40 git Note Added: 0065978
2017-09-29 16:19 aiv Fixed in Version => 7.2.0
2017-09-29 16:25 aiv Status verified => closed