Visualization, TKV3d - eliminate global variables

Branch [archived branch] has been created by Commenter 3.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Mar 24 10:11:26 2016 +0300

    0026886: Visualization, TKV3d - eliminate global variables
    
    AIS_InteractiveContext - create new dummy class field to have an empty TopoDS_Shape object.
    AIS_Point, PrsMgr_PresentableObject - rename static variables to local function variables.
    AIS_Shape, SelectMgr_SelectableObject - remove unused static variables.
    Graphic3d_MaterialAspect, V3d_Viewer - make global static variables as constant.
    V3d_View - move global variable to class field.

+myDummyShape(TopoDS_Shape())

in OCCT fields initialized with default constructors are usually ommited from initialization list.

       // Beeurk.. a revoir - rob-25/04/97
-      static Handle(Graphic3d_AspectMarker3d) PtA = 
-       new Graphic3d_AspectMarker3d ();
+      Handle(Graphic3d_AspectMarker3d) PtA = new Graphic3d_AspectMarker3d ();

please drop the comment above.

+  myZRotation = Standard_False;

please move to constructor initialization list.

+  static const TCollection_AsciiString XLetter("X");
+  static const TCollection_AsciiString YLetter("Y");
+  static const TCollection_AsciiString ZLetter("Z");

please drop these variables and pass constants directly.

Branch [archived branch] has been updated by Commenter 3.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Tue Mar 29 12:57:11 2016 +0300

    0026886: Visualization, TKV3d - eliminate global variables
    
    - Move a variable theCurrentSelection as a field of AIS_InteractiveContext and AIS_LocalContext classes. Multiple selection is not used now, so each Context have an own selection.
    - Move myStructGenId from Graphic3d_StructureManager to Graphic3d_GraphicDriver for identifying the structures in the driver.

-    Group->Text(YLetter.ToCString(),Graphic3d_Vertex(pY.X(),pY.Y(),pY.Z()),1./81.);
+       Group->Text("Y",Graphic3d_Vertex(pY.X(),pY.Y(),pY.Z()),1./81.);

please remove all new occurrences of tabulation symbols within your patch.

Branch [archived branch] has been updated by Commenter 3.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Tue Mar 29 15:48:37 2016 +0300

    0026886: Visualization, TKV3d - eliminate global variables

Branch [archived branch] has been updated forcibly by Commenter 3.

[revision removed]

All changes were committed in branch CR26886.

Dear Marina,

I have just few minor remarks. Please treat them if you find it important.

1) AIS_Selection::Extent(), AIS_Selection::IsSelected() are not static anymore. It looks like they could also have const modifier?
2) Order of initialization of V3d_View members in constructor (myZRotation against MyTrsf) does not correspond to class declaration. Some compilers might throw warning for it.
3) With this patch I do not see how to use AIS_Selection outside of AIS_InteractiveContext. Probably it makes sense to add a sort of "porting note" into comments? For example: methods of AIS_InteractiveContext::InitSelected(), ::MoreSelected(), ::NextSelected() should be used instead of static methods of AIS_Selection. Else you may expose Handle(AIS_Selection) from interface of interactive context, but it would rather make AIS logic more complicated for our users.

No major remarks, in general the patch is OK.

Branch [archived branch] has been updated by Commenter 3.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Wed Mar 30 16:47:11 2016 +0300

    - Make some changes for code optimization.

Branch [archived branch] has been updated forcibly by Commenter 3.

[revision removed]

Dear Commenter 1,

Please test the branch.

Branch [archived branch] has been updated by Commenter 3.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Fri Apr 1 09:20:05 2016 +0300

    - Move default variable (no shading light) from static value to the class field of OpenGL_GraphicDriver.

Branch [archived branch] has been updated by Commenter 3.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Fri Apr 1 10:07:15 2016 +0300

    - Move default variable (no shading light) from static value to the class field of OpenGL_View.

Branch [archived branch] has been updated forcibly by Commenter 3.

[revision removed]

Branch [archived branch] has been updated forcibly by Commenter 3.

[revision removed]

Please test updated patch.

Dear Commenter 1,
Branch CR26886 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]

There are following compilation errors:

