MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0031136Open CASCADE[OCCT] OCCT:Modeling Datapublic2019-11-06 18:512020-09-27 15:39
Reporterkgv 
Assigned Toasuraven 
PrioritynormalSeverityminor 
StatusassignedResolutionopen 
PlatformOSOS Version
Product Version[OCCT] 6.3.1 
Target Version[OCCT] 7.5.0Fixed in Version 
Summary0031136: Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
DescriptionBinXCAF 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
TagsNo tags attached.
Test case number
Attached Filespng file icon cad_ref_OK.png (35,177 bytes) 2019-11-06 18:52
png file icon cad_xcaf_KO.png (37,223 bytes) 2019-11-06 18:52

- Relationships
related to 0031137verifiedbugmaster Modeling Data, BinTools_ShapeSet - avoid allocation of temporary arrays 
related to 0027835closedbugmaster Application Framework, BinXCAF - handle correctly faces with NULL surface within BinTools_ShapeSet 
related to 0031382newgka 0031136: Modeling Data - BinXCAF should preserve length unit information 
child of 0028125newmsv Modeling Algorithms - support of BRep shapes based on tessellated geometry 

-  Notes
(0088756)
git (administrator)
2019-11-06 19:27

Branch CR31136 has been created by kgv.

SHA-1: b9e59ab73fd56c77484a2e80cde9df77d01fe97a


Detailed log of new commits:

Author: kgv
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.
(0090439)
git (administrator)
2020-02-10 10:21

Branch CR31136 has been deleted by kgv.

SHA-1: b9e59ab73fd56c77484a2e80cde9df77d01fe97a
(0090800)
git (administrator)
2020-03-03 16:52

Branch CR31136 has been created by asuraven.

SHA-1: bee13ef1cf7c244bcdf0592f42cf2b5e124c95fd


No new revisions were added by this update.
(0090929)
git (administrator)
2020-03-13 15:45

Branch CR31136 has been updated by asuraven.

SHA-1: 3f104410c39099cdda90d0f8dbaa7c7c333a3ae9


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

(0090946)
kgv (developer)
2020-03-15 11:17

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

olny
(0090970)
asuraven (developer)
2020-03-16 15:13

http://jenkins-test-12.nnov.opencascade.com/view/CR31136-master-ASURAVEN/view/COMPARE/ [^]
Pleace review
(0090972)
asuraven (developer)
2020-03-16 16:07

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
(0091046)
kgv (developer)
2020-03-19 11:36

+enum class BinLDrivers_FormatVersion : Standard_Integer

OCCT has to be compilable by some old compilers not supporting enum class.
(0093173)
msv (developer)
2020-07-17 14:52

Andrey, could you rebase on new master and run tests again?
(0093895)
git (administrator)
2020-09-02 16:50

Branch CR31136 has been updated by asuraven.

SHA-1: e2443485f250d1366a612fa05407e2d1303e490a


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

(0094198)
git (administrator)
2020-09-07 15:19

Branch CR31136 has been updated by asuraven.

SHA-1: 02c283b97313cd37743a4f4719d8eaa94206f291


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

(0094382)
asuraven (developer)
2020-09-10 15:32

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
(0094384)
kgv (developer)
2020-09-10 15:35

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")?
(0094465)
git (administrator)
2020-09-11 15:28

Branch CR31136_1 has been created by asuraven.

SHA-1: a867355764a752dca4b53024af20edb1d521d84c


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)
(0094469)
asuraven (developer)
2020-09-11 15:52

emun class consructions excluded.
grammatical error fixed.
All changes are merged into one 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
(0094768)
msv (developer)
2020-09-14 18:06

Andrey, please re-base on new master, there are conflicts.
And re-test.
(0094819)
git (administrator)
2020-09-15 12:05

Branch CR31136_2 has been created by asuraven.

SHA-1: eece068da44cc796adb6c3d8f6c66ca884d55b71


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)
(0094841)
asuraven (developer)
2020-09-15 15:18

New branch CR31136_2 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
(0094923)
msv (developer)
2020-09-16 15:31

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.
(0094925)
git (administrator)
2020-09-16 15:33

Branch CR31136_2 has been updated by msv.

SHA-1: ea174cd1593a6b1c562a19024e206fb3c353b275


Detailed log of new commits:

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

    #simplify text in upgrade guide

(0094933)
git (administrator)
2020-09-16 17:16

Branch CR31136_2 has been updated by asuraven.

SHA-1: dacc123d84a20f633d043f7eec8598dd458f5960


Detailed log of new commits:

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

    rollback refactoring

(0094960)
git (administrator)
2020-09-17 14:13

Branch CR31136_3 has been created by asuraven.

SHA-1: 2aba087137b0bb7b8730c476ad64d82a87a585c3


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
(0094961)
asuraven (developer)
2020-09-17 14:28

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/ [^]
(0094966)
kgv (developer)
2020-09-17 15:08
edited on: 2020-09-17 15:09

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.

