Fixed memory leaks in the test + code cleanup

TheMostDiligent · TheMostDiligent · commit cf905998d2a1 · 2025-12-12T20:45:20.000-08:00
diff --git a/Graphics/ShaderTools/include/SPIRVShaderResources.hpp b/Graphics/ShaderTools/include/SPIRVShaderResources.hpp
@@ -1,5 +1,5 @@
 /*
- *  Copyright 2019-2024 Diligent Graphics LLC
+ *  Copyright 2019-2025 Diligent Graphics LLC
  *  Copyright 2015-2019 Egor Yusov
  *
  *  Licensed under the Apache License, Version 2.0 (the "License");
@@ -32,9 +32,19 @@
 
 // SPIRVShaderResources class uses continuous chunk of memory to store all resources, as follows:
 //
-//   m_MemoryBuffer                                                                                                                                                                  m_TotalResources
-//    |                                                                                                                                                                                 |                                       |
-//    | Uniform Buffers | Storage Buffers | Storage Images | Sampled Images | Atomic Counters | Separate Samplers | Separate Images | Input Attachments | Accel Structs | Push Constants |   Stage Inputs   |   Resource Names   |
+//                          _ _ _ m_MemoryBuffer
+//    | Uniform Buffers   |
+//    | Storage Buffers   |   |
+//    | Storage Images    |   V
+//    | Sampled Images    |
+//    | Atomic Counters   |
+//    | Separate Samplers |
+//    | Separate Images   |
+//    | Input Attachments |
+//    | Accel Structs     |
+//    | Push Constants    | _ _ _ m_TotalResources
+//    | Stage Inputs      |
+//    | Resource Names    |
 
 #include <memory>
 #include <vector>
@@ -79,7 +89,7 @@ struct SPIRVShaderResourceAttribs
         SeparateSampler,
         InputAttachment,
         AccelerationStructure,
-        PushConstant, // Vulkan push constant buffer - special inline constant that uses vkCmdPushConstants
+        PushConstant,
         NumResourceTypes
     };
 
diff --git a/Graphics/ShaderTools/src/SPIRVShaderResources.cpp b/Graphics/ShaderTools/src/SPIRVShaderResources.cpp
@@ -137,7 +137,7 @@ SPIRVShaderResourceAttribs::SPIRVShaderResourceAttribs(const char*  _Name,
                                                        Uint32       _BufferStaticSize) noexcept :
     // clang-format off
     Name                          {_Name},
-    // For push constants, ArraySize always be 1
+    // For push constants, ArraySize is always 1
     // This is consistent with how inline constants work in the pipeline resource signature
     ArraySize                     {1},
     Type                          {_Type},
@@ -217,8 +217,7 @@ PIPELINE_RESOURCE_FLAGS SPIRVShaderResourceAttribs::GetPipelineResourceFlags(Res
             return PIPELINE_RESOURCE_FLAG_COMBINED_SAMPLER;
 
         case SPIRVShaderResourceAttribs::ResourceType::PushConstant:
-            // Push constants are special inline constants that use vkCmdPushConstants
-            // Mark them with both INLINE_CONSTANTS and VULKAN_PUSH_CONSTANT flags
+            // Push constants map to constant buffer with the inline constants flag
             return PIPELINE_RESOURCE_FLAG_INLINE_CONSTANTS;
 
         default:
@@ -1078,8 +1077,8 @@ std::string SPIRVShaderResources::DumpResources() const
         },
         [&](const SPIRVShaderResourceAttribs& SepImg, Uint32) //
         {
-            VERIFY(SepImg.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateImage ||
-                       SepImg.Type == SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer,
+            VERIFY((SepImg.Type == SPIRVShaderResourceAttribs::ResourceType::SeparateImage ||
+                    SepImg.Type == SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer),
                    "Unexpected resource type");
             if (SepImg.Type == SPIRVShaderResourceAttribs::ResourceType::UniformTexelBuffer)
             {
diff --git a/Tests/DiligentCoreTest/CMakeLists.txt b/Tests/DiligentCoreTest/CMakeLists.txt
@@ -20,7 +20,7 @@ if(NOT WEBGPU_SUPPORTED)
     )
 endif()
 
-if(NOT VULKAN_SUPPORTED OR NOT TARGET glslang)
+if(NOT ${DILIGENT_USE_SPIRV_TOOLCHAIN} OR ${DILIGENT_NO_GLSLANG})
     list(REMOVE_ITEM SOURCE
         ${CMAKE_CURRENT_SOURCE_DIR}/src/ShaderTools/SPIRVShaderResourcesTest.cpp
     )
diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/InputAttachments.psh b/Tests/DiligentCoreTest/assets/shaders/SPIRV/InputAttachments.psh
@@ -11,4 +11,4 @@ float4 main() : SV_Target
 
     // Process the input color
     return color * 2.0;
-}
+}
diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/MixedResources.psh b/Tests/DiligentCoreTest/assets/shaders/SPIRV/MixedResources.psh
@@ -63,4 +63,4 @@ float4 main() : SV_Target
     color.rgb *= sin(g_PushConstants.Time) * 0.5 + 0.5;
 
     return color;
-}
+}
diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/PushConstants.psh b/Tests/DiligentCoreTest/assets/shaders/SPIRV/PushConstants.psh
@@ -23,4 +23,4 @@ float4 main() : SV_Target
     result.xy += g_PushConstants.g_Offset;
 
     return result;
-}
+}
diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/StorageBuffers.psh b/Tests/DiligentCoreTest/assets/shaders/SPIRV/StorageBuffers.psh
@@ -40,4 +40,4 @@ float4 main() : SV_Target
     g_RWAtomicBuffer.Store(0, atomicValue + 1);
 
     return result;
-}
+}
diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/StorageImages.psh b/Tests/DiligentCoreTest/assets/shaders/SPIRV/StorageImages.psh
@@ -17,4 +17,4 @@ float4 main() : SV_Target
     color += g_RWImage3D[uint3(0, 0, 0)];
 
     return color;
-}
+}
diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/TexelBuffers.psh b/Tests/DiligentCoreTest/assets/shaders/SPIRV/TexelBuffers.psh
@@ -12,4 +12,4 @@ float4 main() : SV_Target
 
     // Read back from storage texel buffer
     return g_StorageTexelBuffer[0];
-}
+}
diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/Textures.psh b/Tests/DiligentCoreTest/assets/shaders/SPIRV/Textures.psh
@@ -27,4 +27,4 @@ float4 main() : SV_Target
     float4 color6 = g_Texture.Sample(g_Texture_sampler, float2(0.5, 0.5));
 
     return color1 + color2 + color3 + color4 + color5 + color6;
-}
+}
diff --git a/Tests/DiligentCoreTest/assets/shaders/SPIRV/UniformBuffers.psh b/Tests/DiligentCoreTest/assets/shaders/SPIRV/UniformBuffers.psh
@@ -21,4 +21,4 @@ cbuffer CB2 : register(b2)
 float4 main() : SV_Target
 {
     return g_Color * g_MaterialParams;
-}
+}
diff --git a/Tests/DiligentCoreTest/src/ShaderTools/SPIRVShaderResourcesTest.cpp b/Tests/DiligentCoreTest/src/ShaderTools/SPIRVShaderResourcesTest.cpp
@@ -33,6 +33,7 @@
 
 #include <unordered_map>
 #include <string>
+#include <vector>
 
 #include "TestingEnvironment.hpp"
 #include "gtest/gtest.h"
@@ -43,23 +44,34 @@ using namespace Diligent::Testing;
 namespace
 {
 
+class SPIRVShaderResourcesTest : public ::testing::Test
+{
+protected:
+    static void SetUpTestSuite()
+    {
+        GLSLangUtils::InitializeGlslang();
+    }
+
+    static void TearDownTestSuite()
+    {
+        GLSLangUtils::FinalizeGlslang();
+    }
+};
+
+
 struct SPIRVShaderResourceRefAttribs
 {
     const char* const                              Name;
     const Uint16                                   ArraySize;
     const SPIRVShaderResourceAttribs::ResourceType Type;
-    const Uint8                                    ResourceDim; // RESOURCE_DIMENSION
+    const RESOURCE_DIMENSION                       ResourceDim;
     const Uint8                                    IsMS;
     const Uint32                                   BufferStaticSize;
     const Uint32                                   BufferStride;
 };
 
 std::vector<unsigned int> LoadSPIRVFromHLSL(const char* FilePath, SHADER_TYPE ShaderType = SHADER_TYPE_PIXEL)
 {
-    std::vector<unsigned int> SPIRV;
-
-#if !DILIGENT_NO_HLSL
-
     ShaderCreateInfo ShaderCI;
     ShaderCI.SourceLanguage = SHADER_SOURCE_LANGUAGE_HLSL;
     ShaderCI.FilePath       = FilePath;
@@ -73,19 +85,11 @@ std::vector<unsigned int> LoadSPIRVFromHLSL(const char* FilePath, SHADER_TYPE Sh
 
     ShaderCI.pShaderSourceStreamFactory = pShaderSourceStreamFactory;
 
-    SPIRV = GLSLangUtils::HLSLtoSPIRV(ShaderCI, GLSLangUtils::SpirvVersion::Vk100, nullptr, nullptr);
-
-#endif
-
-    return SPIRV;
+    return GLSLangUtils::HLSLtoSPIRV(ShaderCI, GLSLangUtils::SpirvVersion::Vk100, nullptr, nullptr);
 }
 
 std::vector<unsigned int> LoadSPIRVFromGLSL(const char* FilePath, SHADER_TYPE ShaderType = SHADER_TYPE_PIXEL)
 {
-    std::vector<unsigned int> SPIRV;
-
-#if !DILIGENT_NO_GLSLANG
-
     RefCntAutoPtr<IShaderSourceInputStreamFactory> pShaderSourceStreamFactory;
     CreateDefaultShaderSourceStreamFactory("shaders/SPIRV", &pShaderSourceStreamFactory);
     if (!pShaderSourceStreamFactory)
@@ -96,7 +100,7 @@ std::vector<unsigned int> LoadSPIRVFromGLSL(const char* FilePath, SHADER_TYPE Sh
     if (!pShaderSourceStream)
         return {};
 
-    auto ShaderSourceSize = pShaderSourceStream->GetSize();
+    size_t ShaderSourceSize = pShaderSourceStream->GetSize();
     if (ShaderSourceSize == 0)
         return {};
 
@@ -123,10 +127,7 @@ std::vector<unsigned int> LoadSPIRVFromGLSL(const char* FilePath, SHADER_TYPE Sh
     Attribs.Version                    = Version;
     Attribs.AssignBindings             = true;
 
-    SPIRV = GLSLangUtils::GLSLtoSPIRV(Attribs);
-
-#endif
-    return SPIRV;
+    return GLSLangUtils::GLSLtoSPIRV(Attribs);
 }
 
 void TestSPIRVResources(const char*                                       FilePath,
@@ -135,10 +136,6 @@ void TestSPIRVResources(const char*                                       FilePa
                         const char*                                       CombinedSamplerSuffix = nullptr,
                         bool                                              IsGLSL                = false)
 {
-#if DILIGENT_NO_GLSLANG || DILIGENT_NO_HLSL
-    GTEST_SKIP();
-#endif
-
     const auto SPIRV = IsGLSL ? LoadSPIRVFromGLSL(FilePath, ShaderType) : LoadSPIRVFromHLSL(FilePath, ShaderType);
     ASSERT_FALSE(SPIRV.empty()) << "Failed to compile HLSL to SPIRV: " << FilePath;
 
@@ -156,17 +153,12 @@ void TestSPIRVResources(const char*                                       FilePa
         false, // LoadUniformBufferReflection
         EntryPoint};
 
-    LOG_INFO_MESSAGE("Testing shader:", FilePath);
-    if (CombinedSamplerSuffix != nullptr)
-    {
-        LOG_INFO_MESSAGE("Using CombinedSamplerSuffix:", CombinedSamplerSuffix);
-    }
     LOG_INFO_MESSAGE("SPIRV Resources:\n", Resources.DumpResources());
 
     EXPECT_EQ(size_t{Resources.GetTotalResources()}, RefResources.size());
 
     std::unordered_map<std::string, const SPIRVShaderResourceRefAttribs*> RefResourcesMap;
-    for (const auto& RefRes : RefResources)
+    for (const SPIRVShaderResourceRefAttribs& RefRes : RefResources)
     {
         RefResourcesMap[RefRes.Name] = &RefRes;
     }
@@ -194,7 +186,7 @@ void TestSPIRVResources(const char*                                       FilePa
 
 using SPIRVResourceType = SPIRVShaderResourceAttribs::ResourceType;
 
-TEST(SPIRVShaderResources, UniformBuffers)
+TEST_F(SPIRVShaderResourcesTest, UniformBuffers)
 {
     TestSPIRVResources("UniformBuffers.psh",
                        {
@@ -204,7 +196,7 @@ TEST(SPIRVShaderResources, UniformBuffers)
                        });
 }
 
-TEST(SPIRVShaderResources, StorageBuffers)
+TEST_F(SPIRVShaderResourcesTest, StorageBuffers)
 {
     TestSPIRVResources("StorageBuffers.psh",
                        {
@@ -217,7 +209,7 @@ TEST(SPIRVShaderResources, StorageBuffers)
                        });
 }
 
-TEST(SPIRVShaderResources, TexelBuffers)
+TEST_F(SPIRVShaderResourcesTest, TexelBuffers)
 {
     TestSPIRVResources("TexelBuffers.psh",
                        {
@@ -226,7 +218,7 @@ TEST(SPIRVShaderResources, TexelBuffers)
                        });
 }
 
-TEST(SPIRVShaderResources, Textures)
+TEST_F(SPIRVShaderResourcesTest, Textures)
 {
     TestSPIRVResources("Textures.psh",
                        {
@@ -246,7 +238,7 @@ TEST(SPIRVShaderResources, Textures)
                        });
 }
 
-TEST(SPIRVShaderResources, StorageImages)
+TEST_F(SPIRVShaderResourcesTest, StorageImages)
 {
     TestSPIRVResources("StorageImages.psh",
                        {
@@ -257,7 +249,7 @@ TEST(SPIRVShaderResources, StorageImages)
                        });
 }
 
-TEST(SPIRVShaderResources, AtomicCounters)
+TEST_F(SPIRVShaderResourcesTest, AtomicCounters)
 {
     // Use GLSL for atomic counters. Note: Vulkan does not support atomic_uint (AtomicCounter storage class).
     // We use a storage buffer with atomic operations to simulate atomic counters.
@@ -272,15 +264,15 @@ TEST(SPIRVShaderResources, AtomicCounters)
                        true); // IsGLSL = true
 }
 
-TEST(SPIRVShaderResources, InputAttachments)
+TEST_F(SPIRVShaderResourcesTest, InputAttachments)
 {
     TestSPIRVResources("InputAttachments.psh",
                        {
                            SPIRVShaderResourceRefAttribs{"g_InputAttachment", 1, SPIRVResourceType::InputAttachment, RESOURCE_DIM_UNDEFINED, 0, 0, 0},
                        });
 }
 
-TEST(SPIRVShaderResources, AccelerationStructures)
+TEST_F(SPIRVShaderResourcesTest, AccelerationStructures)
 {
     // Use GLSL for acceleration structures since HLSLtoSPIRV doesn't support raytracing shaders
     // Acceleration structures are used in raytracing shaders, so we use SHADER_TYPE_RAY_GEN
@@ -294,7 +286,7 @@ TEST(SPIRVShaderResources, AccelerationStructures)
                        true); // IsGLSL = true
 }
 
-TEST(SPIRVShaderResources, PushConstants)
+TEST_F(SPIRVShaderResourcesTest, PushConstants)
 {
     // Push constant ArraySize represents the number of 32-bit words, not array elements
     // PushConstants struct: float4x4 (16 floats) + float4 (4 floats) + float2 (2 floats) + float (1 float) + uint (1 uint)
@@ -305,7 +297,7 @@ TEST(SPIRVShaderResources, PushConstants)
                        });
 }
 
-TEST(SPIRVShaderResources, MixedResources)
+TEST_F(SPIRVShaderResourcesTest, MixedResources)
 {
     TestSPIRVResources("MixedResources.psh",
                        {
@@ -325,4 +317,4 @@ TEST(SPIRVShaderResources, MixedResources)
                        });
 }
 
-} // namespace
+} // namespace

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ if(NOT WEBGPU_SUPPORTED)`
`20`	`20`	`)`
`21`	`21`	`endif()`
`22`	`22`
`23`		`-if(NOT VULKAN_SUPPORTED OR NOT TARGET glslang)`
	`23`	`+if(NOT ${DILIGENT_USE_SPIRV_TOOLCHAIN} OR ${DILIGENT_NO_GLSLANG})`
`24`	`24`	`list(REMOVE_ITEM SOURCE`
`25`	`25`	`${CMAKE_CURRENT_SOURCE_DIR}/src/ShaderTools/SPIRVShaderResourcesTest.cpp`
`26`	`26`	`)`