From 012e7fa3bf85e524fc21ddf5a7f0690057817535 Mon Sep 17 00:00:00 2001 From: toloudis Date: Sun, 30 Aug 2026 17:53:06 -0700 Subject: [PATCH 1/9] improved and detailed device selection with or without a surface --- agave_app/QtVulkanSurface.h | 4 +- agave_app/VulkanView3D.cpp | 25 +- agave_app/main.cpp | 31 +- agave_pyvk/src/bindings.cpp | 5 +- docs/agave.rst | 10 +- renderlib/PythonRenderer.h | 2 + renderlib/gfxOpenGL/Backend.cpp | 4 + renderlib/gfxVulkan/Backend.cpp | 501 ++++++++++++++++++++++-- renderlib/gfxVulkan/Backend.h | 19 +- renderlib/gfxVulkan/CMakeLists.txt | 1 + renderlib/gfxVulkan/NativeSurface.h | 19 + renderlib/gfxVulkan/Swapchain.cpp | 2 +- renderlib/gfxVulkan/Swapchain.h | 40 +- renderlib/gfxVulkan/Swapchain_linux.cpp | 45 ++- renderlib/gfxVulkan/Swapchain_mac.mm | 33 +- renderlib/gfxVulkan/Swapchain_win.cpp | 30 +- renderlib/gfxapi/Backend.h | 30 +- renderlib/gfxapi/CMakeLists.txt | 1 + renderlib/gfxapi/WindowSurface.h | 43 ++ renderlib/renderlib.cpp | 9 + 20 files changed, 724 insertions(+), 130 deletions(-) create mode 100644 renderlib/gfxVulkan/NativeSurface.h create mode 100644 renderlib/gfxapi/WindowSurface.h diff --git a/agave_app/QtVulkanSurface.h b/agave_app/QtVulkanSurface.h index 56d350e61..cacdf7274 100644 --- a/agave_app/QtVulkanSurface.h +++ b/agave_app/QtVulkanSurface.h @@ -2,7 +2,7 @@ #if AGAVE_HAS_VULKAN -#include "renderlib/gfxVulkan/Swapchain.h" +#include "renderlib/gfxapi/WindowSurface.h" #include @@ -17,7 +17,7 @@ class QWidget; // widget own the native surface keeps it laid out by Qt exactly like any other // widget, avoiding the geometry offsets that come with embedding a separate // QWindow via QWidget::createWindowContainer. -class QtVulkanSurface : public gfxvulkan::ISwapchainSurface +class QtVulkanSurface : public gfxApi::IWindowSurface { public: explicit QtVulkanSurface(QWidget* widget); diff --git a/agave_app/VulkanView3D.cpp b/agave_app/VulkanView3D.cpp index 173711fb0..5729ab47b 100644 --- a/agave_app/VulkanView3D.cpp +++ b/agave_app/VulkanView3D.cpp @@ -7,6 +7,7 @@ #include "ViewerState.h" #include "renderlib/AppScene.h" +#include "renderlib/Logging.h" #include "renderlib/MoveTool.h" #include "renderlib/RenderSettings.h" #include "renderlib/RotateTool.h" @@ -20,6 +21,7 @@ #include #include #include +#include #include #include @@ -72,7 +74,6 @@ VulkanView3D::VulkanView3D(QCamera* cam, QRenderSettings* qrs, RenderSettings* r : QWidget(parent) , m_qcamera(cam) , m_qrendersettings(qrs) - , m_viewerWindow(std::make_unique(rs)) { // Render directly into this widget's own native surface. Qt lays out a native // widget exactly like any other, so the Vulkan content aligns with its place @@ -89,7 +90,29 @@ VulkanView3D::VulkanView3D(QCamera* cam, QRenderSettings* qrs, RenderSettings* r setMinimumSize(256, 256); setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding); + // Force Qt to realize the native window now. The backend has to build a real + // VkSurfaceKHR from it before it can tell which physical device and queue + // family can present here. + (void)winId(); m_surface = std::make_unique(this); + + // Surface-aware device selection has to happen before ViewerWindow, which + // immediately creates the renderers and gesture renderer -- and those need a + // selected physical device, logical device, queues, and command pool. + gfxApi::Backend* backend = renderlib::graphicsBackend(); + if (!backend) { + const auto msg = "Cannot create a Vulkan view without an initialized graphics backend"; + LOG_ERROR << msg; + throw std::runtime_error(msg); + } + // initDeviceForWindow has already logged which devices were rejected and why. + if (!backend->initDeviceForWindow(m_surface.get())) { + const auto msg = "Failed to initialize a graphics device that can present to the 3D view window"; + LOG_ERROR << msg; + throw std::runtime_error(msg); + } + + m_viewerWindow = std::make_unique(rs); m_swapchain = std::make_unique(m_surface.get()); m_viewerWindow->gesture.input.setDoubleClickTime(static_cast(QApplication::doubleClickInterval()) / 1000.0); diff --git a/agave_app/main.cpp b/agave_app/main.cpp index c6616a974..d381428a9 100644 --- a/agave_app/main.cpp +++ b/agave_app/main.cpp @@ -227,7 +227,10 @@ main(int argc, char* argv[]) QCommandLineOption listDevicesOption( "list_devices", - QCoreApplication::translate("main", "Log the known graphics devices (only valid in --server mode).")); + QCoreApplication::translate( + "main", + "Log the known graphics devices and exit. The indices reported are the ones --gpu selects. With the OpenGL " + "backend this is only valid in --server mode.")); parser.addOption(listDevicesOption); #if AGAVE_HAS_VULKAN const QString defaultGraphicsBackend = "vulkan"; @@ -240,10 +243,14 @@ main(int argc, char* argv[]) QCoreApplication::translate("main", "backend"), defaultGraphicsBackend); parser.addOption(graphicsBackendOption); - QCommandLineOption selectGpuOption("gpu", - QCoreApplication::translate("main", "Select GPU/device by index."), - QCoreApplication::translate("main", "gpu"), - "0"); + // No default value here on purpose: parser.isSet() then distinguishes "the + // user did not ask for a specific GPU" (auto-select) from "the user asked + // for index 0", which must be validated and never silently replaced. + QCommandLineOption selectGpuOption( + "gpu", + QCoreApplication::translate( + "main", "Select GPU/device by index, as listed by --list_devices. Defaults to automatic selection."), + QCoreApplication::translate("main", "gpu")); parser.addOption(selectGpuOption); QCommandLineOption serverConfigOption("config", QCoreApplication::translate("main", "Path to config file."), @@ -258,7 +265,17 @@ main(int argc, char* argv[]) bool hasPort = parser.isSet(serverPortOption); int port = parser.value(serverPortOption).toInt(); bool listDevices = parser.isSet(listDevicesOption); - int selectedGpu = parser.value(selectGpuOption).toInt(); + int selectedGpu = gfxApi::kAutoSelectGpu; + if (parser.isSet(selectGpuOption)) { + bool gpuIndexOk = false; + const int requestedGpu = parser.value(selectGpuOption).toInt(&gpuIndexOk); + if (!gpuIndexOk || requestedGpu < 0) { + LOG_ERROR << "Invalid --gpu value: " << parser.value(selectGpuOption).toStdString() + << ". Expected a zero-based device index; run with --list_devices to see the valid indices."; + return 0; + } + selectedGpu = requestedGpu; + } gfxApi::BackendKind backendKind = #if AGAVE_HAS_VULKAN gfxApi::BackendKind::Vulkan; @@ -322,7 +339,7 @@ main(int argc, char* argv[]) } // Register the cache directory once for the lifetime of the process, after - // renderlib has successfully initialized. + // renderlib has successfully initialized. // Note that caching stays inert until a CacheConfig enables it. CacheManager::initialize(getCacheDirectory()); diff --git a/agave_pyvk/src/bindings.cpp b/agave_pyvk/src/bindings.cpp index 590aa64ac..c3678cab4 100644 --- a/agave_pyvk/src/bindings.cpp +++ b/agave_pyvk/src/bindings.cpp @@ -1,6 +1,7 @@ #include "renderlib/PythonRenderer.h" #include "renderlib/ImageXYZC.h" #include "renderlib/VolumeDimensions.h" +#include "renderlib/gfxapi/Backend.h" #include "renderlib/io/ConvertChannelData.h" #include "renderlib/io/FileReader.h" @@ -185,7 +186,9 @@ NB_MODULE(_native, m) .def(nb::init(), nb::arg("mode") = "pathtrace", nb::arg("asset_path") = "", - nb::arg("gpu") = 0) + // Auto-selects by default; an explicit index is used as given and + // validated rather than silently replaced. + nb::arg("gpu") = gfxApi::kAutoSelectGpu) .def("execute", &execute) .def("load_array", &loadArray, diff --git a/docs/agave.rst b/docs/agave.rst index 94bbf1f85..933adda41 100644 --- a/docs/agave.rst +++ b/docs/agave.rst @@ -798,11 +798,17 @@ AGAVE supports the following command line options: ``--list_devices`` - Only valid in server mode on Linux. AGAVE will dump a list of possible GPU devices and then exit. + AGAVE will dump a list of possible GPU devices and then exit. The indices reported here are what ``--gpu`` refers to. With the OpenGL backend this is only valid in server mode on Linux. ``--gpu number`` - Only valid in server mode on Linux. Selects a device to use from the list provided by list_devices. The device is specified as a zero-based index into the list. + Selects a device to use from the list provided by list_devices. The device is specified as a zero-based index into the list. If this option is omitted, AGAVE automatically selects the best compatible device. + + When a device is named explicitly, AGAVE uses exactly that device or fails with an error; it never falls back to a different one. In windowed mode the named device must also be able to present to the application window, which is not true of every GPU on multi-GPU Linux machines. If it cannot, try another index, ``--graphics_backend opengl``, or ``QT_QPA_PLATFORM=xcb``. + +``--graphics_backend name`` + + Selects the rendering backend: ``vulkan`` (default where available) or ``opengl``. ``-platform offscreen`` diff --git a/renderlib/PythonRenderer.h b/renderlib/PythonRenderer.h index f926d6eae..d1fb91905 100644 --- a/renderlib/PythonRenderer.h +++ b/renderlib/PythonRenderer.h @@ -49,6 +49,8 @@ class PythonRendererValueError : public std::runtime_error class PythonRenderer final : public RendererCommandInterface { public: + // selectedGpu is a zero-based physical device index; gfxApi::kAutoSelectGpu + // (-1) lets the backend pick the best graphics-capable device. PythonRenderer(const std::string& mode, const std::string& assetPath, int selectedGpu); ~PythonRenderer(); diff --git a/renderlib/gfxOpenGL/Backend.cpp b/renderlib/gfxOpenGL/Backend.cpp index efa2003b9..f25d7ffa8 100644 --- a/renderlib/gfxOpenGL/Backend.cpp +++ b/renderlib/gfxOpenGL/Backend.cpp @@ -161,6 +161,10 @@ initEGLDisplay(int selectedGpu) } #endif } + if (selectedGpu == gfxApi::kAutoSelectGpu) { + // No specific GPU was requested; let EGL pick. + return getEGLDefaultDisplay(); + } if (selectedGpu >= numberDevices || selectedGpu < 0) { LOG_WARNING << "Invalid GPU " << selectedGpu << " requested. Using default gpu."; return getEGLDefaultDisplay(); diff --git a/renderlib/gfxVulkan/Backend.cpp b/renderlib/gfxVulkan/Backend.cpp index 42ea03115..ab7b7c477 100644 --- a/renderlib/gfxVulkan/Backend.cpp +++ b/renderlib/gfxVulkan/Backend.cpp @@ -3,6 +3,7 @@ #include "Framebuffer.h" #include "GestureRenderer.h" #include "Logging.h" +#include "NativeSurface.h" #include "RenderVk.h" #include "RenderVkPT.h" #include "RendererVkContext.h" @@ -10,6 +11,7 @@ #include #include #include +#include #include namespace gfxvulkan { @@ -144,16 +146,235 @@ scorePhysicalDevice(VkPhysicalDevice physicalDevice) return score; } +std::string +physicalDeviceName(VkPhysicalDevice physicalDevice) +{ + VkPhysicalDeviceProperties properties = {}; + vkGetPhysicalDeviceProperties(physicalDevice, &properties); + return properties.deviceName; +} + +std::string +apiVersionToString(uint32_t version) +{ + std::ostringstream ss; + ss << VK_API_VERSION_MAJOR(version) << "." << VK_API_VERSION_MINOR(version) << "." << VK_API_VERSION_PATCH(version); + return ss.str(); +} + +// Fallback for devices that cannot report VkPhysicalDeviceDriverProperties. +// The spec mandates no encoding for driverVersion: each vendor packs it +// differently, so decoding it like an API version prints nonsense for the two +// exceptions below. Everyone else does follow the API version layout. +std::string +driverVersionToString(uint32_t driverVersion, uint32_t vendorID) +{ + std::ostringstream ss; + + if (vendorID == 0x10de) { // NVIDIA: 10 | 8 | 8 | 6 bits + ss << ((driverVersion >> 22) & 0x3ff) << "." << ((driverVersion >> 14) & 0x0ff) << "." + << ((driverVersion >> 6) & 0x0ff) << "." << (driverVersion & 0x03f); + return ss.str(); + } +#if defined(_WIN32) + if (vendorID == 0x8086) { // Intel, Windows driver only: 18 | 14 bits + ss << (driverVersion >> 14) << "." << (driverVersion & 0x3fff); + return ss.str(); + } +#endif + + return apiVersionToString(driverVersion); +} + +// VkPhysicalDeviceDriverProperties carries the driver's own name and version +// string -- what the vendor actually publishes -- instead of the packed +// driverVersion integer whose layout the spec leaves undefined. Core since +// Vulkan 1.2, and reachable on 1.1 devices via VK_KHR_driver_properties. +bool +queryDriverProperties(VkPhysicalDevice physicalDevice, + const VkPhysicalDeviceProperties& properties, + VkPhysicalDeviceDriverProperties& driverProperties) +{ + // vkGetPhysicalDeviceProperties2 is only core from Vulkan 1.1. + if (properties.apiVersion < VK_API_VERSION_1_1) { + return false; + } + if (properties.apiVersion < VK_API_VERSION_1_2 && + !containsName(availableDeviceExtensions(physicalDevice), VK_KHR_DRIVER_PROPERTIES_EXTENSION_NAME)) { + return false; + } + + driverProperties = {}; + driverProperties.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_DRIVER_PROPERTIES; + + VkPhysicalDeviceProperties2 properties2 = {}; + properties2.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROPERTIES_2; + properties2.pNext = &driverProperties; + vkGetPhysicalDeviceProperties2(physicalDevice, &properties2); + return true; +} + +// Prefer the driver-reported version string; fall back to decoding the packed +// driverVersion when the device cannot report driver properties. +std::string +driverDescription(VkPhysicalDevice physicalDevice, const VkPhysicalDeviceProperties& properties) +{ + VkPhysicalDeviceDriverProperties driverProperties = {}; + if (queryDriverProperties(physicalDevice, properties, driverProperties)) { + const std::string driverInfo = driverProperties.driverInfo; + const std::string driverName = driverProperties.driverName; + if (!driverInfo.empty()) { + return driverName.empty() ? driverInfo : driverInfo + " (" + driverName + ")"; + } + if (!driverName.empty()) { + return driverVersionToString(properties.driverVersion, properties.vendorID) + " (" + driverName + ")"; + } + } + + return driverVersionToString(properties.driverVersion, properties.vendorID); +} + +const char* +deviceTypeToString(VkPhysicalDeviceType deviceType) +{ + switch (deviceType) { + case VK_PHYSICAL_DEVICE_TYPE_INTEGRATED_GPU: + return "integrated GPU"; + case VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU: + return "discrete GPU"; + case VK_PHYSICAL_DEVICE_TYPE_VIRTUAL_GPU: + return "virtual GPU"; + case VK_PHYSICAL_DEVICE_TYPE_CPU: + return "CPU"; + case VK_PHYSICAL_DEVICE_TYPE_OTHER: + default: + return "other"; + } +} + +// What a physical device can do, evaluated once per device and then reused for +// both the startup log and the selection decision. +struct DeviceCapabilities +{ + // First queue family with VK_QUEUE_GRAPHICS_BIT. + uint32_t graphicsQueueFamilyIndex = UINT32_MAX; + // First queue family with both VK_QUEUE_GRAPHICS_BIT and presentation + // support for the surface that was queried. UINT32_MAX when no surface was + // supplied (headless) or when no single family can do both. + uint32_t graphicsPresentQueueFamilyIndex = UINT32_MAX; + bool hasSwapchainExtension = false; +}; + +// Presentation support is a property of a (device, queue family, surface) +// triple, so this must be given the surface the window will actually present +// to. Pass VK_NULL_HANDLE to skip the presentation queries entirely. +DeviceCapabilities +inspectPhysicalDevice(VkPhysicalDevice physicalDevice, VkSurfaceKHR presentationSurface) +{ + DeviceCapabilities capabilities; + capabilities.hasSwapchainExtension = + containsName(availableDeviceExtensions(physicalDevice), VK_KHR_SWAPCHAIN_EXTENSION_NAME); + + uint32_t queueFamilyCount = 0; + vkGetPhysicalDeviceQueueFamilyProperties(physicalDevice, &queueFamilyCount, nullptr); + std::vector queueFamilies(queueFamilyCount); + vkGetPhysicalDeviceQueueFamilyProperties(physicalDevice, &queueFamilyCount, queueFamilies.data()); + + for (uint32_t i = 0; i < queueFamilyCount; ++i) { + if ((queueFamilies[i].queueFlags & VK_QUEUE_GRAPHICS_BIT) == 0) { + continue; + } + if (capabilities.graphicsQueueFamilyIndex == UINT32_MAX) { + capabilities.graphicsQueueFamilyIndex = i; + } + + if (presentationSurface == VK_NULL_HANDLE || capabilities.graphicsPresentQueueFamilyIndex != UINT32_MAX) { + continue; + } + + VkBool32 supported = VK_FALSE; + VkResult result = vkGetPhysicalDeviceSurfaceSupportKHR(physicalDevice, i, presentationSurface, &supported); + if (result != VK_SUCCESS) { + LOG_WARNING << "vkGetPhysicalDeviceSurfaceSupportKHR failed for " << physicalDeviceName(physicalDevice) + << " queue family " << i << " with VkResult " << result; + continue; + } + if (supported == VK_TRUE) { + capabilities.graphicsPresentQueueFamilyIndex = i; + } + } + + return capabilities; +} + +// Requirements differ by mode: headless only needs a graphics queue, while +// windowed additionally needs to be able to present to the window's surface +// and to create a swapchain. On failure, reason describes what is missing. +bool +isDeviceCompatible(const DeviceCapabilities& capabilities, bool requiresPresent, std::string& reason) +{ + if (capabilities.graphicsQueueFamilyIndex == UINT32_MAX) { + reason = "has no graphics-capable queue family"; + return false; + } + if (!requiresPresent) { + return true; + } + if (!capabilities.hasSwapchainExtension) { + reason = std::string("does not support ") + VK_KHR_SWAPCHAIN_EXTENSION_NAME; + return false; + } + if (capabilities.graphicsPresentQueueFamilyIndex == UINT32_MAX) { + // AGAVE renders and presents on one queue, so a device that can present + // only from a compute/transfer-style family is rejected here rather than + // handled with a separate present queue and image ownership transfers. + reason = "has no queue family that supports both graphics and presentation to this window surface"; + return false; + } + return true; +} + +// Human-readable capability summary for the device list logged at startup. +std::string +describeCapabilities(const DeviceCapabilities& capabilities, bool requiresPresent) +{ + std::string description = capabilities.graphicsQueueFamilyIndex == UINT32_MAX ? "no graphics queue" : "graphics"; + if (requiresPresent) { + description += capabilities.graphicsPresentQueueFamilyIndex == UINT32_MAX ? ", cannot present to this window" + : ", can present to this window"; + if (!capabilities.hasSwapchainExtension) { + description += ", no swapchain extension"; + } + } + return description; +} + +// Extra guidance for the windowed case, where "no device can present" usually +// means a platform/driver mismatch rather than missing hardware. +void +logWindowedSelectionHint() +{ +#if defined(_WIN32) || defined(__APPLE__) + LOG_ERROR << "Try a different --gpu index (see --list_devices) or --graphics_backend opengl."; +#else + LOG_ERROR << "Try a different --gpu index (see --list_devices), --graphics_backend opengl, or " + "QT_QPA_PLATFORM=xcb."; +#endif +} + } // namespace +// Construction only brings up the VkInstance. Device selection is a separate, +// explicit step because it depends on something the constructor cannot know: +// which physical device and queue family are usable is a property of the +// surface that has to be presentable. Windowed callers therefore wait for +// their window and call initDeviceForWindow(); headless callers have no +// surface to wait for and call initDeviceHeadless() immediately; device +// enumeration (--list_devices) needs the instance and nothing more. Backend::Backend(const gfxApi::InitParams& params) : m_params(params) { - m_valid = createInstance() && setupDebugMessenger() && pickPhysicalDevice() && createLogicalDevice(); - if (m_valid) { - m_device.initialize(m_physicalDevice, m_deviceHandle); - m_valid = createCommandPool(); - } + m_valid = createInstance() && setupDebugMessenger(); } Backend::~Backend() @@ -164,6 +385,10 @@ Backend::~Backend() std::unique_ptr Backend::createGestureRenderer() { + if (!m_deviceReady) { + LOG_ERROR << "Cannot create a Vulkan gesture renderer before device initialization"; + return nullptr; + } return std::make_unique(); } @@ -171,12 +396,20 @@ std::unique_ptr Backend::createRendererContext(gfxApi::IGLContext* externalContext) { (void)externalContext; + if (!m_deviceReady) { + LOG_ERROR << "Cannot create a Vulkan renderer context before device initialization"; + return nullptr; + } return std::make_unique(*this); } std::unique_ptr Backend::createRenderWindow(gfxApi::RenderWindowKind kind, RenderSettings* renderSettings) { + if (!m_deviceReady) { + LOG_ERROR << "Cannot create a Vulkan render window before device initialization"; + return nullptr; + } switch (kind) { case gfxApi::RenderWindowKind::RaymarchBlended: return std::make_unique(*this, renderSettings); @@ -189,6 +422,10 @@ Backend::createRenderWindow(gfxApi::RenderWindowKind kind, RenderSettings* rende std::unique_ptr Backend::createFramebuffer(const gfxApi::FramebufferDesc& desc) { + if (!m_deviceReady) { + LOG_ERROR << "Cannot create a Vulkan framebuffer before device initialization"; + return nullptr; + } return std::make_unique(*this, desc); } @@ -200,6 +437,62 @@ Backend::clearCurrentFramebuffer(const gfxApi::ClearColor& color) // active swapchain image inside a command buffer. } +bool +Backend::initDeviceHeadless() +{ + if (m_deviceReady) { + return true; + } + if (!m_valid) { + LOG_ERROR << "Cannot initialize a Vulkan device from an invalid backend"; + return false; + } + if (!m_params.headless) { + LOG_ERROR << "initDeviceHeadless called on a windowed Vulkan backend; use initDeviceForWindow so the device is " + "chosen against the window surface"; + return false; + } + + return initializeDevice(VK_NULL_HANDLE); +} + +bool +Backend::initDeviceForWindow(gfxApi::IWindowSurface* surface) +{ + if (m_deviceReady) { + return true; + } + if (!m_valid) { + LOG_ERROR << "Cannot initialize a Vulkan window device from an invalid backend"; + return false; + } + // A headless backend has no window to select against; the surface, if any, + // is irrelevant. This is the only case where a missing surface is benign. + if (m_params.headless) { + return initDeviceHeadless(); + } + // A null surface must never be read as "headless". In windowed mode it means + // the caller created renderers before the window existed, which is exactly + // the ordering bug this hook is here to catch. + if (!surface) { + LOG_ERROR << "Windowed Vulkan initialization requires a native window surface"; + return false; + } + + // This surface exists only to answer "which device can present here?". The + // Swapchain creates and owns its own VkSurfaceKHR for the same window, since + // it has to be able to drop and rebuild it across resizes. + VkSurfaceKHR presentationSurface = createNativeWindowSurface(m_instance, surface); + if (presentationSurface == VK_NULL_HANDLE) { + LOG_ERROR << "Unable to create a Vulkan surface for device selection"; + return false; + } + + const bool initialized = initializeDevice(presentationSurface); + vkDestroySurfaceKHR(m_instance, presentationSurface, nullptr); + return initialized; +} + bool Backend::createInstance() { @@ -296,7 +589,35 @@ Backend::setupDebugMessenger() } bool -Backend::pickPhysicalDevice() +Backend::initializeDevice(VkSurfaceKHR presentationSurface) +{ + if (m_deviceReady) { + return true; + } + if (!m_params.headless && presentationSurface == VK_NULL_HANDLE) { + LOG_ERROR << "Windowed Vulkan device selection requires a presentation surface"; + return false; + } + + if (!pickPhysicalDevice(presentationSurface)) { + return false; + } + if (!createLogicalDevice()) { + destroyDevice(); + return false; + } + m_device.initialize(m_physicalDevice, m_deviceHandle); + if (!createCommandPool()) { + destroyDevice(); + return false; + } + + m_deviceReady = true; + return true; +} + +bool +Backend::pickPhysicalDevice(VkSurfaceKHR presentationSurface) { uint32_t deviceCount = 0; vkEnumeratePhysicalDevices(m_instance, &deviceCount, nullptr); @@ -308,63 +629,111 @@ Backend::pickPhysicalDevice() std::vector devices(deviceCount); vkEnumeratePhysicalDevices(m_instance, &deviceCount, devices.data()); + // Presentation is only required when this backend is going to drive a window, + // regardless of whether the device was chosen explicitly or automatically. + const bool requiresPresent = !m_params.headless; + const int requestedGpu = m_params.selectedGpu; + + std::vector capabilities; + capabilities.reserve(deviceCount); for (uint32_t i = 0; i < deviceCount; ++i) { - VkPhysicalDeviceProperties properties = {}; - vkGetPhysicalDeviceProperties(devices[i], &properties); - LOG_INFO << "Vulkan device " << i << ": " << properties.deviceName; + capabilities.push_back(inspectPhysicalDevice(devices[i], requiresPresent ? presentationSurface : VK_NULL_HANDLE)); } - if (m_params.selectedGpu >= 0 && static_cast(m_params.selectedGpu) < deviceCount) { - m_physicalDevice = devices[static_cast(m_params.selectedGpu)]; - } else { - m_physicalDevice = *std::max_element(devices.begin(), devices.end(), [](VkPhysicalDevice a, VkPhysicalDevice b) { - return scorePhysicalDevice(a) < scorePhysicalDevice(b); - }); + // These indices are what --gpu N refers to, so log the raw enumeration order. + for (uint32_t i = 0; i < deviceCount; ++i) { + LOG_INFO << "Vulkan device " << i << ": " << physicalDeviceName(devices[i]) << " (" + << describeCapabilities(capabilities[i], requiresPresent) << ")"; } - uint32_t queueFamilyCount = 0; - vkGetPhysicalDeviceQueueFamilyProperties(m_physicalDevice, &queueFamilyCount, nullptr); - std::vector queueFamilies(queueFamilyCount); - vkGetPhysicalDeviceQueueFamilyProperties(m_physicalDevice, &queueFamilyCount, queueFamilies.data()); + uint32_t chosenIndex = UINT32_MAX; - for (uint32_t i = 0; i < queueFamilyCount; ++i) { - if (queueFamilies[i].queueFlags & VK_QUEUE_GRAPHICS_BIT) { - m_graphicsQueueFamilyIndex = i; - break; + if (requestedGpu >= 0) { + // Explicit selection: use exactly this device, but validate it against the + // mode's requirements and fail loudly instead of falling back to another. + if (static_cast(requestedGpu) >= deviceCount) { + LOG_ERROR << "Requested Vulkan device " << requestedGpu << " does not exist; " << deviceCount + << " device(s) are available. Run with --list_devices to see the valid indices."; + return false; } - } - if (m_graphicsQueueFamilyIndex == UINT32_MAX) { - LOG_ERROR << "Selected Vulkan physical device has no graphics queue"; - return false; + std::string reason; + if (!isDeviceCompatible(capabilities[requestedGpu], requiresPresent, reason)) { + LOG_ERROR << "Requested Vulkan device " << requestedGpu << " (" << physicalDeviceName(devices[requestedGpu]) + << ") " << reason << "."; + if (requiresPresent) { + logWindowedSelectionHint(); + } + return false; + } + chosenIndex = static_cast(requestedGpu); + } else { + // Auto-selection: skip incompatible devices, take the highest scoring one + // that remains. + int bestScore = 0; + for (uint32_t i = 0; i < deviceCount; ++i) { + std::string reason; + if (!isDeviceCompatible(capabilities[i], requiresPresent, reason)) { + LOG_INFO << "Skipping Vulkan device " << i << " (" << physicalDeviceName(devices[i]) << "): " << reason; + continue; + } + const int score = scorePhysicalDevice(devices[i]); + if (chosenIndex == UINT32_MAX || score > bestScore) { + chosenIndex = i; + bestScore = score; + } + } + + if (chosenIndex == UINT32_MAX) { + LOG_ERROR << "No Vulkan device is compatible with " << (requiresPresent ? "windowed" : "headless") + << " rendering."; + if (requiresPresent) { + logWindowedSelectionHint(); + } + return false; + } } - VkPhysicalDeviceProperties properties = {}; - vkGetPhysicalDeviceProperties(m_physicalDevice, &properties); - LOG_INFO << "Selected Vulkan device: " << properties.deviceName; + m_physicalDevice = devices[chosenIndex]; + m_graphicsQueueFamilyIndex = requiresPresent ? capabilities[chosenIndex].graphicsPresentQueueFamilyIndex + : capabilities[chosenIndex].graphicsQueueFamilyIndex; + + LOG_INFO << "Selected Vulkan device " << chosenIndex << ": " << physicalDeviceName(m_physicalDevice) + << " (queue family " << m_graphicsQueueFamilyIndex + << (requiresPresent ? ", graphics+present)" : ", graphics)"); return true; } -std::vector -Backend::enabledDeviceExtensions(VkPhysicalDevice physicalDevice) const +bool +Backend::enabledDeviceExtensions(VkPhysicalDevice physicalDevice, std::vector& extensions) const { const std::vector availableExtensions = availableDeviceExtensions(physicalDevice); - std::vector enabledExtensions; + extensions.clear(); - if (containsName(availableExtensions, VK_KHR_SWAPCHAIN_EXTENSION_NAME)) { - enabledExtensions.push_back(VK_KHR_SWAPCHAIN_EXTENSION_NAME); + if (!m_params.headless) { + if (!containsName(availableExtensions, VK_KHR_SWAPCHAIN_EXTENSION_NAME)) { + LOG_ERROR << "Windowed Vulkan device " << physicalDeviceName(physicalDevice) << " is missing " + << VK_KHR_SWAPCHAIN_EXTENSION_NAME; + return false; + } + extensions.push_back(VK_KHR_SWAPCHAIN_EXTENSION_NAME); } if (containsName(availableExtensions, kPortabilitySubsetExtension)) { - enabledExtensions.push_back(kPortabilitySubsetExtension); + extensions.push_back(kPortabilitySubsetExtension); } - return enabledExtensions; + return true; } bool Backend::createLogicalDevice() { + if (m_graphicsQueueFamilyIndex == UINT32_MAX) { + LOG_ERROR << "Cannot create a Vulkan logical device without a selected queue family"; + return false; + } + const float queuePriority = 1.0f; VkDeviceQueueCreateInfo queueCreateInfo = {}; queueCreateInfo.sType = VK_STRUCTURE_TYPE_DEVICE_QUEUE_CREATE_INFO; @@ -373,7 +742,10 @@ Backend::createLogicalDevice() queueCreateInfo.pQueuePriorities = &queuePriority; VkPhysicalDeviceFeatures deviceFeatures = {}; - const std::vector deviceExtensions = enabledDeviceExtensions(m_physicalDevice); + std::vector deviceExtensions; + if (!enabledDeviceExtensions(m_physicalDevice, deviceExtensions)) { + return false; + } VkDeviceCreateInfo createInfo = {}; createInfo.sType = VK_STRUCTURE_TYPE_DEVICE_CREATE_INFO; @@ -410,7 +782,7 @@ Backend::createCommandPool() } void -Backend::destroy() +Backend::destroyDevice() { if (m_deviceHandle != VK_NULL_HANDLE) { vkDeviceWaitIdle(m_deviceHandle); @@ -426,6 +798,17 @@ Backend::destroy() m_deviceHandle = VK_NULL_HANDLE; } + m_physicalDevice = VK_NULL_HANDLE; + m_graphicsQueue = VK_NULL_HANDLE; + m_graphicsQueueFamilyIndex = UINT32_MAX; + m_deviceReady = false; +} + +void +Backend::destroy() +{ + destroyDevice(); + if (m_debugMessenger != VK_NULL_HANDLE && m_instance != VK_NULL_HANDLE) { auto destroyDebugUtilsMessenger = reinterpret_cast( vkGetInstanceProcAddr(m_instance, "vkDestroyDebugUtilsMessengerEXT")); @@ -521,10 +904,48 @@ Backend::endSingleTimeCommands(VkCommandBuffer commandBuffer) const void Backend::listDevices(int selectedGpu) { + // No window exists yet, so build an instance without the windowing-system + // surface extensions. Construction stops at the instance, which is all + // enumeration needs. gfxApi::InitParams params; + params.headless = true; params.selectedGpu = selectedGpu; Backend backend(params); - (void)backend; + if (!backend.isValid()) { + LOG_ERROR << "Unable to create a Vulkan instance to enumerate devices"; + return; + } + + uint32_t deviceCount = 0; + vkEnumeratePhysicalDevices(backend.m_instance, &deviceCount, nullptr); + if (deviceCount == 0) { + LOG_INFO << "No Vulkan physical devices are available"; + return; + } + + std::vector devices(deviceCount); + vkEnumeratePhysicalDevices(backend.m_instance, &deviceCount, devices.data()); + + LOG_INFO << deviceCount << " Vulkan device(s) found. These indices are what --gpu N selects."; + for (uint32_t i = 0; i < deviceCount; ++i) { + VkPhysicalDeviceProperties properties = {}; + vkGetPhysicalDeviceProperties(devices[i], &properties); + // Query without a surface: presentation support depends on the actual + // window surface, which does not exist during device listing. + const DeviceCapabilities capabilities = inspectPhysicalDevice(devices[i], VK_NULL_HANDLE); + + LOG_INFO << "Vulkan device " << i << ": " << properties.deviceName; + LOG_INFO << " API version: " << apiVersionToString(properties.apiVersion); + LOG_INFO << " Driver version: " << driverDescription(devices[i], properties); + LOG_INFO << " Device type: " << deviceTypeToString(properties.deviceType); + LOG_INFO << " Capabilities: " + << (capabilities.graphicsQueueFamilyIndex == UINT32_MAX ? "no graphics queue" : "graphics") + << (capabilities.hasSwapchainExtension ? ", swapchain" : ", no swapchain"); + } + + if (selectedGpu >= 0 && static_cast(selectedGpu) >= deviceCount) { + LOG_WARNING << "--gpu " << selectedGpu << " is out of range for the " << deviceCount << " device(s) listed above"; + } } } // namespace gfxvulkan diff --git a/renderlib/gfxVulkan/Backend.h b/renderlib/gfxVulkan/Backend.h index eee9f4fcb..028911095 100644 --- a/renderlib/gfxVulkan/Backend.h +++ b/renderlib/gfxVulkan/Backend.h @@ -29,7 +29,15 @@ class Backend : public gfxApi::Backend void clearCurrentFramebuffer(const gfxApi::ClearColor& color) override; bool isHeadless() const override { return m_params.headless; } gfxApi::BackendKind kind() const override { return gfxApi::BackendKind::Vulkan; } + bool initDeviceForWindow(gfxApi::IWindowSurface* surface = nullptr) override; + bool isDeviceReady() const override { return m_deviceReady; } + // Headless counterpart to initDeviceForWindow: there is no surface to wait + // for, so the owner brings the device up right after construction. + bool initDeviceHeadless(); + + // True once the VkInstance exists. A valid backend still has no device until + // initDeviceHeadless() or initDeviceForWindow() succeeds. bool isValid() const { return m_valid; } VkInstance instance() const { return m_instance; } @@ -48,13 +56,19 @@ class Backend : public gfxApi::Backend private: bool createInstance(); bool setupDebugMessenger(); - bool pickPhysicalDevice(); + // Select a physical device and bring up the logical device, queues, and + // command pool. presentationSurface is VK_NULL_HANDLE for headless and must + // be a real surface when windowed. + bool initializeDevice(VkSurfaceKHR presentationSurface); + bool pickPhysicalDevice(VkSurfaceKHR presentationSurface); bool createLogicalDevice(); bool createCommandPool(); + // Tear down everything created by initializeDevice, leaving the instance. + void destroyDevice(); void destroy(); std::vector enabledInstanceExtensions() const; - std::vector enabledDeviceExtensions(VkPhysicalDevice physicalDevice) const; + bool enabledDeviceExtensions(VkPhysicalDevice physicalDevice, std::vector& extensions) const; gfxApi::InitParams m_params; Device m_device; @@ -67,6 +81,7 @@ class Backend : public gfxApi::Backend resources::UniqueCommandPool m_commandPool; uint32_t m_graphicsQueueFamilyIndex = UINT32_MAX; bool m_valid = false; + bool m_deviceReady = false; }; } // namespace gfxvulkan diff --git a/renderlib/gfxVulkan/CMakeLists.txt b/renderlib/gfxVulkan/CMakeLists.txt index 02d3f95c2..ff2a874ad 100644 --- a/renderlib/gfxVulkan/CMakeLists.txt +++ b/renderlib/gfxVulkan/CMakeLists.txt @@ -12,6 +12,7 @@ target_sources(renderlib PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}/Framebuffer.h" "${CMAKE_CURRENT_SOURCE_DIR}/GestureRenderer.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/GestureRenderer.h" + "${CMAKE_CURRENT_SOURCE_DIR}/NativeSurface.h" "${CMAKE_CURRENT_SOURCE_DIR}/RenderVk.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/RenderVk.h" "${CMAKE_CURRENT_SOURCE_DIR}/RenderVkPT.cpp" diff --git a/renderlib/gfxVulkan/NativeSurface.h b/renderlib/gfxVulkan/NativeSurface.h new file mode 100644 index 000000000..765856a49 --- /dev/null +++ b/renderlib/gfxVulkan/NativeSurface.h @@ -0,0 +1,19 @@ +#pragma once + +#if AGAVE_HAS_VULKAN + +#include "gfxapi/WindowSurface.h" + +#include + +namespace gfxvulkan { + +// Create a backend-owned VkSurfaceKHR from application-supplied native window +// details. The caller owns and must destroy the returned surface with +// vkDestroySurfaceKHR(instance, surface, nullptr). +VkSurfaceKHR +createNativeWindowSurface(VkInstance instance, gfxApi::IWindowSurface* surface); + +} // namespace gfxvulkan + +#endif // AGAVE_HAS_VULKAN diff --git a/renderlib/gfxVulkan/Swapchain.cpp b/renderlib/gfxVulkan/Swapchain.cpp index ee4a3aced..d990dda87 100644 --- a/renderlib/gfxVulkan/Swapchain.cpp +++ b/renderlib/gfxVulkan/Swapchain.cpp @@ -24,7 +24,7 @@ isResizeResult(VkResult result) } // namespace -Swapchain::Swapchain(ISwapchainSurface* surface) +Swapchain::Swapchain(gfxApi::IWindowSurface* surface) : m_surface(surface) { gfxApi::Backend* backend = renderlib::graphicsBackend(); diff --git a/renderlib/gfxVulkan/Swapchain.h b/renderlib/gfxVulkan/Swapchain.h index 8c944b902..bc0162235 100644 --- a/renderlib/gfxVulkan/Swapchain.h +++ b/renderlib/gfxVulkan/Swapchain.h @@ -4,6 +4,7 @@ #include "Backend.h" #include "Framebuffer.h" +#include "gfxapi/WindowSurface.h" #include @@ -15,46 +16,13 @@ class ViewerWindow; namespace gfxvulkan { -// Provides the platform-native window handle and live surface geometry that the -// Vulkan swapchain needs. Implemented by the application/windowing layer (for -// example a Qt QWindow wrapper in agave_app) so that gfxVulkan stays free of any -// windowing-toolkit dependency. -class ISwapchainSurface -{ -public: - virtual ~ISwapchainSurface() = default; - - // Platform-native window handle used to create the VkSurfaceKHR. - // macOS: NSView* - // Windows: HWND - // X11: xcb_window_t (as a pointer-sized value) - virtual void* nativeHandle() const = 0; - - // Platform-native display / connection handle. Optional: platforms that - // don't need one (macOS, Windows) can leave the default nullptr. Providing - // the same connection the rest of the app uses is preferred on Linux, since - // some Vulkan drivers keep per-connection state for presentation. - // X11: xcb_connection_t* - // Wayland: wl_display* - virtual void* nativeDisplay() const { return nullptr; } - - // True when the surface is visible and can be rendered to. - virtual bool isExposed() const = 0; - - // Size of the surface in physical pixels (logical size times content scale). - virtual void pixelSize(uint32_t& width, uint32_t& height) const = 0; - - // Ratio of physical pixels to logical points (e.g. 2.0 on a Retina display). - virtual double contentScale() const = 0; -}; - // Vulkan swapchain bound to a native window surface. All Vulkan and platform // surface code lives here; the only window-system dependency is the abstract -// ISwapchainSurface supplied by the caller. +// IWindowSurface supplied by the caller. class Swapchain { public: - explicit Swapchain(ISwapchainSurface* surface); + explicit Swapchain(gfxApi::IWindowSurface* surface); ~Swapchain(); bool render(ViewerWindow& viewerWindow); @@ -78,7 +46,7 @@ class Swapchain VkPresentModeKHR choosePresentMode(const std::vector& presentModes) const; VkCompositeAlphaFlagBitsKHR chooseCompositeAlpha(VkCompositeAlphaFlagsKHR supportedCompositeAlpha) const; - ISwapchainSurface* m_surface = nullptr; + gfxApi::IWindowSurface* m_surface = nullptr; Backend* m_backend = nullptr; VkSurfaceKHR m_vkSurface = VK_NULL_HANDLE; diff --git a/renderlib/gfxVulkan/Swapchain_linux.cpp b/renderlib/gfxVulkan/Swapchain_linux.cpp index 0eb60904d..362a177b4 100644 --- a/renderlib/gfxVulkan/Swapchain_linux.cpp +++ b/renderlib/gfxVulkan/Swapchain_linux.cpp @@ -1,6 +1,7 @@ #define VK_USE_PLATFORM_XCB_KHR #define VK_USE_PLATFORM_XLIB_KHR +#include "NativeSurface.h" #include "Swapchain.h" #if AGAVE_HAS_VULKAN && !defined(__APPLE__) && !defined(_WIN32) @@ -16,7 +17,7 @@ namespace gfxvulkan { namespace { -// Fallback xcb connection used only when the ISwapchainSurface didn't hand us +// Fallback xcb connection used only when the IWindowSurface didn't hand us // one from the windowing toolkit. Sharing a single process-wide connection is // fine because it's only used for VkSurfaceKHR presentation; the OS reclaims // it on exit. @@ -39,7 +40,7 @@ fallbackXcbConnection() } // Fallback xlib display used only when the xcb surface extension isn't -// available and the ISwapchainSurface didn't hand us its own display. Held +// available and the IWindowSurface didn't hand us its own display. Held // for the process lifetime for the same reason as the xcb fallback. Display* fallbackXlibDisplay() @@ -56,10 +57,10 @@ fallbackXlibDisplay() } bool -tryCreateXcbSurface(Backend* backend, ISwapchainSurface* surface, xcb_window_t window, VkSurfaceKHR& outSurface) +tryCreateXcbSurface(VkInstance instance, gfxApi::IWindowSurface* surface, xcb_window_t window, VkSurfaceKHR& outSurface) { auto createXcbSurface = - reinterpret_cast(vkGetInstanceProcAddr(backend->instance(), "vkCreateXcbSurfaceKHR")); + reinterpret_cast(vkGetInstanceProcAddr(instance, "vkCreateXcbSurfaceKHR")); if (!createXcbSurface) { return false; } @@ -82,7 +83,7 @@ tryCreateXcbSurface(Backend* backend, ISwapchainSurface* surface, xcb_window_t w createInfo.connection = connection; createInfo.window = window; - VkResult result = createXcbSurface(backend->instance(), &createInfo, nullptr, &outSurface); + VkResult result = createXcbSurface(instance, &createInfo, nullptr, &outSurface); if (result != VK_SUCCESS) { LOG_ERROR << "vkCreateXcbSurfaceKHR failed with VkResult " << result; outSurface = VK_NULL_HANDLE; @@ -92,10 +93,10 @@ tryCreateXcbSurface(Backend* backend, ISwapchainSurface* surface, xcb_window_t w } bool -tryCreateXlibSurface(Backend* backend, Window window, VkSurfaceKHR& outSurface) +tryCreateXlibSurface(VkInstance instance, Window window, VkSurfaceKHR& outSurface) { auto createXlibSurface = - reinterpret_cast(vkGetInstanceProcAddr(backend->instance(), "vkCreateXlibSurfaceKHR")); + reinterpret_cast(vkGetInstanceProcAddr(instance, "vkCreateXlibSurfaceKHR")); if (!createXlibSurface) { return false; } @@ -110,7 +111,7 @@ tryCreateXlibSurface(Backend* backend, Window window, VkSurfaceKHR& outSurface) createInfo.dpy = display; createInfo.window = window; - VkResult result = createXlibSurface(backend->instance(), &createInfo, nullptr, &outSurface); + VkResult result = createXlibSurface(instance, &createInfo, nullptr, &outSurface); if (result != VK_SUCCESS) { LOG_ERROR << "vkCreateXlibSurfaceKHR failed with VkResult " << result; outSurface = VK_NULL_HANDLE; @@ -124,31 +125,43 @@ tryCreateXlibSurface(Backend* backend, Window window, VkSurfaceKHR& outSurface) bool Swapchain::createNativeSurface() { - if (!m_backend || !m_surface) { + if (!m_backend) { return false; } + m_vkSurface = createNativeWindowSurface(m_backend->instance(), m_surface); + return m_vkSurface != VK_NULL_HANDLE; +} + +VkSurfaceKHR +createNativeWindowSurface(VkInstance instance, gfxApi::IWindowSurface* surface) +{ + if (instance == VK_NULL_HANDLE || !surface) { + return VK_NULL_HANDLE; + } + // Qt's QWidget::winId() returns the X11 window handle; the same numeric // value is valid as both xcb_window_t and Xlib Window. - const uintptr_t windowId = reinterpret_cast(m_surface->nativeHandle()); + const uintptr_t windowId = reinterpret_cast(surface->nativeHandle()); if (windowId == 0) { LOG_ERROR << "Unable to get an X11 window ID for the Vulkan window"; - return false; + return VK_NULL_HANDLE; } // Prefer VK_KHR_xcb_surface (matches GLFW's preference: "VK_KHR_xcb_surface // is preferred due to some early ICDs exposing but not correctly // implementing VK_KHR_xlib_surface"). Fall back to VK_KHR_xlib_surface for // instances that only expose the xlib extension. - if (tryCreateXcbSurface(m_backend, m_surface, static_cast(windowId), m_vkSurface)) { - return true; + VkSurfaceKHR vkSurface = VK_NULL_HANDLE; + if (tryCreateXcbSurface(instance, surface, static_cast(windowId), vkSurface)) { + return vkSurface; } - if (tryCreateXlibSurface(m_backend, static_cast(windowId), m_vkSurface)) { - return true; + if (tryCreateXlibSurface(instance, static_cast(windowId), vkSurface)) { + return vkSurface; } LOG_ERROR << "Neither VK_KHR_xcb_surface nor VK_KHR_xlib_surface is available on the current Vulkan instance"; - return false; + return VK_NULL_HANDLE; } void diff --git a/renderlib/gfxVulkan/Swapchain_mac.mm b/renderlib/gfxVulkan/Swapchain_mac.mm index b5e0294f0..f2998d849 100644 --- a/renderlib/gfxVulkan/Swapchain_mac.mm +++ b/renderlib/gfxVulkan/Swapchain_mac.mm @@ -1,5 +1,6 @@ #define VK_USE_PLATFORM_METAL_EXT +#include "NativeSurface.h" #include "Swapchain.h" #if AGAVE_HAS_VULKAN && defined(__APPLE__) @@ -14,14 +15,25 @@ bool Swapchain::createNativeSurface() { - if (!m_backend || !m_surface) { + if (!m_backend) { return false; } - NSView* view = reinterpret_cast(m_surface->nativeHandle()); + m_vkSurface = createNativeWindowSurface(m_backend->instance(), m_surface); + return m_vkSurface != VK_NULL_HANDLE; +} + +VkSurfaceKHR +createNativeWindowSurface(VkInstance instance, gfxApi::IWindowSurface* surface) +{ + if (instance == VK_NULL_HANDLE || !surface) { + return VK_NULL_HANDLE; + } + + NSView* view = reinterpret_cast(surface->nativeHandle()); if (!view) { LOG_ERROR << "Unable to get an NSView for the Vulkan window"; - return false; + return VK_NULL_HANDLE; } // Note on Qt integration: standalone Vulkan apps (e.g. GLFW) usually own the @@ -53,30 +65,29 @@ [[view layer] addSublayer:metalLayer]; } - metalLayer.contentsScale = m_surface->contentScale(); + metalLayer.contentsScale = surface->contentScale(); metalLayer.frame = [view bounds]; auto createMetalSurface = reinterpret_cast( - vkGetInstanceProcAddr(m_backend->instance(), "vkCreateMetalSurfaceEXT")); + vkGetInstanceProcAddr(instance, "vkCreateMetalSurfaceEXT")); if (!createMetalSurface) { LOG_ERROR << "vkCreateMetalSurfaceEXT is not available on the current " "Vulkan instance"; - return false; + return VK_NULL_HANDLE; } VkMetalSurfaceCreateInfoEXT createInfo = {}; createInfo.sType = VK_STRUCTURE_TYPE_METAL_SURFACE_CREATE_INFO_EXT; createInfo.pLayer = metalLayer; - VkResult result = createMetalSurface( - m_backend->instance(), &createInfo, nullptr, &m_vkSurface); + VkSurfaceKHR vkSurface = VK_NULL_HANDLE; + VkResult result = createMetalSurface(instance, &createInfo, nullptr, &vkSurface); if (result != VK_SUCCESS) { LOG_ERROR << "vkCreateMetalSurfaceEXT failed with VkResult " << result; - m_vkSurface = VK_NULL_HANDLE; - return false; + return VK_NULL_HANDLE; } - return true; + return vkSurface; } void diff --git a/renderlib/gfxVulkan/Swapchain_win.cpp b/renderlib/gfxVulkan/Swapchain_win.cpp index 382ef4da2..83ac3169b 100644 --- a/renderlib/gfxVulkan/Swapchain_win.cpp +++ b/renderlib/gfxVulkan/Swapchain_win.cpp @@ -1,5 +1,6 @@ #define VK_USE_PLATFORM_WIN32_KHR +#include "NativeSurface.h" #include "Swapchain.h" #if AGAVE_HAS_VULKAN && defined(_WIN32) @@ -14,21 +15,32 @@ namespace gfxvulkan { bool Swapchain::createNativeSurface() { - if (!m_backend || !m_surface) { + if (!m_backend) { return false; } - HWND hwnd = reinterpret_cast(m_surface->nativeHandle()); + m_vkSurface = createNativeWindowSurface(m_backend->instance(), m_surface); + return m_vkSurface != VK_NULL_HANDLE; +} + +VkSurfaceKHR +createNativeWindowSurface(VkInstance instance, gfxApi::IWindowSurface* surface) +{ + if (instance == VK_NULL_HANDLE || !surface) { + return VK_NULL_HANDLE; + } + + HWND hwnd = reinterpret_cast(surface->nativeHandle()); if (!hwnd) { LOG_ERROR << "Unable to get an HWND for the Vulkan window"; - return false; + return VK_NULL_HANDLE; } auto createWin32Surface = reinterpret_cast( - vkGetInstanceProcAddr(m_backend->instance(), "vkCreateWin32SurfaceKHR")); + vkGetInstanceProcAddr(instance, "vkCreateWin32SurfaceKHR")); if (!createWin32Surface) { LOG_ERROR << "vkCreateWin32SurfaceKHR is not available on the current Vulkan instance"; - return false; + return VK_NULL_HANDLE; } VkWin32SurfaceCreateInfoKHR createInfo = {}; @@ -36,14 +48,14 @@ Swapchain::createNativeSurface() createInfo.hinstance = GetModuleHandle(nullptr); createInfo.hwnd = hwnd; - VkResult result = createWin32Surface(m_backend->instance(), &createInfo, nullptr, &m_vkSurface); + VkSurfaceKHR vkSurface = VK_NULL_HANDLE; + VkResult result = createWin32Surface(instance, &createInfo, nullptr, &vkSurface); if (result != VK_SUCCESS) { LOG_ERROR << "vkCreateWin32SurfaceKHR failed with VkResult " << result; - m_vkSurface = VK_NULL_HANDLE; - return false; + return VK_NULL_HANDLE; } - return true; + return vkSurface; } void diff --git a/renderlib/gfxapi/Backend.h b/renderlib/gfxapi/Backend.h index 8ea653686..dbbcd8017 100644 --- a/renderlib/gfxapi/Backend.h +++ b/renderlib/gfxapi/Backend.h @@ -4,6 +4,7 @@ #include "IGestureRenderer.h" #include "IRenderWindow.h" #include "Framebuffer.h" +#include "WindowSurface.h" #include #include @@ -16,6 +17,9 @@ namespace gfxApi { class IGLContext; +// InitParams::selectedGpu value meaning "pick the best compatible device". +constexpr int kAutoSelectGpu = -1; + // Parameters supplied to a backend at construction time. struct InitParams { @@ -30,8 +34,12 @@ struct InitParams std::string assetPath; // Run without an on-screen surface (offscreen / EGL rendering). bool headless = false; - // Index of the GPU to use when more than one is available. - int selectedGpu = 0; + // Zero-based index of the GPU to use, in the backend's own enumeration order + // (the order reported by --list_devices). kAutoSelectGpu means the backend + // picks the best device that is compatible with the requested mode; an + // explicit index disables that fallback, so an incompatible device is an + // error rather than a silent switch to another one. + int selectedGpu = kAutoSelectGpu; // Install a GL debug logger (verbose; for development). bool enableDebug = false; // Non-headless OpenGL context supplied by the application/windowing layer. @@ -82,6 +90,24 @@ class Backend // The kind of backend this is. virtual BackendKind kind() const = 0; + + // Some backends need the native window surface before they can choose a + // physical device and queue family, because the choice depends on which + // device can actually present to that surface. Call this once, after the + // window exists and before creating any renderers. + // + // A null surface is only valid for a headless backend; in windowed mode it + // is an error rather than an implicit switch to headless behavior. Backends + // where the toolkit owns presentation (OpenGL/Qt) are fully initialized by + // construction and keep this default implementation. + virtual bool initDeviceForWindow(IWindowSurface* surface = nullptr) + { + (void)surface; + return true; + } + + // True once device-backed render resources can be created. + virtual bool isDeviceReady() const { return true; } }; } // namespace gfxApi diff --git a/renderlib/gfxapi/CMakeLists.txt b/renderlib/gfxapi/CMakeLists.txt index 14caa3503..744ea8e99 100644 --- a/renderlib/gfxapi/CMakeLists.txt +++ b/renderlib/gfxapi/CMakeLists.txt @@ -14,4 +14,5 @@ target_sources(renderlib PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}/IRenderWindow.h" "${CMAKE_CURRENT_SOURCE_DIR}/RenderToFramebuffer.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/RenderToFramebuffer.h" + "${CMAKE_CURRENT_SOURCE_DIR}/WindowSurface.h" ) diff --git a/renderlib/gfxapi/WindowSurface.h b/renderlib/gfxapi/WindowSurface.h new file mode 100644 index 000000000..77b493983 --- /dev/null +++ b/renderlib/gfxapi/WindowSurface.h @@ -0,0 +1,43 @@ +#pragma once + +#include + +namespace gfxApi { + +// Platform-native window surface information supplied by the application layer. +// Backends that need explicit presentation setup (Vulkan) use this to create a +// backend-specific surface; backends where the toolkit owns presentation +// (OpenGL/QOpenGLWidget) can ignore it. +// +// TODO: the nativeHandle()/nativeDisplay() pair is enough for XCB-style X11 and +// for Windows/macOS, but a backend currently has to guess which windowing +// system it is looking at. Wayland in particular wants to be told "this is a +// wl_surface/wl_display" rather than inferred, so this interface should grow a +// platform/kind field. +class IWindowSurface +{ +public: + virtual ~IWindowSurface() = default; + + // Platform-native window handle. + // macOS: NSView* + // Windows: HWND + // X11: xcb_window_t (as a pointer-sized value) + virtual void* nativeHandle() const = 0; + + // Optional platform-native display / connection handle. + // X11: xcb_connection_t* + // Wayland: wl_display* + virtual void* nativeDisplay() const { return nullptr; } + + // True when the surface is visible and can be rendered to. + virtual bool isExposed() const = 0; + + // Size of the surface in physical pixels. + virtual void pixelSize(uint32_t& width, uint32_t& height) const = 0; + + // Ratio of physical pixels to logical points. + virtual double contentScale() const = 0; +}; + +} // namespace gfxApi diff --git a/renderlib/renderlib.cpp b/renderlib/renderlib.cpp index 33fe9c34b..36d0984da 100644 --- a/renderlib/renderlib.cpp +++ b/renderlib/renderlib.cpp @@ -75,6 +75,15 @@ createGraphicsBackend(gfxApi::BackendKind kind, const gfxApi::InitParams& params LOG_ERROR << "createGraphicsBackend: Vulkan backend initialization failed"; return nullptr; } + // Constructing a Vulkan backend only creates the instance. Headless has + // no window surface to select a device against, so bring the device up + // now; windowed defers to Backend::initDeviceForWindow() once the view + // has a native window, because presentation support decides which + // physical device and queue family are usable. + if (params.headless && !backend->initDeviceHeadless()) { + LOG_ERROR << "createGraphicsBackend: Vulkan headless device initialization failed"; + return nullptr; + } LOG_INFO << "createGraphicsBackend: Vulkan backend initialized successfully"; return backend; } From 4e32610b0c53e0c6be1f435f4422926a29e044a2 Mon Sep 17 00:00:00 2001 From: toloudis Date: Sun, 30 Aug 2026 18:29:08 -0700 Subject: [PATCH 2/9] some cleanup --- agave_app/VulkanView3D.cpp | 10 ---------- agave_app/VulkanView3D.h | 2 -- renderlib/gfxVulkan/Backend.cpp | 13 ++++++------- renderlib/gfxVulkan/Backend.h | 2 +- renderlib/gfxapi/Backend.h | 11 +++++------ 5 files changed, 12 insertions(+), 26 deletions(-) diff --git a/agave_app/VulkanView3D.cpp b/agave_app/VulkanView3D.cpp index 5729ab47b..4ae69651c 100644 --- a/agave_app/VulkanView3D.cpp +++ b/agave_app/VulkanView3D.cpp @@ -149,16 +149,6 @@ VulkanView3D::sizeHint() const return QSize(800, 600); } -VkInstance -VulkanView3D::vkInstance() const -{ - gfxApi::Backend* backend = renderlib::graphicsBackend(); - if (!backend || backend->kind() != gfxApi::BackendKind::Vulkan) { - return VK_NULL_HANDLE; - } - return static_cast(backend)->instance(); -} - void VulkanView3D::initCameraFromImage(Scene* scene) { diff --git a/agave_app/VulkanView3D.h b/agave_app/VulkanView3D.h index 6c2b2706b..8183a2858 100644 --- a/agave_app/VulkanView3D.h +++ b/agave_app/VulkanView3D.h @@ -39,8 +39,6 @@ class VulkanView3D QSize minimumSizeHint() const override; QSize sizeHint() const override; - VkInstance vkInstance() const; - // VulkanView3D draws to its own native surface (a CAMetalLayer attached to its // NSView), so Qt must not paint over it with the regular backing store. QPaintEngine* paintEngine() const override { return nullptr; } diff --git a/renderlib/gfxVulkan/Backend.cpp b/renderlib/gfxVulkan/Backend.cpp index ab7b7c477..e852b9b59 100644 --- a/renderlib/gfxVulkan/Backend.cpp +++ b/renderlib/gfxVulkan/Backend.cpp @@ -28,7 +28,7 @@ containsName(const std::vector& names, const char* name) } bool -containsExtension(const std::vector& names, const char* name) +containsName(const std::vector& names, const char* name) { return std::any_of( names.begin(), names.end(), [name](const char* current) { return std::strcmp(current, name) == 0; }); @@ -42,7 +42,7 @@ appendIfAvailable(std::vector& enabledExtensions, if (!containsName(availableExtensions, extensionName)) { return; } - if (containsExtension(enabledExtensions, extensionName)) { + if (containsName(enabledExtensions, extensionName)) { return; } enabledExtensions.push_back(extensionName); @@ -466,10 +466,9 @@ Backend::initDeviceForWindow(gfxApi::IWindowSurface* surface) LOG_ERROR << "Cannot initialize a Vulkan window device from an invalid backend"; return false; } - // A headless backend has no window to select against; the surface, if any, - // is irrelevant. This is the only case where a missing surface is benign. if (m_params.headless) { - return initDeviceHeadless(); + LOG_ERROR << "initDeviceForWindow called on a headless Vulkan backend; use initDeviceHeadless"; + return false; } // A null surface must never be read as "headless". In windowed mode it means // the caller created renderers before the window existed, which is exactly @@ -523,14 +522,14 @@ Backend::createInstance() VkInstanceCreateFlags instanceFlags = 0; if (containsName(availableExtensions, VK_KHR_PORTABILITY_ENUMERATION_EXTENSION_NAME) && - !containsExtension(enabledExtensions, VK_KHR_PORTABILITY_ENUMERATION_EXTENSION_NAME)) { + !containsName(enabledExtensions, VK_KHR_PORTABILITY_ENUMERATION_EXTENSION_NAME)) { enabledExtensions.push_back(VK_KHR_PORTABILITY_ENUMERATION_EXTENSION_NAME); instanceFlags |= VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR; } VkDebugUtilsMessengerCreateInfoEXT debugCreateInfo = {}; if (m_params.enableDebug && containsName(availableExtensions, VK_EXT_DEBUG_UTILS_EXTENSION_NAME) && - !containsExtension(enabledExtensions, VK_EXT_DEBUG_UTILS_EXTENSION_NAME)) { + !containsName(enabledExtensions, VK_EXT_DEBUG_UTILS_EXTENSION_NAME)) { enabledExtensions.push_back(VK_EXT_DEBUG_UTILS_EXTENSION_NAME); debugCreateInfo = debugMessengerCreateInfo(); } diff --git a/renderlib/gfxVulkan/Backend.h b/renderlib/gfxVulkan/Backend.h index 028911095..1a5e5eb78 100644 --- a/renderlib/gfxVulkan/Backend.h +++ b/renderlib/gfxVulkan/Backend.h @@ -29,8 +29,8 @@ class Backend : public gfxApi::Backend void clearCurrentFramebuffer(const gfxApi::ClearColor& color) override; bool isHeadless() const override { return m_params.headless; } gfxApi::BackendKind kind() const override { return gfxApi::BackendKind::Vulkan; } + bool initDeviceForWindow(gfxApi::IWindowSurface* surface = nullptr) override; - bool isDeviceReady() const override { return m_deviceReady; } // Headless counterpart to initDeviceForWindow: there is no surface to wait // for, so the owner brings the device up right after construction. diff --git a/renderlib/gfxapi/Backend.h b/renderlib/gfxapi/Backend.h index dbbcd8017..278cbdc98 100644 --- a/renderlib/gfxapi/Backend.h +++ b/renderlib/gfxapi/Backend.h @@ -91,9 +91,11 @@ class Backend // The kind of backend this is. virtual BackendKind kind() const = 0; - // Some backends need the native window surface before they can choose a - // physical device and queue family, because the choice depends on which - // device can actually present to that surface. Call this once, after the + // Two-part initialization. + // We would like to initialize the renderlib graphics backend as early + // as possible. But some backends require the native window surface to + // be available before they can select a suitable device that can + // actually present to that surface. Call this once, after the // window exists and before creating any renderers. // // A null surface is only valid for a headless backend; in windowed mode it @@ -105,9 +107,6 @@ class Backend (void)surface; return true; } - - // True once device-backed render resources can be created. - virtual bool isDeviceReady() const { return true; } }; } // namespace gfxApi From 7a4537b3c52c041100b6c0184d3d090cc48f31fd Mon Sep 17 00:00:00 2001 From: dmt Date: Sat, 29 Aug 2026 20:47:51 -0700 Subject: [PATCH 3/9] unit test fix, observer must outlive loader since observer may be triggered after loader dtor --- test/test_timeSeriesLoader.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/test/test_timeSeriesLoader.cpp b/test/test_timeSeriesLoader.cpp index 1dbaf5503..9a3d4fbd6 100644 --- a/test/test_timeSeriesLoader.cpp +++ b/test/test_timeSeriesLoader.cpp @@ -260,8 +260,8 @@ TEST_CASE("TimeSeriesLoader serves an interactive request", "[timeSeriesLoader]" cache.setConfig(ramConfig(frameBytes() * 64)); auto reader = std::make_shared(); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); // Prefetch off, so this test observes only the interactive path. @@ -351,8 +351,8 @@ TEST_CASE("TimeSeriesLoader with the disk tier off prefetches only the memory wi cache.setConfig(ramConfig(frameBytes() * 5)); auto reader = std::make_shared(); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); TimeSeriesLoader::PrefetchConfig cfg; @@ -417,8 +417,8 @@ TEST_CASE("TimeSeriesLoader adopts an in-flight prefetch instead of duplicating // request it interactively. reader->setDelay(150ms); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); // Start with prefetch off. Otherwise setSeries begins prefetching t=1 @@ -624,8 +624,8 @@ TEST_CASE("TimeSeriesLoader reports prefetch idle when there is nothing left to cache.setConfig(ramConfig(frameBytes() * 64)); auto reader = std::make_shared(); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); TimeSeriesLoader::PrefetchConfig cfg; @@ -696,8 +696,8 @@ TEST_CASE("TimeSeriesLoader reloads a timepoint whose prefetch was cancelled", " auto reader = std::make_shared(); reader->setDelay(200ms); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); // Prefetch off first, so the initial interactive load is not racing a prefetch. @@ -1158,8 +1158,8 @@ TEST_CASE("TimeSeriesLoader reverts DiskCached when the disk tier evicts", "[tim cache.setConfig(diskCacheConfig(frameBytes() * 8, frameBytes() * diskFrames)); auto reader = std::make_shared(); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); TimeSeriesLoader::PrefetchConfig cfg; @@ -1270,8 +1270,8 @@ TEST_CASE("TimeSeriesLoader survives a historyMargin larger than the budget", "[ cache.setConfig(ramConfig(frameBytes() * 3)); auto reader = std::make_shared(); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); TimeSeriesLoader::PrefetchConfig cfg; @@ -1325,8 +1325,8 @@ TEST_CASE("TimeSeriesLoader never fetches backward after a large jump", "[timeSe cache.setConfig(ramConfig(frameBytes() * 10)); auto reader = std::make_shared(); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); TimeSeriesLoader::PrefetchConfig cfg; @@ -1355,8 +1355,8 @@ TEST_CASE("TimeSeriesLoader clamps the disk warm set to the disk budget", "[time cache.setConfig(diskCacheConfig(frameBytes() * 4, frameBytes() * diskFrames)); auto reader = std::make_shared(); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); TimeSeriesLoader::PrefetchConfig cfg; @@ -1435,8 +1435,8 @@ TEST_CASE("TimeSeriesLoader warm-only prefetch does not pull volumes into RAM", cache.clearMemoryCache(); auto reader = std::make_shared(); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); TimeSeriesLoader::PrefetchConfig cfg; @@ -1491,8 +1491,8 @@ TEST_CASE("TimeSeriesLoader three-run cross-session scenario", "[timeSeriesLoade CacheManager cache(dir.str()); cache.setConfig(diskCacheConfig(frameBytes() * ramFrames, frameBytes() * diskFrames)); auto reader = std::make_shared(); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); loader.setPrefetchConfig(makeCfg()); @@ -1528,8 +1528,8 @@ TEST_CASE("TimeSeriesLoader three-run cross-session scenario", "[timeSeriesLoade CacheManager cache(dir.str()); cache.setConfig(diskCacheConfig(frameBytes() * ramFrames, frameBytes() * diskFrames)); auto reader = std::make_shared(); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); loader.setPrefetchConfig(makeCfg()); @@ -1627,8 +1627,8 @@ TEST_CASE("TimeSeriesLoader with prefetch off still caches on-demand loads", "[t cache.setConfig(diskCacheConfig(frameBytes() * 16, 64ULL * 1024 * 1024)); auto reader = std::make_shared(); - TimeSeriesLoader loader(cache); RecordingObserver observer; + TimeSeriesLoader loader(cache); loader.addObserver(&observer); TimeSeriesLoader::PrefetchConfig cfg; From 60b3457119696cf782a811a8bca3cae3d84e53e8 Mon Sep 17 00:00:00 2001 From: dmt Date: Sun, 30 Aug 2026 19:27:22 -0700 Subject: [PATCH 4/9] update for vulkan --- AGENTS.md | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e04248e81..b9f021369 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,13 +7,39 @@ AGAVE (Advanced GPU Accelerated Volume Explorer) is a C++17/Qt6 desktop applicat | Module | Role | | ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | | `agave_app/` | Qt6 GUI layer — widgets, dialogs, dock panels, OpenGL viewport (`GLView3D`) | -| `renderlib/` | Core rendering engine — image I/O (`io/`), GPU pipeline (`graphics/`), camera, scene, gesture handling (`gesture/`), JSON serialization (`json/`) | +| `renderlib/` | Core rendering engine — image I/O (`io/`), graphics abstraction (`gfxapi/`) with OpenGL and Vulkan backends (`gfxOpenGL/`, `gfxVulkan/`), camera, scene, gesture handling (`gesture/`), JSON serialization (`json/`) | | `agave_pyclient/` | Python WebSocket client for remote control of AGAVE in server mode | | `test/` | C++ unit tests (Catch2) | | `webclient/` | JavaScript client | `agave_app` depends on `renderlib` for all rendering and data operations. Keep GUI concerns out of `renderlib`. `renderlib` should have no Qt dependencies and be testable in isolation. The Python client and web client communicate with the C++ engine via a binary command protocol defined in `renderlib/command.h` and implemented in `renderlib/command.cpp`. Commands must be added in all three locations to stay in sync (see "Adding a New Command" below). +### Graphics backends (`gfxapi`) + +`renderlib/gfxapi/` is a backend-agnostic graphics abstraction so the rest of the +renderer never talks to OpenGL or Vulkan directly. Its central interface is +`gfxApi::Backend`, which owns the GPU device and any backend-global state and +creates the concrete renderers, framebuffers, and contexts. `IGraphicsDevice` +exposes the device-level primitives (shaders and shader programs today; the +surface is deliberately minimal and grows as more primitives move behind it), and +sibling interfaces cover the render window (`IRenderWindow`), gesture/manipulator +drawing (`IGestureRenderer`), `Framebuffer`, `IGLContext`, and `IWindowSurface`. + +Concrete implementations live in `gfxOpenGL/` and `gfxVulkan/` (Vulkan is gated +behind `AGAVE_HAS_VULKAN`; `WebGPU` is enumerated in `BackendKind` but not yet +implemented). `renderlib::initialize` creates exactly **one** `Backend` for the +process lifetime, chosen from `InitParams::backendKind`. The `createGraphicsBackend` +function in `renderlib.cpp` is the single place that maps a `BackendKind` onto a +concrete backend — the abstract `gfxapi/` layer must not depend on any concrete +backend, and renderer code should reach GPU functionality through `Backend::device()` +and the `gfxapi` interfaces rather than backend-specific APIs. + +Backend bring-up is two-phase: construction creates the instance/context, then a +device is selected. Windowed Vulkan defers device selection to +`Backend::initDeviceForWindow()` once a native surface exists; headless uses EGL +(OpenGL) or `initDeviceHeadless()` (Vulkan). A null surface is only valid in +headless mode. + ## Build and Test Prerequisites and platform-specific setup are in [INSTALL.md](INSTALL.md). Dependencies are fetched via CMake FetchContent (GLM, Catch2) and require Qt 6.9.3 installed on the system. @@ -99,7 +125,7 @@ clang-tidy -p build --fix renderlib/RenderSettings.cpp ## Adding a New Command -Commands are the binary protocol connecting the C++ engine, Python client, and web client. Every command must be added to all four locations to stay in sync. +Commands are the binary protocol connecting the C++ engine, Python client, and web client. Every command must be added with the following steps to stay in sync. ### 1. `renderlib/command.h` — declare data struct + command class @@ -195,9 +221,53 @@ set_foo(x: number, mode: number) { } ``` +Steps 9–11 also make the command available in `agave_pyvk`, the in-process, +headless Vulkan package. It does **not** serialize command buffers: `AgaveRenderer` +passes typed arguments straight into `renderlib` through nanobind, and the C++ side +dispatches on the integer command ID via `PythonRenderer`. Because that dispatch is +data-driven, `agave_pyvk/src/bindings.cpp` is generic and needs **no** per-command +changes. + +### 9. `renderlib/PythonRenderer.cpp` — register in both switches + +```cpp +// In commandArgumentTypes(): maps ID -> argument type list. +COMMAND_ARGUMENT_TYPES(52, SetFooCommand); + +// In execute(): constructs the command from positional args and runs it. +// The count and the arg(args, N) order/types must match the command's CMD_ARGS. +EXECUTE_COMMAND(52, 2, SetFooCommand, SetFooCommandD{ arg(args, 0), arg(args, 1) }); +``` + +### 10. `agave_pyvk/agave_pyvk/commandbuffer.py` — add to `COMMANDS` + +This module keeps only the ID (no argument-type list); it exists for API/source +parity with `agave_pyclient`. + +```python +"SET_FOO": 52, +``` + +### 11. `agave_pyvk/agave_pyvk/agave.py` — add method to `AgaveRenderer` + +Mirror the `agave_pyclient` method, but dispatch through `self._execute`, which +calls the native renderer synchronously and returns its result. + +```python +def set_foo(self, x: float, mode: int): + return self._execute("SET_FOO", x, mode) +``` + **Key rules:** -- The integer ID must be unique and match across all four locations +- The integer ID must be unique and match across all locations, including + `renderlib/command.h` and both `PythonRenderer` switches - Argument types are `F32`, `I32`, `S` (string), `F32A` (float array), `I32A` (int array) - Python method name uses snake_case; `COMMANDS` dict key is UPPERCASE -- `parse()`/`write()` field order must match the `CMD_ARGS` type list exactly +- `parse()`/`write()` field order must match the `CMD_ARGS` type list exactly, as + must the `EXECUTE_COMMAND` arg count and `arg()` order/types +- `agave_pyvk/src/bindings.cpp` is generic — do not add per-command code there +- The public method must exist in **both** `agave_pyclient/agave.py` and + `agave_pyvk/agave.py`. `test_public_api_matches_pyclient` in + `agave_pyvk/tests/test_api.py` enforces that every `agave_pyclient` method is + also present on `agave_pyvk` (which only adds `load_array`) From b1521a525c6ad1baa6ea0c049576a090db800e5f Mon Sep 17 00:00:00 2001 From: dmt Date: Sun, 30 Aug 2026 19:34:53 -0700 Subject: [PATCH 5/9] set debug flag --- agave_app/main.cpp | 4 ++++ renderlib/PythonRenderer.cpp | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/agave_app/main.cpp b/agave_app/main.cpp index d381428a9..288632711 100644 --- a/agave_app/main.cpp +++ b/agave_app/main.cpp @@ -332,6 +332,10 @@ main(int argc, char* argv[]) initParams.headless = isServer; initParams.selectedGpu = selectedGpu; initParams.windowedContext = bootstrapGLContext.get(); +#ifndef NDEBUG + // Enable graphics API validation/debug output in debug builds. + initParams.enableDebug = true; +#endif if (0 == renderlib::initialize(initParams, listDevices)) { renderlib::cleanup(); diff --git a/renderlib/PythonRenderer.cpp b/renderlib/PythonRenderer.cpp index 6ef6991dd..7b75a48a6 100644 --- a/renderlib/PythonRenderer.cpp +++ b/renderlib/PythonRenderer.cpp @@ -99,6 +99,10 @@ PythonRenderer::initialize(const std::string& mode, const std::string& assetPath params.assetPath = assetPath; params.headless = true; params.selectedGpu = selectedGpu; +#ifndef NDEBUG + // Enable graphics API validation/debug output in debug builds. + params.enableDebug = true; +#endif if (!renderlib::initialize(params)) { throw std::runtime_error("Unable to initialize the headless Vulkan backend"); } From 31f99d317c3a879b5ead77264a0e7eb007e7a203 Mon Sep 17 00:00:00 2001 From: dmt Date: Sun, 30 Aug 2026 19:38:30 -0700 Subject: [PATCH 6/9] log out the renderlib init params --- renderlib/renderlib.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/renderlib/renderlib.cpp b/renderlib/renderlib.cpp index 36d0984da..ce95d371c 100644 --- a/renderlib/renderlib.cpp +++ b/renderlib/renderlib.cpp @@ -48,6 +48,20 @@ toRenderWindowKind(renderlib::RendererType rendererType) } } +const char* +backendKindName(gfxApi::BackendKind kind) +{ + switch (kind) { + case gfxApi::BackendKind::OpenGL: + return "OpenGL"; + case gfxApi::BackendKind::Vulkan: + return "Vulkan"; + case gfxApi::BackendKind::WebGPU: + return "WebGPU"; + } + return "Unknown"; +} + } // namespace // Backend selection lives here, in renderlib, rather than in gfxapi: the @@ -115,6 +129,10 @@ renderlib::initialize(const gfxApi::InitParams& initParams, bool listDevices) } LOG_INFO << "Renderlib startup"; + LOG_INFO << " backend: " << backendKindName(params.backendKind) << ", headless: " << params.headless + << ", selectedGpu: " << params.selectedGpu << ", enableDebug: " << params.enableDebug + << ", assetPath: " << params.assetPath; + // --list-devices: enumerate the available GPUs and quit. This only needs the // backend's device enumeration, not a fully initialized backend. From 3358f55c26d45a91a237da9139fa45ef0ac4da5f Mon Sep 17 00:00:00 2001 From: dmt Date: Sun, 30 Aug 2026 19:42:04 -0700 Subject: [PATCH 7/9] bug fix --- agave_app/CacheSettingsWidget.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/agave_app/CacheSettingsWidget.cpp b/agave_app/CacheSettingsWidget.cpp index 4ddd8b65b..b40cd538c 100644 --- a/agave_app/CacheSettingsWidget.cpp +++ b/agave_app/CacheSettingsWidget.cpp @@ -55,6 +55,10 @@ CacheSettingsWidget::CacheSettingsWidget(QWidget* parent, AgaveSettingsData* set layout->addRow(QString(), m_applyButton); layout->addRow(QString(), m_clearDiskButton); setLayout(layout); + + // Populate the fields from the already-loaded settings; otherwise the spin + // boxes show their default 0 until something else triggers a refresh. + updateUiFromSettings(); } void From 85b9eb2329cf5da90e9eb37d2fb9250457ff3d2a Mon Sep 17 00:00:00 2001 From: dmt Date: Sun, 30 Aug 2026 20:17:15 -0700 Subject: [PATCH 8/9] cleanup --- agave_app/VulkanView3D.cpp | 2 +- agave_pyvk/src/bindings.cpp | 2 -- renderlib/gfxapi/Backend.h | 6 ++---- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/agave_app/VulkanView3D.cpp b/agave_app/VulkanView3D.cpp index 4ae69651c..3906154ba 100644 --- a/agave_app/VulkanView3D.cpp +++ b/agave_app/VulkanView3D.cpp @@ -105,7 +105,7 @@ VulkanView3D::VulkanView3D(QCamera* cam, QRenderSettings* qrs, RenderSettings* r LOG_ERROR << msg; throw std::runtime_error(msg); } - // initDeviceForWindow has already logged which devices were rejected and why. + if (!backend->initDeviceForWindow(m_surface.get())) { const auto msg = "Failed to initialize a graphics device that can present to the 3D view window"; LOG_ERROR << msg; diff --git a/agave_pyvk/src/bindings.cpp b/agave_pyvk/src/bindings.cpp index c3678cab4..49748bd8b 100644 --- a/agave_pyvk/src/bindings.cpp +++ b/agave_pyvk/src/bindings.cpp @@ -186,8 +186,6 @@ NB_MODULE(_native, m) .def(nb::init(), nb::arg("mode") = "pathtrace", nb::arg("asset_path") = "", - // Auto-selects by default; an explicit index is used as given and - // validated rather than silently replaced. nb::arg("gpu") = gfxApi::kAutoSelectGpu) .def("execute", &execute) .def("load_array", diff --git a/renderlib/gfxapi/Backend.h b/renderlib/gfxapi/Backend.h index 278cbdc98..5ff6f074d 100644 --- a/renderlib/gfxapi/Backend.h +++ b/renderlib/gfxapi/Backend.h @@ -91,7 +91,7 @@ class Backend // The kind of backend this is. virtual BackendKind kind() const = 0; - // Two-part initialization. + // Two-part initialization when windowed. // We would like to initialize the renderlib graphics backend as early // as possible. But some backends require the native window surface to // be available before they can select a suitable device that can @@ -99,9 +99,7 @@ class Backend // window exists and before creating any renderers. // // A null surface is only valid for a headless backend; in windowed mode it - // is an error rather than an implicit switch to headless behavior. Backends - // where the toolkit owns presentation (OpenGL/Qt) are fully initialized by - // construction and keep this default implementation. + // is an error. virtual bool initDeviceForWindow(IWindowSurface* surface = nullptr) { (void)surface; From 4a271bad2dd795d2a585bc8b937a57914749f824 Mon Sep 17 00:00:00 2001 From: dmt Date: Sun, 30 Aug 2026 20:47:30 -0700 Subject: [PATCH 9/9] log info of autoselected device --- renderlib/gfxVulkan/Backend.cpp | 112 +++++++++++++++++++++++--------- 1 file changed, 82 insertions(+), 30 deletions(-) diff --git a/renderlib/gfxVulkan/Backend.cpp b/renderlib/gfxVulkan/Backend.cpp index e852b9b59..4854539a3 100644 --- a/renderlib/gfxVulkan/Backend.cpp +++ b/renderlib/gfxVulkan/Backend.cpp @@ -162,28 +162,73 @@ apiVersionToString(uint32_t version) return ss.str(); } -// Fallback for devices that cannot report VkPhysicalDeviceDriverProperties. -// The spec mandates no encoding for driverVersion: each vendor packs it -// differently, so decoding it like an API version prints nonsense for the two -// exceptions below. Everyone else does follow the API version layout. +/** + * Parses the raw driverVersion bitfield from VkPhysicalDeviceProperties + * into a vendor-specific formatted version string. + * + * @param vendorID The unique PCI/Khronos vendor identifier (properties.vendorID) + * @param driverVersion The raw encoded integer (properties.driverVersion) + * @return A human-readable version string matching the vendor platform's layout + */ std::string driverVersionToString(uint32_t driverVersion, uint32_t vendorID) { - std::ostringstream ss; + std::stringstream ss; - if (vendorID == 0x10de) { // NVIDIA: 10 | 8 | 8 | 6 bits - ss << ((driverVersion >> 22) & 0x3ff) << "." << ((driverVersion >> 14) & 0x0ff) << "." - << ((driverVersion >> 6) & 0x0ff) << "." << (driverVersion & 0x03f); - return ss.str(); - } -#if defined(_WIN32) - if (vendorID == 0x8086) { // Intel, Windows driver only: 18 | 14 bits - ss << (driverVersion >> 14) << "." << (driverVersion & 0x3fff); + // 1. NVIDIA (Vendor ID: 0x10DE) + if (vendorID == 0x10DE) { + uint32_t major = (driverVersion >> 22) & 0x3FF; + uint32_t minor = (driverVersion >> 14) & 0x0FF; + uint32_t subMinor = (driverVersion >> 6) & 0x0FF; + uint32_t patch = (driverVersion) & 0x03F; + + ss << major << "." << minor << "." << subMinor << "." << patch; + } + // 2. INTEL (Vendor ID: 0x8086) + else if (vendorID == 0x8086) { + // Intel encodes differently depending on Windows vs Linux (Mesa) + // If the standard major mask results in 0, it indicates Windows WHQL encoding layout + if (VK_VERSION_MAJOR(driverVersion) == 0) { + uint32_t baselineBuild = (driverVersion >> 18) & 0x3FFF; + uint32_t finalBuild = (driverVersion) & 0x3FFFF; + ss << "101." << baselineBuild << "." << finalBuild; // e.g., 101.5445 + } else { + // Standard Khronos format used by Intel open-source ANV / Mesa drivers + ss << VK_VERSION_MAJOR(driverVersion) << "." + << VK_VERSION_MINOR(driverVersion) << "." + << VK_VERSION_PATCH(driverVersion); + } + } + // 3. AMD (Vendor ID: 0x1002) + else if (vendorID == 0x1002) { + // AMD uses a modified layout: Major (10 bits), Minor (0/Unassigned), Patch (22 bits) + uint32_t major = (driverVersion >> 22) & 0x3FF; + uint32_t patch = (driverVersion) & 0x3FFFFF; // 22 bits + + // Note: For Mesa RADV on Linux, it defaults back to standard Khronos format + if (VK_VERSION_MINOR(driverVersion) != 0) { + ss << VK_VERSION_MAJOR(driverVersion) << "." + << VK_VERSION_MINOR(driverVersion) << "." + << VK_VERSION_PATCH(driverVersion); + } else { + ss << major << "." << patch; // e.g., 2.0.94 + } + } + // 4. APPLE / MOLTENVK (Vendor ID: 0x106B) + else if (vendorID == 0x106B) { + // MoltenVK maps directly to standard Khronos layout formatting + ss << VK_VERSION_MAJOR(driverVersion) << "." + << VK_VERSION_MINOR(driverVersion) << "." + << VK_VERSION_PATCH(driverVersion); + } + // 5. FALLBACK (Unknown or standard conformance driver layers) + else { + // Fall back to standard Khronos Core API Version layout macros + ss << VK_VERSION_MAJOR(driverVersion) << "." + << VK_VERSION_MINOR(driverVersion) << "." + << VK_VERSION_PATCH(driverVersion); + } return ss.str(); - } -#endif - - return apiVersionToString(driverVersion); } // VkPhysicalDeviceDriverProperties carries the driver's own name and version @@ -349,6 +394,23 @@ describeCapabilities(const DeviceCapabilities& capabilities, bool requiresPresen return description; } +// Multi-line dump of a device's name, versions, type, and capabilities. Shared +// by --list_devices and the selected-device log so both read identically. +void +logDeviceInfo(uint32_t index, VkPhysicalDevice physicalDevice, const DeviceCapabilities& capabilities) +{ + VkPhysicalDeviceProperties properties = {}; + vkGetPhysicalDeviceProperties(physicalDevice, &properties); + + LOG_INFO << "Vulkan device " << index << ": " << properties.deviceName; + LOG_INFO << " API version: " << apiVersionToString(properties.apiVersion); + LOG_INFO << " Driver version: " << driverDescription(physicalDevice, properties); + LOG_INFO << " Device type: " << deviceTypeToString(properties.deviceType); + LOG_INFO << " Capabilities: " + << (capabilities.graphicsQueueFamilyIndex == UINT32_MAX ? "no graphics queue" : "graphics") + << (capabilities.hasSwapchainExtension ? ", swapchain" : ", no swapchain"); +} + // Extra guidance for the windowed case, where "no device can present" usually // means a platform/driver mismatch rather than missing hardware. void @@ -697,9 +759,8 @@ Backend::pickPhysicalDevice(VkSurfaceKHR presentationSurface) m_graphicsQueueFamilyIndex = requiresPresent ? capabilities[chosenIndex].graphicsPresentQueueFamilyIndex : capabilities[chosenIndex].graphicsQueueFamilyIndex; - LOG_INFO << "Selected Vulkan device " << chosenIndex << ": " << physicalDeviceName(m_physicalDevice) - << " (queue family " << m_graphicsQueueFamilyIndex - << (requiresPresent ? ", graphics+present)" : ", graphics)"); + LOG_INFO << "Selected Vulkan device:"; + logDeviceInfo(chosenIndex, m_physicalDevice, capabilities[chosenIndex]); return true; } @@ -927,19 +988,10 @@ Backend::listDevices(int selectedGpu) LOG_INFO << deviceCount << " Vulkan device(s) found. These indices are what --gpu N selects."; for (uint32_t i = 0; i < deviceCount; ++i) { - VkPhysicalDeviceProperties properties = {}; - vkGetPhysicalDeviceProperties(devices[i], &properties); // Query without a surface: presentation support depends on the actual // window surface, which does not exist during device listing. const DeviceCapabilities capabilities = inspectPhysicalDevice(devices[i], VK_NULL_HANDLE); - - LOG_INFO << "Vulkan device " << i << ": " << properties.deviceName; - LOG_INFO << " API version: " << apiVersionToString(properties.apiVersion); - LOG_INFO << " Driver version: " << driverDescription(devices[i], properties); - LOG_INFO << " Device type: " << deviceTypeToString(properties.deviceType); - LOG_INFO << " Capabilities: " - << (capabilities.graphicsQueueFamilyIndex == UINT32_MAX ? "no graphics queue" : "graphics") - << (capabilities.hasSwapchainExtension ? ", swapchain" : ", no swapchain"); + logDeviceInfo(i, devices[i], capabilities); } if (selectedGpu >= 0 && static_cast(selectedGpu) >= deviceCount) {