View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0029765 | Community | OCCT:Modeling Algorithms | public | 2018-05-14 15:28 | 2018-06-23 13:56 |
| Reporter | BenjaminBihler | Assigned To | bugmaster | ||
| Priority | normal | Severity | just a question | ||
| Status | closed | Resolution | fixed | ||
| Product Version | 7.2.0 | ||||
| Target Version | 7.4.0 | Fixed in Version | 7.4.0 | ||
| Summary | 0029765: Modeling Algorithms - BOPTools_AlgoTools::MakeSplitEdge creates Illegal Edge | ||||
| Description | I use BOPTools_AlgoTools::MakeSplitEdge to split an edge at two vertices which I have computed and projected onto the edge. If by chance the parameter of the first vertex is greater than the parameter of the second vertex, edge splitting seems to be successful. Yet, when I later use that edge for distance computations with BRepExtrema_DistShapeShape, then in GeomAdaptor_Curve::Load() a Standard_ConstructionError is thrown, because the parameter UFirst is greater than the parameter ULast. Of course there is a simple workaround for that: if (parameter1 < parameter2) { BOPTools_AlgoTools::MakeSplitEdge(edge, vertex1, parameter1, vertex2, parameter2, result); } else { BOPTools_AlgoTools::MakeSplitEdge(edge, vertex2, parameter2, vertex1, parameter1, result); } Still I wonder whether BOPTools_AlgoTools::MakeSplitEdge should be modified to do that parameter check automatically or -otherwise- whether the documentation of MakeSplitEdge should draw attention to the parameters order. On the other hand, if it is a standard behaviour throughout OCCT that the caller of such methods has to care for parameter orders, then nothing needs to be changed at all. What is your opinion? | ||||
| Steps To Reproduce | Not required | ||||
| Tags | No tags attached. | ||||
| Test case number | Not needed | ||||
|
|
BOPTools_AlgoTools::MakeSplitEdge is the internal tool used in Boolean Operations algorithm. It expects the ordered arguments, and across Boolean Operations the arguments for the method are given that way. To make the method generic, I think it is better to make the check for the order of the arguments inside the MakeSplitEdge method. We'll make the changes when we have free resources. Although, you are welcome to contribute the changes yourself. |
|
|
Branch CR29765 has been created by BenjaminBihler. SHA-1: 828db1c254371bf975b577f005edcb86b8dda424 Detailed log of new commits: Author: Benjamin Bihler Date: Tue May 15 09:43:57 2018 +0200 0029765: BOPTools_AlgoTools::MakeSplitEdge Creates Illegal Edge Method MakeSplitEdge checks arguments order. This makes the method more generic. |
|
|
Dear Benjamin, Thank you for the patch. I would like to make a remark. In an edge, the vertex corresponding to first parameter must have orientation forward, and the vertex corresponding to second parameter must have orientation reversed. So, the method must be reorganized taking this into account, so that to make sure vertices are added to the edge with correct orientations. Regards, Mikhail |
|
|
Branch CR29765 has been updated by BenjaminBihler. SHA-1: a3409ce875cf448561f667f636849fcc313ed3e2 Detailed log of new commits: Author: Benjamin Bihler Date: Tue May 15 16:31:38 2018 +0200 0029765: Modeling Algorithms - BOPTools_AlgoTools::MakeSplitEdge creates Illegal Edge Taking vertex orientation into account. |
|
|
Dear Mikhail, I am not sure whether I have understood you correctly. But I have tried to improve the code according to your remark. Would you check it again? Benjamin |
|
|
You understood me right. The fix is under testing now. Job is CR29765-master-msv. |
|
|
Reviewed. |
|
|
Combination - OCCT branch : CR29765 SHA - a3409ce875cf448561f667f636849fcc313ed3e2 Products branch : master SHA - 300cf879a836fb8a5c4636713070ca9cf544749f 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: Debian70-64: OCCT Total CPU difference: 18333.999999999785 / 18243.73999999987 [+0.49%] Products Total CPU difference: 7499.610000000047 / 7505.500000000054 [-0.08%] Windows-64-VC10: OCCT Total CPU difference: 18105.30445889858 / 18049.78370299853 [+0.31%] Products Total CPU difference: 7673.954391699951 / 7697.713343999945 [-0.31%] Image differences : No differences that require special attention Memory differences : No differences that require special attention |
|
|
Branch CR29765 has been deleted by kgv. SHA-1: a3409ce875cf448561f667f636849fcc313ed3e2 |
|
occt: master 01226433 2018-05-15 07:43:57 Committer: bugmaster Details Diff |
0029765: BOPTools_AlgoTools::MakeSplitEdge Creates Illegal Edge Method MakeSplitEdge checks arguments order. This makes the method more generic. Taking vertex orientation into account. |
Affected Issues 0029765 |
|
| mod - src/BOPTools/BOPTools_AlgoTools_2.cxx | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2018-05-14 15:28 | BenjaminBihler | New Issue | |
| 2018-05-14 15:28 | BenjaminBihler | Assigned To | => msv |
| 2018-05-14 15:54 |
|
Assigned To | msv => emv |
| 2018-05-14 15:54 |
|
Status | new => assigned |
| 2018-05-14 16:14 |
|
Note Added: 0075965 | |
| 2018-05-15 10:47 | git | Note Added: 0075973 | |
| 2018-05-15 10:48 | BenjaminBihler | Assigned To | emv => msv |
| 2018-05-15 10:48 | BenjaminBihler | Status | assigned => resolved |
| 2018-05-15 10:48 | BenjaminBihler | Steps to Reproduce Updated | |
| 2018-05-15 10:53 | kgv | Summary | BOPTools_AlgoTools::MakeSplitEdge Creates Illegal Edge => Modeling Algorithms - BOPTools_AlgoTools::MakeSplitEdge creates Illegal Edge |
| 2018-05-15 17:05 |
|
Note Added: 0075978 | |
| 2018-05-15 17:05 |
|
Assigned To | msv => BenjaminBihler |
| 2018-05-15 17:05 |
|
Status | resolved => assigned |
| 2018-05-15 17:31 | git | Note Added: 0075979 | |
| 2018-05-15 17:32 | BenjaminBihler | Note Added: 0075980 | |
| 2018-05-15 17:33 | BenjaminBihler | Assigned To | BenjaminBihler => msv |
| 2018-05-15 17:33 | BenjaminBihler | Status | assigned => feedback |
| 2018-05-15 17:47 |
|
Note Added: 0075982 | |
| 2018-05-15 19:49 |
|
Note Added: 0075985 | |
| 2018-05-15 19:49 |
|
Assigned To | msv => bugmaster |
| 2018-05-15 19:49 |
|
Status | feedback => reviewed |
| 2018-05-23 12:03 | bugmaster | Note Added: 0076150 | |
| 2018-05-23 12:03 | bugmaster | Status | reviewed => tested |
| 2018-05-23 12:03 | bugmaster | Test case number | => Not needed |
| 2018-06-14 18:20 | bugmaster | Changeset attached | => occt master 01226433 |
| 2018-06-14 18:20 | bugmaster | Status | tested => verified |
| 2018-06-14 18:20 | bugmaster | Resolution | open => fixed |
| 2018-06-23 13:56 | git | Note Added: 0076946 |
