MantisBT - Open CASCADE
View Issue Details
0028840Open CASCADE[OCCT] OCCT:Data Exchangepublic2017-06-13 11:452018-09-11 18:18
aml 
bugmaster 
normalintegration request 
closedfixed 
[OCCT] 7.1.0 
[OCCT] 7.2.0[OCCT] 7.2.0 
group : de_mesh
0028840: Data Exchange - rewrite the STL Reader/Writer
The 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.
New grid "de_mesh" has been added.
Test shapes can be downloaded from: \\aml\Share\bug28840.zip
No tags attached.
related to 0025388closed bugmaster Open CASCADE Improve STL reader to generate one triangulation instead of compound of faces 
related to 0017422closed bugmaster Open CASCADE StlMesh_MeshDomain::AddOnlyNewVertex is incorrect 
related to 0027005closed bugmaster Community Standard_Integer StlMesh_MeshDomain::AddOnlyNewVertex(...) does not work 
parent of 0028974closed bugmaster Open CASCADE Test cases for STL reader/writer 
parent of 0030130verified bugmaster Open CASCADE Coding Rules - MSVC 2017 gives warnings about using of std::fpos::seekpos() within RWStl 
related to 0026338closed bugmaster Community STL export (especially binary) needs a lot of time if selected export path is not local 
related to 0024729closed bugmaster Community Duplicate code to compute the normals for TopoDS_Face 
related to 0028965closed kgv Open CASCADE Jenkings Certification Tool - CLang warning is ignored within final report for LLVM/Clang 
related to 0026100closed abv Open CASCADE Incorrect printout in StlTransfer::RetrieveMesh 
Issue History
2017-06-13 11:45amlNew Issue
2017-06-13 11:45amlAssigned To => aml
2017-06-13 11:47amlRelationship addedrelated to 0025388
2017-06-13 11:47amlRelationship addedrelated to 0017422
2017-06-13 11:47amlRelationship addedrelated to 0027005
2017-06-13 13:23abvRelationship addedrelated to 0026338
2017-06-13 13:38kgvSeverityminor => integration request
2017-06-13 13:38kgvSummaryRewrite the STL Reader/Writer => Data Exchange - rewrite the STL Reader/Writer
2017-06-16 13:55gitNote Added: 0067484
2017-06-16 14:04kgvNote Added: 0067486
2017-06-22 07:01gitNote Added: 0067577
2017-06-22 07:04amlNote Added: 0067578
2017-06-22 07:04amlAssigned Toaml => abv
2017-06-22 07:04amlStatusnew => resolved
2017-06-22 07:04amlSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=16932#r16932
2017-06-22 11:24kgvNote Added: 0067584
2017-06-22 11:29kgvNote Added: 0067585
2017-06-22 11:31szvNote Added: 0067586
2017-06-22 12:10kgvNote Added: 0067587
2017-06-22 12:11kgvAssigned Toabv => aml
2017-06-22 12:11kgvStatusresolved => assigned
2017-06-22 12:15kgvNote Added: 0067588
2017-06-22 12:26kgvNote Added: 0067589
2017-06-23 10:04gitNote Added: 0067626
2017-06-23 10:36amlNote Added: 0067628
2017-06-23 10:36amlAssigned Toaml => kgv
2017-06-23 10:36amlStatusassigned => resolved
2017-06-23 10:38kgvAssigned Tokgv => abv
2017-06-23 11:16kgvNote Added: 0067632
2017-06-23 13:29gitNote Added: 0067640
2017-06-23 13:31gitNote Added: 0067641
2017-06-23 13:54amlAssigned Toabv => aml
2017-06-23 13:54amlStatusresolved => assigned
2017-06-23 14:04gitNote Added: 0067648
2017-06-26 10:02gitNote Added: 0067675
2017-06-26 10:03amlNote Added: 0067676
2017-06-26 10:03amlAssigned Toaml => kgv
2017-06-26 10:03amlStatusassigned => resolved
2017-06-26 10:10kgvAssigned Tokgv => abv
2017-06-27 08:48kgvNote Added: 0067705
2017-06-27 11:43gitNote Added: 0067720
2017-06-27 12:25gitNote Added: 0067721
2017-07-09 22:44gitNote Added: 0068112
2017-07-10 10:25kgvRelationship addedrelated to 0024729
2017-07-20 07:50gitNote Added: 0068461
2017-07-27 11:19gitNote Added: 0068767
2017-07-27 19:45gitNote Added: 0068822
2017-07-29 22:56gitNote Added: 0068865
2017-07-30 00:24gitNote Added: 0068866
2017-07-30 09:26abvRelationship addedrelated to 0028680
2017-07-30 10:01gitNote Added: 0068867
2017-07-30 10:30gitNote Added: 0068868
2017-07-30 22:38gitNote Added: 0068870
2017-07-30 22:39abvNote Added: 0068871
2017-07-30 22:42abvNote Added: 0068872
2017-07-30 22:49abvNote Added: 0068873
2017-07-30 22:50abvAssigned Toabv => kgv
2017-07-31 09:44gitNote Added: 0068880
2017-07-31 12:18gitNote Added: 0068888
2017-07-31 12:20gitNote Added: 0068889
2017-07-31 12:21gitNote Added: 0068890
2017-07-31 12:37gitNote Added: 0068893
2017-07-31 12:39gitNote Added: 0068894
2017-07-31 14:58gitNote Added: 0068903
2017-07-31 14:59gitNote Added: 0068904
2017-08-01 08:35gitNote Added: 0068995
2017-08-01 08:36gitNote Added: 0068996
2017-08-01 08:37gitNote Added: 0068997
2017-08-01 08:48kgvRelationship addedrelated to 0028965
2017-08-02 15:43kgvRelationship addedrelated to 0028970
2017-08-03 17:31bugmasterChangeset attached => occt master 4178b353
2017-08-03 17:31bugmasterAssigned Tokgv => bugmaster
2017-08-03 17:31bugmasterStatusresolved => verified
2017-08-03 17:31bugmasterResolutionopen => fixed
2017-08-07 09:45azvRelationship addedparent of 0028974
2017-08-15 14:28azvRelationship addedrelated to 0028996
2017-08-18 13:04bugmasterTest case number => group : de_mesh
2017-08-18 17:00gitNote Added: 0069631
2017-08-18 17:00gitNote Added: 0069632
2017-08-18 17:00gitNote Added: 0069633
2017-08-18 17:00gitNote Added: 0069634
2017-08-18 17:00gitNote Added: 0069635
2017-08-18 17:00gitNote Added: 0069636
2017-08-18 17:00gitNote Added: 0069637
2017-09-29 16:17aivFixed in Version => 7.2.0
2017-09-29 16:29aivStatusverified => closed
2017-10-04 11:42abvRelationship addedrelated to 0026100
2018-09-11 18:18kgvRelationship addedparent of 0030130