http://jenkins-test-07.nnov.opencascade.com:8080/view/CR26886-master/job/CR26886-master-OCCT-Windows-64-VC10-mfc-samples/1/parsed_console/
1>..\..\..\..\Common\DimensionDlg.cpp(769): error C2039: 'CurrentSelection' : is not a member of 'AIS_Selection'
1> d:\install\CR26886-master\Windows-64-VC10-opt\OCCT\inc\AIS_Selection.hxx(41) : see declaration of 'AIS_Selection'
1>..\..\..\..\Common\DimensionDlg.cpp(769): error C2227: left of '->Value' must point to class/struct/union/generic type
1> type is ''unknown-type''
1>..\..\..\..\Common\DimensionDlg.cpp(769): error C3861: 'CurrentSelection': identifier not found

http://jenkins-test-07.nnov.opencascade.com:8080/view/CR26886-master/job/CR26886-master-Products-Windows-64-VC10-csharp-samples/1/parsed_console/
1>occcsharp_wrap.obj : error LNK2019: unresolved external symbol "public: class TCollection_AsciiString const & __cdecl AIS_InteractiveContext::CurrentName(void)const " (?CurrentName@AIS_InteractiveContext@@QEBAAEBVTCollection_AsciiString@@XZ) referenced in function CSharp_AIS_InteractiveContext_CurrentName
1>d:\install\CR26886-master\Windows-64-VC10-opt\Products\samples\csharp\\win64\VC10\bin\OCCwrapCSharp.dll : fatal error LNK1120: 1 unresolved externals

http://jenkins-test-07.nnov.opencascade.com:8080/view/CR26886-master/job/CR26886-master-Products-Windows-64-VC10-java-samples/1/parsed_console/
occjava_wrap.obj : error LNK2019: unresolved external symbol "public: class TCollection_AsciiString const & __cdecl AIS_InteractiveContext::CurrentName(void)const " (?CurrentName@AIS_InteractiveContext@@QEBAAEBVTCollection_AsciiString@@XZ) referenced in function Java_opencascade_OCCwrapJavaJNI_AIS_1InteractiveContext_1CurrentName [D:\install\CR26886-master\Windows-64-VC10-opt\Products\samples\java\win64\VC10\cmake\OCCwrapJava.vcxproj]
D:\install\CR26886-master\Windows-64-VC10-opt\Products\samples\java\win64\VC10\bin\OCCwrapJava.dll : fatal error LNK1120: 1 unresolved externals [D:\install\CR26886-master\Windows-64-VC10-opt\Products\samples\java\win64\VC10\cmake\OCCwrapJava.vcxproj]
  occjava_wrap.obj : error LNK2019: unresolved external symbol "public: class TCollection_AsciiString const & __cdecl AIS_InteractiveContext::CurrentName(void)const " (?CurrentName@AIS_InteractiveContext@@QEBAAEBVTCollection_AsciiString@@XZ) referenced in function Java_opencascade_OCCwrapJavaJNI_AIS_1InteractiveContext_1CurrentName [D:\install\CR26886-master\Windows-64-VC10-opt\Products\samples\java\win64\VC10\cmake\OCCwrapJava.vcxproj]
  D:\install\CR26886-master\Windows-64-VC10-opt\Products\samples\java\win64\VC10\bin\OCCwrapJava.dll : fatal error LNK1120: 1 unresolved externals [D:\install\CR26886-master\Windows-64-VC10-opt\Products\samples\java\win64\VC10\cmake\OCCwrapJava.vcxproj]

