tpaviot / oce

OpenCASCADE Community Edition (OCE): a community driven fork of the Open CASCADE library.
http://groups.google.com/group/oce-dev
GNU Lesser General Public License v2.1
815 stars 284 forks source link

IntCurveSurface_Polygon::Init may throw unhandled exception #602

Closed mikedstrong closed 8 years ago

mikedstrong commented 8 years ago

When initializing and IntCurveSurface_Polygon, a deflection is calculated. If two consecutive points are coincident, and unhandled Standard_ConstructionError exception is thrown when the the gp_Lin is created using a zero-length gp_Dir in line 109 of IntCurveSurface_Polygon.gxx. I believe the correct thing to do is to skip zero-length segments, so I placed a try block around this code as follows:

do {
  gp_Pnt Pm=TheCurveTool::Value(C,u);
  gp_Pnt P1=ThePnts.Value(i);
  gp_Pnt P2=ThePnts.Value(i+1);
  // ignore this segment if zero-length (throws exception)
  try {
    gp_Lin L(P1,gp_Dir(gp_Vec(P1,P2)));
    Standard_Real t=L.Distance(Pm);

    if(t>TheDeflection) {
              TheDeflection = t;
    }
  }
  catch(Standard_ConstructionError) {
  }
  u+=du;
  i++;
}
while(i<NbPntIn);
ghost commented 8 years ago

Instead of using exception handling, I think this would be better handled by checking to see if the distance is practically zero before constructing the line, i.e.

if (P1.Distance(P2) < Precision::Confusion())
    // do something
ghost commented 8 years ago

Could also use

if (P1.IsEqual(P2, Precision::Confusion()))
    // do something
mikedstrong commented 8 years ago

That's certainly true. However, I think it is important that the tolerance has the same value that is used to throw the exception (gp::Resolution()). If Precision::Confusion() is greater that gp::Resolution, the exception could still get thrown. For reference the code in gp_Dir.lxx is:

Standard_Real D = sqrt(X * X + Y * Y + Z * Z); Standard_ConstructionError_Raise_if (D <= gp::Resolution(), "");

ghost commented 8 years ago

Agreed, I was just wondering what the more appropriate tolerance would be.

So something like:

    do {
      gp_Pnt Pm=TheCurveTool::Value(C,u);
      gp_Pnt P1=ThePnts.Value(i);
      gp_Pnt P2=ThePnts.Value(i+1);
      if (P1.Distance(P2) > gp::Resolution()) {
        gp_Lin L(P1,gp_Dir(gp_Vec(P1,P2)));
        Standard_Real t=L.Distance(Pm);

        if(t>TheDeflection) {
          TheDeflection = t;
        }
      }
      u+=du;
      i++;
    }
    while(i<NbPntIn);
ghost commented 8 years ago

Try https://github.com/tpaviot/oce/tree/ja/IntCurveSurface_Polygon-fix

mikedstrong commented 8 years ago

Thanks, Jacob. Looks good. Mike

From: Jacob Abel [mailto:notifications@github.com] Sent: Tuesday, January 12, 2016 6:57 PM To: tpaviot/oce oce@noreply.github.com Cc: Strong, Michael D. mike.strong@te.com Subject: Re: [oce] IntCurveSurface_Polygon::Init may throw unhandled exception (#602)

Try https://github.com/tpaviot/oce/tree/ja/IntCurveSurface_Polygon-fix

— Reply to this email directly or view it on GitHubhttps://github.com/tpaviot/oce/issues/602#issuecomment-171103921.