MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0028840Open CASCADE[OCCT] OCCT:Data Exchangepublic2017-06-13 11:452018-09-11 18:18
Reporteraml 
Assigned Tobugmaster 
PrioritynormalSeverityintegration request 
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version[OCCT] 7.1.0 
Target Version[OCCT] 7.2.0Fixed in Version[OCCT] 7.2.0 
Summary0028840: Data Exchange - rewrite the STL Reader/Writer
DescriptionThe current implementation has the following problems:

1) It is not possible to get Poly_Triangulation from the STL reader. It reads to a StlMesh object which is not used outside of the reader.
2) Performance is poor. Performance can be significantly improved by using the block-based reading for the binary files.
Steps To ReproduceNew grid "de_mesh" has been added.
Test shapes can be downloaded from: \\aml\Share\bug28840.zip
TagsNo tags attached.
Test case numbergroup : de_mesh
Attached Files

- Relationships
related to 0025388closedbugmaster Open CASCADE Improve STL reader to generate one triangulation instead of compound of faces 
related to 0017422newbugmaster Open CASCADE StlMesh_MeshDomain::AddOnlyNewVertex is incorrect 
related to 0027005closedbugmaster Community Standard_Integer StlMesh_MeshDomain::AddOnlyNewVertex(...) does not work 
parent of 0028974closedbugmaster Open CASCADE Test cases for STL reader/writer 
parent of 0030130verifiedbugmaster Open CASCADE Coding Rules - MSVC 2017 gives warnings about using of std::fpos::seekpos() within RWStl 
related to 0026338closedbugmaster Community STL export (especially binary) needs a lot of time if selected export path is not local 
related to 0024729closedbugmaster Community Duplicate code to compute the normals for TopoDS_Face 
related to 0028965closedkgv Open CASCADE Jenkings Certification Tool - CLang warning is ignored within final report for LLVM/Clang 
related to 0026100closedabv Open CASCADE Incorrect printout in StlTransfer::RetrieveMesh 

-  Notes
(0067484)
git (administrator)
2017-06-16 13:55

Branch CR28840 has been created by aml.

SHA-1: 796c0ded7fa3dd118722cc51de54fb5323533048


Detailed log of new commits:

Author: aml
Date: Wed Jun 14 08:07:26 2017 +0300

    0028840: Data Exchange - rewrite the STL Reader/Writer
    
    New implementation of the STL Reader / Writer. New implementation has the following advantages:
    1) Possibility to read STL file to the Poly_Triangulation has been added.
    2) Chunk-based reading for the binary files has been added. It significantly improves reading and writing of the binary files.
    3) New implementation does not have dependencies.
    
    Obsolete code related to the old implementation has been deleted.
    Test cases have been added.
(0067486)
kgv (developer)
2017-06-16 14:04

+    FILE * aF = fopen(aCPath, "wb");

OSD_OpenFile
(0067577)
git (administrator)
2017-06-22 07:01

Branch CR28840 has been updated forcibly by aml.

SHA-1: 724ef94a8cb2c3de0fd88a72c1cce6776b2c1520
(0067578)
aml (developer)
2017-06-22 07:04

Dear abv,

Could you please review the CR28840 branch?
(0067584)
kgv (developer)
2017-06-22 11:24

+  // Calculate number of lines in input file.
+  Standard_Integer aNbLines = 0;
+  fseek(theF, 0L, SEEK_END);
+  long filesize = ftell(theF);
+  rewind(theF);
+  for (long aPos = 0; aPos < filesize; ++aPos)
+  {
+    if (getc(theF) == '\n')
+        aNbLines++;
+  }
+  rewind(theF);

This nice efficient code is still there after all...
(0067585)
kgv (developer)
2017-06-22 11:29

+  const Standard_Integer CHUNKSIZE = 4096;
+  Standard_Character* aDataChunk = new Standard_Character[CHUNKSIZE * THE_STL_SIZEOF_FACET];
...
+  delete[] aDataChunk;

Dynamic allocation with manual deallocation is not recommended.