http://jenkins-test-07.nnov.opencascade.com:8080/view/CR26886-master/job/CR26886-master-Products-Windows-64-VC10-mfc-samples/1/parsed_console/
1>Kernel\Kernel_Operator.cxx(252): error C2039: 'CurrentSelection' : is not a member of 'AIS_Selection'
1> d:\install\CR26886-master\Windows-64-VC10-opt\OCCT\inc\AIS_Selection.hxx(41) : see declaration of 'AIS_Selection'
1>Kernel\Kernel_Operator.cxx(252): error C3861: 'CurrentSelection': identifier not found
1>Kernel\ShapeHealingDoc.cpp(963): error C2039: 'CurrentSelection' : is not a member of 'AIS_Selection'
1> d:\install\CR26886-master\Windows-64-VC10-opt\OCCT\inc\AIS_Selection.hxx(41) : see declaration of 'AIS_Selection'
1>Kernel\ShapeHealingDoc.cpp(963): error C3861: 'CurrentSelection': identifier not found
1>Kernel\ShapeHealingDoc.cpp(985): error C2352: 'AIS_Selection::AddSelect' : illegal call of non-static member function
1> d:\install\CR26886-master\Windows-64-VC10-opt\OCCT\inc\AIS_Selection.hxx(60) : see declaration of 'AIS_Selection::AddSelect'
1>Kernel\ShapeHealingDoc.cpp(1591): error C2039: 'CurrentSelection' : is not a member of 'AIS_Selection'
1> d:\install\CR26886-master\Windows-64-VC10-opt\OCCT\inc\AIS_Selection.hxx(41) : see declaration of 'AIS_Selection'
1>Kernel\ShapeHealingDoc.cpp(1591): error C3861: 'CurrentSelection': identifier not found


Number of compiler warnings:

occt component :
Linux: 6 (0 on master)
Windows: 0 (0 on master)
MacOS : 2 (0 on master)

products component :
Linux: 68 (68 on master)
Windows: 0 (0 on master)
MacOS : 1121

There are new additional compilation warnings on Linux and MacOS platforms:
http://jenkins-test-07.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_13912/job/CR26886-master-OCCT-Debian70-64-opt-compile/1/warnings17Result/
AIS_InteractiveContext.cxx:78, GNU C Compiler 4 (gcc), Priority: Normal
when initialized here [-Wreorder]
AIS_InteractiveContext.hxx:1627, GNU C Compiler 4 (gcc), Priority: Normal
'Quantity_NameOfColor AIS_InteractiveContext::myDefaultColor' [-Wreorder]
AIS_InteractiveContext.hxx:1640, GNU C Compiler 4 (gcc), Priority: Normal
'AIS_InteractiveContext::mySelection' will be initialized after [-Wreorder]
AIS_LocalContext.cxx:61, GNU C Compiler 4 (gcc), Priority: Normal
when initialized here [-Wreorder]
AIS_LocalContext.hxx:415, GNU C Compiler 4 (gcc), Priority: Normal
'opencascade::handle AIS_LocalContext::mySelection' [-Wreorder]
AIS_LocalContext.hxx:422, GNU C Compiler 4 (gcc), Priority: Normal
'AIS_LocalContext::myMapOfOwner' will be initialized after [-Wreorder]
http://jenkins-test-07.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_13912/job/CR26886-master-OCCT-MacOS-opt-compile/1/warnings7Result/
AIS_InteractiveContext.cxx:89, Clang (LLVM based), Priority: Normal
field 'mySelection' will be initialized after field 'myDefaultColor'
AIS_LocalContext.cxx:75, Clang (LLVM based), Priority: Normal
field 'myMapOfOwner' will be initialized after field 'mySelection'

Regressions/Differences/Improvements:
http://occt-tests/CR26886-master-OCCT/Windows-64-VC10/summary.html
http://occt-tests/CR26886-master-Products/Windows-64-VC10/summary.html

Testing cases:
Not needed

Testing on Windows:
occt component :
Total MEMORY difference: 55366077 / 57116204 [-3.06%]
Total CPU difference: 17814.939797598865 / 17822.44344569882 [-0.04%]
products component :
Total MEMORY difference: 17757941 / 17813429 [-0.31%]
Total CPU difference: 5151.80822419995 / 5073.66732329995 [+1.54%]

There are no differences in images found by testdiff.

