View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0028227Open CASCADEOCCT:Modeling Algorithmspublic2016-12-15 13:372017-09-29 16:25
ReporteremvAssigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version7.1.0 
Target Version7.2.0Fixed in Version7.2.0 
Summary0028227: ShapeUpgrade_UnifySameDomain modifies the edges even if it is not requested
DescriptionThe result of ShapeUpgrade_UnifySameDomain always contains the new edges, even if the unification of edges has not been requested.

Such behavior may be the result of NonDestructive mode of the algorithm. In order to avoid the modification of the initial shape (creation of new pcurve for the edge on the unified face e.g.) the new shape is created. In this case there should be possibility in algorithm to switch this mode off and allow modification of the shape.
Steps To Reproducerestore bug28226.brep s
unifysamedom r s -e
explode s e

unifysamedomgen re s_1
don r re s_1
compare re s_1
# shapes are not same

unifysamedomgen re s_6
don r re s_6
compare r re s_6
# shapes are not same

TagsNo tags attached.
Test case numberheal unify_same_domain A5

Relationships

related to 0028627 feedbackmsv Open CASCADE Revise UnifySameDomain implementation to get rid of usage of ShapeFix_Shape 
related to 0028602 assignedisn Open CASCADE Prevent ShapeFix_Shape to modify the input shape 
Not all the children of this issue are yet resolved or closed.

Activities

git

2017-03-27 19:29

administrator   ~0064730

Branch CR28227 has been created by imn.

SHA-1: 2af571904a3c4ae877a99e8e8132f82ffbb77eef


Detailed log of new commits:

Author: imn
Date: Mon Mar 27 19:29:12 2017 +0300

    0028227: ShapeUpgrade_UnifySameDomain modifies the edges even if it is not requested
    
    - Added flag "myMutableInput" and method "SetMutableInput"
      for allow/disallow modification of the shape in "ShapeUpgrade_UnifySameDomain" class
    - Added option "-nmi" in draw command "unifysamedom"

imn

2017-03-27 19:31

developer   ~0064731

Last edited: 2017-03-27 19:35

 Dear Mikhail, could you please review CR28227.

msv

2017-03-28 12:03

developer   ~0064740

Remarks:

I would change the bug target. Now the behavior regarding input shapes is uncontrolled. Some modifications can be done directly in input sub-shapes, and for some cases new shapes are created. The better goal would be not to try to leave the old shapes in unsafe mode, but to do the best not to modify the input shapes in safe mode. And all substitutions must be remembered in history.

As for unwanted modifications, they are done silently in ShapeFix algorithms. See the bugs 0028601, 0028602. If these bugs cannot be fixed in short term period it would be fine to get rid of ShapeFix functionality in UnifySameDomain.

src\ShapeFix\ShapeFix_ComposeShell.hxx
- 145: too long line

src\ShapeFix\ShapeFix_ComposeShell.cxx
- It is unclear why ShapeFix_ComposeShell::DispatchWires is corrected here. It seems to be already safe regarding the input shapes.

src\ShapeUpgrade\ShapeUpgrade_UnifySameDomain.hxx
- Rename "mutable input" to "safe mode".
- 79-81: Use a point at the end of sentence. Re-write the description considering new option name.

src\ShapeUpgrade\ShapeUpgrade_UnifySameDomain.cxx
- ctor at 1163 does not initializes the new flag.
- 1768: if ShapeFix_ComposeShell is safe by default, please do not add a new option there.
- 722,809: in these lines the original vertex is modified.

src\SWDRAW\SWDRAW_ShapeUpgrade.cxx
- Rename option "-nmi" to "-safe"

tests\heal\unify_same_domain\A5
- test case in incorrect. It is needed to use the command "setflags locked" in order to check if the input is not modified.
- 1662: update help of the command.

git

2017-03-28 16:01

administrator   ~0064753

Branch CR28227_1 has been created by imn.

SHA-1: 5022bd728dadf3f4e4c98654d0a6744867c05196


Detailed log of new commits:

Author: imn
Date: Tue Mar 28 16:00:13 2017 +0300

    0028227: ShapeUpgrade_UnifySameDomain modifies the edges even if it is not requested
    
    - Added flag "mySelfMode" and method "SetSelfMode"
    for allow/disallow modification of the shape in "ShapeUpgrade_UnifySameDomain" class
    - Added option "-safe" in draw command "unifysamedom"

imn

2017-03-28 16:09

developer   ~0064754

Dear Mikhail, could you please review CR28227_1. Remarks correction, without rid of ShapeFix functionality. Test case A5 should be OK, after fix bugs 0028602, 0028456.