(0095051)
msv (developer)
2020-09-18 19:45

- 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.
(0095165)
git (administrator)
2020-09-20 20:00

Branch CR31136_3 has been updated by asuraven.

SHA-1: 4755e78f3753641e2e38627bef40725869e5676c


Detailed log of new commits:

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

    fix formatting

(0095189)
git (administrator)
2020-09-21 16:05

Branch CR31136_3 has been updated by asuraven.

SHA-1: 4d7c3a3934db011bc5f2928a1af7c4966c117f9e


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

(0095198)
git (administrator)
2020-09-22 13:12

Branch CR31136_3 has been deleted by asuraven.

SHA-1: 4d7c3a3934db011bc5f2928a1af7c4966c117f9e
(0095199)
git (administrator)
2020-09-22 13:12

Branch CR31136_3 has been created by asuraven.

SHA-1: ad95ce6d8e322157db5716cb27aff330b7526193


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
(0095207)
git (administrator)
2020-09-22 17:25

Branch CR31136_3 has been updated by asuraven.

SHA-1: d1b8e2a83665bab5b0caa2130be4db723c3db960


Detailed log of new commits:

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

    fix

(0095208)
git (administrator)
2020-09-22 17:32

Branch CR31136_4 has been created by asuraven.

SHA-1: c9333c9d74e7a79e8469acadf8856aaf1c68d564


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
(0095209)
git (administrator)
2020-09-22 17:35

Branch CR31136_3 has been deleted by asuraven.

SHA-1: d1b8e2a83665bab5b0caa2130be4db723c3db960
(0095210)
git (administrator)
2020-09-22 17:36

Branch CR31136_3 has been created by asuraven.

SHA-1: a7519080d0f0d899316a7ed32fb8468d320ca722


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
(0095211)
asuraven (developer)
2020-09-22 17:38

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/ [^]
(0095212)
msv (developer)
2020-09-22 19:17

+ 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.
(0095213)
msv (developer)
2020-09-22 19:20

Test results are not relevant to this branch. You have tested master-master configuration.
(0095214)
kgv (developer)
2020-09-22 19:22

> 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.
(0095215)
kgv (developer)
2020-09-22 19:23

It seems that test case has been lost for this patch.
(0095322)
git (administrator)
2020-09-25 15:09

Branch CR31136_4 has been updated by asuraven.

SHA-1: 7867e0baab97b4252bc6de3a284f75e5ce702e50


Detailed log of new commits:

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

    review fix

(0095325)
asuraven (developer)
2020-09-25 15:25

The another problems indicated by msv & kgv have been fixed.
Tests task started and now is in progress.
(0095328)
msv (developer)
2020-09-25 16:00

+ // 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"
(0095330)
msv (developer)
2020-09-25 16:05

+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.
(0095331)
msv (developer)
2020-09-25 16:15

+vinit v1/v1
...
+vinit v2/v2
...
+vinit v3/v3

Why don't you use the same v1 view name for each test?
(0095333)
git (administrator)
2020-09-25 16:36

Branch CR31136_4 has been updated by asuraven.

SHA-1: 285bd4d8147f9189f32f511aafae88d1473fa50f


Detailed log of new commits:

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

    # review fix 2

(0095334)
git (administrator)
2020-09-25 17:05

Branch CR31136_4 has been updated by asuraven.

SHA-1: 72e2b12951806f3f4d2fdd59fd58e0b3e65244d5


Detailed log of new commits:

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

    # fix 3

(0095424)
git (administrator)
2020-09-27 15:39

Branch CR31136_4 has been deleted by asuraven.

SHA-1: 72e2b12951806f3f4d2fdd59fd58e0b3e65244d5
(0095425)
git (administrator)
2020-09-27 15:39

Branch CR31136_4 has been created by asuraven.

SHA-1: 2b2ac9ceb1f2926654fbe66632241a21f7d18ea7


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

