Archived issue #0032279

Draw Harness - protect incmesh from hanging on syntax error

Open CASCADEOCCT:DRAWclosed29 public notes

Search issues

Description

Passing the following input to incmesh:
psphere s 1
incmesh s 0.008 -a 0

and
psphere s1 1
psphere s2 1
psphere s3 1
incmesh s1 s2 s3 0.008

doesn't raise any errors and instead leads to infinite loop due to passing 0 as deflection parameters.
It is desired improving BRepMesh to raise exception on obviously invalid input parameters (<=0, though alternative might be clamping too small values to some reasonable lower limit), as well as incmesh to better validate input parameters.

Steps to reproduce

Not required

Public activity

29 archived notes

Participants are labeled by their role within this record.

01Commenter 1
Solution elaboration, Testing
02Commenter 2
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: mkrylova
Date: Tue May 18 13:51:22 2021 +0300

    0032279: Draw Harness - protect incmesh from hanging on syntax error
    
    - improved incmesh to raise exception on obviously invalid input parameters
    - added possibility to operate multiple objects
03Author
    TCollection_AsciiString aName = aNamesOfShapes.Value (anIter);
    TopoDS_Shape aShape = DBRep::Get (aName);
    if (aShape.IsNull())
    {
      di << " Null shapes are not allowed here\n";
      continue;
    }

Please move this code to "for (Standard_Integer anArgIter = 1; anArgIter < nbarg; ++anArgIter)" loop, so that NCollection_Sequence<TCollection_AsciiString> aNamesOfShapes could be replaced by TopoDS_ListOfShape.

  for (Standard_Integer anIter = 1; anIter <= aNamesOfShapes.Length(); ++anIter)
  {

Please don't call meshing tool for each shape individually - make a TopoDS_Compound instead if there are more than 1 shape in input.

      di << " Null shapes are not allowed here\n";
      continue;

This should be an error within "return 1" result.

      if (aVal <= Precision::Confusion())
      {
        di << "Syntax error: invalid input parameter '" << argv[anArgIter] << "'";
        return 1;
      }

Please try also embedding such checks into the algorithm itself (throwing Standard_NumericError or similar).
04Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: mkrylova
Date: Wed May 19 19:42:13 2021 +0300

    # kgv remarks
    - fixed remarks
    - added checking for too small values in BrepMesh algorithm

05Commenter 2
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: mkrylova
Date: Tue May 18 13:51:22 2021 +0300

    0032279: Draw Harness - protect incmesh from hanging on syntax error
    
    - improved incmesh to raise exception on invalid input parameters
    - added possibility to operate multiple objects
    - added checking for too small values in BrepMesh algorithm
06Commenter 6
Fixing remarks, Testing
07Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: mkrylova
Date: Thu May 20 12:59:28 2021 +0300

    # kgv remarks
    - fixed remarks

08Commenter 2
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: mkrylova
Date: Tue May 18 13:51:22 2021 +0300

    0032279: Draw Harness - protect incmesh from hanging on syntax error
    
    - improved incmesh to raise exception on invalid input parameters
    - added possibility to operate multiple objects
    - added checking for too small values in BrepMesh algorithm
09Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: mkrylova
Date: Thu May 20 16:16:15 2021 +0300

    # kgv remarks
    - moved converting values to radians

10Commenter 2
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: mkrylova
Date: Tue May 18 13:51:22 2021 +0300

    0032279: Draw Harness - protect incmesh from hanging on syntax error
    
    - improved incmesh to raise exception on invalid input parameters
    - added possibility to operate multiple objects
    - added checking for too small values in BrepMesh algorithm
11Commenter 11
Solution implementation

12Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
13Commenter 2
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: mkrylova
Date: Tue May 18 13:51:22 2021 +0300

    0032279: Draw Harness - protect incmesh from hanging on syntax error
    
    - improved incmesh to raise exception on invalid input parameters
    - added possibility to operate multiple objects
    - added checking for too small values in BrepMesh algorithm
14Commenter 14
Solution implementation, Testing
15Commenter 15
16Author
-  const Standard_Real aDeflection = GetDeflection (theShape, theDrawer);
+  const Standard_Real aDeflection = Max (GetDeflection (theShape, theDrawer), Precision::Confusion());

Could you please share a specific test case (preferably localized) which triggers this issue?
Does GetDeflection() returns 0.0 in this case of value between 0 and Precision::Confusion()?
In the latter case, I guess that it would be better clamping computed within GetDeflection() method, or change Incmesh checks to protect from 0.0 instead of Precision::Confusion().
17Commenter 17
It triggered in the bugs/caf/bug26293_2 test http://occt-tests/CR32279_3-master-MKRYLOVA-OCCT/Windows-64-VC14/bugs/caf/bug26293_2.html. The GetDeflection() method returned a value much less than Precision::Confusion (), so an error was triggered from BRepMesh_IncrementalMesh
18Author
Please update method Prs3d::GetDeflection() to clamp output value instead of currently modified place:
  static Standard_Real GetDeflection (const Graphic3d_Vec3d& theBndMin,
                                      const Graphic3d_Vec3d& theBndMax,
                                      const Standard_Real theDeviationCoefficient)
  {
    const Graphic3d_Vec3d aDiag = theBndMax - theBndMin;
    return Max (aDiag.maxComp() * theDeviationCoefficient * 4.0, Precision::Confusion());
  }
19Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: mkrylova
Date: Fri May 28 14:22:37 2021 +0300

    # kgv remarks

20Commenter 2
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: mkrylova
Date: Tue May 18 13:51:22 2021 +0300

    0032279: Draw Harness - protect incmesh from hanging on syntax error
    
    - improved incmesh to raise exception on invalid input parameters
    - added possibility to operate multiple objects
    - added checking for too small values in BrepMesh algorithm
21Author
Please raise corrected patch
- OCCT: branch CR32279_5.
22Commenter 22
Solution implementation
23Commenter 1
Combination -
OCCT branch : [archived branch]
master SHA - [revision removed]
[revision removed]
Products branch : [archived branch] SHA - [revision removed]
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:
Debian80-64:
OCCT
Total CPU difference: 17574.5700000004 / 17867.41000000037 [-1.64%]
Products
Total CPU difference: 11533.700000000124 / 11535.680000000108 [-0.02%]
Windows-64-VC14:
OCCT
Total CPU difference: 19393.5625 / 19367.75 [+0.13%]
Products
Total CPU difference: 12891.8125 / 12920.9375 [-0.23%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
24Commenter 2
Branch [archived branch] has been deleted by Participant.

[revision removed]
25Commenter 2
Branch [archived branch] has been deleted by Participant.

[revision removed]
26Commenter 2
Branch [archived branch] has been deleted by Participant.

[revision removed]
27Commenter 2
Branch [archived branch] has been deleted by Participant.

[revision removed]
28Commenter 2
Branch [archived branch] has been deleted by Participant.

[revision removed]
29Commenter 2
Branch [archived branch] has been deleted by Participant.

[revision removed]