MantisBT - Open CASCADE
View Issue Details
0031382Open CASCADE[OCCT] OCCT:Data Exchangepublic2020-02-18 17:302021-09-09 19:55
kgv 
bugmaster 
normalfeature 
verifiedfixed 
 
[OCCT] 7.6.0* 
bugs/xde/bug31382
0031382: Data Exchange - BinXCAF should preserve length unit information
Currently, XCAF document (XBF) is loaded / written without length unit conversions, so that document implicitly preserves system units.
As different OCCT-based applications may choose different system units, opening the documents across such applications becomes problematic as there are no related information preserved in the file no standard tools for units conversion of XCAF document content.

As minimal requirement, it is necessary preserving length unit information within XBF/Xml file, which should be set and processed by unit-aware applications (by default, it might be reasonable keeping length unit UNDEFINED).
bug31382
No tags attached.
related to 0031136verified bugmaster Open CASCADE Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces 
related to 0032452new gka Community IGES reader returns invalid shapes if xstep.cascade.unit property is changed from default 
related to 0032512new gka Community DXF Import - OCC is not reading $INSUNITS variable from DXF file 
related to 0032514new gka Community DXF Export - OCC export DXF file without $INSUNITS variable 
Issue History
2020-02-18 17:30kgvNew Issue
2020-02-18 17:30kgvAssigned To => gka
2020-02-18 17:30kgvRelationship addedrelated to 0031136
2020-09-11 17:35gkaTarget Version7.5.0 => 7.6.0*
2020-10-01 10:32kgvSummary0031136: Modeling Data - BinXCAF should preserve length unit information => Modeling Data - BinXCAF should preserve length unit information
2020-10-01 11:35kgvSummaryModeling Data - BinXCAF should preserve length unit information => Data Exchange - BinXCAF should preserve length unit information
2020-10-29 09:45szyAssigned Togka => skl
2020-10-29 09:45szyStatusnew => assigned
2020-11-10 07:53gitNote Added: 0096632
2020-11-10 07:55gitNote Added: 0096633
2020-11-10 08:41gitNote Added: 0096634
2020-11-10 14:36gitNote Added: 0096638
2020-11-11 10:19gitNote Added: 0096652
2020-11-11 15:54sklNote Added: 0096659
2020-11-11 15:54sklAssigned Toskl => mpv
2020-11-11 15:54sklStatusassigned => resolved
2020-11-11 15:54sklSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=23971#r23971
2020-11-11 17:01msvNote Added: 0096662
2020-11-11 17:05msvNote Added: 0096664
2020-11-11 17:11kgvNote Added: 0096666
2020-11-11 17:13kgvNote Edited: 0096666bug_revision_view_page.php?bugnote_id=96666#r23973
2020-11-11 17:13kgvNote Edited: 0096666bug_revision_view_page.php?bugnote_id=96666#r23974
2020-11-11 17:26msvNote Added: 0096668
2020-11-11 17:28mpvNote Added: 0096669
2020-11-11 17:28mpvAssigned Tompv => skl
2020-11-11 17:28mpvStatusresolved => assigned
2020-11-12 11:13gitNote Added: 0096690
2020-11-12 18:10sklNote Added: 0096726
2020-11-12 18:10sklAssigned Toskl => mpv
2020-11-12 18:10sklStatusassigned => resolved
2020-11-12 19:16kgvNote Added: 0096730
2020-11-13 17:50mpvAssigned Tompv => skl
2020-11-13 17:50mpvStatusresolved => assigned
2020-11-15 07:18gitNote Added: 0096778
2020-11-16 12:08gitNote Added: 0096788
2020-11-17 10:42gitNote Added: 0096818
2020-11-17 15:04sklNote Added: 0096828
2020-11-17 15:04sklAssigned Toskl => msv
2020-11-17 15:04sklStatusassigned => resolved
2020-11-17 15:10kgvNote Added: 0096829
2020-11-17 15:10kgvNote Edited: 0096829bug_revision_view_page.php?bugnote_id=96829#r24022
2020-11-17 15:55msvNote Added: 0096831
2020-11-17 15:55msvAssigned Tomsv => skl
2020-11-17 15:55msvStatusresolved => assigned
2020-11-17 16:07gitNote Added: 0096833
2020-11-18 07:02gitNote Added: 0096858
2020-11-18 11:34sklNote Added: 0096873
2020-11-18 11:34sklAssigned Toskl => msv
2020-11-18 11:34sklStatusassigned => resolved
2020-11-18 12:17msvNote Added: 0096878
2020-11-18 12:17msvAssigned Tomsv => skl
2020-11-18 12:17msvStatusresolved => assigned
2020-11-18 12:28gitNote Added: 0096880
2020-11-18 12:29sklAssigned Toskl => msv
2020-11-18 12:29sklStatusassigned => resolved
2020-11-18 12:32msvNote Added: 0096881
2020-11-18 12:32msvAssigned Tomsv => bugmaster
2020-11-18 12:32msvStatusresolved => reviewed
2020-11-19 10:56kgvNote Added: 0096891
2020-11-19 10:56kgvAssigned Tobugmaster => skl
2020-11-19 10:56kgvStatusreviewed => assigned
2020-11-19 10:59kgvNote Edited: 0096891bug_revision_view_page.php?bugnote_id=96891#r24037
2020-11-19 11:02kgvNote Edited: 0096891bug_revision_view_page.php?bugnote_id=96891#r24038
2020-11-19 15:11sklNote Added: 0096900
2020-11-19 15:11sklAssigned Toskl => kgv
2020-11-19 15:11sklStatusassigned => feedback
2020-11-20 10:00kgvRelationship addedrelated to 0031950
2020-11-24 15:27gitNote Added: 0097032
2020-12-10 10:33kgvNote Added: 0097453
2020-12-24 16:30gitNote Added: 0097891
2020-12-25 16:47gitNote Added: 0097906
2020-12-25 17:26gitNote Added: 0097907
2020-12-28 12:58sklAssigned Tokgv => skl
2020-12-28 12:58sklStatusfeedback => assigned
2020-12-28 13:03gitNote Added: 0097958
2020-12-28 16:52gitNote Added: 0097962
2020-12-28 18:59gitNote Added: 0097964
2020-12-29 09:25gitNote Added: 0097969
2020-12-30 11:00gitNote Added: 0097985
2020-12-31 10:18sklNote Added: 0097995
2020-12-31 10:18sklAssigned Toskl => msv
2020-12-31 10:18sklStatusassigned => resolved
2020-12-31 11:49msvNote Added: 0097997
2020-12-31 11:50msvAssigned Tomsv => skl
2020-12-31 11:50msvStatusresolved => assigned
2020-12-31 16:15gitNote Added: 0098000
2021-01-11 12:05sklNote Added: 0098050
2021-01-11 12:05sklAssigned Toskl => msv
2021-01-11 12:05sklStatusassigned => resolved
2021-01-11 14:11msvNote Added: 0098061
2021-01-11 14:12msvAssigned Tomsv => skl
2021-01-11 14:12msvStatusresolved => assigned
2021-01-11 14:23msvNote Added: 0098065
2021-01-11 14:25msvNote Deleted: 0098065
2021-01-11 14:26msvNote Added: 0098066
2021-01-11 14:28msvNote Added: 0098067
2021-01-11 15:19gitNote Added: 0098072
2021-01-11 15:22gitNote Added: 0098073
2021-01-12 10:04sklNote Added: 0098083
2021-01-12 10:04sklAssigned Toskl => msv
2021-01-12 10:04sklStatusassigned => resolved
2021-01-12 11:16kgvNote Added: 0098085
2021-01-12 11:24msvNote Added: 0098086
2021-01-12 11:24msvAssigned Tomsv => skl
2021-01-12 11:24msvStatusresolved => assigned
2021-01-13 12:37sklNote Added: 0098098
2021-01-13 12:37sklAssigned Toskl => msv
2021-01-13 12:37sklStatusassigned => resolved
2021-01-13 14:47msvNote Added: 0098103
2021-01-13 14:52msvNote Added: 0098104
2021-01-13 14:52msvAssigned Tomsv => bugmaster
2021-01-13 14:52msvStatusresolved => reviewed
2021-01-13 16:24kgvNote Added: 0098106
2021-01-13 16:24kgvAssigned Tobugmaster => skl
2021-01-13 16:24kgvStatusreviewed => assigned
2021-01-13 16:24kgvNote Edited: 0098106bug_revision_view_page.php?bugnote_id=98106#r24305
2021-01-13 16:25kgvNote Edited: 0098106bug_revision_view_page.php?bugnote_id=98106#r24306
2021-01-13 16:25kgvNote Edited: 0098106bug_revision_view_page.php?bugnote_id=98106#r24307
2021-01-13 16:25kgvNote Edited: 0098106bug_revision_view_page.php?bugnote_id=98106#r24308
2021-01-29 16:28kgvRelationship addedrelated to 0031156
2021-01-29 16:28kgvAssigned Toskl => dpasukhi
2021-02-09 23:26dpasukhiNote Added: 0098803
2021-02-10 23:58dpasukhiNote Added: 0098822
2021-02-11 23:47dpasukhiNote Added: 0098834
2021-02-12 23:56dpasukhiNote Added: 0098854
2021-03-27 01:58dpasukhiNote Added: 0099775
2021-03-30 23:14dpasukhiNote Added: 0099897
2021-04-16 19:58dpasukhiNote Added: 0100355
2021-05-02 23:38dpasukhiNote Added: 0100750
2021-05-09 21:50dpasukhiNote Added: 0100835
2021-05-16 23:13dpasukhiNote Added: 0101078
2021-05-17 12:04dpasukhiNote Edited: 0101078bug_revision_view_page.php?bugnote_id=101078#r25210
2021-05-20 03:23gitNote Added: 0101193
2021-05-23 23:16dpasukhiNote Added: 0101293
2021-05-24 02:28gitNote Added: 0101295
2021-05-24 16:02abvNote Added: 0101330
2021-05-25 13:45gitNote Added: 0101356
2021-05-26 13:25gitNote Added: 0101388
2021-05-26 14:44gitNote Added: 0101394
2021-05-30 23:32dpasukhiNote Added: 0101514
2021-06-03 04:19gitNote Added: 0101565
2021-06-03 12:28gitNote Added: 0101573
2021-06-06 23:45dpasukhiNote Added: 0101648
2021-06-06 23:51dpasukhiNote Edited: 0101648bug_revision_view_page.php?bugnote_id=101648#r25341
2021-06-17 17:36kgvRelationship addedrelated to 0032452
2021-06-23 12:21gitNote Added: 0102006
2021-06-24 15:31gitNote Added: 0102030
2021-06-24 15:35gitNote Added: 0102031
2021-06-27 22:59dpasukhiNote Added: 0102108
2021-08-02 02:59gitNote Added: 0102917
2021-08-03 18:13kgvRelationship addedrelated to 0032512
2021-08-03 18:13kgvRelationship addedrelated to 0032514
2021-08-12 02:57gitNote Added: 0103144
2021-08-13 19:06gitNote Added: 0103191
2021-08-13 19:14gitNote Added: 0103192
2021-08-15 23:50dpasukhiNote Added: 0103225
2021-08-18 15:08gitNote Added: 0103274
2021-08-18 21:13gitNote Added: 0103292
2021-08-18 22:45gitNote Added: 0103295
2021-08-18 23:21gitNote Added: 0103296
2021-08-19 12:34gitNote Added: 0103301
2021-08-19 14:50gitNote Added: 0103306
2021-08-19 16:20gitNote Added: 0103311
2021-08-19 17:41dpasukhiNote Added: 0103312
2021-08-19 17:41dpasukhiAssigned Todpasukhi => kgv
2021-08-19 17:41dpasukhiStatusassigned => resolved
2021-08-19 17:59kgvAssigned Tokgv => bugmaster
2021-08-19 17:59kgvStatusresolved => reviewed
2021-08-19 19:07kgvAssigned Tobugmaster => dpasukhi
2021-08-19 19:07kgvStatusreviewed => assigned
2021-08-19 22:28gitNote Added: 0103314
2021-08-20 10:00kgvAssigned Todpasukhi => bugmaster
2021-08-20 10:00kgvStatusassigned => resolved
2021-08-20 10:00kgvStatusresolved => reviewed
2021-08-20 10:06dpasukhiNote Edited: 0103312bug_revision_view_page.php?bugnote_id=103312#r25641
2021-08-20 15:47kgvNote Added: 0103319
2021-08-21 14:19bugmasterNote Added: 0103326
2021-08-21 14:19bugmasterStatusreviewed => tested
2021-08-21 14:22bugmasterChangeset attached => occt master da80ff68
2021-08-21 14:22bugmasterStatustested => verified
2021-08-21 14:22bugmasterResolutionopen => fixed
2021-08-21 14:31gitNote Added: 0103329
2021-08-21 14:31gitNote Added: 0103330
2021-08-21 14:31gitNote Added: 0103331
2021-08-21 14:31gitNote Added: 0103332
2021-08-21 14:31gitNote Added: 0103333
2021-08-21 14:31gitNote Added: 0103334
2021-08-21 14:32gitNote Added: 0103335
2021-08-21 14:32gitNote Added: 0103336
2021-08-27 16:22dpasukhiRelationship addedrelated to 0032543
2021-09-04 16:01smoskvinTest case number => pointcloud/reader/ply_units_m,move_to_origin
2021-09-04 16:16bugmasterTest case numberpointcloud/reader/ply_units_m,move_to_origin => bugs/xde/bug31382
2021-09-09 17:28dpasukhiRelationship addedparent of 0032562
2021-09-09 19:55dpasukhiRelationship addedparent of 0032563

