WIP

.github/workflows/validation.yml

@@ -2,9 +2,9 @@ name: Validation Build
 
 on:
   push:
-    branches:
-      - 'master'
-      - 'dev'
+#   branches:
+#     - 'master'
+#     - 'dev'
     paths-ignore:
       - '**.md'
       - '**.glsl'

CMakeLists.txt

@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.12 FATAL_ERROR)
+cmake_minimum_required(VERSION 3.13 FATAL_ERROR)
 
 include(CMakeDependentOption)
 
@@ -22,7 +22,8 @@ foreach(CONFIG_TYPE ${CMAKE_CONFIGURATION_TYPES})
   set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY_${CONFIG_TYPE} ${OUTPUT_DIR})
 endforeach()
 
-option(GATLING_BUILD_HDGATLING "Build the gatling hydra render delegate." ON)
+option(GTL_BUILD_HDGATLING "Build the gatling hydra render delegate." ON)
+option(GTL_VERBOSE "Enable verbose logging." OFF)
 
 if(${GATLING_BUILD_HDGATLING})
   find_package(USD REQUIRED HINTS ${USD_ROOT} NAMES pxr)

cmake/BuildConfig.cmake

@@ -22,3 +22,20 @@ set(CMAKE_BUILD_TYPE "${CMAKE_BUILD_TYPE}" CACHE STRING "Choose the type of buil
 
 # Append list of configs for multi-output generators like VS
 list(APPEND CMAKE_CONFIGURATION_TYPES "Develop")
+
+
+# TODO: define GTL_COMPILE_OPTIONS that is then manually set for each target
+# TODO: define GTL_LINK_OPTIONS -> target_link_options (CMake 3.13). e.g. warnings as errors?
+if(MSVC)
+  set(GTL_COMPILE_OPTIONS "")#"/Wall")
+  # Disable "not enough arguments for function-like macro invocation" warning caused
+  # by the transitive inclusion of boost headers and non-conformant MSVC preprocessor
+  # behaviour. See USD's msvcdefaults.cmake file for a detailed comment.
+  set(GTL_COMPILE_OPTIONS "${GTL_COMPILE_OPTIONS}" "/wd4003")
+else()
+  set(GTL_COMPILE_OPTIONS "-Wall" "-Wextra" "-Wformat-security")
+endif()
+
+# TODO: for headers, either use a #pragma OR in case of USD/pxr, add to the target additionally
+
+

extern/CMakeLists.txt

@@ -116,6 +116,7 @@ set(OPENEXR_INSTALL OFF)
 set(OPENEXR_INSTALL_TOOLS OFF)
 set(OPENEXR_INSTALL_EXAMPLES OFF)
 add_subdirectory(openexr)
+target_compile_options(OpenEXR PRIVATE "-Wno-#warnings")
 
 #
 # stb

src/cgpu/CMakeLists.txt

@@ -24,6 +24,12 @@ target_link_libraries(
     spirv-reflect-static
 )
 
+target_compile_options(
+  cgpu
+  PRIVATE
+    ${GTL_COMPILE_OPTIONS}
+)
+
 set_target_properties(
   cgpu
   PROPERTIES

src/cgpu/impl/Cgpu.cpp

@@ -27,18 +27,23 @@
 
 #include <volk.h>
 
+#include <gtl/gb/Fmt.h>
+#include <gtl/gb/Log.h>
+
 #ifdef __clang__
 #pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-variable"
+#pragma clang diagnostic ignored "-Wunused-parameter"
+#pragma clang diagnostic ignored "-Wmissing-field-initializers"
 // Silence nullability log spam on AppleClang
 #pragma clang diagnostic ignored "-Wnullability-completeness"
 #endif
 
-// Uncomment for VMA debug logging
-//#define VMA_DEBUG_LOG_FORMAT(format, ...) do { \
-//    fprintf(stderr, (format), __VA_ARGS__);    \
-//    fprintf(stderr, "\n");                     \
-//    GB_LOG((format), __VA_ARGS__);             \
-//  } while (false)
+#ifdef GTL_VERBOSE
+#define VMA_DEBUG_LOG_FORMAT(format, ...) do { \
+    GB_DEBUG_DYN("[VMA] {}", GB_FMT_SPRINTF((format), __VA_ARGS__)); \
+  } while (false)
+#endif
 
 #define VMA_IMPLEMENTATION
 #include <vk_mem_alloc.h>
@@ -373,6 +378,7 @@ namespace gtl
 
   /* API method implementation. */
 
+#ifndef NDEBUG
   static bool cgpuFindLayer(const char* name, size_t layerCount, VkLayerProperties* layers)
   {
     for (size_t i = 0; i < layerCount; i++)
@@ -384,6 +390,7 @@ namespace gtl
     }
     return false;
   }