+    if (aVNorm.SquareMagnitude() > gp::Resolution())
+      aVNorm.Normalize();
+    else
+      aVNorm.SetCoord(0.0, 0.0, 0.0);
+
+     Sprintf (aBuffer,

Broken indentation.
(0067586)
szv (manager)
2017-06-22 11:31

DC,

The principle of counting lines in the file will also not allow us to read correctly multi-domain STL files (non standard, but may exist).

See the issue https://tracker.dev.opencascade.org/view.php?id=28680. [^]
(0067587)
kgv (developer)
2017-06-22 12:10

   theCommands.Add ("readstl",   "shape file",__FILE__,readstl,g);
+  theCommands.Add ("readstlastri", "shape file",__FILE__,readstlastri,g);

Creating a new command is not recommended - better extended syntax of existing command.

+  if (theArgc < 3)
+    theDi << "wrong number of parameters" << "\n";
...
+  return 0;

This is unusual returning 0 (success) for wrong input.

+  // Create temporary triangulation.
+   Handle(Poly_Triangulation) aMesh = new Poly_Triangulation(aNbNodes, 
...
+  MergeNodes(aNodes, aTriangles);
+  // Simple copy to the OCCT data structures.
+ Handle(Poly_Triangulation) aMesh = new Poly_Triangulation(aNodes.Size(), 

Broken indentation.

+  Standard_Integer aNbNodes = 0;
+  Standard_Integer aNbTriangles = 0;
+
+  // Calculate total number of the nodes and triangles.
+  TopExp_Explorer anExpSF(theShape, TopAbs_FACE);
+  for ( ; anExpSF.More() ; anExpSF.Next() )
   {
+    TopLoc_Location aLoc;
+    Handle(Poly_Triangulation) aTriangulation 
+      = BRep_Tool::Triangulation(TopoDS::Face(anExpSF.Current()), aLoc);
+
+    aNbNodes += aTriangulation->NbNodes();
+    aNbTriangles += aTriangulation->NbTriangles();
   }
+
+  // Create temporary triangulation.
+   Handle(Poly_Triangulation) aMesh = new Poly_Triangulation(aNbNodes, 

Creating a single Poly_Triangulation for saving a bunch of shapes does not look efficient.

+//=============================================================================
+// IN
+//=============================================================================
+
+  //! Resulting nodes.
+  NCollection_Vector<gp_Pnt> myResPoints;
+
+  //! Merging tolerance.
+  Standard_Real myMergingTol;
+
+//=============================================================================
+// INTERNAL
+//=============================================================================

Unusual documentation syntax. Please use Doxygen-compatible syntax (like "private: //! @name Internal fields") instead, or drop redundant comments.

+  //! Returns resulting nodes array.
+  const NCollection_Vector<gp_Pnt>& GetResultingArray() const { return myResPoints; }
+
+  //! Returns resulting index in range [1, NbNodes].
+  //! \param[in] theOrigNode original index in range [1, NbNodes].
+  Standard_Integer GetResultingNode(const Standard_Integer theOrigNode) const { return myInpOutMap(theOrigNode 
- 1); }
+
+  //! Returns merging map.
+  const NCollection_Vector<Standard_Integer>& GetMergingMap() const { return myInpOutMap; 
}
...
+    //! Get mesh in the internal representation.
+  const Handle(Poly_Triangulation)& GetMesh() const { return myTriangulation; }

Recommended name convention is suffix "Set" for setter and no suffix for getter (used almost everywhere in OCCT with a few exceptions).

+inline int HashCode(const gp_Pnt& thePnt,
+                    const int theUpper)
+{
+  double aVal = thePnt.X() * thePnt.X() +
+                thePnt.Y() * thePnt.Y() +
+                thePnt.Z() * thePnt.Z() +
+                thePnt.SquareDistance(gp_Pnt(1.0, 0.1, 2.0));
+
+  return HashCode(aVal, theUpper);
+}
+
+//=============================================================================
+//function : IsEqual
+//purpose  :
+//=============================================================================
+inline bool IsEqual(const gp_Pnt& thePnt1,
+                    const gp_Pnt& thePnt2)

Please move to a Hasher struct instead of global functions.
Please also document what HashCode() formula means.

+  if (thePnt1.SquareDistance(thePnt2) < gp::Resolution())
+    return true;
+
+  return false;

return thePnt1.SquareDistance(thePnt2) < gp::Resolution();

+RWStl_NodesMerger::RWStl_NodesMerger(const Standard_Real thePrecision)
+{
+  myMergingTol = thePrecision;
+  myMap.Clear();
+  myIsNew = Standard_False;
+}
...
+RWStl::RWStl()
+{
+  myWriteBinary = Standard_True;

Please use member initialization list.
myMap.Clear() looks redundant in constructor.

+// Include self.
+#include <RWStl_NodesMerger.hxx>

The comments look redundant.

 private:
 
+  //! Checks whether ASCII or binary.
+  Standard_Boolean IsASCII(const Standard_CString thePath);
 
+  //! Read ASCII version.
+  Standard_Boolean ReadASCII(FILE *theF,
+                             const Handle(Message_ProgressIndicator)& theProgInd);
...
+inline static void ConvertInteger(const Standard_Integer theValue,
+                                  Standard_Character* theResult)
...
+inline static void ConvertDouble(const Standard_Real theValue,
+                                 Standard_Character* theResult

Recommended name convention is starting private/protected methods and functions with lower case.

+     fprintf(theF, "%s", aBuffer);
...
+  fprintf(theF, "%s", "endsolid\n");
...
+  fprintf(theF, "%s", "endsolid\n");

Using fprintf for writing a string of known size looks unnatural - please consider using fwrite instead.

+  fprintf(theF, "%s", "endsolid\n");
+  return Standard_True;
...
+  // Write last part if necessary.
+  if (j != CHUNKSIZE * THE_STL_SIZEOF_FACET)
+    fwrite(aDataChunk, 1, j, theF);
+
+  delete[] aDataChunk;
+
+  return Standard_True;
...
+    Standard_Boolean isOK = WriteBinary(aF, theProgInd);
+    fclose(aF);
+    if (!isOK)
+      return Standard_False;
...
+    Standard_Boolean isOK = WriteASCII(aF, theProgInd);
+    fclose(aF);
+    if (!isOK)
+      return Standard_False;

Write checks are missing.

+  Standard_Character* aDataChunk = new char[CHUNKSIZE * THE_STL_SIZEOF_FACET];

Inconsistent Standard_Character/char.

+  const Standard_Integer NBTriangles = myTriangulation->NbTriangles();
+  for (Standard_Integer i = 1; i <= NBTriangles; ++i)

Name conventions for local variables (suffix a/an).

+  for (Standard_Integer i  = 0; i < aNodes.Size();  ++i)

Extra spaces.

+Standard_Boolean RWStl::IsASCII(const Standard_CString thePath)

It would be better having a method using already opened file - opening file might be an expensive operation (e.g. within Windows Network).

+    FILE * aF = OSD_OpenFile(aCPath, "r");
...
+    FILE * aF = OSD_OpenFile(aCPath, "w");

It would be better opening ASCII STL files in binary mode.

+  TCollection_AsciiString aPath; thePath.SystemName(aPath);
+  Standard_CString aCPath = aPath.ToCString();

There is no point declaring an extra temporary variable aCPath (it is also unsafe practice).
(0067588)
kgv (developer)
2017-06-22 12:15

For reading file by chunks, maybe the following class might be considered useful:
//! Auxiliary tool for buffered reading from input stream
//! within chunks of constant size.
class CadImportBuffer
{
public:

  //! Constructor with initialization.
  CadImportBuffer (int64_t theDataLen,
                   size_t  theChunkLen)
  : myBufferPtr(NULL),
    myBufferEnd(NULL),
    myDataLen  (0),
    myDataRead (0),
    myChunkLen (0),
    myNbChunks (0),
    myBufferLen(0)
  {
    Init (theDataLen, theChunkLen);
  }

  //! Initialize the buffer.
  //! @param theDataLen  the full length of input data to read from stream.
  //! @param theChunkLen the length of single chunk to read
  void Init (int64_t theDataLen,
             size_t  theChunkLen)
  {
    myDataRead  = 0;
    myDataLen   = theDataLen - theDataLen % int64_t(theChunkLen);
    myChunkLen  = theChunkLen;
    myNbChunks  = sizeof(myBuffer) / theChunkLen;
    myBufferLen = theChunkLen * myNbChunks;

    myBufferEnd = myBuffer + sizeof(myBuffer);
    myBufferPtr = myBuffer + sizeof(myBuffer);
    memset (myBuffer, 0, sizeof(myBuffer));

    if (theChunkLen > sizeof(myBuffer))
    {
      Standard_ProgramError::Raise ("Internal error - chunk size is greater then preallocated buffer");
    
}
  }

  //! Return TRUE if amount of read bytes is equal to requested length of entire data.
  bool IsDone() const
  {
    return myDataRead == myDataLen;
  }

  //! Read next chunk.
  //! @return pointer to the chunk or NULL on error / end of reading buffer
  template<typename Chunk_T, typename Stream_T>
  Chunk_T* ReadChunk (Stream_T& theStream)
  {
    return reinterpret_cast<Chunk_T*> (readRawDataChunk (theStream));
  }

  //! Read next chunk.
  //! @return pointer to the chunk or NULL on error / end of reading buffer
  template<typename Stream_T>
  char* ReadDataChunk (Stream_T& theStream)
  {
    return readRawDataChunk (theStream);
  }

private:

  //! Read next chunk.
  //! @return pointer to the chunk or NULL on error / end of reading buffer
  template<typename Stream_T>
  char* readRawDataChunk (Stream_T& theStream)
  {
    myBufferPtr += myChunkLen;
    if (myBufferPtr < myBufferEnd)
    {
      return myBufferPtr;
    }

    const int64_t aDataLeft = myDataLen - myDataRead;
    if (aDataLeft == 0) // myDataLen should be multiple of myChunkLen
    {
      myBufferPtr = NULL;
      return NULL;
    }

    const size_t aDataToRead = int64_t(myBufferLen) > aDataLeft ? size_t(aDataLeft) : myBufferLen;
    
if (!readStream (theStream, aDataToRead))
    {
      myBufferPtr = NULL;
      return NULL;
    }

    myBufferPtr = myBuffer;
    myBufferEnd = myBuffer + aDataToRead;
    myDataRead += aDataToRead;
    return myBufferPtr;
  }

  //! Read from stl stream.
  bool readStream (std::istream& theStream,
                   size_t theLen)
  {
    theStream.read (myBuffer, theLen);
    return theStream.good();
  }

  //! Read from FILE stream.
  bool readStream (FILE* theStream,
                   size_t theLen)
  {
    return ::fread (myBuffer, 1, theLen, theStream) == theLen;
  }

private:

  char        myBuffer[4096]; //!< data cache
  char*       myBufferPtr;    //!< current position within the buffer
  const char* myBufferEnd;    //!< end of the buffer
  int64_t     myDataLen;      //!< length of entire data to read
  int64_t     myDataRead;     //!< amount of data already processed
  size_t      myChunkLen;     //!< length of single chunk that caller would like to read (e.g. iterator 
increment)
  size_t      myNbChunks;     //!< number of cached chunks
  size_t      myBufferLen;    //!< effective length of the buffer to be read at once (multiple of 
chunk length)

};


To be used like this:
  // normal + 3 nodes + 2 extra bytes
  const size_t aVec3Size  = sizeof(float) * 3;
  const size_t aFaceDataLen = aVec3Size * 4 + 2;
  CadImportBuffer aBuffer (aDataLen, aFaceDataLen);
  for (int aFacetIter = 0; aFacetIter < aNbFacets && aPSentry.More(); ++aFacetIter, ++aNbRead, 
aPSentry.Next())
  {
    const char* aBufferPtr = aBuffer.ReadDataChunk (theFile);
    if (aBufferPtr == NULL)
    {
      Message::DefaultMessenger()->Send (TCollection_AsciiString ("File is corrupted or not in 
STL format!"), Message_Fail);
      return false;
    }

    readStlFloatVec3 (aBufferPtr, aNorm);
    readStlFloatVec3 (aBufferPtr + aVec3Size,     aCtx.Places[0]);
    readStlFloatVec3 (aBufferPtr + aVec3Size * 2, aCtx.Places[1]);
    readStlFloatVec3 (aBufferPtr + aVec3Size * 3, aCtx.Places[2]);
    if (!aCtx.pushTriangle())
    {
      return false;
    }
  }
(0067589)
kgv (developer)
2017-06-22 12:26

Please also don't forget updating Products / samples.
(0067626)
git (administrator)
2017-06-23 10:04

Branch CR28840 has been updated by aml.

SHA-1: 5a8e6cc4515db296517b2aa4a2eb229aa1a002ae


Detailed log of new commits:

Author: aml
Date: Fri Jun 23 10:03:15 2017 +0300

    remarks correction.

(0067628)
aml (developer)
2017-06-23 10:36

Dear kgv,

Thank you for the reviewing. Could you please check updated version of the CR28840 branch in OCCT and CR28840 products? All remarks except inefficient shape saving should be fixed. If you want to implement it, please create a new issue since this activity is out of the scope of this issue.
(0067632)
kgv (developer)
2017-06-23 11:16

     TCollection_AsciiString aPath((Standard_CString)file.toLatin1().constData());
-    OSD_Path aFile( aPath );
-    Handle(StlMesh_Mesh) aMesh = RWStl::ReadFile (aFile);
+
+    Handle(Poly_Triangulation) aMesh;
+    StlAPI_Reader::Read(aMesh, aPath.ToCString());

While you near these lines, it would be nice also correcting UNICODE path:
> TCollection_AsciiString aPath (file.toUtf8().data());

-  Standard_Character* aDataChunk = new char[CHUNKSIZE * THE_STL_SIZEOF_FACET];
+  const Standard_Integer SIZE = CHUNKSIZE * THE_STL_SIZEOF_FACET;
+  NCollection_Array1<Standard_Character> aData(1, SIZE);
+  Standard_Character* aDataChunk = &aData.ChangeFirst();

You have corrected only ONE occurrence of this chunks code.

+  for (Standard_Integer aTriIdx = 0; aTriIdx < aNbTri && aPS.More() ;)
...
+  return Standard_True;

nit: it would be better not returning TRUE if user has aborted progress before file has been completely read or written.
(0067640)
git (administrator)
2017-06-23 13:29

Branch CR28840 has been updated by aml.

SHA-1: ea411d0c912377bf896aee45754bc5279ca537fc


Detailed log of new commits:

Author: aml
Date: Fri Jun 23 13:29:24 2017 +0300

    remarks correction.

(0067641)
git (administrator)
2017-06-23 13:31

Branch CR28840_1 has been created by aml.

SHA-1: 143eec14106666e25b4e79488a868a5b573fe26a


Detailed log of new commits:

Author: aml
Date: Wed Jun 14 08:07:26 2017 +0300

    0028840: Data Exchange - rewrite the STL Reader/Writer
    
    New implementation of the STL Reader / Writer. New implementation has the following advantages:
    1) Possibility to read STL file to the Poly_Triangulation has been added.
    2) Chunk-based reading for the binary files has been added. It significantly improves reading and writing of the binary files.
    3) New implementation does not have dependencies.
    
    Obsolete code related to the old implementation has been deleted.
    Test cases have been added.
    Upgrade guide has been updated.
(0067648)
git (administrator)
2017-06-23 14:04

Branch CR28840_1 has been updated forcibly by aml.

SHA-1: 284012614a5716a9586f5d3abbb95dc36f79c113
(0067675)
git (administrator)
2017-06-26 10:02

Branch CR28840_1 has been updated forcibly by aml.

SHA-1: f109ea399e6eaab9f5740e49fbe57927862acf21
(0067676)
aml (developer)
2017-06-26 10:03

Dear kgv,
Please check updated versions of the CR28840_1 in the OCCT repository and CR28840 in products repository. Problems with samples compilation have been fixed.
(0067705)
kgv (developer)
2017-06-27 08:48

It would be nice having scale/units conversion API in STL import/export, but this is out of scope of this bug.
(0067720)
git (administrator)
2017-06-27 11:43

Branch CR28840_1 has been updated forcibly by aml.

SHA-1: e3654cf4cd183288878c98ea30c5c2f3bc77b310
(0067721)
git (administrator)
2017-06-27 12:25

Branch CR28840_1 has been updated forcibly by aml.

SHA-1: 943d02a38969dfad54d440a2629eb4f78bdab444
(0068112)
git (administrator)
2017-07-09 22:44

Branch CR28840_2 has been created by abv.

SHA-1: 31b3f1140d0705c8a4ce850c5d5bbaefc390045c


Detailed log of new commits:

Author: abv
Date: Sun Jul 9 17:08:52 2017 +0300

    Refactoring of STL reader:
    
    - basic reading is separated to abstract class RWStl_Reader which is not bound to particular data structures
    - different implementation is taken as basis for reader

Author: aml
Date: Wed Jun 14 08:07:26 2017 +0300

    0028840: Data Exchange - rewrite the STL Reader/Writer
    
    New implementation of the STL Reader / Writer. New implementation has the following advantages:
    1) Possibility to read STL file to the Poly_Triangulation has been added.
    2) Chunk-based reading for the binary files has been added. It significantly improves reading and writing of the binary files.
    3) New implementation does not have dependencies.
    
    Obsolete code related to the old implementation has been deleted.
    Test cases have been added.
    Upgrade guid and user guides have been updated.
