DiscussionsIssue archiveOCCT:Modeling Data

Archived issue #0031136

Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces

Open CASCADEOCCT:Modeling Dataclosed107 public notes

Search issues

Description

BinXCAF persistence (BinTools_ShapeSet) does not store per-vertex normal information.

This is a problem in case of triangulation-only Faces, as there is no analytical geometry to restore normals.

Steps to reproduce

pload MODELING XDE OCAF VISUALIZATION
source $env(CSF_OCCTSamplesPath)/tcl/cad.tcl
trinfo res
wavefront res o
readobj o o.obj
binsave o b.bbrep
binrestore b.bbrep b
# same for ASCII format
#save o b.brep
#restore b.brep b
# and for XBF (using BinTools_ShapeSet internally)
#XSave D1 b.xbf
#XOpen b.xbf D2

vclear
vclose ALL

vinit v1/v1
vfront
vdisplay -dispMode 1 res
vfit
vrenderparams -shadingModel phong

vinit v2/v2
vbottom
vdisplay -dispMode 1 b
vfit
vrenderparams -shadingModel phong

Public activity

107 archived notes

Participants are labeled by their role within this record.

01Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Wed Nov 6 19:26:24 2019 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    BinTools - added missing tools for reading/writing short reals.
    BinTools_ShapeSet now defines maps with proper types instead of Standard_Transient.
    BinTools_ShapeSet::ReadTriangulation() - fixed inefficient reading of triangulation data
    with allocation or redundant temporary arrays.
    wavefront command - export "f" instead of obsolete "fo" keys into file.
02Commenter 2
Branch [archived branch] has been deleted by Author.

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

[revision removed]


No new revisions were added by this update.
04Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Fri Mar 13 15:42:53 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces

05Author
+  *  @f$ f_{3} @f$ -- IGNORED(version 1 olny) \\ checked (version 2 or later); 

olny
06Commenter 6
07Commenter 7
Details:
*** Binary Document ***
BinLDrivers_FormatVersion.hxx (added)
- enum class BinLDrivers_FormatVersion added

BinDrivers
- BinDrivers::StorageVersion() changed

BinLDrivers
- Current version type changed from #define to static const BinLDrivers_FormatVersion class member
- Current version value increased from 10 to 11
- BinLDrivers::StorageVersion() changed to BinLDrivers::StringStorageVersion()

BinLDrivers_DocumentRetrievalDriver
BinLDrivers_DocumentSection
BinMDataStd
BinMDataStd_AsciiStringDriver
BinMDataStd_ByteArrayDriver
BinMDataStd_ExtStringArrayDriver
BinMDataStd_IntegerArrayDriver
BinMDataStd_IntegerDriver
BinMDataStd_IntPackedMapDriver
BinMDataStd_NameDriver
BinMDataStd_RealArrayDriver
BinMDataStd_RealDriver
- Magic constants changed to BinLDrivers_FormatVersion enum

*** Binary Shape ***
BinTools_ShapeSet
- enum class BinTools_FormatVersion added
- Function SetFormatNb() is replaced by SetFormat() (behavior changed)
- Type of myTriangulations changed from NCollection_IndexedMap to NCollection_IndexedDataMap (for store info about the need to write normals into a file)
- Version info strings moved from global area to class private section
- Magic constants changed to BinTools_FormatVersion enum
- WriteTriangulation() / ReadTriangulation()
  * Save/Read NeedToWriteNormals for every triangulation (only for VERSION_4)
  * Save/Read triangulation's normals values if it needs (only for VERSION_4)

BinMNaming_NamedShapeDriver
- Current version type changed from #define to static const BinTools_FormatVersion class member
- Current version value increased from 3 to 4

BinMNaming_NamingDriver
BinMXCAFDoc_LocationDriver
BRepTools_ShapeSet
- Magic constants changed to BinTools_FormatVersion enum

BinTools
- added theVersion parameter to Write() functions

*** Text Shape ***
TopTools_ShapeSet
- enum class TopTools_FormatVersion added
- Function SetFormatNb() is replaced by SetFormat() (behavior changed)
- Current version value increased from 2 to 3
- Version info strings moved from global area to class private section

BRepTools_ShapeSet
- Type of myTriangulations changed from TColStd_IndexedMapOfTransient to NCollection_IndexedDataMap (for store info about the need to write normals into a file)
- WriteTriangulation() / ReadTriangulation()
  * Save/Read NeedToWriteNormals for every triangulation (only for VERSION_3)
  * Save/Read triangulation's normals values if it needs (only for VERSION_3)
  
Draw_SaveAndRestoreBase (added)
- Class Draw_SaveAndRestoreBase declaration moved from Draw_Appli.hxx and changed to abstract base class
- Class Draw_SaveAndRestoreBase realization moved from Draw_VariableCommands.cxx
- Draw_First static pointer moved from Draw_VariableCommands.cxx as static member
- GetFirst() static function added
- Derived class Draw_SaveAndRestoreNumber added