Notes
(0067484)
git   
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   
2017-06-16 14:04   
+    FILE * aF = fopen(aCPath, "wb");

OSD_OpenFile
(0067577)
git   
2017-06-22 07:01   
Branch CR28840 has been updated forcibly by aml.

SHA-1: 724ef94a8cb2c3de0fd88a72c1cce6776b2c1520
(0067578)
aml   
2017-06-22 07:04   
Dear abv,

Could you please review the CR28840 branch?
(0067584)
kgv   
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   
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   
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   
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   
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   
2017-06-22 12:26   
Please also don't forget updating Products / samples.
(0067626)
git   
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   
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   
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   
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   
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   
2017-06-23 14:04   
Branch CR28840_1 has been updated forcibly by aml.

SHA-1: 284012614a5716a9586f5d3abbb95dc36f79c113
(0067675)
git   
2017-06-26 10:02   
Branch CR28840_1 has been updated forcibly by aml.

SHA-1: f109ea399e6eaab9f5740e49fbe57927862acf21
(0067676)
aml   
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   
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   
2017-06-27 11:43   
Branch CR28840_1 has been updated forcibly by aml.

SHA-1: e3654cf4cd183288878c98ea30c5c2f3bc77b310
(0067721)
git   
2017-06-27 12:25   
Branch CR28840_1 has been updated forcibly by aml.

SHA-1: 943d02a38969dfad54d440a2629eb4f78bdab444
(0068112)
git   
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   
2017-07-20 07:50   
Branch CR28840_2 has been updated forcibly by abv.

SHA-1: f229de48c82e699a05836bdf43dd2b4cfd8c25fd
(0068767)
git   
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   
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   
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   
2017-07-30 00:24   
Branch CR28840_3 has been updated forcibly by abv.

SHA-1: 786578a2c2889a7e93c3bafb3c7bf04c555fd357
(0068867)
git   
2017-07-30 10:01   
Branch CR28840_3 has been updated forcibly by abv.

SHA-1: 341e0b0e461b59938d556d63d7fe598dec589a09
(0068868)
git   
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   
2017-07-30 22:38   
Branch CR28840_3 has been updated forcibly by abv.

SHA-1: d2a143034a30ecd51931a28f90eeb88a7723c531
(0068871)
abv   
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   
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   
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   
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   
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   
2017-07-31 12:20   
Branch CR28840_4 has been updated forcibly by kgv.

SHA-1: c273d4caf7bd3a02a2d5696f191abbff8d0df384
(0068890)
git   
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   
2017-07-31 12:37   
Branch CR28840_3 has been updated forcibly by kgv.

SHA-1: c479c6368045a364e685a573f8f7fb79d2674604
(0068894)
git   
2017-07-31 12:39   
Branch CR28840_4 has been updated forcibly by kgv.

SHA-1: f5dc528070ad3793b4809b45fefc46db863c7862
(0068903)
git   
2017-07-31 14:58   
Branch CR28840_4 has been updated forcibly by kgv.

SHA-1: 564aa6c0ea249c47365ac69005d5191ae31b5452
(0068904)
git   
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   
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   
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   
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.
(0069634)
git   
2017-08-18 17:00   
Branch CR28840_3 has been deleted by kgv.

SHA-1: e4e55d64f65481533795600b40daa27be80f9d58
(0069633)
git   
2017-08-18 17:00   
Branch CR28840_2 has been deleted by kgv.

SHA-1: db819d8bce7bde18b73914cc2fcfc2634ad907e5
(0069632)
git   
2017-08-18 17:00   
Branch CR28840_1 has been deleted by kgv.

SHA-1: 943d02a38969dfad54d440a2629eb4f78bdab444
(0069631)
git   
2017-08-18 17:00   
Branch CR28840 has been deleted by kgv.

SHA-1: ea411d0c912377bf896aee45754bc5279ca537fc
(0069635)
git   
2017-08-18 17:00   
Branch CR28840_4 has been deleted by kgv.

SHA-1: bf68a591500c5549fe2007b36323995d3132140b
(0069636)
git   
2017-08-18 17:00   
Branch CR28840_4_new_testgrid has been deleted by kgv.

SHA-1: 878b7844e3b8c91d44f12b5150795d1498e96321
(0069637)
git   
2017-08-18 17:00   
Branch CR28840_5 has been deleted by kgv.

SHA-1: 049239b4137f475fedf30d42e6c82c6c61e86938