git

2017-03-28 16:16

administrator   ~0064755

Branch CR28227_1 has been updated forcibly by imn.

SHA-1: 49aa5d14bb98932a7e8da3a25a3f0f146562654e

git

2017-03-28 17:13

administrator   ~0064756

Branch CR28227_1 has been updated forcibly by imn.

SHA-1: d9de5ef6f18435de56be8c0d1a25b4ddec3b674f

msv

2017-04-03 17:29

developer   ~0064872

src\SWDRAW\SWDRAW_ShapeUpgrade.cxx
- please change the name of "-safe" option to "-nosafe" (to exclude collision with other commands where "-safe" will mean turn on).

src\ShapeUpgrade\ShapeUpgrade_UnifySameDomain.hxx
- 79: "define" -> "defining"
- 81: "equal" -> "is equal"

src\ShapeUpgrade\ShapeUpgrade_UnifySameDomain.cxx
- 721,794: reuse possible replacement of vertex in theContext made on previous calls.

git

2017-04-03 18:20

administrator   ~0064873

Branch CR28227_1 has been updated forcibly by imn.

SHA-1: c5acd8caaf1358d13af506b43b341f03db76d451

imn

2017-04-03 18:29

developer   ~0064876

Dear Mikhail, could you please review CR28227_1. Did I understand correctly that don't need to replace vertices in context or just need replace vertices in the "MergeSeq" stage?

msv

2017-04-04 09:33

developer   ~0064882

src\SWDRAW\SWDRAW_ShapeUpgrade.cxx
- option -nosafe is not implemented.

- You understood incorrectly. Vertex must be replaced in context, but only once for each vertex. Ask context if this vertex is recorded there.

git

2017-04-04 12:56

administrator   ~0064886

Branch CR28227_1 has been updated forcibly by imn.

SHA-1: 826c4b921744d151b852b48973d9e8e0b78806d5

imn

2017-04-04 12:58

developer   ~0064887

Dear Mikhail, could you please review CR28227_1, branch is updated

msv

2017-04-04 18:46

developer   ~0064909

Reviewed.

apv

2017-04-05 11:46

tester   ~0064926

Dear BugMaster,

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

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

Regressions/Differences:
http://occt-tests/CR28227_1-master-OCCT/Debian70-64/summary.html
http://occt-tests/CR28227_1-master-OCCT/Windows-64-VC10/summary.html

Testing cases:
heal unify_same_domain A5 - FAILED (known problem)
http://occt-tests/CR28227_1-master-OCCT/Debian70-64/heal/unify_same_domain/A5.html
http://occt-tests/CR28227_1-master-OCCT/Windows-64-VC10/heal/unify_same_domain/A5.html

Testing on Linux:
Total MEMORY difference: 90847126 / 90469481 [+0.42%]
Total CPU difference: 19044.00000000029 / 18956.70000000024 [+0.46%]

Testing on Windows:
Total MEMORY difference: 57691214 / 57855806 [-0.28%]
Total CPU difference: 18190.68380619859 / 18063.40259029849 [+0.70%]

apv

2017-04-05 11:48

tester   ~0064927

Last edited: 2017-04-05 11:48

 Dear Ivan,

Branch CR28227_1 has been rejected due to:
- additional warnings
- regressions/differences/improvements

git

2017-04-07 11:58

administrator   ~0065016

Branch CR28227_1 has been updated by imn.

SHA-1: 54d887c16233260a10976b6c5d36e440bdbb22ee


Detailed log of new commits:

Author: imn
Date: Fri Apr 7 11:58:24 2017 +0300

    // Eliminating warnings and regressions.

imn

2017-04-07 11:59

developer   ~0065017

Dear Mikhail, could you please review CR28227_1, branch is updated

msv

2017-04-07 17:23

developer   ~0065043

Please make agreed corrections.

git

2017-04-10 14:21

administrator   ~0065089

Branch CR28227_1 has been updated by imn.

SHA-1: 71cc1b21e48a0b3591d3a1c1def6e8439b76f410


Detailed log of new commits:

Author: imn
Date: Mon Apr 10 14:20:01 2017 +0300

    // Continue eliminating warnings and regressions.
    
    Predictable behavior changing the number sub-shapes occurs because of the prohibition modifications input shape. If the same thing shape used with different "Location" then a copy of this shape will be created with each "Location".

imn

2017-04-10 14:24

developer   ~0065090

Dear Mikhail, could you please review CR28227_1, branch is updated

