MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0029765Community[OCCT] OCCT:Modeling Algorithmspublic2018-05-14 15:282018-06-23 13:56
ReporterBenjaminBihler 
Assigned Tobugmaster 
PrioritynormalSeverityjust a question 
StatusverifiedResolutionfixed 
PlatformOSOS Version
Product Version[OCCT] 7.2.0 
Target Version[OCCT] 7.4.0*Fixed in Version 
Summary0029765: Modeling Algorithms - BOPTools_AlgoTools::MakeSplitEdge creates Illegal Edge
DescriptionI 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 ReproduceNot required
TagsNo tags attached.
Test case numberNot needed
Attached Files

- Relationships

-  Notes
(0075965)
emv (developer)
2018-05-14 16:14

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.
(0075973)
git (administrator)
2018-05-15 10:47

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.
(0075978)
msv (developer)
2018-05-15 17:05

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
(0075979)
git (administrator)
2018-05-15 17:31

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.

(0075980)
BenjaminBihler (developer)
2018-05-15 17:32

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
(0075982)
msv (developer)
2018-05-15 17:47

You understood me right.

The fix is under testing now. Job is CR29765-master-msv.
(0075985)
msv (developer)
2018-05-15 19:49

Reviewed.
(0076150)
bugmaster (administrator)
2018-05-23 12:03

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
(0076946)
git (administrator)
2018-06-23 13:56

Branch CR29765 has been deleted by kgv.

SHA-1: a3409ce875cf448561f667f636849fcc313ed3e2

- Related Changesets
occt: master 01226433
Timestamp: 2018-05-15 07:43:57
Author: BenjaminBihler
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.
mod - src/BOPTools/BOPTools_AlgoTools_2.cxx Diff ] File ]

- Issue History
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 msv Assigned To msv => emv
2018-05-14 15:54 msv Status new => assigned
2018-05-14 16:14 emv 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 View Revisions
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 msv Note Added: 0075978
2018-05-15 17:05 msv Assigned To msv => BenjaminBihler
2018-05-15 17:05 msv 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 msv Note Added: 0075982
2018-05-15 19:49 msv Note Added: 0075985
2018-05-15 19:49 msv Assigned To msv => bugmaster
2018-05-15 19:49 msv 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


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker