View Issue Details

IDProjectCategoryView StatusLast Update
0028840Open CASCADEOCCT:Data Exchangepublic2021-06-30 13:28
ReporteramlAssigned Tobugmaster  
PrioritynormalSeverityintegration request 
Status closedResolutionfixed 
Product Version7.1.0 
Target Version7.2.0Fixed in Version7.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

Relationships

related to 0025388 closedbugmaster Open CASCADE Improve STL reader to generate one triangulation instead of compound of faces 
related to 0017422 closedbugmaster Open CASCADE StlMesh_MeshDomain::AddOnlyNewVertex is incorrect 
related to 0027005 closedbugmaster Community Standard_Integer StlMesh_MeshDomain::AddOnlyNewVertex(...) does not work 
parent of 0028974 closedbugmaster Open CASCADE Test cases for STL reader/writer 
parent of 0030130 closedbugmaster Open CASCADE Coding Rules - MSVC 2017 gives warnings about using of std::fpos::seekpos() within RWStl 
parent of 0032441 closedbugmaster Open CASCADE Draw Harness - command readstl doesn't print error message on failure 
related to 0026338 closedbugmaster Community STL export (especially binary) needs a lot of time if selected export path is not local 
related to 0024729 closedbugmaster Community Duplicate code to compute the normals for TopoDS_Face 
related to 0026100 closedabv Open CASCADE Incorrect printout in StlTransfer::RetrieveMesh 
related to 0032127 assignedsnn Community Data Exchange - faceted STEP file will not open 

Activities

git

2017-06-16 13:55

administrator   ~0067484

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.

kgv

2017-06-16 14:04

developer   ~0067486

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

OSD_OpenFile

git

2017-06-22 07:01

administrator   ~0067577

Branch CR28840 has been updated forcibly by aml.

SHA-1: 724ef94a8cb2c3de0fd88a72c1cce6776b2c1520

aml

2017-06-22 07:04

developer   ~0067578

Dear abv,

Could you please review the CR28840 branch?

kgv

2017-06-22 11:24

developer   ~0067584

+  // 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...

kgv

2017-06-22 11:29

developer   ~0067585

+  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.

szv

2017-06-22 11:31

manager   ~0067586

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.

kgv

2017-06-22 12:10

developer   ~0067587

   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).

kgv

2017-06-22 12:15

developer   ~0067588

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;
    }
  }

kgv

2017-06-22 12:26

developer   ~0067589

Please also don't forget updating Products / samples.

git

2017-06-23 10:04

administrator   ~0067626

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.

aml

2017-06-23 10:36

developer   ~0067628

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.

kgv

2017-06-23 11:16

developer   ~0067632

     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.

git

2017-06-23 13:29

administrator   ~0067640

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.

git

2017-06-23 13:31

administrator   ~0067641

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.

git

2017-06-23 14:04

administrator   ~0067648

Branch CR28840_1 has been updated forcibly by aml.

SHA-1: 284012614a5716a9586f5d3abbb95dc36f79c113

git

2017-06-26 10:02

administrator   ~0067675

Branch CR28840_1 has been updated forcibly by aml.

SHA-1: f109ea399e6eaab9f5740e49fbe57927862acf21

aml

2017-06-26 10:03

developer   ~0067676

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.

kgv

2017-06-27 08:48

developer   ~0067705

It would be nice having scale/units conversion API in STL import/export, but this is out of scope of this bug.

git

2017-06-27 11:43

administrator   ~0067720

Branch CR28840_1 has been updated forcibly by aml.

SHA-1: e3654cf4cd183288878c98ea30c5c2f3bc77b310

git

2017-06-27 12:25

administrator   ~0067721

Branch CR28840_1 has been updated forcibly by aml.

SHA-1: 943d02a38969dfad54d440a2629eb4f78bdab444

git

2017-07-09 22:44

administrator   ~0068112

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.

git

2017-07-20 07:50

administrator   ~0068461

Branch CR28840_2 has been updated forcibly by abv.

SHA-1: f229de48c82e699a05836bdf43dd2b4cfd8c25fd

git

2017-07-27 11:19

administrator   ~0068767

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

git

2017-07-27 19:45

administrator   ~0068822

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

git

2017-07-29 22:56

administrator   ~0068865

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.

git

2017-07-30 00:24

administrator   ~0068866

Branch CR28840_3 has been updated forcibly by abv.

SHA-1: 786578a2c2889a7e93c3bafb3c7bf04c555fd357

git

2017-07-30 10:01

administrator   ~0068867

Branch CR28840_3 has been updated forcibly by abv.

SHA-1: 341e0b0e461b59938d556d63d7fe598dec589a09

git

2017-07-30 10:30

administrator   ~0068868

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)

git

2017-07-30 22:38

administrator   ~0068870

Branch CR28840_3 has been updated forcibly by abv.

SHA-1: d2a143034a30ecd51931a28f90eeb88a7723c531

abv

2017-07-30 22:39

manager   ~0068871

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.

abv

2017-07-30 22:42

manager   ~0068872

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.

abv

2017-07-30 22:49

manager   ~0068873

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...

git

2017-07-31 09:44

administrator   ~0068880

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.

git

2017-07-31 12:18

administrator   ~0068888

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

git

2017-07-31 12:20

administrator   ~0068889

Branch CR28840_4 has been updated forcibly by kgv.

SHA-1: c273d4caf7bd3a02a2d5696f191abbff8d0df384

git

2017-07-31 12:21

administrator   ~0068890

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.

git

2017-07-31 12:37

administrator   ~0068893

Branch CR28840_3 has been updated forcibly by kgv.

SHA-1: c479c6368045a364e685a573f8f7fb79d2674604

git

2017-07-31 12:39

administrator   ~0068894

Branch CR28840_4 has been updated forcibly by kgv.

SHA-1: f5dc528070ad3793b4809b45fefc46db863c7862

git

2017-07-31 14:58

administrator   ~0068903

Branch CR28840_4 has been updated forcibly by kgv.

SHA-1: 564aa6c0ea249c47365ac69005d5191ae31b5452

git

2017-07-31 14:59

administrator   ~0068904

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

git

2017-08-01 08:35

administrator   ~0068995

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

git

2017-08-01 08:36

administrator   ~0068996

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

git

2017-08-01 08:37

administrator   ~0068997

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.

git

2017-08-18 17:00

administrator   ~0069631

Branch CR28840 has been deleted by kgv.

SHA-1: ea411d0c912377bf896aee45754bc5279ca537fc

git

2017-08-18 17:00

administrator   ~0069632

Branch CR28840_1 has been deleted by kgv.

SHA-1: 943d02a38969dfad54d440a2629eb4f78bdab444

git

2017-08-18 17:00

administrator   ~0069633

Branch CR28840_2 has been deleted by kgv.

SHA-1: db819d8bce7bde18b73914cc2fcfc2634ad907e5

git

2017-08-18 17:00

administrator   ~0069634

Branch CR28840_3 has been deleted by kgv.

SHA-1: e4e55d64f65481533795600b40daa27be80f9d58

git

2017-08-18 17:00

administrator   ~0069635

Branch CR28840_4 has been deleted by kgv.

SHA-1: bf68a591500c5549fe2007b36323995d3132140b

git

2017-08-18 17:00

administrator   ~0069636

Branch CR28840_4_new_testgrid has been deleted by kgv.

SHA-1: 878b7844e3b8c91d44f12b5150795d1498e96321

git

2017-08-18 17:00

administrator   ~0069637

Branch CR28840_5 has been deleted by kgv.

SHA-1: 049239b4137f475fedf30d42e6c82c6c61e86938

Related Changesets

occt: master 4178b353

2017-06-14 05:07:26

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.
Affected Issues
0028840
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
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 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-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-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 aiv Fixed in Version => 7.2.0
2017-09-29 16:29 aiv 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
2021-02-12 12:10 kgv Relationship added related to 0032127
2021-06-30 13:28 kgv Relationship added parent of 0032441