(0068461)
git (administrator)
2017-07-20 07:50

Branch CR28840_2 has been updated forcibly by abv.

SHA-1: f229de48c82e699a05836bdf43dd2b4cfd8c25fd
(0068767)
git (administrator)
2017-07-27 11:19

Branch CR28840_2 has been updated by abv.

SHA-1: bc1498bdbf1298c4addaec536a3fafe3066d6d8a


Detailed log of new commits:

Author: abv
Date: Thu Jul 27 11:18:29 2017 +0300

    Port to GCC

Author: abv
Date: Thu Jul 27 10:54:04 2017 +0300

    Fix compiler warning

(0068822)
git (administrator)
2017-07-27 19:45

Branch CR28840_2 has been updated by abv.

SHA-1: db819d8bce7bde18b73914cc2fcfc2634ad907e5


Detailed log of new commits:

Author: abv
Date: Thu Jul 27 19:44:53 2017 +0300

    Port to GCC

(0068865)
git (administrator)
2017-07-29 22:56

Branch CR28840_3 has been created by abv.

SHA-1: 4bec40622bdaa82f67bb6080460995b0aa69b013


Detailed log of new commits:

Author: aml
Date: Wed Jun 14 08:07:26 2017 +0300

    0028840: Data Exchange - rewrite the STL Reader/Writer
    
    STL Reader and Writer tools have been refactored to improve performance and usability:
    - Basic reading of STL file is separated to abstract class RWStl_Reader which is not bound to particular data structures; the target data model can be bound via inheritance.
    - RWStl package uses class Poly_Triangulation to represent triangular mesh.
    - Obsolete data structures and tools (packages StlMesh and StlTransfer) are removed.
    
    Test cases have been added.
    
    Linker option /LARGEADDRESAWARE is added to DRAW executable generated by genproj to allow testing of memory-hungry procedures in 32-bit DRAW.