Branch [archived branch] has been created by Commenter 3.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Wed Apr 6 13:46:55 2016 +0300

    0026886: Visualization, TKV3d - eliminate global variables
    
    - AIS_InteractiveContext - create new dummy class field to have an empty TopoDS_Shape object.
    - AIS_Point, PrsMgr_PresentableObject - rename static variables to local function variables.
    - AIS_Shape, SelectMgr_SelectableObject - remove unused static variables.
    - Graphic3d_MaterialAspect, V3d_Viewer - make global static variables as constant.
    - V3d_View - move global variable zRotation to class field.
    - Move a variable theCurrentSelection as a field of AIS_InteractiveContext and AIS_LocalContext classes. Multiple selection is not used now, so each Context have an own selection.
    - Move myStructGenId from Graphic3d_StructureManager to Graphic3d_GraphicDriver for identifying the structures in the driver.
    - Move default variable (no shading light) from static value to the class field of OpenGL_View.
    Porting note:
    - Static methods of AIS_Selection is not used now. Methods of
      AIS_InteractiveContext::InitSelected(),::MoreSelected(),::NextSelected()
      should be used instead of static methods of AIS_Selection.

Dear Kirill, please review.
All changes are in the CR26886_1 branch now.

Moreover, the same branch(CR26886_1) has been created in occt-products with update in samples.

Please test updated patch.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Commenter 1,
Branch CR26886_1 was rebased on current master of occt git-repository.
[revision removed]
Branch CR26886_1 was rebased on current master of products git-repository.
[revision removed]

Dear Commenter 1,
Branch CR26886_1 from occt git-repository (and CR26886_1 from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]
[revision removed]

There are following compilation errors:
Windows:
http://jenkins-test-07.nnov.opencascade.com:8080/view/CR26886-1-CR26886-1/job/CR26886-1-CR26886-1-Products-Windows-64-VC10-mfc-samples/2/parsed_console/
1>Kernel\ShapeHealingDoc.cpp(1588): error C2059: syntax error : '->'
1>Kernel\ShapeHealingDoc.cpp(1589): error C2039: 'myAISContext' : is not a member of 'AIS_InteractiveContext'
1> d:\install\CR26886-1-CR26886-1\Windows-64-VC10-opt\OCCT\inc\AIS_InteractiveContext.hxx(135) : see declaration of 'AIS_InteractiveContext'
1>Kernel\ShapeHealingDoc.cpp(1589): error C2059: syntax error : '->'
1>Kernel\ShapeHealingDoc.cpp(1590): error C2039: 'myAISContext' : is not a member of 'AIS_InteractiveContext'
1> d:\install\CR26886-1-CR26886-1\Windows-64-VC10-opt\OCCT\inc\AIS_InteractiveContext.hxx(135) : see declaration of 'AIS_InteractiveContext'
1>Kernel\ShapeHealingDoc.cpp(1590): error C2059: syntax error : '->'
1>Kernel\ShapeHealingDoc.cpp(1591): error C2143: syntax error : missing ';' before '{'
1>Kernel\ShapeHealingDoc.cpp(1593): error C2039: 'myAISContext' : is not a member of 'AIS_InteractiveContext'
1> d:\install\CR26886-1-CR26886-1\Windows-64-VC10-opt\OCCT\inc\AIS_InteractiveContext.hxx(135) : see declaration of 'AIS_InteractiveContext'
1>Kernel\ShapeHealingDoc.cpp(1593): error C2232: '->AIS_InteractiveContext::SelectedOwner' : left operand has '' type, use '.'
1>Kernel\ShapeHealingDoc.cpp(1593): error C2227: left of '->Value' must point to class/struct/union/generic type
1>Kernel\ShapeHealingDoc.cpp(1598): error C2043: illegal break

Number of compiler warnings:

occt component :
Linux: 3 (0 on master)
Windows: 0 (0 on master)
MacOS : 1 (0 on master)

products component :
Linux: 68 (68 on master)
Windows: 0 (0 on master)
MacOS : 1123

There are new additional compilation warnings on Linux and MacOS platforms:
http://jenkins-test-07.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_13912/job/CR26886-1-CR26886-1-OCCT-Debian70-64-opt-compile/1/warnings17Result/
AIS_LocalContext.cxx:61, GNU C Compiler 4 (gcc), Priority: Normal
when initialized here [-Wreorder]
AIS_LocalContext.hxx:417, GNU C Compiler 4 (gcc), Priority: Normal
'opencascade::handle AIS_LocalContext::mySelection' [-Wreorder]
AIS_LocalContext.hxx:424, GNU C Compiler 4 (gcc), Priority: Normal
'AIS_LocalContext::myMapOfOwner' will be initialized after [-Wreorder]
http://jenkins-test-07.nnov.opencascade.com:8080/user/mnt/my-views/view/A_mnt_warnings/portlet/dashboard_portlet_13912/job/CR26886-1-CR26886-1-OCCT-MacOS-opt-compile/1/warnings7Result/
AIS_LocalContext.cxx:75, Clang (LLVM based), Priority: Normal
field 'myMapOfOwner' will be initialized after field 'mySelection'

Regressions/Differences/Improvements:
No regressions/differences

Testing cases:
Not needed

Testing on Linux:
occt component :
Total MEMORY difference: 87959106 / 89134925 [-1.32%]
Total CPU difference: 19303.380000000012 / 19563.780000000057 [-1.33%]
products component :
Total MEMORY difference: 25198790 / 25291832 [-0.37%]
Total CPU difference: 5220.159999999992 / 5207.169999999992 [+0.25%]

Testing on Windows:
occt component :
Total MEMORY difference: 55702205 / 57547141 [-3.21%]
Total CPU difference: 18406.901192198853 / 17768.513899998856 [+3.59%]
products component :
Total MEMORY difference: 17319351 / 17370131 [-0.29%]
Total CPU difference: 5235.705561999962 / 5014.230942299948 [+4.42%]

There are no differences in images found by testdiff.

Dear mpa,
Branch [archived branch] has been rejected due to:
- compilation errors
- additional warnings

Branch [archived branch] has been created by Commenter 3.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Fri Apr 8 09:47:47 2016 +0300

    0026886: Visualization, TKV3d - eliminate global variables
    
    - AIS_InteractiveContext - create new dummy class field to have an empty TopoDS_Shape object.
    - AIS_Point, PrsMgr_PresentableObject - rename static variables to local function variables.
    - AIS_Shape, SelectMgr_SelectableObject - remove unused static variables.
    - Graphic3d_MaterialAspect, V3d_Viewer - make global static variables as constant.
    - V3d_View - move global variable zRotation to class field.
    - Move a variable theCurrentSelection as a field of AIS_InteractiveContext and AIS_LocalContext classes. Multiple selection is not used now, so each Context have an own selection.
    - Move myStructGenId from Graphic3d_StructureManager to Graphic3d_GraphicDriver for identifying the structures in the driver.
    - Move default variable (no shading light) from static value to the class field of OpenGL_View.
    Porting note:
    - Static methods of AIS_Selection is not used now. Methods of
      AIS_InteractiveContext::InitSelected(),::MoreSelected(),::NextSelected()
      should be used instead of static methods of AIS_Selection.

Dear Kirill, please review.
All changes are in the CR26886_2 branch for OCCT and occt-products.

Please check updated patch.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Commenter 1,
Branch CR26886_2 from occt git-repository (and CR26886_2 from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]
[revision removed]:

Number of compiler warnings:

occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)

products component :
Linux: 68 (68 on master)
Windows: 0 (0 on master)
MacOS : 1135

Regressions/Differences/Improvements:
No regressions/differences

Testing cases:
Not needed

Testing on Linux:
occt component :
Total MEMORY difference: 88319104 / 90755610 [-2.68%]
Total CPU difference: 19423.780000000068 / 19577.11000000022 [-0.78%]
products component :
Total MEMORY difference: 27447631 / 27524349 [-0.28%]
Total CPU difference: 5310.349999999996 / 5435.7999999999765 [-2.31%]

Testing on Windows:
occt component :
Total MEMORY difference: 55830428 / 57720563 [-3.27%]
Total CPU difference: 18628.9218153987 / 18068.26982149889 [+3.10%]
products component :
Total MEMORY difference: 19154571 / 19253144 [-0.51%]
Total CPU difference: 5297.684759299964 / 5269.479778499955 [+0.54%]

There are no differences in images found by testdiff.

Dear Commenter 1,
Branch CR26886_2 is TESTED.

Branch [archived branch] has been deleted by Author.

[revision removed]

Branch [archived branch] has been deleted by Author.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]