+#endif
 
   static bool cgpuFindExtension(const char* name, size_t extensionCount, VkExtensionProperties* extensions)
   {
@@ -407,7 +414,7 @@ namespace gtl
       .pObjectName = name
     };
 
-    VkResult result = vkSetDebugUtilsObjectNameEXT(device, &info);
+    [[maybe_unused]] VkResult result = vkSetDebugUtilsObjectNameEXT(device, &info);
 
     assert(result == VK_SUCCESS);
   }
@@ -689,7 +696,7 @@ namespace gtl
     GbSmallVector<VkQueueFamilyProperties, 32> queueFamilies(queueFamilyCount);
     vkGetPhysicalDeviceQueueFamilyProperties(idevice->physicalDevice, &queueFamilyCount, queueFamilies.data());
 
-    int32_t queueFamilyIndex = -1;
+    uint32_t queueFamilyIndex = UINT32_MAX;
     for (uint32_t i = 0; i < queueFamilyCount; ++i)
     {
       const VkQueueFamilyProperties* queue_family = &queueFamilies[i];
@@ -699,7 +706,7 @@ namespace gtl
         queueFamilyIndex = i;
       }
     }
-    if (queueFamilyIndex == -1)
+    if (queueFamilyIndex == UINT32_MAX)
     {
       iinstance->ideviceStore.free(handle);
       CGPU_RETURN_ERROR("no suitable queue family");
@@ -945,7 +952,7 @@ namespace gtl
       .sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO,
       .pNext = nullptr,
       .flags = VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT,
-      .queueFamilyIndex = (uint32_t) queueFamilyIndex,
+      .queueFamilyIndex = queueFamilyIndex,
     };
 
     result = idevice->table.vkCreateCommandPool(
@@ -1941,7 +1948,7 @@ namespace gtl
 
     for (uint32_t i = 0; i < groups.size(); i++)
     {
-      VkRayTracingShaderGroupCreateInfoKHR createInfo = {
+      VkRayTracingShaderGroupCreateInfoKHR sgCreateInfo = {
         .sType = VK_STRUCTURE_TYPE_RAY_TRACING_SHADER_GROUP_CREATE_INFO_KHR,
         .pNext = nullptr,
         .type = VK_RAY_TRACING_SHADER_GROUP_TYPE_GENERAL_KHR,
@@ -1951,7 +1958,8 @@ namespace gtl
         .intersectionShader = VK_SHADER_UNUSED_KHR,
         .pShaderGroupCaptureReplayHandle = nullptr,
       };
-      groups[i] = createInfo;
+
+      groups[i] = sgCreateInfo;
     }
 
     bool anyNullClosestHitShader = false;
@@ -2257,7 +2265,7 @@ namespace gtl
       .pNext = nullptr,
       .geometryType = VK_GEOMETRY_TYPE_TRIANGLES_KHR,
       .geometry = asGeomData,
-      .flags = createInfo.isOpaque ? VK_GEOMETRY_OPAQUE_BIT_KHR : VkGeometryFlagsKHR(0),
+      .flags = VkGeometryFlagsKHR(createInfo.isOpaque ? VK_GEOMETRY_OPAQUE_BIT_KHR : 0)
     };
 
     bool creationSuccessul = cgpuCreateTopOrBottomAs(device, VK_ACCELERATION_STRUCTURE_TYPE_BOTTOM_LEVEL_KHR,
@@ -2366,7 +2374,7 @@ namespace gtl
           }
         },
       },
-      .flags = areAllBlasOpaque ? VK_GEOMETRY_OPAQUE_BIT_KHR : VkGeometryFlagsKHR(0),
+      .flags = VkGeometryFlagsKHR(areAllBlasOpaque ? VK_GEOMETRY_OPAQUE_BIT_KHR : 0)
     };
 
     if (!cgpuCreateTopOrBottomAs(device, VK_ACCELERATION_STRUCTURE_TYPE_TOP_LEVEL_KHR, &asGeom, createInfo.instanceCount, &itlas->buffer, &itlas->as))