msv

2017-04-10 16:43

developer   ~0065102

src\ShapeUpgrade\ShapeUpgrade_UnifySameDomain.cxx
- Why the same thing is made in different way in lines 726 and 802?
- 1412: it should cause some doubt if theGMapEdgeFaces does not contain the edge. If the edge was modified somewhere then we must use its replacement here, instead of simply skipping it.

- Have you checked that the number of edges (or faces) has been increased correctly in each test case? Has number of vertices changed or no?

git

2017-04-14 14:23

administrator   ~0065220

Branch CR28227_2 has been created by imn.

SHA-1: 50c05a5f014fdf2405cace449f4a0055ec5ec319


Detailed log of new commits:

Author: imn
Date: Fri Apr 7 11:58:24 2017 +0300

    // Eliminating warnings and regressions.
    - Increase of number of sub-shapes in some test cases is explained by the fact that some sub-shapes were using the same TShape with different locations, and due to forbidden modification of input shapes for each such sub-shape a new copy is created.

Author: imn
Date: Tue Mar 28 16:00:13 2017 +0300

    0028227: ShapeUpgrade_UnifySameDomain modifies the edges even if it is not requested
    
    - Added flag "mySafeInputMode" and method "SetSafeInputMode"
    for allow/disallow modification of the shape in "ShapeUpgrade_UnifySameDomain" class
    - Added option "-safe" in draw command "unifysamedom"

imn

2017-04-14 14:32

developer   ~0065229

Dear Mikhail, could you please review CR28227_2, branch is updated and rebase onto current master

SAFE MODE(NEW) | NO SAFE MODE(OLD)

    offset shape_type_i_c XM1
VERTEX : 129 | 124
EDGE : 191 | 186

    offset shape_type_i_c ZX1
VERTEX : 45 | 42
EDGE : 63 | 60

    offset shape_type_i_c ZX3
VERTEX : 45 | 42
EDGE : 63 | 60
 
    offset shape_type_i_c ZX4
VERTEX : 45 | 42
EDGE : 63 | 60

    offset shape_type_i_c ZX9
VERTEX : 41 | 40
EDGE : 61 | 60

    offset shape_type_i_c ZY5
VERTEX : 45 | 42
EDGE : 63 | 60

    offset shape_type_i_c ZY6
VERTEX : 41 | 40
EDGE : 61 | 60

    bugs modalg_6 bug27315
VERTEX : 99 | 94
EDGE : 146 | 141

    bugs heal bug26219_1
VERTEX : 133 | 128
EDGE : 197 | 192

bugs heal bug26219_gehause_rohteil
VERTEX : 86 | 82
EDGE : 129 | 125

The number of faces and wires has not changed

msv

2017-04-17 19:03

developer   ~0065271

src\ShapeUpgrade\ShapeUpgrade_UnifySameDomain.cxx
- When checking an edge/vertex for belonging to the map myKeepShapes it is needed to consider that the shape could be replaced to another one, and the original will not be found in the map.

tests\offset\shape_type_i_c\XM1
- Changing of number of vertex/edge here shows real regression, because the algorithm does not merge some obvious edges. Repeated run of the algorithm on the result merges them completely.

Please analyze it more deeply, and check also other tests where you change reference data.

git

2017-04-21 20:19

administrator   ~0065450

Branch CR28227_3 has been created by imn.

SHA-1: 192ada059bf2dab2156deb928bec6d9059c27528


Detailed log of new commits:

Author: imn
Date: Fri Apr 7 11:58:24 2017 +0300

    // Eliminating warnings and regressions.
    - Added new static methods "UpdateMapEdgeFaces" and "UpdateKeepShapes" in ShapeUpgrade_UnifySameDomain class for consider that the shape could be replaced to another one when "safe mode" is on.

Author: imn
Date: Tue Mar 28 16:00:13 2017 +0300

    0028227: ShapeUpgrade_UnifySameDomain modifies the edges even if it is not requested
    
    - Added flag "mySafeInputMode" and method "SetSafeInputMode"
    for allow/disallow modification of the shape in "ShapeUpgrade_UnifySameDomain" class
    - Added option "-safe" in draw command "unifysamedom"

imn

2017-04-21 20:27

developer   ~0065452

The problem is found after merging the faces and begining merging the edges.
In file: "ShapeUpgrade_UnifySameDomain.cxx" line 2023, the map doesn't contain an edge.
I'm not sure, but maybe it's necessary to take the face without context.
At least tests don't have regressions(with/without safe input mode ):
offset shape_type_i_c XM1,ZX1,ZX3,ZX4,ZX9,ZY5,ZY6
bugs modalg_6 bug27315
bugs heal bug26219_1
bugs heal bug26219_gehause_rohteil

