Visualization, TKOpenGl - support PBR Metallic-Roughness shading model

Useful threads related to light sources definition merged as KHR_lights_punctual extension to glTF 2.0:

https://github.com/KhronosGroup/glTF/blob/master/extensions/2.0/Khronos/KHR_lights_punctual/README.md
https://github.com/KhronosGroup/glTF/issues/945
https://github.com/KhronosGroup/glTF/pull/1223

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Jun 20 09:53:20 2019 +0300

    0030700: 0030700: Visualization, TKOpenGl - support PBR Metallic-Roughness shading model
    
    Metallic-Roughness shading model has been implemented.
    New materials descriptors has been added to standard materal library.
    Light sources intensities are supported now in pbr shading model.

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

[revision removed]

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Jun 20 15:17:11 2019 +0300

    # implement facet pbr shading model

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Fri Jun 21 16:31:41 2019 +0300

    # generating and header storage of environment LUT has been implemented

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

[revision removed]

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

[revision removed]

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

[revision removed]

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

[revision removed]

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Mon Sep 16 17:22:55 2019 +0300

    # implement PBREnvironment baking control to RenderParams

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Wed Sep 18 12:45:10 2019 +0300

    # add documentation
Author: Commenter 3
Date: Wed Sep 18 12:31:50 2019 +0300

    # remove 'uint' type from all PBR functions
Author: Commenter 3
Date: Wed Sep 18 11:57:35 2019 +0300

    # fix missing sampler parameters applying
Author: Commenter 3
Date: Tue Sep 17 17:37:50 2019 +0300

    # add warning messages
    # add basic test cases
    # fix crash during changing shading model to PBR first time

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

[revision removed]

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

[revision removed]

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Fri Sep 20 13:01:45 2019 +0300

    # fix compiler warnings

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

[revision removed]

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

[revision removed]

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Jun 20 09:53:20 2019 +0300

    0030700: Visualization, TKOpenGl - support PBR Metallic-Roughness shading model
    
    Metallic-Roughness shading model has been implemented.
    New materials descriptors has been added to standard materal library.
    Light sources intensities are supported now in pbr shading model.
    
    implement facet pbr shading model
    generating and header storage of environment LUT has been implemented
    implement PBREnvironment baking

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

[revision removed]

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

[revision removed]

+  //! in order to provide environment indirect illumination in PBR shading model (Image Based Lightning).

Lighting, as there no thunder around.

+  virtual void GenPBREnvironment() = 0;

It is better avoiding confusing abbreviations in API methods - GeneratePBREnvironment() or UpdatePBREnvironment().

+++ b/src/Graphic3d/Graphic3d_PBRMaterial.cxx
@@ -0,0 +1,346 @@
+// Author: Author
+// Copyright (c) 2016-2019 OPEN CASCADE SAS

Please check headers for new files.

+void Graphic3d_PBRMaterial::SetMetallic (Standard_ShortReal theMetallic)
+{
+  myMetallic = clamp (theMetallic);
+}

It would be better throwing Graphic3d_MaterialDefinitionError exception on wrong input.

+void Graphic3d_PBRMaterial::SetColor (const Quantity_Color& theColor)
+{
+  SetColor (Graphic3d_Vec3 (static_cast<Standard_ShortReal> (theColor.Red()),
+                            static_cast<Standard_ShortReal> (theColor.Green()),
+                            static_cast<Standard_ShortReal> (theColor.Blue())));
+}

Quantity_Color defines cast operator to Graphic3d_Vec3.

+void Graphic3d_PBRMaterial::SetBSDF (const Graphic3d_BSDF& theBSDF)
+{

Would be good having also a method performing conversion in opposite direction.

+Standard_Boolean Graphic3d_PBRMaterial::operator== (const Graphic3d_PBRMaterial &theOther) const
+{
+  return (myMetallic == theOther.myMetallic)

Better inlining trivial methods within Graphic3d_PBRMaterial.

+void Graphic3d_PBRMaterial::GenerateEnvLUT (const Handle(Image_PixMap) &theLUT,
+Graphic3d_Vec3 Graphic3d_PBRMaterial::importanceSample (const Graphic3d_Vec2 &theHammerslayPoint,
+Graphic3d_Vec3 Graphic3d_PBRMaterial::view (Standard_ShortReal theCosV)
+Graphic3d_Vec3 Graphic3d_PBRMaterial::reflect (const Graphic3d_Vec3 &theVector,
+                                               const Graphic3d_Vec3 &theAxis)

Unexpected &.
GenerateEnvironmentLUT?
Maybe internal methods intended for LUT generation like importanceSample() could be prefixed somehow [lutImportanceSample()]?

+      theLUT->ChangeValue<Graphic3d_Vec3>(theLUT->SizeX() - 1 - y, x) = aResult;

theLUT->SizeY()?

+    if ((theNumber >> i) != 0)
+    {
+      aPhi2 += ((theNumber >> i) & 1) / Standard_ShortReal(1 << (i + 1));
+    }
+    else
+    {
+      break;
+    }

Putting a 'break' first without 'else' would probably look more natural.

+//! Class implementing Metallic-Roughness physically based material system
+class Graphic3d_PBRMaterial

"material system" -> "material definition"?

+
+  static const Standard_ShortReal MIN_ROUGHNESS; //!< Roughness cannot be 0 in real calculations, so it is minimal achievable level of roughness in practice

Non-integer constants may cause problems sometimes.
Better defining as method.

+  Graphic3d_Vec4     myColor;     //!< base color with alpha component [0, 1]

As it called color and used as a color - please define it as Quantity_ColorRGBA.

+  Graphic3d_Vec3     myEmission;  //!< light intensity emitted by material [>= 0]
+  Standard_ShortReal myIOR;       //!< index of refraction [1, 3]
+  Graphic3d_Vec4     myColor;     //!< base color with alpha component [0, 1]

Is there some reason putting myEmission before the color?
I would reorder properties to put color first.

+  //! Returns base color with alpha component of material.
+  //! All 4 values are in range [0, 1].
+  //! (1, 1, 1, 1) by default.
+  const Graphic3d_Vec4& Color() const { return myColor; }

Might worth mentioning another alias of this property in description - albedo.

+  //! (1, 1, 1, 1) by default.
+  const Graphic3d_Vec4& Color() const { return myColor; }
...
+  myColor     (0.f, 0.f, 0.f, 1.f),

Description/implementation mismatch.
I would drop by default mentioning, as there is no meaningful default for material color - empty constructor description could be extended with initialization parameters instead.

   Standard_ShortReal                WhitePoint;                  //!< white point value used in filmic tone mapping (path tracing), 1.0 by default
 
+  Standard_Integer                  PBREnvPow2Size;              //!< size of IBL maps side can be calculated as 2^PBREnvPow2Size
+  Standard_Integer                  PBREnvSpecMapLevelsNumber;   //!< number of levels used in specular IBL map
+  Standard_Integer                  PBREnvBakingSamplesNumber;   //!< number of samples used in Monte-Carlo integration during PBR environment generation

Please put PBR rendering parameters before RayTracing options.

+  //! Physically based illumination system.
+  Graphic3d_TOSM_PBR,

Metalic-roughness physically based (PBR) illumination system.

+  //! Flat physically based shading.
+  Graphic3d_TOSM_PBR_FACET,

//! Same as Graphic3d_TOSM_PBR but using flat per-triangle normal.

+  Graphic3d_TextureUnit myPBRDiffIBLMapTexUnit; //!< texture unit where specular IBL map is expected to  be binded (0 if PBR is not supported)

"to be bound".

+    glGetIntegerv (GL_DRAW_FRAMEBUFFER_BINDING, &myFBO);

This would cause issues on Android as GL_DRAW_FRAMEBUFFER_BINDING has been introduced singe GLES3.

+  Standard_ShortReal           myClearColor[4];

Graphic3d_Vec4.

+  Handle(OpenGl_PBREnvironment) anEnvironment;
+
+  if (theCtx->arbFBO != NULL)
+  {

if (theCtx->arbFBO = NULL)
{
  return Handle(OpenGl_PBREnvironment)();
}

+  :
+  myPow2Size (std::max (1u, thePowOf2Size)),

I see some rationale for this syntax with : on dedicated line,
but it is uncommon to OCCT, where is usually placed on the same line with first field:
+: myPow2Size (std::max (1u, thePowOf2Size)),

+  if (theCtx != NULL)
+  {
+    theCtx->arbFBO->glDeleteFramebuffers (1, &myFBO);
+  }

myFBO is not checked for validness and not reset after deletion.

+  GLuint              myFBO;        //!< frame buffer object to generate or clear IBL maps

myFBO is not specified within field initialization list.

+  myIsComplete (Standard_True)
...
+  myIsComplete = myIsComplete && initVAO(theCtx);
+  myIsComplete = myIsComplete && initTextures (theCtx);
+  myIsComplete = myIsComplete && checkFBOComplentess (theCtx);

Weird style - it is better initializing myIsComplete=false and then setting it.

+void OpenGl_PBREnvironment::initFBO (const Handle(OpenGl_Context)& theCtx)
+{
+  theCtx->arbFBO->glGenFramebuffers (1, &myFBO);

Looks more like "createFBO()" and method lacks check if myFBO was already generated.

+// =======================================================================
+// function : processCubeMapSide
+// purpose  :
+// =======================================================================
+void OpenGl_PBREnvironment::processIBLMap (const Handle(OpenGl_Context)& theCtx,

Mismatch.

+  theCtx->ShaderManager()->BindPBREnvBakingProgram();
+  if (theCtx->ActiveProgram().IsNull())

Usually BindPBREnvBakingProgram() returns TRUE on success.

+  myVBO.BindAttribute (theCtx, Graphic3d_TOA_POS);
+  theCtx->arbFBO->glBindFramebuffer (GL_DRAW_FRAMEBUFFER, myFBO);

I would expect FBO being bound before VBOs.

+class OpenGl_PBREnvironment : public Standard_Transient
+{

OpenGl_NamedResource.

+  setParameter(theCtx, theSampler, theTarget, GL_TEXTURE_BASE_LEVEL, theParams->BaseLevel());
+  setParameter(theCtx, theSampler, theTarget, GL_TEXTURE_MAX_LEVEL,  theParams->MaxLevel());

if (IsGlGreaterEqual (1, 2))

 #include <OpenGl_Workspace.hxx>
+#include "../Textures/Textures_EnvLUT.pxx"
 
 #include <TCollection_ExtendedString.hxx>

Better moving after all "normal" includes.

+//! The same function as THE_FUNC_transformNormal but is used in PBR pipeline.
+//! The normals are expected to be in world coordinate system in PBR pipeline.
+const char THE_FUNC_PBR_transformNormal[] =

Please add coordinate system description to THE_FUNC_transformNormal.
And it might be better renaming variables to THE_FUNC_transformNormal_Model + THE_FUNC_transformNormal_World or similar instead of "PBR", which looks irrelevant.

+const char THE_FUNC_PBR_transformNormal[] =
+EOL"vec3 transformNormal (in vec3 theNormal)"
+EOL"{"

Indentation is inconsistent to other string constants in the file.

+const char THE_FUNC_PBR_lightDef[] =
+  EOL"vec3  DirectLightning;"  //!< Accumulator of direct lightning from light sources
+  EOL"vec4  BaseColor;"        //!< Base color (albedo) of material for PBR
+  EOL"float Metallic;"         //!< Metallic coefficient of material
+  EOL"float Roughness;"        //!< Roughness coefficient of material
+  EOL"float IOR;"              //!< Material's index of refraction
+  EOL"vec3  Emission;";        //!< Light intensity emitted by material

Please consider reordering variables taking into account pushing these constants into UBO in some future.

+  EOL"float geometry (in float theCosV,"
+  EOL"                in float theCosL,"
+  EOL"                in float theRoughness)"
+  EOL"{"
+  EOL"  float k = theRoughness + 1.0;"
+  EOL"  k *= 0.125 * k;"
+  EOL"  float g1 = 1.0;"
+  EOL"  g1 /= g1 * (1.0 - k) + k;"
+  EOL"  float g2 = 1.0;"
+  EOL"  g2 /= g2 * (1.0 - k) + k;"
+  EOL"  return g1 * g2;"
+  EOL"}";

Why not just
+ EOL" float k = theRoughness + 1.0;"
+ EOL" k *= 0.125 * k;"
+ EOL" float g1 = 1.0 / ((1.0 - k) + k);"
+ EOL" return g1 * g1;"
?

+  EOL""

"" is redundant (just EOL).

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Tue Sep 24 14:33:48 2019 +0300

    
    # implement 2-channel image type

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Tue Sep 24 14:40:41 2019 +0300

    # fix compiler warnings 2

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Tue Sep 24 15:00:24 2019 +0300

    # fix compiler warnings
    # delete 'lightning'

+//! Returns coordinates of point theNumber from hammersley point set having size theSize.

Hammersley, as it is a name.

+  theProgram->SetUniform (myContext, "occSpecIBLLevelsNumber", myLightSourceState.SpecIBLMapLevels());

Should be added to OpenGl_StateVariable.
And probably renamed occSpecIBLLevelsNumber -> occNbSpecIBLLevels.

+    }
+
+

Please avoid redundant double empty lines.

-    + aLightsLoop
-    + EOL"  vec4 aMatAmbient  = " + aGetMatAmbient
-    + EOL"  vec4 aMatDiffuse  = " + aGetMatDiffuse
-    + EOL"  vec4 aMatSpecular = theIsFront ? occFrontMaterial_Specular() : occBackMaterial_Specular();"
+      + aLightsLoop
+      + EOL"  vec4 aMatAmbient  = " + aGetMatAmbient
+      + EOL"  vec4 aMatDiffuse  = " + aGetMatDiffuse
+      + EOL"  vec4 aMatSpecular = theIsFront ? occFrontMaterial_Specular() : occBackMaterial_Specular();"

Unexpected change.

+      + THE_FUNC_lightDef
+      + aLightsFunc
+      + EOL
       EOL"vec4 computeLighting (in vec3 theNormal,"

Please adjust indentation.

+      EOL"  vec2 Coeff = occTexture2D (occEnvLUT, vec2(abs(dot(theView, theNormal)), Roughness)).xy;"

const vec2 aCoeff

+  const TCollection_AsciiString aProgId = TCollection_AsciiString (theIsFlatNormal ? "flat-" : "phong-") + (theIsPBR ? "PBR-" : "") + genLightKey (myLightSourceState.LightSources()) + "-";

Shader ids are lower-case by convention.

+  Handle(Graphic3d_ShaderProgram) aProgramSrc = new Graphic3d_ShaderProgram;

new Graphic3d_ShaderProgram()

+  TCollection_AsciiString aSrcFrag, aSrcVert;
+
+  aSrcVert += TCollection_AsciiString()
...
+
+  aSrcFrag += TCollection_AsciiString()
+  + THE_FUNC_PBR_hammersley

+= looks awkward here.

+Graphic3d_TypeOfShadingModel OpenGl_ShaderManager::PBRShadingModelFallback (Graphic3d_TypeOfShadingModel theShadingModel,
+                                                                            Standard_Boolean             thePBRIsAllowed

theIsPbrAllowed.
Please move to header.

+  //! Checks whether one of PBR shading models is set as default model.
+  Standard_Boolean PBRIsAllowed() const { return myShadingModel == Graphic3d_TOSM_PBR
+                                              || myShadingModel == Graphic3d_TOSM_PBR_FACET; }

IsPbrAllowed().

+  //! Sets number of mipmap levels used in specular IBL map.
+  void SetSpecIBLMapLevels (Standard_Integer theSpecIBLMapLevels) { mySpecIBLMapLevels = theSpecIBLMapLevels; }
+
+  //! Returns number of mipmap levels used in specular IBL map.
+  //! 0 by default or in case of using non-PBR shading model.
+  Standard_Integer SpecIBLMapLevels() const { return mySpecIBLMapLevels; }

Please declare getter first.

+  PBRColor    = theMat.PBRMaterial().Color();

Looks like color of "non-physical" material is ignored here.

+  aFile << "//this file generated by vgenenvlut draw command\n";

has been generated

+  OpenGl_Vec4 Emission;       //!< material emission
+  OpenGl_Vec4 Params;         //!< extra packed parameters
+  OpenGl_Vec4 PBRColor;       //!< base color of PBR material with alpha component
..
+vec4  occFrontMaterial_Specular(void);            //!< Specular reflection
+vec4  occFrontMaterial_PBRColor(void);            //!< Base color of PBR material

Please separate common/PBR material definition parameters passed to GLSL program:
- Split materials within OpenGl_Material in two groups (either as sub-structures, or as OpenGl_PBRMaterial).
- Add flag IsPBR() or similar to Graphic3d_ShaderProgram.
- Add THE_IS_PBR macros to Declarations.glsl managed by IsPBR().
- Use different variables for PBR within DeclarationsImpl.glsl (e.g. occPBRFrontMaterial or similar).
- The same might be applied to light sources, if not all obsolete parameters are used by PBR lighting.

+  //! Enum classified the type of IBL map
+  enum OpenGl_TypeOfIBLMap
+  {
+    OpenGl_TOIM_Diffuse,
+    OpenGl_TOIM_Specular
+  };
...
+  //! State of PBR environment.
+  enum PBREnvironmentState
+  {
+    OpenGl_PES_NONEXISTENT,
+    OpenGl_PES_UNAVAILABLE, // indicates failed try to create PBR environment
+    OpenGl_PES_CREATED
+  };
+
+  //! Type of action which can be done with PBR environment.
+  enum PBREnvironmentRequest
+  {
+    OpenGl_PER_NONE,
+    OpenGl_PER_BAKE,
+    OpenGl_PER_CLEAR
+  };

OpenGl_TOIM_ is an old CDL style for enumeration values, it is no more recommended (complicates association of OpenGl_TOIM_ values with enumeration name).
More convenient style is using full enumeration name as prefix, e.g. OpenGl_TypeOfIBLMap_Diffuse.

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Jun 20 09:53:20 2019 +0300

    0030700: Visualization, TKOpenGl - support PBR Metallic-Roughness shading model
    
    Metallic-Roughness shading model has been implemented.
    New materials descriptors has been added to standard materal library.
    Light sources intensities are supported now in pbr shading model.

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Fri Sep 27 12:03:37 2019 +0300

    # fix remarks

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

[revision removed]

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

[revision removed]

+      EOL"  anIndirectLightingSpec *= occTextureCubeLod (occSpecIBLMap, -reflect (theView, theNormal), Roughness * float (occNbSpecIBLLevels - 1)).rgb;"

textureCubeLod() with 'Lod' suffix is disallowed by specs in Fragment shader for versions before GLSL 130 and GLSL 300 es:
> The built-ins suffixed with “Lod” are allowed only in a vertex shader.

Either, the fallback not using occTextureCubeLod() in fragment shader should be provided, or minimal requirements for PBR renderer should be increased.

Please also extend vlistmaterials with PBR metalic-roughness material properties.

Building issues on Android platform:

C:\occt.git\src\OpenGl\OpenGl_FrameBuffer.cxx: In static member function 'static Standard_Boolean OpenGl_FrameBuffer::BufferDump(const opencascade::handle<OpenGl_Context>&, const opencascade::handle<OpenGl_FrameBuffer>&, Image_PixMap&, Graphic3d_BufferType)':
C:\occt.git\src\OpenGl\OpenGl_FrameBuffer.cxx:745:10: warning: enumeration value 'Image_Format_RGF' not handled in switch [-Wswitch]
   switch (theImage.Format())
          ^
C:\occt.git\src\OpenGl\OpenGl_Sampler.cxx: In static member function 'static void OpenGl_Sampler::applySamplerParams(const opencascade::handle<OpenGl_Context>&, const opencascade::handle<Graphic3d_TextureParams>&, OpenGl_Sampler*, GLenum, bool)':
C:\occt.git\src\OpenGl\OpenGl_Sampler.cxx:267:50: error: 'GL_TEXTURE_BASE_LEVEL' was not declared in this scope
     setParameter (theCtx, theSampler, theTarget, GL_TEXTURE_BASE_LEVEL, theParams->BaseLevel());
                                                  ^
C:\occt.git\src\OpenGl\OpenGl_Sampler.cxx:268:50: error: 'GL_TEXTURE_MAX_LEVEL' was not declared in this scope
     setParameter (theCtx, theSampler, theTarget, GL_TEXTURE_MAX_LEVEL,  theParams->MaxLevel());

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Sep 27 21:09:06 2019 +0300

    # corrections

+  GLint aLocation = theProgram->GetStateLocation (OpenGl_OCCT_NB_SPEC_IBL_LEVELS);
+  if (aLocation != OpenGl_ShaderProgram::INVALID_LOCATION)

For new code OpenGl_ShaderUniformLocation is preferred:
if (const OpenGl_ShaderUniformLocation aLocBack = theProgram->GetStateLocation (OpenGl_OCCT_NB_SPEC_IBL_LEVELS))

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Sep 27 21:51:13 2019 +0300

    OpenGl_PBREnvironment::Release() - fix crash on accessing invalid OpenGL context

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Sep 27 22:15:14 2019 +0300

    # fix material transparency loss

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Sep 27 22:50:43 2019 +0300

    # do not extend list of deprecated constants
    # fix texture internal format for Image_Format_RGF

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

[revision removed]

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Sat Sep 28 22:05:24 2019 +0300

    # OpenGl_PBREnvironment::Bind() - avoid non-symmetrical bind/unbind texture units

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Sat Sep 28 22:36:25 2019 +0300

    # OpenGl_PBREnvironment - remove redundant default argument theIsDrawAction value

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Sat Sep 28 23:35:03 2019 +0300

    # THE_FUNC_PBR_directionalLightFirst - remove unused variable aHalf

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

[revision removed]

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Tue Oct 1 17:51:53 2019 +0300

    # OpenGl_View::SetBackgroundCubeMap() - do not crash on NULL argument; added TODOs

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Tue Oct 1 18:05:49 2019 +0300

    # OpenGl_PBREnvironment - fix binding VBO without default VAO in Core Profile

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Wed Oct 2 09:27:51 2019 +0300

    # OpenGl_ShaderManager - fix cubemap shader compilation on Andreno due to implicit cast

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Wed Oct 2 09:51:12 2019 +0300

    # occLight_Intensity() - revert expansion of light sources array definition, pack intensity

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Wed Oct 2 10:54:02 2019 +0300

    # OpenGl_Material - separate Commong and PBR material definitions

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Wed Oct 2 16:57:59 2019 +0300

    # OpenGl_ShaderManager::defaultGlslVersion() - define minimal GLSL version for PBR
    # as OpenGL 3.0, OpenGL ES 3.0 or OpenGL 2.1 + GL_EXT_gpu_shader4

Author: Author
Date: Wed Oct 2 16:13:40 2019 +0300

    occLight_IsHeadlight() now returns bool instead of int.
    Avoid using lowp for enumerations.
    
    Workaround occLight_IsHeadlight() ignorance on Adreno 308
    caused by some GLSL optimizator bugs.

Author: Author
Date: Wed Oct 2 13:44:01 2019 +0300

    # THE_FUNC_PBR_directionalLightFirst - eliminate redundant duplicate

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Wed Oct 2 17:45:06 2019 +0300

    # OpenGl_FrameBuffer::BufferDump - suppress compiler warning concerning Image_Format_RGF

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Wed Oct 2 22:26:26 2019 +0300

    # fix LUT texture initialization on Android, where GL_RG32F is NOT texture-filterable format on most hardware.
    # GL_RG16F is used instead.

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Oct 3 15:32:08 2019 +0300

    # fix GLSL compilation errors on PBR env baking program

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Oct 3 20:28:53 2019 +0300

    # cosmetics

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Oct 3 21:18:46 2019 +0300

    # cosmetics

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Oct 3 21:38:57 2019 +0300

    OpenGl_Texture::EstimatedDataSize() - fixed estimation for Cubemap textures

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Oct 4 00:25:47 2019 +0300

    OpenGl_Sampler::applySamplerParams() - fixed uninitialized GL_TEXTURE_WRAP_R in case of GL_TEXTURE_CUBE_MAP target.

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Oct 4 13:22:41 2019 +0300

    # OpenGl_PBREnvironment - performance profiling

Adreno 512 - IBL baking hangs (application doesn't respond for some reason).
Times with added glFinish(), 1024 samples:

10-06 10:34:03.824 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 5 16x16) filled in 0.020619 s
10-06 10:34:03.839 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 5 16x16) filled in 0.013906 s
10-06 10:34:03.853 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 5 16x16) filled in 0.013695 s
10-06 10:34:03.866 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 5 16x16) filled in 0.012632 s
10-06 10:34:03.879 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 5 16x16) filled in 0.013041 s
10-06 10:34:03.892 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 5 16x16) filled in 0.012706 s
10-06 10:34:03.925 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 4 32x32) filled in 0.032434 s
10-06 10:34:03.958 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 4 32x32) filled in 0.032421 s
10-06 10:34:03.990 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 4 32x32) filled in 0.03109 s
10-06 10:34:04.015 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 4 32x32) filled in 0.024692 s
10-06 10:34:04.035 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 4 32x32) filled in 0.019536 s
10-06 10:34:04.055 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 4 32x32) filled in 0.019762 s
10-06 10:34:04.125 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 3 64x64) filled in 0.069585 s
10-06 10:34:04.170 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 3 64x64) filled in 0.044642 s
10-06 10:34:04.213 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 3 64x64) filled in 0.042468 s
10-06 10:34:04.255 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 3 64x64) filled in 0.042348 s
10-06 10:34:04.300 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 3 64x64) filled in 0.043973 s
10-06 10:34:04.347 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 3 64x64) filled in 0.04662 s
10-06 10:34:04.507 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 2 128x128) filled in 0.159907 s
10-06 10:34:04.667 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 2 128x128) filled in 0.158927 s
10-06 10:34:04.823 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 2 128x128) filled in 0.15533 s
10-06 10:34:04.981 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 2 128x128) filled in 0.158143 s
10-06 10:34:05.140 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 2 128x128) filled in 0.157671 s
10-06 10:34:05.299 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 2 128x128) filled in 0.158583 s
10-06 10:34:05.890 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 1 256x256) filled in 0.591154 s
10-06 10:34:06.485 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 1 256x256) filled in 0.594755 s
10-06 10:34:07.071 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 1 256x256) filled in 0.585593 s
10-06 10:34:07.212 W/MiuiPerfServiceClient( 3422): interceptAndQueuing:2521|com.android.systemui|286|286|unknown|null|StatusBar|21902967364767|Slow swap buffers|1
10-06 10:34:07.660 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 1 256x256) filled in 0.588466 s
10-06 10:34:08.249 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 1 256x256) filled in 0.588234 s
10-06 10:34:08.841 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 1 256x256) filled in 0.591177 s
10-06 10:34:09.197 D/PerfShielderService( 2317): com.android.systemui|StatusBar|461|21904143358257|461|6|1
10-06 10:34:09.198 W/MiuiPerfServiceClient( 3422): interceptAndQueuing:2521|com.android.systemui|461|461|unknown|null|StatusBar|21904143358257|Slow swap buffers|1
10-06 10:34:10.842 I/OpenGLRenderer( 2521): Davey! duration=1640ms; Flags=0, IntendedVsync=21905682908909, Vsync=21905682908909, OldestInputEvent=9223372036854775807, NewestInputEvent=0, HandleInputStart=21905683588100, AnimationStart=21905683628360, PerformTraversalsStart=21905683631641, DrawStart=21905683784923, SyncQueued=21905683817683, SyncStart=21905683907891, IssueDrawCommandsStart=21905684211537, SwapBuffers=21905688584141, FrameCompleted=21907323042266, DequeueBufferDuration=393000, QueueBufferDuration=1633728000, 
10-06 10:34:10.862 W/Adreno-GSL(11562): <gsl_ldd_control:548>: ioctl fd 55 code 0x400c0907 (IOCTL_KGSL_DEVICE_WAITTIMESTAMP_CTXTID) failed: errno 35 Resource deadlock would occur
10-06 10:34:10.862 W/Adreno-GSL(11562): <log_gpu_snapshot:457>: panel.gpuSnapshotPath is not set.not generating user snapshot
10-06 10:34:10.862 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 0 512x512) filled in 2.02112 s
10-06 10:34:10.863 W/Adreno-GSL(11562): <gsl_ldd_control:548>: ioctl fd 55 code 0xc040094a (IOCTL_KGSL_GPU_COMMAND) failed: errno 35 Resource deadlock would occur
10-06 10:34:10.863 W/Adreno-GSL(11562): <log_gpu_snapshot:457>: panel.gpuSnapshotPath is not set.not generating user snapshot
10-06 10:34:10.863 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 0 512x512) filled in 0.000637 s
10-06 10:34:10.864 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 0 512x512) filled in 0.000138 s
10-06 10:34:10.864 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 0 512x512) filled in 1.3e-05 s
10-06 10:34:10.864 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 0 512x512) filled in 1.1e-05 s
10-06 10:34:10.864 D/CadAssistant(11562): processIBLMap(SPECULAR, DRAW, 0 512x512) filled in 1.1e-05 s
10-06 10:34:10.864 D/CadAssistant(11562): processIBLMap(DIFFUSE, DRAW, 0 512x512) filled in 1.1e-05 s
10-06 10:34:10.865 D/CadAssistant(11562): processIBLMap(DIFFUSE, DRAW, 0 512x512) filled in 1.2e-05 s
10-06 10:34:10.865 D/CadAssistant(11562): processIBLMap(DIFFUSE, DRAW, 0 512x512) filled in 1.1e-05 s
10-06 10:34:10.865 D/CadAssistant(11562): processIBLMap(DIFFUSE, DRAW, 0 512x512) filled in 1.1e-05 s
10-06 10:34:10.866 D/CadAssistant(11562): processIBLMap(DIFFUSE, DRAW, 0 512x512) filled in 1.1e-05 s
10-06 10:34:10.866 D/CadAssistant(11562): IBL 512x512 is baked in 7.06202 s

Mali-G51 (application works), 1024 samples:

10-06 10:51:27.521 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 5 16x16) filled in 0.020527 s
10-06 10:51:27.536 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 5 16x16) filled in 0.014951 s
10-06 10:51:27.551 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 5 16x16) filled in 0.014991 s
10-06 10:51:27.565 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 5 16x16) filled in 0.012778 s
10-06 10:51:27.578 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 5 16x16) filled in 0.013232 s
10-06 10:51:27.593 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 5 16x16) filled in 0.013895 s
10-06 10:51:27.635 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 4 32x32) filled in 0.041807 s
10-06 10:51:27.678 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 4 32x32) filled in 0.042113 s
10-06 10:51:27.721 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 4 32x32) filled in 0.042033 s
10-06 10:51:27.759 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 4 32x32) filled in 0.037676 s
10-06 10:51:27.802 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 4 32x32) filled in 0.041821 s
10-06 10:51:27.838 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 4 32x32) filled in 0.03608 s
10-06 10:51:27.906 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 3 64x64) filled in 0.0673 s
10-06 10:51:27.974 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 3 64x64) filled in 0.067517 s
10-06 10:51:28.042 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 3 64x64) filled in 0.066943 s
10-06 10:51:28.114 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 3 64x64) filled in 0.064439 s
10-06 10:51:28.175 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 3 64x64) filled in 0.060736 s
10-06 10:51:28.236 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 3 64x64) filled in 0.059916 s
10-06 10:51:28.440 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 2 128x128) filled in 0.202953 s
10-06 10:51:28.600 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 2 128x128) filled in 0.159407 s
10-06 10:51:28.743 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 2 128x128) filled in 0.14278 s
10-06 10:51:29.185 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 2 128x128) filled in 0.154375 s
10-06 10:51:29.773 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 1 256x256) filled in 0.586699 s
10-06 10:51:30.367 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 1 256x256) filled in 0.593637 s
10-06 10:51:30.955 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 1 256x256) filled in 0.587042 s
10-06 10:51:31.549 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 1 256x256) filled in 0.593025 s
10-06 10:51:32.129 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 1 256x256) filled in 0.579944 s
10-06 10:51:32.718 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 1 256x256) filled in 0.588718 s
10-06 10:51:37.367 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 0 512x512) filled in 2.32203 s
10-06 10:51:44.333 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 0 512x512) filled in 2.31021 s
10-06 10:51:46.672 D/CadAssistant(24425): processIBLMap(SPECULAR, DRAW, 0 512x512) filled in 2.33801 s
10-06 10:51:48.673 D/CadAssistant(24425): processIBLMap(DIFFUSE, DRAW, 0 512x512) filled in 2.00075 s
10-06 10:51:50.691 D/CadAssistant(24425): processIBLMap(DIFFUSE, DRAW, 0 512x512) filled in 2.01699 s
10-06 10:51:52.712 D/CadAssistant(24425): processIBLMap(DIFFUSE, DRAW, 0 512x512) filled in 2.02012 s
10-06 10:51:54.718 D/CadAssistant(24425): processIBLMap(DIFFUSE, DRAW, 0 512x512) filled in 2.00565 s
10-06 10:51:56.733 D/CadAssistant(24425): processIBLMap(DIFFUSE, DRAW, 0 512x512) filled in 2.01491 s
10-06 10:51:58.744 D/CadAssistant(24425): processIBLMap(DIFFUSE, DRAW, 0 512x512) filled in 2.00954 s
10-06 10:51:58.744 D/CadAssistant(24425): IBL 512x512 is baked in 31.244 s

For comparison, it looks like three.js uses just 32 samples by default (1024 in OCCT within current patch):

    var PMREMGenerator = function ( sourceTexture, samplesPerLevel, resolution ) {
        this.resolution = ( resolution !== undefined ) ? resolution : 256; // NODE: 256 is currently hard coded in the glsl code for performance reasons
this.samplesPerLevel = ( samplesPerLevel !== undefined ) ? samplesPerLevel : 32;

but this is probably done with some hack in math and causes visual artifacts:
> By using the previous convolved maps,
> a lower number of samples are generally sufficient
>(right now 32, which gives okay results
> unless we see the reflection very carefully, or zoom in too much

Adreno 308 bakes 512 environment with 100 samples within 28 seconds.

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Oct 17 12:30:47 2019 +0300

    # spherical harmonics work in progress

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Fri Oct 18 15:44:35 2019 +0300

    # implement spherical harmonics for diffuse IBL map

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Tue Oct 22 13:12:24 2019 +0300

    # fix implicit casts in GLSL code bakeDiffuseMap()/bakeDiffuseSH()/bakeSpecularMap()/occDiffIBLMap()

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Tue Oct 22 16:22:37 2019 +0300

    # implement spherical harmonics for diffuse IBL map
    # fix bugs
    # improve documentation

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Jun 20 09:53:20 2019 +0300

    0030700: Visualization, TKOpenGl - support PBR Metallic-Roughness shading model
    
    Metallic-Roughness shading model has been implemented.
    New materials descriptors has been added to standard materal library.
    Light sources intensities are supported now in pbr shading model.

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

[revision removed]

+// function : SetRoughness
+// purpose  :
+// =======================================================================
+void Graphic3d_PBRMaterial::SetIOR (Standard_ShortReal theIOR)

Misprint.

+Graphic3d_PBRMaterial& Graphic3d_PBRMaterial::operator= (const Graphic3d_BSDF& theBSDF)
+{
+  SetBSDF (theBSDF);

I'm not sure this is really useful.
Explicit SetBSDF() call would be more straightforward and operator=() can be removed.

+// =======================================================================
+// function : GenerateEnvLUT
+// purpose  :
+// =======================================================================
+void Graphic3d_PBRMaterial::GenerateEnvironmentLUT (const Handle(Image_PixMap) &theLUT,
+                                                    unsigned int theIntegralSamplesNumber)

GenerateEnvLUT
const Handle(Image_PixMap)& theLUT
theNbIntegralSamples

+// =======================================================================
+// function : geometryFactor
+// purpose  :
+// =======================================================================
+Standard_ShortReal Graphic3d_PBRMaterial::lutgenGeometryFactor

comment

+Graphic3d_Vec3 Graphic3d_PBRMaterial::lutgenReflect (const Graphic3d_Vec3 &theVector,
+                                                     const Graphic3d_Vec3 &theAxis)
+{
+  return theAxis * theAxis.Dot(theVector) * 2.f - theVector;
+}
\ No newline at end of file
...
+#endif // _Graphic3d_PBRMaterial_HeaderFile
\ No newline at end of file
...
+#endif // _OpenGl_PBREnvironment_HeaderFile
\ No newline at end of file

Please add empty line at the end of files.

+  static Standard_ShortReal lutgenGeometryFactor (Standard_ShortReal theCosL,
+  static Graphic3d_Vec2 lutgenHammersley (
+  static Standard_ShortReal lutgenImportanceSampleCosTheta
+  static Graphic3d_Vec3 lutgenImportanceSample 
+  static Graphic3d_Vec3 lutgenView (Standard_ShortReal theCosV);
+  static Graphic3d_Vec3 lutgenReflec

'lutGen' prefix.

+  Standard_Integer                  PbrEnvPow2Size;              //!< size of IBL maps side can be calculated as 2^PbrEnvPow2Size (> 0)
+  Standard_Integer                  PbrEnvSpecMapNbLevels;       //!< number of levels used in specular IBL map (> 1)
+  Standard_Integer                  PbrEnvBakingDiffNbSamples;   //!< number of samples used in Monte-Carlo integration during diffuse IBL map's
+                                                                 //!  spherical harmonics coefficients generation (> 0)
+  Standard_Integer                  PbrEnvBakingSpecNbSamples;   //!< number of samples used in Monte-Carlo integration during specular IBL map's generation (> 0)
+  Standard_ShortReal                PbrEnvBakingProbability;

Default values should be in comment.

+    theCtx->core11fwd->glEnable (GL_SCISSOR_TEST);
+    theCtx->core11fwd->glScissor (0, 0, 1, 1);
+    glClear (GL_COLOR_BUFFER_BIT);
+
+    glClearColor (0.f, 0.f, 0.f, 1.f);
+    theCtx->core11fwd->glScissor (1, 0, 8, 1);
+    glClear (GL_COLOR_BUFFER_BIT);

It might be preferable to call glClear() first without scissor test (so that the driver will be aware, that full content is invalidated), and after that clear one pixel with scissor test.

+        glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
+      }
+      else
+      {
+        glClear(GL_COLOR_BUFFER_BIT);

Here and in other places - please prefer using aCtx->core11fwd instead of direct functions, even if they are supposed to be always available.

+  Standard_EXPORT virtual void Release (OpenGl_Context* theCtx = NULL) Standard_OVERRIDE;

Default parameter value is redundant.

+//! Calculates direct illumination using Cook-Torrance BRDF.
+const char THE_FUNC_PBR_illumination[] =
+  EOL"vec3 illumination (in vec3  theView,"

Please move new self-sustained non-trivial GLSL functions to Shaders resource folder with generation of .pxx.

+  GLint aLocation = theProgram->GetStateLocation (OpenGl_OCCT_NB_SPEC_IBL_LEVELS);
+  if (aLocation != OpenGl_ShaderProgram::INVALID_LOCATION)

Please use OpenGl_ShaderUniformLocation in new code, e.g.
> if (const OpenGl_ShaderUniformLocation aLocation = theProgram->GetStateLocation (OpenGl_OCCT_NB_SPEC_IBL_LEVELS))

+  Standard_EXPORT static Graphic3d_TypeOfShadingModel PBRShadingModelFallback (Graphic3d_TypeOfShadingModel theShadingModel,
+                                                                               Standard_Boolean             theIsPbrAllowed = Standard_False)
+  {

Unexpected Standard_EXPORT.

-  const OpenGl_Aspects* anAspectsBackup = myWorkspace->SetAspects (myCubeMapParams);
+  const OpenGl_Aspects* anAspectsBackup = myWorkspace->SetAspects(myCubeMapParams);
...
-  myWorkspace->SetAspects (anAspectsBackup);
+  myWorkspace->SetAspects(anAspectsBackup);

Unrelated.

+protected: //! @name methods related to PBR
+
+  //! Checks whether PBR is available.
+  Standard_Boolean checkPBRAvailability() const;
+
+  //! Generates IBL maps used in PBR pipeline.
+  //! If background cubemap is not set clears all IBL maps. 
+  void bakePBREnvironment (const Handle(OpenGl_Context)& theCtx);
+
+  //! Fills all IBL maps with white color.
+  //! So that environment lighting is considered to be constant
+  //! and is completely controls by ambient light sources.
+  void clearPBREnvironment (const Handle(OpenGl_Context)& theCtx);
+
+  //! Process requests to generate or to clear PBR environment.
+  void processPBREnvRequest (const Handle(OpenGl_Context)& theCtx);

Protected methods should be Standard_EXPORT'ed.

@@ -782,6 +784,61 @@ void OpenGl_View::redraw (const Graphic3d_Camera::Projection theProjection,
                           OpenGl_FrameBuffer*                theOitAccumFbo)
 {
   Handle(OpenGl_Context) aCtx = myWorkspace->GetGlContext();
+
+  // process PBR environment 
+  if (myShadingModel == Graphic3d_TOSM_PBR
+   || myShadingModel == Graphic3d_TOSM_PBR_FACET)
+  {
+    if (!myPBREnvironment.IsNull()
+     && myPBREnvironment->SizesAreDifferent (myRenderParams.PbrEnvPow2Size,
+                                             myRenderParams.PbrEnvSpecMapNbLevels))

OpenGl_View::Redraw() is currently used as a place for preparing global resources (FBOs, etc.).

@@ -310,6 +316,16 @@ const OpenGl_Aspects* OpenGl_Workspace::ApplyAspects()
     myGlContext->BindTextures (myEnvironmentTexture);
   }
 
+  if ((myView->myShadingModel == Graphic3d_TOSM_PBR
+    || myView->myShadingModel == Graphic3d_TOSM_PBR_FACET)
+   && !myView->myPBREnvironment.IsNull())
+  {
+    myView->myPBREnvironment->Bind (myGlContext);
+    Handle(OpenGl_Texture) anEnvLUT;
+    myGlContext->GetResource ("EnvLUT", anEnvLUT);
+    anEnvLUT->Bind (myGlContext);
+  }

If this happens at every ApplyAspects() call, then it should be optimized to avoid redundant re-biding of "EnvLUT" texture.

+vec3 occDiffIBLMap (in vec3 theNormal, in vec3 theSHCoeffs[9])
+{

This can be badly optimized by GLSL compiler (big theSHCoeffs array can be entirely copied within each call).

+//=============================================================================
+//function : GenPBREnvironment
+//purpose  :
+//=============================================================================
+void V3d_View::GeneratePBREnvironment (Standard_Boolean theToUpdate)

Comment.

+static std::string envlutWriteToFile (Standard_ShortReal theValue)

envLutWriteToFile

+  std::fstream aFile;
+
+  OSD_OpenStream (aFile, aFilePath, std::ios::out);

std::ofstream
std::ios::out | std::ios::binary

+  Handle(Image_PixMap) aPixMap = new Image_PixMap;

new Image_PixMap()

+  theCommands.Add("vgenenvlut",
+                  "vgenenvlut [-size size = 128]",

Description is missing.

+    std::cerr << "Error: 'vgenpbrenv' command does not have any arguments\n";
+    return 1;

std::cout
Shorter message would be enough "Syntax error: not enough arguments\n".

+  theCommands.Add("vgenpbrenv",
+    "forcible generates IBL maps used in PBR pipeline",
+    __FILE__, VGenPBREnvironment, group);
+  theCommands.Add("vclearpbrenv",
+    "forcible clears IBL maps used in PBR pipeline",
+    __FILE__, VClearPBREnvironment, group);

It is better merging commands into single one "vpbrenv" with options '-clear' and '-generate'.

Please extend test groups de_mesh/gltf_read and de_mesh/obj_read with making screenshots with PBR shading model turned ON.

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

[revision removed]

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Oct 24 00:39:43 2019 +0300

    # make screenshots in PBR shading mode within de_mesh/gltf_read and de_mesh/obj_read test groups

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Oct 24 02:18:29 2019 +0300

    # add test case displaying spheres grid with different metallic-roughness values

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

[revision removed]

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Oct 24 17:05:43 2019 +0300

    # fix remarks

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Oct 24 17:41:03 2019 +0300

    # fix remarks of remarks

There are errors in Ray-Tracing v3d/raytrace/textures:

Error: Failed to get 64-bit handle of OpenGL texture #1280

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Fri Oct 25 11:52:43 2019 +0300

    # fix bug with sampler parameter

> src/Textures/Textures_EnvLUT.pxx: 392 KB
Please compress the file a little bit:
- Replace 1 space between R and G with 0 space
- Replace 2 spaces between RG values with 1 space
- Remove 2 spaces at the beginning of each file.
The result would be 358 KB which is a little bit better without sacrificing noticeably the file content.

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

[revision removed]

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Fri Oct 25 15:45:13 2019 +0300

    # fix lightsources shader misprints
    # compress env lut

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Jun 20 09:53:20 2019 +0300

    0030700: Visualization, TKOpenGl - support PBR Metallic-Roughness shading model
    
    Metallic-Roughness shading model Graphic3d_TOSM_PBR has been implemented.
    New materials descriptors Graphic3d_PBRMaterial have been added to Graphic3d_MaterialAspect.
    PBR shading model requires OpenGL 3.0+ or OpenGL ES 3.0+ hardware.
    Environment cubemap is expected to be provided for realistic look of metallic materials.
    
    occLight_IsHeadlight() now returns bool instead of int.
    Avoid using lowp for enumerations to workaround occLight_IsHeadlight()
    ignorance on Adreno 308 caused by some GLSL optimizator bugs.
    
    OpenGl_Texture::EstimatedDataSize() - fixed estimation for Cubemap textures.
    OpenGl_Sampler::applySamplerParams() - fixed uninitialized GL_TEXTURE_WRAP_R in case of GL_TEXTURE_CUBE_MAP target.

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

[revision removed]

Please raise the patch.

Test case v3d/materials/bug24855 has been removed from patch - will be integrated within another issue.

Windows-32/64-vc9/vc10/vc11/vc12

Graphic3d_PBRMaterial.cxx:186, MSBuild, Priority: Normal
nonstandard extension used: enum 'Image_Format' used in qualified name

OpenGl_PBREnvironment.cxx:117, MSBuild, Priority: Normal
'OpenGl_PBREnvironmentSentry' : assignment operator could not be generated

OpenGl_View_Redraw.cxx:381, MSBuild, Priority: Normal
nonstandard extension used: enum 'Image_Format' used in qualified name

ViewerTest_OpenGlCommands.cxx:1439, MSBuild, Priority: Normal
nonstandard extension used: enum 'Image_Format' used in qualified name

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

[revision removed]

Warning should be fixed now.

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

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Tue Oct 29 17:40:28 2019 +0300

    # fix base color texture using in PBR shader

> # fix base color texture using in PBR shader
This commit produces white screenshots on Jenkins.

> Please also extend vlistmaterials with PBR metalic-roughness material properties.
This one still not taken into account.

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

[revision removed]

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Oct 31 15:58:44 2019 +0300

    # add PBR properties to vlistmaterials command

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Commenter 3
Date: Thu Jun 20 09:53:20 2019 +0300

    0030700: Visualization, TKOpenGl - support PBR Metallic-Roughness shading model
    
    Metallic-Roughness shading model Graphic3d_TOSM_PBR has been implemented.
    New materials descriptors Graphic3d_PBRMaterial have been added to Graphic3d_MaterialAspect.
    PBR shading model requires OpenGL 3.0+ or OpenGL ES 3.0+ hardware.
    Environment cubemap is expected to be provided for realistic look of metallic materials.
    
    occLight_IsHeadlight() now returns bool instead of int.
    Avoid using lowp for enumerations to workaround occLight_IsHeadlight()
    ignorance on Adreno 308 caused by some GLSL optimizator bugs.
    
    OpenGl_Texture::EstimatedDataSize() - fixed estimation for Cubemap textures.
    OpenGl_Sampler::applySamplerParams() - fixed uninitialized GL_TEXTURE_WRAP_R in case of GL_TEXTURE_CUBE_MAP target.

Please take CR30700_4 into IR.

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Oct 31 17:43:29 2019 +0300

    # XCAFDoc_VisMaterial::FillMaterialAspect() - fix undefined BSDF material

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

[revision removed]

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

[revision removed]

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Nov 1 10:48:40 2019 +0300

    # Graphic3d_PBRMaterial::RoughnessFromSpecular() - move Common->PBR math

Branch [archived branch] has been updated by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Fri Nov 1 10:52:51 2019 +0300

    # fix CLang warning

Branch [archived branch] has been deleted by Participant.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]