GL backend: updated comments

9 files changed, 91 insertions, 11 deletions

diff --git a/Graphics/GraphicsEngineOpenGL/include/GLPipelineResourceLayout.h b/Graphics/GraphicsEngineOpenGL/include/GLPipelineResourceLayout.h
index 31d3d9b5..6db22447 100644
--- a/Graphics/GraphicsEngineOpenGL/include/GLPipelineResourceLayout.h
+++ b/Graphics/GraphicsEngineOpenGL/include/GLPipelineResourceLayout.h
@@ -23,6 +23,46 @@
 
 #pragma once
 
+// GLPipelineResourceLayout class manages resource bindings for all stages in a pipeline
+
+//
+//   
+//                                                      To            program              resource                  cache
+//
+//                                               A          A                  A        A              A           A              A            A
+//                                               |          |                  |        |              |           |              |            |
+//                                            Binding    Binding            Binding   Binding       Binding     Binding        Binding      Binding
+//      ___________________                  ____|__________|__________________|________|______________|___________|______________|____________|____________
+//     |                   |                |          |          |       |        |        |       |        |        |       |          |          |       |
+//     |GLProgramResources |--------------->|   UB[0]  |   UB[1]  |  ...  | Sam[0] | Sam[1] |  ...  | Img[0] | Img[1] |  ...  | SSBOs[0] | SSBOs[1] |  ...  |
+//     |___________________|                |__________|__________|_______|________|________|_______|________|________|_______|__________|__________|_______|
+//                                                A                                    A                        A                            A
+//                                                |                                    |                        |                            |
+//                                               Ref                                  Ref                      Ref                          Ref
+//    .-==========================-.         _____|____________________________________|________________________|____________________________|______________
+//    ||                          ||        |           |           |       |            |            |       |            |         |           |          |
+//  __|| GLPipelineResourceLayout ||------->| UBInfo[0] | UBInfo[1] |  ...  | SamInfo[0] | SamInfo[1] |  ...  | ImgInfo[0] |   ...   |  SSBO[0]  |   ...    |
+// |  ||                          ||        |___________|___________|_______|____________|____________|_______|____________|_________|___________|__________|
+// |  '-==========================-'                     /                                         \
+// |                                                   Ref                                         Ref
+// |                                                  /                                              \
+// |    ___________________                  ________V________________________________________________V_____________________________________________________
+// |   |                   |                |          |          |       |        |        |       |        |        |       |          |          |       |
+// |   |GLProgramResources |--------------->|   UB[0]  |   UB[1]  |  ...  | Sam[0] | Sam[1] |  ...  | Img[0] | Img[1] |  ...  | SSBOs[0] | SSBOs[1] |  ...  |
+// |   |___________________|                |__________|__________|_______|________|________|_______|________|________|_______|__________|__________|_______|
+// |                                             |           |                |         |                |        |                |           |
+// |                                          Binding     Binding          Binding    Binding          Binding  Binding         Binding      Binding
+// |                                             |           |                |         |                |        |                |           |
+// |    _______________________              ____V___________V________________V_________V________________V________V________________V___________V_____________
+// |   |                       |            |                           |                           |                           |                           |
+// '-->|GLProgramResourceCache |----------->|      Uinform Buffers      |          Samplers         |          Images           |       Storge Buffers      |
+//     |_______________________|            |___________________________|___________________________|___________________________|___________________________|
+//
+//
+// Note that GLProgramResources are kept by PipelineStateGLImpl. GLPipelineResourceLayout class is either part of the same PSO class,
+// or part of ShaderResourceBindingGLImpl object that keeps a strong reference to the pipeline. So all references from GLVariableBase
+// are always valid.
+
 #include <array>
 
 #include "Object.h"
diff --git a/Graphics/GraphicsEngineOpenGL/include/GLProgramResourceCache.h b/Graphics/GraphicsEngineOpenGL/include/GLProgramResourceCache.h
index 216292fd..08572f6c 100644
--- a/Graphics/GraphicsEngineOpenGL/include/GLProgramResourceCache.h
+++ b/Graphics/GraphicsEngineOpenGL/include/GLProgramResourceCache.h
@@ -33,7 +33,7 @@ namespace Diligent
 /// The class implements a cache that holds resources bound to a specific GL program
 // All resources are stored in the continuous memory using the following layout:
 //
-//   |        Cached UBs        |     Cached Samplers     |       Cached Images      | Cached Strg Blocks        |
+//   |        Cached UBs        |     Cached Samplers     |       Cached Images      | Cached Storage Blocks     |
 //   |----------------------------------------------------|--------------------------|---------------------------|
 //   |  0 | 1 | ... | UBCount-1 | 0 | 1 | ...| SmpCount-1 | 0 | 1 | ... | ImgCount-1 | 0 | 1 |  ... | SBOCount-1 |
 //    -----------------------------------------------------------------------------------------------------------