(0068866)
git (administrator)
2017-07-30 00:24

Branch CR28840_3 has been updated forcibly by abv.

SHA-1: 786578a2c2889a7e93c3bafb3c7bf04c555fd357
(0068867)
git (administrator)
2017-07-30 10:01

Branch CR28840_3 has been updated forcibly by abv.

SHA-1: 341e0b0e461b59938d556d63d7fe598dec589a09
(0068868)
git (administrator)
2017-07-30 10:30

Branch CR28840_3 has been updated by abv.

SHA-1: d25bcb9456f1e7693868a0e8a8308015b577078f


Detailed log of new commits:

Author: abv
Date: Sun Jul 30 10:30:18 2017 +0300

    Added check of "endsolid", "facet", and "outer loop" keywords in Ascii file reader (-10% performance)

(0068870)
git (administrator)
2017-07-30 22:38

Branch CR28840_3 has been updated forcibly by abv.

SHA-1: d2a143034a30ecd51931a28f90eeb88a7723c531
(0068871)
abv (manager)
2017-07-30 22:39

I have pushed modified version of the patch to CR28840_3, please review.

The changes are:
- Implementation of STL reader is borrowed from another project (about twice faster).
- STL reader is separated to abstract class not bound to particular data structure; binding is done by inheritance. This should allow reusing this reader for different target data structures with minimum overheads.
- Unnecessary changes in interface of RWStl and StlAPI classes are reverted to reduce porting issues (remaining changes are replacement of StlMesh_Mesh by Poly_Triangulation, and StlAPI_ErrorStatus by boolean).