DBRep
- Draw_SaveAndRestoreDBRep derived from Draw_SaveAndRestoreBase class added
- Named argument "-version" added to global binsave() function
- static non-member functions changed to Draw_SaveAndRestoreDBRep overrided virtual functions

DrawTrSurf
- Following derived from Draw_SaveAndRestoreBase classes added:
  * Draw_SaveAndRestoreCurve
  * Draw_SaveAndRestoreBezierCurve
  * Draw_SaveAndRestoreBSplineCurve
  * Draw_SaveAndRestoreCurve2d
  * Draw_SaveAndRestoreBezierCurve2d
  * Draw_SaveAndRestoreBSplineCurve2d
  * Draw_SaveAndRestoreSurface
  * Draw_SaveAndRestoreBezierSurface
  * Draw_SaveAndRestoreBSplineSurface
  * Draw_SaveAndRestorePoint
  * Draw_SaveAndRestoreTriangulation
  * Draw_SaveAndRestorePolygon3D
  * Draw_SaveAndRestorePolygon2D
- static non-member functions changed to derived classes overrided virtual functions

HLRTest
- Derived class Draw_SaveAndRestoreHLRTest added
- static non-member functions changed to derived classes overrided virtual functions

Poly_Triangulation
- Poly_Triangulation constructor with "hasNormals" parameter added

StandardCommands
- Named argument "-version" added to tcl save() DRAW command
  
  
*** XML Document ***
XmlLDrivers_FormatVersion (added)
- enum class XmlLDrivers_FormatVersion added

XmlLDrivers
- Current version type changed from #define to static const XmlLDrivers_FormatVersion class member
- Current version value changed from 9 to 10
- int StorageVersion() replaced by TCollection_AsciiString XmlLDrivers::StringFormatVersion()

CDM_Document
XmlLDrivers_DocumentRetrievalDriver
XmlLDrivers_DocumentStorageDriver
XmlMDataStd_ByteArrayDriver
XmlMDataStd_ExtStringArrayDriver
XmlMDataStd_IntegerArrayDriver
XmlMDataStd_IntPackedMapDriver
XmlMDataStd_RealArrayDriver
XmlMDataStd_TreeNodeDriver
XmlMDF
XmlMNaming_NamedShapeDriver
XmlMNaming_NamingDriver
XmlMXCAFDoc_LocationDriver
- Magic constants changed to XmlLDrivers_FormatVersion enum

CDM_Document
- myStorageFormatVersion type changed from Standard_Integer to XmlLDrivers_FormatVersion


