From 758e255447f5352ca2712782e0cf006f688e1d44 Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 11:13:43 -0700 Subject: [PATCH 01/20] viz/core: VK_EXT_debug_utils + VK_EXT_validation_features (debug builds) Foundation for the upcoming vk::raii migration: get the validation harness in place before we start moving handles around. When Config::enable_validation is on AND VK_LAYER_KHRONOS_validation is available, VkContext now also: - enables VK_EXT_debug_utils (instance extension) - chains VkValidationFeaturesEXT into instance create info with BEST_PRACTICES + SYNCHRONIZATION_VALIDATION - registers a create-time messenger via VkInstanceCreateInfo::pNext (catches errors from vkCreateInstance itself) - registers a persistent VkDebugUtilsMessengerEXT routing warning+error messages to stderr The persistent messenger is torn down before vkDestroyInstance. Object naming helpers (vkSetDebugUtilsObjectNameEXT) land later when there are concrete call sites to name. Co-Authored-By: Claude Sonnet 4.6 --- src/viz/core/cpp/inc/viz/core/vk_context.hpp | 1 + src/viz/core/cpp/vk_context.cpp | 80 +++++++++++++++++++- 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/viz/core/cpp/inc/viz/core/vk_context.hpp b/src/viz/core/cpp/inc/viz/core/vk_context.hpp index a4014084b..d4044d1f5 100644 --- a/src/viz/core/cpp/inc/viz/core/vk_context.hpp +++ b/src/viz/core/cpp/inc/viz/core/vk_context.hpp @@ -129,6 +129,7 @@ class VkContext bool initialized_ = false; bool validation_enabled_ = false; VkInstance instance_ = VK_NULL_HANDLE; + VkDebugUtilsMessengerEXT debug_messenger_ = VK_NULL_HANDLE; VkPhysicalDevice physical_device_ = VK_NULL_HANDLE; VkDevice device_ = VK_NULL_HANDLE; uint32_t queue_family_index_ = UINT32_MAX; diff --git a/src/viz/core/cpp/vk_context.cpp b/src/viz/core/cpp/vk_context.cpp index 9ff3d91cc..ec07e30d9 100644 --- a/src/viz/core/cpp/vk_context.cpp +++ b/src/viz/core/cpp/vk_context.cpp @@ -51,6 +51,28 @@ bool is_validation_layer_available() return false; } +VKAPI_ATTR VkBool32 VKAPI_CALL debug_messenger_callback(VkDebugUtilsMessageSeverityFlagBitsEXT severity, + VkDebugUtilsMessageTypeFlagsEXT /*types*/, + const VkDebugUtilsMessengerCallbackDataEXT* data, + void* /*user*/) +{ + const char* level = "verbose"; + if (severity & VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT) + { + level = "ERROR"; + } + else if (severity & VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT) + { + level = "warn"; + } + else if (severity & VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT) + { + level = "info"; + } + std::cerr << "[Vulkan " << level << "] " << (data && data->pMessage ? data->pMessage : "(null)") << std::endl; + return VK_FALSE; +} + bool device_supports_extensions(VkPhysicalDevice device, const std::vector& required) { uint32_t count = 0; @@ -215,6 +237,16 @@ void VkContext::destroy() vkDestroyDevice(device_, nullptr); device_ = VK_NULL_HANDLE; } + if (debug_messenger_ != VK_NULL_HANDLE && instance_ != VK_NULL_HANDLE) + { + auto vkDestroyDebugUtilsMessengerEXT = reinterpret_cast( + vkGetInstanceProcAddr(instance_, "vkDestroyDebugUtilsMessengerEXT")); + if (vkDestroyDebugUtilsMessengerEXT != nullptr) + { + vkDestroyDebugUtilsMessengerEXT(instance_, debug_messenger_, nullptr); + } + debug_messenger_ = VK_NULL_HANDLE; + } if (instance_ != VK_NULL_HANDLE) { vkDestroyInstance(instance_, nullptr); @@ -296,11 +328,43 @@ void VkContext::create_instance(const Config& config) } std::vector instance_extensions; - instance_extensions.reserve(config.instance_extensions.size()); + instance_extensions.reserve(config.instance_extensions.size() + 1); for (const auto& s : config.instance_extensions) { instance_extensions.push_back(s.c_str()); } + if (validation_enabled_) + { + instance_extensions.push_back(VK_EXT_DEBUG_UTILS_EXTENSION_NAME); + } + + // Best-practices + sync validation are off by default; enabling + // them costs a bit of perf but catches a wide class of bugs the + // base layer misses. + const VkValidationFeatureEnableEXT enables[] = { + VK_VALIDATION_FEATURE_ENABLE_BEST_PRACTICES_EXT, + VK_VALIDATION_FEATURE_ENABLE_SYNCHRONIZATION_VALIDATION_EXT, + }; + VkValidationFeaturesEXT validation_features{}; + validation_features.sType = VK_STRUCTURE_TYPE_VALIDATION_FEATURES_EXT; + validation_features.enabledValidationFeatureCount = sizeof(enables) / sizeof(enables[0]); + validation_features.pEnabledValidationFeatures = enables; + + // Create-time messenger via pNext catches errors from + // vkCreateInstance itself (the persistent messenger created + // below misses those). + VkDebugUtilsMessengerCreateInfoEXT debug_create_info{}; + debug_create_info.sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT; + debug_create_info.messageSeverity = + VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT | VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT; + debug_create_info.messageType = VK_DEBUG_UTILS_MESSAGE_TYPE_GENERAL_BIT_EXT | + VK_DEBUG_UTILS_MESSAGE_TYPE_VALIDATION_BIT_EXT | + VK_DEBUG_UTILS_MESSAGE_TYPE_PERFORMANCE_BIT_EXT; + debug_create_info.pfnUserCallback = debug_messenger_callback; + if (validation_enabled_) + { + debug_create_info.pNext = &validation_features; + } VkInstanceCreateInfo create_info{}; create_info.sType = VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO; @@ -309,12 +373,26 @@ void VkContext::create_instance(const Config& config) create_info.ppEnabledLayerNames = layers.data(); create_info.enabledExtensionCount = static_cast(instance_extensions.size()); create_info.ppEnabledExtensionNames = instance_extensions.data(); + if (validation_enabled_) + { + create_info.pNext = &debug_create_info; + } const VkResult result = vkCreateInstance(&create_info, nullptr, &instance_); if (result != VK_SUCCESS) { throw std::runtime_error("vkCreateInstance failed: VkResult=" + std::to_string(result)); } + + if (validation_enabled_) + { + auto vkCreateDebugUtilsMessengerEXT = reinterpret_cast( + vkGetInstanceProcAddr(instance_, "vkCreateDebugUtilsMessengerEXT")); + if (vkCreateDebugUtilsMessengerEXT != nullptr) + { + (void)vkCreateDebugUtilsMessengerEXT(instance_, &debug_create_info, nullptr, &debug_messenger_); + } + } } void VkContext::select_physical_device(const Config& config) From a342a4bbacdb4a18a8c5eb6f7d63ec78fd44ce2a Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 11:22:03 -0700 Subject: [PATCH 02/20] =?UTF-8?q?viz/core:=20vk.hpp=20project=20include=20?= =?UTF-8?q?=E2=80=94=20vulkan-hpp=20+=20vk::raii=20conventions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The single canonical include point for Televiz Vulkan code. Defines VULKAN_HPP_NO_CONSTRUCTORS so struct types are aggregates and C++20 designated initializers work, then pulls in vulkan.hpp and vulkan_raii.hpp. Header comment documents the conventions migration commits will follow: - vk::raii::* for owned handles - vk::StructureChain for pNext chains - designated initializers for struct creation - raw handle extraction (*handle_) only at marked CUDA / OpenXR interop boundaries Two unit tests pin the toolchain: vk::raii::Context constructs without throwing (loader is reachable), and a designated-init + StructureChain combination compiles + behaves correctly. No existing code migrated yet — that starts in the next commit. Co-Authored-By: Claude Sonnet 4.6 --- src/viz/core/cpp/CMakeLists.txt | 1 + src/viz/core/cpp/inc/viz/core/vk.hpp | 29 ++++++++++++++++++++++ src/viz/core_tests/cpp/CMakeLists.txt | 1 + src/viz/core_tests/cpp/test_vk_hpp.cpp | 33 ++++++++++++++++++++++++++ 4 files changed, 64 insertions(+) create mode 100644 src/viz/core/cpp/inc/viz/core/vk.hpp create mode 100644 src/viz/core_tests/cpp/test_vk_hpp.cpp diff --git a/src/viz/core/cpp/CMakeLists.txt b/src/viz/core/cpp/CMakeLists.txt index 6962bc5ca..6711684ce 100644 --- a/src/viz/core/cpp/CMakeLists.txt +++ b/src/viz/core/cpp/CMakeLists.txt @@ -34,6 +34,7 @@ add_library(viz_core STATIC inc/viz/core/render_target.hpp inc/viz/core/viz_buffer.hpp inc/viz/core/viz_types.hpp + inc/viz/core/vk.hpp inc/viz/core/vk_context.hpp ) diff --git a/src/viz/core/cpp/inc/viz/core/vk.hpp b/src/viz/core/cpp/inc/viz/core/vk.hpp new file mode 100644 index 000000000..995a38c79 --- /dev/null +++ b/src/viz/core/cpp/inc/viz/core/vk.hpp @@ -0,0 +1,29 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +// Project-wide vulkan-hpp + vk::raii include header. +// +// Conventions for Televiz Vulkan code: +// * Owned handles use vk::raii::* (Instance, Device, Image, Semaphore, ...) +// * pNext chains use vk::StructureChain +// * Initialize structs with C++20 designated initializers +// (`vk::ImageCreateInfo{.imageType = ..., .format = ..., ...}`) +// * Extract raw handles via *handle_ ONLY at deliberate interop +// boundaries (CUDA external memory FD, XrGraphicsBindingVulkanKHR). +// Mark such sites with a comment so they read as boundary code. +// +// We use the default static dispatch for vulkan-hpp; vk::raii types +// own their dispatcher automatically — no VULKAN_HPP_DEFAULT_DISPATCHER +// initialization needed. +// +// VULKAN_HPP_NO_CONSTRUCTORS removes vulkan-hpp's hand-written +// constructors so structs become aggregates, enabling C++20 +// designated initializers (`vk::ImageCreateInfo{.format = ..., ...}`). +// Builder methods like setFormat() still work; we just lose the +// positional parameter-list constructors (which we wouldn't use anyway). +#define VULKAN_HPP_NO_CONSTRUCTORS + +#include +#include diff --git a/src/viz/core_tests/cpp/CMakeLists.txt b/src/viz/core_tests/cpp/CMakeLists.txt index 76cec20e6..d2d586769 100644 --- a/src/viz/core_tests/cpp/CMakeLists.txt +++ b/src/viz/core_tests/cpp/CMakeLists.txt @@ -21,6 +21,7 @@ add_executable(viz_core_tests test_frame_sync.cpp test_host_image.cpp test_render_target.cpp + test_vk_hpp.cpp test_viz_buffer.cpp test_viz_types.cpp test_vk_context.cpp diff --git a/src/viz/core_tests/cpp/test_vk_hpp.cpp b/src/viz/core_tests/cpp/test_vk_hpp.cpp new file mode 100644 index 000000000..f71a5f938 --- /dev/null +++ b/src/viz/core_tests/cpp/test_vk_hpp.cpp @@ -0,0 +1,33 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#include +#include + +TEST_CASE("vulkan-hpp + vk::raii toolchain compiles and loads", "[unit][vk_hpp]") +{ + // vk::raii::Context wraps the dynamic loader. Constructing it + // verifies vulkan.hpp + vulkan_raii.hpp link cleanly and the + // loader is reachable — no instance / device / GPU required. + REQUIRE_NOTHROW(vk::raii::Context{}); +} + +TEST_CASE("designated initializers + vk::StructureChain compile", "[unit][vk_hpp]") +{ + // No runtime check; the value here is the compile-time guarantee + // that the convention works on this toolchain. + constexpr vk::ApplicationInfo app{ + .pApplicationName = "Televiz", + .applicationVersion = 1, + .pEngineName = "Televiz", + .engineVersion = 1, + .apiVersion = VK_API_VERSION_1_2, + }; + static_assert(app.apiVersion == VK_API_VERSION_1_2); + + vk::StructureChain chain{ + vk::InstanceCreateInfo{}.setPApplicationInfo(&app), + vk::ValidationFeaturesEXT{}, + }; + REQUIRE(chain.get().pApplicationInfo == &app); +} From 3896199948db426b7ea18a9b0b718652725f10c5 Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 11:27:54 -0700 Subject: [PATCH 03/20] viz/core: migrate VkContext internals to vk::raii MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VkContext now owns its Vulkan handles as vk::raii::* members (Instance, DebugUtilsMessengerEXT, PhysicalDevice, Device, Queue, PipelineCache). The reverse-declaration order matches Vulkan's parent-before-child destruction requirement: pipeline_cache_ → queue_ → device_ → physical_device_ → debug_messenger_ → instance_. destroy() now just resets each member to nullptr (move-from nullptr triggers vk::raii's destructor) — eliminates the manual "if (h \!= VK_NULL_HANDLE) { vkDestroy*; h = VK_NULL_HANDLE; }" chain. Helpers (is_validation_layer_available, device_supports_extensions, find_graphics_compute_queue_family, score_physical_device) and enumerate_physical_devices() similarly use vulkan-hpp / vk::raii equivalents. Public API surface preserved: * Existing raw-handle getters (instance(), device(), ...) extract via *member_ — consumers (viz_session, viz_layers) keep working unchanged. * Added raii getters (raii_instance(), raii_device(), ...) for in-tree consumers to use during their own migration commits. create_instance / create_logical_device / create_pipeline_cache use vk::raii::* constructors directly. Designated initializers on the create-info structs make the call sites read top-down. All 52 unit tests pass. Co-Authored-By: Claude Sonnet 4.6 --- src/viz/core/cpp/inc/viz/core/vk_context.hpp | 126 +++--- src/viz/core/cpp/vk_context.cpp | 418 +++++++------------ 2 files changed, 197 insertions(+), 347 deletions(-) diff --git a/src/viz/core/cpp/inc/viz/core/vk_context.hpp b/src/viz/core/cpp/inc/viz/core/vk_context.hpp index d4044d1f5..8e6de8649 100644 --- a/src/viz/core/cpp/inc/viz/core/vk_context.hpp +++ b/src/viz/core/cpp/inc/viz/core/vk_context.hpp @@ -3,7 +3,7 @@ #pragma once -#include +#include #include #include @@ -13,110 +13,90 @@ namespace viz { // Read-only info about a Vulkan physical device. -// -// Returned by VkContext::enumerate_physical_devices(). Use this to discover -// available GPUs and choose one explicitly via Config::physical_device_index -// when multiple GPUs are present (e.g. servers with two NVIDIA cards). struct PhysicalDeviceInfo { - uint32_t index = 0; // Index in vkEnumeratePhysicalDevices order - std::string name; // deviceName from VkPhysicalDeviceProperties - uint32_t vendor_id = 0; // PCI vendor ID (e.g. 0x10DE for NVIDIA) - uint32_t device_id = 0; // PCI device ID - bool is_discrete = false; // True for discrete (dedicated) GPUs - bool meets_requirements = false; // True if suitable for VkContext (API 1.2+, - // queue family, required extensions) + uint32_t index = 0; + std::string name; + uint32_t vendor_id = 0; + uint32_t device_id = 0; + bool is_discrete = false; + bool meets_requirements = false; }; -// Standalone Vulkan instance/device creation for Televiz. +// Vulkan instance + device + queue + pipeline cache for Televiz. // -// Today this is the standalone path only: enumerate physical devices directly, -// pick one (auto or explicit), and create a logical device with a graphics + -// compute + transfer queue. The OpenXR-negotiated path -// (xrCreateVulkanInstanceKHR / xrCreateVulkanDeviceKHR) is added later when -// XR rendering is implemented. +// Standalone path today (raw enumeration + selection); the OpenXR- +// negotiated path is added with the XR backend. // // The selected physical device must support: // - Vulkan API 1.2 or newer -// - VK_KHR_external_memory + VK_KHR_external_memory_fd (CUDA-Vulkan interop) -// - VK_KHR_external_semaphore + VK_KHR_external_semaphore_fd (CUDA sync) -// - A queue family with graphics + compute + transfer flags -// -// VkContext owns the Vulkan handles and tears them down on destruction. +// - VK_KHR_external_memory + VK_KHR_external_memory_fd +// - VK_KHR_external_semaphore + VK_KHR_external_semaphore_fd +// - A graphics + compute + transfer queue family // // init() also matches the current CUDA device to the chosen Vulkan -// physical device by UUID, so every viz_core type that touches CUDA -// can assume the two APIs are on the same GPU. +// physical device by UUID. class VkContext { public: struct Config { - // Enables VK_LAYER_KHRONOS_validation if available. bool enable_validation = false; - - // Additional instance/device extensions to enable beyond the - // Televiz-required set. std::vector instance_extensions; std::vector device_extensions; - - // Physical device selection. - // -1 (default): auto-pick the best suitable device (NVIDIA discrete - // GPUs preferred; must support required extensions). - // >=0: use the device at this index from - // vkEnumeratePhysicalDevices. The device must still - // meet Televiz requirements or init() throws. Use - // enumerate_physical_devices() to discover available - // indices. + // -1 = auto-pick best; otherwise the explicit index from + // vkEnumeratePhysicalDevices. int physical_device_index = -1; }; VkContext() = default; + ~VkContext(); - // Non-copyable, non-movable for now (owns Vulkan handles). VkContext(const VkContext&) = delete; VkContext& operator=(const VkContext&) = delete; VkContext(VkContext&&) = delete; VkContext& operator=(VkContext&&) = delete; - ~VkContext(); - - // Initializes Vulkan: instance + physical device selection + logical - // device + queue. Throws std::runtime_error on Vulkan failure or if no - // suitable physical device is found. Throws std::logic_error if the - // context is already initialized. Throws std::out_of_range if - // Config::physical_device_index is set but out of range. void init(const Config& config); - - // Releases all Vulkan handles. Idempotent (safe to call multiple times, - // and on a non-initialized context). void destroy(); - bool is_initialized() const noexcept; + // Raw-handle getters — extracted from the owned vk::raii types. + // Use these at CUDA / OpenXR interop boundaries; pure-Vulkan + // consumers should prefer the raii getters below for chained + // child handles. VkInstance instance() const noexcept; VkPhysicalDevice physical_device() const noexcept; VkDevice device() const noexcept; uint32_t queue_family_index() const noexcept; VkQueue queue() const noexcept; - - // Process-wide VkPipelineCache for driver-side compiled-state - // reuse across pipeline creations. VK_NULL_HANDLE before init(). VkPipelineCache pipeline_cache() const noexcept; - // CUDA device id matched to the chosen Vulkan physical device. - // Layers created on worker threads should - // cudaSetDevice(ctx.cuda_device_id()) before any CUDA call — - // cudaSetDevice is per-host-thread. Returns -1 before init(). + // raii getters for in-tree consumers constructing further + // vk::raii::* handles. References stay valid until destroy(). + vk::raii::Instance const& raii_instance() const noexcept + { + return instance_; + } + vk::raii::PhysicalDevice const& raii_physical_device() const noexcept + { + return physical_device_; + } + vk::raii::Device const& raii_device() const noexcept + { + return device_; + } + vk::raii::Queue const& raii_queue() const noexcept + { + return queue_; + } + vk::raii::PipelineCache const& raii_pipeline_cache() const noexcept + { + return pipeline_cache_; + } + int cuda_device_id() const noexcept; - // Enumerates all Vulkan-capable physical devices and returns their - // properties. Useful for picking a specific GPU index on multi-GPU - // machines before calling init(). - // - // Creates a minimal temporary VkInstance internally and tears it down. - // Does not throw. Returns an empty vector if the Vulkan loader is - // unavailable, vkCreateInstance fails, or no devices are present. static std::vector enumerate_physical_devices(); private: @@ -128,14 +108,18 @@ class VkContext bool initialized_ = false; bool validation_enabled_ = false; - VkInstance instance_ = VK_NULL_HANDLE; - VkDebugUtilsMessengerEXT debug_messenger_ = VK_NULL_HANDLE; - VkPhysicalDevice physical_device_ = VK_NULL_HANDLE; - VkDevice device_ = VK_NULL_HANDLE; - uint32_t queue_family_index_ = UINT32_MAX; - VkQueue queue_ = VK_NULL_HANDLE; - VkPipelineCache pipeline_cache_ = VK_NULL_HANDLE; int cuda_device_id_ = -1; + uint32_t queue_family_index_ = UINT32_MAX; + + // Declared parent-first so reverse-order destruction tears + // children down before parents (pipeline cache → device → ... → instance). + vk::raii::Context context_{}; + vk::raii::Instance instance_{ nullptr }; + vk::raii::DebugUtilsMessengerEXT debug_messenger_{ nullptr }; + vk::raii::PhysicalDevice physical_device_{ nullptr }; + vk::raii::Device device_{ nullptr }; + vk::raii::Queue queue_{ nullptr }; + vk::raii::PipelineCache pipeline_cache_{ nullptr }; }; } // namespace viz diff --git a/src/viz/core/cpp/vk_context.cpp b/src/viz/core/cpp/vk_context.cpp index ec07e30d9..dfd1eb20e 100644 --- a/src/viz/core/cpp/vk_context.cpp +++ b/src/viz/core/cpp/vk_context.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include namespace viz @@ -24,10 +25,8 @@ constexpr const char* kEngineName = "Televiz"; constexpr uint32_t kEngineVersion = VK_MAKE_VERSION(1, 0, 0); constexpr uint32_t kApiVersion = VK_API_VERSION_1_2; -// Vendor IDs. constexpr uint32_t kVendorNvidia = 0x10DE; -// Device extensions Televiz always requires (for CUDA-Vulkan interop). const std::vector kRequiredDeviceExtensions = { VK_KHR_EXTERNAL_MEMORY_EXTENSION_NAME, VK_KHR_EXTERNAL_MEMORY_FD_EXTENSION_NAME, @@ -37,11 +36,7 @@ const std::vector kRequiredDeviceExtensions = { bool is_validation_layer_available() { - uint32_t count = 0; - vkEnumerateInstanceLayerProperties(&count, nullptr); - std::vector layers(count); - vkEnumerateInstanceLayerProperties(&count, layers.data()); - for (const auto& layer : layers) + for (const auto& layer : vk::enumerateInstanceLayerProperties()) { if (std::strcmp(layer.layerName, kValidationLayerName) == 0) { @@ -73,24 +68,13 @@ VKAPI_ATTR VkBool32 VKAPI_CALL debug_messenger_callback(VkDebugUtilsMessageSever return VK_FALSE; } -bool device_supports_extensions(VkPhysicalDevice device, const std::vector& required) +bool device_supports_extensions(vk::PhysicalDevice device, const std::vector& required) { - uint32_t count = 0; - vkEnumerateDeviceExtensionProperties(device, nullptr, &count, nullptr); - std::vector available(count); - vkEnumerateDeviceExtensionProperties(device, nullptr, &count, available.data()); - + const auto available = device.enumerateDeviceExtensionProperties(); for (const char* req : required) { - bool found = false; - for (const auto& ext : available) - { - if (std::strcmp(ext.extensionName, req) == 0) - { - found = true; - break; - } - } + const bool found = std::any_of(available.begin(), available.end(), + [&](const auto& ext) { return std::strcmp(ext.extensionName, req) == 0; }); if (!found) { return false; @@ -99,30 +83,17 @@ bool device_supports_extensions(VkPhysicalDevice device, const std::vector input (avoids forcing -// callers to materialize a vector just for the check). -bool device_supports_extensions(VkPhysicalDevice device, const std::vector& required) +bool device_supports_extensions(vk::PhysicalDevice device, const std::vector& required) { if (required.empty()) { return true; } - uint32_t count = 0; - vkEnumerateDeviceExtensionProperties(device, nullptr, &count, nullptr); - std::vector available(count); - vkEnumerateDeviceExtensionProperties(device, nullptr, &count, available.data()); - + const auto available = device.enumerateDeviceExtensionProperties(); for (const auto& req : required) { - bool found = false; - for (const auto& ext : available) - { - if (req == ext.extensionName) - { - found = true; - break; - } - } + const bool found = + std::any_of(available.begin(), available.end(), [&](const auto& ext) { return req == ext.extensionName; }); if (!found) { return false; @@ -131,15 +102,12 @@ bool device_supports_extensions(VkPhysicalDevice device, const std::vector families(count); - vkGetPhysicalDeviceQueueFamilyProperties(device, &count, families.data()); - - constexpr VkQueueFlags required_flags = VK_QUEUE_GRAPHICS_BIT | VK_QUEUE_COMPUTE_BIT | VK_QUEUE_TRANSFER_BIT; - for (uint32_t i = 0; i < count; ++i) + constexpr auto required_flags = + vk::QueueFlagBits::eGraphics | vk::QueueFlagBits::eCompute | vk::QueueFlagBits::eTransfer; + const auto families = device.getQueueFamilyProperties(); + for (uint32_t i = 0; i < families.size(); ++i) { if ((families[i].queueFlags & required_flags) == required_flags) { @@ -149,46 +117,34 @@ uint32_t find_graphics_compute_queue_family(VkPhysicalDevice device) return UINT32_MAX; } -// Score a physical device. Higher is better; -1 means unsuitable. -int score_physical_device(VkPhysicalDevice device) +int score_physical_device(vk::PhysicalDevice device) { - VkPhysicalDeviceProperties props; - vkGetPhysicalDeviceProperties(device, &props); - - // Required: API 1.2 or newer. + const auto props = device.getProperties(); if (props.apiVersion < kApiVersion) { return -1; } - - // Required: graphics+compute+transfer queue family. if (find_graphics_compute_queue_family(device) == UINT32_MAX) { return -1; } - - // Required: external memory extensions (CUDA interop dependency). if (!device_supports_extensions(device, kRequiredDeviceExtensions)) { return -1; } - int score = 0; - - // Strongly prefer NVIDIA GPUs (CUDA interop is NVIDIA-only). if (props.vendorID == kVendorNvidia) { score += 1000; } - if (props.deviceType == VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU) + if (props.deviceType == vk::PhysicalDeviceType::eDiscreteGpu) { score += 500; } - else if (props.deviceType == VK_PHYSICAL_DEVICE_TYPE_INTEGRATED_GPU) + else if (props.deviceType == vk::PhysicalDeviceType::eIntegratedGpu) { score += 100; } - return score; } @@ -205,9 +161,6 @@ void VkContext::init(const Config& config) { throw std::logic_error("VkContext::init: already initialized"); } - // Roll back any partial state if a later step throws so the context is - // left in a clean uninitialized state (no leaked instance/device handles) - // and is safe to retry init() on. try { create_instance(config); @@ -226,36 +179,15 @@ void VkContext::init(const Config& config) void VkContext::destroy() { - // Destroy device-owned objects (pipeline cache) before the device. - if (pipeline_cache_ != VK_NULL_HANDLE && device_ != VK_NULL_HANDLE) - { - vkDestroyPipelineCache(device_, pipeline_cache_, nullptr); - pipeline_cache_ = VK_NULL_HANDLE; - } - if (device_ != VK_NULL_HANDLE) - { - vkDestroyDevice(device_, nullptr); - device_ = VK_NULL_HANDLE; - } - if (debug_messenger_ != VK_NULL_HANDLE && instance_ != VK_NULL_HANDLE) - { - auto vkDestroyDebugUtilsMessengerEXT = reinterpret_cast( - vkGetInstanceProcAddr(instance_, "vkDestroyDebugUtilsMessengerEXT")); - if (vkDestroyDebugUtilsMessengerEXT != nullptr) - { - vkDestroyDebugUtilsMessengerEXT(instance_, debug_messenger_, nullptr); - } - debug_messenger_ = VK_NULL_HANDLE; - } - if (instance_ != VK_NULL_HANDLE) - { - vkDestroyInstance(instance_, nullptr); - instance_ = VK_NULL_HANDLE; - } - physical_device_ = VK_NULL_HANDLE; - queue_ = VK_NULL_HANDLE; + // Reverse parent/child order. Each move-from-nullptr destroys the + // existing handle via vk::raii's destructor. + pipeline_cache_ = nullptr; + queue_ = nullptr; + device_ = nullptr; + physical_device_ = nullptr; + debug_messenger_ = nullptr; + instance_ = nullptr; queue_family_index_ = UINT32_MAX; - pipeline_cache_ = VK_NULL_HANDLE; cuda_device_id_ = -1; validation_enabled_ = false; initialized_ = false; @@ -268,17 +200,17 @@ bool VkContext::is_initialized() const noexcept VkInstance VkContext::instance() const noexcept { - return instance_; + return *instance_; } VkPhysicalDevice VkContext::physical_device() const noexcept { - return physical_device_; + return *physical_device_; } VkDevice VkContext::device() const noexcept { - return device_; + return *device_; } uint32_t VkContext::queue_family_index() const noexcept @@ -288,12 +220,12 @@ uint32_t VkContext::queue_family_index() const noexcept VkQueue VkContext::queue() const noexcept { - return queue_; + return *queue_; } VkPipelineCache VkContext::pipeline_cache() const noexcept { - return pipeline_cache_; + return *pipeline_cache_; } int VkContext::cuda_device_id() const noexcept @@ -303,13 +235,13 @@ int VkContext::cuda_device_id() const noexcept void VkContext::create_instance(const Config& config) { - VkApplicationInfo app_info{}; - app_info.sType = VK_STRUCTURE_TYPE_APPLICATION_INFO; - app_info.pApplicationName = kAppName; - app_info.applicationVersion = kAppVersion; - app_info.pEngineName = kEngineName; - app_info.engineVersion = kEngineVersion; - app_info.apiVersion = kApiVersion; + const vk::ApplicationInfo app_info{ + .pApplicationName = kAppName, + .applicationVersion = kAppVersion, + .pEngineName = kEngineName, + .engineVersion = kEngineVersion, + .apiVersion = kApiVersion, + }; std::vector layers; if (config.enable_validation) @@ -338,204 +270,154 @@ void VkContext::create_instance(const Config& config) instance_extensions.push_back(VK_EXT_DEBUG_UTILS_EXTENSION_NAME); } - // Best-practices + sync validation are off by default; enabling - // them costs a bit of perf but catches a wide class of bugs the - // base layer misses. - const VkValidationFeatureEnableEXT enables[] = { - VK_VALIDATION_FEATURE_ENABLE_BEST_PRACTICES_EXT, - VK_VALIDATION_FEATURE_ENABLE_SYNCHRONIZATION_VALIDATION_EXT, + const vk::ValidationFeatureEnableEXT enables[] = { + vk::ValidationFeatureEnableEXT::eBestPractices, + vk::ValidationFeatureEnableEXT::eSynchronizationValidation, + }; + const vk::ValidationFeaturesEXT validation_features{ + .enabledValidationFeatureCount = static_cast(std::size(enables)), + .pEnabledValidationFeatures = enables, }; - VkValidationFeaturesEXT validation_features{}; - validation_features.sType = VK_STRUCTURE_TYPE_VALIDATION_FEATURES_EXT; - validation_features.enabledValidationFeatureCount = sizeof(enables) / sizeof(enables[0]); - validation_features.pEnabledValidationFeatures = enables; - - // Create-time messenger via pNext catches errors from - // vkCreateInstance itself (the persistent messenger created - // below misses those). - VkDebugUtilsMessengerCreateInfoEXT debug_create_info{}; - debug_create_info.sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT; - debug_create_info.messageSeverity = - VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT | VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT; - debug_create_info.messageType = VK_DEBUG_UTILS_MESSAGE_TYPE_GENERAL_BIT_EXT | - VK_DEBUG_UTILS_MESSAGE_TYPE_VALIDATION_BIT_EXT | - VK_DEBUG_UTILS_MESSAGE_TYPE_PERFORMANCE_BIT_EXT; - debug_create_info.pfnUserCallback = debug_messenger_callback; - if (validation_enabled_) - { - debug_create_info.pNext = &validation_features; - } - VkInstanceCreateInfo create_info{}; - create_info.sType = VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO; - create_info.pApplicationInfo = &app_info; - create_info.enabledLayerCount = static_cast(layers.size()); - create_info.ppEnabledLayerNames = layers.data(); - create_info.enabledExtensionCount = static_cast(instance_extensions.size()); - create_info.ppEnabledExtensionNames = instance_extensions.data(); - if (validation_enabled_) - { - create_info.pNext = &debug_create_info; - } + // Catches errors emitted during instance creation itself. + const vk::DebugUtilsMessengerCreateInfoEXT debug_create_info{ + .pNext = validation_enabled_ ? &validation_features : nullptr, + .messageSeverity = + vk::DebugUtilsMessageSeverityFlagBitsEXT::eWarning | vk::DebugUtilsMessageSeverityFlagBitsEXT::eError, + .messageType = vk::DebugUtilsMessageTypeFlagBitsEXT::eGeneral | vk::DebugUtilsMessageTypeFlagBitsEXT::eValidation | + vk::DebugUtilsMessageTypeFlagBitsEXT::ePerformance, + // C ABI callback; vk::Flags wrappers are layout-compatible + // with the raw C flag types but the function-pointer type + // signatures aren't, hence the reinterpret_cast. + .pfnUserCallback = reinterpret_cast(debug_messenger_callback), + }; - const VkResult result = vkCreateInstance(&create_info, nullptr, &instance_); - if (result != VK_SUCCESS) - { - throw std::runtime_error("vkCreateInstance failed: VkResult=" + std::to_string(result)); - } + const vk::InstanceCreateInfo create_info{ + .pNext = validation_enabled_ ? &debug_create_info : nullptr, + .pApplicationInfo = &app_info, + .enabledLayerCount = static_cast(layers.size()), + .ppEnabledLayerNames = layers.data(), + .enabledExtensionCount = static_cast(instance_extensions.size()), + .ppEnabledExtensionNames = instance_extensions.data(), + }; + + instance_ = vk::raii::Instance{ context_, create_info }; if (validation_enabled_) { - auto vkCreateDebugUtilsMessengerEXT = reinterpret_cast( - vkGetInstanceProcAddr(instance_, "vkCreateDebugUtilsMessengerEXT")); - if (vkCreateDebugUtilsMessengerEXT != nullptr) - { - (void)vkCreateDebugUtilsMessengerEXT(instance_, &debug_create_info, nullptr, &debug_messenger_); - } + debug_messenger_ = vk::raii::DebugUtilsMessengerEXT{ instance_, debug_create_info }; } } void VkContext::select_physical_device(const Config& config) { - uint32_t count = 0; - vkEnumeratePhysicalDevices(instance_, &count, nullptr); - if (count == 0) + auto devices = vk::raii::PhysicalDevices{ instance_ }; + if (devices.empty()) { throw std::runtime_error("No Vulkan-capable physical devices found"); } - std::vector devices(count); - vkEnumeratePhysicalDevices(instance_, &count, devices.data()); - - // A device is "suitable" iff it passes the always-required check - // (score >= 0) AND supports any caller-requested device extensions. - // Validating caller extensions here surfaces a clear error / lets - // auto-pick skip the device, instead of failing later inside - // vkCreateDevice with a generic VK_ERROR_EXTENSION_NOT_PRESENT. - auto is_suitable = [&](VkPhysicalDevice d) + const auto is_suitable = [&](vk::PhysicalDevice d) { return score_physical_device(d) >= 0 && device_supports_extensions(d, config.device_extensions); }; if (config.physical_device_index >= 0) { - // Explicit index: pick that device, validate it meets requirements. - const auto requested = static_cast(config.physical_device_index); - if (requested >= count) + const auto requested = static_cast(config.physical_device_index); + if (requested >= devices.size()) { throw std::out_of_range("VkContext: physical_device_index " + std::to_string(requested) + - " is out of range (only " + std::to_string(count) + " device(s) available)"); + " is out of range (only " + std::to_string(devices.size()) + " device(s) available)"); } - if (!is_suitable(devices[requested])) + if (!is_suitable(*devices[requested])) { - VkPhysicalDeviceProperties props; - vkGetPhysicalDeviceProperties(devices[requested], &props); + const auto props = devices[requested].getProperties(); throw std::runtime_error("VkContext: physical device at index " + std::to_string(requested) + " (" + - props.deviceName + + std::string(props.deviceName.data()) + ") does not meet Televiz requirements " "(need API 1.2+, graphics+compute queue, " "required + caller-requested extensions)"); } - physical_device_ = devices[requested]; + physical_device_ = std::move(devices[requested]); } else { - // Auto-pick: highest-scoring suitable device. int best_score = -1; - VkPhysicalDevice best_device = VK_NULL_HANDLE; - for (VkPhysicalDevice candidate : devices) + size_t best_index = devices.size(); + for (size_t i = 0; i < devices.size(); ++i) { - if (!is_suitable(candidate)) + if (!is_suitable(*devices[i])) { continue; } - const int s = score_physical_device(candidate); + const int s = score_physical_device(*devices[i]); if (s > best_score) { best_score = s; - best_device = candidate; + best_index = i; } } - - if (best_device == VK_NULL_HANDLE) + if (best_index == devices.size()) { throw std::runtime_error( "No suitable Vulkan physical device found " "(need API 1.2+, graphics+compute queue, " "required + caller-requested extensions)"); } - - physical_device_ = best_device; + physical_device_ = std::move(devices[best_index]); } - queue_family_index_ = find_graphics_compute_queue_family(physical_device_); + queue_family_index_ = find_graphics_compute_queue_family(*physical_device_); } void VkContext::create_logical_device(const Config& config) { const float queue_priority = 1.0f; - VkDeviceQueueCreateInfo queue_info{}; - queue_info.sType = VK_STRUCTURE_TYPE_DEVICE_QUEUE_CREATE_INFO; - queue_info.queueFamilyIndex = queue_family_index_; - queue_info.queueCount = 1; - queue_info.pQueuePriorities = &queue_priority; + const vk::DeviceQueueCreateInfo queue_info{ + .queueFamilyIndex = queue_family_index_, + .queueCount = 1, + .pQueuePriorities = &queue_priority, + }; - // Build extension list: required + caller-provided. std::vector extensions(kRequiredDeviceExtensions); for (const auto& s : config.device_extensions) { extensions.push_back(s.c_str()); } - VkPhysicalDeviceFeatures device_features{}; - - // Enable the Vulkan 1.2 timeline semaphore feature so DeviceImage - // can use VK_SEMAPHORE_TYPE_TIMELINE for CUDA-Vulkan interop. - VkPhysicalDeviceVulkan12Features features12{}; - features12.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_VULKAN_1_2_FEATURES; - features12.timelineSemaphore = VK_TRUE; - - VkDeviceCreateInfo device_info{}; - device_info.sType = VK_STRUCTURE_TYPE_DEVICE_CREATE_INFO; - device_info.pNext = &features12; - device_info.queueCreateInfoCount = 1; - device_info.pQueueCreateInfos = &queue_info; - device_info.enabledExtensionCount = static_cast(extensions.size()); - device_info.ppEnabledExtensionNames = extensions.data(); - device_info.pEnabledFeatures = &device_features; - - const VkResult result = vkCreateDevice(physical_device_, &device_info, nullptr, &device_); - if (result != VK_SUCCESS) - { - throw std::runtime_error("vkCreateDevice failed: VkResult=" + std::to_string(result)); - } + const vk::PhysicalDeviceFeatures device_features{}; + + // VK_SEMAPHORE_TYPE_TIMELINE for CUDA-Vulkan interop. + const vk::PhysicalDeviceVulkan12Features features12{ + .timelineSemaphore = VK_TRUE, + }; - vkGetDeviceQueue(device_, queue_family_index_, 0, &queue_); + const vk::DeviceCreateInfo device_info{ + .pNext = &features12, + .queueCreateInfoCount = 1, + .pQueueCreateInfos = &queue_info, + .enabledExtensionCount = static_cast(extensions.size()), + .ppEnabledExtensionNames = extensions.data(), + .pEnabledFeatures = &device_features, + }; + + device_ = vk::raii::Device{ physical_device_, device_info }; + queue_ = device_.getQueue(queue_family_index_, 0); } void VkContext::create_pipeline_cache() { // Empty cache; the driver populates it as pipelines are created. - // Not persisted across runs — purely in-process reuse. - VkPipelineCacheCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_PIPELINE_CACHE_CREATE_INFO; - const VkResult result = vkCreatePipelineCache(device_, &info, nullptr, &pipeline_cache_); - if (result != VK_SUCCESS) - { - throw std::runtime_error("vkCreatePipelineCache failed: VkResult=" + std::to_string(result)); - } + pipeline_cache_ = vk::raii::PipelineCache{ device_, vk::PipelineCacheCreateInfo{} }; } void VkContext::match_cuda_device_to_vulkan() { - // Find the CUDA device whose UUID matches the chosen Vulkan - // physical device and make it current. Required so CUDA-Vulkan - // interop on multi-GPU machines doesn't pick a different GPU - // than Vulkan. - VkPhysicalDeviceIDProperties id_props{}; - id_props.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ID_PROPERTIES; - VkPhysicalDeviceProperties2 props2{}; - props2.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROPERTIES_2; - props2.pNext = &id_props; - vkGetPhysicalDeviceProperties2(physical_device_, &props2); + // Find the CUDA device whose UUID matches the Vulkan physical + // device. Required so CUDA-Vulkan interop on multi-GPU machines + // doesn't pick a different GPU than Vulkan. + const auto props_chain = + physical_device_.getProperties2(); + const auto& id_props = props_chain.get(); int cuda_count = 0; cudaError_t err = cudaGetDeviceCount(&cuda_count); @@ -553,7 +435,7 @@ void VkContext::match_cuda_device_to_vulkan() { continue; } - if (std::memcmp(prop.uuid.bytes, id_props.deviceUUID, VK_UUID_SIZE) == 0) + if (std::memcmp(prop.uuid.bytes, id_props.deviceUUID.data(), VK_UUID_SIZE) == 0) { err = cudaSetDevice(i); if (err != cudaSuccess) @@ -572,51 +454,35 @@ void VkContext::match_cuda_device_to_vulkan() std::vector VkContext::enumerate_physical_devices() { std::vector result; - - // Create a minimal temporary instance just to enumerate devices. - VkApplicationInfo app_info{}; - app_info.sType = VK_STRUCTURE_TYPE_APPLICATION_INFO; - app_info.pApplicationName = "viz_enumerate_probe"; - app_info.apiVersion = kApiVersion; - - VkInstanceCreateInfo create_info{}; - create_info.sType = VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO; - create_info.pApplicationInfo = &app_info; - - VkInstance instance = VK_NULL_HANDLE; - if (vkCreateInstance(&create_info, nullptr, &instance) != VK_SUCCESS) - { - return result; // Vulkan loader missing or instance creation failed. - } - - uint32_t count = 0; - vkEnumeratePhysicalDevices(instance, &count, nullptr); - if (count == 0) + try { - vkDestroyInstance(instance, nullptr); - return result; + vk::raii::Context ctx{}; + const vk::ApplicationInfo app_info{ + .pApplicationName = "viz_enumerate_probe", + .apiVersion = kApiVersion, + }; + const vk::InstanceCreateInfo create_info{ .pApplicationInfo = &app_info }; + vk::raii::Instance instance{ ctx, create_info }; + vk::raii::PhysicalDevices devices{ instance }; + + result.reserve(devices.size()); + for (size_t i = 0; i < devices.size(); ++i) + { + const auto props = devices[i].getProperties(); + PhysicalDeviceInfo info; + info.index = static_cast(i); + info.name = std::string(props.deviceName.data()); + info.vendor_id = props.vendorID; + info.device_id = props.deviceID; + info.is_discrete = (props.deviceType == vk::PhysicalDeviceType::eDiscreteGpu); + info.meets_requirements = (score_physical_device(*devices[i]) >= 0); + result.push_back(std::move(info)); + } } - - std::vector devices(count); - vkEnumeratePhysicalDevices(instance, &count, devices.data()); - - result.reserve(count); - for (uint32_t i = 0; i < count; ++i) + catch (...) { - VkPhysicalDeviceProperties props; - vkGetPhysicalDeviceProperties(devices[i], &props); - - PhysicalDeviceInfo info; - info.index = i; - info.name = props.deviceName; - info.vendor_id = props.vendorID; - info.device_id = props.deviceID; - info.is_discrete = (props.deviceType == VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU); - info.meets_requirements = (score_physical_device(devices[i]) >= 0); - result.push_back(std::move(info)); + // Loader missing, instance creation failed, or no devices. } - - vkDestroyInstance(instance, nullptr); return result; } From ca18b3d2467ec7ebfefaf1e266f36e8c6266fc6f Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 11:37:30 -0700 Subject: [PATCH 04/20] viz/core: restore API-doc comments on PhysicalDeviceInfo + Config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Over-pruned in 38961999 — these aren't narration, they describe what each Config field controls and what each PhysicalDeviceInfo field maps to in Vulkan. Useful for callers reading the header. Co-Authored-By: Claude Sonnet 4.6 --- src/viz/core/cpp/inc/viz/core/vk_context.hpp | 34 +++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/viz/core/cpp/inc/viz/core/vk_context.hpp b/src/viz/core/cpp/inc/viz/core/vk_context.hpp index 8e6de8649..3e1246330 100644 --- a/src/viz/core/cpp/inc/viz/core/vk_context.hpp +++ b/src/viz/core/cpp/inc/viz/core/vk_context.hpp @@ -13,14 +13,19 @@ namespace viz { // Read-only info about a Vulkan physical device. +// +// Returned by VkContext::enumerate_physical_devices(). Use this to discover +// available GPUs and choose one explicitly via Config::physical_device_index +// when multiple GPUs are present (e.g. servers with two NVIDIA cards). struct PhysicalDeviceInfo { - uint32_t index = 0; - std::string name; - uint32_t vendor_id = 0; - uint32_t device_id = 0; - bool is_discrete = false; - bool meets_requirements = false; + uint32_t index = 0; // Index in vkEnumeratePhysicalDevices order + std::string name; // deviceName from VkPhysicalDeviceProperties + uint32_t vendor_id = 0; // PCI vendor ID (e.g. 0x10DE for NVIDIA) + uint32_t device_id = 0; // PCI device ID + bool is_discrete = false; // True for discrete (dedicated) GPUs + bool meets_requirements = false; // True if suitable for VkContext (API 1.2+, + // queue family, required extensions) }; // Vulkan instance + device + queue + pipeline cache for Televiz. @@ -41,11 +46,24 @@ class VkContext public: struct Config { + // Enables VK_LAYER_KHRONOS_validation if available, plus + // VK_EXT_debug_utils messenger and best-practices + + // synchronization validation features. bool enable_validation = false; + + // Additional instance/device extensions to enable beyond the + // Televiz-required set. std::vector instance_extensions; std::vector device_extensions; - // -1 = auto-pick best; otherwise the explicit index from - // vkEnumeratePhysicalDevices. + + // Physical device selection. + // -1 (default): auto-pick the best suitable device (NVIDIA discrete + // GPUs preferred; must support required extensions). + // >=0: use the device at this index from + // vkEnumeratePhysicalDevices. The device must still + // meet Televiz requirements or init() throws. Use + // enumerate_physical_devices() to discover available + // indices. int physical_device_index = -1; }; From 36db5a0e46d1176c6ff2de57a89419bf2efc76f2 Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 11:39:43 -0700 Subject: [PATCH 05/20] viz/core: migrate RenderTarget internals to vk::raii MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Color + depth attachments (image, memory, view), render pass, and framebuffer become vk::raii::* members, declared parent-first so reverse-order destruction matches Vulkan's child-before-parent requirement (framebuffer → views → images → memory). destroy() / destroy_attachments() collapse from ~30 lines of manual vkDestroy* chain to nullptr-resets. resize() restore-on- failure logic is preserved unchanged. Public raw-handle accessors keep working — return *member_ from the underlying raii types — so viz_session and viz_layers consumers don't need updates. All 52 unit tests pass. Co-Authored-By: Claude Sonnet 4.6 --- .../core/cpp/inc/viz/core/render_target.hpp | 44 +- src/viz/core/cpp/render_target.cpp | 394 ++++++++---------- 2 files changed, 197 insertions(+), 241 deletions(-) diff --git a/src/viz/core/cpp/inc/viz/core/render_target.hpp b/src/viz/core/cpp/inc/viz/core/render_target.hpp index e46fe46e0..c4ac0a74c 100644 --- a/src/viz/core/cpp/inc/viz/core/render_target.hpp +++ b/src/viz/core/cpp/inc/viz/core/render_target.hpp @@ -4,7 +4,7 @@ #pragma once #include -#include +#include #include @@ -26,10 +26,6 @@ class VkContext; // The render pass clears both attachments at load and stores the color // attachment (the depth attachment is dontCare on store — we never read it // back). -// -// The class owns the Vulkan handles (images, image views, memory, render -// pass, framebuffer) and tears them down in destroy() / destructor. It does -// not own the VkContext. class RenderTarget { public: @@ -44,33 +40,31 @@ class RenderTarget // on Vulkan failure or std::invalid_argument if resolution is zero. static std::unique_ptr create(const VkContext& ctx, const Config& config); - // Releases all Vulkan handles. Idempotent. ~RenderTarget(); void destroy(); - // Non-copyable, non-movable for now (owns Vulkan handles). RenderTarget(const RenderTarget&) = delete; RenderTarget& operator=(const RenderTarget&) = delete; RenderTarget(RenderTarget&&) = delete; RenderTarget& operator=(RenderTarget&&) = delete; - // Vulkan handle accessors for the compositor / custom layers. + // Raw-handle accessors for the compositor / custom layers. VkRenderPass render_pass() const noexcept { - return render_pass_; + return *render_pass_; } VkFramebuffer framebuffer() const noexcept { - return framebuffer_; + return *framebuffer_; } VkImage color_image() const noexcept { - return color_image_; + return *color_image_; } VkImageView color_image_view() const noexcept { - return color_view_; + return *color_view_; } VkFormat color_format() const noexcept { @@ -79,11 +73,11 @@ class RenderTarget VkImage depth_image() const noexcept { - return depth_image_; + return *depth_image_; } VkImageView depth_image_view() const noexcept { - return depth_view_; + return *depth_view_; } VkFormat depth_format() const noexcept { @@ -109,24 +103,24 @@ class RenderTarget void create_depth_image(const Config& config); void create_render_pass(); void create_framebuffer(); - void destroy_attachments(); // images + views + memory + framebuffer + void destroy_attachments(); const VkContext* ctx_ = nullptr; - Resolution resolution_{}; VkFormat color_format_ = VK_FORMAT_R8G8B8A8_SRGB; - VkImage color_image_ = VK_NULL_HANDLE; - VkDeviceMemory color_memory_ = VK_NULL_HANDLE; - VkImageView color_view_ = VK_NULL_HANDLE; - VkFormat depth_format_ = VK_FORMAT_D32_SFLOAT; - VkImage depth_image_ = VK_NULL_HANDLE; - VkDeviceMemory depth_memory_ = VK_NULL_HANDLE; - VkImageView depth_view_ = VK_NULL_HANDLE; - VkRenderPass render_pass_ = VK_NULL_HANDLE; - VkFramebuffer framebuffer_ = VK_NULL_HANDLE; + // Declared parent-first so reverse-order destruction tears children + // down before parents (framebuffer → views → images → memory). + vk::raii::DeviceMemory color_memory_{ nullptr }; + vk::raii::Image color_image_{ nullptr }; + vk::raii::ImageView color_view_{ nullptr }; + vk::raii::DeviceMemory depth_memory_{ nullptr }; + vk::raii::Image depth_image_{ nullptr }; + vk::raii::ImageView depth_view_{ nullptr }; + vk::raii::RenderPass render_pass_{ nullptr }; + vk::raii::Framebuffer framebuffer_{ nullptr }; }; } // namespace viz diff --git a/src/viz/core/cpp/render_target.cpp b/src/viz/core/cpp/render_target.cpp index 3767453a7..cd36f0b2a 100644 --- a/src/viz/core/cpp/render_target.cpp +++ b/src/viz/core/cpp/render_target.cpp @@ -6,7 +6,6 @@ #include #include -#include namespace viz { @@ -17,10 +16,9 @@ namespace // Find a memory type matching `type_bits` (bitfield from // VkMemoryRequirements::memoryTypeBits) that has all required `properties`. // Throws if no match (callers should request DEVICE_LOCAL for attachments). -uint32_t find_memory_type(VkPhysicalDevice physical_device, uint32_t type_bits, VkMemoryPropertyFlags properties) +uint32_t find_memory_type(vk::PhysicalDevice physical_device, uint32_t type_bits, vk::MemoryPropertyFlags properties) { - VkPhysicalDeviceMemoryProperties mem_props; - vkGetPhysicalDeviceMemoryProperties(physical_device, &mem_props); + const auto mem_props = physical_device.getMemoryProperties(); for (uint32_t i = 0; i < mem_props.memoryTypeCount; ++i) { const bool type_ok = (type_bits & (1u << i)) != 0; @@ -33,14 +31,6 @@ uint32_t find_memory_type(VkPhysicalDevice physical_device, uint32_t type_bits, throw std::runtime_error("RenderTarget: no Vulkan memory type matching requested properties"); } -void check_vk(VkResult result, const char* what) -{ - if (result != VK_SUCCESS) - { - throw std::runtime_error(std::string("RenderTarget: ") + what + " failed: VkResult=" + std::to_string(result)); - } -} - } // namespace std::unique_ptr RenderTarget::create(const VkContext& ctx, const Config& config) @@ -87,61 +77,19 @@ void RenderTarget::init(const Config& config) void RenderTarget::destroy() { - if (ctx_ == nullptr) - { - return; - } - const VkDevice device = ctx_->device(); - if (device == VK_NULL_HANDLE) - { - return; - } destroy_attachments(); - if (render_pass_ != VK_NULL_HANDLE) - { - vkDestroyRenderPass(device, render_pass_, nullptr); - render_pass_ = VK_NULL_HANDLE; - } + render_pass_ = nullptr; } void RenderTarget::destroy_attachments() { - const VkDevice device = ctx_->device(); - if (framebuffer_ != VK_NULL_HANDLE) - { - vkDestroyFramebuffer(device, framebuffer_, nullptr); - framebuffer_ = VK_NULL_HANDLE; - } - if (depth_view_ != VK_NULL_HANDLE) - { - vkDestroyImageView(device, depth_view_, nullptr); - depth_view_ = VK_NULL_HANDLE; - } - if (depth_image_ != VK_NULL_HANDLE) - { - vkDestroyImage(device, depth_image_, nullptr); - depth_image_ = VK_NULL_HANDLE; - } - if (depth_memory_ != VK_NULL_HANDLE) - { - vkFreeMemory(device, depth_memory_, nullptr); - depth_memory_ = VK_NULL_HANDLE; - } - if (color_view_ != VK_NULL_HANDLE) - { - vkDestroyImageView(device, color_view_, nullptr); - color_view_ = VK_NULL_HANDLE; - } - if (color_image_ != VK_NULL_HANDLE) - { - vkDestroyImage(device, color_image_, nullptr); - color_image_ = VK_NULL_HANDLE; - } - if (color_memory_ != VK_NULL_HANDLE) - { - vkFreeMemory(device, color_memory_, nullptr); - color_memory_ = VK_NULL_HANDLE; - } + framebuffer_ = nullptr; + depth_view_ = nullptr; + depth_image_ = nullptr; + depth_memory_ = nullptr; + color_view_ = nullptr; + color_image_ = nullptr; + color_memory_ = nullptr; } void RenderTarget::resize(Resolution new_size) @@ -191,176 +139,190 @@ void RenderTarget::resize(Resolution new_size) void RenderTarget::create_color_image(const Config& config) { - const VkDevice device = ctx_->device(); - - VkImageCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO; - info.imageType = VK_IMAGE_TYPE_2D; - info.format = color_format_; - info.extent = { config.resolution.width, config.resolution.height, 1 }; - info.mipLevels = 1; - info.arrayLayers = 1; - info.samples = VK_SAMPLE_COUNT_1_BIT; - info.tiling = VK_IMAGE_TILING_OPTIMAL; - // COLOR_ATTACHMENT for rendering, TRANSFER_SRC for readback / blit-to-display, - // SAMPLED for future custom layers that read prior frames. - info.usage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_SAMPLED_BIT; - info.sharingMode = VK_SHARING_MODE_EXCLUSIVE; - info.initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; - - check_vk(vkCreateImage(device, &info, nullptr, &color_image_), "vkCreateImage(color)"); - - VkMemoryRequirements reqs; - vkGetImageMemoryRequirements(device, color_image_, &reqs); - - VkMemoryAllocateInfo alloc{}; - alloc.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO; - alloc.allocationSize = reqs.size; - alloc.memoryTypeIndex = - find_memory_type(ctx_->physical_device(), reqs.memoryTypeBits, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT); - check_vk(vkAllocateMemory(device, &alloc, nullptr, &color_memory_), "vkAllocateMemory(color)"); - check_vk(vkBindImageMemory(device, color_image_, color_memory_, 0), "vkBindImageMemory(color)"); - - VkImageViewCreateInfo view_info{}; - view_info.sType = VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO; - view_info.image = color_image_; - view_info.viewType = VK_IMAGE_VIEW_TYPE_2D; - view_info.format = color_format_; - view_info.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; - view_info.subresourceRange.baseMipLevel = 0; - view_info.subresourceRange.levelCount = 1; - view_info.subresourceRange.baseArrayLayer = 0; - view_info.subresourceRange.layerCount = 1; - check_vk(vkCreateImageView(device, &view_info, nullptr, &color_view_), "vkCreateImageView(color)"); + const auto& device = ctx_->raii_device(); + + const vk::ImageCreateInfo info{ + .imageType = vk::ImageType::e2D, + .format = static_cast(color_format_), + .extent = { config.resolution.width, config.resolution.height, 1 }, + .mipLevels = 1, + .arrayLayers = 1, + .samples = vk::SampleCountFlagBits::e1, + .tiling = vk::ImageTiling::eOptimal, + // COLOR_ATTACHMENT for rendering, TRANSFER_SRC for readback / blit-to-display, + // SAMPLED for future custom layers that read prior frames. + .usage = vk::ImageUsageFlagBits::eColorAttachment | vk::ImageUsageFlagBits::eTransferSrc | + vk::ImageUsageFlagBits::eSampled, + .sharingMode = vk::SharingMode::eExclusive, + .initialLayout = vk::ImageLayout::eUndefined, + }; + color_image_ = vk::raii::Image{ device, info }; + + const auto reqs = color_image_.getMemoryRequirements(); + const vk::MemoryAllocateInfo alloc{ + .allocationSize = reqs.size, + .memoryTypeIndex = find_memory_type( + ctx_->raii_physical_device(), reqs.memoryTypeBits, vk::MemoryPropertyFlagBits::eDeviceLocal), + }; + color_memory_ = vk::raii::DeviceMemory{ device, alloc }; + color_image_.bindMemory(*color_memory_, 0); + + const vk::ImageViewCreateInfo view_info{ + .image = *color_image_, + .viewType = vk::ImageViewType::e2D, + .format = static_cast(color_format_), + .subresourceRange = + { + .aspectMask = vk::ImageAspectFlagBits::eColor, + .baseMipLevel = 0, + .levelCount = 1, + .baseArrayLayer = 0, + .layerCount = 1, + }, + }; + color_view_ = vk::raii::ImageView{ device, view_info }; } void RenderTarget::create_depth_image(const Config& config) { - const VkDevice device = ctx_->device(); - - VkImageCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO; - info.imageType = VK_IMAGE_TYPE_2D; - info.format = depth_format_; - info.extent = { config.resolution.width, config.resolution.height, 1 }; - info.mipLevels = 1; - info.arrayLayers = 1; - info.samples = VK_SAMPLE_COUNT_1_BIT; - info.tiling = VK_IMAGE_TILING_OPTIMAL; - info.usage = VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT; - info.sharingMode = VK_SHARING_MODE_EXCLUSIVE; - info.initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; - check_vk(vkCreateImage(device, &info, nullptr, &depth_image_), "vkCreateImage(depth)"); - - VkMemoryRequirements reqs; - vkGetImageMemoryRequirements(device, depth_image_, &reqs); - - VkMemoryAllocateInfo alloc{}; - alloc.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO; - alloc.allocationSize = reqs.size; - alloc.memoryTypeIndex = - find_memory_type(ctx_->physical_device(), reqs.memoryTypeBits, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT); - check_vk(vkAllocateMemory(device, &alloc, nullptr, &depth_memory_), "vkAllocateMemory(depth)"); - check_vk(vkBindImageMemory(device, depth_image_, depth_memory_, 0), "vkBindImageMemory(depth)"); - - VkImageViewCreateInfo view_info{}; - view_info.sType = VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO; - view_info.image = depth_image_; - view_info.viewType = VK_IMAGE_VIEW_TYPE_2D; - view_info.format = depth_format_; - view_info.subresourceRange.aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT; - view_info.subresourceRange.baseMipLevel = 0; - view_info.subresourceRange.levelCount = 1; - view_info.subresourceRange.baseArrayLayer = 0; - view_info.subresourceRange.layerCount = 1; - check_vk(vkCreateImageView(device, &view_info, nullptr, &depth_view_), "vkCreateImageView(depth)"); + const auto& device = ctx_->raii_device(); + + const vk::ImageCreateInfo info{ + .imageType = vk::ImageType::e2D, + .format = static_cast(depth_format_), + .extent = { config.resolution.width, config.resolution.height, 1 }, + .mipLevels = 1, + .arrayLayers = 1, + .samples = vk::SampleCountFlagBits::e1, + .tiling = vk::ImageTiling::eOptimal, + .usage = vk::ImageUsageFlagBits::eDepthStencilAttachment, + .sharingMode = vk::SharingMode::eExclusive, + .initialLayout = vk::ImageLayout::eUndefined, + }; + depth_image_ = vk::raii::Image{ device, info }; + + const auto reqs = depth_image_.getMemoryRequirements(); + const vk::MemoryAllocateInfo alloc{ + .allocationSize = reqs.size, + .memoryTypeIndex = find_memory_type( + ctx_->raii_physical_device(), reqs.memoryTypeBits, vk::MemoryPropertyFlagBits::eDeviceLocal), + }; + depth_memory_ = vk::raii::DeviceMemory{ device, alloc }; + depth_image_.bindMemory(*depth_memory_, 0); + + const vk::ImageViewCreateInfo view_info{ + .image = *depth_image_, + .viewType = vk::ImageViewType::e2D, + .format = static_cast(depth_format_), + .subresourceRange = + { + .aspectMask = vk::ImageAspectFlagBits::eDepth, + .baseMipLevel = 0, + .levelCount = 1, + .baseArrayLayer = 0, + .layerCount = 1, + }, + }; + depth_view_ = vk::raii::ImageView{ device, view_info }; } void RenderTarget::create_render_pass() { - const VkDevice device = ctx_->device(); - - std::array attachments{}; - // Color: clear on load, store, transition to TRANSFER_SRC so the - // compositor / readback path can copy without an extra pipeline barrier. - attachments[0].format = color_format_; - attachments[0].samples = VK_SAMPLE_COUNT_1_BIT; - attachments[0].loadOp = VK_ATTACHMENT_LOAD_OP_CLEAR; - attachments[0].storeOp = VK_ATTACHMENT_STORE_OP_STORE; - attachments[0].stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE; - attachments[0].stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE; - attachments[0].initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; - attachments[0].finalLayout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL; - // Depth: clear on load, dontCare on store (we never read it back). - attachments[1].format = depth_format_; - attachments[1].samples = VK_SAMPLE_COUNT_1_BIT; - attachments[1].loadOp = VK_ATTACHMENT_LOAD_OP_CLEAR; - attachments[1].storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE; - attachments[1].stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE; - attachments[1].stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE; - attachments[1].initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; - attachments[1].finalLayout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; - - VkAttachmentReference color_ref{}; - color_ref.attachment = 0; - color_ref.layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; - - VkAttachmentReference depth_ref{}; - depth_ref.attachment = 1; - depth_ref.layout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; - - VkSubpassDescription subpass{}; - subpass.pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS; - subpass.colorAttachmentCount = 1; - subpass.pColorAttachments = &color_ref; - subpass.pDepthStencilAttachment = &depth_ref; + const auto& device = ctx_->raii_device(); + + const std::array attachments{ { + // Color: clear on load, store, transition to TRANSFER_SRC so the + // compositor / readback path can copy without an extra pipeline barrier. + { + .format = static_cast(color_format_), + .samples = vk::SampleCountFlagBits::e1, + .loadOp = vk::AttachmentLoadOp::eClear, + .storeOp = vk::AttachmentStoreOp::eStore, + .stencilLoadOp = vk::AttachmentLoadOp::eDontCare, + .stencilStoreOp = vk::AttachmentStoreOp::eDontCare, + .initialLayout = vk::ImageLayout::eUndefined, + .finalLayout = vk::ImageLayout::eTransferSrcOptimal, + }, + // Depth: clear on load, dontCare on store (we never read it back). + { + .format = static_cast(depth_format_), + .samples = vk::SampleCountFlagBits::e1, + .loadOp = vk::AttachmentLoadOp::eClear, + .storeOp = vk::AttachmentStoreOp::eDontCare, + .stencilLoadOp = vk::AttachmentLoadOp::eDontCare, + .stencilStoreOp = vk::AttachmentStoreOp::eDontCare, + .initialLayout = vk::ImageLayout::eUndefined, + .finalLayout = vk::ImageLayout::eDepthStencilAttachmentOptimal, + }, + } }; + + const vk::AttachmentReference color_ref{ + .attachment = 0, + .layout = vk::ImageLayout::eColorAttachmentOptimal, + }; + const vk::AttachmentReference depth_ref{ + .attachment = 1, + .layout = vk::ImageLayout::eDepthStencilAttachmentOptimal, + }; + + const vk::SubpassDescription subpass{ + .pipelineBindPoint = vk::PipelineBindPoint::eGraphics, + .colorAttachmentCount = 1, + .pColorAttachments = &color_ref, + .pDepthStencilAttachment = &depth_ref, + }; // External -> subpass: ensure prior writes / readbacks complete before // we clear and render. Subpass -> external: render output is available // to subsequent transfer reads (matches color attachment finalLayout). - std::array deps{}; - deps[0].srcSubpass = VK_SUBPASS_EXTERNAL; - deps[0].dstSubpass = 0; - deps[0].srcStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT | VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT | - VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT; - deps[0].dstStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT | VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT; - deps[0].srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT; - deps[0].dstAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; - deps[1].srcSubpass = 0; - deps[1].dstSubpass = VK_SUBPASS_EXTERNAL; - deps[1].srcStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; - deps[1].dstStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; - deps[1].srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT; - deps[1].dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT; - - VkRenderPassCreateInfo rp_info{}; - rp_info.sType = VK_STRUCTURE_TYPE_RENDER_PASS_CREATE_INFO; - rp_info.attachmentCount = static_cast(attachments.size()); - rp_info.pAttachments = attachments.data(); - rp_info.subpassCount = 1; - rp_info.pSubpasses = &subpass; - rp_info.dependencyCount = static_cast(deps.size()); - rp_info.pDependencies = deps.data(); - - check_vk(vkCreateRenderPass(device, &rp_info, nullptr, &render_pass_), "vkCreateRenderPass"); + const std::array deps{ { + { + .srcSubpass = VK_SUBPASS_EXTERNAL, + .dstSubpass = 0, + .srcStageMask = vk::PipelineStageFlagBits::eTransfer | vk::PipelineStageFlagBits::eColorAttachmentOutput | + vk::PipelineStageFlagBits::eEarlyFragmentTests, + .dstStageMask = + vk::PipelineStageFlagBits::eColorAttachmentOutput | vk::PipelineStageFlagBits::eEarlyFragmentTests, + .srcAccessMask = vk::AccessFlagBits::eTransferRead, + .dstAccessMask = vk::AccessFlagBits::eColorAttachmentWrite | vk::AccessFlagBits::eDepthStencilAttachmentWrite, + }, + { + .srcSubpass = 0, + .dstSubpass = VK_SUBPASS_EXTERNAL, + .srcStageMask = vk::PipelineStageFlagBits::eColorAttachmentOutput, + .dstStageMask = vk::PipelineStageFlagBits::eTransfer, + .srcAccessMask = vk::AccessFlagBits::eColorAttachmentWrite, + .dstAccessMask = vk::AccessFlagBits::eTransferRead, + }, + } }; + + const vk::RenderPassCreateInfo rp_info{ + .attachmentCount = static_cast(attachments.size()), + .pAttachments = attachments.data(), + .subpassCount = 1, + .pSubpasses = &subpass, + .dependencyCount = static_cast(deps.size()), + .pDependencies = deps.data(), + }; + + render_pass_ = vk::raii::RenderPass{ device, rp_info }; } void RenderTarget::create_framebuffer() { - const VkDevice device = ctx_->device(); - - const std::array attachments{ color_view_, depth_view_ }; - - VkFramebufferCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_FRAMEBUFFER_CREATE_INFO; - info.renderPass = render_pass_; - info.attachmentCount = static_cast(attachments.size()); - info.pAttachments = attachments.data(); - info.width = resolution_.width; - info.height = resolution_.height; - info.layers = 1; - - check_vk(vkCreateFramebuffer(device, &info, nullptr, &framebuffer_), "vkCreateFramebuffer"); + const auto& device = ctx_->raii_device(); + const std::array attachments{ *color_view_, *depth_view_ }; + + const vk::FramebufferCreateInfo info{ + .renderPass = *render_pass_, + .attachmentCount = static_cast(attachments.size()), + .pAttachments = attachments.data(), + .width = resolution_.width, + .height = resolution_.height, + .layers = 1, + }; + + framebuffer_ = vk::raii::Framebuffer{ device, info }; } } // namespace viz From 2f1c34d348a9420a0e4b3262e322451b06820ea5 Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 11:44:24 -0700 Subject: [PATCH 06/20] viz/core: migrate FrameSync internals to vk::raii Fence + two semaphores become vk::raii::Fence / vk::raii::Semaphore members. destroy() collapses to nullptr-resets. wait() / reset() go through raii_device().waitForFences / resetFences. Public raw-handle accessors keep returning the underlying handles via *member_ for compositor consumers. All 52 unit tests pass. Co-Authored-By: Claude Sonnet 4.6 --- src/viz/core/cpp/frame_sync.cpp | 69 +++++--------------- src/viz/core/cpp/inc/viz/core/frame_sync.hpp | 16 ++--- 2 files changed, 24 insertions(+), 61 deletions(-) diff --git a/src/viz/core/cpp/frame_sync.cpp b/src/viz/core/cpp/frame_sync.cpp index 52ac47168..dc8ddad87 100644 --- a/src/viz/core/cpp/frame_sync.cpp +++ b/src/viz/core/cpp/frame_sync.cpp @@ -5,24 +5,10 @@ #include #include -#include namespace viz { -namespace -{ - -void check_vk(VkResult result, const char* what) -{ - if (result != VK_SUCCESS) - { - throw std::runtime_error(std::string("FrameSync: ") + what + " failed: VkResult=" + std::to_string(result)); - } -} - -} // namespace - std::unique_ptr FrameSync::create(const VkContext& ctx) { if (!ctx.is_initialized()) @@ -45,21 +31,17 @@ FrameSync::~FrameSync() void FrameSync::init() { - const VkDevice device = ctx_->device(); + const auto& device = ctx_->raii_device(); - VkFenceCreateInfo fence_info{}; - fence_info.sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO; // Start signaled so the first wait()/reset() pair is a no-op. - fence_info.flags = VK_FENCE_CREATE_SIGNALED_BIT; - - VkSemaphoreCreateInfo sem_info{}; - sem_info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO; + const vk::FenceCreateInfo fence_info{ .flags = vk::FenceCreateFlagBits::eSignaled }; + const vk::SemaphoreCreateInfo sem_info{}; try { - check_vk(vkCreateFence(device, &fence_info, nullptr, &in_flight_fence_), "vkCreateFence"); - check_vk(vkCreateSemaphore(device, &sem_info, nullptr, &image_available_), "vkCreateSemaphore(image_available)"); - check_vk(vkCreateSemaphore(device, &sem_info, nullptr, &render_complete_), "vkCreateSemaphore(render_complete)"); + in_flight_fence_ = vk::raii::Fence{ device, fence_info }; + image_available_ = vk::raii::Semaphore{ device, sem_info }; + render_complete_ = vk::raii::Semaphore{ device, sem_info }; } catch (...) { @@ -70,48 +52,31 @@ void FrameSync::init() void FrameSync::destroy() { - if (ctx_ == nullptr) - { - return; - } - const VkDevice device = ctx_->device(); - if (device == VK_NULL_HANDLE) - { - return; - } - if (render_complete_ != VK_NULL_HANDLE) - { - vkDestroySemaphore(device, render_complete_, nullptr); - render_complete_ = VK_NULL_HANDLE; - } - if (image_available_ != VK_NULL_HANDLE) - { - vkDestroySemaphore(device, image_available_, nullptr); - image_available_ = VK_NULL_HANDLE; - } - if (in_flight_fence_ != VK_NULL_HANDLE) - { - vkDestroyFence(device, in_flight_fence_, nullptr); - in_flight_fence_ = VK_NULL_HANDLE; - } + render_complete_ = nullptr; + image_available_ = nullptr; + in_flight_fence_ = nullptr; } void FrameSync::wait(uint64_t timeout_ns) { - if (in_flight_fence_ == VK_NULL_HANDLE) + if (!*in_flight_fence_) { throw std::logic_error("FrameSync::wait: not initialized"); } - check_vk(vkWaitForFences(ctx_->device(), 1, &in_flight_fence_, VK_TRUE, timeout_ns), "vkWaitForFences"); + const vk::Result r = ctx_->raii_device().waitForFences({ *in_flight_fence_ }, VK_TRUE, timeout_ns); + if (r != vk::Result::eSuccess) + { + throw std::runtime_error("FrameSync: vkWaitForFences returned " + vk::to_string(r)); + } } void FrameSync::reset() { - if (in_flight_fence_ == VK_NULL_HANDLE) + if (!*in_flight_fence_) { throw std::logic_error("FrameSync::reset: not initialized"); } - check_vk(vkResetFences(ctx_->device(), 1, &in_flight_fence_), "vkResetFences"); + ctx_->raii_device().resetFences({ *in_flight_fence_ }); } } // namespace viz diff --git a/src/viz/core/cpp/inc/viz/core/frame_sync.hpp b/src/viz/core/cpp/inc/viz/core/frame_sync.hpp index 23f79af41..8d99b504b 100644 --- a/src/viz/core/cpp/inc/viz/core/frame_sync.hpp +++ b/src/viz/core/cpp/inc/viz/core/frame_sync.hpp @@ -3,7 +3,7 @@ #pragma once -#include +#include #include @@ -31,8 +31,6 @@ class VkContext; class FrameSync { public: - // Creates the three sync objects. Throws std::runtime_error on Vulkan - // failure or std::invalid_argument if ctx is not initialized. static std::unique_ptr create(const VkContext& ctx); ~FrameSync(); @@ -53,15 +51,15 @@ class FrameSync VkFence in_flight_fence() const noexcept { - return in_flight_fence_; + return *in_flight_fence_; } VkSemaphore image_available_semaphore() const noexcept { - return image_available_; + return *image_available_; } VkSemaphore render_complete_semaphore() const noexcept { - return render_complete_; + return *render_complete_; } private: @@ -70,9 +68,9 @@ class FrameSync const VkContext* ctx_ = nullptr; - VkFence in_flight_fence_ = VK_NULL_HANDLE; - VkSemaphore image_available_ = VK_NULL_HANDLE; - VkSemaphore render_complete_ = VK_NULL_HANDLE; + vk::raii::Fence in_flight_fence_{ nullptr }; + vk::raii::Semaphore image_available_{ nullptr }; + vk::raii::Semaphore render_complete_{ nullptr }; }; } // namespace viz From f4005b08552b2d619488fc56f874ac1b3b72320c Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 11:50:06 -0700 Subject: [PATCH 07/20] viz/core: migrate DeviceImage internals to vk::raii MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The riskiest single file in the migration because of the CUDA-Vulkan interop: external memory FD export, timeline semaphore FD export, and a cudaMipmappedArray imported from those. Vulkan side: - VkImage / VkDeviceMemory / VkImageView / VkSemaphore / VkCommandPool become vk::raii::* members. - pNext chains use vk::StructureChain: Image: ImageCreateInfo + ExternalMemoryImageCreateInfo Memory: MemoryAllocateInfo + ExportMemoryAllocateInfo Semaphore: SemaphoreCreateInfo + ExportSemaphoreCreateInfo + SemaphoreTypeCreateInfo (timeline) - vkGetMemoryFdKHR / vkGetSemaphoreFdKHR replaced with the vk::raii::Device member functions (vulkan-hpp dispatches the extension proc internally). - One-shot layout transition uses vk::raii::CommandBuffers and vk::raii::Queue::submit() — no more manual cmd-buffer lifetime. destroy() preserves explicit ordering: cudaDeviceSynchronize + CUDA-side teardown FIRST (the imports are pinned against the Vulkan handles), THEN waitIdle + raii nullptr-resets. CUDA handles stay raw — interop boundary. All 52 unit tests pass. -50 LOC net. Co-Authored-By: Claude Sonnet 4.6 --- src/viz/core/cpp/device_image.cpp | 367 ++++++++---------- .../core/cpp/inc/viz/core/device_image.hpp | 24 +- 2 files changed, 164 insertions(+), 227 deletions(-) diff --git a/src/viz/core/cpp/device_image.cpp b/src/viz/core/cpp/device_image.cpp index d6b53246f..2e1130053 100644 --- a/src/viz/core/cpp/device_image.cpp +++ b/src/viz/core/cpp/device_image.cpp @@ -36,14 +36,6 @@ namespace viz namespace { -void check_vk(VkResult result, const char* what) -{ - if (result != VK_SUCCESS) - { - throw std::runtime_error(std::string("DeviceImage: ") + what + " failed: VkResult=" + std::to_string(result)); - } -} - void check_cuda(cudaError_t result, const char* what) { if (result != cudaSuccess) @@ -52,10 +44,9 @@ void check_cuda(cudaError_t result, const char* what) } } -uint32_t find_memory_type(VkPhysicalDevice physical_device, uint32_t type_bits, VkMemoryPropertyFlags properties) +uint32_t find_memory_type(vk::PhysicalDevice physical_device, uint32_t type_bits, vk::MemoryPropertyFlags properties) { - VkPhysicalDeviceMemoryProperties mem_props; - vkGetPhysicalDeviceMemoryProperties(physical_device, &mem_props); + const auto mem_props = physical_device.getMemoryProperties(); for (uint32_t i = 0; i < mem_props.memoryTypeCount; ++i) { if ((type_bits & (1u << i)) != 0 && (mem_props.memoryTypes[i].propertyFlags & properties) == properties) @@ -172,8 +163,9 @@ void DeviceImage::destroy() (void)cudaSetDevice(ctx_->cuda_device_id()); } - // CUDA side first — VkDeviceMemory must outlive the CUDA - // mapping. Sync drains any caller-issued async work first. + // CUDA side first — the imports are pinned against the Vulkan + // memory + semaphore handles, so they must close before the + // raii types release the underlying VkDeviceMemory / VkSemaphore. if (cuda_mipmapped_array_ != nullptr || cuda_external_memory_ != nullptr || cuda_cuda_done_writing_ != nullptr) { (void)cudaDeviceSynchronize(); @@ -202,133 +194,109 @@ void DeviceImage::destroy() memory_fd_ = -1; } - if (ctx_ == nullptr) - { - return; - } - const VkDevice device = ctx_->device(); - if (device == VK_NULL_HANDLE) - { - return; - } // Wait for all GPU work to retire before tearing down Vulkan - // resources. - (void)vkDeviceWaitIdle(device); - if (cuda_done_writing_ != VK_NULL_HANDLE) - { - vkDestroySemaphore(device, cuda_done_writing_, nullptr); - cuda_done_writing_ = VK_NULL_HANDLE; - } - if (command_pool_ != VK_NULL_HANDLE) - { - vkDestroyCommandPool(device, command_pool_, nullptr); - command_pool_ = VK_NULL_HANDLE; - } - if (image_view_ != VK_NULL_HANDLE) - { - vkDestroyImageView(device, image_view_, nullptr); - image_view_ = VK_NULL_HANDLE; - } - if (image_ != VK_NULL_HANDLE) + // resources (raii destruction below would do it too, but we + // want it before the explicit nulling so layout transitions in + // flight aren't racing). + if (ctx_ != nullptr && ctx_->is_initialized()) { - vkDestroyImage(device, image_, nullptr); - image_ = VK_NULL_HANDLE; - } - if (memory_ != VK_NULL_HANDLE) - { - vkFreeMemory(device, memory_, nullptr); - memory_ = VK_NULL_HANDLE; + ctx_->raii_device().waitIdle(); } + + cuda_done_writing_ = nullptr; + command_pool_ = nullptr; + image_view_ = nullptr; + image_ = nullptr; + memory_ = nullptr; current_layout_ = VK_IMAGE_LAYOUT_UNDEFINED; } void DeviceImage::create_vk_image_with_external_memory() { - const VkDevice device = ctx_->device(); - - // Image with external-memory export flag. Optimal tiling — CUDA - // accesses the image via cudaArray_t, not raw memory, so opaque - // GPU layout is fine. - VkExternalMemoryImageCreateInfo ext_image_info{}; - ext_image_info.sType = VK_STRUCTURE_TYPE_EXTERNAL_MEMORY_IMAGE_CREATE_INFO; - ext_image_info.handleTypes = VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT; - - VkImageCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO; - info.pNext = &ext_image_info; - info.imageType = VK_IMAGE_TYPE_2D; - // Storage in linear-space format (UNORM); we'll attach the SRGB - // view in create_vk_image_view(). VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT - // is what allows view format != image format among compatible - // formats (UNORM <-> SRGB are in the same compatibility class). - info.flags = VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT; - info.format = to_vk_storage_format(format_); - info.extent = { resolution_.width, resolution_.height, 1 }; - info.mipLevels = 1; // Single level. If XR distance views show - // moiré, expose mipLevels via Config and - // generate via vkCmdBlitImage pre-render. - info.arrayLayers = 1; - info.samples = VK_SAMPLE_COUNT_1_BIT; - info.tiling = VK_IMAGE_TILING_OPTIMAL; - info.usage = VK_IMAGE_USAGE_SAMPLED_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT; - info.sharingMode = VK_SHARING_MODE_EXCLUSIVE; - info.initialLayout = VK_IMAGE_LAYOUT_UNDEFINED; - - check_vk(vkCreateImage(device, &info, nullptr, &image_), "vkCreateImage"); - - VkMemoryRequirements reqs; - vkGetImageMemoryRequirements(device, image_, &reqs); + const auto& device = ctx_->raii_device(); + + // Optimal tiling — CUDA accesses the image via cudaArray_t, not + // raw memory, so opaque GPU layout is fine. + vk::StructureChain image_chain{ + vk::ImageCreateInfo{ + // Storage in linear-space format (UNORM); SRGB view + // attached in create_vk_image_view(). + // VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT is what allows view + // format != image format among compatible formats + // (UNORM <-> SRGB are in the same compatibility class). + .flags = vk::ImageCreateFlagBits::eMutableFormat, + .imageType = vk::ImageType::e2D, + .format = static_cast(to_vk_storage_format(format_)), + .extent = { resolution_.width, resolution_.height, 1 }, + // Single level. If XR distance views show moiré, expose + // mipLevels via Config and generate via vkCmdBlitImage + // pre-render. + .mipLevels = 1, + .arrayLayers = 1, + .samples = vk::SampleCountFlagBits::e1, + .tiling = vk::ImageTiling::eOptimal, + .usage = vk::ImageUsageFlagBits::eSampled | vk::ImageUsageFlagBits::eTransferDst | + vk::ImageUsageFlagBits::eTransferSrc, + .sharingMode = vk::SharingMode::eExclusive, + .initialLayout = vk::ImageLayout::eUndefined, + }, + vk::ExternalMemoryImageCreateInfo{ + .handleTypes = vk::ExternalMemoryHandleTypeFlagBits::eOpaqueFd, + }, + }; + image_ = vk::raii::Image{ device, image_chain.get() }; + + const auto reqs = image_.getMemoryRequirements(); // Device-local + exportable as POSIX fd. Generic allocation // (no VkMemoryDedicatedAllocateInfo) suffices for sampled 2D. - VkExportMemoryAllocateInfo export_info{}; - export_info.sType = VK_STRUCTURE_TYPE_EXPORT_MEMORY_ALLOCATE_INFO; - export_info.handleTypes = VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT; - - VkMemoryAllocateInfo alloc{}; - alloc.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO; - alloc.pNext = &export_info; - alloc.allocationSize = reqs.size; - alloc.memoryTypeIndex = - find_memory_type(ctx_->physical_device(), reqs.memoryTypeBits, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT); - check_vk(vkAllocateMemory(device, &alloc, nullptr, &memory_), "vkAllocateMemory"); - check_vk(vkBindImageMemory(device, image_, memory_, 0), "vkBindImageMemory"); - - auto vkGetMemoryFdKHR = reinterpret_cast(vkGetDeviceProcAddr(device, "vkGetMemoryFdKHR")); - if (vkGetMemoryFdKHR == nullptr) - { - throw std::runtime_error( - "DeviceImage: vkGetMemoryFdKHR not available " - "(VK_KHR_external_memory_fd not enabled?)"); - } - VkMemoryGetFdInfoKHR fd_info{}; - fd_info.sType = VK_STRUCTURE_TYPE_MEMORY_GET_FD_INFO_KHR; - fd_info.memory = memory_; - fd_info.handleType = VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT; - check_vk(vkGetMemoryFdKHR(device, &fd_info, &memory_fd_), "vkGetMemoryFdKHR"); + vk::StructureChain alloc_chain{ + vk::MemoryAllocateInfo{ + .allocationSize = reqs.size, + .memoryTypeIndex = find_memory_type( + ctx_->raii_physical_device(), reqs.memoryTypeBits, vk::MemoryPropertyFlagBits::eDeviceLocal), + }, + vk::ExportMemoryAllocateInfo{ + .handleTypes = vk::ExternalMemoryHandleTypeFlagBits::eOpaqueFd, + }, + }; + memory_ = vk::raii::DeviceMemory{ device, alloc_chain.get() }; + image_.bindMemory(*memory_, 0); + + memory_fd_ = device.getMemoryFdKHR({ + .memory = *memory_, + .handleType = vk::ExternalMemoryHandleTypeFlagBits::eOpaqueFd, + }); // Used only for transition_to_*; tiny pool, default flags. - VkCommandPoolCreateInfo pool_info{}; - pool_info.sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO; - pool_info.flags = VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT; - pool_info.queueFamilyIndex = ctx_->queue_family_index(); - check_vk(vkCreateCommandPool(device, &pool_info, nullptr, &command_pool_), "vkCreateCommandPool"); + command_pool_ = vk::raii::CommandPool{ + device, + vk::CommandPoolCreateInfo{ + .flags = vk::CommandPoolCreateFlagBits::eResetCommandBuffer, + .queueFamilyIndex = ctx_->queue_family_index(), + }, + }; } void DeviceImage::create_vk_image_view() { - VkImageViewCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO; - info.image = image_; - info.viewType = VK_IMAGE_VIEW_TYPE_2D; - info.format = vk_format_; - info.subresourceRange.aspectMask = - (format_ == PixelFormat::kD32F) ? VK_IMAGE_ASPECT_DEPTH_BIT : VK_IMAGE_ASPECT_COLOR_BIT; - info.subresourceRange.baseMipLevel = 0; - info.subresourceRange.levelCount = 1; - info.subresourceRange.baseArrayLayer = 0; - info.subresourceRange.layerCount = 1; - check_vk(vkCreateImageView(ctx_->device(), &info, nullptr, &image_view_), "vkCreateImageView"); + image_view_ = vk::raii::ImageView{ + ctx_->raii_device(), + vk::ImageViewCreateInfo{ + .image = *image_, + .viewType = vk::ImageViewType::e2D, + .format = static_cast(vk_format_), + .subresourceRange = + { + .aspectMask = (format_ == PixelFormat::kD32F) ? vk::ImageAspectFlagBits::eDepth + : vk::ImageAspectFlagBits::eColor, + .baseMipLevel = 0, + .levelCount = 1, + .baseArrayLayer = 0, + .layerCount = 1, + }, + }, + }; } void DeviceImage::import_to_cuda() @@ -337,8 +305,7 @@ void DeviceImage::import_to_cuda() // init thread, re-pin here for worker-thread create() callers. check_cuda(cudaSetDevice(ctx_->cuda_device_id()), "cudaSetDevice"); - VkMemoryRequirements reqs; - vkGetImageMemoryRequirements(ctx_->device(), image_, &reqs); + const auto reqs = image_.getMemoryRequirements(); cudaExternalMemoryHandleDesc ext_desc{}; ext_desc.type = cudaExternalMemoryHandleTypeOpaqueFd; @@ -366,41 +333,26 @@ void DeviceImage::import_to_cuda() void DeviceImage::create_interop_semaphores() { - const VkDevice device = ctx_->device(); - - auto vkGetSemaphoreFdKHR = - reinterpret_cast(vkGetDeviceProcAddr(device, "vkGetSemaphoreFdKHR")); - if (vkGetSemaphoreFdKHR == nullptr) - { - throw std::runtime_error( - "DeviceImage: vkGetSemaphoreFdKHR not available " - "(VK_KHR_external_semaphore_fd not enabled?)"); - } + const auto& device = ctx_->raii_device(); // Timeline semaphore (initial value 0) exported via OPAQUE_FD and - // imported into CUDA. CUDA dups the fd internally; we close ours - // after the import. - VkSemaphoreTypeCreateInfo type_info{}; - type_info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_TYPE_CREATE_INFO; - type_info.semaphoreType = VK_SEMAPHORE_TYPE_TIMELINE; - type_info.initialValue = 0; - - VkExportSemaphoreCreateInfo export_info{}; - export_info.sType = VK_STRUCTURE_TYPE_EXPORT_SEMAPHORE_CREATE_INFO; - export_info.pNext = &type_info; - export_info.handleTypes = VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_OPAQUE_FD_BIT; - - VkSemaphoreCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO; - info.pNext = &export_info; - check_vk(vkCreateSemaphore(device, &info, nullptr, &cuda_done_writing_), "vkCreateSemaphore"); - - int fd = -1; - VkSemaphoreGetFdInfoKHR fd_info{}; - fd_info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_GET_FD_INFO_KHR; - fd_info.semaphore = cuda_done_writing_; - fd_info.handleType = VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_OPAQUE_FD_BIT; - check_vk(vkGetSemaphoreFdKHR(device, &fd_info, &fd), "vkGetSemaphoreFdKHR"); + // imported into CUDA. + vk::StructureChain sem_chain{ + vk::SemaphoreCreateInfo{}, + vk::ExportSemaphoreCreateInfo{ + .handleTypes = vk::ExternalSemaphoreHandleTypeFlagBits::eOpaqueFd, + }, + vk::SemaphoreTypeCreateInfo{ + .semaphoreType = vk::SemaphoreType::eTimeline, + .initialValue = 0, + }, + }; + cuda_done_writing_ = vk::raii::Semaphore{ device, sem_chain.get() }; + + const int fd = device.getSemaphoreFdKHR({ + .semaphore = *cuda_done_writing_, + .handleType = vk::ExternalSemaphoreHandleTypeFlagBits::eOpaqueFd, + }); cudaExternalSemaphoreHandleDesc ext_desc{}; ext_desc.type = cudaExternalSemaphoreHandleTypeTimelineSemaphoreFd; @@ -412,6 +364,7 @@ void DeviceImage::create_interop_semaphores() throw std::runtime_error(std::string("DeviceImage: cudaImportExternalSemaphore(cuda_done_writing) failed: ") + cudaGetErrorString(err)); } + // CUDA dup'd the fd internally; close ours so we don't leak. close_fd(fd); } @@ -465,61 +418,49 @@ void DeviceImage::run_one_shot_layout_transition(VkImageLayout old_layout, VkPipelineStageFlags src_stage, VkPipelineStageFlags dst_stage) { - const VkDevice device = ctx_->device(); - - VkCommandBufferAllocateInfo alloc{}; - alloc.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO; - alloc.commandPool = command_pool_; - alloc.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY; - alloc.commandBufferCount = 1; - VkCommandBuffer cmd = VK_NULL_HANDLE; - check_vk(vkAllocateCommandBuffers(device, &alloc, &cmd), "vkAllocateCommandBuffers(transition)"); - - // RAII: free the command buffer on every exit path (including - // exceptions from the check_vk calls below). The pool would - // eventually reclaim it on destroy(), but a retry loop after a - // transient queue submit failure would leak one cmd per attempt. - struct CmdGuard - { - VkDevice device; - VkCommandPool pool; - VkCommandBuffer cmd; - ~CmdGuard() - { - vkFreeCommandBuffers(device, pool, 1, &cmd); - } - } guard{ device, command_pool_, cmd }; - - VkCommandBufferBeginInfo begin{}; - begin.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO; - begin.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT; - check_vk(vkBeginCommandBuffer(cmd, &begin), "vkBeginCommandBuffer(transition)"); - - VkImageMemoryBarrier barrier{}; - barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; - barrier.oldLayout = old_layout; - barrier.newLayout = new_layout; - barrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; - barrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; - barrier.image = image_; - barrier.subresourceRange.aspectMask = - (format_ == PixelFormat::kD32F) ? VK_IMAGE_ASPECT_DEPTH_BIT : VK_IMAGE_ASPECT_COLOR_BIT; - barrier.subresourceRange.baseMipLevel = 0; - barrier.subresourceRange.levelCount = 1; - barrier.subresourceRange.baseArrayLayer = 0; - barrier.subresourceRange.layerCount = 1; - barrier.srcAccessMask = src_access; - barrier.dstAccessMask = dst_access; - vkCmdPipelineBarrier(cmd, src_stage, dst_stage, 0, 0, nullptr, 0, nullptr, 1, &barrier); - - check_vk(vkEndCommandBuffer(cmd), "vkEndCommandBuffer(transition)"); - - VkSubmitInfo submit{}; - submit.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO; - submit.commandBufferCount = 1; - submit.pCommandBuffers = &cmd; - check_vk(vkQueueSubmit(ctx_->queue(), 1, &submit, VK_NULL_HANDLE), "vkQueueSubmit(transition)"); - check_vk(vkQueueWaitIdle(ctx_->queue()), "vkQueueWaitIdle(transition)"); + const auto& device = ctx_->raii_device(); + + auto cmd_buffers = vk::raii::CommandBuffers{ + device, + vk::CommandBufferAllocateInfo{ + .commandPool = *command_pool_, + .level = vk::CommandBufferLevel::ePrimary, + .commandBufferCount = 1, + }, + }; + auto& cmd = cmd_buffers.front(); + + cmd.begin(vk::CommandBufferBeginInfo{ .flags = vk::CommandBufferUsageFlagBits::eOneTimeSubmit }); + + const vk::ImageMemoryBarrier barrier{ + .srcAccessMask = static_cast(src_access), + .dstAccessMask = static_cast(dst_access), + .oldLayout = static_cast(old_layout), + .newLayout = static_cast(new_layout), + .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .image = *image_, + .subresourceRange = + { + .aspectMask = (format_ == PixelFormat::kD32F) ? vk::ImageAspectFlagBits::eDepth + : vk::ImageAspectFlagBits::eColor, + .baseMipLevel = 0, + .levelCount = 1, + .baseArrayLayer = 0, + .layerCount = 1, + }, + }; + cmd.pipelineBarrier(static_cast(src_stage), static_cast(dst_stage), + {}, {}, {}, { barrier }); + cmd.end(); + + const VkCommandBuffer raw = *cmd; + ctx_->raii_queue().submit({ vk::SubmitInfo{ + .commandBufferCount = 1, + .pCommandBuffers = reinterpret_cast(&raw), + } }, + VK_NULL_HANDLE); + ctx_->raii_queue().waitIdle(); } } // namespace viz diff --git a/src/viz/core/cpp/inc/viz/core/device_image.hpp b/src/viz/core/cpp/inc/viz/core/device_image.hpp index a98c8876c..71d405b8e 100644 --- a/src/viz/core/cpp/inc/viz/core/device_image.hpp +++ b/src/viz/core/cpp/inc/viz/core/device_image.hpp @@ -5,7 +5,7 @@ #include // PixelFormat — used in API signatures #include -#include +#include #include #include @@ -61,11 +61,11 @@ class DeviceImage // init; transition_to_*() below moves it back and forth. VkImage vk_image() const noexcept { - return image_; + return *image_; } VkImageView vk_image_view() const noexcept { - return image_view_; + return *image_view_; } VkFormat vk_format() const noexcept { @@ -76,7 +76,7 @@ class DeviceImage // value returned by cuda_done_writing_value() before sampling. VkSemaphore cuda_done_writing() const noexcept { - return cuda_done_writing_; + return *cuda_done_writing_; } // Latest value CUDA has signaled successfully. Vulkan uses this @@ -132,10 +132,12 @@ class DeviceImage VkFormat vk_format_ = VK_FORMAT_R8G8B8A8_UNORM; VkImageLayout current_layout_ = VK_IMAGE_LAYOUT_UNDEFINED; - VkImage image_ = VK_NULL_HANDLE; - VkDeviceMemory memory_ = VK_NULL_HANDLE; - VkImageView image_view_ = VK_NULL_HANDLE; - VkCommandPool command_pool_ = VK_NULL_HANDLE; // For layout transitions only. + // Declared parent-first so reverse-destruction is correct. + vk::raii::DeviceMemory memory_{ nullptr }; + vk::raii::Image image_{ nullptr }; + vk::raii::ImageView image_view_{ nullptr }; + vk::raii::CommandPool command_pool_{ nullptr }; // for layout transitions only + vk::raii::Semaphore cuda_done_writing_{ nullptr }; // CUDA dup's the fd internally on import; we close ours after. int memory_fd_ = -1; @@ -144,12 +146,6 @@ class DeviceImage cudaMipmappedArray_t cuda_mipmapped_array_ = nullptr; cudaArray_t cuda_array_ = nullptr; // Level-0 view, non-owning. - // Producer→consumer timeline semaphore exported via - // VK_KHR_external_semaphore_fd and imported into CUDA. Two atomic - // counters (next reservation, last committed) so a failed - // cudaSignal can't leave the public value pointing at something - // that was never signaled. - VkSemaphore cuda_done_writing_ = VK_NULL_HANDLE; cudaExternalSemaphore_t cuda_cuda_done_writing_ = nullptr; std::atomic cuda_done_writing_next_{ 0 }; std::atomic cuda_done_writing_value_{ 0 }; From 63ac90a51f530abb855a280143e931a9e9e43000 Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 11:55:08 -0700 Subject: [PATCH 08/20] =?UTF-8?q?viz/core:=20review=20fixes=20=E2=80=94=20?= =?UTF-8?q?validation=20features=20chain,=20cmd=20buffer=20cast,=20NO=5FCO?= =?UTF-8?q?NSTRUCTORS=20placement?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues from review: 1. VkValidationFeaturesEXT was wrongly chained as pNext of VkDebugUtilsMessengerCreateInfoEXT. Per spec it's only valid as pNext of VkInstanceCreateInfo. Also missing the VK_EXT_validation_features instance extension itself; without it, the validation features struct is silently ignored. Same debug_create_info was reused for the persistent messenger, propagating the bogus pNext. Fix: enable VK_EXT_validation_features when validation is on. Use vk::StructureChain for instance creation. Pass a plain DebugUtilsMessengerCreateInfoEXT (no pNext) to the persistent messenger constructor. 2. DeviceImage::run_one_shot_layout_transition() was reinterpret_cast'ing a VkCommandBuffer pointer to vk::CommandBuffer*. vk::CommandBuffer wraps the raw handle 1:1 today but layout punning isn't guaranteed. Replace with a real vk::CommandBuffer local (constructed from *cmd) and take its address. 3. VULKAN_HPP_NO_CONSTRUCTORS was defined inside viz/core/vk.hpp, so it took effect only for TUs that included viz/core/vk.hpp BEFORE vulkan.hpp. If another header pulled vulkan.hpp first, the macro silently no-op'd and structs lost designated-init. Promote to a viz_core PUBLIC compile_definition so every Televiz TU gets it regardless of include order. All 52 unit tests pass. Co-Authored-By: Claude Sonnet 4.6 --- src/viz/core/cpp/CMakeLists.txt | 6 +++++ src/viz/core/cpp/device_image.cpp | 4 ++-- src/viz/core/cpp/inc/viz/core/vk.hpp | 13 ++++++----- src/viz/core/cpp/vk_context.cpp | 35 ++++++++++++++++++++-------- 4 files changed, 40 insertions(+), 18 deletions(-) diff --git a/src/viz/core/cpp/CMakeLists.txt b/src/viz/core/cpp/CMakeLists.txt index 6711684ce..81294eba6 100644 --- a/src/viz/core/cpp/CMakeLists.txt +++ b/src/viz/core/cpp/CMakeLists.txt @@ -51,4 +51,10 @@ target_link_libraries(viz_core ) # Aliased as viz::core (consumers say viz::core, not viz::viz_core). +# Defined PUBLIC so every consumer of viz_core (and transitively +# every Televiz Vulkan TU) sees vulkan-hpp's structs as aggregates, +# regardless of include order. Defining it only inside vk.hpp would +# silently break if another header included vulkan.hpp first. +target_compile_definitions(viz_core PUBLIC VULKAN_HPP_NO_CONSTRUCTORS) + add_library(viz::core ALIAS viz_core) diff --git a/src/viz/core/cpp/device_image.cpp b/src/viz/core/cpp/device_image.cpp index 2e1130053..6b9fc3e49 100644 --- a/src/viz/core/cpp/device_image.cpp +++ b/src/viz/core/cpp/device_image.cpp @@ -454,10 +454,10 @@ void DeviceImage::run_one_shot_layout_transition(VkImageLayout old_layout, {}, {}, {}, { barrier }); cmd.end(); - const VkCommandBuffer raw = *cmd; + const vk::CommandBuffer cmd_handle = *cmd; ctx_->raii_queue().submit({ vk::SubmitInfo{ .commandBufferCount = 1, - .pCommandBuffers = reinterpret_cast(&raw), + .pCommandBuffers = &cmd_handle, } }, VK_NULL_HANDLE); ctx_->raii_queue().waitIdle(); diff --git a/src/viz/core/cpp/inc/viz/core/vk.hpp b/src/viz/core/cpp/inc/viz/core/vk.hpp index 995a38c79..684b3e28b 100644 --- a/src/viz/core/cpp/inc/viz/core/vk.hpp +++ b/src/viz/core/cpp/inc/viz/core/vk.hpp @@ -18,12 +18,13 @@ // own their dispatcher automatically — no VULKAN_HPP_DEFAULT_DISPATCHER // initialization needed. // -// VULKAN_HPP_NO_CONSTRUCTORS removes vulkan-hpp's hand-written -// constructors so structs become aggregates, enabling C++20 -// designated initializers (`vk::ImageCreateInfo{.format = ..., ...}`). -// Builder methods like setFormat() still work; we just lose the -// positional parameter-list constructors (which we wouldn't use anyway). -#define VULKAN_HPP_NO_CONSTRUCTORS +// VULKAN_HPP_NO_CONSTRUCTORS is defined as a project-wide compile +// flag in viz_core's CMakeLists (PUBLIC propagation), not here — +// otherwise the macro would only take effect for TUs that happen to +// include this header before vulkan.hpp. The compile flag enforces +// it everywhere, removing vulkan-hpp's hand-written constructors so +// structs are aggregates and C++20 designated initializers work +// (`vk::ImageCreateInfo{.format = ..., ...}`). #include #include diff --git a/src/viz/core/cpp/vk_context.cpp b/src/viz/core/cpp/vk_context.cpp index dfd1eb20e..7771b1139 100644 --- a/src/viz/core/cpp/vk_context.cpp +++ b/src/viz/core/cpp/vk_context.cpp @@ -268,20 +268,18 @@ void VkContext::create_instance(const Config& config) if (validation_enabled_) { instance_extensions.push_back(VK_EXT_DEBUG_UTILS_EXTENSION_NAME); + instance_extensions.push_back(VK_EXT_VALIDATION_FEATURES_EXTENSION_NAME); } const vk::ValidationFeatureEnableEXT enables[] = { vk::ValidationFeatureEnableEXT::eBestPractices, vk::ValidationFeatureEnableEXT::eSynchronizationValidation, }; - const vk::ValidationFeaturesEXT validation_features{ - .enabledValidationFeatureCount = static_cast(std::size(enables)), - .pEnabledValidationFeatures = enables, - }; - // Catches errors emitted during instance creation itself. + // Plain create-info (no pNext) — same struct serves both the + // chained create-time messenger and the persistent post-create + // messenger, since neither needs further chained structures. const vk::DebugUtilsMessengerCreateInfoEXT debug_create_info{ - .pNext = validation_enabled_ ? &validation_features : nullptr, .messageSeverity = vk::DebugUtilsMessageSeverityFlagBitsEXT::eWarning | vk::DebugUtilsMessageSeverityFlagBitsEXT::eError, .messageType = vk::DebugUtilsMessageTypeFlagBitsEXT::eGeneral | vk::DebugUtilsMessageTypeFlagBitsEXT::eValidation | @@ -292,8 +290,7 @@ void VkContext::create_instance(const Config& config) .pfnUserCallback = reinterpret_cast(debug_messenger_callback), }; - const vk::InstanceCreateInfo create_info{ - .pNext = validation_enabled_ ? &debug_create_info : nullptr, + const vk::InstanceCreateInfo base_info{ .pApplicationInfo = &app_info, .enabledLayerCount = static_cast(layers.size()), .ppEnabledLayerNames = layers.data(), @@ -301,12 +298,30 @@ void VkContext::create_instance(const Config& config) .ppEnabledExtensionNames = instance_extensions.data(), }; - instance_ = vk::raii::Instance{ context_, create_info }; - if (validation_enabled_) { + // Instance pNext chain: + // InstanceCreateInfo + // → DebugUtilsMessengerCreateInfoEXT (catches errors emitted + // during vkCreateInstance) + // → ValidationFeaturesEXT (best-practices + sync validation; + // valid as pNext of InstanceCreateInfo, + // NOT of DebugUtilsMessengerCreateInfoEXT). + vk::StructureChain chain{ + base_info, + debug_create_info, + vk::ValidationFeaturesEXT{ + .enabledValidationFeatureCount = static_cast(std::size(enables)), + .pEnabledValidationFeatures = enables, + }, + }; + instance_ = vk::raii::Instance{ context_, chain.get() }; debug_messenger_ = vk::raii::DebugUtilsMessengerEXT{ instance_, debug_create_info }; } + else + { + instance_ = vk::raii::Instance{ context_, base_info }; + } } void VkContext::select_physical_device(const Config& config) From 1c6865c27d2878eeb0b2f7af63ea0c608630965e Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 11:59:04 -0700 Subject: [PATCH 09/20] =?UTF-8?q?viz/core:=20reorder=20validation=20pNext?= =?UTF-8?q?=20chain=20=E2=80=94=20ValidationFeaturesEXT=20first?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer flagged the previous chain ordering as misleading: physically, StructureChain linked ValidationFeaturesEXT as pNext of the messenger struct, even though both extensions extend InstanceCreateInfo and the loader walks the whole list regardless of order. Reorder to put ValidationFeaturesEXT directly after InstanceCreateInfo, and rewrite the comment to explain that pNext is a flat linked list — order is not semantic, but the physical linkage now matches the mental model of "both structs attach to the instance create info". Co-Authored-By: Claude Opus 4.7 --- src/viz/core/cpp/vk_context.cpp | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/viz/core/cpp/vk_context.cpp b/src/viz/core/cpp/vk_context.cpp index 7771b1139..5b4e43d67 100644 --- a/src/viz/core/cpp/vk_context.cpp +++ b/src/viz/core/cpp/vk_context.cpp @@ -300,20 +300,21 @@ void VkContext::create_instance(const Config& config) if (validation_enabled_) { - // Instance pNext chain: - // InstanceCreateInfo - // → DebugUtilsMessengerCreateInfoEXT (catches errors emitted - // during vkCreateInstance) - // → ValidationFeaturesEXT (best-practices + sync validation; - // valid as pNext of InstanceCreateInfo, - // NOT of DebugUtilsMessengerCreateInfoEXT). - vk::StructureChain chain{ + // Both ValidationFeaturesEXT and DebugUtilsMessengerCreateInfoEXT + // extend VkInstanceCreateInfo. The loader walks the entire pNext + // list and dispatches each struct by sType, so chain order is + // not semantically meaningful — but vulkan-hpp's StructureChain + // physically links them in declaration order, so we list them + // in the order they conceptually attach to the instance create + // info to keep the linkage easy to reason about. + vk::ValidationFeaturesEXT validation_features{ + .enabledValidationFeatureCount = static_cast(std::size(enables)), + .pEnabledValidationFeatures = enables, + }; + vk::StructureChain chain{ base_info, + validation_features, debug_create_info, - vk::ValidationFeaturesEXT{ - .enabledValidationFeatureCount = static_cast(std::size(enables)), - .pEnabledValidationFeatures = enables, - }, }; instance_ = vk::raii::Instance{ context_, chain.get() }; debug_messenger_ = vk::raii::DebugUtilsMessengerEXT{ instance_, debug_create_info }; From cf77d95c813184ca836234843a61b7961d8032ca Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 12:06:21 -0700 Subject: [PATCH 10/20] viz/session: migrate Swapchain internals to vk::raii Internal handles (VkSwapchainKHR, per-image semaphores, surface capabilities/formats/present-mode queries) move to vk::raii and vulkan-hpp types. Public API (raw VkImage, VkSemaphore in AcquiredImage; raw VkSurfaceKHR in create) stays unchanged so WindowBackend keeps working through this commit. destroy_semaphores / destroy_swapchain_only collapse into a single destroy() since vk::raii handles cleanup themselves; recreate() now moves the old swapchain into a local raii object whose destructor runs after init() so the oldSwapchain handle stays alive across the new vkCreateSwapchainKHR call. vkQueuePresentKHR is still called via the C entry point because raii::Queue::presentKHR throws on the OUT_OF_DATE / SUBOPTIMAL result codes we use for flow control. Co-Authored-By: Claude Opus 4.7 --- .../session/cpp/inc/viz/session/swapchain.hpp | 26 +- src/viz/session/cpp/swapchain.cpp | 273 +++++++----------- 2 files changed, 113 insertions(+), 186 deletions(-) diff --git a/src/viz/session/cpp/inc/viz/session/swapchain.hpp b/src/viz/session/cpp/inc/viz/session/swapchain.hpp index 88f1cdeed..8d335aeb3 100644 --- a/src/viz/session/cpp/inc/viz/session/swapchain.hpp +++ b/src/viz/session/cpp/inc/viz/session/swapchain.hpp @@ -3,8 +3,8 @@ #pragma once +#include #include -#include #include #include @@ -58,11 +58,11 @@ class Swapchain } VkFormat format() const noexcept { - return format_; + return static_cast(format_); } VkSwapchainKHR handle() const noexcept { - return swapchain_; + return *swapchain_; } uint32_t image_count() const noexcept { @@ -71,7 +71,7 @@ class Swapchain // Look up a swapchain image by acquired index; VK_NULL_HANDLE if out of range. VkImage image_at(uint32_t index) const noexcept { - return index < images_.size() ? images_[index] : VK_NULL_HANDLE; + return index < images_.size() ? static_cast(images_[index]) : VK_NULL_HANDLE; } private: @@ -79,22 +79,20 @@ class Swapchain // old_swapchain is passed as VkSwapchainCreateInfoKHR::oldSwapchain // so the driver recycles resources. VK_NULL_HANDLE on first create. void init(Resolution preferred_size, VkSwapchainKHR old_swapchain = VK_NULL_HANDLE); - void destroy_swapchain_only(); void create_semaphores(); - void destroy_semaphores(); const VkContext* ctx_ = nullptr; - VkSurfaceKHR surface_ = VK_NULL_HANDLE; - VkSwapchainKHR swapchain_ = VK_NULL_HANDLE; - VkFormat format_ = VK_FORMAT_UNDEFINED; - VkColorSpaceKHR color_space_ = VK_COLOR_SPACE_SRGB_NONLINEAR_KHR; - VkExtent2D extent_{}; - std::vector images_; // not owned (swapchain owns) + VkSurfaceKHR surface_ = VK_NULL_HANDLE; // not owned (GlfwWindow / XR backend owns) + vk::raii::SwapchainKHR swapchain_{ nullptr }; + vk::Format format_ = vk::Format::eUndefined; + vk::ColorSpaceKHR color_space_ = vk::ColorSpaceKHR::eSrgbNonlinear; + vk::Extent2D extent_{}; + std::vector images_; // not owned (swapchain owns) // Per-image-slot semaphore ring so an in-flight image never tries // to reuse a semaphore another in-flight image still consumes. - std::vector image_available_; - std::vector render_done_; + std::vector image_available_; + std::vector render_done_; uint32_t frame_slot_ = 0; }; diff --git a/src/viz/session/cpp/swapchain.cpp b/src/viz/session/cpp/swapchain.cpp index 60583a808..6421d33f0 100644 --- a/src/viz/session/cpp/swapchain.cpp +++ b/src/viz/session/cpp/swapchain.cpp @@ -14,37 +14,30 @@ namespace viz namespace { -void check_vk(VkResult r, const char* what) -{ - if (r != VK_SUCCESS) - { - throw std::runtime_error(std::string("Swapchain: ") + what + " failed: VkResult=" + std::to_string(r)); - } -} - // Pick a surface format. Prefer B8G8R8A8_SRGB (common Linux default, // matches our intermediate framebuffer's sRGB color space). Fall back // to any *_SRGB format. Else accept whatever the runtime offers first. -VkSurfaceFormatKHR pick_surface_format(const std::vector& formats) +vk::SurfaceFormatKHR pick_surface_format(const std::vector& formats) { for (const auto& f : formats) { - if (f.format == VK_FORMAT_B8G8R8A8_SRGB && f.colorSpace == VK_COLOR_SPACE_SRGB_NONLINEAR_KHR) + if (f.format == vk::Format::eB8G8R8A8Srgb && f.colorSpace == vk::ColorSpaceKHR::eSrgbNonlinear) { return f; } } for (const auto& f : formats) { - if (f.format == VK_FORMAT_R8G8B8A8_SRGB && f.colorSpace == VK_COLOR_SPACE_SRGB_NONLINEAR_KHR) + if (f.format == vk::Format::eR8G8B8A8Srgb && f.colorSpace == vk::ColorSpaceKHR::eSrgbNonlinear) { return f; } } - return formats.empty() ? VkSurfaceFormatKHR{ VK_FORMAT_UNDEFINED, VK_COLOR_SPACE_SRGB_NONLINEAR_KHR } : formats[0]; + return formats.empty() ? vk::SurfaceFormatKHR{ vk::Format::eUndefined, vk::ColorSpaceKHR::eSrgbNonlinear } + : formats[0]; } -VkExtent2D clamp_extent(const VkSurfaceCapabilitiesKHR& caps, Resolution preferred) +vk::Extent2D clamp_extent(const vk::SurfaceCapabilitiesKHR& caps, Resolution preferred) { // Surface may dictate the extent (currentExtent != UINT32_MAX); // otherwise we pick within minImageExtent..maxImageExtent. @@ -52,7 +45,7 @@ VkExtent2D clamp_extent(const VkSurfaceCapabilitiesKHR& caps, Resolution preferr { return caps.currentExtent; } - VkExtent2D e{ preferred.width, preferred.height }; + vk::Extent2D e{ preferred.width, preferred.height }; e.width = std::clamp(e.width, caps.minImageExtent.width, caps.maxImageExtent.width); e.height = std::clamp(e.height, caps.minImageExtent.height, caps.maxImageExtent.height); return e; @@ -86,10 +79,8 @@ std::unique_ptr Swapchain::create(const VkContext& ctx, VkSurfaceKHR // Proper fix is a presentation-support callback through // VkContext::Config (e.g., glfwGetPhysicalDevicePresentationSupport) // — deferred until a real multi-GPU user reports this. - VkBool32 present_supported = VK_FALSE; - check_vk(vkGetPhysicalDeviceSurfaceSupportKHR( - ctx.physical_device(), ctx.queue_family_index(), surface, &present_supported), - "vkGetPhysicalDeviceSurfaceSupportKHR"); + const bool present_supported = + ctx.raii_physical_device().getSurfaceSupportKHR(ctx.queue_family_index(), vk::SurfaceKHR{ surface }); if (!present_supported) { throw std::runtime_error("Swapchain::create: chosen queue family does not support present on this surface"); @@ -113,22 +104,14 @@ void Swapchain::init(Resolution preferred_size, VkSwapchainKHR old_swapchain) { try { - const VkPhysicalDevice phys = ctx_->physical_device(); - const VkDevice device = ctx_->device(); + const auto& phys = ctx_->raii_physical_device(); + const vk::SurfaceKHR surface{ surface_ }; - VkSurfaceCapabilitiesKHR caps{}; - check_vk(vkGetPhysicalDeviceSurfaceCapabilitiesKHR(phys, surface_, &caps), - "vkGetPhysicalDeviceSurfaceCapabilitiesKHR"); + const vk::SurfaceCapabilitiesKHR caps = phys.getSurfaceCapabilitiesKHR(surface); + const std::vector formats = phys.getSurfaceFormatsKHR(surface); - uint32_t format_count = 0; - vkGetPhysicalDeviceSurfaceFormatsKHR(phys, surface_, &format_count, nullptr); - std::vector formats(format_count); - if (format_count > 0) - { - vkGetPhysicalDeviceSurfaceFormatsKHR(phys, surface_, &format_count, formats.data()); - } - const VkSurfaceFormatKHR chosen = pick_surface_format(formats); - if (chosen.format == VK_FORMAT_UNDEFINED) + const vk::SurfaceFormatKHR chosen = pick_surface_format(formats); + if (chosen.format == vk::Format::eUndefined) { throw std::runtime_error("Swapchain::init: surface reports no formats"); } @@ -143,223 +126,169 @@ void Swapchain::init(Resolution preferred_size, VkSwapchainKHR old_swapchain) image_count = std::min(image_count, caps.maxImageCount); } - VkSwapchainCreateInfoKHR info{}; - info.sType = VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR; - info.surface = surface_; - info.minImageCount = image_count; - info.imageFormat = format_; - info.imageColorSpace = color_space_; - info.imageExtent = extent_; - info.imageArrayLayers = 1; - // TRANSFER_DST: we blit the intermediate framebuffer into the - // swapchain image. No COLOR_ATTACHMENT — we never render - // directly into swapchain images. - info.imageUsage = VK_IMAGE_USAGE_TRANSFER_DST_BIT; - info.imageSharingMode = VK_SHARING_MODE_EXCLUSIVE; - info.preTransform = caps.currentTransform; - info.compositeAlpha = VK_COMPOSITE_ALPHA_OPAQUE_BIT_KHR; - // Prefer MAILBOX (no compositor sync stalls); FIFO is the // universal fallback. App throttles its own render rate. - VkPresentModeKHR present_mode = VK_PRESENT_MODE_FIFO_KHR; - uint32_t pm_count = 0; - vkGetPhysicalDeviceSurfacePresentModesKHR(phys, surface_, &pm_count, nullptr); - std::vector available_modes(pm_count); - if (pm_count > 0) + vk::PresentModeKHR present_mode = vk::PresentModeKHR::eFifo; + for (auto m : phys.getSurfacePresentModesKHR(surface)) { - vkGetPhysicalDeviceSurfacePresentModesKHR(phys, surface_, &pm_count, available_modes.data()); - } - for (VkPresentModeKHR m : available_modes) - { - if (m == VK_PRESENT_MODE_MAILBOX_KHR) + if (m == vk::PresentModeKHR::eMailbox) { present_mode = m; break; } } - info.presentMode = present_mode; - info.clipped = VK_TRUE; - info.oldSwapchain = old_swapchain; - check_vk(vkCreateSwapchainKHR(device, &info, nullptr, &swapchain_), "vkCreateSwapchainKHR"); - - uint32_t actual = 0; - vkGetSwapchainImagesKHR(device, swapchain_, &actual, nullptr); - images_.resize(actual); - vkGetSwapchainImagesKHR(device, swapchain_, &actual, images_.data()); + const vk::SwapchainCreateInfoKHR info{ + .surface = surface, + .minImageCount = image_count, + .imageFormat = format_, + .imageColorSpace = color_space_, + .imageExtent = extent_, + .imageArrayLayers = 1, + // TRANSFER_DST: we blit the intermediate framebuffer into + // the swapchain image. No COLOR_ATTACHMENT — we never + // render directly into swapchain images. + .imageUsage = vk::ImageUsageFlagBits::eTransferDst, + .imageSharingMode = vk::SharingMode::eExclusive, + .preTransform = caps.currentTransform, + .compositeAlpha = vk::CompositeAlphaFlagBitsKHR::eOpaque, + .presentMode = present_mode, + .clipped = VK_TRUE, + .oldSwapchain = vk::SwapchainKHR{ old_swapchain }, + }; + + swapchain_ = vk::raii::SwapchainKHR{ ctx_->raii_device(), info }; + images_ = swapchain_.getImages(); create_semaphores(); } catch (...) { - destroy_swapchain_only(); + // Drain and reset partially-built state so retry is sane. + if (*ctx_->raii_device() != VK_NULL_HANDLE) + { + (void)ctx_->raii_device().waitIdle(); + } + image_available_.clear(); + render_done_.clear(); + swapchain_ = nullptr; + images_.clear(); + extent_ = vk::Extent2D{ 0, 0 }; + frame_slot_ = 0; throw; } } void Swapchain::create_semaphores() { - const VkDevice device = ctx_->device(); - image_available_.resize(images_.size(), VK_NULL_HANDLE); - render_done_.resize(images_.size(), VK_NULL_HANDLE); - VkSemaphoreCreateInfo sem_info{}; - sem_info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO; + image_available_.reserve(images_.size()); + render_done_.reserve(images_.size()); + const vk::SemaphoreCreateInfo sem_info{}; for (size_t i = 0; i < images_.size(); ++i) { - check_vk( - vkCreateSemaphore(device, &sem_info, nullptr, &image_available_[i]), "vkCreateSemaphore(image_available)"); - check_vk(vkCreateSemaphore(device, &sem_info, nullptr, &render_done_[i]), "vkCreateSemaphore(render_done)"); + image_available_.emplace_back(ctx_->raii_device(), sem_info); + render_done_.emplace_back(ctx_->raii_device(), sem_info); } } -void Swapchain::destroy_semaphores() +void Swapchain::destroy() { - if (ctx_ == nullptr) - { - return; - } - const VkDevice device = ctx_->device(); - if (device == VK_NULL_HANDLE) - { - image_available_.clear(); - render_done_.clear(); - return; - } - for (VkSemaphore s : image_available_) + if (ctx_ != nullptr && *ctx_->raii_device() != VK_NULL_HANDLE) { - if (s != VK_NULL_HANDLE) - { - vkDestroySemaphore(device, s, nullptr); - } + // Drain so we don't destroy semaphores still referenced by the queue. + (void)ctx_->raii_device().waitIdle(); } image_available_.clear(); - for (VkSemaphore s : render_done_) - { - if (s != VK_NULL_HANDLE) - { - vkDestroySemaphore(device, s, nullptr); - } - } render_done_.clear(); -} - -void Swapchain::destroy_swapchain_only() -{ - if (ctx_ == nullptr) - { - return; - } - const VkDevice device = ctx_->device(); - if (device != VK_NULL_HANDLE) - { - // Drain so we don't destroy semaphores still referenced by the queue. - (void)vkDeviceWaitIdle(device); - } - destroy_semaphores(); - if (swapchain_ != VK_NULL_HANDLE && device != VK_NULL_HANDLE) - { - vkDestroySwapchainKHR(device, swapchain_, nullptr); - swapchain_ = VK_NULL_HANDLE; - } + swapchain_ = nullptr; images_.clear(); - extent_ = VkExtent2D{ 0, 0 }; + extent_ = vk::Extent2D{ 0, 0 }; frame_slot_ = 0; -} - -void Swapchain::destroy() -{ - destroy_swapchain_only(); surface_ = VK_NULL_HANDLE; ctx_ = nullptr; } void Swapchain::recreate(Resolution preferred_size) { - if (swapchain_ == VK_NULL_HANDLE) + if (*swapchain_ == VK_NULL_HANDLE) { init(preferred_size); return; } - const VkDevice device = ctx_->device(); - (void)vkDeviceWaitIdle(device); + (void)ctx_->raii_device().waitIdle(); - // Hand the old swapchain to vkCreateSwapchainKHR via oldSwapchain - // so the driver can recycle resources. Keep the old handle alive - // until init() succeeds; destroy it after. - VkSwapchainKHR old = swapchain_; - swapchain_ = VK_NULL_HANDLE; - destroy_semaphores(); + // Release the old swapchain only after the new one is created + // (init passes the old handle as oldSwapchain so the driver can + // recycle resources). On success, the local `old` raii object + // destroys the original handle as it goes out of scope. + vk::raii::SwapchainKHR old = std::move(swapchain_); + swapchain_ = vk::raii::SwapchainKHR{ nullptr }; + image_available_.clear(); + render_done_.clear(); images_.clear(); - extent_ = VkExtent2D{ 0, 0 }; + extent_ = vk::Extent2D{ 0, 0 }; frame_slot_ = 0; - try - { - init(preferred_size, old); - } - catch (...) - { - if (old != VK_NULL_HANDLE) - { - vkDestroySwapchainKHR(device, old, nullptr); - } - throw; - } - - // Success: the new swapchain has assumed ownership of any - // recyclable resources. Destroy the old handle now. - vkDestroySwapchainKHR(device, old, nullptr); + init(preferred_size, *old); } std::optional Swapchain::acquire_next_image() { - if (swapchain_ == VK_NULL_HANDLE || image_available_.empty()) + if (*swapchain_ == VK_NULL_HANDLE || image_available_.empty()) { return std::nullopt; } - const VkSemaphore sem = image_available_[frame_slot_]; - uint32_t image_index = 0; - const VkResult r = vkAcquireNextImageKHR(ctx_->device(), swapchain_, UINT64_MAX, sem, VK_NULL_HANDLE, &image_index); + const auto& sem = image_available_[frame_slot_]; + const auto result = swapchain_.acquireNextImage(UINT64_MAX, *sem, VK_NULL_HANDLE); + const vk::Result r = result.first; + const uint32_t image_index = result.second; // OUT_OF_DATE: caller must recreate. SUBOPTIMAL: image is valid, // pass it through and let the WSI scale on present. - if (r == VK_ERROR_OUT_OF_DATE_KHR) + if (r == vk::Result::eErrorOutOfDateKHR) { return std::nullopt; } - if (r != VK_SUCCESS && r != VK_SUBOPTIMAL_KHR) + if (r != vk::Result::eSuccess && r != vk::Result::eSuboptimalKHR) { - throw std::runtime_error("Swapchain::acquire_next_image: VkResult=" + std::to_string(r)); + throw std::runtime_error("Swapchain::acquire_next_image: VkResult=" + + std::to_string(static_cast(r))); } - return AcquiredImage{ image_index, images_[image_index], sem, render_done_[frame_slot_] }; + return AcquiredImage{ image_index, static_cast(images_[image_index]), *sem, *render_done_[frame_slot_] }; } bool Swapchain::present(uint32_t image_index, VkSemaphore render_done) { - if (swapchain_ == VK_NULL_HANDLE) + if (*swapchain_ == VK_NULL_HANDLE) { return false; } - VkPresentInfoKHR info{}; - info.sType = VK_STRUCTURE_TYPE_PRESENT_INFO_KHR; - info.waitSemaphoreCount = (render_done != VK_NULL_HANDLE) ? 1 : 0; - info.pWaitSemaphores = (render_done != VK_NULL_HANDLE) ? &render_done : nullptr; - info.swapchainCount = 1; - info.pSwapchains = &swapchain_; - info.pImageIndices = &image_index; - const VkResult r = vkQueuePresentKHR(ctx_->queue(), &info); + const vk::Semaphore wait_sem{ render_done }; + const vk::SwapchainKHR sc = *swapchain_; + const vk::PresentInfoKHR info{ + .waitSemaphoreCount = (render_done != VK_NULL_HANDLE) ? 1u : 0u, + .pWaitSemaphores = (render_done != VK_NULL_HANDLE) ? &wait_sem : nullptr, + .swapchainCount = 1, + .pSwapchains = &sc, + .pImageIndices = &image_index, + }; + // raii::Queue::presentKHR throws on the OUT_OF_DATE / SUBOPTIMAL + // result codes that we want to handle as flow control. Fall through + // to the C entry point so the result code is observable. + const vk::Result r = static_cast( + vkQueuePresentKHR(ctx_->queue(), reinterpret_cast(&info))); // Advance the slot regardless — next frame needs fresh semaphores. if (!images_.empty()) { frame_slot_ = (frame_slot_ + 1) % static_cast(images_.size()); } - if (r == VK_ERROR_OUT_OF_DATE_KHR) + if (r == vk::Result::eErrorOutOfDateKHR) { return false; } - if (r != VK_SUCCESS && r != VK_SUBOPTIMAL_KHR) + if (r != vk::Result::eSuccess && r != vk::Result::eSuboptimalKHR) { - throw std::runtime_error("Swapchain::present: VkResult=" + std::to_string(r)); + throw std::runtime_error("Swapchain::present: VkResult=" + std::to_string(static_cast(r))); } return true; } From e141180efca5e89a936f8aa74d033ff6bb794ae5 Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 12:08:19 -0700 Subject: [PATCH 11/20] viz/session: migrate GlfwWindow surface to vk::raii::SurfaceKHR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GlfwWindow::create now takes const vk::raii::Instance& instead of a raw VkInstance, and adopts the raw VkSurfaceKHR returned by glfwCreateWindowSurface into vk::raii::SurfaceKHR via the raw-handle adopt constructor. destroy() drops the surface (via raii reset) before glfwDestroyWindow, preserving destruction ordering. WindowBackend passes ctx.raii_instance(); the test that asserted invalid_argument on a null VkInstance is dropped — the type system now prevents it at the call site. Co-Authored-By: Claude Opus 4.7 --- src/viz/session/cpp/glfw_window.cpp | 27 ++++++++++--------- .../cpp/inc/viz/session/glfw_window.hpp | 12 ++++----- src/viz/session/cpp/window_backend.cpp | 2 +- .../cpp/test_window_primitives.cpp | 25 ++++++++--------- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/viz/session/cpp/glfw_window.cpp b/src/viz/session/cpp/glfw_window.cpp index 7916a5674..b38f8df82 100644 --- a/src/viz/session/cpp/glfw_window.cpp +++ b/src/viz/session/cpp/glfw_window.cpp @@ -62,9 +62,12 @@ void GlfwWindow::release() noexcept } } -std::unique_ptr GlfwWindow::create(VkInstance instance, uint32_t width, uint32_t height, const std::string& title) +std::unique_ptr GlfwWindow::create(const vk::raii::Instance& instance, + uint32_t width, + uint32_t height, + const std::string& title) { - if (instance == VK_NULL_HANDLE) + if (*instance == VK_NULL_HANDLE) { throw std::invalid_argument("GlfwWindow::create: instance is VK_NULL_HANDLE"); } @@ -88,23 +91,25 @@ std::unique_ptr GlfwWindow::create(VkInstance instance, uint32_t wid (desc ? desc : "(no description)")); } - VkSurfaceKHR surface = VK_NULL_HANDLE; - const VkResult r = glfwCreateWindowSurface(instance, w, nullptr, &surface); + // glfwCreateWindowSurface is a C API returning a raw handle; adopt + // it into vk::raii::SurfaceKHR so destruction is automatic. + VkSurfaceKHR raw_surface = VK_NULL_HANDLE; + const VkResult r = glfwCreateWindowSurface(*instance, w, nullptr, &raw_surface); if (r != VK_SUCCESS) { glfwDestroyWindow(w); GlfwWindow::release(); throw std::runtime_error("GlfwWindow: glfwCreateWindowSurface failed: VkResult=" + std::to_string(r)); } + vk::raii::SurfaceKHR surface{ instance, raw_surface }; - std::unique_ptr self(new GlfwWindow(instance, w, surface)); + std::unique_ptr self(new GlfwWindow(w, std::move(surface))); glfwSetWindowUserPointer(w, self.get()); glfwSetFramebufferSizeCallback(w, &GlfwWindow::framebuffer_resize_callback); return self; } -GlfwWindow::GlfwWindow(VkInstance instance, GLFWwindow* window, VkSurfaceKHR surface) - : instance_(instance), window_(window), surface_(surface) +GlfwWindow::GlfwWindow(GLFWwindow* window, vk::raii::SurfaceKHR surface) : window_(window), surface_(std::move(surface)) { } @@ -115,11 +120,9 @@ GlfwWindow::~GlfwWindow() void GlfwWindow::destroy() { - if (surface_ != VK_NULL_HANDLE && instance_ != VK_NULL_HANDLE) - { - vkDestroySurfaceKHR(instance_, surface_, nullptr); - surface_ = VK_NULL_HANDLE; - } + // Surface must be released before the window goes away (the + // surface holds a reference to the window's native handles). + surface_ = nullptr; if (window_ != nullptr) { glfwDestroyWindow(window_); diff --git a/src/viz/session/cpp/inc/viz/session/glfw_window.hpp b/src/viz/session/cpp/inc/viz/session/glfw_window.hpp index c4438712b..8440be541 100644 --- a/src/viz/session/cpp/inc/viz/session/glfw_window.hpp +++ b/src/viz/session/cpp/inc/viz/session/glfw_window.hpp @@ -4,7 +4,7 @@ #pragma once #include -#include +#include #include #include @@ -26,7 +26,7 @@ class GlfwWindow // Creates the window + surface. Throws std::runtime_error if // GLFW init fails (no display, missing libs) — call sites should // catch and SKIP if running headless. - static std::unique_ptr create(VkInstance instance, + static std::unique_ptr create(const vk::raii::Instance& instance, uint32_t width, uint32_t height, const std::string& title); @@ -50,9 +50,10 @@ class GlfwWindow { return window_; } + // Raw boundary: Swapchain::create takes VkSurfaceKHR. VkSurfaceKHR surface() const noexcept { - return surface_; + return *surface_; } bool should_close() const noexcept; void poll_events() noexcept; @@ -67,12 +68,11 @@ class GlfwWindow } private: - GlfwWindow(VkInstance instance, GLFWwindow* window, VkSurfaceKHR surface); + GlfwWindow(GLFWwindow* window, vk::raii::SurfaceKHR surface); static void framebuffer_resize_callback(GLFWwindow* w, int width, int height); - VkInstance instance_ = VK_NULL_HANDLE; GLFWwindow* window_ = nullptr; - VkSurfaceKHR surface_ = VK_NULL_HANDLE; + vk::raii::SurfaceKHR surface_{ nullptr }; std::atomic resized_{ false }; }; diff --git a/src/viz/session/cpp/window_backend.cpp b/src/viz/session/cpp/window_backend.cpp index 476c21eb0..8543b60a8 100644 --- a/src/viz/session/cpp/window_backend.cpp +++ b/src/viz/session/cpp/window_backend.cpp @@ -93,7 +93,7 @@ void WindowBackend::init(const VkContext& ctx, Resolution preferred_size) ctx_ = &ctx; try { - window_ = GlfwWindow::create(ctx.instance(), preferred_size.width, preferred_size.height, config_.title); + window_ = GlfwWindow::create(ctx.raii_instance(), preferred_size.width, preferred_size.height, config_.title); swapchain_ = Swapchain::create(ctx, window_->surface(), preferred_size); // Match intermediate extent to swapchain for a 1:1 post-render blit. render_target_ = RenderTarget::create(ctx, RenderTarget::Config{ swapchain_->extent() }); diff --git a/src/viz/session_tests/cpp/test_window_primitives.cpp b/src/viz/session_tests/cpp/test_window_primitives.cpp index aefed95bc..0359ad648 100644 --- a/src/viz/session_tests/cpp/test_window_primitives.cpp +++ b/src/viz/session_tests/cpp/test_window_primitives.cpp @@ -85,7 +85,7 @@ TEST_CASE("GlfwWindow construct + destroy with a real Vulkan instance", "[gpu][w VkContext ctx; ctx.init(cfg); - auto win = GlfwWindow::create(ctx.instance(), 320, 240, "viz-test"); + auto win = GlfwWindow::create(ctx.raii_instance(), 320, 240, "viz-test"); REQUIRE(win != nullptr); CHECK(win->glfw() != nullptr); CHECK(win->surface() != VK_NULL_HANDLE); @@ -101,23 +101,20 @@ TEST_CASE("GlfwWindow construct + destroy with a real Vulkan instance", "[gpu][w win->destroy(); // idempotent } -TEST_CASE("GlfwWindow rejects null instance and zero dims", "[gpu][window]") +TEST_CASE("GlfwWindow rejects zero dims", "[gpu][window]") { - if (!window_environment_available()) - { - SKIP("No display"); - } - CHECK_THROWS_AS(GlfwWindow::create(VK_NULL_HANDLE, 320, 240, "x"), std::invalid_argument); - // Need a valid instance to exercise the dim check. - if (!is_gpu_available()) + // Null-instance check is enforced by the type system now + // (create takes const vk::raii::Instance&) — only the runtime + // dim check is reachable from C++. + if (!is_gpu_available() || !window_environment_available()) { - SKIP("No GPU"); + SKIP("No GPU or no display"); } VkContext::Config cfg{}; cfg.instance_extensions = glfw_required_instance_extensions(); VkContext ctx; ctx.init(cfg); - CHECK_THROWS_AS(GlfwWindow::create(ctx.instance(), 0, 240, "x"), std::invalid_argument); + CHECK_THROWS_AS(GlfwWindow::create(ctx.raii_instance(), 0, 240, "x"), std::invalid_argument); } TEST_CASE("Swapchain creates with non-zero image count and matching extent", "[gpu][window]") @@ -133,7 +130,7 @@ TEST_CASE("Swapchain creates with non-zero image count and matching extent", "[g VkContext ctx; ctx.init(cfg); - auto win = GlfwWindow::create(ctx.instance(), 320, 240, "viz-test-sc"); + auto win = GlfwWindow::create(ctx.raii_instance(), 320, 240, "viz-test-sc"); auto sc = Swapchain::create(ctx, win->surface(), Resolution{ 320, 240 }); REQUIRE(sc != nullptr); CHECK(sc->image_count() >= 2); @@ -155,7 +152,7 @@ TEST_CASE("Swapchain recreate preserves usable state", "[gpu][window]") VkContext ctx; ctx.init(cfg); - auto win = GlfwWindow::create(ctx.instance(), 320, 240, "viz-test-sc-recreate"); + auto win = GlfwWindow::create(ctx.raii_instance(), 320, 240, "viz-test-sc-recreate"); auto sc = Swapchain::create(ctx, win->surface(), Resolution{ 320, 240 }); const uint32_t before = sc->image_count(); @@ -178,7 +175,7 @@ TEST_CASE("Swapchain destroy is idempotent", "[gpu][window]") VkContext ctx; ctx.init(cfg); - auto win = GlfwWindow::create(ctx.instance(), 320, 240, "viz-test-sc-idem"); + auto win = GlfwWindow::create(ctx.raii_instance(), 320, 240, "viz-test-sc-idem"); auto sc = Swapchain::create(ctx, win->surface(), Resolution{ 320, 240 }); sc->destroy(); sc->destroy(); From f90cb1512d5108e6225cbd87f2188764b03e514f Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 12:09:29 -0700 Subject: [PATCH 12/20] viz/session: migrate OffscreenBackend internals to vk::raii MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VkBuffer/VkDeviceMemory and VkCommandPool/VkCommandBuffer become vk::raii equivalents (CommandBuffers — the owning vector — for the single readback cmd buffer). destroy_readback_staging is folded into destroy() since raii handles cleanup; mapMemory/unmapMemory now go through vk::raii::DeviceMemory. readback_byte_size_ promoted to vk::DeviceSize (nameless typedef of VkDeviceSize), allocationSize / size now use designated initializers. Co-Authored-By: Claude Opus 4.7 --- .../cpp/inc/viz/session/offscreen_backend.hpp | 13 +- src/viz/session/cpp/offscreen_backend.cpp | 157 +++++++----------- 2 files changed, 68 insertions(+), 102 deletions(-) diff --git a/src/viz/session/cpp/inc/viz/session/offscreen_backend.hpp b/src/viz/session/cpp/inc/viz/session/offscreen_backend.hpp index e64882202..a1331e138 100644 --- a/src/viz/session/cpp/inc/viz/session/offscreen_backend.hpp +++ b/src/viz/session/cpp/inc/viz/session/offscreen_backend.hpp @@ -3,6 +3,7 @@ #pragma once +#include #include #include @@ -32,20 +33,20 @@ class OffscreenBackend final : public DisplayBackend private: void create_readback_staging(); - void destroy_readback_staging(); const VkContext* ctx_ = nullptr; Resolution extent_{}; std::unique_ptr render_target_; - // Pre-allocated; reused per readback. - VkBuffer readback_buffer_ = VK_NULL_HANDLE; - VkDeviceMemory readback_memory_ = VK_NULL_HANDLE; + // Pre-allocated; reused per readback. Declared parent-first so + // reverse-destruction is correct (memory after buffer/pool). + vk::raii::DeviceMemory readback_memory_{ nullptr }; + vk::raii::Buffer readback_buffer_{ nullptr }; VkDeviceSize readback_byte_size_ = 0; // Dedicated cmd buffer so readback never races the compositor's. - VkCommandPool readback_command_pool_ = VK_NULL_HANDLE; - VkCommandBuffer readback_command_buffer_ = VK_NULL_HANDLE; + vk::raii::CommandPool readback_command_pool_{ nullptr }; + vk::raii::CommandBuffers readback_command_buffers_{ nullptr }; }; } // namespace viz diff --git a/src/viz/session/cpp/offscreen_backend.cpp b/src/viz/session/cpp/offscreen_backend.cpp index 9b2a86ac1..156013084 100644 --- a/src/viz/session/cpp/offscreen_backend.cpp +++ b/src/viz/session/cpp/offscreen_backend.cpp @@ -14,18 +14,11 @@ namespace viz namespace { -void check_vk(VkResult r, const char* what) +uint32_t find_memory_type(const vk::raii::PhysicalDevice& physical_device, + uint32_t type_bits, + vk::MemoryPropertyFlags properties) { - if (r != VK_SUCCESS) - { - throw std::runtime_error(std::string("OffscreenBackend: ") + what + " failed: VkResult=" + std::to_string(r)); - } -} - -uint32_t find_memory_type(VkPhysicalDevice physical_device, uint32_t type_bits, VkMemoryPropertyFlags properties) -{ - VkPhysicalDeviceMemoryProperties mem_props; - vkGetPhysicalDeviceMemoryProperties(physical_device, &mem_props); + const auto mem_props = physical_device.getMemoryProperties(); for (uint32_t i = 0; i < mem_props.memoryTypeCount; ++i) { if ((type_bits & (1u << i)) != 0 && (mem_props.memoryTypes[i].propertyFlags & properties) == properties) @@ -67,7 +60,11 @@ void OffscreenBackend::init(const VkContext& ctx, Resolution preferred_size) void OffscreenBackend::destroy() { - destroy_readback_staging(); + readback_command_buffers_ = nullptr; + readback_command_pool_ = nullptr; + readback_buffer_ = nullptr; + readback_memory_ = nullptr; + readback_byte_size_ = 0; render_target_.reset(); extent_ = Resolution{}; ctx_ = nullptr; @@ -103,109 +100,77 @@ Resolution OffscreenBackend::current_extent() const HostImage OffscreenBackend::readback_to_host() { - if (render_target_ == nullptr || readback_buffer_ == VK_NULL_HANDLE) + if (render_target_ == nullptr || *readback_buffer_ == VK_NULL_HANDLE) { throw std::runtime_error("OffscreenBackend::readback_to_host: backend not initialized"); } - // RT is in TRANSFER_SRC_OPTIMAL from the render pass's final layout. - check_vk(vkResetCommandBuffer(readback_command_buffer_, 0), "vkResetCommandBuffer(readback)"); + auto& cmd = readback_command_buffers_[0]; - VkCommandBufferBeginInfo begin{}; - begin.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO; - begin.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT; - check_vk(vkBeginCommandBuffer(readback_command_buffer_, &begin), "vkBeginCommandBuffer(readback)"); + // RT is in TRANSFER_SRC_OPTIMAL from the render pass's final layout. + cmd.reset(); + cmd.begin(vk::CommandBufferBeginInfo{ .flags = vk::CommandBufferUsageFlagBits::eOneTimeSubmit }); - VkBufferImageCopy region{}; - region.imageSubresource.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; - region.imageSubresource.layerCount = 1; - region.imageExtent = { extent_.width, extent_.height, 1 }; - vkCmdCopyImageToBuffer(readback_command_buffer_, render_target_->color_image(), - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, readback_buffer_, 1, ®ion); + const vk::BufferImageCopy region{ + .imageSubresource = { .aspectMask = vk::ImageAspectFlagBits::eColor, .layerCount = 1 }, + .imageExtent = { extent_.width, extent_.height, 1 }, + }; + cmd.copyImageToBuffer(vk::Image{ render_target_->color_image() }, vk::ImageLayout::eTransferSrcOptimal, + *readback_buffer_, region); - check_vk(vkEndCommandBuffer(readback_command_buffer_), "vkEndCommandBuffer(readback)"); + cmd.end(); - VkSubmitInfo submit{}; - submit.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO; - submit.commandBufferCount = 1; - submit.pCommandBuffers = &readback_command_buffer_; - check_vk(vkQueueSubmit(ctx_->queue(), 1, &submit, VK_NULL_HANDLE), "vkQueueSubmit(readback)"); - check_vk(vkQueueWaitIdle(ctx_->queue()), "vkQueueWaitIdle(readback)"); + const vk::CommandBuffer cmd_handle = *cmd; + ctx_->raii_queue().submit(vk::SubmitInfo{ .commandBufferCount = 1, .pCommandBuffers = &cmd_handle }, + VK_NULL_HANDLE); + ctx_->raii_queue().waitIdle(); HostImage result(extent_, PixelFormat::kRGBA8); - void* mapped = nullptr; - check_vk(vkMapMemory(ctx_->device(), readback_memory_, 0, readback_byte_size_, 0, &mapped), "vkMapMemory(readback)"); + void* mapped = readback_memory_.mapMemory(0, readback_byte_size_); std::memcpy(result.data(), mapped, readback_byte_size_); - vkUnmapMemory(ctx_->device(), readback_memory_); + readback_memory_.unmapMemory(); return result; } void OffscreenBackend::create_readback_staging() { readback_byte_size_ = - static_cast(extent_.width) * extent_.height * bytes_per_pixel(PixelFormat::kRGBA8); - - VkBufferCreateInfo bi{}; - bi.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO; - bi.size = readback_byte_size_; - bi.usage = VK_BUFFER_USAGE_TRANSFER_DST_BIT; - bi.sharingMode = VK_SHARING_MODE_EXCLUSIVE; - check_vk(vkCreateBuffer(ctx_->device(), &bi, nullptr, &readback_buffer_), "vkCreateBuffer(readback)"); - - VkMemoryRequirements reqs; - vkGetBufferMemoryRequirements(ctx_->device(), readback_buffer_, &reqs); - - VkMemoryAllocateInfo ai{}; - ai.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO; - ai.allocationSize = reqs.size; - ai.memoryTypeIndex = find_memory_type(ctx_->physical_device(), reqs.memoryTypeBits, - VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT); - check_vk(vkAllocateMemory(ctx_->device(), &ai, nullptr, &readback_memory_), "vkAllocateMemory(readback)"); - check_vk(vkBindBufferMemory(ctx_->device(), readback_buffer_, readback_memory_, 0), "vkBindBufferMemory(readback)"); + static_cast(extent_.width) * extent_.height * bytes_per_pixel(PixelFormat::kRGBA8); + + const auto& device = ctx_->raii_device(); + readback_buffer_ = vk::raii::Buffer{ device, + vk::BufferCreateInfo{ + .size = readback_byte_size_, + .usage = vk::BufferUsageFlagBits::eTransferDst, + .sharingMode = vk::SharingMode::eExclusive, + } }; + + const auto reqs = readback_buffer_.getMemoryRequirements(); + readback_memory_ = vk::raii::DeviceMemory{ + device, + vk::MemoryAllocateInfo{ + .allocationSize = reqs.size, + .memoryTypeIndex = + find_memory_type(ctx_->raii_physical_device(), reqs.memoryTypeBits, + vk::MemoryPropertyFlagBits::eHostVisible | vk::MemoryPropertyFlagBits::eHostCoherent), + }, + }; + readback_buffer_.bindMemory(*readback_memory_, 0); // Dedicated cmd pool — never races the compositor's per-frame buffer. - VkCommandPoolCreateInfo pi{}; - pi.sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO; - pi.flags = VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT; - pi.queueFamilyIndex = ctx_->queue_family_index(); - check_vk(vkCreateCommandPool(ctx_->device(), &pi, nullptr, &readback_command_pool_), "vkCreateCommandPool(readback)"); - VkCommandBufferAllocateInfo ai2{}; - ai2.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO; - ai2.commandPool = readback_command_pool_; - ai2.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY; - ai2.commandBufferCount = 1; - check_vk(vkAllocateCommandBuffers(ctx_->device(), &ai2, &readback_command_buffer_), - "vkAllocateCommandBuffers(readback)"); -} - -void OffscreenBackend::destroy_readback_staging() -{ - if (ctx_ == nullptr) - { - return; - } - const VkDevice device = ctx_->device(); - if (device == VK_NULL_HANDLE) - { - return; - } - if (readback_command_pool_ != VK_NULL_HANDLE) - { - vkDestroyCommandPool(device, readback_command_pool_, nullptr); - readback_command_pool_ = VK_NULL_HANDLE; - readback_command_buffer_ = VK_NULL_HANDLE; - } - if (readback_buffer_ != VK_NULL_HANDLE) - { - vkDestroyBuffer(device, readback_buffer_, nullptr); - readback_buffer_ = VK_NULL_HANDLE; - } - if (readback_memory_ != VK_NULL_HANDLE) - { - vkFreeMemory(device, readback_memory_, nullptr); - readback_memory_ = VK_NULL_HANDLE; - } - readback_byte_size_ = 0; + readback_command_pool_ = vk::raii::CommandPool{ + device, vk::CommandPoolCreateInfo{ + .flags = vk::CommandPoolCreateFlagBits::eResetCommandBuffer, + .queueFamilyIndex = ctx_->queue_family_index(), + } + }; + readback_command_buffers_ = vk::raii::CommandBuffers{ + device, vk::CommandBufferAllocateInfo{ + .commandPool = *readback_command_pool_, + .level = vk::CommandBufferLevel::ePrimary, + .commandBufferCount = 1, + } + }; } } // namespace viz From ba0bb1fe36d0cdcd3763ff5b278af7590fce6211 Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 12:13:07 -0700 Subject: [PATCH 13/20] viz/session: migrate VizCompositor internals to vk::raii MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Command pool / buffer move to vk::raii (CommandBuffers vector for the single buffer slot). Recording path converts to vulkan-hpp methods (cmd.begin / beginRenderPass / setScissor / endRenderPass / end) and a StructureChain for the timeline submit, with raii_queue() taking over from vkQueueSubmit's raw entry point only where flow control needs the result code (kept raw via reinterpret_cast for the submit_or_signal fallback path). Layer record() and backend record_post_render_pass remain raw VkCommandBuffer recipients — that's the recording boundary the layer base class declares. Public Config keeps VkClearColorValue (raw boundary); the union is layout-compatible with vk::ClearColorValue, so it's reinterpreted when constructing the vk::ClearValue array. Co-Authored-By: Claude Opus 4.7 --- .../cpp/inc/viz/session/viz_compositor.hpp | 11 +- src/viz/session/cpp/viz_compositor.cpp | 190 ++++++++---------- 2 files changed, 90 insertions(+), 111 deletions(-) diff --git a/src/viz/session/cpp/inc/viz/session/viz_compositor.hpp b/src/viz/session/cpp/inc/viz/session/viz_compositor.hpp index 857ba7935..23c134a0f 100644 --- a/src/viz/session/cpp/inc/viz/session/viz_compositor.hpp +++ b/src/viz/session/cpp/inc/viz/session/viz_compositor.hpp @@ -6,7 +6,7 @@ #include #include #include -#include +#include #include #include @@ -54,21 +54,20 @@ class VizCompositor VizCompositor(const VkContext& ctx, DisplayBackend& backend, const Config& config); void init(); - void create_command_pool(); - void create_command_buffer(); + void create_command_pool_and_buffer(); // vkQueueSubmit wrapper. On failure, posts an empty submit so the // fence still gets signaled — converts "silent deadlock on next // wait" into "throw on next call". - void submit_or_signal_fence(const VkSubmitInfo& info, const char* what); + void submit_or_signal_fence(const vk::SubmitInfo& info, const char* what); const VkContext* ctx_ = nullptr; DisplayBackend* backend_ = nullptr; Config config_{}; std::unique_ptr frame_sync_; - VkCommandPool command_pool_ = VK_NULL_HANDLE; - VkCommandBuffer command_buffer_ = VK_NULL_HANDLE; + vk::raii::CommandPool command_pool_{ nullptr }; + vk::raii::CommandBuffers command_buffers_{ nullptr }; }; } // namespace viz diff --git a/src/viz/session/cpp/viz_compositor.cpp b/src/viz/session/cpp/viz_compositor.cpp index 7b13e4264..9f65720af 100644 --- a/src/viz/session/cpp/viz_compositor.cpp +++ b/src/viz/session/cpp/viz_compositor.cpp @@ -17,14 +17,6 @@ namespace viz namespace { -void check_vk(VkResult result, const char* what) -{ - if (result != VK_SUCCESS) - { - throw std::runtime_error(std::string("VizCompositor: ") + what + " failed: VkResult=" + std::to_string(result)); - } -} - Rect2D to_rect2d(const VkRect2D& r) { return Rect2D{ r.offset.x, r.offset.y, r.extent.width, r.extent.height }; @@ -58,8 +50,7 @@ void VizCompositor::init() try { frame_sync_ = FrameSync::create(*ctx_); - create_command_pool(); - create_command_buffer(); + create_command_pool_and_buffer(); } catch (...) { @@ -70,55 +61,43 @@ void VizCompositor::init() void VizCompositor::destroy() { - if (ctx_ == nullptr) - { - return; - } - const VkDevice device = ctx_->device(); - if (device == VK_NULL_HANDLE) - { - return; - } - if (command_pool_ != VK_NULL_HANDLE) - { - // Pool destruction frees all command buffers allocated from it. - vkDestroyCommandPool(device, command_pool_, nullptr); - command_pool_ = VK_NULL_HANDLE; - command_buffer_ = VK_NULL_HANDLE; - } + command_buffers_ = nullptr; + command_pool_ = nullptr; frame_sync_.reset(); } -void VizCompositor::create_command_pool() +void VizCompositor::create_command_pool_and_buffer() { - VkCommandPoolCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO; - info.flags = VK_COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT; - info.queueFamilyIndex = ctx_->queue_family_index(); - check_vk(vkCreateCommandPool(ctx_->device(), &info, nullptr, &command_pool_), "vkCreateCommandPool"); + command_pool_ = vk::raii::CommandPool{ + ctx_->raii_device(), vk::CommandPoolCreateInfo{ + .flags = vk::CommandPoolCreateFlagBits::eResetCommandBuffer, + .queueFamilyIndex = ctx_->queue_family_index(), + } + }; + command_buffers_ = vk::raii::CommandBuffers{ + ctx_->raii_device(), vk::CommandBufferAllocateInfo{ + .commandPool = *command_pool_, + .level = vk::CommandBufferLevel::ePrimary, + .commandBufferCount = 1, + } + }; } -void VizCompositor::create_command_buffer() +void VizCompositor::submit_or_signal_fence(const vk::SubmitInfo& info, const char* what) { - VkCommandBufferAllocateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO; - info.commandPool = command_pool_; - info.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY; - info.commandBufferCount = 1; - check_vk(vkAllocateCommandBuffers(ctx_->device(), &info, &command_buffer_), "vkAllocateCommandBuffers"); -} - -void VizCompositor::submit_or_signal_fence(const VkSubmitInfo& info, const char* what) -{ - const VkResult r = vkQueueSubmit(ctx_->queue(), 1, &info, frame_sync_->in_flight_fence()); - if (r == VK_SUCCESS) + const vk::Result r = + static_cast(vkQueueSubmit(ctx_->queue(), 1, reinterpret_cast(&info), + frame_sync_->in_flight_fence())); + if (r == vk::Result::eSuccess) { return; } - VkSubmitInfo empty{}; - empty.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO; - (void)vkQueueSubmit(ctx_->queue(), 1, &empty, frame_sync_->in_flight_fence()); - throw std::runtime_error(std::string("VizCompositor: ") + what + " failed: VkResult=" + std::to_string(r)); + // Fall back: signal the fence with an empty submit so the next + // wait() doesn't deadlock, then surface the original failure. + const vk::SubmitInfo empty{}; + (void)vkQueueSubmit(ctx_->queue(), 1, reinterpret_cast(&empty), frame_sync_->in_flight_fence()); + throw std::runtime_error(std::string("VizCompositor: ") + what + + " failed: VkResult=" + std::to_string(static_cast(r))); } void VizCompositor::render(const std::vector& layers) @@ -126,6 +105,8 @@ void VizCompositor::render(const std::vector& layers) // Wait for previous frame (1 frame in flight). frame_sync_->wait(); + auto& cmd = command_buffers_[0]; + // RAII: leave the command buffer in INITIAL state on every exit // path (success or throw). VizSession::pump_events() runs between // render() calls and may destroy framebuffer attachments, which @@ -134,15 +115,15 @@ void VizCompositor::render(const std::vector& layers) // below guarantees we're never PENDING when this destructor runs. struct CmdResetGuard { - VkCommandBuffer cmd; + vk::raii::CommandBuffer* cmd; ~CmdResetGuard() { - if (cmd != VK_NULL_HANDLE) + if (cmd != nullptr && **cmd != VK_NULL_HANDLE) { - (void)vkResetCommandBuffer(cmd, 0); + cmd->reset(); } } - } cmd_guard{ command_buffer_ }; + } cmd_guard{ &cmd }; // Snapshot visible layers ONCE — is_visible() is atomic; reading // it twice could record a draw without the matching wait (or vice @@ -208,99 +189,98 @@ void VizCompositor::render(const std::vector& layers) tiles = tile_layout(aspects, rt_extent, /*padding=*/0); } - VkCommandBufferBeginInfo begin{}; - begin.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO; - begin.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT; - check_vk(vkBeginCommandBuffer(command_buffer_, &begin), "vkBeginCommandBuffer"); - - std::array clears{}; - clears[0].color = config_.clear_color; - clears[1].depthStencil = { 1.0f, 0 }; + cmd.begin(vk::CommandBufferBeginInfo{ .flags = vk::CommandBufferUsageFlagBits::eOneTimeSubmit }); - VkRenderPassBeginInfo rp{}; - rp.sType = VK_STRUCTURE_TYPE_RENDER_PASS_BEGIN_INFO; - rp.renderPass = rt.render_pass(); - rp.framebuffer = rt.framebuffer(); - rp.renderArea.offset = { 0, 0 }; - rp.renderArea.extent = { rt_extent.width, rt_extent.height }; - rp.clearValueCount = static_cast(clears.size()); - rp.pClearValues = clears.data(); + std::array clears{}; + // VkClearColorValue and vk::ClearColorValue are layout-compatible + // unions; reinterpret instead of selecting a discriminator. + clears[0].color = *reinterpret_cast(&config_.clear_color); + clears[1].depthStencil = vk::ClearDepthStencilValue{ 1.0f, 0 }; - vkCmdBeginRenderPass(command_buffer_, &rp, VK_SUBPASS_CONTENTS_INLINE); + cmd.beginRenderPass(vk::RenderPassBeginInfo{ + .renderPass = rt.render_pass(), + .framebuffer = rt.framebuffer(), + .renderArea = vk::Rect2D{ vk::Offset2D{ 0, 0 }, vk::Extent2D{ rt_extent.width, rt_extent.height } }, + .clearValueCount = static_cast(clears.size()), + .pClearValues = clears.data(), + }, + vk::SubpassContents::eInline); // Per-layer: pre-bind scissor (tile.outer); per-layer ViewInfo - // gets viewport = tile.content. + // gets viewport = tile.content. Layer record() takes raw + // VkCommandBuffer — it's a recording boundary. + const vk::CommandBuffer cmd_hpp = *cmd; + const VkCommandBuffer raw_cmd = cmd_hpp; for (size_t i = 0; i < visible_layers.size(); ++i) { - const VkRect2D scissor_rect = tiles[i].outer; - const VkRect2D viewport_rect = tiles[i].content; - vkCmdSetScissor(command_buffer_, 0, 1, &scissor_rect); + // VkRect2D and vk::Rect2D are layout-compatible (vk-hpp guarantees + // ABI parity) — reinterpret rather than rebuilding the offset/extent. + cmd.setScissor(0, *reinterpret_cast(&tiles[i].outer)); std::vector layer_views = frame->views; if (layer_views.empty()) { layer_views.push_back(ViewInfo{}); } - layer_views[0].viewport = to_rect2d(viewport_rect); - visible_layers[i]->record(command_buffer_, layer_views, rt); + layer_views[0].viewport = to_rect2d(tiles[i].content); + visible_layers[i]->record(raw_cmd, layer_views, rt); } - vkCmdEndRenderPass(command_buffer_); + cmd.endRenderPass(); // Backend-specific post-render commands (blit + transitions etc.). - backend_->record_post_render_pass(command_buffer_, *frame); + backend_->record_post_render_pass(raw_cmd, *frame); - check_vk(vkEndCommandBuffer(command_buffer_), "vkEndCommandBuffer"); + cmd.end(); // Layer waits (timeline) + backend's wait_before_render (binary, // value 0 ignored). - std::vector wait_semaphores; + std::vector wait_semaphores; std::vector wait_values; - std::vector wait_stages; + std::vector wait_stages; for (LayerBase* layer : visible_layers) { for (const auto& w : layer->get_wait_semaphores()) { if (w.semaphore != VK_NULL_HANDLE) { - wait_semaphores.push_back(w.semaphore); + wait_semaphores.emplace_back(w.semaphore); wait_values.push_back(w.value); - wait_stages.push_back(w.wait_stage); + wait_stages.emplace_back(static_cast(w.wait_stage)); } } } if (frame->wait_before_render != VK_NULL_HANDLE) { - wait_semaphores.push_back(frame->wait_before_render); + wait_semaphores.emplace_back(frame->wait_before_render); wait_values.push_back(0); - wait_stages.push_back(frame->wait_stage); + wait_stages.emplace_back(static_cast(frame->wait_stage)); } - std::vector signal_semaphores; + std::vector signal_semaphores; std::vector signal_values; if (frame->signal_after_render != VK_NULL_HANDLE) { - signal_semaphores.push_back(frame->signal_after_render); + signal_semaphores.emplace_back(frame->signal_after_render); signal_values.push_back(0); } - VkTimelineSemaphoreSubmitInfo timeline{}; - timeline.sType = VK_STRUCTURE_TYPE_TIMELINE_SEMAPHORE_SUBMIT_INFO; - timeline.waitSemaphoreValueCount = static_cast(wait_values.size()); - timeline.pWaitSemaphoreValues = wait_values.empty() ? nullptr : wait_values.data(); - timeline.signalSemaphoreValueCount = static_cast(signal_values.size()); - timeline.pSignalSemaphoreValues = signal_values.empty() ? nullptr : signal_values.data(); - - VkSubmitInfo submit{}; - submit.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO; - submit.pNext = &timeline; - submit.commandBufferCount = 1; - submit.pCommandBuffers = &command_buffer_; - submit.waitSemaphoreCount = static_cast(wait_semaphores.size()); - submit.pWaitSemaphores = wait_semaphores.empty() ? nullptr : wait_semaphores.data(); - submit.pWaitDstStageMask = wait_stages.empty() ? nullptr : wait_stages.data(); - submit.signalSemaphoreCount = static_cast(signal_semaphores.size()); - submit.pSignalSemaphores = signal_semaphores.empty() ? nullptr : signal_semaphores.data(); + const vk::SubmitInfo submit_info{ + .waitSemaphoreCount = static_cast(wait_semaphores.size()), + .pWaitSemaphores = wait_semaphores.empty() ? nullptr : wait_semaphores.data(), + .pWaitDstStageMask = wait_stages.empty() ? nullptr : wait_stages.data(), + .commandBufferCount = 1, + .pCommandBuffers = &cmd_hpp, + .signalSemaphoreCount = static_cast(signal_semaphores.size()), + .pSignalSemaphores = signal_semaphores.empty() ? nullptr : signal_semaphores.data(), + }; + const vk::TimelineSemaphoreSubmitInfo timeline_info{ + .waitSemaphoreValueCount = static_cast(wait_values.size()), + .pWaitSemaphoreValues = wait_values.empty() ? nullptr : wait_values.data(), + .signalSemaphoreValueCount = static_cast(signal_values.size()), + .pSignalSemaphoreValues = signal_values.empty() ? nullptr : signal_values.data(), + }; + vk::StructureChain submit_chain{ submit_info, timeline_info }; // Reset the fence immediately before submit. Anything that // throws above this point leaves the fence signaled from the @@ -308,7 +288,7 @@ void VizCompositor::render(const std::vector& layers) // submit_or_signal_fence handles vkQueueSubmit failure by // submitting an empty signal so the fence still transitions. frame_sync_->reset(); - submit_or_signal_fence(submit, "vkQueueSubmit"); + submit_or_signal_fence(submit_chain.get(), "vkQueueSubmit"); // Drain before end_frame: if end_frame throws, the cmd buffer is // EXECUTABLE (resettable by CmdResetGuard) instead of PENDING. From b902d0c9b59c89665b655d7f178b962d7e75d27a Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 12:15:43 -0700 Subject: [PATCH 14/20] viz/layers: migrate QuadLayer internals to vk::raii Sampler / DescriptorSetLayout / PipelineLayout / Pipeline / DescriptorPool / DescriptorSets all become vk::raii. ShaderModule locals in create_pipeline use vk::raii::ShaderModule so the hand-written ShaderGuard goes away (raii handles that). destroy() collapses to nullptr-resets in reverse-creation order; manual sequencing was already required (sets-before-pool, pipeline- before-layout) and the declared field order makes raii preserve it. Recording path uses vk::CommandBuffer wrappers for bindPipeline / bindDescriptorSets / draw; the cmd parameter is still raw VkCommandBuffer since that's the layer base class's contract. updateDescriptorSets goes through raii_device(). Co-Authored-By: Claude Opus 4.7 --- .../layers/cpp/inc/viz/layers/quad_layer.hpp | 19 +- src/viz/layers/cpp/quad_layer.cpp | 381 ++++++++---------- 2 files changed, 172 insertions(+), 228 deletions(-) diff --git a/src/viz/layers/cpp/inc/viz/layers/quad_layer.hpp b/src/viz/layers/cpp/inc/viz/layers/quad_layer.hpp index cf8fc120a..1c2162890 100644 --- a/src/viz/layers/cpp/inc/viz/layers/quad_layer.hpp +++ b/src/viz/layers/cpp/inc/viz/layers/quad_layer.hpp @@ -6,8 +6,8 @@ #include #include #include +#include #include -#include #include #include @@ -132,15 +132,18 @@ class QuadLayer : public LayerBase // One DeviceImage per mailbox slot. std::array, kSlotCount> slots_; - VkSampler sampler_ = VK_NULL_HANDLE; - VkDescriptorSetLayout descriptor_set_layout_ = VK_NULL_HANDLE; - VkPipelineLayout pipeline_layout_ = VK_NULL_HANDLE; - VkPipeline pipeline_ = VK_NULL_HANDLE; - - VkDescriptorPool descriptor_pool_ = VK_NULL_HANDLE; + // Declared parent-first so reverse-destruction is correct + // (descriptor_pool_ must outlive descriptor_sets_; pipeline must + // outlive pipeline_layout_; pipeline_layout_ must outlive + // descriptor_set_layout_). + vk::raii::Sampler sampler_{ nullptr }; + vk::raii::DescriptorSetLayout descriptor_set_layout_{ nullptr }; + vk::raii::PipelineLayout pipeline_layout_{ nullptr }; + vk::raii::Pipeline pipeline_{ nullptr }; + vk::raii::DescriptorPool descriptor_pool_{ nullptr }; // One descriptor set per slot, each binding the corresponding // DeviceImage's sRGB view. record() picks the one for in_use_. - std::array descriptor_sets_{}; + vk::raii::DescriptorSets descriptor_sets_{ nullptr }; // Mailbox state. Both atomic so producer and renderer can // touch them without locks. diff --git a/src/viz/layers/cpp/quad_layer.cpp b/src/viz/layers/cpp/quad_layer.cpp index 6c5fb00dd..0ef551be8 100644 --- a/src/viz/layers/cpp/quad_layer.cpp +++ b/src/viz/layers/cpp/quad_layer.cpp @@ -17,14 +17,6 @@ namespace viz namespace { -void check_vk(VkResult result, const char* what) -{ - if (result != VK_SUCCESS) - { - throw std::runtime_error(std::string("QuadLayer: ") + what + " failed: VkResult=" + std::to_string(result)); - } -} - void check_cuda(cudaError_t result, const char* what) { if (result != cudaSuccess) @@ -33,15 +25,14 @@ void check_cuda(cudaError_t result, const char* what) } } -VkShaderModule create_shader_module(VkDevice device, const unsigned char* spv, size_t size) +vk::raii::ShaderModule create_shader_module(const vk::raii::Device& device, const unsigned char* spv, size_t size) { - VkShaderModuleCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO; - info.codeSize = size; - info.pCode = reinterpret_cast(spv); - VkShaderModule mod = VK_NULL_HANDLE; - check_vk(vkCreateShaderModule(device, &info, nullptr, &mod), "vkCreateShaderModule"); - return mod; + return vk::raii::ShaderModule{ + device, vk::ShaderModuleCreateInfo{ + .codeSize = size, + .pCode = reinterpret_cast(spv), + } + }; } // Once destroy() has run, slots_[0] is the canonical "alive" signal @@ -115,46 +106,16 @@ void QuadLayer::init() void QuadLayer::destroy() { - if (ctx_ == nullptr) - { - return; - } - const VkDevice device = ctx_->device(); - if (device == VK_NULL_HANDLE) - { - for (auto& slot : slots_) - { - slot.reset(); - } - return; - } - if (descriptor_pool_ != VK_NULL_HANDLE) - { - // descriptor_sets_ are freed implicitly with the pool. - vkDestroyDescriptorPool(device, descriptor_pool_, nullptr); - descriptor_pool_ = VK_NULL_HANDLE; - descriptor_sets_.fill(VK_NULL_HANDLE); - } - if (pipeline_ != VK_NULL_HANDLE) - { - vkDestroyPipeline(device, pipeline_, nullptr); - pipeline_ = VK_NULL_HANDLE; - } - if (pipeline_layout_ != VK_NULL_HANDLE) - { - vkDestroyPipelineLayout(device, pipeline_layout_, nullptr); - pipeline_layout_ = VK_NULL_HANDLE; - } - if (descriptor_set_layout_ != VK_NULL_HANDLE) - { - vkDestroyDescriptorSetLayout(device, descriptor_set_layout_, nullptr); - descriptor_set_layout_ = VK_NULL_HANDLE; - } - if (sampler_ != VK_NULL_HANDLE) - { - vkDestroySampler(device, sampler_, nullptr); - sampler_ = VK_NULL_HANDLE; - } + // Reverse of init(): descriptor sets back to the pool, pipeline + // before its layout, sampler last. raii handles the actual + // destruction order via reset-to-nullptr in declared order + // (parent-first declaration → reverse runs child-first). + descriptor_sets_ = nullptr; + descriptor_pool_ = nullptr; + pipeline_ = nullptr; + pipeline_layout_ = nullptr; + descriptor_set_layout_ = nullptr; + sampler_ = nullptr; for (auto& slot : slots_) { slot.reset(); @@ -271,9 +232,9 @@ void QuadLayer::record(VkCommandBuffer cmd, const std::vector& views, return; } - vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline_); - vkCmdBindDescriptorSets( - cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline_layout_, 0, 1, &descriptor_sets_[cur], 0, nullptr); + const vk::CommandBuffer cmd_hpp{ cmd }; + cmd_hpp.bindPipeline(vk::PipelineBindPoint::eGraphics, *pipeline_); + cmd_hpp.bindDescriptorSets(vk::PipelineBindPoint::eGraphics, *pipeline_layout_, 0, *descriptor_sets_[cur], {}); // 1 view in window/offscreen, 2 in XR stereo. Compositor pre-bound // the layer's scissor; we bind viewport per view and draw. @@ -282,7 +243,7 @@ void QuadLayer::record(VkCommandBuffer cmd, const std::vector& views, bind_view_viewport(cmd, view); // 3 vertices, no vertex buffer — vertex shader emits a // fullscreen triangle from gl_VertexIndex. - vkCmdDraw(cmd, 3, 1, 0, 0); + cmd_hpp.draw(3, 1, 0, 0); } } @@ -313,204 +274,184 @@ std::vector QuadLayer::get_wait_semaphores() const void QuadLayer::create_sampler() { - VkSamplerCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_SAMPLER_CREATE_INFO; - info.magFilter = VK_FILTER_LINEAR; - info.minFilter = VK_FILTER_LINEAR; - info.mipmapMode = VK_SAMPLER_MIPMAP_MODE_NEAREST; - info.addressModeU = VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_EDGE; - info.addressModeV = VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_EDGE; - info.addressModeW = VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_EDGE; - info.anisotropyEnable = VK_FALSE; // enable later when XR distance views need it - info.maxAnisotropy = 1.0f; - info.borderColor = VK_BORDER_COLOR_INT_OPAQUE_BLACK; - info.unnormalizedCoordinates = VK_FALSE; - info.compareEnable = VK_FALSE; - info.compareOp = VK_COMPARE_OP_ALWAYS; - info.minLod = 0.0f; - info.maxLod = 0.0f; - check_vk(vkCreateSampler(ctx_->device(), &info, nullptr, &sampler_), "vkCreateSampler"); + sampler_ = vk::raii::Sampler{ + ctx_->raii_device(), + vk::SamplerCreateInfo{ + .magFilter = vk::Filter::eLinear, + .minFilter = vk::Filter::eLinear, + .mipmapMode = vk::SamplerMipmapMode::eNearest, + .addressModeU = vk::SamplerAddressMode::eClampToEdge, + .addressModeV = vk::SamplerAddressMode::eClampToEdge, + .addressModeW = vk::SamplerAddressMode::eClampToEdge, + .anisotropyEnable = VK_FALSE, // enable later when XR distance views need it + .maxAnisotropy = 1.0f, + .compareEnable = VK_FALSE, + .compareOp = vk::CompareOp::eAlways, + .minLod = 0.0f, + .maxLod = 0.0f, + .borderColor = vk::BorderColor::eIntOpaqueBlack, + .unnormalizedCoordinates = VK_FALSE, + } + }; } void QuadLayer::create_descriptor_set_layout() { - VkDescriptorSetLayoutBinding binding{}; - binding.binding = 0; - binding.descriptorType = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; - binding.descriptorCount = 1; - binding.stageFlags = VK_SHADER_STAGE_FRAGMENT_BIT; - binding.pImmutableSamplers = nullptr; - - VkDescriptorSetLayoutCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO; - info.bindingCount = 1; - info.pBindings = &binding; - check_vk(vkCreateDescriptorSetLayout(ctx_->device(), &info, nullptr, &descriptor_set_layout_), - "vkCreateDescriptorSetLayout"); + const vk::DescriptorSetLayoutBinding binding{ + .binding = 0, + .descriptorType = vk::DescriptorType::eCombinedImageSampler, + .descriptorCount = 1, + .stageFlags = vk::ShaderStageFlagBits::eFragment, + .pImmutableSamplers = nullptr, + }; + descriptor_set_layout_ = vk::raii::DescriptorSetLayout{ + ctx_->raii_device(), + vk::DescriptorSetLayoutCreateInfo{ .bindingCount = 1, .pBindings = &binding }, + }; } void QuadLayer::create_pipeline_layout() { - VkPipelineLayoutCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_PIPELINE_LAYOUT_CREATE_INFO; - info.setLayoutCount = 1; - info.pSetLayouts = &descriptor_set_layout_; - info.pushConstantRangeCount = 0; - check_vk(vkCreatePipelineLayout(ctx_->device(), &info, nullptr, &pipeline_layout_), "vkCreatePipelineLayout"); + const vk::DescriptorSetLayout layout = *descriptor_set_layout_; + pipeline_layout_ = vk::raii::PipelineLayout{ + ctx_->raii_device(), + vk::PipelineLayoutCreateInfo{ + .setLayoutCount = 1, + .pSetLayouts = &layout, + .pushConstantRangeCount = 0, + }, + }; } void QuadLayer::create_pipeline() { - const VkDevice device = ctx_->device(); + const auto& device = ctx_->raii_device(); - VkShaderModule vert = + const auto vert = create_shader_module(device, viz::shaders::kTexturedQuadVertSpv, viz::shaders::kTexturedQuadVertSpvSize); - VkShaderModule frag = + const auto frag = create_shader_module(device, viz::shaders::kTexturedQuadFragSpv, viz::shaders::kTexturedQuadFragSpvSize); - // RAII: shader modules are only needed during pipeline creation. - struct ShaderGuard - { - VkDevice device; - VkShaderModule vert; - VkShaderModule frag; - ~ShaderGuard() - { - if (vert != VK_NULL_HANDLE) - { - vkDestroyShaderModule(device, vert, nullptr); - } - if (frag != VK_NULL_HANDLE) - { - vkDestroyShaderModule(device, frag, nullptr); - } - } - } guard{ device, vert, frag }; - - VkPipelineShaderStageCreateInfo stages[2]{}; - stages[0].sType = VK_STRUCTURE_TYPE_PIPELINE_SHADER_STAGE_CREATE_INFO; - stages[0].stage = VK_SHADER_STAGE_VERTEX_BIT; - stages[0].module = vert; - stages[0].pName = "main"; - stages[1].sType = VK_STRUCTURE_TYPE_PIPELINE_SHADER_STAGE_CREATE_INFO; - stages[1].stage = VK_SHADER_STAGE_FRAGMENT_BIT; - stages[1].module = frag; - stages[1].pName = "main"; - - VkPipelineVertexInputStateCreateInfo vertex_input{}; - vertex_input.sType = VK_STRUCTURE_TYPE_PIPELINE_VERTEX_INPUT_STATE_CREATE_INFO; + const std::array stages{ + vk::PipelineShaderStageCreateInfo{ .stage = vk::ShaderStageFlagBits::eVertex, .module = *vert, .pName = "main" }, + vk::PipelineShaderStageCreateInfo{ + .stage = vk::ShaderStageFlagBits::eFragment, .module = *frag, .pName = "main" }, + }; - VkPipelineInputAssemblyStateCreateInfo input_assembly{}; - input_assembly.sType = VK_STRUCTURE_TYPE_PIPELINE_INPUT_ASSEMBLY_STATE_CREATE_INFO; - input_assembly.topology = VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST; + const vk::PipelineVertexInputStateCreateInfo vertex_input{}; + const vk::PipelineInputAssemblyStateCreateInfo input_assembly{ .topology = vk::PrimitiveTopology::eTriangleList }; // Viewport / scissor are dynamic so one pipeline works across // resolutions. - VkPipelineViewportStateCreateInfo viewport_state{}; - viewport_state.sType = VK_STRUCTURE_TYPE_PIPELINE_VIEWPORT_STATE_CREATE_INFO; - viewport_state.viewportCount = 1; - viewport_state.scissorCount = 1; - - VkPipelineRasterizationStateCreateInfo rasterizer{}; - rasterizer.sType = VK_STRUCTURE_TYPE_PIPELINE_RASTERIZATION_STATE_CREATE_INFO; - rasterizer.polygonMode = VK_POLYGON_MODE_FILL; - rasterizer.cullMode = VK_CULL_MODE_NONE; - rasterizer.frontFace = VK_FRONT_FACE_COUNTER_CLOCKWISE; - rasterizer.lineWidth = 1.0f; - - VkPipelineMultisampleStateCreateInfo multisample{}; - multisample.sType = VK_STRUCTURE_TYPE_PIPELINE_MULTISAMPLE_STATE_CREATE_INFO; - multisample.rasterizationSamples = VK_SAMPLE_COUNT_1_BIT; + const vk::PipelineViewportStateCreateInfo viewport_state{ .viewportCount = 1, .scissorCount = 1 }; + + const vk::PipelineRasterizationStateCreateInfo rasterizer{ + .polygonMode = vk::PolygonMode::eFill, + .cullMode = vk::CullModeFlagBits::eNone, + .frontFace = vk::FrontFace::eCounterClockwise, + .lineWidth = 1.0f, + }; + + const vk::PipelineMultisampleStateCreateInfo multisample{ .rasterizationSamples = vk::SampleCountFlagBits::e1 }; // Depth disabled — fullscreen blits don't need it. - VkPipelineDepthStencilStateCreateInfo depth_stencil{}; - depth_stencil.sType = VK_STRUCTURE_TYPE_PIPELINE_DEPTH_STENCIL_STATE_CREATE_INFO; - depth_stencil.depthTestEnable = VK_FALSE; - depth_stencil.depthWriteEnable = VK_FALSE; - - VkPipelineColorBlendAttachmentState blend_attachment{}; - blend_attachment.blendEnable = VK_FALSE; - blend_attachment.colorWriteMask = - VK_COLOR_COMPONENT_R_BIT | VK_COLOR_COMPONENT_G_BIT | VK_COLOR_COMPONENT_B_BIT | VK_COLOR_COMPONENT_A_BIT; - - VkPipelineColorBlendStateCreateInfo color_blend{}; - color_blend.sType = VK_STRUCTURE_TYPE_PIPELINE_COLOR_BLEND_STATE_CREATE_INFO; - color_blend.attachmentCount = 1; - color_blend.pAttachments = &blend_attachment; - - const VkDynamicState dynamic_states[] = { VK_DYNAMIC_STATE_VIEWPORT, VK_DYNAMIC_STATE_SCISSOR }; - VkPipelineDynamicStateCreateInfo dynamic{}; - dynamic.sType = VK_STRUCTURE_TYPE_PIPELINE_DYNAMIC_STATE_CREATE_INFO; - dynamic.dynamicStateCount = sizeof(dynamic_states) / sizeof(dynamic_states[0]); - dynamic.pDynamicStates = dynamic_states; - - VkGraphicsPipelineCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_GRAPHICS_PIPELINE_CREATE_INFO; - info.stageCount = 2; - info.pStages = stages; - info.pVertexInputState = &vertex_input; - info.pInputAssemblyState = &input_assembly; - info.pViewportState = &viewport_state; - info.pRasterizationState = &rasterizer; - info.pMultisampleState = &multisample; - info.pDepthStencilState = &depth_stencil; - info.pColorBlendState = &color_blend; - info.pDynamicState = &dynamic; - info.layout = pipeline_layout_; - info.renderPass = render_pass_; - info.subpass = 0; - - check_vk(vkCreateGraphicsPipelines(device, ctx_->pipeline_cache(), 1, &info, nullptr, &pipeline_), - "vkCreateGraphicsPipelines"); + const vk::PipelineDepthStencilStateCreateInfo depth_stencil{ + .depthTestEnable = VK_FALSE, + .depthWriteEnable = VK_FALSE, + }; + + const vk::PipelineColorBlendAttachmentState blend_attachment{ + .blendEnable = VK_FALSE, + .colorWriteMask = vk::ColorComponentFlagBits::eR | vk::ColorComponentFlagBits::eG | + vk::ColorComponentFlagBits::eB | vk::ColorComponentFlagBits::eA, + }; + + const vk::PipelineColorBlendStateCreateInfo color_blend{ + .attachmentCount = 1, + .pAttachments = &blend_attachment, + }; + + const std::array dynamic_states{ vk::DynamicState::eViewport, vk::DynamicState::eScissor }; + const vk::PipelineDynamicStateCreateInfo dynamic{ + .dynamicStateCount = static_cast(dynamic_states.size()), + .pDynamicStates = dynamic_states.data(), + }; + + pipeline_ = vk::raii::Pipeline{ + device, ctx_->raii_pipeline_cache(), + vk::GraphicsPipelineCreateInfo{ + .stageCount = static_cast(stages.size()), + .pStages = stages.data(), + .pVertexInputState = &vertex_input, + .pInputAssemblyState = &input_assembly, + .pViewportState = &viewport_state, + .pRasterizationState = &rasterizer, + .pMultisampleState = &multisample, + .pDepthStencilState = &depth_stencil, + .pColorBlendState = &color_blend, + .pDynamicState = &dynamic, + .layout = *pipeline_layout_, + .renderPass = render_pass_, + .subpass = 0, + } + }; } void QuadLayer::create_descriptor_pool() { - VkDescriptorPoolSize pool_size{}; - pool_size.type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; - pool_size.descriptorCount = kSlotCount; - - VkDescriptorPoolCreateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO; - info.maxSets = kSlotCount; - info.poolSizeCount = 1; - info.pPoolSizes = &pool_size; - check_vk(vkCreateDescriptorPool(ctx_->device(), &info, nullptr, &descriptor_pool_), "vkCreateDescriptorPool"); + const vk::DescriptorPoolSize pool_size{ + .type = vk::DescriptorType::eCombinedImageSampler, + .descriptorCount = kSlotCount, + }; + descriptor_pool_ = vk::raii::DescriptorPool{ + ctx_->raii_device(), + vk::DescriptorPoolCreateInfo{ + // freeDescriptorSet bit not set: sets are freed implicitly + // when the pool is destroyed (raii handles that). + .maxSets = kSlotCount, + .poolSizeCount = 1, + .pPoolSizes = &pool_size, + }, + }; } void QuadLayer::allocate_descriptor_sets() { - std::array layouts{}; - layouts.fill(descriptor_set_layout_); - - VkDescriptorSetAllocateInfo info{}; - info.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO; - info.descriptorPool = descriptor_pool_; - info.descriptorSetCount = kSlotCount; - info.pSetLayouts = layouts.data(); - check_vk(vkAllocateDescriptorSets(ctx_->device(), &info, descriptor_sets_.data()), "vkAllocateDescriptorSets"); + std::array layouts{}; + layouts.fill(*descriptor_set_layout_); + + descriptor_sets_ = vk::raii::DescriptorSets{ + ctx_->raii_device(), + vk::DescriptorSetAllocateInfo{ + .descriptorPool = *descriptor_pool_, + .descriptorSetCount = kSlotCount, + .pSetLayouts = layouts.data(), + }, + }; } void QuadLayer::update_descriptor_sets() { // One write per slot, each pointing at the slot's own image view. - std::array image_infos{}; - std::array writes{}; + std::array image_infos{}; + std::array writes{}; for (uint32_t i = 0; i < kSlotCount; ++i) { - image_infos[i].sampler = sampler_; - image_infos[i].imageView = slots_[i]->vk_image_view(); - image_infos[i].imageLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; - - writes[i].sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET; - writes[i].dstSet = descriptor_sets_[i]; - writes[i].dstBinding = 0; - writes[i].dstArrayElement = 0; - writes[i].descriptorCount = 1; - writes[i].descriptorType = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; - writes[i].pImageInfo = &image_infos[i]; + image_infos[i] = vk::DescriptorImageInfo{ + .sampler = *sampler_, + .imageView = slots_[i]->vk_image_view(), + .imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal, + }; + writes[i] = vk::WriteDescriptorSet{ + .dstSet = *descriptor_sets_[i], + .dstBinding = 0, + .dstArrayElement = 0, + .descriptorCount = 1, + .descriptorType = vk::DescriptorType::eCombinedImageSampler, + .pImageInfo = &image_infos[i], + }; } - vkUpdateDescriptorSets(ctx_->device(), kSlotCount, writes.data(), 0, nullptr); + ctx_->raii_device().updateDescriptorSets(writes, {}); } } // namespace viz From ce226492ff918e0d2c4fa6d4dc18419399ef3fd0 Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 12:16:32 -0700 Subject: [PATCH 15/20] viz: clang-format pass on migrated files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Whitespace / brace-position adjustments only — no semantic changes. Picked up by `clang-format --dry-run --Werror` after the vk::raii migration commits. CI's clang-format check now passes. Co-Authored-By: Claude Opus 4.7 --- src/viz/layers/cpp/quad_layer.cpp | 83 +++++++++---------- .../session/cpp/inc/viz/session/swapchain.hpp | 2 +- src/viz/session/cpp/offscreen_backend.cpp | 41 ++++----- src/viz/session/cpp/swapchain.cpp | 11 ++- src/viz/session/cpp/viz_compositor.cpp | 45 +++++----- 5 files changed, 83 insertions(+), 99 deletions(-) diff --git a/src/viz/layers/cpp/quad_layer.cpp b/src/viz/layers/cpp/quad_layer.cpp index 0ef551be8..ca33d817c 100644 --- a/src/viz/layers/cpp/quad_layer.cpp +++ b/src/viz/layers/cpp/quad_layer.cpp @@ -27,12 +27,10 @@ void check_cuda(cudaError_t result, const char* what) vk::raii::ShaderModule create_shader_module(const vk::raii::Device& device, const unsigned char* spv, size_t size) { - return vk::raii::ShaderModule{ - device, vk::ShaderModuleCreateInfo{ - .codeSize = size, - .pCode = reinterpret_cast(spv), - } - }; + return vk::raii::ShaderModule{ device, vk::ShaderModuleCreateInfo{ + .codeSize = size, + .pCode = reinterpret_cast(spv), + } }; } // Once destroy() has run, slots_[0] is the canonical "alive" signal @@ -274,25 +272,23 @@ std::vector QuadLayer::get_wait_semaphores() const void QuadLayer::create_sampler() { - sampler_ = vk::raii::Sampler{ - ctx_->raii_device(), - vk::SamplerCreateInfo{ - .magFilter = vk::Filter::eLinear, - .minFilter = vk::Filter::eLinear, - .mipmapMode = vk::SamplerMipmapMode::eNearest, - .addressModeU = vk::SamplerAddressMode::eClampToEdge, - .addressModeV = vk::SamplerAddressMode::eClampToEdge, - .addressModeW = vk::SamplerAddressMode::eClampToEdge, - .anisotropyEnable = VK_FALSE, // enable later when XR distance views need it - .maxAnisotropy = 1.0f, - .compareEnable = VK_FALSE, - .compareOp = vk::CompareOp::eAlways, - .minLod = 0.0f, - .maxLod = 0.0f, - .borderColor = vk::BorderColor::eIntOpaqueBlack, - .unnormalizedCoordinates = VK_FALSE, - } - }; + sampler_ = vk::raii::Sampler{ ctx_->raii_device(), vk::SamplerCreateInfo{ + .magFilter = vk::Filter::eLinear, + .minFilter = vk::Filter::eLinear, + .mipmapMode = vk::SamplerMipmapMode::eNearest, + .addressModeU = vk::SamplerAddressMode::eClampToEdge, + .addressModeV = vk::SamplerAddressMode::eClampToEdge, + .addressModeW = vk::SamplerAddressMode::eClampToEdge, + .anisotropyEnable = VK_FALSE, // enable later when XR + // distance views need it + .maxAnisotropy = 1.0f, + .compareEnable = VK_FALSE, + .compareOp = vk::CompareOp::eAlways, + .minLod = 0.0f, + .maxLod = 0.0f, + .borderColor = vk::BorderColor::eIntOpaqueBlack, + .unnormalizedCoordinates = VK_FALSE, + } }; } void QuadLayer::create_descriptor_set_layout() @@ -334,8 +330,7 @@ void QuadLayer::create_pipeline() const std::array stages{ vk::PipelineShaderStageCreateInfo{ .stage = vk::ShaderStageFlagBits::eVertex, .module = *vert, .pName = "main" }, - vk::PipelineShaderStageCreateInfo{ - .stage = vk::ShaderStageFlagBits::eFragment, .module = *frag, .pName = "main" }, + vk::PipelineShaderStageCreateInfo{ .stage = vk::ShaderStageFlagBits::eFragment, .module = *frag, .pName = "main" }, }; const vk::PipelineVertexInputStateCreateInfo vertex_input{}; @@ -377,24 +372,22 @@ void QuadLayer::create_pipeline() .pDynamicStates = dynamic_states.data(), }; - pipeline_ = vk::raii::Pipeline{ - device, ctx_->raii_pipeline_cache(), - vk::GraphicsPipelineCreateInfo{ - .stageCount = static_cast(stages.size()), - .pStages = stages.data(), - .pVertexInputState = &vertex_input, - .pInputAssemblyState = &input_assembly, - .pViewportState = &viewport_state, - .pRasterizationState = &rasterizer, - .pMultisampleState = &multisample, - .pDepthStencilState = &depth_stencil, - .pColorBlendState = &color_blend, - .pDynamicState = &dynamic, - .layout = *pipeline_layout_, - .renderPass = render_pass_, - .subpass = 0, - } - }; + pipeline_ = vk::raii::Pipeline{ device, ctx_->raii_pipeline_cache(), + vk::GraphicsPipelineCreateInfo{ + .stageCount = static_cast(stages.size()), + .pStages = stages.data(), + .pVertexInputState = &vertex_input, + .pInputAssemblyState = &input_assembly, + .pViewportState = &viewport_state, + .pRasterizationState = &rasterizer, + .pMultisampleState = &multisample, + .pDepthStencilState = &depth_stencil, + .pColorBlendState = &color_blend, + .pDynamicState = &dynamic, + .layout = *pipeline_layout_, + .renderPass = render_pass_, + .subpass = 0, + } }; } void QuadLayer::create_descriptor_pool() diff --git a/src/viz/session/cpp/inc/viz/session/swapchain.hpp b/src/viz/session/cpp/inc/viz/session/swapchain.hpp index 8d335aeb3..c0cc41a96 100644 --- a/src/viz/session/cpp/inc/viz/session/swapchain.hpp +++ b/src/viz/session/cpp/inc/viz/session/swapchain.hpp @@ -3,8 +3,8 @@ #pragma once -#include #include +#include #include #include diff --git a/src/viz/session/cpp/offscreen_backend.cpp b/src/viz/session/cpp/offscreen_backend.cpp index 156013084..6ec64458e 100644 --- a/src/viz/session/cpp/offscreen_backend.cpp +++ b/src/viz/session/cpp/offscreen_backend.cpp @@ -115,14 +115,13 @@ HostImage OffscreenBackend::readback_to_host() .imageSubresource = { .aspectMask = vk::ImageAspectFlagBits::eColor, .layerCount = 1 }, .imageExtent = { extent_.width, extent_.height, 1 }, }; - cmd.copyImageToBuffer(vk::Image{ render_target_->color_image() }, vk::ImageLayout::eTransferSrcOptimal, - *readback_buffer_, region); + cmd.copyImageToBuffer( + vk::Image{ render_target_->color_image() }, vk::ImageLayout::eTransferSrcOptimal, *readback_buffer_, region); cmd.end(); const vk::CommandBuffer cmd_handle = *cmd; - ctx_->raii_queue().submit(vk::SubmitInfo{ .commandBufferCount = 1, .pCommandBuffers = &cmd_handle }, - VK_NULL_HANDLE); + ctx_->raii_queue().submit(vk::SubmitInfo{ .commandBufferCount = 1, .pCommandBuffers = &cmd_handle }, VK_NULL_HANDLE); ctx_->raii_queue().waitIdle(); HostImage result(extent_, PixelFormat::kRGBA8); @@ -138,12 +137,11 @@ void OffscreenBackend::create_readback_staging() static_cast(extent_.width) * extent_.height * bytes_per_pixel(PixelFormat::kRGBA8); const auto& device = ctx_->raii_device(); - readback_buffer_ = vk::raii::Buffer{ device, - vk::BufferCreateInfo{ - .size = readback_byte_size_, - .usage = vk::BufferUsageFlagBits::eTransferDst, - .sharingMode = vk::SharingMode::eExclusive, - } }; + readback_buffer_ = vk::raii::Buffer{ device, vk::BufferCreateInfo{ + .size = readback_byte_size_, + .usage = vk::BufferUsageFlagBits::eTransferDst, + .sharingMode = vk::SharingMode::eExclusive, + } }; const auto reqs = readback_buffer_.getMemoryRequirements(); readback_memory_ = vk::raii::DeviceMemory{ @@ -158,19 +156,16 @@ void OffscreenBackend::create_readback_staging() readback_buffer_.bindMemory(*readback_memory_, 0); // Dedicated cmd pool — never races the compositor's per-frame buffer. - readback_command_pool_ = vk::raii::CommandPool{ - device, vk::CommandPoolCreateInfo{ - .flags = vk::CommandPoolCreateFlagBits::eResetCommandBuffer, - .queueFamilyIndex = ctx_->queue_family_index(), - } - }; - readback_command_buffers_ = vk::raii::CommandBuffers{ - device, vk::CommandBufferAllocateInfo{ - .commandPool = *readback_command_pool_, - .level = vk::CommandBufferLevel::ePrimary, - .commandBufferCount = 1, - } - }; + readback_command_pool_ = + vk::raii::CommandPool{ device, vk::CommandPoolCreateInfo{ + .flags = vk::CommandPoolCreateFlagBits::eResetCommandBuffer, + .queueFamilyIndex = ctx_->queue_family_index(), + } }; + readback_command_buffers_ = vk::raii::CommandBuffers{ device, vk::CommandBufferAllocateInfo{ + .commandPool = *readback_command_pool_, + .level = vk::CommandBufferLevel::ePrimary, + .commandBufferCount = 1, + } }; } } // namespace viz diff --git a/src/viz/session/cpp/swapchain.cpp b/src/viz/session/cpp/swapchain.cpp index 6421d33f0..5756a3b7d 100644 --- a/src/viz/session/cpp/swapchain.cpp +++ b/src/viz/session/cpp/swapchain.cpp @@ -33,8 +33,8 @@ vk::SurfaceFormatKHR pick_surface_format(const std::vector return f; } } - return formats.empty() ? vk::SurfaceFormatKHR{ vk::Format::eUndefined, vk::ColorSpaceKHR::eSrgbNonlinear } - : formats[0]; + return formats.empty() ? vk::SurfaceFormatKHR{ vk::Format::eUndefined, vk::ColorSpaceKHR::eSrgbNonlinear } : + formats[0]; } vk::Extent2D clamp_extent(const vk::SurfaceCapabilitiesKHR& caps, Resolution preferred) @@ -251,8 +251,7 @@ std::optional Swapchain::acquire_next_image() } if (r != vk::Result::eSuccess && r != vk::Result::eSuboptimalKHR) { - throw std::runtime_error("Swapchain::acquire_next_image: VkResult=" + - std::to_string(static_cast(r))); + throw std::runtime_error("Swapchain::acquire_next_image: VkResult=" + std::to_string(static_cast(r))); } return AcquiredImage{ image_index, static_cast(images_[image_index]), *sem, *render_done_[frame_slot_] }; } @@ -275,8 +274,8 @@ bool Swapchain::present(uint32_t image_index, VkSemaphore render_done) // raii::Queue::presentKHR throws on the OUT_OF_DATE / SUBOPTIMAL // result codes that we want to handle as flow control. Fall through // to the C entry point so the result code is observable. - const vk::Result r = static_cast( - vkQueuePresentKHR(ctx_->queue(), reinterpret_cast(&info))); + const vk::Result r = + static_cast(vkQueuePresentKHR(ctx_->queue(), reinterpret_cast(&info))); // Advance the slot regardless — next frame needs fresh semaphores. if (!images_.empty()) { diff --git a/src/viz/session/cpp/viz_compositor.cpp b/src/viz/session/cpp/viz_compositor.cpp index 9f65720af..1e3523002 100644 --- a/src/viz/session/cpp/viz_compositor.cpp +++ b/src/viz/session/cpp/viz_compositor.cpp @@ -68,26 +68,22 @@ void VizCompositor::destroy() void VizCompositor::create_command_pool_and_buffer() { - command_pool_ = vk::raii::CommandPool{ - ctx_->raii_device(), vk::CommandPoolCreateInfo{ - .flags = vk::CommandPoolCreateFlagBits::eResetCommandBuffer, - .queueFamilyIndex = ctx_->queue_family_index(), - } - }; - command_buffers_ = vk::raii::CommandBuffers{ - ctx_->raii_device(), vk::CommandBufferAllocateInfo{ - .commandPool = *command_pool_, - .level = vk::CommandBufferLevel::ePrimary, - .commandBufferCount = 1, - } - }; + command_pool_ = + vk::raii::CommandPool{ ctx_->raii_device(), vk::CommandPoolCreateInfo{ + .flags = vk::CommandPoolCreateFlagBits::eResetCommandBuffer, + .queueFamilyIndex = ctx_->queue_family_index(), + } }; + command_buffers_ = vk::raii::CommandBuffers{ ctx_->raii_device(), vk::CommandBufferAllocateInfo{ + .commandPool = *command_pool_, + .level = vk::CommandBufferLevel::ePrimary, + .commandBufferCount = 1, + } }; } void VizCompositor::submit_or_signal_fence(const vk::SubmitInfo& info, const char* what) { - const vk::Result r = - static_cast(vkQueueSubmit(ctx_->queue(), 1, reinterpret_cast(&info), - frame_sync_->in_flight_fence())); + const vk::Result r = static_cast( + vkQueueSubmit(ctx_->queue(), 1, reinterpret_cast(&info), frame_sync_->in_flight_fence())); if (r == vk::Result::eSuccess) { return; @@ -197,14 +193,15 @@ void VizCompositor::render(const std::vector& layers) clears[0].color = *reinterpret_cast(&config_.clear_color); clears[1].depthStencil = vk::ClearDepthStencilValue{ 1.0f, 0 }; - cmd.beginRenderPass(vk::RenderPassBeginInfo{ - .renderPass = rt.render_pass(), - .framebuffer = rt.framebuffer(), - .renderArea = vk::Rect2D{ vk::Offset2D{ 0, 0 }, vk::Extent2D{ rt_extent.width, rt_extent.height } }, - .clearValueCount = static_cast(clears.size()), - .pClearValues = clears.data(), - }, - vk::SubpassContents::eInline); + cmd.beginRenderPass( + vk::RenderPassBeginInfo{ + .renderPass = rt.render_pass(), + .framebuffer = rt.framebuffer(), + .renderArea = vk::Rect2D{ vk::Offset2D{ 0, 0 }, vk::Extent2D{ rt_extent.width, rt_extent.height } }, + .clearValueCount = static_cast(clears.size()), + .pClearValues = clears.data(), + }, + vk::SubpassContents::eInline); // Per-layer: pre-bind scissor (tile.outer); per-layer ViewInfo // gets viewport = tile.content. Layer record() takes raw From 3dd5352f0d737c18ea64ab6d14a8712a87cbe48d Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 12:18:20 -0700 Subject: [PATCH 16/20] viz/session: don't throw on OUT_OF_DATE in Swapchain::acquire_next_image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vk::raii::SwapchainKHR::acquireNextImage throws on the OUT_OF_DATE / SUBOPTIMAL result codes that we use for flow control (caller's signal to recreate). The migration regressed window-resize: the throw escaped out of WindowBackend::begin_frame and aborted the smoke binary. Drop to vkAcquireNextImageKHR so the VkResult is observable, mirroring the same workaround already in place for vkQueuePresentKHR. Repro: ./build/examples/televiz/window_smoke/viz_window_smoke, resize the GLFW window — main loop terminated with "vk::SwapchainKHR::acquireNextImage: ErrorOutOfDateKHR". Co-Authored-By: Claude Opus 4.7 --- src/viz/session/cpp/swapchain.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/viz/session/cpp/swapchain.cpp b/src/viz/session/cpp/swapchain.cpp index 5756a3b7d..e9fde8636 100644 --- a/src/viz/session/cpp/swapchain.cpp +++ b/src/viz/session/cpp/swapchain.cpp @@ -240,11 +240,12 @@ std::optional Swapchain::acquire_next_image() return std::nullopt; } const auto& sem = image_available_[frame_slot_]; - const auto result = swapchain_.acquireNextImage(UINT64_MAX, *sem, VK_NULL_HANDLE); - const vk::Result r = result.first; - const uint32_t image_index = result.second; - // OUT_OF_DATE: caller must recreate. SUBOPTIMAL: image is valid, - // pass it through and let the WSI scale on present. + // raii::SwapchainKHR::acquireNextImage throws on OUT_OF_DATE / + // SUBOPTIMAL — same flow-control codes we treat as normal here. + // Drop to the C entry point so the result is observable. + uint32_t image_index = 0; + const vk::Result r = static_cast( + vkAcquireNextImageKHR(*ctx_->raii_device(), *swapchain_, UINT64_MAX, *sem, VK_NULL_HANDLE, &image_index)); if (r == vk::Result::eErrorOutOfDateKHR) { return std::nullopt; From 41fbf04799925863eea1b40bb9d25f22a4bc53a8 Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 12:31:10 -0700 Subject: [PATCH 17/20] viz/core: portable debug-callback assignment + gate validation_features on availability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI reported on a different vulkan-hpp SDK that vk::PFN_DebugUtilsMessengerCallbackEXT is not declared, breaking the previous reinterpret_cast. Newer SDKs (including the local toolchain) wrap the callback as a vk::PFN_* typedef using vk::Flags<...>; older ones leave it as the raw C PFN_vk*. Direct assignment fails on the new SDK because vk::Flags<> is structurally distinct from the raw flag type. Derive the field type via decltype(std::declval<...>().pfnUserCallback) so the cast resolves to whatever the active SDK declares — no need to spell either name. The ABI is identical (vk::Flags is a trivial uint32_t wrapper). Also gate VK_EXT_validation_features on instance-extension availability (not just on the validation layer being present): some loaders advertise the layer without the extension, in which case unconditionally requesting it would fail vkCreateInstance. New helper is_instance_extension_available() and a third instance-create branch covers the layer-but-no-features case. Co-Authored-By: Claude Opus 4.7 --- src/viz/core/cpp/vk_context.cpp | 50 ++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/src/viz/core/cpp/vk_context.cpp b/src/viz/core/cpp/vk_context.cpp index 5b4e43d67..b07b5f89f 100644 --- a/src/viz/core/cpp/vk_context.cpp +++ b/src/viz/core/cpp/vk_context.cpp @@ -46,6 +46,18 @@ bool is_validation_layer_available() return false; } +bool is_instance_extension_available(const char* name) +{ + for (const auto& ext : vk::enumerateInstanceExtensionProperties()) + { + if (std::strcmp(ext.extensionName, name) == 0) + { + return true; + } + } + return false; +} + VKAPI_ATTR VkBool32 VKAPI_CALL debug_messenger_callback(VkDebugUtilsMessageSeverityFlagBitsEXT severity, VkDebugUtilsMessageTypeFlagsEXT /*types*/, const VkDebugUtilsMessengerCallbackDataEXT* data, @@ -260,15 +272,24 @@ void VkContext::create_instance(const Config& config) } std::vector instance_extensions; - instance_extensions.reserve(config.instance_extensions.size() + 1); + instance_extensions.reserve(config.instance_extensions.size() + 2); for (const auto& s : config.instance_extensions) { instance_extensions.push_back(s.c_str()); } + bool validation_features_enabled = false; if (validation_enabled_) { instance_extensions.push_back(VK_EXT_DEBUG_UTILS_EXTENSION_NAME); - instance_extensions.push_back(VK_EXT_VALIDATION_FEATURES_EXTENSION_NAME); + // VK_EXT_validation_features is bundled with recent SDKs but not + // every loader/driver advertises it. Gate the pNext chain on + // availability so vkCreateInstance doesn't fail when validation + // is requested but this extension isn't present. + if (is_instance_extension_available(VK_EXT_VALIDATION_FEATURES_EXTENSION_NAME)) + { + instance_extensions.push_back(VK_EXT_VALIDATION_FEATURES_EXTENSION_NAME); + validation_features_enabled = true; + } } const vk::ValidationFeatureEnableEXT enables[] = { @@ -279,15 +300,21 @@ void VkContext::create_instance(const Config& config) // Plain create-info (no pNext) — same struct serves both the // chained create-time messenger and the persistent post-create // messenger, since neither needs further chained structures. + // + // pfnUserCallback's declared type varies across vk-hpp SDKs: newer + // versions wrap it as vk::PFN_DebugUtilsMessengerCallbackEXT (with + // vk::Flags<...> for the messageType parameter), older versions + // leave it as the raw PFN_vkDebugUtilsMessengerCallbackEXT C + // typedef. Our callback uses the C signature; reinterpret_cast + // through decltype lets the same code compile against both. The + // ABI is identical (vk::Flags is a trivial uint32_t wrapper). + using PfnUserCallbackT = decltype(std::declval().pfnUserCallback); const vk::DebugUtilsMessengerCreateInfoEXT debug_create_info{ .messageSeverity = vk::DebugUtilsMessageSeverityFlagBitsEXT::eWarning | vk::DebugUtilsMessageSeverityFlagBitsEXT::eError, .messageType = vk::DebugUtilsMessageTypeFlagBitsEXT::eGeneral | vk::DebugUtilsMessageTypeFlagBitsEXT::eValidation | vk::DebugUtilsMessageTypeFlagBitsEXT::ePerformance, - // C ABI callback; vk::Flags wrappers are layout-compatible - // with the raw C flag types but the function-pointer type - // signatures aren't, hence the reinterpret_cast. - .pfnUserCallback = reinterpret_cast(debug_messenger_callback), + .pfnUserCallback = reinterpret_cast(debug_messenger_callback), }; const vk::InstanceCreateInfo base_info{ @@ -298,7 +325,7 @@ void VkContext::create_instance(const Config& config) .ppEnabledExtensionNames = instance_extensions.data(), }; - if (validation_enabled_) + if (validation_features_enabled) { // Both ValidationFeaturesEXT and DebugUtilsMessengerCreateInfoEXT // extend VkInstanceCreateInfo. The loader walks the entire pNext @@ -319,6 +346,15 @@ void VkContext::create_instance(const Config& config) instance_ = vk::raii::Instance{ context_, chain.get() }; debug_messenger_ = vk::raii::DebugUtilsMessengerEXT{ instance_, debug_create_info }; } + else if (validation_enabled_) + { + // Validation layer available, but VK_EXT_validation_features is + // not — chain only the create-time messenger. + vk::StructureChain chain{ base_info, + debug_create_info }; + instance_ = vk::raii::Instance{ context_, chain.get() }; + debug_messenger_ = vk::raii::DebugUtilsMessengerEXT{ instance_, debug_create_info }; + } else { instance_ = vk::raii::Instance{ context_, base_info }; From 992b09a1bc7c9308667c6bd685f0cd944c8bacb2 Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 12:59:10 -0700 Subject: [PATCH 18/20] viz/core: portable find_memory_type signature + lift StructureChain args to locals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two CI failures on Ubuntu ARM64 Debug, both caused by a stricter / older vulkan-hpp than the local SDK: 1. find_memory_type in render_target.cpp and device_image.cpp took vk::PhysicalDevice by value and relied on implicit conversion from const vk::raii::PhysicalDevice& at the call site. The CI SDK doesn't provide that conversion. Standardize on const vk::raii::PhysicalDevice& (matches OffscreenBackend's already-portable signature) — the body calls .getMemoryProperties() which works directly on the raii type. 2. vk::StructureChain<...> constructed with anonymous designated-init aggregates as arguments fails template-argument deduction on the CI SDK. Same workaround we already used in vk_context.cpp and viz_compositor.cpp: lift each chain element to a named local first, then pass the locals to the chain constructor. Three sites in device_image.cpp (image, allocate, semaphore chains). Local build + sanitizer build + 52 unit tests stay green; clang-format clean. Co-Authored-By: Claude Opus 4.7 --- src/viz/core/cpp/device_image.cpp | 91 +++++++++++++++--------------- src/viz/core/cpp/render_target.cpp | 4 +- 2 files changed, 50 insertions(+), 45 deletions(-) diff --git a/src/viz/core/cpp/device_image.cpp b/src/viz/core/cpp/device_image.cpp index 6b9fc3e49..750b520a6 100644 --- a/src/viz/core/cpp/device_image.cpp +++ b/src/viz/core/cpp/device_image.cpp @@ -44,7 +44,9 @@ void check_cuda(cudaError_t result, const char* what) } } -uint32_t find_memory_type(vk::PhysicalDevice physical_device, uint32_t type_bits, vk::MemoryPropertyFlags properties) +uint32_t find_memory_type(const vk::raii::PhysicalDevice& physical_device, + uint32_t type_bits, + vk::MemoryPropertyFlags properties) { const auto mem_props = physical_device.getMemoryProperties(); for (uint32_t i = 0; i < mem_props.memoryTypeCount; ++i) @@ -217,49 +219,49 @@ void DeviceImage::create_vk_image_with_external_memory() // Optimal tiling — CUDA accesses the image via cudaArray_t, not // raw memory, so opaque GPU layout is fine. - vk::StructureChain image_chain{ - vk::ImageCreateInfo{ - // Storage in linear-space format (UNORM); SRGB view - // attached in create_vk_image_view(). - // VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT is what allows view - // format != image format among compatible formats - // (UNORM <-> SRGB are in the same compatibility class). - .flags = vk::ImageCreateFlagBits::eMutableFormat, - .imageType = vk::ImageType::e2D, - .format = static_cast(to_vk_storage_format(format_)), - .extent = { resolution_.width, resolution_.height, 1 }, - // Single level. If XR distance views show moiré, expose - // mipLevels via Config and generate via vkCmdBlitImage - // pre-render. - .mipLevels = 1, - .arrayLayers = 1, - .samples = vk::SampleCountFlagBits::e1, - .tiling = vk::ImageTiling::eOptimal, - .usage = vk::ImageUsageFlagBits::eSampled | vk::ImageUsageFlagBits::eTransferDst | - vk::ImageUsageFlagBits::eTransferSrc, - .sharingMode = vk::SharingMode::eExclusive, - .initialLayout = vk::ImageLayout::eUndefined, - }, - vk::ExternalMemoryImageCreateInfo{ - .handleTypes = vk::ExternalMemoryHandleTypeFlagBits::eOpaqueFd, - }, + // + // Storage in linear-space format (UNORM); SRGB view attached in + // create_vk_image_view(). VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT + // allows view format != image format among compatible formats + // (UNORM <-> SRGB are in the same compatibility class). + const vk::ImageCreateInfo image_info{ + .flags = vk::ImageCreateFlagBits::eMutableFormat, + .imageType = vk::ImageType::e2D, + .format = static_cast(to_vk_storage_format(format_)), + .extent = { resolution_.width, resolution_.height, 1 }, + // Single level. If XR distance views show moiré, expose + // mipLevels via Config and generate via vkCmdBlitImage + // pre-render. + .mipLevels = 1, + .arrayLayers = 1, + .samples = vk::SampleCountFlagBits::e1, + .tiling = vk::ImageTiling::eOptimal, + .usage = vk::ImageUsageFlagBits::eSampled | vk::ImageUsageFlagBits::eTransferDst | + vk::ImageUsageFlagBits::eTransferSrc, + .sharingMode = vk::SharingMode::eExclusive, + .initialLayout = vk::ImageLayout::eUndefined, + }; + const vk::ExternalMemoryImageCreateInfo image_external_info{ + .handleTypes = vk::ExternalMemoryHandleTypeFlagBits::eOpaqueFd, }; + vk::StructureChain image_chain{ image_info, + image_external_info }; image_ = vk::raii::Image{ device, image_chain.get() }; const auto reqs = image_.getMemoryRequirements(); // Device-local + exportable as POSIX fd. Generic allocation // (no VkMemoryDedicatedAllocateInfo) suffices for sampled 2D. - vk::StructureChain alloc_chain{ - vk::MemoryAllocateInfo{ - .allocationSize = reqs.size, - .memoryTypeIndex = find_memory_type( - ctx_->raii_physical_device(), reqs.memoryTypeBits, vk::MemoryPropertyFlagBits::eDeviceLocal), - }, - vk::ExportMemoryAllocateInfo{ - .handleTypes = vk::ExternalMemoryHandleTypeFlagBits::eOpaqueFd, - }, + const vk::MemoryAllocateInfo alloc_info{ + .allocationSize = reqs.size, + .memoryTypeIndex = find_memory_type( + ctx_->raii_physical_device(), reqs.memoryTypeBits, vk::MemoryPropertyFlagBits::eDeviceLocal), }; + const vk::ExportMemoryAllocateInfo alloc_external_info{ + .handleTypes = vk::ExternalMemoryHandleTypeFlagBits::eOpaqueFd, + }; + vk::StructureChain alloc_chain{ alloc_info, + alloc_external_info }; memory_ = vk::raii::DeviceMemory{ device, alloc_chain.get() }; image_.bindMemory(*memory_, 0); @@ -337,15 +339,16 @@ void DeviceImage::create_interop_semaphores() // Timeline semaphore (initial value 0) exported via OPAQUE_FD and // imported into CUDA. + const vk::SemaphoreCreateInfo sem_info{}; + const vk::ExportSemaphoreCreateInfo sem_export_info{ + .handleTypes = vk::ExternalSemaphoreHandleTypeFlagBits::eOpaqueFd, + }; + const vk::SemaphoreTypeCreateInfo sem_type_info{ + .semaphoreType = vk::SemaphoreType::eTimeline, + .initialValue = 0, + }; vk::StructureChain sem_chain{ - vk::SemaphoreCreateInfo{}, - vk::ExportSemaphoreCreateInfo{ - .handleTypes = vk::ExternalSemaphoreHandleTypeFlagBits::eOpaqueFd, - }, - vk::SemaphoreTypeCreateInfo{ - .semaphoreType = vk::SemaphoreType::eTimeline, - .initialValue = 0, - }, + sem_info, sem_export_info, sem_type_info }; cuda_done_writing_ = vk::raii::Semaphore{ device, sem_chain.get() }; diff --git a/src/viz/core/cpp/render_target.cpp b/src/viz/core/cpp/render_target.cpp index cd36f0b2a..df5eee7d8 100644 --- a/src/viz/core/cpp/render_target.cpp +++ b/src/viz/core/cpp/render_target.cpp @@ -16,7 +16,9 @@ namespace // Find a memory type matching `type_bits` (bitfield from // VkMemoryRequirements::memoryTypeBits) that has all required `properties`. // Throws if no match (callers should request DEVICE_LOCAL for attachments). -uint32_t find_memory_type(vk::PhysicalDevice physical_device, uint32_t type_bits, vk::MemoryPropertyFlags properties) +uint32_t find_memory_type(const vk::raii::PhysicalDevice& physical_device, + uint32_t type_bits, + vk::MemoryPropertyFlags properties) { const auto mem_props = physical_device.getMemoryProperties(); for (uint32_t i = 0; i < mem_props.memoryTypeCount; ++i) From e12786054e6c9db87b5f9b798d65c7ba626c5a14 Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 13:10:26 -0700 Subject: [PATCH 19/20] fix --- .../layers/cpp/inc/viz/layers/quad_layer.hpp | 8 +++++++- src/viz/layers/cpp/quad_layer.cpp | 19 ++++++++----------- .../cpp/inc/viz/session/offscreen_backend.hpp | 5 ++++- .../cpp/inc/viz/session/viz_compositor.hpp | 5 ++++- src/viz/session/cpp/offscreen_backend.cpp | 14 +++++++------- src/viz/session/cpp/viz_compositor.cpp | 14 +++++++------- 6 files changed, 37 insertions(+), 28 deletions(-) diff --git a/src/viz/layers/cpp/inc/viz/layers/quad_layer.hpp b/src/viz/layers/cpp/inc/viz/layers/quad_layer.hpp index 1c2162890..a2a855e38 100644 --- a/src/viz/layers/cpp/inc/viz/layers/quad_layer.hpp +++ b/src/viz/layers/cpp/inc/viz/layers/quad_layer.hpp @@ -14,6 +14,7 @@ #include #include #include +#include #include namespace viz @@ -143,7 +144,12 @@ class QuadLayer : public LayerBase vk::raii::DescriptorPool descriptor_pool_{ nullptr }; // One descriptor set per slot, each binding the corresponding // DeviceImage's sRGB view. record() picks the one for in_use_. - vk::raii::DescriptorSets descriptor_sets_{ nullptr }; + // + // Wrapped in std::optional because vk::raii::DescriptorSets is a + // vector-style raii type — older vulkan-hpp SDKs (e.g., Ubuntu + // 22.04 ARM64 CI) lack the nullptr ctor / nullptr-assign that + // newer SDKs added. optional gives us portable lazy init. + std::optional descriptor_sets_; // Mailbox state. Both atomic so producer and renderer can // touch them without locks. diff --git a/src/viz/layers/cpp/quad_layer.cpp b/src/viz/layers/cpp/quad_layer.cpp index ca33d817c..9cd68576d 100644 --- a/src/viz/layers/cpp/quad_layer.cpp +++ b/src/viz/layers/cpp/quad_layer.cpp @@ -108,7 +108,7 @@ void QuadLayer::destroy() // before its layout, sampler last. raii handles the actual // destruction order via reset-to-nullptr in declared order // (parent-first declaration → reverse runs child-first). - descriptor_sets_ = nullptr; + descriptor_sets_.reset(); descriptor_pool_ = nullptr; pipeline_ = nullptr; pipeline_layout_ = nullptr; @@ -232,7 +232,7 @@ void QuadLayer::record(VkCommandBuffer cmd, const std::vector& views, const vk::CommandBuffer cmd_hpp{ cmd }; cmd_hpp.bindPipeline(vk::PipelineBindPoint::eGraphics, *pipeline_); - cmd_hpp.bindDescriptorSets(vk::PipelineBindPoint::eGraphics, *pipeline_layout_, 0, *descriptor_sets_[cur], {}); + cmd_hpp.bindDescriptorSets(vk::PipelineBindPoint::eGraphics, *pipeline_layout_, 0, *(*descriptor_sets_)[cur], {}); // 1 view in window/offscreen, 2 in XR stereo. Compositor pre-bound // the layer's scissor; we bind viewport per view and draw. @@ -413,14 +413,11 @@ void QuadLayer::allocate_descriptor_sets() std::array layouts{}; layouts.fill(*descriptor_set_layout_); - descriptor_sets_ = vk::raii::DescriptorSets{ - ctx_->raii_device(), - vk::DescriptorSetAllocateInfo{ - .descriptorPool = *descriptor_pool_, - .descriptorSetCount = kSlotCount, - .pSetLayouts = layouts.data(), - }, - }; + descriptor_sets_.emplace(ctx_->raii_device(), vk::DescriptorSetAllocateInfo{ + .descriptorPool = *descriptor_pool_, + .descriptorSetCount = kSlotCount, + .pSetLayouts = layouts.data(), + }); } void QuadLayer::update_descriptor_sets() @@ -436,7 +433,7 @@ void QuadLayer::update_descriptor_sets() .imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal, }; writes[i] = vk::WriteDescriptorSet{ - .dstSet = *descriptor_sets_[i], + .dstSet = *(*descriptor_sets_)[i], .dstBinding = 0, .dstArrayElement = 0, .descriptorCount = 1, diff --git a/src/viz/session/cpp/inc/viz/session/offscreen_backend.hpp b/src/viz/session/cpp/inc/viz/session/offscreen_backend.hpp index a1331e138..4e9b6bf14 100644 --- a/src/viz/session/cpp/inc/viz/session/offscreen_backend.hpp +++ b/src/viz/session/cpp/inc/viz/session/offscreen_backend.hpp @@ -7,6 +7,7 @@ #include #include +#include namespace viz { @@ -46,7 +47,9 @@ class OffscreenBackend final : public DisplayBackend // Dedicated cmd buffer so readback never races the compositor's. vk::raii::CommandPool readback_command_pool_{ nullptr }; - vk::raii::CommandBuffers readback_command_buffers_{ nullptr }; + // Wrapped in std::optional — older vulkan-hpp SDKs lack the + // nullptr ctor on the vector-style raii types. + std::optional readback_command_buffers_; }; } // namespace viz diff --git a/src/viz/session/cpp/inc/viz/session/viz_compositor.hpp b/src/viz/session/cpp/inc/viz/session/viz_compositor.hpp index 23c134a0f..a5dc2c904 100644 --- a/src/viz/session/cpp/inc/viz/session/viz_compositor.hpp +++ b/src/viz/session/cpp/inc/viz/session/viz_compositor.hpp @@ -9,6 +9,7 @@ #include #include +#include #include namespace viz @@ -67,7 +68,9 @@ class VizCompositor std::unique_ptr frame_sync_; vk::raii::CommandPool command_pool_{ nullptr }; - vk::raii::CommandBuffers command_buffers_{ nullptr }; + // Wrapped in std::optional — older vulkan-hpp SDKs lack the + // nullptr ctor on the vector-style raii types. + std::optional command_buffers_; }; } // namespace viz diff --git a/src/viz/session/cpp/offscreen_backend.cpp b/src/viz/session/cpp/offscreen_backend.cpp index 6ec64458e..e053b1b0b 100644 --- a/src/viz/session/cpp/offscreen_backend.cpp +++ b/src/viz/session/cpp/offscreen_backend.cpp @@ -60,7 +60,7 @@ void OffscreenBackend::init(const VkContext& ctx, Resolution preferred_size) void OffscreenBackend::destroy() { - readback_command_buffers_ = nullptr; + readback_command_buffers_.reset(); readback_command_pool_ = nullptr; readback_buffer_ = nullptr; readback_memory_ = nullptr; @@ -105,7 +105,7 @@ HostImage OffscreenBackend::readback_to_host() throw std::runtime_error("OffscreenBackend::readback_to_host: backend not initialized"); } - auto& cmd = readback_command_buffers_[0]; + auto& cmd = (*readback_command_buffers_)[0]; // RT is in TRANSFER_SRC_OPTIMAL from the render pass's final layout. cmd.reset(); @@ -161,11 +161,11 @@ void OffscreenBackend::create_readback_staging() .flags = vk::CommandPoolCreateFlagBits::eResetCommandBuffer, .queueFamilyIndex = ctx_->queue_family_index(), } }; - readback_command_buffers_ = vk::raii::CommandBuffers{ device, vk::CommandBufferAllocateInfo{ - .commandPool = *readback_command_pool_, - .level = vk::CommandBufferLevel::ePrimary, - .commandBufferCount = 1, - } }; + readback_command_buffers_.emplace(device, vk::CommandBufferAllocateInfo{ + .commandPool = *readback_command_pool_, + .level = vk::CommandBufferLevel::ePrimary, + .commandBufferCount = 1, + }); } } // namespace viz diff --git a/src/viz/session/cpp/viz_compositor.cpp b/src/viz/session/cpp/viz_compositor.cpp index 1e3523002..bb950d236 100644 --- a/src/viz/session/cpp/viz_compositor.cpp +++ b/src/viz/session/cpp/viz_compositor.cpp @@ -61,7 +61,7 @@ void VizCompositor::init() void VizCompositor::destroy() { - command_buffers_ = nullptr; + command_buffers_.reset(); command_pool_ = nullptr; frame_sync_.reset(); } @@ -73,11 +73,11 @@ void VizCompositor::create_command_pool_and_buffer() .flags = vk::CommandPoolCreateFlagBits::eResetCommandBuffer, .queueFamilyIndex = ctx_->queue_family_index(), } }; - command_buffers_ = vk::raii::CommandBuffers{ ctx_->raii_device(), vk::CommandBufferAllocateInfo{ - .commandPool = *command_pool_, - .level = vk::CommandBufferLevel::ePrimary, - .commandBufferCount = 1, - } }; + command_buffers_.emplace(ctx_->raii_device(), vk::CommandBufferAllocateInfo{ + .commandPool = *command_pool_, + .level = vk::CommandBufferLevel::ePrimary, + .commandBufferCount = 1, + }); } void VizCompositor::submit_or_signal_fence(const vk::SubmitInfo& info, const char* what) @@ -101,7 +101,7 @@ void VizCompositor::render(const std::vector& layers) // Wait for previous frame (1 frame in flight). frame_sync_->wait(); - auto& cmd = command_buffers_[0]; + auto& cmd = (*command_buffers_)[0]; // RAII: leave the command buffer in INITIAL state on every exit // path (success or throw). VizSession::pump_events() runs between From d9e18f1edee33a1cf5a4f0cbe60334a5c5207f7d Mon Sep 17 00:00:00 2001 From: Farbod Motlagh Date: Wed, 6 May 2026 13:28:13 -0700 Subject: [PATCH 20/20] viz/session: cast through raw VkXxx for VK_NULL_HANDLE comparisons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Older vulkan-hpp SDKs (Ubuntu 22.04 stock) declare both vk::Xxx::operator==(const vk::Xxx&) const = default; // from <=> // built-in: operator==(VkXxx, VkXxx) which makes `*raii_obj == VK_NULL_HANDLE` ambiguous — VK_NULL_HANDLE matches both candidates. Cast to the raw VkXxx so the comparison binds unambiguously to the C builtin. Eight sites: glfw_window (instance), viz_compositor (cmd buffer), offscreen_backend (buffer), swapchain (swapchain x3, device x2). Co-Authored-By: Claude Opus 4.7 --- src/viz/session/cpp/glfw_window.cpp | 2 +- src/viz/session/cpp/offscreen_backend.cpp | 2 +- src/viz/session/cpp/swapchain.cpp | 10 +++++----- src/viz/session/cpp/viz_compositor.cpp | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/viz/session/cpp/glfw_window.cpp b/src/viz/session/cpp/glfw_window.cpp index b38f8df82..530b83dcf 100644 --- a/src/viz/session/cpp/glfw_window.cpp +++ b/src/viz/session/cpp/glfw_window.cpp @@ -67,7 +67,7 @@ std::unique_ptr GlfwWindow::create(const vk::raii::Instance& instanc uint32_t height, const std::string& title) { - if (*instance == VK_NULL_HANDLE) + if (static_cast(*instance) == VK_NULL_HANDLE) { throw std::invalid_argument("GlfwWindow::create: instance is VK_NULL_HANDLE"); } diff --git a/src/viz/session/cpp/offscreen_backend.cpp b/src/viz/session/cpp/offscreen_backend.cpp index e053b1b0b..bc4f5d678 100644 --- a/src/viz/session/cpp/offscreen_backend.cpp +++ b/src/viz/session/cpp/offscreen_backend.cpp @@ -100,7 +100,7 @@ Resolution OffscreenBackend::current_extent() const HostImage OffscreenBackend::readback_to_host() { - if (render_target_ == nullptr || *readback_buffer_ == VK_NULL_HANDLE) + if (render_target_ == nullptr || static_cast(*readback_buffer_) == VK_NULL_HANDLE) { throw std::runtime_error("OffscreenBackend::readback_to_host: backend not initialized"); } diff --git a/src/viz/session/cpp/swapchain.cpp b/src/viz/session/cpp/swapchain.cpp index e9fde8636..ab1449987 100644 --- a/src/viz/session/cpp/swapchain.cpp +++ b/src/viz/session/cpp/swapchain.cpp @@ -165,7 +165,7 @@ void Swapchain::init(Resolution preferred_size, VkSwapchainKHR old_swapchain) catch (...) { // Drain and reset partially-built state so retry is sane. - if (*ctx_->raii_device() != VK_NULL_HANDLE) + if (static_cast(*ctx_->raii_device()) != VK_NULL_HANDLE) { (void)ctx_->raii_device().waitIdle(); } @@ -193,7 +193,7 @@ void Swapchain::create_semaphores() void Swapchain::destroy() { - if (ctx_ != nullptr && *ctx_->raii_device() != VK_NULL_HANDLE) + if (ctx_ != nullptr && static_cast(*ctx_->raii_device()) != VK_NULL_HANDLE) { // Drain so we don't destroy semaphores still referenced by the queue. (void)ctx_->raii_device().waitIdle(); @@ -210,7 +210,7 @@ void Swapchain::destroy() void Swapchain::recreate(Resolution preferred_size) { - if (*swapchain_ == VK_NULL_HANDLE) + if (static_cast(*swapchain_) == VK_NULL_HANDLE) { init(preferred_size); return; @@ -235,7 +235,7 @@ void Swapchain::recreate(Resolution preferred_size) std::optional Swapchain::acquire_next_image() { - if (*swapchain_ == VK_NULL_HANDLE || image_available_.empty()) + if (static_cast(*swapchain_) == VK_NULL_HANDLE || image_available_.empty()) { return std::nullopt; } @@ -259,7 +259,7 @@ std::optional Swapchain::acquire_next_image() bool Swapchain::present(uint32_t image_index, VkSemaphore render_done) { - if (*swapchain_ == VK_NULL_HANDLE) + if (static_cast(*swapchain_) == VK_NULL_HANDLE) { return false; } diff --git a/src/viz/session/cpp/viz_compositor.cpp b/src/viz/session/cpp/viz_compositor.cpp index bb950d236..2e2ec8a5d 100644 --- a/src/viz/session/cpp/viz_compositor.cpp +++ b/src/viz/session/cpp/viz_compositor.cpp @@ -114,7 +114,7 @@ void VizCompositor::render(const std::vector& layers) vk::raii::CommandBuffer* cmd; ~CmdResetGuard() { - if (cmd != nullptr && **cmd != VK_NULL_HANDLE) + if (cmd != nullptr && static_cast(**cmd) != VK_NULL_HANDLE) { cmd->reset(); }