Some remarks:

1. The reader class now works with C++ streams which in my experiments have not shown any performance penalties as compared to C files (note that only unformatted read functions are used). Usage of streams should allow reading STL data from arbitrary streams (e.g. HTTP, byte streams embedded in documents, zipped files etc.). It also makes it easy to implement reading of multi-domain files (see #28680).

2. Binary reader relies on number of facets encoded in the header, rather than estimating it by the file size. I believe this should be more consistent (unless there is counter-example).

3. In the Ascii reader additional check for consistency of data is added: "endsolid" stops reading, and missing "facet" and "outer" on relevant lines causes error. Empty lines are not supported (as before). This should make reader more reliable, but causes ~ 10% downgrade of performance.
(0068872)
abv (manager)
2017-07-30 22:42

Performance has been tested using two large STL files (the same data in binary and Ascii format), and one smaller Ascii STL file, on workstation with i7-4790 CPU.

Here are the timings:

Read A1_cleaned.stl (binary 860 M):
CAD Assistant: 8.45 sec
CR28840_1: 18.4 sec (25.4)
CR28840_2: 8.8 sec (9.9)
streams: 8.75 sec (9.8)
CR28840_3 (streams + hash): 7.2 sec (8.3) [32-bit: 8 sec (9.2)]
streams + hash + std::vector: 6.8 sec (7.8)

Read A1_cleaned.stla (Ascii 4670 M):
CAD Assistant: 102 sec
CR28840_1: 213 sec (3:45)
CR28840_2: 113 sec (1:56)
streams: 107 sec (2:13) [32-bit: 163 sec (3:37)]
CR28840_3 (streams + hash + checks): 117 sec

Read liver.stl (Ascii 6582 K)
CR28840_1: 0.31 sec (0.33)
CR28840_2: 0.19 sec (0.19)
streams: 0.15 (0.23)

Note that improvement of hash function for points accelerated binary reader by ~ 20%.

Usage of std::vector instead of NCollection_Vector for intermediate buffer yields additional ~ 5% acceleration but leads to memory overheads (about 1000 Mb vs 900 Mb of peak), hence reverted.
(0068873)
abv (manager)
2017-07-30 22:49

Compilation has been tested on Windows (vc 10 64 and 32 bit, vc 17) and Linux (gcc 5.2.1); still it is necessary to check building on other compilers, especially old GCC.

Tests included with the patch are based on STL files located on AML workstation that I do not have access to (\\aml\Share\bug28840.zip); these are to be added to tests database (unless theu are very large).

Also I do not have access to Products that may need to be corrected...
(0068880)
git (administrator)
2017-07-31 09:44

Branch CR28840_4 has been created by kgv.

SHA-1: a75ae1087f950ab7235ed11e7238261ce2b439fd


Detailed log of new commits:

Author: aml
Date: Wed Jun 14 08:07:26 2017 +0300

    0028840: Data Exchange - rewrite the STL Reader/Writer
    
    STL Reader and Writer tools have been refactored to improve performance and usability:
    - Basic reading of STL file is separated to abstract class RWStl_Reader which is not bound to particular data structures; the target data model can be bound via inheritance.
    - RWStl package uses class Poly_Triangulation to represent triangular mesh.
    - Obsolete data structures and tools (packages StlMesh and StlTransfer) are removed.
    
    Test cases have been added.
(0068888)
git (administrator)
2017-07-31 12:18

Branch CR28840_3 has been updated by kgv.

SHA-1: b4f418a3b2ef472f626090444e7a82a71dbd4838


Detailed log of new commits:

Author: aml
Date: Wed Jun 14 08:07:26 2017 +0300

    # cosmetics

(0068889)
git (administrator)
2017-07-31 12:20

Branch CR28840_4 has been updated forcibly by kgv.

SHA-1: c273d4caf7bd3a02a2d5696f191abbff8d0df384
(0068890)
git (administrator)
2017-07-31 12:21

Branch CR28840_4_new_testgrid has been created by kgv.

SHA-1: 878b7844e3b8c91d44f12b5150795d1498e96321


Detailed log of new commits:

Author: aml
Date: Wed Jun 14 08:07:26 2017 +0300

    0028840: Data Exchange - rewrite the STL Reader/Writer
    
    Add new test grid for STL read/write.
(0068893)
git (administrator)
2017-07-31 12:37

Branch CR28840_3 has been updated forcibly by kgv.

SHA-1: c479c6368045a364e685a573f8f7fb79d2674604
(0068894)
git (administrator)
2017-07-31 12:39

Branch CR28840_4 has been updated forcibly by kgv.

SHA-1: f5dc528070ad3793b4809b45fefc46db863c7862
(0068903)
git (administrator)
2017-07-31 14:58

Branch CR28840_4 has been updated forcibly by kgv.

SHA-1: 564aa6c0ea249c47365ac69005d5191ae31b5452
(0068904)
git (administrator)
2017-07-31 14:59

Branch CR28840_3 has been updated by kgv.

SHA-1: c8fbe631754e5462cd3dc30d8b1e5b21d0810973


Detailed log of new commits:

Author: kgv
Date: Mon Jul 31 14:02:45 2017 +0300

    # fix VC14 compilation error

(0068995)
git (administrator)
2017-08-01 08:35

Branch CR28840_3 has been updated by kgv.

SHA-1: e4e55d64f65481533795600b40daa27be80f9d58


Detailed log of new commits:

Author: kgv
Date: Tue Aug 1 08:34:40 2017 +0300

    # eliminate GCC warnings -Wsign-compare and CLang warnings -Wunused-const-variable -Winconsistent-missing-override

(0068996)
git (administrator)
2017-08-01 08:36

Branch CR28840_4 has been updated by kgv.

SHA-1: bf68a591500c5549fe2007b36323995d3132140b


Detailed log of new commits:

Author: kgv
Date: Tue Aug 1 08:34:40 2017 +0300

    # eliminate GCC warnings -Wsign-compare and CLang warnings -Wunused-const-variable -Winconsistent-missing-override

(0068997)
git (administrator)
2017-08-01 08:37

Branch CR28840_5 has been created by kgv.

SHA-1: 049239b4137f475fedf30d42e6c82c6c61e86938


Detailed log of new commits:

Author: aml
Date: Wed Jun 14 08:07:26 2017 +0300

    0028840: Data Exchange - rewrite the STL Reader/Writer
    
    STL Reader and Writer tools have been refactored to improve performance and usability:
    - Basic reading of STL file is separated to abstract class RWStl_Reader which is not bound to particular data structures; the target data model can be bound via inheritance.
    - RWStl package uses class Poly_Triangulation to represent triangular mesh.
    - Obsolete data structures and tools (packages StlMesh and StlTransfer) are removed.
(0069631)
git (administrator)
2017-08-18 17:00

Branch CR28840 has been deleted by kgv.

SHA-1: ea411d0c912377bf896aee45754bc5279ca537fc
(0069632)
git (administrator)
2017-08-18 17:00

Branch CR28840_1 has been deleted by kgv.

SHA-1: 943d02a38969dfad54d440a2629eb4f78bdab444
(0069633)
git (administrator)
2017-08-18 17:00

Branch CR28840_2 has been deleted by kgv.

SHA-1: db819d8bce7bde18b73914cc2fcfc2634ad907e5
(0069634)
git (administrator)
2017-08-18 17:00

Branch CR28840_3 has been deleted by kgv.

SHA-1: e4e55d64f65481533795600b40daa27be80f9d58
(0069635)
git (administrator)
2017-08-18 17:00

Branch CR28840_4 has been deleted by kgv.

SHA-1: bf68a591500c5549fe2007b36323995d3132140b
(0069636)
git (administrator)
2017-08-18 17:00

Branch CR28840_4_new_testgrid has been deleted by kgv.

SHA-1: 878b7844e3b8c91d44f12b5150795d1498e96321
(0069637)
git (administrator)
2017-08-18 17:00

Branch CR28840_5 has been deleted by kgv.

SHA-1: 049239b4137f475fedf30d42e6c82c6c61e86938

- Related Changesets
occt: master 4178b353
Timestamp: 2017-06-14 05:07:26
Author: aml
Committer: bugmaster
Details ] Diff ]
0028840: Data Exchange - rewrite the STL Reader/Writer