*** Common ***
Storage_HeaderData
- StorageVersion() string function replaced by StringStorageVersion(), BinStorageVersion() and XmlStorageVersion() for compatibility with format version`s enum classes
- SetStorageVersion(const BinLDrivers_FormatVersion&) and SetStorageVersion (const XmlLDrivers_FormatVersion&) overloaded fuctions added

*** Documentation ***
brep_wp.md
- Info about changes in BRep format added
upgrade.md
- Info about changes in API added
08Author
+enum class BinLDrivers_FormatVersion : Standard_Integer

OCCT has to be compilable by some old compilers not supporting enum class.
09Commenter 9
Andrey, could you rebase on new master and run tests again?
10Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Wed Sep 2 16:52:22 2020 +0300

    correction

Author: asuraven
Date: Wed Sep 2 16:38:56 2020 +0300

    Merge branch 'CR31136' of ssh://git.dev.opencascade.org/occt into CR31136
    
    # Conflicts:
    # dox/dev_guides/upgrade/upgrade.md
    # src/BinLDrivers/BinLDrivers_DocumentSection.hxx
    # src/BinMNaming/BinMNaming_NamedShapeDriver.cxx
    # src/BinTools/BinTools.cxx
    # src/BinTools/BinTools.hxx
    # src/BinTools/BinTools_ShapeSet.hxx
    # src/DBRep/DBRep.cxx
    # src/Draw/FILES
    # src/Storage/Storage_Schema.cxx
    # src/TopTools/TopTools_ShapeSet.hxx

Author: asuraven
Date: Wed Sep 2 16:15:20 2020 +0300

    rebase to master

Author: asuraven
Date: Fri Mar 13 15:42:53 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Mon Sep 7 15:21:30 2020 +0300

    Merge branch 'CR31136' of ssh://git.dev.opencascade.org/occt into CR31136
    
    # Conflicts:
    # src/BinMNaming/BinMNaming_NamingDriver.cxx
    # src/Storage/Storage_Schema.cxx
    # src/TopTools/TopTools_ShapeSet.hxx

Author: asuraven
Date: Mon Sep 7 15:11:54 2020 +0300

    Debug code changed

Author: asuraven
Date: Fri Mar 13 15:42:53 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces

Author: asuraven
Date: Wed Sep 2 16:15:20 2020 +0300

    rebase to master

Author: asuraven
Date: Fri Mar 13 15:42:53 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces

12Commenter 12
Mikhail, I have rebased and run tests.
Test results are here: http://occt-tests/CR31136-master-ASURAVEN-Products/Windows-64-VC14/diff_summary.html
Pleace review
13Author
Andrey, git log shows a broken branch.
Could you please correct branch to meet recommended practice (a single bug commit in front of current "master")?
14Commenter 2
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Fri Mar 13 15:42:53 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces (cumulative commit)
15Commenter 15
emun class consructions excluded.
grammatical error fixed.
All changes are merged into [archived branch] commit in Branch CR31136_1
Tests task created and is running now:
http://jenkins-test-12.nnov.opencascade.com/view/CR31136_1-master-ASURAVEN/
Mikhail, Pleace review
16Commenter 16
Andrey, please re-base on new master, there are conflicts.
And re-test.
17Commenter 2
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Fri Mar 13 15:42:53 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces (cumulative commit)
18Commenter 18
New Branch [archived branch] has been created on new master
New tests results: http://jenkins-test-12.nnov.opencascade.com/view/CR31136_2-master-ASURAVEN/view/COMPARE/
Please review
19Commenter 19
In commit message, remove words "(cumulative commit)".
Also, add short information about what has changed.

Please remove all not relevant changes in formatting. It really makes reviewing hard work.

Revert renaming of StorageVersion to StringStorageVersion in BinLDrivers, Storage_Data, Storage_HeaderData, XmlLDrivers. It is unjustified change of API.
20Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: msv
Date: Wed Sep 16 15:35:56 2020 +0300

    #simplify text in upgrade guide

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Wed Sep 16 17:19:23 2020 +0300

    rollback refactoring

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    * 0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Files written with the new version will not be readable by applications of old versions
23Commenter 23
Commit messege changed.
Not relevant changes removed.
StorageVersion function names restored.
All changes commited in new branch CR31136_3
Tests task created and now in progress:
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31136_3-master-ASURAVEN/
24Author
Patch mixes up introduction of new format version and Draw Harness tools to manage versions (+ some methods and constants in public API listing old and new versions).
As the latter consumers considerable amount of patch, it would be better if this part will be moved to dedicated (preceding) patch.

-          myTriangulations.Add(CR->Triangulation());
+          myTriangulations.Add(CR->Triangulation(), Standard_False); // edge triangulation does not need normals
...
     // Add the surface geometry
+    Standard_Boolean needNormals(Standard_False);
     Handle(BRep_TFace) TF = Handle(BRep_TFace)::DownCast(S.TShape());
...
-      if (!Tr.IsNull()) myTriangulations.Add(Tr);
+      if (!Tr.IsNull()) myTriangulations.Add(Tr, needNormals);

This logic looks broken or fragile (if it works as expected - then the note is missing).
Triangulations are shared between polygon-on-edges and surfaces.
At the same time, NCollection_IndexedDataMap::Add() does not modify the value of already added key.
Therefore, behavior will depend on the order of adding the same triangulation for an Edge and for a Face of the surface.

+@subsection upgrade_750_visualizationOfTriangulation Changes in storage of shapes

Section name is misleading as patch is not directly related to visualization but rather to storage formats.

+    const Standard_Boolean NeedToWriteNormals = myTriangulations(i);

toWriteNormals (no upper-case for local variables in new code, please).

+      if (FormatNb() >= TOP_TOOLS_VERSION_3)
+        OS << ((T->HasNormals() && NeedToWriteNormals) ? "1" : "0") << " ";

Here and in other places - please put single-line statements in brackets.

-  TColStd_IndexedMapOfTransient myTriangulations;
+  NCollection_IndexedDataMap<Handle(Standard_Transient), Standard_Boolean, TColStd_MapTransientHasher> myTriangulations;

NCollection_IndexedDataMap<Handle(Poly_Triangulation), Standard_Boolean>
and please put description of the value stored within the map.

+
+enum BinLDrivers_FormatVersion
+{
+  BIN_LDRIVERS_VERSION_2 = 2, // First supported version
...
 
+enum BinTools_FormatVersion
+{

Please add enumeration description and use "//!<" for values.

+// Created on: 2020-03-06
+// Created by: Andrey SURAVENKOV
+// Copyright (c) 2002-2020 OPEN CASCADE SAS

Copyright range looks confusing and non-realistic.

--- a/src/BinMNaming/BinMNaming_NamedShapeDriver.hxx
+++ b/src/BinMNaming/BinMNaming_NamedShapeDriver.hxx
@@ -80,6 +80,7 @@ public:
   //! get the format of topology
     BinTools_LocationSet& GetShapesLocations();
 
+  static const BinTools_FormatVersion THE_CURRENT_VERSION = BIN_TOOLS_VERSION_4;
...
   void SetWithTriangles (const Standard_Boolean isWithTriangles) { myWithTriangles = isWithTriangles; }
+  
+  static const BinTools_FormatVersion THE_CURRENT_VERSION = BIN_TOOLS_VERSION_4;
...
   Standard_Boolean myWithTriangles;
 
+  static Standard_CString Version_1, Version_2, Version_3, Version_4;
...
+  Standard_Boolean         myDisplay;
+
+  static Draw_SaveAndRestoreBase* Draw_FirstSaveAndRestore;

Please put constants and global variables into dedicated "public:" sections.

--- a/src/BinMNaming/BinMNaming_NamingDriver.cxx
+++ b/src/BinMNaming/BinMNaming_NamingDriver.cxx
-         if(!entry.IsEmpty() && !entry.IsEqual(TCollection_AsciiString(NULL_ENTRY))) 
-           {
-             TDF_Label tLab; // Null label.
-             TDF_Tool::Label(anAtt->Label().Data(),entry, tLab, Standard_True);
-             if (!tLab.IsNull()) 
-               aName.ContextLabel(tLab);

Please revert large formatting block changes - the functional changes are pretty local and could be put with previous formatting (even if it was broken).

+  myFormatNb = THE_CURRENT_VERSION;
+}
+//=======================================================================

Please add missing empty line.

+      std::cout << "Syntax error: unknown argument '" << aParam << "'\n";
+      return 1;

Please put error messages into either theDI or Message::SendFail() to make them appear in RED in Draw Harness console.

+               # set version
+        if { $arg == "-version" } {
...
+               # unsupported option
+        error "Error: unsupported option \"$arg\""

Broken indentation.

25Commenter 25
- if(aDocFormatVersion > 9) { // process user defined guid
+ if(aDocFormatVersion >= BIN_LDRIVERS_VERSION_8) { // process user defined guid

Why the condition has been changed?

- if(RelocTable.GetHeaderData()->StorageVersion().IntegerValue() > 8) { // process user defined guid
- const Standard_Integer& aPos = Source.Position();
- Standard_GUID aGuid;
- ok = Source >> aGuid;
- if (!ok) {
- Source.SetPosition(aPos);
- aStrAtt->SetID(TDataStd_AsciiString::GetID());
- ok = Standard_True;
- } else {
- aStrAtt->SetID(aGuid);
- }
+ if(RelocTable.GetHeaderData()->StorageVersion().IntegerValue() >= BIN_LDRIVERS_VERSION_9) { // process user defined guid
+ const Standard_Integer& aPos = Source.Position();
+ Standard_GUID aGuid;
+ ok = Source >> aGuid;
+ if (!ok) {
+ Source.SetPosition(aPos);
+ aStrAtt->SetID(TDataStd_AsciiString::GetID());
+ ok = Standard_True;
+ } else {
+ aStrAtt->SetID(aGuid);
+ }
   } else
- aStrAtt->SetID(TDataStd_AsciiString::GetID());
+ aStrAtt->SetID(TDataStd_AsciiString::GetID());

Not relevant space changes.

- : BinMDF_ADriver (theMsgDriver, STANDARD_TYPE(TNaming_NamedShape)->Name()), myShapeSet(Standard_False),myFormatNb(FORMAT_NUMBER)
+ : BinMDF_ADriver (theMsgDriver, STANDARD_TYPE(TNaming_NamedShape)->Name()), myShapeSet(Standard_False), myFormatNb(THE_CURRENT_VERSION)

Why do you create a new constant BinMNaming_NamedShapeDriver::THE_CURRENT_VERSION instead of using BinTools_ShapeSet::THE_CURRENT_VERSION?

Move enum BinTools_FormatVersion to a dedicated header file.

+ //! Sets the format version to <THE_CURRENT_VERSION>
+ void SetCurrentFormat();

This method is not used, please remove it.

+ static Standard_CString Version_1, Version_2, Version_3, Version_4;

Define each field on independent line.

In BinMNaming_NamingDriver.cxx there are a lot of not relevant space changes.

In CDM_Document.hxx, useless include XmlLDrivers_FormatVersion.hxx. In CDM_Document::CDM_Document(), why do you initialize myStorageFormatVersion with XML_LDRIVERS_VERSION_2? CDM is a base interface and it does not know about XML or BIN formats.

Move enum TopTools_FormatVersion to a dedicated header file.
Move description of format versions from description of TopTools_ShapeSet::FormatNb to the definition of TopTools_FormatVersion. Leave here only the reference to the type TopTools_FormatVersion.

+ static Standard_CString Version_1, Version_2, Version_3;

Define each field on independent line.

+       for {set narg 0} {$narg < [llength $args]} {incr narg} {
+        set arg [lindex $args $narg]
+               
+               # set version


Mix of tabs with spaces ruins indentation. Do not use tabs.

+ theVersion; // to suppress a warning

Use "(void) theVersion;"

+  Handle(DrawTrSurf_BezierCurve) N = new DrawTrSurf_BezierCurve(G,
+                                                                CurvColor,
+                                                                PolesColor,
+                                                                ShowPoles,
+                                                                                 Discret,
+                                                                Deflection,
+                                                                DrawMode);

Do not use tabs. The same in below changed lines in the file DrawTrSurf.cxx.

DrawTrSurf_SaveAndRestoreSurface* classes should be local to cxx file. Move them from the header.
The same is for Draw_SaveAndRestoreDBRep, HLRTest_SaveAndRestore.
Rename Draw_SaveAndRestoreDBRep to DBRep_SaveAndRestore.

+ //! enable a 2D representation). Here the hasNormals flag indicates whether
+ //! normals will be associated with 3D ones

will be given and associated with nodes

+void Storage_HeaderData::SetStorageVersion(const Standard_Integer aVersion)
+{
+  myStorageVersion = TCollection_AsciiString(aVersion);
+}

Make this method inline and call from it the method with the string.

+ if (theFormatNb >= TOP_TOOLS_VERSION_1 && theFormatNb <= THE_CURRENT_VERSION)
+ myFormatNb = theFormatNb;
+ else
+ myFormatNb = THE_CURRENT_VERSION;

It is better to not silently set current version, but make assertion here. The same is in BinTools_ShapeSet::SetFormatNb.

Remove Leading "* " from the first line of the commit message.
Make empty line after the first line of the commit message.

* Files written with the new version will not be readable by applications of old versions

This info is for upgrade guide, but not for commit message.

In commit it is worth mentioning the new optional parameter of save/binsave draw commands.
26Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Sun Sep 20 20:03:28 2020 +0300

    fix formatting

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Mon Sep 21 15:55:48 2020 +0300

    Merge branch 'CR31136_3' of ssh://git.dev.opencascade.org/occt into CR31136_3
    
    # Conflicts:
    # src/Draw/Draw_VariableCommands.cxx
    # src/XmlMDF/XmlMDF.cxx

Author: asuraven
Date: Sun Sep 20 20:03:28 2020 +0300

    fix formatting

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    * 0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Files written with the new version will not be readable by applications of old versions

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

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Sun Sep 20 20:03:28 2020 +0300

    fixes for review

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    * 0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Files written with the new version will not be readable by applications of old versions
30Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Tue Sep 22 17:28:11 2020 +0300

    fix

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands
32Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Sun Sep 20 20:03:28 2020 +0300

    fixes for review

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    * 0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Files written with the new version will not be readable by applications of old versions
34Commenter 34
The problems indicated by msv & kgv have been fixed, except for:
- Dividing the path to two part – because a lot of not relevant changes have been removed,
- Possible broken logic in store needNormals for triangulation – Current logic is correct because Bin(Brep)Tools_ShapeSet::AddGeometry() call from Bin(Brep)Tools_ShapeSet::Add() that recursively down from complex to elementary shapes. As a result, the TopAbs_FACE’s will be processed earlier than the TopAbs_EDGE’s.
The commit message problems have been fixed in new CR31136_4 containing one commit.
Tests task is created and now is in progress:
http://jenkins-test-12.nnov.opencascade.com/view/CR31136_4-master-ASURAVEN/
35Commenter 35
+ if (!Compact)
+ {
+ OS << std::setw(10) << j << " : ";
+ }
+ if (!Compact)
+ {
+ OS << std::setw(17);
+ }

Unite the two 'if' blocks.

BRepTools_ShapeSet::ReadTriangulation:
+ if (FormatNb() >= TOP_TOOLS_VERSION_3)
+ {
+ if (hasNormals)

hasNormals is enough condition, you can omit checking format here.

Please put definition of BinTools_ShapeSet::THE_CURRENT_VERSION into dedicated "public:" section.

Include of Draw_SaveAndRestore.hxx is extra in DrawTrSurf.hxx, DBRep.hxx.

It is better to make standard indents in the contents of classes inheriting Draw_SaveAndRestore (DrawTrSurf.cxx, DBRep.cxx, HLRTest.cxx)

Move the definition of inline method Storage_HeaderData::SetStorageVersion from cxx to hxx. The definition of inline method must be accessible via the header. And the word Standard_EXPORT must not be given in its declaration. The word inline is extra when the definition is included in the class.

+// Copyright (c) 2002-2020 OPEN CASCADE SAS

Remove 2002.

+ //! the file on reading calls Check() method.

"on reading" is the beginning of a new sentence.
36Commenter 36
Test results are not relevant to this branch. You have tested master-master configuration.
37Author
> Current logic is correct because...
This notice should be put into the code (e.g. alongside "edge triangulation does not need normals" comment).

+
+enum BinLDrivers_FormatVersion
+{

It is desired having also description of enumeration itself, not only of its values.

+#include <Standard_Typedef.hxx>
+  //! two formats available for the moment:
+ 
+
+enum BinTools_FormatVersion

Broken formatting.

+#endif
+
+
+
+

Please remove redundant empty lines - only one empty line at end of file is expected.

--- /dev/null
+++ b/src/BinTools/BinTools_FormatVersion.hxx
@@ -0,0 +1,38 @@
+// Copyright (c) 2002-2020 OPEN CASCADE SAS
...
--- /dev/null
+++ b/src/TopTools/TopTools_FormatVersion.hxx
@@ -0,0 +1,32 @@
+// Copyright (c) 2002-2020 OPEN CASCADE SAS

It doesn't look like these files were created in '2002.

+  -version versnumber: a number of format version to save

Listing versions range and default version in command help would be helpful.

+inline void Storage_HeaderData::SetStorageVersion(const Standard_Integer aVersion)

It doesn't look like exported method could be inlined.
38Author
It seems that test case has been lost for this patch.
39Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Fri Sep 25 15:06:30 2020 +0300

    review fix

40Commenter 40
The another problems indicated by msv & kgv have been fixed.
Tests task started and now is in progress.
41Commenter 41
+ // Bin(Brep)Tools_ShapeSet::AddGeometry() call from Bin(Brep)Tools_ShapeSet::Add()

is called from

+ // that recursively down from complex to elementary shapes.

that processes shapes recursively from complex to elementary ones

+ // As a result, the TopAbs_FACE’s will be processed earlier than the TopAbs_EDGE’s.

Avoid using non-ascii characters.

+puts "=========="
+puts "31136_1"
+puts "=========="
+puts ""
+################################################################
+# Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
+################################################################

Move bug title from comments to the puts statement to leave it in the test log, like
puts "0031136: BinXCAF persistence loses normals from triangulation-only Faces"
42Commenter 42
+checkview -screenshot -3d -path ${imagedir}/${test_image_brep}.png
+vclear
+vclose ALL

Do not clear the view after test finish. It is useful to see the view after a test during interactive session.
43Commenter 43
+vinit v1/v1
...
+vinit v2/v2
...
+vinit v3/v3

Why don't you use the same v1 view name for each test?
44Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Fri Sep 25 16:38:54 2020 +0300

    # review fix 2

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Fri Sep 25 17:08:42 2020 +0300

    # fix 3

46Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Fri Sep 25 16:38:54 2020 +0300

    # fix 2

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands

Author: asuraven
Date: Fri Sep 25 15:06:30 2020 +0300

    review fix
48Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands
50Commenter 50
+puts "31136_1"
...
+puts "31136_2"
...
+puts "31136_1"

Please replace these lines with
puts "0031136: BinXCAF persistence loses normals from triangulation-only Faces"
51Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands
53Commenter 2
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Wed Oct 28 14:54:45 2020 +0300

    fix Draw_SaveAndRestoreBase

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Thu Oct 29 11:45:08 2020 +0300

    temporary wavefront log

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Fri Oct 30 16:05:24 2020 +0300

    readobj log

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Mon Nov 2 12:50:20 2020 +0300

    add test log

57Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Mon Nov 2 12:50:20 2020 +0300

    add test log

Author: asuraven
Date: Fri Oct 30 16:05:24 2020 +0300

    readobj log

Author: asuraven
Date: Thu Oct 29 11:45:08 2020 +0300

    temporary wavefront log

Author: asuraven
Date: Wed Oct 28 14:54:45 2020 +0300

    fix Draw_SaveAndRestoreBase

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands
59Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands
    * Change file path variable in wavefront function from char[100] to TCollection_AsciiString to prevent long file path problem
61Author
 * Change file path variable in wavefront function from char[100] to TCollection_AsciiString to prevent long file path problem

This is unrelated to the bug - please register a dedicated issue and push related changes there.
62Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands
    * Change file path variable in wavefront function from char[100] to TCollection_AsciiString to prevent long file path problem
64Commenter 64
65Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands
    * Change file path variable in wavefront function from char[100] to TCollection_AsciiString to prevent long file path problem
67Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands
69Commenter 69
kgv:
This is unrelated to the bug - please register a dedicated issue and push related changes there.

Kirill, the path variable lenght error prevented the bug31136 tests from running on Jenkins for Windows. We can't close 31136 issue without this change. I suggest not to separate this into a separate task to speed up the work
70Author
> I suggest not to separate this into a separate task to speed up the work
Andrey, registering a trivial bugfix in scope of another issue should not slow down integration process.
In fact, many portions of your patch could be already in master if they would be put into separate issues.

71Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands
73Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Thu Sep 17 14:16:44 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands

Author: asuraven
Date: Tue Nov 3 17:22:14 2020 +0300

    0031908: Draw Harness, TKTopTest - wavefront file path length is limited to 100 characters
    
    * Change file path variable in wavefront function from char[100] to TCollection_AsciiString
75Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Tue Nov 3 17:22:14 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands
77Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Tue Nov 3 17:22:14 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands
79Commenter 79
For integration:
occt - CR31136_4
products - none
80Commenter 2
Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Nov 12 11:24:16 2020 +0300

    # fix compatibility with old compilers

81Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Tue Nov 3 17:22:14 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    * Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    * Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)
    * Add new optional -version parameter for save/binsave draw commands

Author: asuraven
Date: Tue Nov 3 17:22:14 2020 +0300

    0031908: Draw Harness, TKTopTest - wavefront file path length is limited to 100 characters
    
    * Change file path variable in wavefront function from char[100] to TCollection_AsciiString
82Author
Andrey,

> * Add new optional -version parameter for save/binsave draw commands
I can see that commands save/binsave have been extended with version parameter.
But cannot find anything related to saving into XBF document (XSave command, test cases checking that saving into old/new version works as expected)...

How it is supposed to save an XBF document compatible with old OCCT?
Without such option (also available in Draw Harness) the patch cannot be integrated to OCCT.
83Commenter 83
Kirill, is it really necessary to have ability to save in old format? Up to now, each update of version of OCAF persistence was done without such compatibility.
84Commenter 84
Compatibility is important, and we need to understand clearly what possible implications on compatibility this change can produce.

Andrey, can you please describe the logic of using different versions of the format?

I see that currently plain "save" of the box shape generates BREP file with version V1. This means that V2 is generated in some specific cases, is it? How V3 will be applied?
85Author
Mikhail,

format version bumps always caused compatibility issues and headache to OCCT-based applications in the past.
Although we have ignored such a problem in the past, it is not necessary to be done in the same in future.

In this case XBF format changes are caused not by fundamental OCAF changes but rather by local tool which maintains different versions anyway.

Moreover, current modification gives a small value to majority of applications, as most scenarios define exact geometry, which can be written lossless in previous version of format.

Thus, in my opinion we should give application a way for writing XBF file with geometry in previous version of format for compatibility with older OCCT releases (in fact, this might be even better making a DEFAULT, until we integrate some other breaking compatibility improvements to XBF or OCAF).

We should also check how old OCCT versions read new XBF files to ensure that user will see a meaningful message, and might need making some provisional changes for better reporting / clearer versioning in XBF format. As far as I understand, for now XBF format does not define any version at all - there is only a general OCAF version and B-Rep format version.

86Commenter 86
> How it is supposed to save an XBF document compatible with old OCCT?
Kirill, a "-version" option now added only for save/binsave commands.
In which other DRAW commands do I need to add this option? Save, SaveAs, XSave?
Should I do it as part of this issue or create a new one?

> Andrey, can you please describe the logic of using different versions of the format?
> I see that currently plain "save" of the box shape generates BREP file with version V1.
> This means that V2 is generated in some specific cases, is it? How V3 will be applied?
Andrey, I could not find out why V2 was created. This change was made a long time ago (about 20 years):
http://svn.nnov.opencascade.com/websvn/revision.php?repname=occt&path=%2Ftrunk%2Fsrc%2FBRepTools%2FBRepTools_ShapeSet.cxx&rev=5953&peg=8617
In this issue V3 format includes changes of V2 version. If it is wrong, what behavior should I implement?
87Commenter 2
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Tue Nov 17 20:37:01 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    Versions of formats have been changed (11 for BinOCAF, 10 for XmlOCAF, 4 for BRep Shape and 3 for Binary BRep Shape)

Author: asuraven
Date: Tue Nov 3 17:22:14 2020 +0300

    0031946: Modeling Data - replace version numbers with enumerations in TopTools and BinTools
    
    Added enumerations BinLDrivers_FormatVersion, BinTools_FormatVersion, TopTools_FormatVersion
    and XmlLDrivers_FormatVersion for more clear version tracking in the code.
    
    Add new optional -version parameter for save/binsave draw commands.
88Author
Andrey (Commenter 3), I have moved the patch adding version enumerations and extending "save" and "binsave" commands with -version parameter to dedicated patch #0031136. Version enumerations have been corrected to start with enum name, confusing "DEFAULT" items equal to "0" have been removed.

Apparently, patch for "binsave" is incomplete or has been broken at some step - version parameter is parsed but ignored. Could you please finalize that patch first?
89Commenter 2
Branch [archived branch] has been updated forcibly by Author.

[revision removed]
90Commenter 2
Branch [archived branch] has been updated forcibly by Author.

[revision removed]
91Author
Andrey (Commenter 3),

> In which other DRAW commands do I need to add this option? Save, SaveAs, XSave?
The interface for setting format version to XML persistence has been introduced by #0028691.
Note that it looks like your current patch breaks this feature as XML driver sets shape format to new version unconditionally (I've put a TODO there in latest branch).

There is a command SetStorageVersion, but currently it operates only with XmlLDrivers, which doesn't look provisional - see also remarks to this interface within bug #0029217, which are desired to be taken into account to handle also BinLDrivers.

There is no need to support writing any version of BinLDrivers, so far backward-compatible export efforts could be started with this format version bump. It is necessary to handle this in scope of this bug (some preparations might be moved to another prerequisite patch, if found useful).

92Commenter 92
I propose we shall first elaborate and describe some consistent approach to versioning of data formats.

- what versions of formats are known for each format, how they correlate with OCCT versions?
- how they are combined for composite data formats (e.g. I guess that BREP V2 is used in XML persistence only, in some recent versions -- is that?)
- are different format versions forward and / or backward compatible, to what extent?
- how they can be managed in an understandable way?
93Commenter 2
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Tue Nov 17 20:37:01 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    Versions of formats have been changed (11 for TDocStd, 4 for BRep Shape and 3 for Binary BRep Shape)
94Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Tue Nov 17 20:37:01 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    Versions of formats have been changed (11 for TDocStd, 4 for BRep Shape and 3 for Binary BRep Shape)
96Commenter 96
97Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Tue Nov 17 20:37:01 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    Versions of formats have been changed (11 for TDocStd, 4 for BRep Shape and 3 for Binary BRep Shape)
99Author
Please proceed remarks.

- Tools should provide an option writing Normals even when they can be restored from surface (as performance optimization).
- BinXCaf/XmlXCaf should write shapes in new persistence formats (TopTools_FormatVersion/BinTools_FormatVersion) only for new TDocStd_FormatVersion versions.
- Command "StoreTriangulation" should be also extended to propagate new withNormals option to BinXCaf/XmlXCaf drivers with new test case.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Mon Jan 11 13:14:51 2021 +0300

    * BRepTools::Write() theWithNormals parameter added
    * BRepTools_ShapeSet:
      - constructor theWithNormals parameter added
      - IsWithNormals()/SetWithNormals() function added
    * BinLDrivers_DocumentSection: theDocFormatVersion parameter added to WriteTOC()/Write()/ReadTOC() functions
    * BinLDrivers_DocumentStorageDriver::WriteShapeSection() theDocVer parameter added
    * BinMNaming_NamedShapeDriver::WriteShapeSection() theDocVer parameter added
    * BinTools_ShapeSet/TopTools_ShapeSet THE_ASCII_VERSIONS arrays created
    * BinTools_FormatVersion/TopTools_FormatVersion/TDocStd_FormatVersion LOWER/UPPER enums added

101Commenter 2
Branch [archived branch] has been deleted by Participant.

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

[revision removed]


Detailed log of new commits:

Author: asuraven
Date: Mon Jan 11 13:14:51 2021 +0300

    Additional changes:
    
    * BRepTools::Write() theWithNormals parameter added
    * BRepTools_ShapeSet:
      - constructor theWithNormals parameter added
      - IsWithNormals()/SetWithNormals() function added
    * BinLDrivers_DocumentSection: theDocFormatVersion parameter added to WriteTOC()/Write()/ReadTOC() functions
    * BinLDrivers_DocumentStorageDriver::WriteShapeSection() theDocVer parameter added
    * BinMNaming_NamedShapeDriver::WriteShapeSection() theDocVer parameter added
    * BinTools_ShapeSet/TopTools_ShapeSet THE_ASCII_VERSIONS arrays created
    * BinTools_FormatVersion/TopTools_FormatVersion/TDocStd_FormatVersion LOWER/UPPER enums added

Author: asuraven
Date: Tue Nov 17 20:37:01 2020 +0300

    0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
    
    Information about normals are stored in BinOCAF, XmlOCAF, BRep and BBRep (in case of triangulation-only Faces).
    Versions of formats have been changed (11 for TDocStd, 4 for BRep Shape and 3 for Binary BRep Shape)
103Commenter 103
Test results for cumulative branch CR31136_8
 http://jenkins-test-12.nnov.opencascade.com/view/CR31136_8-master-ASURAVEN/view/COMPARE/
You can see also sequential new commints in branch CR31136_7
104Author
-  myFormatNb (BinTools_FormatVersion_VERSION_1)
..
-  myShapeSet.SetFormatNb(myFormatNb);
+  if (theDocVer >= TDocStd_FormatVersion_VERSION_11)
+  {
+    myShapeSet.SetFormatNb(BinTools_FormatVersion_VERSION_4);

Please remove no more used class field BinMNaming_NamedShapeDriver::myFormatNb.

-  \<version\> = ("CASCADE Topology V1, (c)  Matra-Datavision" | "CASCADE Topology V2, (c) Matra-Datavision")\<_\\n\>;
+  \<version\> = ("CASCADE Topology V1, (c)  Matra-Datavision" | "CASCADE Topology V2, (c) Matra-Datavision" | "Open CASCADE Topology V3 (c)")\<_\\n\>;

Please update documentation to match source code.

-BinTools_ShapeSet::BinTools_ShapeSet (const Standard_Boolean theWithTriangles)
+BinTools_ShapeSet::BinTools_ShapeSet (const Standard_Boolean theWithTriangles,
+                                      const Standard_Boolean theWithNormals)

There is no much use in extending this class constructor, I propose keeping it the same (consider that we will add 10 more flags to the class in future, which would make class constructor argument list unreadable).
Calling BinTools_ShapeSet::SetWithNormals() and BinTools_ShapeSet::SetWithTriangles() will make code cleaner - just need to document default values of these properties in empty constructor.

+                                       //!< * BIN, XML: TopTools_FormatVersion_CURRENT changed 3 and 

...changed _to_ 3...

+                                       //!< BinTools_FormatVersion_CURRENT changed to 4 to Stores per-vertex normal 

...to preserver per-vertex normal ...

+  "Open CASCADE Topology V4, (c) Open CASCADE"

(c) Open Cascade
105Commenter 105
106Author
Please raise the patch
- OCCT branch: [archived branch]
107Commenter 1
Combination -
OCCT branch : [archived branch]
master SHA - [revision removed]
[revision removed]
Products branch : [archived branch] SHA - [revision removed]
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17720.020000000055 / 17721.200000000135 [-0.01%]
Products
Total CPU difference: 12428.380000000123 / 12412.630000000105 [+0.13%]
Windows-64-VC14:
OCCT
Total CPU difference: 19367.234375 / 19300.265625 [+0.35%]
Products
Total CPU difference: 13821.875 / 13740.03125 [+0.60%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

Related records