Notes
(0096632)
git   
2020-11-10 07:53   
Branch CR31382 has been created by skl.

SHA-1: 8638eb6923dd4c515bd8d899cf3c8f233b1734e7


Detailed log of new commits:

Author: skl
Date: Tue Nov 10 07:52:30 2020 +0300

    0031382: Data Exchange - BinXCAF should preserve length unit information
    
    Add LengthUnit info to XCAF document.
(0096633)
git   
2020-11-10 07:55   
Branch CR31382 has been updated forcibly by skl.

SHA-1: 70bc6ab2cf4ecc77b2856b82f4324978a498398f
(0096634)
git   
2020-11-10 08:41   
Branch CR31382 has been updated forcibly by skl.

SHA-1: d3ebb3870574430d07f8657fc4f45114ca82f8d0
(0096638)
git   
2020-11-10 14:36   
Branch CR31382 has been updated forcibly by skl.

SHA-1: 3a4f47ea05b5faf8469cc349fc7d4fd8854effd8
(0096652)
git   
2020-11-11 10:19   
Branch CR31382 has been updated forcibly by skl.

SHA-1: 4bdf1ebadccec4c4696508d5b294fbbf7ae561ab
(0096659)
skl   
2020-11-11 15:54   
Result of tests:
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31382-master/view/COMPARE/ [^]
(0096662)
msv   
2020-11-11 17:01   
It would be helpful to add to the LengthUnitTool a method that returned scale ratio between two units.
Use case:
  XCAFDoc_LengthUnitTool::UnitValue aUnit = aLUTool->GetLengthUnit();
  double aScale = XCAFDoc_LengthUnitTool::GetScale (XCAFDoc_LengthUnitTool::UV_METER, aUnit);