STL Reader and Writer tools have been refactored to improve performance and usability:
- Basic reading of STL file is separated to abstract class RWStl_Reader which is not bound to particular data structures; the target data model can be bound via inheritance.
- RWStl package uses class Poly_Triangulation to represent triangular mesh.
- Obsolete data structures and tools (packages StlMesh and StlTransfer) are removed.
mod - adm/UDLIST Diff ] File ]
mod - dox/dev_guides/upgrade/upgrade.md Diff ] File ]
mod - dox/user_guides/visualization/visualization.md Diff ] File ]
mod - samples/xaml/MainPage.xaml.cpp Diff ] File ]
mod - src/QABugs/QABugs_2.cxx Diff ] File ]
mod - src/RWStl/FILES Diff ] File ]
mod - src/RWStl/RWStl.cxx Diff ] File ]
mod - src/RWStl/RWStl.hxx Diff ] File ]
add - src/RWStl/RWStl_Reader.cxx Diff ] File ]
add - src/RWStl/RWStl_Reader.hxx Diff ] File ]
mod - src/StlAPI/FILES Diff ] File ]
mod - src/StlAPI/StlAPI.cxx Diff ] File ]
mod - src/StlAPI/StlAPI.hxx Diff ] File ]
rm - src/StlAPI/StlAPI_ErrorStatus.hxx Diff ] File ]
mod - src/StlAPI/StlAPI_Reader.cxx Diff ] File ]
mod - src/StlAPI/StlAPI_Reader.hxx Diff ] File ]
mod - src/StlAPI/StlAPI_Writer.cxx Diff ] File ]
mod - src/StlAPI/StlAPI_Writer.hxx Diff ] File ]
rm - src/StlMesh/FILES Diff ] File ]
rm - src/StlMesh/StlMesh.cxx Diff ] File ]
rm - src/StlMesh/StlMesh.hxx Diff ] File ]
rm - src/StlMesh/StlMesh_Mesh.cxx Diff ] File ]
rm - src/StlMesh/StlMesh_Mesh.hxx Diff ] File ]
rm - src/StlMesh/StlMesh_Mesh.lxx Diff ] File ]
rm - src/StlMesh/StlMesh_MeshDomain.cxx Diff ] File ]
rm - src/StlMesh/StlMesh_MeshDomain.hxx Diff ] File ]
rm - src/StlMesh/StlMesh_MeshDomain.lxx Diff ] File ]
rm - src/StlMesh/StlMesh_MeshExplorer.cxx Diff ] File ]
rm - src/StlMesh/StlMesh_MeshExplorer.hxx Diff ] File ]
rm - src/StlMesh/StlMesh_MeshExplorer.lxx Diff ] File ]
rm - src/StlMesh/StlMesh_MeshTriangle.cxx Diff ] File ]
rm - src/StlMesh/StlMesh_MeshTriangle.hxx Diff ] File ]
rm - src/StlMesh/StlMesh_SequenceOfMesh.hxx Diff ] File ]
rm - src/StlMesh/StlMesh_SequenceOfMeshDomain.hxx Diff ] File ]
rm - src/StlMesh/StlMesh_SequenceOfMeshTriangle.hxx Diff ] File ]
rm - src/StlTransfer/FILES Diff ] File ]
rm - src/StlTransfer/StlTransfer.cxx Diff ] File ]
rm - src/StlTransfer/StlTransfer.hxx Diff ] File ]
mod - src/TKSTL/PACKAGES Diff ] File ]
mod - src/XSDRAWSTLVRML/XSDRAWSTLVRML.cxx Diff ] File ]
mod - src/XSDRAWSTLVRML/XSDRAWSTLVRML_DataSource.cxx Diff ] File ]
mod - src/XSDRAWSTLVRML/XSDRAWSTLVRML_DataSource.hxx Diff ] File ]
mod - src/XSDRAWSTLVRML/XSDRAWSTLVRML_DataSource3D.cxx Diff ] File ]
rm - tests/bugs/moddata_1/bug1048 Diff ] File ]

