MantisBT - Open CASCADE
View Issue Details
0031382Open CASCADE[OCCT] OCCT:Data Exchangepublic2020-02-18 17:302021-01-13 16:25
kgv 
skl 
normalfeature 
assignedopen 
 
[OCCT] 7.6.0* 
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 0031136reviewed bugmaster Open CASCADE Modeling Data - BinXCAF persistence loses normals from triangulation-only Faces 
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

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.