OCCT3D
DiscussionsIssue archiveOCCT:Visualization

Archived issue #0023028

Visualization, TKOpenGl - eliminate global static variables

Open CASCADEOCCT:Visualizationclosed23 public notes

Search issues

Description

Current implementation exploits the fact that only one graphic driver instance can be used in application and a lot of static variables and collections are placed in global scope.

However there are no any protection from creation of multiple instance and in this case behaviour of TKOpenGl module become undefined.

For example shared instance 'Handle(OpenGl_Display) openglDisplay' is nullified on Graphic3d_{WNT}GraphicDevice destruction and means memory corruption if there any other Graphic3d_{WNT}GraphicDevice instance exists and used.

As a side-effect of this problem, currently OpenGl_Window instances are never destroyed, to keep GL rendering contexts for proper GL resource deallocation.

This is suggested either:
- To move all global variables into OpenGl_GraphicDriver definition (user will be responsible for usage of multiple instances / sharing common instance);
- Make OpenGl_GraphicDriver singleton (protect usage of TKOpenGl library in undesigned manner).

Steps to reproduce

N/A

Public activity

23 archived notes

Participants are labeled by their role within this record.

01Author
Notice there also regression related to this bug introduced by #0022819 patch:

OpenGl_telem_view.cxx, line 71
> /* prp between front and back planes */
> if (!openglDisplay.IsNull() || !openglDisplay->Walkthrough())
Wrong condition followed by NULL handle access if shared openglDisplay instance is NULL.
02Author
OpenGl_Display is no more global since #0024228.
However there are still several global variables like
static float  mAmb[4];

in OpenGl_Workspace::UpdateMaterial().
03Author
Remaining artifacts,

in OpenGl_CappingAlgo::RenderCapping():
  static Handle(OpenGl_CappingAlgoFilter) aCappingFilter

should be moved outside function body;

in OpenGl_CappingAlgo.hxx:
  static Handle(OpenGl_RenderFilter) myRenderFilter;
  static OpenGl_AspectFace myFrontCulling;
  static OpenGl_AspectFace myNoneCulling;
  static Standard_Boolean myIsInit;


in OpenGl_AspectFace.cxx:
  static OPENGL_SURF_PROP THE_DEFAULT_MATERIAL
  static TEL_POFFSET_PARAM THE_DEFAULT_POFFSET

seems to be constants;

in OpenGl_GraphicDriver_Layer.cxx:
static Standard_Boolean TheLayerIsOpen = Standard_False;
static OpenGl_LAYER_PROP TheLayerProp;

should be removed within #0025789.

in OpenGl_Trihedron.cxx:
static TEL_COLOUR theXColor = {{ 1.F, 0.F, 0.F, 0.6F }};
static TEL_COLOUR theYColor = {{ 0.F, 1.F, 0.F, 0.6F }};
static TEL_COLOUR theZColor = {{ 0.F, 0.F, 1.F, 0.6F }};
static float theRatio = 0.8f;
static float theDiameter = 0.05f;
static int   theNbFacettes = 12;

should be removed within #0025974.

in OpenGl_Workspace_Raytrace.cxx:
TCollection_AsciiString OpenGl_Workspace::ShaderSource::Source() const
{
  static const TCollection_AsciiString aVersion = "#version 140";

to be moved outside the function body;

in OpenGl_AVIWriter.hxx:
  static OpenGl_AVIWriter       * MyAVIWriterInstance;

should be removed within #0025382.

in OpenGl_ShaderProgram.hxx:
  static OpenGl_VariableSetterSelector mySetterSelector;


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

[revision removed]


Detailed log of new commits:

Author: isk
Date: Wed Apr 8 17:28:39 2015 +0300

    0023028: Visualization, TKOpenGl - eliminate global static variables
05Commenter 3
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: isk
Date: Fri Apr 10 13:28:45 2015 +0300

    0023028: Visualization, TKOpenGl - eliminate global static variables
06Commenter 3
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: isk
Date: Mon Apr 27 16:59:04 2015 +0300

    Remove static variables from OpenGl_CappingAlgo
07Author
-  static Handle(OpenGl_CappingAlgoFilter) aCappingFilter = new OpenGl_CappingAlgoFilter();
-  Handle(OpenGl_RenderFilter) aRenderFilter = theWorkspace->GetRenderFilter();
+  Handle(OpenGl_CappingAlgoFilter) aCappingFilter = new OpenGl_CappingAlgoFilter();
+  Handle(OpenGl_RenderFilter)      aRenderFilter  = theWorkspace->GetRenderFilter();
   theWorkspace->SetRenderFilter (aCappingFilter);
 
+  // create empty culling
+  OpenGl_AspectFace aCulling;

please preserve optimizations in new code; default filters and aspects can be stored in OpenGl_Workspace.
08Commenter 3
Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: isk
Date: Wed Apr 29 10:56:19 2015 +0300

    Default CappinAlgo filters and aspects are stored in OpenGl_Workspace.

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

[revision removed]


Detailed log of new commits:

Author: isk
Date: Wed Apr 29 15:08:39 2015 +0300

    Static variables in OpenGl_View_2.cxx is deleted.
    Default ambient and spot are stored in OpenGl_Workspace.

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

[revision removed]


Detailed log of new commits:

Author: isk
Date: Wed Apr 29 16:08:27 2015 +0300

    Cosmetics

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

[revision removed]


Detailed log of new commits:

Author: isk
Date: Mon Jun 8 10:17:01 2015 +0300

    0023028: Visualization, TKOpenGl - eliminate global static variables
12Commenter 12
Dear Kirill,
please review the branch CR23028_3.
13Commenter 3
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: isk
Date: Mon Jun 22 14:57:45 2015 +0300

    0025305: Visualization, TKOpenGl - support stipple line aspects within built-in GLSL programs
    
    OpenGl_LineAttributes - drop display lists for stipple lines.
    OpenGl_Context - add methods OpenGl_Context::SetTypeOfLine() and OpenGl_Context::SetLineWidth() to setup line aspects.
    OpenGl_ShaderManager::prepareStdProgramFlat() - support new bit OpenGl_PO_StippleLine.
    
    vaspects command - add -setlinetype option.
14Author
Please test the patch in branch CR23028_4.
15Commenter 3
Branch [archived branch] has been updated forcibly by Author.

[revision removed]
16Commenter 16
Dear Commenter 1,

Branch CR23028_4 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
[revision removed]

Number of compiler warnings:
occt component:
   Linux: 25 (25 on master)
   Windows: 0 (0 on master)
products component:
   Linux: 37 (37 on master)
   Windows: 0 (0 on master)

Regressions/Differences:
Not detected

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 97570995 / 97808123 [-0.24%]
Total CPU difference: 18207.119999999763 / 18376.230000000123 [-0.92%]

Testing on Windows:
Total MEMORY difference: 56913676 / 56863564 [+0.09%]
Total CPU difference: 17806.125741098847 / 17659.42240069884 [+0.83%]
17Commenter 3
Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: isk
Date: Mon Jun 22 15:01:37 2015 +0300

    0023028: Visualization, TKOpenGl - eliminate global static variables
    
    OpenGl_CappingAlgo - move global variables to OpenGl_Workspace class fields.
    OpenGl_View::ShaderSource::Source() - drop static for local variable.
    OpenGl_Window - drop redundant global constant THE_DEFAULT_BG_COLOR.
18Commenter 3
Branch [archived branch] has been deleted by Author.

[revision removed]
19Commenter 3
Branch [archived branch] has been deleted by Author.

[revision removed]
20Commenter 3
Branch [archived branch] has been deleted by Author.

[revision removed]
21Commenter 3
Branch [archived branch] has been deleted by Author.

[revision removed]
22Commenter 3
Branch [archived branch] has been deleted by Author.

[revision removed]
23Commenter 3
Branch [archived branch] has been deleted by Author.

[revision removed]

Related records