GetScale (XCAFDoc_LengthUnitTool::UV_METER, XCAFDoc_LengthUnitTool::UV_MILLIMETER) is expected to return 1000.
(0096664)
msv   
2020-11-11 17:05   
Sergey, you can make the code compact (and less error prone) by separating the code that converts a string to the unit enum into a dedicated function in XDEDRAW.cxx.
(0096666)
kgv   
2020-11-11 17:11   
(edited on: 2020-11-11 17:13)
To make patch actually useful, it is desired to:
1. Update existing XCAF document (e.g. STEP, IGES, glTF, JT) filling/creation tools to set up units information, when it is known.
2. Analyze how to convert XCAF document from one unit to another (while reading or by other means). This might be really complicated considering reading of TopoDS_Shape / PMI information / etc., but feasible.

This is not necessary to be put into this patch, but before processing with integration of this step we should analyze how to go further (and potentially alter this patch to suite better further needs).

(0096668)
msv   
2020-11-11 17:26   
What is missing in this fix is storing units in standard saving procedure. Can we use the current value of the parameter xstep.cascade.unit to store the unit in the saved file? If some unit-aware application does not use this parameter it will set the units in the document explicitly, and in this case the standard procedure must not override the value of unit set in the document.
(0096669)
mpv   
2020-11-11 17:28   
To get conversion from one unit to other you may use method UnitsMethods::GetLengthFactorValue

I don't know, may be it is worth synchronize somehow enumeration with integer values used in this method. May be we could use the same enum in both UnitsMethods and XCAFDoc_lengthUnitTool?

Also in XCAFDoc_LengthUnitTool::SetLengthUnit you may remove everything except the last line, because TDataStd_Integer::Set checks the attribute already exists and changes the value. You don't need to forget the attribute just to change it.
(0096690)
git   
2020-11-12 11:13   
Branch CR31382 has been updated forcibly by skl.

SHA-1: e068e892f3134df99842e18d7881e87e2d22984a
(0096726)
skl   
2020-11-12 18:10   
Result of tests:
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31382-master/view/COMPARE/ [^]
(0096730)
kgv   
2020-11-12 19:16   
+  enum LengthUnit {

This should be moved to dedicated enum header following recommendations in OCCT Coding Rules.

+UnitsMethods::LengthUnit lengthUnitByStr(TCollection_AsciiString theStr)

static
const TCollection_AsciiString&

+  if (theStr.IsEqual("MM"))

It is preferred making comparison case-insensitive.

+  di.Add("XCheckLengthUnit",

This command makes no sense for general usage - it could be replaced by Tcl comparison check using XGetLengthUnit command.

+  di.Add("XDumpLengthUnit",

XDump* prefix is unusual for getters in Draw Harness plugins - other getters have XGet* prefix.
(0096778)
git   
2020-11-15 07:18   
Branch CR31382 has been updated forcibly by skl.

SHA-1: ab1c68a13e150ae3ec6b1ba806d092de4001f640
(0096788)
git   
2020-11-16 12:08   
Branch CR31382 has been updated forcibly by skl.

SHA-1: 763bab75800c94d58dae4db90953d1e9c98c6a5d
(0096818)
git   
2020-11-17 10:42   
Branch CR31382 has been updated forcibly by skl.

SHA-1: cfd5bf689758ece85287db5ba9c9357138906e70
(0096828)
skl   
2020-11-17 15:04   
In order to avoid situation with undefined length unit in the document special protection in the XCAFDoc_ShapeTool is added: before adding of first TopoDS_Shape to document value of length unit is checked and if it is undefined, a value from session (param xstep.cascade.unit) sets into document as length unit.

Results of tests:
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31382-master/view/COMPARE/ [^]

(0096829)
kgv   
2020-11-17 15:10   
+  case UnitsMethods_LengthUnit::LU_MILLIMETER: return "MILLIMETER";

This enumeration value syntax will raise old compiler warning on weekly Jenkins, please drop it.

Please also rename enumeration values to include full enumeration name, e.g.:
- UnitsMethods_LengthUnit::LU_MILLIMETER -> UnitsMethods_LengthUnit_Millimeter

(0096831)
msv   
2020-11-17 15:55   
+XSetLengthUnit D IncH
Why last letter is capital? It causes reader to find a hidden sense of it. Please use the simple word instead.

+ Handle(XCAFDoc_LengthUnitTool) aLUTool = XCAFDoc_LengthUnitTool::Set(Label().Father().FindChild(11, Standard_True));
Use LengthUnitLabel from document tool to access to the label of units.
(0096833)
git   
2020-11-17 16:07   
Branch CR31382 has been updated forcibly by skl.

SHA-1: 05da37a7aac569198019f4b4cf1a0508d426f8c1
(0096858)
git   
2020-11-18 07:02   
Branch CR31382 has been updated forcibly by skl.

SHA-1: 7de4b54ab5b9f28877857fdde5f276e9e7845bc4
(0096873)
skl   
2020-11-18 11:34   
Result of tests:
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31382-master/view/COMPARE/ [^]
(0096878)
msv   
2020-11-18 12:17   
Please rebase on the latest master.
(0096880)
git   
2020-11-18 12:28   
Branch CR31382 has been updated forcibly by skl.

SHA-1: cdaf43ee09e80fbc6d368d0c94b7fafb488142b5
(0096881)
msv   
2020-11-18 12:32   
For integration:
occt - CR31382
products - none
(0096891)
kgv   
2020-11-19 10:56   
(edited on: 2020-11-19 11:02)
--- a/src/TKXCAF/EXTERNLIB
+++ b/src/TKXCAF/EXTERNLIB
@@ -10,3 +10,5 @@ TKLCAF
 TKG3d
 TKCAF
 TKVCAF
+TKXSBase

Putting STEP reader/writer as dependency for XCAF document library doesn't look like an acceptable change.

+UnitsMethods_LengthUnitEnum.hxx

File name should be "UnitsMethods_LengthUnit.hxx" according to OCCT coding rules, not "UnitsMethods_LengthUnitEnum.hxx".

+UnitsMethods_LengthUnit lengthUnitByStr(TCollection_AsciiString theStr)

static

+UnitsMethods_LengthUnit lengthUnitByStr(TCollection_AsciiString theStr)
+{
...
+  else
+    return UnitsMethods_LengthUnit_Undefined;
...

+  TCollection_AsciiString aUnit(argv[2]);
+  aLUTool->SetLengthUnit(lengthUnitByStr(argv[2]));

Command is expected to print user an error in case of wrong input, not silently commit UNDEFINED unit.

+  if (argc < 3) {

Unknown arguments (>3) should be reported as a syntax error as well.

+  if (argc < 3) {
...
+enum UnitsMethods_LengthUnit {

Please follow OCCT coding rules in new code - put { on the next line.

@@ -548,6 +551,17 @@ TDF_Label XCAFDoc_ShapeTool::AddShape (const TopoDS_Shape& theShape,
...
+      aLU = (UnitsMethods_LengthUnit)Interface_Static::IVal("xstep.cascade.unit");

Reading STEP-specific parameter within the base tool is illogical.
In contrary, it is a STEP translator filling in XCAF document should put length unit information (and print a warning if document has already specified non-matching length unit).

+UnitsMethods_LengthUnit XCAFDoc_LengthUnitTool::GetLengthUnit() const
...
+    aVal = (UnitsMethods_LengthUnit)anAttribute->Get();

As there is no dedicated driver, no versioning and no guarantee that UnitsMethods_LengthUnit will not be extended in future, it would be helpful verifying the enumeration range before cast.
By the way, I think it might be more useful storing length unit as a string rather than enum in XCAF document.

+  static Standard_GUID LengthUnitID("4eba571f-de90-4840-8639-eab9b72360e0");

static const

+//! The Enumeration describes possible values for length units
+
+enum UnitsMethods_LengthUnit {

Unexpected empty line between documentation and enum.

+};
+#endif // _XCAFDoc_LengthUnitTool_HeaderFile

Please put an empty line after };

(0096900)
skl   
2020-11-19 15:11   
Package UnitsMethods is in toolkit TKXSBase.
Is it reasonable to move this package to another toolkit (for example to TKernel) in order to avoid dependency between TKXCAF and TKXSBase?
(0097032)
git   
2020-11-24 15:27   
Branch CR31382 has been updated forcibly by skl.

SHA-1: a1e2fe4eb0f4d439b71ea756b41793d9aeb1ca73
(0097453)
kgv   
2020-12-10 10:33   
> Is it reasonable to move this package to another toolkit
> (for example to TKernel) in order to avoid
> dependency between TKXCAF and TKXSBase?
UnitsMethods currently defines not only global "cascade units" property (GetCasCadeLengthUnit()), but also auxiliary geometry conversion methods.
So that while technically it looks reasonable moving GetCasCadeLengthUnit() outside, this cannot be done for the whole UnitsMethods package as it has to be split into parts.
(0097891)
git   
2020-12-24 16:30   
Branch CR31382 has been updated forcibly by skl.

SHA-1: 0425c2c264d1563a48e08a386a50d9d9b6032f39
(0097906)
git   
2020-12-25 16:47   
Branch CR31382 has been updated by skl.

SHA-1: 22a2d00cc3be85718bad5173f31d327326bb84bb


Detailed log of new commits:

Author: skl
Date: Fri Dec 25 16:47:44 2020 +0300

    0031382: Data Exchange - BinXCAF should preserve length unit information
    
    Split package UnitsMethods.

(0097907)
git   
2020-12-25 17:26   
Branch CR31382 has been updated forcibly by skl.

SHA-1: 3c5c03080bf878bed9c24bc86f6d6fbc00f254be
(0097958)
git   
2020-12-28 13:03   
Branch CR31382 has been updated by skl.

SHA-1: e250344fe147c0b792f013f4231824e145d9c35a


Detailed log of new commits:

Author: skl
Date: Mon Dec 28 13:03:29 2020 +0300

    0031382: Data Exchange - BinXCAF should preserve length unit information
    
    Try to avoid using "xstep.cascade.unit".

(0097962)
git   
2020-12-28 16:52   
Branch CR31382 has been updated forcibly by skl.

SHA-1: dd51932a9680eb7c6f2ab685ffb639758f3966db
(0097964)
git   
2020-12-28 18:59   
Branch CR31382 has been updated forcibly by skl.

SHA-1: 3e974687d560c8e8afe519b32f2de4b361ae7fbf
(0097969)
git   
2020-12-29 09:25   
Branch CR31382 has been updated forcibly by skl.

SHA-1: 3c086c829ba4912cd6616d5beb31b6e695342469
(0097985)
git   
2020-12-30 11:00   
Branch CR31382 has been updated forcibly by skl.

SHA-1: cccbf41f43b51078d31fb8c68cf8054468d312f5
(0097995)
skl   
2020-12-31 10:18   
Package UnitsMethods is splitted: geom methods was placed to new package UnitsGeomMethods which is in the toolkit TKXSBase. Package UnitsMethods is placed to toolkit TKernel.
Since for all supported formats value CasCadeLengthUnit is used for set unit of length it is reasonable to avoid using of parameter "xstep.cascade.unit".
New Draw command "setlengthunit" for set length unit is added. This command have to be used istead of command "param xstep.cascade.unit".
All using "xstep.cascade.unit" in the read/write classes is removed.
But for supporting of existed Draw tests and other using in "old" code setting for "xstep.cascade.unit" in class XSAlgo.cxx is kept.

Result of tests:
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31382-CR31382-skl/view/COMPARE/ [^]


(0097997)
msv   
2020-12-31 11:49   
The description of the method UnitsMethods::GetLengthUnitByFactorValue is not valid. Please correct it.

The new methods FactRD and FactDR in UnitsMethods have unclear names and no description. Please rename them like FactorRadianDegree and add description.

Why did you remove the virtual method XSAlgo_AlgoContainer::PrepareForTransfer? It can be redefined in some other code.

I propose to replace in the tests usage of "param xstep.cascade.unit" with "setlengthunit". It is in order to remove the usage of obsolete mechanism from OCCT.
(0098000)
git   
2020-12-31 16:15   
Branch CR31382 has been updated forcibly by skl.

SHA-1: f96ad714e2d84bf57b637cf82c60c9b2ccd992b3
(0098050)
skl   
2021-01-11 12:05   
Needed changes is done.
Result of tests:
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31382-CR31382-skl/view/COMPARE/ [^]
(0098061)
msv   
2021-01-11 14:11   
Again, the description of the method UnitsMethods::GetLengthUnitByFactorValue is not valid. Please correct it.
(0098066)
msv   
2021-01-11 14:26   
In XtControl_ActorRead::Transfer(), why did you commented out the following line? I think we should leave the call of this virtual method.
  //XSAlgo::AlgoContainer()->PrepareForTransfer();
(0098067)
msv   
2021-01-11 14:28   
Prepare new ready for integration branches with one commit in both occt and products.
(0098072)
git   
2021-01-11 15:19   
Branch CR31382 has been updated forcibly by skl.

SHA-1: cd6512ffde9a700723eafee98341526242620a48
(0098073)
git   
2021-01-11 15:22   
Branch CR31382 has been updated forcibly by skl.

SHA-1: d4fd7db52d1ec856ceda766cd822c1e102f0ef53
(0098083)
skl   
2021-01-12 10:04   
Needed changes is done.
Result of tests:
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31382-CR31382-skl/view/COMPARE/ [^]
(0098085)
kgv   
2021-01-12 11:16   
> Add LengthUnit info to XCAF document.
> Split package UnitsMethods.
> Try to avoid using "xstep.cascade.unit".

This comment description is too basic - it is expected to see details on modifications (how UnitsMethods package has been split? which interface is introduced in XCAF for LengthUnit info? what does it mean "try to avoid using xstep.cascade.unit"?).

+  di << "current value of length unit is " << UnitsMethods::DumpLengthUnit(aLU);


This is an overloaded output for the command intended to return variable value - please consider removing redundant text and keep only output of value itself.
So that Tcl script could be able to use "set aUnit [lengthunit]" without necessity to parse text.

--- a/src/TKernel/PACKAGES
+++ b/src/TKernel/PACKAGES
@@ -14,3 +14,4 @@ Units
 UnitsAPI
 NCollection
 Message
+UnitsMethods

Please apply alphabetical order for new entities.

+
+class UnitsGeomMethods 

Please provide description for new class and it's methods.

+  
+protected:
+
+private:
+
+};

No need in copying empty sections.

+  if (fabs(1. - theFactorValue) < aPreci)

fabs() -> Abs()

+enum UnitsMethods_LengthUnit {

Please follow OCCT Coding Style (newline before opening bracket).
(0098086)
msv   
2021-01-12 11:24   
src/UnitsMethods/UnitsMethods.hxx
  //! Returns the enumeration corresnoding to the given scale factor
Misprint "corresnoding"
(0098098)
skl   
2021-01-13 12:37   
Results of tests:
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31382-CR31382-skl/view/COMPARE/ [^]
(0098103)
msv   
2021-01-13 14:47   
I have corrected the spelling of commit message in CR31382_1.
(0098104)
msv   
2021-01-13 14:52   
For integration:
occt - CR31382_1
products - CR31382
(0098106)
kgv   
2021-01-13 16:24   
(edited on: 2021-01-13 16:25)
+static Standard_Integer setLengthUnit(Draw_Interpretor& di, Standard_Integer argc, const char** 
argv)
+{
+  if (argc < 2)
...
+static Standard_Integer lengthUnit(Draw_Interpretor& di, Standard_Integer 
+{
+  UnitsMethods_LengthUnit aLU = UnitsMethods::GetLengthUnitByFactorValue(UnitsMethods::GetCasCadeLengthUnit());

+  di << UnitsMethods::DumpLengthUnit(aLU);
+  return 0;

This is a weak check - it allows passing extra arguments to commands without user noticing they will be ignored. Please check for equality instead.

+  theCommands.Add("lengthunit", "return value of length unit"
+  theCommands.Add("setlengthunit", "set value of length unit"

Defining two commands for the same thing looks redundant - it will be simpler having a single command "lengthunit" doing both tasks depending on a number of arguments, as many other Draw Harness commands.

--- a/src/Interface/Interface_Static.cxx
+++ b/src/Interface/Interface_Static.cxx
+    if (strcmp(name, "xstep.cascade.unit") == 0)
+    {
+      Standard_Integer aUnit = item->IntegerValue();
+      UnitsMethods::SetCasCadeLengthUnit(aUnit);
+    }

Keeping this hack indefinitely within the basic interface for arbitrary parameters doesn't look like a good idea.
I propose generating a runtime warning (Message::SendWarning()) about deprecation of "xstep.cascade.unit" parameter, so that we will remove this code after a couple of OCCT releases.

+//! Class contains conversion methods for 2d geom objects
+class UnitsGeomMethods
+{

I don't know if these methods have been supposed to be public / have any use for external applications, but if yes - this API change should be mentioned in Upgrade Guide, as user will be unable to figure out where the methods have gone.

(0098803)
dpasukhi   
2021-02-09 23:26   
Solution elaboration
Elaboration storing of new XCAF attribute ( Units )
(0098822)
dpasukhi   
2021-02-10 23:58   
Solution elaboration
(0098834)
dpasukhi   
2021-02-11 23:47   
Solution elaboration & testing

Implementation of a new attribute for storing units
(0098854)
dpasukhi   
2021-02-12 23:56   
Solution elaboration & testing
(0099775)
dpasukhi   
2021-03-27 01:58   
Solution elaboration
(0099897)
dpasukhi   
2021-03-30 23:14   
Solution elaboration
(0100355)
dpasukhi   
2021-04-16 19:58   
Solution elaboration
(0100750)
dpasukhi   
2021-05-02 23:38   
Solution elaboration
(0100835)
dpasukhi   
2021-05-09 21:50   
(0101078)
dpasukhi   
2021-05-16 23:13   
(edited on: 2021-05-17 12:04)
Solution implementation

(0101193)
git   
2021-05-20 03:23   
Branch CR31382_2 has been created by dpasukhi.

SHA-1: 9e94dab88a63b3abb68f26ec0abd7e8ec6ff87a7


Detailed log of new commits:

Author: dpasukhi
Date: Tue Nov 10 07:52:30 2020 +0300

    0031382: Data Exchange - BinXCAF should preserve length unit information
    
    Possibility for adding LengthUnit info to XCAF document using special class TDataStd_LenghtUnit is implemented
    Package UnitsMethods is splited: geom methods was placed to new file GeomConvert_Units which is in the toolkit TKXSBase, internal step scale factors was placed to StepData
    All global units was replace by UnitsAPI
    Since for all supported formats value CasCadeLengthUnit is used for set unit of length using of parameter "xstep.cascade.unit" for set length unit is not needed.
    New Draw command "setlengthunit" for set value of length unit is added. This command have to be used istead of command "param xstep.cascade.unit".
    All using "xstep.cascade.unit" in the read/write classes is removed.
    Initialization of "xstep.cascade.unit" is kept in the method XSAlgo::Init() in order to support old versions.
(0101293)
dpasukhi   
2021-05-23 23:16   
Solution implementation & testing
(0101295)
git   
2021-05-24 02:28   
Branch CR31382_2 has been updated forcibly by dpasukhi.

SHA-1: 42ddefea9995a4500b5876c59397c197649e0c28
(0101330)
abv   
2021-05-24 16:02   
A remark following our discussion: we do not seem to have a strong need of global parameter defining units of OCCT apart of DE components (which use xstep.cascade,units). These units are defined by the application and can be managed in application-specific way (usially this is not customizable).

At the same time, there is apparently a need to have possibility to convert XCAF document defined in one units, to some other units. This operation shall be performed after loading XCAF document if it was written in units different from those used by the application.
(0101356)
git   
2021-05-25 13:45   
Branch CR31382_2 has been updated by dpasukhi.

SHA-1: 89fcc93f371d6319627c8b0486885d0ccfab683b


Detailed log of new commits:

Author: dpasukhi
Date: Tue May 25 13:45:45 2021 +0300

    // fix unit initialization ( test version )

(0101388)
git   
2021-05-26 13:25   
Branch CR31382_2 has been updated forcibly by dpasukhi.

SHA-1: 58595f39c67bb437624bc500e138673b9db31db8
(0101394)
git   
2021-05-26 14:44   
Branch CR31382_2 has been updated forcibly by dpasukhi.

SHA-1: 5a42d15f8e88affe6b1716123a8e567b7fe7bddc
(0101514)
dpasukhi   
2021-05-30 23:32   
Solution elaboration

Revert old realization. Development according to the splitting of the task
(0101565)
git   
2021-06-03 04:19   
Branch CR31382_3 has been created by dpasukhi.

SHA-1: 73bf8b601034a798dd65840b67bff8d94bdfa280


Detailed log of new commits:

Author: dpasukhi
Date: Tue Nov 10 07:52:30 2020 +0300

    0031382: Data Exchange - BinXCAF should preserve length unit information
    
    Possibility for adding LengthUnit info to XCAF document using special class TDataStd_LenghtUnit is implemented
    Package UnitsMethods is splited: geom methods was placed to new file GeomConvert_Units which is in the toolkit TKXSBase, internal step scale factors was placed to StepData
    Since for all supported formats value CasCadeLengthUnit is used for set unit of length using of parameter "xstep.cascade.unit" for set length unit is not needed.
    New Draw command "XSetlengthunit" for set XDE attribute for in the doc.
(0101573)
git   
2021-06-03 12:28   
Branch CR31382_3 has been updated forcibly by dpasukhi.

SHA-1: 5683e2df45c38fae113f16769e8c6763acc74585
(0101648)
dpasukhi   
2021-06-06 23:45   
(edited on: 2021-06-06 23:51)
Solution implementation

(0102006)
git   
2021-06-23 12:21   
Branch CR31382_4 has been created by dpasukhi.

SHA-1: 7c6a6aa14c8b4abc316cdc09a162b800ff09798a


Detailed log of new commits:

Author: dpasukhi
Date: Tue Nov 10 07:52:30 2020 +0300

    0031382: Data Exchange - BinXCAF should preserve length unit information
    
    Possibility for adding LengthUnit info to XCAF document using special class TDataStd_LenghtUnit is implemented
    Package UnitsMethods is splited: geom methods was placed to new file GeomConvert_Units which is in the toolkit TKXSBase, internal step scale factors was placed to StepData
    Since for all supported formats value CasCadeLengthUnit is used for set unit of length using of parameter "xstep.cascade.unit" for set length unit is not needed.
    New Draw command "XSetlengthunit" for set XDE attribute for in the doc.
    Upgraded tests\de to check "loop back" algorithms with any unit
(0102030)
git   
2021-06-24 15:31   
Branch CR31382_4 has been updated forcibly by dpasukhi.

SHA-1: 5d30d207c7121ee0abca1d9c729125543280bb13
(0102031)
git   
2021-06-24 15:35   
Branch CR31382_4 has been updated forcibly by dpasukhi.

SHA-1: ea11886458d8e952efc82c295731913f822658cd
(0102108)
dpasukhi   
2021-06-27 22:59   
(0102917)
git   
2021-08-02 02:59   
Branch CR31382_5 has been created by dpasukhi.

SHA-1: 0c735100b2ce35614453c58f5d388f4be168e263


Detailed log of new commits:

Author: dpasukhi
Date: Tue Nov 10 07:52:30 2020 +0300

    0031382: Data Exchange - BinXCAF should preserve length unit information
    
    Possibility for adding LengthUnit info to XCAF document using special class TDataStd_LenghtUnit is implemented
    Package UnitsMethods is splited: geom methods was placed to new file GeomConvert_Units which is in the toolkit TKXSBase, internal step scale factors was placed to StepData
    Since for all supported formats value CasCadeLengthUnit is used for set unit of length using of parameter "xstep.cascade.unit" for set length unit is not needed.
    New Draw command "XSetlengthunit" for set XDE attribute for in the doc.
    Upgraded tests\de to check "loop back" algorithms with any unit
(0103144)
git   
2021-08-12 02:57   
Branch CR31382_5 has been updated forcibly by dpasukhi.

SHA-1: 0e0fecee8da5a25eff16a1a082b7841b3ec2ce40
(0103191)
git   
2021-08-13 19:06   
Branch CR31382_5 has been updated by dpasukhi.

SHA-1: c8002f3986bf65d4c0572be4c074de8d5fa737f0


Detailed log of new commits:

Author: dpasukhi
Date: Fri Aug 13 19:05:11 2021 +0300

    // Added Length unit attribute to Doc as new type of an attribute

(0103192)
git   
2021-08-13 19:14   
Branch CR31382_5 has been updated forcibly by dpasukhi.

SHA-1: 0bf1a152f08be0a3d8a4f0f88282f56598594a1c
(0103225)
dpasukhi   
2021-08-15 23:50   
(0103274)
git   
2021-08-18 15:08   
Branch CR31382_6 has been created by dpasukhi.

SHA-1: c346bcbc181da9d3cbd88490085aff73f1b61dc6


Detailed log of new commits:

Author: dpasukhi
Date: Tue Nov 10 07:52:30 2020 +0300

    0031382: Data Exchange - BinXCAF should preserve length unit information
    
    Possibility for adding LengthUnit info to XCAF document using special class XCAFDoc_LenghtUnit and XCAFDoc_LenghtUnitTool is implemented
    Package UnitsMethods is splited: geom methods was placed to new file GeomConvert_Units which is in the toolkit TKXSBase, internal step scale factors was placed to StepData
    Updated UnitMethods to convert scale factor to different unit types.
    Now, XSAlgo::XSAlgo_AlgoContainer used for update unit info from static interface values.
    New Draw command "XSetlengthunit" and "XGetLengthUnit" for set or get XDE attribute
    Upgraded tests for step, iges, obj, gltf, vrlm formats to check area regressing with used unit
    Upgraded tests\de test cases to use any units in the "loop back" algorithms
(0103292)
git   
2021-08-18 21:13   
Branch CR31382_6 has been updated by dpasukhi.

SHA-1: 1dd95deadc82abb91b90af69b1b661621d815641


Detailed log of new commits:

Author: dpasukhi
Date: Wed Aug 18 21:09:42 2021 +0300

    // remarks have been fixed

(0103295)
git   
2021-08-18 22:45   
Branch CR31382_6 has been updated by dpasukhi.

SHA-1: 8403a799b6c4383570f960fb71a37ef60fb9c8b4


Detailed log of new commits:

Author: dpasukhi
Date: Wed Aug 18 22:45:10 2021 +0300

    // fixed problem with StepCafControl_Writer and IGESCAFControl_Writer transferring

(0103296)
git   
2021-08-18 23:21   
Branch CR31382_6 has been updated forcibly by dpasukhi.

SHA-1: ad0cc3c986b72d249fe80d7a3cf29e115d78ab9e
(0103301)
git   
2021-08-19 12:34   
Branch CR31382_7 has been created by dpasukhi.

SHA-1: 7263ddef83eb434547862d305fa115ea2af94650


Detailed log of new commits:

Author: dpasukhi
Date: Tue Nov 10 07:52:30 2020 +0300

    0031382: Data Exchange - BinXCAF should preserve length unit information
    
    Possibility for adding LengthUnit info to XCAF document using special class XCAFDoc_LenghtUnit and XCAFDoc_LenghtUnitTool is implemented
    Package UnitsMethods is splited: geom methods was placed to new file GeomConvert_Units which is in the toolkit TKXSBase, internal step scale factors was placed to StepData
    Updated UnitMethods to convert scale factor to different unit types.
    Now, XSAlgo::XSAlgo_AlgoContainer used for update unit info from static interface values.
    New Draw command "XSetLengthUnit" and "XGetLengthUnit" for set or get XDE attribute
    Upgraded tests for step, iges, obj, gltf, vrlm formats to check area regressing with used unit
    Upgraded tests\de test cases to use any units in the "loop back" algorithms
(0103306)
git   
2021-08-19 14:50   
Branch CR31382_7 has been updated forcibly by dpasukhi.

SHA-1: d8e12e9581b74dd0891791ad8c7d25f1629f9ed1
(0103311)
git   
2021-08-19 16:20   
Branch CR31382_7 has been updated forcibly by dpasukhi.

SHA-1: 4db9971672adfb59601da91281314860198940c8
(0103312)
dpasukhi   
2021-08-19 17:41   
(edited on: 2021-08-20 10:06)
Dear KGV,
please review the CR31382_7

OCCT: CR31382_7
PRODUCTS: NO

All tests are OK, see:
http://jenkins-test-08.nnov.opencascade.com/view/CR31382-master-dpasukhi/view/COMPARE/ [^]

(0103314)
git   
2021-08-19 22:28   
Branch CR31382_7 has been updated forcibly by dpasukhi.

SHA-1: 19807e831e719b183eb58fef7fafc4726307f907
(0103319)
kgv   
2021-08-20 15:47   
(0103326)
bugmaster   
2021-08-21 14:19   
Combination -
OCCT branch : IR-2021-08-20
master SHA - da80ff68f1e9853c1015a5dbb6199541e92789ce
a87b7ddc8cb44606b91e3f37113847c3f5f50fdc
Products branch : IR-2021-08-20 SHA - dd282797d1b40a373376bcd4a820edeb3d4d52f6
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: 17364.550000000334 / 17371.860000000408 [-0.04%]
Products
Total CPU difference: 11522.500000000102 / 11493.740000000085 [+0.25%]
Windows-64-VC14:
OCCT
Total CPU difference: 19242.96875 / 19148.140625 [+0.50%]
Products
Total CPU difference: 12876.5625 / 12805.328125 [+0.56%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0103329)
git   
2021-08-21 14:31   
Branch CR31382 has been deleted by mnt.

SHA-1: 8c60f25ff0d623a713738776f3f6b846908a3f29
(0103330)
git   
2021-08-21 14:31   
Branch CR31382_1 has been deleted by mnt.

SHA-1: 1f4050bc7036002ba99a5cdff459f9f648711759
(0103331)
git   
2021-08-21 14:31   
Branch CR31382_2 has been deleted by mnt.

SHA-1: 5a42d15f8e88affe6b1716123a8e567b7fe7bddc
(0103332)
git   
2021-08-21 14:31   
Branch CR31382_3 has been deleted by mnt.

SHA-1: 5683e2df45c38fae113f16769e8c6763acc74585
(0103333)
git   
2021-08-21 14:31   
Branch CR31382_4 has been deleted by mnt.

SHA-1: ea11886458d8e952efc82c295731913f822658cd
(0103334)
git   
2021-08-21 14:31   
Branch CR31382_5 has been deleted by mnt.

SHA-1: 0bf1a152f08be0a3d8a4f0f88282f56598594a1c
(0103335)
git   
2021-08-21 14:32   
Branch CR31382_6 has been deleted by mnt.

SHA-1: ad0cc3c986b72d249fe80d7a3cf29e115d78ab9e
(0103336)
git   
2021-08-21 14:32   
Branch CR31382_7 has been deleted by mnt.

SHA-1: 19807e831e719b183eb58fef7fafc4726307f907