git

2017-04-25 15:01

administrator   ~0065508

Branch CR28227_4 has been created by msv.

SHA-1: 37c0e08a78f9e261bafe68b8e9606ed6b076ce4c


Detailed log of new commits:

Author: imn
Date: Tue Mar 28 16:00:13 2017 +0300

    0028227: ShapeUpgrade_UnifySameDomain modifies the edges even if it is not requested
    
    - The option SafeInputMode has been added in the class ShapeUpgrade_UnifySameDomain. If it is set then the input shape is protected against modifications of any aspects of its sub-shapes. Default value is true.
    - The option "-nosafe" has been added in draw command "unifysamedom". If it is not set the algorithm is run with SafeInputMode switched off.

msv

2017-04-25 15:02

developer   ~0065509

Please test the new version.

apv

2017-04-26 10:27

tester   ~0065530

Dear BugMaster,

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

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: 1200

Regressions/Differences:
Not detected

Testing cases:
heal unify_same_domain A5 - FAILED (known problem)
http://occt-tests/CR28227_4-master-OCCT/Debian70-64/heal/unify_same_domain/A5.html
http://occt-tests/CR28227_4-master-OCCT/Windows-64-VC10/heal/unify_same_domain/A5.html

Testing on Linux:
Total MEMORY difference: 92349910 / 91872090 [+0.52%]
Total CPU difference: 19470.020000000328 / 19560.840000000328 [-0.46%]

Testing on Windows:
Total MEMORY difference: 58042878 / 58034445 [+0.01%]
Total CPU difference: 18137.42506479857 / 18123.182173498568 [+0.08%]

git

2017-05-12 11:35

administrator   ~0065902

Branch CR28227 has been deleted by kgv.

SHA-1: 2af571904a3c4ae877a99e8e8132f82ffbb77eef

git

2017-05-12 11:35

administrator   ~0065903

Branch CR28227_1 has been deleted by kgv.

SHA-1: 71cc1b21e48a0b3591d3a1c1def6e8439b76f410

git

2017-05-12 11:35

administrator   ~0065904

Branch CR28227_2 has been deleted by kgv.

SHA-1: 50c05a5f014fdf2405cace449f4a0055ec5ec319

git

2017-05-12 11:35

administrator   ~0065905

Branch CR28227_3 has been deleted by kgv.

SHA-1: 192ada059bf2dab2156deb928bec6d9059c27528

git

2017-05-12 11:35

administrator   ~0065906

Branch CR28227_4 has been deleted by kgv.

SHA-1: 37c0e08a78f9e261bafe68b8e9606ed6b076ce4c

Related Changesets

occt: master 632175c3

2017-03-28 13:00:13

imn


Committer: bugmaster Details Diff		 0028227: ShapeUpgrade_UnifySameDomain modifies the edges even if it is not requested

- The option SafeInputMode has been added in the class ShapeUpgrade_UnifySameDomain. If it is set then the input shape is protected against modifications of any aspects of its sub-shapes. Default value is true.
- The option "-nosafe" has been added in draw command "unifysamedom". If it is not set the algorithm is run with SafeInputMode switched off.		 Affected Issues
0028227
mod - src/ShapeUpgrade/ShapeUpgrade_UnifySameDomain.cxx Diff File
mod - src/ShapeUpgrade/ShapeUpgrade_UnifySameDomain.hxx Diff File
mod - src/SWDRAW/SWDRAW_ShapeUpgrade.cxx Diff File
add - tests/heal/unify_same_domain/A5 Diff File

Issue History

Date Modified Username Field Change
2016-12-15 13:37 emv New Issue
2016-12-15 13:37 emv Assigned To => msv
2016-12-15 15:02 emv Steps to Reproduce Updated
2016-12-23 09:40 msv Assigned To msv => imn
2016-12-23 09:40 msv Status new => assigned
2017-03-27 19:29 git Note Added: 0064730
2017-03-27 19:31 imn Note Added: 0064731
2017-03-27 19:31 imn Assigned To imn => msv
2017-03-27 19:31 imn Status assigned => resolved
2017-03-27 19:35 imn Note Edited: 0064731
2017-03-28 12:03 msv Note Added: 0064740
2017-03-28 12:03 msv Assigned To msv => imn
2017-03-28 12:03 msv Status resolved => assigned
2017-03-28 16:01 git Note Added: 0064753
2017-03-28 16:09 imn Note Added: 0064754
2017-03-28 16:09 imn Assigned To imn => msv
2017-03-28 16:09 imn Status assigned => resolved
2017-03-28 16:16 git Note Added: 0064755
2017-03-28 17:13 git Note Added: 0064756
2017-04-03 17:29 msv Note Added: 0064872
2017-04-03 17:29 msv Assigned To msv => imn
2017-04-03 17:29 msv Status resolved => assigned
2017-04-03 18:20 git Note Added: 0064873
2017-04-03 18:29 imn Note Added: 0064876
2017-04-03 18:29 imn Assigned To imn => msv
2017-04-03 18:29 imn Status assigned => resolved
2017-04-04 09:33 msv Note Added: 0064882
2017-04-04 09:33 msv Assigned To msv => imn
2017-04-04 09:33 msv Status resolved => assigned
2017-04-04 12:56 git Note Added: 0064886
2017-04-04 12:58 imn Note Added: 0064887
2017-04-04 12:58 imn Assigned To imn => msv
2017-04-04 12:58 imn Status assigned => resolved
2017-04-04 18:35 msv Relationship added related to 0028627
2017-04-04 18:35 msv Relationship added related to 0028602
2017-04-04 18:46 msv Note Added: 0064909
2017-04-04 18:46 msv Assigned To msv => bugmaster
2017-04-04 18:46 msv Status resolved => reviewed
2017-04-04 18:55 apv Assigned To bugmaster => apv
2017-04-05 11:32 apv Test case number => heal unify_same_domain A5
2017-04-05 11:46 apv Note Added: 0064926
2017-04-05 11:46 apv Assigned To apv => imn
2017-04-05 11:46 apv Status reviewed => assigned
2017-04-05 11:48 apv Note Added: 0064927
2017-04-05 11:48 apv Note Edited: 0064927
2017-04-07 11:58 git Note Added: 0065016
2017-04-07 11:59 imn Note Added: 0065017
2017-04-07 11:59 imn Assigned To imn => msv
2017-04-07 11:59 imn Status assigned => resolved
2017-04-07 17:23 msv Note Added: 0065043
2017-04-07 17:23 msv Assigned To msv => imn
2017-04-07 17:23 msv Status resolved => assigned
2017-04-10 14:21 git Note Added: 0065089
2017-04-10 14:24 imn Note Added: 0065090
2017-04-10 14:24 imn Assigned To imn => msv
2017-04-10 14:24 imn Status assigned => resolved
2017-04-10 16:43 msv Note Added: 0065102
2017-04-10 16:43 msv Assigned To msv => imn
2017-04-10 16:43 msv Status resolved => assigned
2017-04-14 14:23 git Note Added: 0065220
2017-04-14 14:32 imn Note Added: 0065229
2017-04-14 14:32 imn Assigned To imn => msv
2017-04-14 14:32 imn Status assigned => resolved
2017-04-17 19:03 msv Note Added: 0065271
2017-04-17 19:03 msv Assigned To msv => imn
2017-04-17 19:03 msv Status resolved => assigned
2017-04-21 20:19 git Note Added: 0065450
2017-04-21 20:26 imn Assigned To imn => emv
2017-04-21 20:27 imn Note Added: 0065452
2017-04-21 20:27 imn Assigned To emv => msv
2017-04-21 20:27 imn Status assigned => feedback
2017-04-25 15:01 git Note Added: 0065508
2017-04-25 15:01 msv Status feedback => resolved
2017-04-25 15:02 msv Note Added: 0065509
2017-04-25 15:02 msv Assigned To msv => bugmaster
2017-04-25 15:02 msv Status resolved => reviewed
2017-04-25 15:44 apv Assigned To bugmaster => apv
2017-04-26 10:27 apv Note Added: 0065530
2017-04-26 10:27 apv Assigned To apv => bugmaster
2017-04-26 10:27 apv Status reviewed => tested
2017-04-28 13:08 bugmaster Changeset attached => occt master 632175c3
2017-04-28 13:08 bugmaster Status tested => verified
2017-04-28 13:08 bugmaster Resolution open => fixed
2017-05-12 11:35 git Note Added: 0065902
2017-05-12 11:35 git Note Added: 0065903
2017-05-12 11:35 git Note Added: 0065904
2017-05-12 11:35 git Note Added: 0065905
2017-05-12 11:35 git Note Added: 0065906
2017-09-29 16:20 aiv Fixed in Version => 7.2.0
2017-09-29 16:25 aiv Status verified => closed