View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0028226 | Open CASCADE | OCCT:Modeling Algorithms | public | 2016-12-15 13:22 | 2017-09-29 16:24 |
Reporter | Assigned To | bugmaster | |||
Priority | normal | Severity | minor | ||
Status | closed | Resolution | fixed | ||
Product Version | 7.1.0 | ||||
Target Version | 7.2.0 | Fixed in Version | 7.2.0 | ||
Summary | 0028226: Incorrect history support in ShapeUpgrade_UnifySameDomain algorithm | ||||
Description | The ShapeUpgrade_UnifySameDomain algorithm lacks some history methods such as Modified() and IsDeleted() and uses Generated() instead. It leads to confusing results - edges that have been removed have Genereted() shapes (see the Steps to reproduce and the attached image). | ||||
Steps To Reproduce | restore bug28226.brep s unifysamedom r s explode s e unifysamedomgen re s_2 donly r re | ||||
Tags | No tags attached. | ||||
Test case number | heal unify_same_domain A4 | ||||
2016-12-15 13:22 developer |
001.png (5,528 bytes) |
|
Branch CR28226 has been created by imn. SHA-1: dadb09f211884c50b0394012f12af1f1c4bc5734 Detailed log of new commits: Author: imn Date: Fri Feb 10 11:36:42 2017 +0300 0028226: Incorrect history support in ShapeUpgrade_UnifySameDomain algorithm - Added methods "Modified" and "IsDeleted" for history support in "ShapeUpgrade_UnifySameDomain" class - Added commands "unifysamedommod" "unifysamedomisdel" in Draw |
|
Dear Mikhail, could you please review CR28226. |
|
src\ShapeUpgrade\ShapeUpgrade_UnifySameDomain.hxx - Please add in description of the method Generated what is returned if no shape was generated from the given shape. Describe in which cases the method returns non-null shape. - Please add in description of the method Modified what is returned if the given shape was not modified. Describe in which cases the method returns non-null shape. - For IsDeleted, describe in which cases the shape is considered deleted and in which it is not. The work of history methods is contradictory. - For the edges a_3 and a_4 (they were not merged): * Generated returns non-null result. * Modified returns null, though they have been modified. - For edges e_10 and e_11 (they were merged): * Modified returns non-null for e_10, but null for e_11. Why? * IsDeleted returns true for e_11 and false for e_10. Why? - For edges e_16 and e_15 (they were merged, too): * Modified returns null for both. Why this result is different in compare with e_10 and e_11. - For edge a_1 (it was not touched at all): * Generated returns non-null. * Modified returns null. |
|
Also, please move the test case from bugs/modalg_6 to the new category heal/unify_same_domain, because this is a new development indeed. And do the same with test for 28228. |
|
Branch CR28226_1 has been created by imn. SHA-1: 6e6fd9b9ab79e899b31e8efb71251861783591a1 Detailed log of new commits: Author: imn Date: Fri Feb 17 21:31:20 2017 +0300 0028226: Incorrect history support in ShapeUpgrade_UnifySameDomain algorithm - Added methods "Modified" and "IsDeleted" for history support in "ShapeUpgrade_UnifySameDomain" class - Added commands "unifysamedommod" "unifysamedomisdel" in Draw - Updated calls for methods "Generated" and "Modified" |
|
Dear Mikhail, could you please review CR28226_1 |
|
Remarks: src\ShapeUpgrade\ShapeUpgrade_UnifySameDomain.hxx - In description of IsDeleted, add that the result does not contain even trace of aShape. src\ShapeUpgrade\ShapeUpgrade_UnifySameDomain.cxx - Generated: if the merged edge/face is modified then the result will not contain the shape stored in myOldToGeneratedShapes. So, it may be needed to ask the version from myContext. - Modified: why myOldToGeneratedShapes is used to get generated shape here? The method Modified() must not deal with generated shapes. - IsDeleted: incorrect, because IsRecorded returns true in the case of removed shape. You should not use context for this method. It is needed to fill in a special map during computation. Only vertex/edge can be considered deleted when containing it edges/faces are merged. src\SWDRAW\SWDRAW_ShapeUpgrade.cxx - 1664: misprint 'unifysamemod' => 'unifysamedom' src\BRepOffsetAPI\BRepOffsetAPI_MiddlePath.cxx src\BOPAlgo\BOPAlgo_CellsBuilder.cxx - As history behavior has been changed it is needed to review its usage in these algorithms. It is needed to update upgrade guide to reflect changes in API that a user will face when porting to the new OCCT. |
|
Branch CR28226_2 has been created by imn. SHA-1: f1b476ff2c6a666533594d18915dadd9677c6c8f Detailed log of new commits: Author: imn Date: Thu Mar 9 19:20:43 2017 +0300 0028226: Incorrect history support in ShapeUpgrade_UnifySameDomain algorithm - Added methods "Modified" and "IsDeleted" for history support in "ShapeUpgrade_UnifySameDomain" class - Added commands "unifysamedommod" "unifysamedomisdel" in Draw - Updated calls for methods "Generated" and "Modified" |
|
Dear Mikhail, could you please review CR28226_2 |
|
Remarks: src/ShapeUpgrade/ShapeUpgrade_UnifySameDomain.cxx - 1109: use anOldEdge variable - 1114, 2169: method Contains of list works slow. Please use Map instead of List. - 1548: use const & - 1545: add vertices of the edges in aMapEdges, so that not consider them removed (removed edge may share vertex with left edges). To check, use unifysamedom with option '-e', the vertex a_1_8 will be not deleted but IsDeleted returns true. - 2137: why not "*aNewShape"? |
|
Branch CR28226_2 has been updated by imn. SHA-1: 827ccf86ba2ccbfb009950588a254dfea3607d55 Detailed log of new commits: Author: imn Date: Fri Mar 10 18:11:25 2017 +0300 Remarks correction |
|
Dear Mikhail, could you please review CR28226_2, branch is updated according to remarks |
|
Remarks: src\ShapeUpgrade\ShapeUpgrade_UnifySameDomain.cxx - 1109: instead of creating a new variable anEdge change type of anOldEdge from shape to edge. - 1104: mistake, here we should use union instead of old edge. - 1114: check for Contains() is extra, this check is done in Add() anyway. - 1545-1552: replace this code with one call to TopExp::MapShapes(const TopoDS_Shape& S,TopTools_IndexedMapOfShape& M) |
|
Branch CR28226_2 has been updated by imn. SHA-1: 4c81ec1ffc3b97c2758808bd33d16f9e771943e8 Detailed log of new commits: Author: imn Date: Mon Mar 13 11:28:05 2017 +0300 Additional remarks correction |
|
Dear Mikhail, could you please review CR28226_2, branch is updated according to remarks |
|
Reviewed. |
|
Dear BugMaster, Branch CR28226_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode. SHA-1: 4c81ec1ffc3b97c2758808bd33d16f9e771943e8 Number of compiler warnings: occt component : Linux: 0 (0 on master) Windows: 0 (0 on master) MacOS : 0 (0 on master) products component : Linux: 64 (64 on master) Windows: 0 (0 on master) MacOS : 1185 Regressions/Differences/Improvements: http://occt-tests/CR28226_2-master-OCCT/Debian70-64/summary.html http://occt-tests/CR28226_2-master-OCCT/Windows-64-VC10/summary.html Failed: bugs modalg_2 bug23367_1: FAILED (exception) bugs modalg_2 bug23367_2: FAILED (exception) bugs modalg_2 bug23367_3: FAILED (exception) bugs modalg_2 bug23367_4: FAILED (exception) bugs modalg_2 bug23367_5: FAILED (exception) bugs modalg_2 bug23367_6: FAILED (exception) bugs modalg_2 bug23367_8: FAILED (exception) bugs modalg_2 bug23367_9: FAILED (exception) bugs modalg_2 bug23367_10: FAILED (exception) bugs modalg_2 bug23367_11: FAILED (exception) bugs modalg_2 bug23367_13: FAILED (exception) bugs modalg_2 bug23367_14: FAILED (exception) bugs modalg_2 bug23367_15: FAILED (exception) Testing cases: http://occt-tests/CR28226_2-master-OCCT/Debian70-64/heal/unify_same_domain/A4.html http://occt-tests/CR28226_2-master-OCCT/Windows-64-VC10/heal/unify_same_domain/A4.html heal unify_same_domain A4: OK Testing on Linux: occt component : Total MEMORY difference: 93627120 / 92298294 [+1.44%] Total CPU difference: 20072.730000000203 / 19839.690000000395 [+1.17%] products component : Total MEMORY difference: 31239200 / 31147739 [+0.29%] Total CPU difference: 5477.769999999979 / 5456.869999999986 [+0.38%] Testing on Windows: occt component : Total MEMORY difference: 57675025 / 57663643 [+0.02%] Total CPU difference: 18756.374632398576 / 18635.5206576986 [+0.65%] products component : Total MEMORY difference: 22289862 / 22249574 [+0.18%] Total CPU difference: 5502.0616693999755 / 5512.435735899986 [-0.19%] There are no differences in images found by testdiff. |
|
Dear imn, Branch CR28226_2 has been rejected due to: - regressions/differences/improvements |
|
Branch CR28226_2 has been updated by imn. SHA-1: 51652ca29d68e1b31cd1e414ee3739781a50013c Detailed log of new commits: Author: imn Date: Thu Mar 16 20:51:07 2017 +0300 Fixed regressions |
|
Dear Mikhail, could you please review CR28226_2, branch is updated according to fixed regressions |
|
Branch CR28226_3 has been created by msv. SHA-1: f0d65531f742d398af8faae9160f71e04f6ec0a9 Detailed log of new commits: Author: imn Date: Thu Mar 9 19:20:43 2017 +0300 0028226: Incorrect history support in ShapeUpgrade_UnifySameDomain algorithm - The methods "Modified" and "IsDeleted" have been added for history support in "ShapeUpgrade_UnifySameDomain" class. - The new Draw commands "unifysamedommod" and "unifysamedomisdel" have been added. - Adoption of other algorithms using this one to its new behavior. |
|
I have made small correction and put the new branch rebased to current master. Please test. |
|
Please wait a little, I have made a mistake in my correction. |
|
Branch CR28226_3 has been updated by msv. SHA-1: 6a9bf54bbe987285b6cfc4b761786d6e6e619db2 Detailed log of new commits: Author: msv Date: Mon Mar 20 12:29:06 2017 +0300 // more correction: remove code duplication |
|
Dear Ivan, please review the last change. |
|
Reviewed. Please test. |
|
Dear BugMaster, Branch CR28226_3 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode. SHA-1: 6a9bf54bbe987285b6cfc4b761786d6e6e619db2 Number of compiler warnings: occt component : Linux: 0 (0 on master) Windows: 0 (0 on master) MacOS : 0 (0 on master) products component : Linux: 64 (64 on master) Windows: 0 (0 on master) MacOS : 1199 Regressions/Differences/Improvements: http://occt-tests/CR28226_3-master-OCCT/Debian70-64/bugs/modalg_2/bug23367_2.html http://occt-tests/CR28226_3-master-OCCT/Windows-64-VC10/bugs/modalg_2/bug23367_2.html bugs modalg_2 bug23367_2: FAILED (exception) http://occt-tests/CR28226_3-master-OCCT/Debian70-64/bugs/modalg_2/bug23367_3.html http://occt-tests/CR28226_3-master-OCCT/Windows-64-VC10/bugs/modalg_2/bug23367_3.html bugs modalg_2 bug23367_3: FAILED (exception) Testing cases: http://occt-tests/CR28226_3-master-OCCT/Debian70-64/heal/unify_same_domain/A4.html http://occt-tests/CR28226_3-master-OCCT/Windows-64-VC10/heal/unify_same_domain/A4.html heal unify_same_domain A4: OK Testing on Linux: occt component : Total MEMORY difference: 92801148 / 92530623 [+0.29%] Total CPU difference: 20067.490000000147 / 20186.900000000103 [-0.59%] products component : Total MEMORY difference: 31017248 / 31146794 [-0.42%] Total CPU difference: 5473.689999999977 / 5412.149999999975 [+1.14%] Testing on Windows: occt component : Total MEMORY difference: 57787449 / 57782265 [+0.01%] Total CPU difference: 18796.950492498545 / 18782.551600198512 [+0.08%] products component : Total MEMORY difference: 22289652 / 22249574 [+0.18%] Total CPU difference: 5522.029797399978 / 5512.435735899986 [+0.17%] There are no differences in images found by testdiff. |
|
Dear msv, Branch CR28226_3 has been rejected due to: - regressions/differences/improvements |
|
Branch CR28226_3 has been updated by imn. SHA-1: fe31da183a04deb7c4b0ee76e094f5f94d6d5cc8 Detailed log of new commits: Author: imn Date: Tue Mar 21 17:20:59 2017 +0300 Fixed regressions |
|
Dear Mikhail, could you please review CR28226_3, branch is updated according to fixed regressions |
|
The method TopTools_MapOfShape::Add returns Boolean indicating whether the key was already in the map. So, simplify the code at 281-282: if (aGeneratedEdges.Add(aShape)) { |
|
Branch CR28226_3 has been updated by imn. SHA-1: 6d0c6c071c4e7dd931e6492ddc0aaaed279356d9 Detailed log of new commits: Author: imn Date: Tue Mar 21 18:11:34 2017 +0300 Remarks correction |
|
Dear Mikhail, please review the last change |
|
OK. |
|
Dear BugMaster, Branch CR28226_3 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode. SHA-1: 6d0c6c071c4e7dd931e6492ddc0aaaed279356d9 Number of compiler warnings: occt component : Linux: 0 (0 on master) Windows: 0 (0 on master) MacOS : 0 (0 on master) products component : Linux: 64 (64 on master) Windows: 0 (0 on master) MacOS : 1188 Regressions/Differences/Improvements: No regressions/differences Testing cases: http://occt-tests/CR28226_3-master-OCCT/Debian70-64/heal/unify_same_domain/A4.html http://occt-tests/CR28226_3-master-OCCT/Windows-64-VC10/heal/unify_same_domain/A4.html heal unify_same_domain A4: OK Testing on Linux: occt component : Total MEMORY difference: 92202283 / 92528196 [-0.35%] Total CPU difference: 20107.21000000011 / 20186.790000000106 [-0.39%] products component : Total MEMORY difference: 30986538 / 31146794 [-0.51%] Total CPU difference: 5467.1899999999805 / 5412.149999999975 [+1.02%] Testing on Windows: occt component : Total MEMORY difference: 57787258 / 57782265 [+0.01%] Total CPU difference: 18806.029750698566 / 18782.551600198512 [+0.12%] products component : Total MEMORY difference: 22293110 / 22254740 [+0.17%] Total CPU difference: 5459.769798299984 / 5421.409152399973 [+0.71%] There are no differences in images found by testdiff. |
|
Dear BugMaster, Branch CR28226_3 is TESTED. |
|
Branch CR28226_3 has been updated by imn. SHA-1: 7056bfba10e6f932fbec085aa218974331f81482 Detailed log of new commits: Author: imn Date: Fri Mar 24 12:25:37 2017 +0300 Fixed warnings |
|
Branch CR28226 has been deleted by kgv. SHA-1: dadb09f211884c50b0394012f12af1f1c4bc5734 |
|
Branch CR28226_1 has been deleted by kgv. SHA-1: 6e6fd9b9ab79e899b31e8efb71251861783591a1 |
|
Branch CR28226_2 has been deleted by kgv. SHA-1: 51652ca29d68e1b31cd1e414ee3739781a50013c |
|
Branch CR28226_3 has been deleted by kgv. SHA-1: 7056bfba10e6f932fbec085aa218974331f81482 |
occt: master 20aa0d3f 2017-03-09 16:20:43 Committer: bugmaster Details Diff |
0028226: Incorrect history support in ShapeUpgrade_UnifySameDomain algorithm - The methods "Modified" and "IsDeleted" have been added for history support in "ShapeUpgrade_UnifySameDomain" class. - The new Draw commands "unifysamedommod" and "unifysamedomisdel" have been added. - Adoption of other algorithms using this one to its new behavior. Removing code duplication Correcting regressions Correcting remarks |
Affected Issues 0028226 |
|
mod - dox/dev_guides/upgrade/upgrade.md | Diff File | ||
mod - src/BOPAlgo/BOPAlgo_CellsBuilder.cxx | Diff File | ||
mod - src/BRepOffsetAPI/BRepOffsetAPI_MiddlePath.cxx | Diff File | ||
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/A4 | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2016-12-15 13:22 |
|
New Issue | |
2016-12-15 13:22 |
|
Assigned To | => msv |
2016-12-15 13:22 |
|
File Added: 001.png | |
2016-12-15 13:23 |
|
Steps to Reproduce Updated | |
2016-12-23 09:39 |
|
Assigned To | msv => imn |
2016-12-23 09:39 |
|
Status | new => assigned |
2017-02-10 11:38 | git | Note Added: 0063743 | |
2017-02-10 11:41 | imn | Note Added: 0063744 | |
2017-02-10 11:41 | imn | Assigned To | imn => msv |
2017-02-10 11:41 | imn | Status | assigned => resolved |
2017-02-10 19:33 |
|
Note Added: 0063752 | |
2017-02-10 19:33 |
|
Assigned To | msv => imn |
2017-02-10 19:33 |
|
Status | resolved => assigned |
2017-02-10 19:39 |
|
Note Added: 0063753 | |
2017-02-17 21:31 | git | Note Added: 0063899 | |
2017-02-20 11:29 | imn | Note Added: 0063905 | |
2017-02-20 11:29 | imn | Assigned To | imn => msv |
2017-02-20 11:29 | imn | Status | assigned => resolved |
2017-03-03 16:58 |
|
Note Added: 0064112 | |
2017-03-03 16:58 |
|
Assigned To | msv => imn |
2017-03-03 16:58 |
|
Status | resolved => assigned |
2017-03-09 19:30 | git | Note Added: 0064218 | |
2017-03-10 11:03 | imn | Note Added: 0064225 | |
2017-03-10 11:03 | imn | Assigned To | imn => msv |
2017-03-10 11:03 | imn | Status | assigned => resolved |
2017-03-10 16:26 |
|
Note Added: 0064247 | |
2017-03-10 16:26 |
|
Assigned To | msv => imn |
2017-03-10 16:26 |
|
Status | resolved => assigned |
2017-03-10 18:11 | git | Note Added: 0064251 | |
2017-03-10 18:14 | imn | Note Added: 0064252 | |
2017-03-10 18:14 | imn | Assigned To | imn => msv |
2017-03-10 18:14 | imn | Status | assigned => resolved |
2017-03-13 10:10 |
|
Note Added: 0064255 | |
2017-03-13 10:10 |
|
Assigned To | msv => imn |
2017-03-13 10:10 |
|
Status | resolved => assigned |
2017-03-13 11:29 | git | Note Added: 0064259 | |
2017-03-13 11:31 | imn | Note Added: 0064260 | |
2017-03-13 11:31 | imn | Assigned To | imn => msv |
2017-03-13 11:31 | imn | Status | assigned => resolved |
2017-03-13 13:03 |
|
Note Added: 0064263 | |
2017-03-13 13:03 |
|
Assigned To | msv => bugmaster |
2017-03-13 13:03 |
|
Status | resolved => reviewed |
2017-03-13 13:04 |
|
Assigned To | bugmaster => mkv |
2017-03-16 13:13 |
|
Test case number | => heal unify_same_domain A4 |
2017-03-16 13:23 |
|
Note Added: 0064376 | |
2017-03-16 13:24 |
|
Note Added: 0064377 | |
2017-03-16 13:24 |
|
Assigned To | mkv => imn |
2017-03-16 13:24 |
|
Status | reviewed => assigned |
2017-03-16 20:51 | git | Note Added: 0064389 | |
2017-03-16 20:55 | imn | Note Added: 0064390 | |
2017-03-16 20:55 | imn | Assigned To | imn => msv |
2017-03-16 20:55 | imn | Status | assigned => resolved |
2017-03-17 19:45 | git | Note Added: 0064419 | |
2017-03-17 19:46 |
|
Note Added: 0064420 | |
2017-03-17 19:46 |
|
Assigned To | msv => mkv |
2017-03-17 19:46 |
|
Status | resolved => reviewed |
2017-03-20 12:19 |
|
Assigned To | mkv => msv |
2017-03-20 12:19 |
|
Status | reviewed => assigned |
2017-03-20 12:19 |
|
Note Added: 0064437 | |
2017-03-20 12:29 | git | Note Added: 0064438 | |
2017-03-20 12:30 |
|
Note Added: 0064439 | |
2017-03-20 12:30 |
|
Assigned To | msv => imn |
2017-03-20 12:30 |
|
Status | assigned => resolved |
2017-03-20 12:39 | imn | Note Added: 0064440 | |
2017-03-20 12:39 | imn | Assigned To | imn => mkv |
2017-03-20 12:39 | imn | Status | resolved => reviewed |
2017-03-20 20:31 |
|
Note Added: 0064553 | |
2017-03-20 20:32 |
|
Note Added: 0064554 | |
2017-03-20 20:32 |
|
Assigned To | mkv => msv |
2017-03-20 20:32 |
|
Status | reviewed => assigned |
2017-03-21 09:29 |
|
Assigned To | msv => imn |
2017-03-21 17:21 | git | Note Added: 0064585 | |
2017-03-21 17:23 | imn | Note Added: 0064586 | |
2017-03-21 17:23 | imn | Assigned To | imn => msv |
2017-03-21 17:23 | imn | Status | assigned => resolved |
2017-03-21 18:04 |
|
Note Added: 0064588 | |
2017-03-21 18:04 |
|
Assigned To | msv => imn |
2017-03-21 18:04 |
|
Status | resolved => assigned |
2017-03-21 18:11 | git | Note Added: 0064589 | |
2017-03-21 18:14 | imn | Note Added: 0064590 | |
2017-03-21 18:14 | imn | Assigned To | imn => msv |
2017-03-21 18:14 | imn | Status | assigned => resolved |
2017-03-21 18:17 |
|
Note Added: 0064591 | |
2017-03-21 18:17 |
|
Assigned To | msv => bugmaster |
2017-03-21 18:17 |
|
Status | resolved => reviewed |
2017-03-21 18:25 |
|
Assigned To | bugmaster => mkv |
2017-03-22 16:00 |
|
Note Added: 0064622 | |
2017-03-22 16:00 |
|
Note Added: 0064623 | |
2017-03-22 16:00 |
|
Assigned To | mkv => bugmaster |
2017-03-22 16:00 |
|
Status | reviewed => tested |
2017-03-24 12:25 | git | Note Added: 0064677 | |
2017-03-24 15:53 | bugmaster | Changeset attached | => occt master 20aa0d3f |
2017-03-24 15:53 | bugmaster | Status | tested => verified |
2017-03-24 15:53 | bugmaster | Resolution | open => fixed |
2017-05-12 11:35 | git | Note Added: 0065898 | |
2017-05-12 11:35 | git | Note Added: 0065899 | |
2017-05-12 11:35 | git | Note Added: 0065900 | |
2017-05-12 11:35 | git | Note Added: 0065901 | |
2017-09-29 16:20 |
|
Fixed in Version | => 7.2.0 |
2017-09-29 16:24 |
|
Status | verified => closed |