- Issue History
Date Modified Username Field Change
2017-06-13 11:45 aml New Issue
2017-06-13 11:45 aml Assigned To => aml
2017-06-13 11:47 aml Relationship added related to 0025388
2017-06-13 11:47 aml Relationship added related to 0017422
2017-06-13 11:47 aml Relationship added related to 0027005
2017-06-13 13:23 abv Relationship added related to 0026338
2017-06-13 13:38 kgv Severity minor => integration request
2017-06-13 13:38 kgv Summary Rewrite the STL Reader/Writer => Data Exchange - rewrite the STL Reader/Writer
2017-06-16 13:55 git Note Added: 0067484
2017-06-16 14:04 kgv Note Added: 0067486
2017-06-22 07:01 git Note Added: 0067577
2017-06-22 07:04 aml Note Added: 0067578
2017-06-22 07:04 aml Assigned To aml => abv
2017-06-22 07:04 aml Status new => resolved
2017-06-22 07:04 aml Steps to Reproduce Updated View Revisions
2017-06-22 11:24 kgv Note Added: 0067584
2017-06-22 11:29 kgv Note Added: 0067585
2017-06-22 11:31 szv Note Added: 0067586
2017-06-22 12:10 kgv Note Added: 0067587
2017-06-22 12:11 kgv Assigned To abv => aml
2017-06-22 12:11 kgv Status resolved => assigned
2017-06-22 12:15 kgv Note Added: 0067588
2017-06-22 12:26 kgv Note Added: 0067589
2017-06-23 10:04 git Note Added: 0067626
2017-06-23 10:36 aml Note Added: 0067628
2017-06-23 10:36 aml Assigned To aml => kgv
2017-06-23 10:36 aml Status assigned => resolved
2017-06-23 10:38 kgv Assigned To kgv => abv
2017-06-23 11:16 kgv Note Added: 0067632
2017-06-23 13:29 git Note Added: 0067640
2017-06-23 13:31 git Note Added: 0067641
2017-06-23 13:54 aml Assigned To abv => aml
2017-06-23 13:54 aml Status resolved => assigned
2017-06-23 14:04 git Note Added: 0067648
2017-06-26 10:02 git Note Added: 0067675
2017-06-26 10:03 aml Note Added: 0067676
2017-06-26 10:03 aml Assigned To aml => kgv
2017-06-26 10:03 aml Status assigned => resolved
2017-06-26 10:10 kgv Assigned To kgv => abv
2017-06-27 08:48 kgv Note Added: 0067705
2017-06-27 11:43 git Note Added: 0067720
2017-06-27 12:25 git Note Added: 0067721
2017-07-09 22:44 git Note Added: 0068112
2017-07-10 10:25 kgv Relationship added related to 0024729
2017-07-20 07:50 git Note Added: 0068461
2017-07-27 11:19 git Note Added: 0068767
2017-07-27 19:45 git Note Added: 0068822
2017-07-29 22:56 git Note Added: 0068865
2017-07-30 00:24 git Note Added: 0068866
2017-07-30 09:26 abv Relationship added related to 0028680
2017-07-30 10:01 git Note Added: 0068867
2017-07-30 10:30 git Note Added: 0068868
2017-07-30 22:38 git Note Added: 0068870
2017-07-30 22:39 abv Note Added: 0068871
2017-07-30 22:42 abv Note Added: 0068872
2017-07-30 22:49 abv Note Added: 0068873
2017-07-30 22:50 abv Assigned To abv => kgv
2017-07-31 09:44 git Note Added: 0068880
2017-07-31 12:18 git Note Added: 0068888
2017-07-31 12:20 git Note Added: 0068889
2017-07-31 12:21 git Note Added: 0068890
2017-07-31 12:37 git Note Added: 0068893
2017-07-31 12:39 git Note Added: 0068894
2017-07-31 14:58 git Note Added: 0068903
2017-07-31 14:59 git Note Added: 0068904
2017-08-01 08:35 git Note Added: 0068995
2017-08-01 08:36 git Note Added: 0068996
2017-08-01 08:37 git Note Added: 0068997
2017-08-01 08:48 kgv Relationship added related to 0028965
2017-08-02 15:43 kgv Relationship added related to 0028970
2017-08-03 17:31 bugmaster Changeset attached => occt master 4178b353
2017-08-03 17:31 bugmaster Assigned To kgv => bugmaster
2017-08-03 17:31 bugmaster Status resolved => verified
2017-08-03 17:31 bugmaster Resolution open => fixed
2017-08-07 09:45 azv Relationship added parent of 0028974
2017-08-15 14:28 azv Relationship added related to 0028996
2017-08-18 13:04 bugmaster Test case number => group : de_mesh
2017-08-18 17:00 git Note Added: 0069631
2017-08-18 17:00 git Note Added: 0069632
2017-08-18 17:00 git Note Added: 0069633
2017-08-18 17:00 git Note Added: 0069634
2017-08-18 17:00 git Note Added: 0069635
2017-08-18 17:00 git Note Added: 0069636
2017-08-18 17:00 git Note Added: 0069637
2017-09-29 16:17 user533 Fixed in Version => 7.2.0
2017-09-29 16:29 user533 Status verified => closed
2017-10-04 11:42 abv Relationship added related to 0026100
2018-09-11 18:18 kgv Relationship added parent of 0030130


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker