Archived issue #0028456
BRepBuilderAPI_MakeFace modifies the input shape
Description
BRepBuilderAPI_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.
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 reproduce
box 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)
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)
Public activity
27 archived notes
Participants are labeled by their role within this record.
Branch [archived branch] has been created by Author.
[revision removed]
Detailed log of new commits:
Author: Author
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
[revision removed]
Detailed log of new commits:
Author: Author
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
Branch [archived branch] has been created by Author.
[revision removed]
Detailed log of new commits:
Author: Author
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
[revision removed]
Detailed log of new commits:
Author: Author
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
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.
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.
Branch [archived branch] has been updated by Author.
[revision removed]
Detailed log of new commits:
Author: Author
Date: Fri Mar 10 15:19:23 2017 +0300
corrections
[revision removed]
Detailed log of new commits:
Author: Author
Date: Fri Mar 10 15:19:23 2017 +0300
corrections
Branch [archived branch] has been updated forcibly by Author.
[revision removed]
[revision removed]
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
- 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
Branch [archived branch] has been updated by Author.
[revision removed]
Detailed log of new commits:
Author: Author
Date: Fri Mar 31 14:27:35 2017 +0300
corrections
[revision removed]
Detailed log of new commits:
Author: Author
Date: Fri Mar 31 14:27:35 2017 +0300
corrections
Branch [archived branch] has been updated forcibly by Author.
[revision removed]
[revision removed]
Branch [archived branch] has been updated forcibly by Author.
[revision removed]
[revision removed]
Branch [archived branch] has been updated forcibly by Author.
[revision removed]
[revision removed]
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.
- 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.
And please rebase on last master.
Branch [archived branch] has been created by Author.
[revision removed]
Detailed log of new commits:
Author: Author
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
[revision removed]
Detailed log of new commits:
Author: Author
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
Branch [archived branch] has been updated forcibly by Author.
[revision removed]
[revision removed]
- 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
- 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
Branch [archived branch] has been updated by Author.
[revision removed]
Detailed log of new commits:
Author: Author
Date: Mon Apr 10 17:51:02 2017 +0300
update of tests/docs
[revision removed]
Detailed log of new commits:
Author: Author
Date: Mon Apr 10 17:51:02 2017 +0300
update of tests/docs
src\BRepTest\BRepTest_BasicCommands.cxx
- 330,367: there will be warning on other platforms; include sub-expressions in parentheses.
- 330,367: there will be warning on other platforms; include sub-expressions in parentheses.
Branch [archived branch] has been updated by Author.
[revision removed]
Detailed log of new commits:
Author: Author
Date: Tue Apr 11 15:38:16 2017 +0300
remark
[revision removed]
Detailed log of new commits:
Author: Author
Date: Tue Apr 11 15:38:16 2017 +0300
remark
Reviewed.
Dear Commenter 1,
Branch CR28456_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
[revision removed]
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%]
Branch CR28456_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
[revision removed]
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%]
Dear Ilya,
Branch [archived branch] has been rejected due to:
- additional warnings
- regressions/difference/improvements
Branch [archived branch] has been rejected due to:
- additional warnings
- regressions/difference/improvements
Branch [archived branch] has been updated by Author.
[revision removed]
Detailed log of new commits:
Author: Author
Date: Wed Apr 12 14:38:26 2017 +0300
elimination of warnings; corrections of test
[revision removed]
Detailed log of new commits:
Author: Author
Date: Wed Apr 12 14:38:26 2017 +0300
elimination of warnings; corrections of test
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.
>>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.
Dear Commenter 1,
Branch CR28456_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
[revision removed]
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%]
Branch CR28456_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
[revision removed]
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%]
Branch [archived branch] has been deleted by Participant.
[revision removed]
[revision removed]
Branch [archived branch] has been deleted by Participant.
[revision removed]
[revision removed]
Branch [archived branch] has been deleted by Participant.
[revision removed]
[revision removed]
Related records