Thu, 08/23/2007 - 18:13
Hello,
I found a severe error in BRepFill_CompatibleWires::SameNumberByPolarMethod. The algorithm uses two variables scal1 and scal2 in order to determine the correct orientation of an edge. Both variables take the value of some dot product, which means they can be negative, but this case is not considered.
IF only one of these values is negative, this can lead to an incorrect orientation, which in turn produces erroneous results for BRepOffsetAPI_ThruSections.
But if both values are negative, the variable Esol is a null shape, which leads to an exception on the subsequent call of BRepLib_MakeWire::Add with this null edge.
Since the method in question has more than 300 lines of only sparesly documented code, it is not clear how to fix these problems. It should be clear, however, that the negative case has somehow to be taken into account.
Kind regards,
Kris.
Thu, 08/23/2007 - 21:56
Its seems a pretty complicated minefield, but scal1 and scal2 seem only to be used as a part of an oriented peak-picker to determine scalmax. Perhaps adding a line under the dot product like
scal1 = N1.Dot(Ns);
scal1 = scal1 > 0 ? scal1 : -scal1;
{or "scal1 = abs(N1.Dot(Ns));" if you like but this might require another #INCLUDE for the math libraries} and similarly for scal2, will use the magnitude of the dot products instead. Do you have a test case that recreates the problem?
Pete
Fri, 08/24/2007 - 13:22
Well, it is evident that scal1 and scal2 are used to decide whether to use the current edge E as is, or whether it has to be reversed. This decision is based on the orientation of all previous edges, and that is the part I don't understand.
Two points (PP1 and PP2) are selected on the curve of the current edge, and these points are then successively transformed by parameters derived from all previous edges:
for (rang=i;rang>ideb;rang--) {
Transform(WithRotation, PP1,
Pos->Value(rang), Axe->Value(rang),
Pos->Value(rang-1), Axe->Value(rang-1), PP1);
Transform(WithRotation, PP2,
Pos->Value(rang), Axe->Value(rang),
Pos->Value(rang-1), Axe->Value(rang-1), PP2);
}
And I have no idea what the reasoning behind this computation might be. But it is crucial to decide which road to take:
1. Use the signed values of scal1 and scal2, and adapt only scalmax:
Standard_Real scalmax=-Precision::Infinite();
2. Use only the absolute values of scal1 and scal2:
scal1 = Abs(N1.Dot(Ns));
scal2 = Abs(N2.Dot(Ns));
(no additional include necessary for this!)
When I have the time, I will try both ways, and see what happens.
And yes, Pete, I have various test cases, both for the access violation and the incorrect orientations, but these cases surfaced in our product environment, and it would take too much time for me to reformulate them in purely OCC terms.
Regards, Kris.
Fri, 08/24/2007 - 15:12
I think whats its trying do is something like.
Its trying to compare a candidate curve E against a another curve ECur, and whether the direction needs to be reversed. Its first calculated an estimated of the ECur curve direction over the first 10% of the curve. For the comparison curve its calucates the directions for the first 10% and last 10%. It then projects these vectors onto the comparison vector via the dot product operator to find the "best" end and if its the high 10% reverses the curve direction in ESol. I can scenarios involving curve reversals over the middle 80% could give false responses, but your option 2 gives the better chance of sucess. In the case of option 1, if the two vectors are perpendicular (i.e. dot product=0) this will give an Esol which could be the "worst" case as 0 > -Precision::Infinite().
Also changing the comparisons to "if (scal1>=scalmax)... " might help in some corner cases, at the expense of some performance hit.
Good Luck
Pete
Fri, 08/24/2007 - 16:05
(ignore the ">=" option)
...and thinking about it further, I'm not sure that the answer isn't just
scal2 = - N2.Dot(Ns);
This should be provable on a simple model with only straight edges and a "cout << scal1 << "," << scal2 "\n";" debug step to check the signs of the dot products.
Pete
Wed, 09/15/2010 - 20:33
Hi:
Has anyone come up with a solution to this problem? It was referenced in the following posts:
http://www.opencascade.org/org/forum/thread_17640/
http://www.opencascade.org/org/forum/thread_16497/
If anyone has insights, including OCCT, please post them here.
Regards,
Jim
Fri, 09/17/2010 - 16:53
Hi,
I would like to inform you that the posted problem is checked and reproduced.
It is OCCT bug. The corresponding bug with ID = OCC22021 is assigned.
Later you can track the status of the problem by the specified ID.
We will try to do all our best to fix it in the reasonable time period depending on our technical capability and availability of resources.
If you can't wait and the problem is urgent for you, you may contact us via Contact Form http://www.opencascade.org/about/contacts/.
We will try to find a solution acceptable for you.
Regards
sergey
Mon, 09/20/2010 - 22:17
Sergey:
Thank you very much for fixing this problem; We appreciate it!