@@ -3322,6 +3330,7 @@ namespace gtl
     VkSubmitInfo2KHR submitInfo = {
       .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO_2_KHR,
       .pNext = nullptr,
+      .flags = 0,
       .waitSemaphoreInfoCount = (uint32_t) waitSubmitInfos.size(),
       .pWaitSemaphoreInfos = waitSubmitInfos.data(),
       .commandBufferInfoCount = 1,

src/gb/CMakeLists.txt

@@ -26,5 +26,15 @@ target_link_libraries(
     quill
 )
 
+target_compile_options(
+  gb
+  PRIVATE
+    ${GTL_COMPILE_OPTIONS}
+)
+
+if(GTL_VERBOSE)
+  target_compile_definitions(gb PUBLIC GTL_VERBOSE=1)
+endif()
+
 # Required since library is linked into hdGatling DSO
 set_target_properties(gb PROPERTIES POSITION_INDEPENDENT_CODE ON)

src/gb/gtl/gb/Fmt.h

@@ -18,5 +18,7 @@
 #pragma once
 
 #include <quill/bundled/fmt/format.h>
+#include <quill/bundled/fmt/printf.h>
 
 #define GB_FMT(fmt, ...) fmtquill::format(fmt, __VA_ARGS__)
+#define GB_FMT_SPRINTF(fmt, ...) fmtquill::sprintf(fmt, __VA_ARGS__)

src/gb/gtl/gb/Log.h

@@ -26,9 +26,11 @@
 #include <quill/std/UnorderedSet.h>
 #include <quill/std/Vector.h>
 
-#define GB_LOG(fmt, ...) QUILL_LOG_INFO(gtl::gbGetLogger(), fmt, ##__VA_ARGS__)
-#define GB_WARN(fmt, ...) QUILL_LOG_WARNING(gtl::gbGetLogger(), fmt, ##__VA_ARGS__)
 #define GB_ERROR(fmt, ...) QUILL_LOG_ERROR(gtl::gbGetLogger(), fmt, ##__VA_ARGS__)
+#define GB_WARN(fmt, ...) QUILL_LOG_WARNING(gtl::gbGetLogger(), fmt, ##__VA_ARGS__)
+#define GB_LOG(fmt, ...) QUILL_LOG_INFO(gtl::gbGetLogger(), fmt, ##__VA_ARGS__)
+#define GB_DEBUG(fmt, ...) QUILL_LOG_DEBUG(gtl::gbGetLogger(), fmt, ##__VA_ARGS__)
+#define GB_DEBUG_DYN(fmt, ...) QUILL_LOG_DYNAMIC(gtl::gbGetLogger(), quill::LogLevel::Debug, fmt, ##__VA_ARGS__)
 
 namespace gtl
 {

src/gb/impl/Log.cpp

@@ -39,6 +39,9 @@ namespace gtl
     const char* logPattern = "[%(time)] (%(log_level)) %(message)";
     const char* timestampFormat = "%H:%M:%S.%Qms";
     s_logger = quill::Frontend::create_or_get_logger("root", std::move(sink), logPattern, timestampFormat);
+#ifdef GTL_VERBOSE
+    s_logger->set_log_level(quill::LogLevel::Debug);
+#endif
 
     quill::BackendOptions options;
     options.thread_name = "GbLog";

src/ggpu/CMakeLists.txt

@@ -30,6 +30,12 @@ target_link_libraries(
     cgpu
 )
 
+target_compile_options(
+  ggpu
+  PRIVATE
+    ${GTL_COMPILE_OPTIONS}
+)
+
 set_target_properties(
   ggpu
   PROPERTIES

src/ggpu/gtl/ggpu/LinearDataStore.h

@@ -85,6 +85,5 @@ namespace gtl
 
     GbHandleStore m_handleStore;
     GgpuSyncBuffer m_buffer;
-    uint8_t* m_mappedMem = nullptr;
   };
 }

src/ggpu/gtl/ggpu/SyncBuffer.h

@@ -77,7 +77,6 @@ namespace gtl
   private:
     CgpuDevice m_device;
     GgpuStager& m_stager;
-    GgpuDelayedResourceDestroyer& m_delayedResourceDestroyer;
     uint64_t m_elementSize;
     UpdateStrategy m_updateStrategy;
 

src/ggpu/impl/LinearDataStore.cpp

@@ -20,14 +20,15 @@
 namespace
 {
   // https://graphics.stanford.edu/~seander/bithacks.html#RoundUpPowerOf2
-  uint32_t _NextPowerOfTwo(uint32_t v)
+  uint64_t _NextPowerOfTwo(uint64_t v)
   {
     v--;
     v |= v >> 1;
     v |= v >> 2;
     v |= v >> 4;
     v |= v >> 8;
     v |= v >> 16;
+    v |= v >> 32;
     v++;
     return v;
   }
@@ -107,8 +108,8 @@ namespace gtl
     // A resize is very unlikely and can be expensive
     if (byteOffset >= m_buffer.byteSize())
     {
-      uint32_t minSize = m_elementSize * m_minCapacity;
-      uint32_t newSize = std::max(_NextPowerOfTwo(byteOffset), minSize);
+      uint64_t minSize = m_elementSize * m_minCapacity;
+      uint64_t newSize = std::max(_NextPowerOfTwo(byteOffset), minSize);
 
       bool result = false;
 
@@ -146,6 +147,9 @@ namespace gtl
         cgpuDestroyCommandBuffer(m_device, commandBuffer);
       if (semaphore.handle)
         cgpuDestroySemaphore(m_device, semaphore);
+
+      if (!result)
+        byteOffset = UINT64_MAX;
     }
 
     return byteOffset;

src/ggpu/impl/Stager.cpp

@@ -157,7 +157,7 @@ namespace gtl
 
   bool GgpuStager::stageToImage(const uint8_t* src, uint64_t size, CgpuImage dst, uint32_t width, uint32_t height, uint32_t depth)
   {
-    uint64_t rowCount = height;
+    uint32_t rowCount = height;
     uint64_t rowSize = size / rowCount;
 
     if (rowSize > BUFFER_HALF_SIZE)
@@ -170,7 +170,7 @@ namespace gtl
     while (rowsStaged < rowCount)
     {
       uint64_t remainingSpace = BUFFER_HALF_SIZE - m_stagedBytes;
-      uint64_t maxCopyRowCount = remainingSpace / rowSize; // truncate
+      uint32_t maxCopyRowCount = uint32_t(remainingSpace / rowSize); // truncate
 
       if (maxCopyRowCount == 0)
       {
@@ -179,13 +179,13 @@ namespace gtl
           return false;
         }
 
-        maxCopyRowCount = BUFFER_HALF_SIZE / rowSize; // truncate
+        maxCopyRowCount = uint32_t(BUFFER_HALF_SIZE / rowSize); // truncate
       }
 
-      uint64_t remainingRowCount = rowCount - rowsStaged;
-      uint64_t copyRowCount = std::min(remainingRowCount, maxCopyRowCount);
+      uint32_t remainingRowCount = rowCount - rowsStaged;
+      uint32_t copyRowCount = std::min(remainingRowCount, maxCopyRowCount);
 
-      auto copyFunc = [this, dst, rowsStaged, width, depth, copyRowCount](uint64_t srcOffset, uint64_t dstOffset, uint64_t size) {
+      auto copyFunc = [this, dst, rowsStaged, width, depth, copyRowCount](uint64_t srcOffset, [[maybe_unused]] uint64_t dstOffset, [[maybe_unused]] uint64_t size) {
         CgpuBufferImageCopyDesc desc;
         desc.bufferOffset = srcOffset;
         desc.texelOffsetX = 0;

src/ggpu/impl/SyncBuffer.cpp

@@ -30,18 +30,17 @@ namespace gtl
                                  CgpuBufferUsageFlags bufferUsage)
     : m_device(device)
     , m_stager(stager)
-    , m_delayedResourceDestroyer(delayedResourceDestroyer)
     , m_elementSize(elementSize)
     , m_updateStrategy(updateStrategy)
+    , m_deviceBuffer(m_device,
+                     delayedResourceDestroyer,
+                     bufferUsage | CGPU_BUFFER_USAGE_FLAG_TRANSFER_DST,
+                     CGPU_MEMORY_PROPERTY_FLAG_DEVICE_LOCAL)
     , m_hostBuffer(m_device,
                    delayedResourceDestroyer,
                    CGPU_BUFFER_USAGE_FLAG_STORAGE_BUFFER | CGPU_BUFFER_USAGE_FLAG_TRANSFER_SRC,
                    CGPU_MEMORY_PROPERTY_FLAG_HOST_VISIBLE | CGPU_MEMORY_PROPERTY_FLAG_HOST_COHERENT |
                      (updateStrategy == UpdateStrategy::PersistentMapping ? CGPU_MEMORY_PROPERTY_FLAG_DEVICE_LOCAL : 0))
-    , m_deviceBuffer(m_device,
-                     delayedResourceDestroyer,
-                     bufferUsage | CGPU_BUFFER_USAGE_FLAG_TRANSFER_DST,
-                     CGPU_MEMORY_PROPERTY_FLAG_DEVICE_LOCAL)
   {
 
   }
@@ -54,7 +53,7 @@ namespace gtl
     assert(m_updateStrategy == UpdateStrategy::PersistentMapping || m_deviceBuffer.size() == 0);
   }
 
-  uint8_t* GgpuSyncBuffer::read(uint64_t byteOffset, uint64_t byteSize)
+  uint8_t* GgpuSyncBuffer::read(uint64_t byteOffset, [[maybe_unused]] uint64_t byteSize) // TODO: remove byteSize?
   {
     assert((byteOffset + byteSize) <= m_size);
     return &m_mappedHostMem[byteOffset];