@@ -61,7 +61,7 @@ public:
     /// Describes a resource bound to a sampler or an image slot
     struct CachedResourceView
     {
-        /// Wee keep strong reference to the view instead of the reference
+        /// We keep strong reference to the view instead of the reference
         /// to the texture or buffer because this is more efficient from
         /// performance point of view: this avoids one pair of 
         /// AddStrongRef()/ReleaseStrongRef(). The view holds a strong reference 
diff --git a/Graphics/GraphicsEngineOpenGL/include/GLProgramResources.h b/Graphics/GraphicsEngineOpenGL/include/GLProgramResources.h
index 6ef48f96..19947e96 100644
--- a/Graphics/GraphicsEngineOpenGL/include/GLProgramResources.h
+++ b/Graphics/GraphicsEngineOpenGL/include/GLProgramResources.h
@@ -23,6 +23,18 @@
 
 #pragma once
 
+// GLProgramResources class allocates single continuous chunk of memory to store all program resources, as follows:
+//
+//
+//       m_UniformBuffers        m_Samplers                 m_Images                   m_StorageBlocks                      
+//        |                       |                          |                          |                         |                  |
+//        |  UB[0]  ... UB[Nu-1]  |  Sam[0]  ...  Sam[Ns-1]  |  Img[0]  ...  Img[Ni-1]  |  SB[0]  ...  SB[Nsb-1]  |  Resource Names  |
+//
+//  Nu  - number of uniform buffers
+//  Ns  - number of samplers
+//  Ni  - number of images
+//  Nsb - number of storage blocks
+
 #include <vector>
 
 #include "Object.h"
@@ -43,6 +55,7 @@ namespace Diligent
         GLProgramResources& operator = (const GLProgramResources&)  = delete;
         GLProgramResources& operator = (      GLProgramResources&&) = delete;
 
+        /// Loads program uniforms and assigns bindings
         void LoadUniforms(SHADER_TYPE                           ShaderStages,
                           const GLObjectWrappers::GLProgramObj& GLProgram,
                           class GLContextState&                 State,
@@ -361,9 +374,9 @@ namespace Diligent
                                    THandleSampler  HandleSampler,
                                    THandleImg      HandleImg,
                                    THandleSB       HandleSB,
-                                    const PipelineResourceLayoutDesc*    pResourceLayout = nullptr,
-                                    const SHADER_RESOURCE_VARIABLE_TYPE* AllowedVarTypes = nullptr,
-                                    Uint32                               NumAllowedTypes = 0)const
+                                   const PipelineResourceLayoutDesc*    pResourceLayout = nullptr,
+                                   const SHADER_RESOURCE_VARIABLE_TYPE* AllowedVarTypes = nullptr,
+                                   Uint32                               NumAllowedTypes = 0)const
         {
             const Uint32 AllowedTypeBits = GetAllowedTypeBits(AllowedVarTypes, NumAllowedTypes);
             auto CheckResourceType = [&](const char* Name)
diff --git a/Graphics/GraphicsEngineOpenGL/include/PipelineStateGLImpl.h b/Graphics/GraphicsEngineOpenGL/include/PipelineStateGLImpl.h
index d7dd7a96..6bf79f0c 100644
--- a/Graphics/GraphicsEngineOpenGL/include/PipelineStateGLImpl.h
+++ b/Graphics/GraphicsEngineOpenGL/include/PipelineStateGLImpl.h
@@ -78,15 +78,26 @@ private:
     GLObjectWrappers::GLPipelineObj& GetGLProgramPipeline(GLContext::NativeGLContextType Context);
     void InitStaticSamplersInResourceCache(const GLPipelineResourceLayout& ResourceLayout, GLProgramResourceCache& Cache)const;
 
+    // Linked GL programs for every shader stage. Every pipeline needs to have its own programs
+    // because resource bindings assigned by GLProgramResources::LoadUniforms depend on other
+    // shader stages.
     std::vector<GLObjectWrappers::GLProgramObj> m_GLPrograms;
 
     ThreadingTools::LockFlag m_ProgPipelineLockFlag;
     std::vector< std::pair<GLContext::NativeGLContextType, GLObjectWrappers::GLPipelineObj > > m_GLProgPipelines;
 
+    // Resource layout that keeps variables of all types, but does not reference a
+    // resource cache.
+    // This layout is used by SRB objects to initialize only mutable and dynamic variables and by
+    // DeviceContextGLImpl::BindProgramResources to verify resource bindings.
     GLPipelineResourceLayout m_ResourceLayout;
+
+    // Resource layout that only keeps static variables
     GLPipelineResourceLayout m_StaticResourceLayout;
+    // Resource cache for static resource variables only
     GLProgramResourceCache   m_StaticResourceCache;
 
+    // Program resources for all shader stages in the pipeline
     std::vector<GLProgramResources> m_ProgramResources;
 
     Uint32  m_TotalUniformBufferBindings = 0;   
diff --git a/Graphics/GraphicsEngineOpenGL/include/ShaderResourceBindingGLImpl.h b/Graphics/GraphicsEngineOpenGL/include/ShaderResourceBindingGLImpl.h
index 962e3bc6..e53d5109 100644
--- a/Graphics/GraphicsEngineOpenGL/include/ShaderResourceBindingGLImpl.h
+++ b/Graphics/GraphicsEngineOpenGL/include/ShaderResourceBindingGLImpl.h
@@ -67,8 +67,12 @@ public:
     const GLProgramResourceCache& GetResourceCache(PipelineStateGLImpl* pdbgPSO);
 
 private:
+    // The resource layout only references mutable and dynamic variables
     GLPipelineResourceLayout m_ResourceLayout;
+
+    // The resource cache holds resource bindings for all variables
     GLProgramResourceCache   m_ResourceCache;
+
     bool                     m_bIsStaticResourcesBound = false;
 };
 