- Issue History
Date Modified Username Field Change
2019-11-06 18:51 kgv New Issue
2019-11-06 18:51 kgv Assigned To => gka
2019-11-06 18:52 kgv File Added: cad_ref_OK.png
2019-11-06 18:52 kgv File Added: cad_xcaf_KO.png
2019-11-06 18:53 kgv Assigned To gka => kgv
2019-11-06 18:53 kgv Category OCCT:Data Exchange => OCCT:Modeling Data
2019-11-06 18:53 kgv Product Version 7.3.0 => 6.3.1
2019-11-06 18:53 kgv Summary Data Exchange - BinXCAF persistence loses normals from triangulation-only Faces => Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces
2019-11-06 19:27 git Note Added: 0088756
2019-11-07 11:25 kgv Relationship added related to 0031137
2019-11-07 11:26 kgv Relationship added related to 0027835
2019-11-07 11:26 kgv Relationship added child of 0028125
2019-11-07 11:30 kgv Steps to Reproduce Updated View Revisions
2020-02-10 10:21 git Note Added: 0090439
2020-02-10 15:31 asuraven Assigned To kgv => asuraven
2020-02-18 17:30 kgv Relationship added related to 0031382
2020-03-03 16:52 git Note Added: 0090800
2020-03-04 12:35 szy Status new => assigned
2020-03-13 15:45 git Note Added: 0090929
2020-03-15 11:17 kgv Note Added: 0090946
2020-03-16 15:13 asuraven Note Added: 0090970
2020-03-16 15:13 asuraven Assigned To asuraven => msv
2020-03-16 15:13 asuraven Status assigned => resolved
2020-03-16 16:07 asuraven Note Added: 0090972
2020-03-19 11:36 kgv Note Added: 0091046
2020-07-17 14:52 msv Note Added: 0093173
2020-07-17 14:52 msv Assigned To msv => asuraven
2020-07-17 14:52 msv Status resolved => assigned
2020-09-02 16:50 git Note Added: 0093895
2020-09-07 15:19 git Note Added: 0094198
2020-09-10 15:31 asuraven Assigned To asuraven => msv
2020-09-10 15:32 asuraven Note Added: 0094382
2020-09-10 15:32 asuraven Status assigned => resolved
2020-09-10 15:35 kgv Note Added: 0094384
2020-09-10 15:35 kgv Assigned To msv => asuraven
2020-09-10 15:35 kgv Status resolved => assigned
2020-09-11 15:28 git Note Added: 0094465
2020-09-11 15:52 asuraven Note Added: 0094469
2020-09-11 15:53 asuraven Assigned To asuraven => msv
2020-09-11 15:53 asuraven Status assigned => resolved
2020-09-14 18:06 msv Note Added: 0094768
2020-09-14 18:06 msv Assigned To msv => asuraven
2020-09-14 18:06 msv Status resolved => assigned
2020-09-15 12:05 git Note Added: 0094819
2020-09-15 15:18 asuraven Note Added: 0094841
2020-09-15 15:18 asuraven Assigned To asuraven => msv
2020-09-15 15:18 asuraven Status assigned => resolved
2020-09-16 15:31 msv Note Added: 0094923
2020-09-16 15:33 git Note Added: 0094925
2020-09-16 15:38 msv Assigned To msv => asuraven
2020-09-16 15:38 msv Status resolved => assigned
2020-09-16 17:16 git Note Added: 0094933
2020-09-17 14:13 git Note Added: 0094960
2020-09-17 14:28 asuraven Note Added: 0094961
2020-09-17 14:28 asuraven Assigned To asuraven => msv
2020-09-17 14:28 asuraven Status assigned => resolved
2020-09-17 15:08 kgv Note Added: 0094966
2020-09-17 15:09 kgv Note Edited: 0094966 View Revisions
2020-09-17 20:42 kgv Assigned To msv => asuraven
2020-09-17 20:42 kgv Status resolved => assigned
2020-09-18 19:45 msv Note Added: 0095051
2020-09-20 20:00 git Note Added: 0095165
2020-09-21 16:05 git Note Added: 0095189
2020-09-22 13:12 git Note Added: 0095198
2020-09-22 13:12 git Note Added: 0095199
2020-09-22 17:25 git Note Added: 0095207
2020-09-22 17:32 git Note Added: 0095208
2020-09-22 17:35 git Note Added: 0095209
2020-09-22 17:36 git Note Added: 0095210
2020-09-22 17:38 asuraven Note Added: 0095211
2020-09-22 17:38 asuraven Assigned To asuraven => msv
2020-09-22 17:38 asuraven Status assigned => resolved
2020-09-22 19:17 msv Note Added: 0095212
2020-09-22 19:17 msv Assigned To msv => asuraven
2020-09-22 19:17 msv Status resolved => assigned
2020-09-22 19:20 msv Note Added: 0095213
2020-09-22 19:22 kgv Note Added: 0095214
2020-09-22 19:23 kgv Note Added: 0095215
2020-09-25 15:09 git Note Added: 0095322
2020-09-25 15:25 asuraven Note Added: 0095325
2020-09-25 15:25 asuraven Assigned To asuraven => msv
2020-09-25 15:25 asuraven Status assigned => resolved
2020-09-25 16:00 msv Note Added: 0095328
2020-09-25 16:05 msv Note Added: 0095330
2020-09-25 16:15 msv Note Added: 0095331
2020-09-25 16:20 msv Assigned To msv => asuraven
2020-09-25 16:20 msv Status resolved => assigned
2020-09-25 16:36 git Note Added: 0095333
2020-09-25 17:05 git Note Added: 0095334
2020-09-27 15:39 git Note Added: 0095424
2020-09-27 15:39 git Note Added: 0095425


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker