TKOpenGl - port ray-tracing from OpenCL to GLSL for better integration and portability

Dear kgv,

please review the path in branch CR24739_2.

Please take into account the following remarks:

-    IsReflectionsEnabled (1),
+    IsReflectionsEnabled (0),

-//#define RAY_TRACE_PRINT_INFO
+#define RAY_TRACE_PRINT_INFO

Unrelated changes?

-OpenGl_Flipper.cxx
+OpenGl_Flipper.cxx
\ No newline at end of file

Please avoid such changes.

+    if (aTriangleSet->BVH()->Depth() > myBottomLevelTreeDepth)
+    {
+      myBottomLevelTreeDepth = aTriangleSet->BVH()->Depth();
+    }

Max?

+  theCtx->core20->glBindAttribLocation (myProgramID, theIndex, theName);
+
+  return Standard_True;

Here and in other places - please prefer core20fwd.

+  myTraversalStackSize = 24;

Replace 24 with global constant and document why 24 is used by default.

+    //! Loads shader source from specified files.
+    void Load (const TCollection_AsciiString* theFileNames, const Standard_Integer theCount);

Sequence of TCollection_HAsciiString or NULL-terminated list?

   Standard_Boolean Raytrace (const Graphic3d_CView& theCView,
-              const int theSizeX, const int theSizeY, const Tint theToSwap);
+                             const Standard_Integer theSizeX,
+                             const Standard_Integer theSizeY,
+                             const Standard_Integer theToSwap);

theToSwap - Standard_Boolean?

+  Standard_Integer myUniformLocations[2][9];

magic numbers...

+  Handle(OpenGl_FrameBuffer) myRaytraceFBO2;
+
+  Handle(OpenGl_ShaderObject) LoadShader (const ShaderSource& theSource, GLenum theType);

Mixed declarations of functions and fields.

+        myView->TextureEnv()->Bind (myGlContext, GL_TEXTURE12);
+      aProgram->SetSampler (myGlContext, "uEnvironmentMapTexture", 12);

Enumeration instead of 12 and GL_TEXTURE12 (GL_TEXTURE0 + Samplers_EnvTexture)?

+  TCollection_AsciiString aFolder = OSD_Environment ("CSF_ShadersDirectory").Value();

Graphic3d_ShaderProgram::ShadersFolder().

+      const TCollection_ExtendedString aMessage = "Error: Failed to set vertex shader source";
+     
+      myGlContext->PushMessage (GL_DEBUG_SOURCE_APPLICATION_ARB,
+        GL_DEBUG_TYPE_ERROR_ARB, 0, GL_DEBUG_SEVERITY_HIGH_ARB, aMessage);

Here and in other places - "Error: " prefix looks redundant here since it is created by PushMessage().

+  if (!myRaytraceProgram.IsNull())
+    myRaytraceProgram->Release (myGlContext.operator->());

Consider creation of auxiliary function to improve readability and robustness.

inline void NullifyResource (Handle(OpenGl_Resource)&      theResource,
                             const Handle(OpenGl_Context)& theCtx)
{
  if (!theResource.IsNull())
  {
    theResource->Release (theCtx.operator->());
    theResource.Nullify();
  }
}

+  if (aBVH->NodeInfoBuffer().size() > 0)

!empty()

+  if (aTotalBVHNodesNb > 0)
+  {

!= 0

+  glDrawArrays (GL_TRIANGLES, 0, 6);

core15fwd

+  NCollection_Mat4<GLdouble> aOrientationMatrix;
+  NCollection_Mat4<GLdouble> aViewMappingMatrix;
+  NCollection_Mat4<GLdouble> aOrientationInvers;

Nit: aOrientationMatrix, aViewMappingMatrix, aOrientationInvers;

+  static const TCollection_AsciiString aVersion = "#version 130";

Are there particular features used from GLSL130?

+  if (!myGlContext->IsGlGreaterEqual (3, 0))

If GLSL130 is mandatory, than we might consider version OpenGL3.1 as minimum (required for TBO) - otherwise OpenGl2.1 + arbTBO. OpenGL3.0 does not make much sense in real world.

CSF_OPENCL should be removed from OpenGl/EXTERNLIB and TKOpenGl/EXTERNLIB.

Dear kgv,

please review updated patch in branch CR24739_5.

Please test patch in branch CR24739.

Dear kgv,
could you please rebase branch CR24739, there are conflict files.

Please test re-based patch in branch CR24739_7.

Dear Commenter 2,

Branch CR24739_7 (and products from GIT master) was compiled on Linux and Windows platforms and tested.
[revision removed]

Number of compiler warnings:

occt component :
Linux: 21 (27 on master)
Windows: 0 (0 on master)

products component :
Linux: 12 (12 on master)
Windows: 2 (2 on master)

Regressions/Differences:
No regressions/differences

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 393715464 / 393242136
Total CPU difference: 48697.61999999997 / 57189.309999999896

Testing on Windows:
Total MEMORY difference: 420961968 / 414274688
Total CPU difference: 35780.953125 / 42798.984375

There are no differences in images found by testdiff.

There are compilation errors on MacOS:

In file included from [user path removed]/tools/WOK670/wok_entities/LOC/dev/IR-2014-04-03-IR-2014-04-03-occt/adm/cmake/TKMath/../../../inc/BVH_BinnedBuilder.hxx:67:
[user path removed]/tools/WOK670/wok_entities/LOC/dev/IR-2014-04-03-IR-2014-04-03-occt/adm/cmake/TKMath/../../../inc/BVH_BinnedBuilder.lxx:289:5: error: use of undeclared identifier 'UpdateDepth'
UpdateDepth (theBVH, theBVH->Level (aChildIndex));
^
this->
[user path removed]/tools/WOK670/wok_entities/LOC/dev/IR-2014-04-03-IR-2014-04-03-occt/src/BVH/BVH.cxx:80:16: note: in instantiation of member function 'BVH_BinnedBuilder<double, 2, 32>::BuildNode' requested here
template class BVH_BinnedBuilder<Standard_Real, 2>;
^
[user path removed]/tools/WOK670/wok_entities/LOC/dev/IR-2014-04-03-IR-2014-04-03-occt/adm/cmake/TKMath/../../../inc/BVH_Builder.hxx:57:8: note: must qualify identifier to find this declaration in dependent base class
void UpdateDepth (BVH_Tree<T, N>* theBVH, const Standard_Integer theLevel)
^
In file included from [user path removed]/tools/WOK670/wok_entities/LOC/dev/IR-2014-04-03-IR-2014-04-03-occt/src/BVH/BVH.cxx:19:
In file included from [user path removed]/tools/WOK670/wok_entities/LOC/dev/IR-2014-04-03-IR-2014-04-03-occt/adm/cmake/TKMath/../../../inc/BVH_SweepPlaneBuilder.hxx:43:
[user path removed]/tools/WOK670/wok_entities/LOC/dev/IR-2014-04-03-IR-2014-04-03-occt/adm/cmake/TKMath/../../../inc/BVH_SweepPlaneBuilder.lxx:188:5: error: use of undeclared identifier 'UpdateDepth'
UpdateDepth (theBVH, theBVH->Level (aChildIndex));

CR24739_7

Dear Commenter 2,

Please set patch status to 'tested'.