diff --git a/Graphics/GraphicsEngineOpenGL/src/GLPipelineResourceLayout.cpp b/Graphics/GraphicsEngineOpenGL/src/GLPipelineResourceLayout.cpp
index adc7b3bc..e99f7921 100644
--- a/Graphics/GraphicsEngineOpenGL/src/GLPipelineResourceLayout.cpp
+++ b/Graphics/GraphicsEngineOpenGL/src/GLPipelineResourceLayout.cpp
@@ -350,6 +350,7 @@ void GLPipelineResourceLayout::StorageBufferBindInfo::BindResource(IDeviceObject
 #ifdef DEVELOPMENT
     {
         auto& CachedSSBO = ResourceCache.GetConstSSBO(m_Attribs.Binding + ArrayIndex);
+        // HLSL structured buffers are mapped to SSBOs in GLSL
         VerifyResourceViewBinding(m_Attribs, GetType(), ArrayIndex, pView, pViewGL.RawPtr(), {BUFFER_VIEW_SHADER_RESOURCE, BUFFER_VIEW_UNORDERED_ACCESS}, CachedSSBO.pBufferView.RawPtr());
     }
 #endif
diff --git a/Graphics/GraphicsEngineOpenGL/src/GLProgramResources.cpp b/Graphics/GraphicsEngineOpenGL/src/GLProgramResources.cpp
index 2ad23df1..d885aa77 100644
--- a/Graphics/GraphicsEngineOpenGL/src/GLProgramResources.cpp
+++ b/Graphics/GraphicsEngineOpenGL/src/GLProgramResources.cpp
@@ -198,6 +198,7 @@ void GLProgramResources::LoadUniforms(SHADER_TYPE                           Shad
                                       Uint32&                               ImageBinding,
                                       Uint32&                               StorageBufferBinding)
 {
+    // Load uniforms to temporary arrays. We will then pack all variables into a single chunk of memory.
     std::vector<UniformBufferInfo> UniformBlocks;
     std::vector<SamplerInfo>       Samplers;
     std::vector<ImageInfo>         Images;
@@ -421,15 +422,19 @@ void GLProgramResources::LoadUniforms(SHADER_TYPE                           Shad
                         {
                             LOG_WARNING_MESSAGE("Failed to set binding for image uniform '", Name.data(), "'[", arr_ind,
                                                 "]. Expected binding: ", ImageBinding, 
-                                                " Make sure that this binding is explicitly assigned in the shader source."
-                                                " Note that bindings are properly assigned by HLSL->GLSL converter.");
+                                                " Make sure that this binding is explicitly assigned in shader source code."
+                                                " Note that if the source code is converted from HLSL and if images are only used"
+                                                " by a single shader stage, then bindings automatically assigned by HLSL->GLSL"
+                                                " converter will work fine.");
                         }
                         else
                         {
                             LOG_WARNING_MESSAGE("Failed to set binding for image uniform '", Name.data(), "'."
                                                 " Expected binding: ", ImageBinding, 
-                                                " Make sure that this binding is explicitly assigned in the shader source."
-                                                " Note that bindings are properly assigned by HLSL->GLSL converter.");
+                                                " Make sure that this binding is explicitly assigned in shader source code."
+                                                " Note that if the source code is converted from HLSL and if images are only used"
+                                                " by a single shader stage, then bindings automatically assigned by HLSL->GLSL"
+                                                " converter will work fine.");
                         }
                     }
                 }
@@ -569,8 +574,10 @@ void GLProgramResources::LoadUniforms(SHADER_TYPE                           Shad
             LOG_WARNING_MESSAGE("glShaderStorageBlockBinding is not available on this device and "
                                 "the engine is unable to automatically assign shader storage block bindindg for '",
                                 Name.data(), "' variable. Expected binding: ", StorageBufferBinding, 
-                                " Make sure that this binding is explicitly assigned in the shader source."
-                                " Note that bindings are properly assigned by HLSL->GLSL converter.");
+                                " Make sure that this binding is explicitly assigned in shader source code."
+                                " Note that if the source code is converted from HLSL and if storage blocks are only used"
+                                " by a single shader stage, then bindings automatically assigned by HLSL->GLSL"
+                                " converter will work fine.");
         }
         ++StorageBufferBinding;
     }
diff --git a/Graphics/GraphicsEngineOpenGL/src/PipelineStateGLImpl.cpp b/Graphics/GraphicsEngineOpenGL/src/PipelineStateGLImpl.cpp
index 351cdffd..83b199f4 100644
--- a/Graphics/GraphicsEngineOpenGL/src/PipelineStateGLImpl.cpp
+++ b/Graphics/GraphicsEngineOpenGL/src/PipelineStateGLImpl.cpp
@@ -84,6 +84,7 @@ PipelineStateGLImpl::PipelineStateGLImpl(IReferenceCounters*      pRefCounters,
                 auto* pShaderGL = GetShader<ShaderGLImpl>(i);
                 const auto& ShaderDesc = pShaderGL->GetDesc();
                 m_GLPrograms.emplace_back(ShaderGLImpl::LinkProgram(&m_ppShaders[i], 1, true));
+                // Load uniforms and assign bindings
                 m_ProgramResources[i].LoadUniforms(ShaderDesc.ShaderType, m_GLPrograms[i], GLState,
                     m_TotalUniformBufferBindings,
                     m_TotalSamplerBindings,
@@ -112,6 +113,7 @@ PipelineStateGLImpl::PipelineStateGLImpl(IReferenceCounters*      pRefCounters,
             m_ShaderResourceLayoutHash = m_ProgramResources[0].GetHash();
         }
 
+        // Initialize master resource layout that keeps all variable types and does not reference a resource cache
         m_ResourceLayout.Initialize(m_ProgramResources.data(), static_cast<Uint32>(m_GLPrograms.size()), m_Desc.ResourceLayout, nullptr, 0, nullptr);
     }
    
@@ -122,6 +124,7 @@ PipelineStateGLImpl::PipelineStateGLImpl(IReferenceCounters*      pRefCounters,
     }
 
     {
+        // Clone only static variables into static resource layout, assign and initialize static resource cache
         const SHADER_RESOURCE_VARIABLE_TYPE StaticVars[] = {SHADER_RESOURCE_VARIABLE_TYPE_STATIC};
         m_StaticResourceLayout.Initialize(m_ProgramResources.data(), static_cast<Uint32>(m_GLPrograms.size()), m_Desc.ResourceLayout, StaticVars, _countof(StaticVars), &m_StaticResourceCache);
         InitStaticSamplersInResourceCache(m_StaticResourceLayout, m_StaticResourceCache);
diff --git a/Graphics/GraphicsEngineOpenGL/src/ShaderResourceBindingGLImpl.cpp b/Graphics/GraphicsEngineOpenGL/src/ShaderResourceBindingGLImpl.cpp
index 0236e282..da861c71 100644
--- a/Graphics/GraphicsEngineOpenGL/src/ShaderResourceBindingGLImpl.cpp
+++ b/Graphics/GraphicsEngineOpenGL/src/ShaderResourceBindingGLImpl.cpp
@@ -43,6 +43,7 @@ ShaderResourceBindingGLImpl::ShaderResourceBindingGLImpl(IReferenceCounters*
 {
     pPSO->InitializeSRBResourceCache(m_ResourceCache);
 
+    // Copy only mutable and dynamic variables from master resource layout
     const SHADER_RESOURCE_VARIABLE_TYPE SRBVarTypes[] = {SHADER_RESOURCE_VARIABLE_TYPE_MUTABLE, SHADER_RESOURCE_VARIABLE_TYPE_DYNAMIC};
     const auto& ResourceLayout = pPSO->GetDesc().ResourceLayout;
     m_ResourceLayout.Initialize(ProgramResources, NumPrograms, ResourceLayout, SRBVarTypes, _countof(SRBVarTypes), &m_ResourceCache);