From 435d5a9b9e3e8f096c8da3c6150704e55f8219fa Mon Sep 17 00:00:00 2001 From: Lubosz Sarnecki Date: Wed, 3 Jun 2020 09:50:01 +0200 Subject: [PATCH] c/comp: Remove internal Vulkan validation init. Instead of maintaining this chunk of code and build options, the Vulkan loader can be used to load up validation. This has the advantage that no layer name needs to be hard coded inside Monado, which was subject of change recently. Instead of using our own environment variable we can easily set the one from the loader, e.g. `VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation`. --- CMakeLists.txt | 1 - meson_options.txt | 5 -- src/xrt/auxiliary/vk/vk_helpers.c | 74 ----------------------- src/xrt/auxiliary/vk/vk_helpers.h | 12 ---- src/xrt/compositor/CMakeLists.txt | 5 -- src/xrt/compositor/main/comp_compositor.c | 25 -------- src/xrt/compositor/main/comp_settings.c | 2 - src/xrt/compositor/main/comp_settings.h | 3 - src/xrt/compositor/meson.build | 4 -- 9 files changed, 131 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 24617f003..6483ba4e8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,7 +10,6 @@ if(POLICY CMP0072) endif() option(XRT_OPENXR_INSTALL_ABSOLUTE_RUNTIME_PATH "Use the absolute path to the runtime in the installed manifest, rather than a bare filename." ON) -option(XRT_VULKAN_ENABLE_VALIDATION "Enable Vulkan validation for Compositor" ON) option(XRT_OPENXR_INSTALL_ACTIVE_RUNTIME "Make Monado the default OpenXR runtime on install" ON) ### diff --git a/meson_options.txt b/meson_options.txt index 5ab9e59b0..312ae86d1 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -32,11 +32,6 @@ option('tracking', value: 'auto', description: 'Enable tracking support') -option('vulkan-validation', - type: 'boolean', - value: true, - description: 'Enable Vulkan validation for compositor') - option('install-active-runtime', type: 'boolean', value: true, diff --git a/src/xrt/auxiliary/vk/vk_helpers.c b/src/xrt/auxiliary/vk/vk_helpers.c index 67d8f351c..4b9061aca 100644 --- a/src/xrt/auxiliary/vk/vk_helpers.c +++ b/src/xrt/auxiliary/vk/vk_helpers.c @@ -568,80 +568,6 @@ vk_init_cmd_pool(struct vk_bundle *vk) return ret; } - -/* - * - * Debug code. - * - */ - -#define ENUM_TO_STR(r) \ - case r: return #r - -static const char * -vk_debug_report_string(VkDebugReportFlagsEXT code) -{ - switch (code) { - ENUM_TO_STR(VK_DEBUG_REPORT_INFORMATION_BIT_EXT); - ENUM_TO_STR(VK_DEBUG_REPORT_WARNING_BIT_EXT); - ENUM_TO_STR(VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT); - ENUM_TO_STR(VK_DEBUG_REPORT_ERROR_BIT_EXT); - ENUM_TO_STR(VK_DEBUG_REPORT_DEBUG_BIT_EXT); - ENUM_TO_STR(VK_DEBUG_REPORT_FLAG_BITS_MAX_ENUM_EXT); - } - return "UNKNOWN REPORT"; -} - -static VkBool32 VKAPI_PTR -_validation_cb(VkDebugReportFlagsEXT flags, - VkDebugReportObjectTypeEXT object_type, - uint64_t object, - size_t location, - int32_t message_code, - const char *layer_prefix, - const char *message, - void *user_data) -{ - fprintf(stderr, "%s %s %zu:%d: %s\n", vk_debug_report_string(flags), - layer_prefix, location, message_code, message); - return VK_FALSE; -} - -DEBUG_GET_ONCE_BOOL_OPTION(vulkan_spew, "XRT_COMPOSITOR_VULKAN_SPEW", false) - -void -vk_init_validation_callback(struct vk_bundle *vk) -{ - VkDebugReportFlagsEXT flags = 0; - flags |= VK_DEBUG_REPORT_ERROR_BIT_EXT; - flags |= VK_DEBUG_REPORT_WARNING_BIT_EXT; - - if (debug_get_bool_option_vulkan_spew()) { - flags |= VK_DEBUG_REPORT_INFORMATION_BIT_EXT; - flags |= VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT; - flags |= VK_DEBUG_REPORT_DEBUG_BIT_EXT; - } - - VkDebugReportCallbackCreateInfoEXT info = { - .sType = VK_STRUCTURE_TYPE_DEBUG_REPORT_CREATE_INFO_EXT, - .flags = flags, - .pfnCallback = _validation_cb, - }; - - vk->vkCreateDebugReportCallbackEXT(vk->instance, &info, NULL, - &vk->debug_report_cb); -} - -void -vk_destroy_validation_callback(struct vk_bundle *vk) -{ - if (vk->debug_report_cb != VK_NULL_HANDLE) { - vk->vkDestroyDebugReportCallbackEXT(vk->instance, - vk->debug_report_cb, NULL); - vk->debug_report_cb = VK_NULL_HANDLE; - } -} - /* * * Function getting code. diff --git a/src/xrt/auxiliary/vk/vk_helpers.h b/src/xrt/auxiliary/vk/vk_helpers.h index 59e58b9fb..324e1fc13 100644 --- a/src/xrt/auxiliary/vk/vk_helpers.h +++ b/src/xrt/auxiliary/vk/vk_helpers.h @@ -247,18 +247,6 @@ vk_has_error(VkResult res, const char *fun, const char *file, int line); if (vk_has_error(res, fun, __FILE__, __LINE__)) \ return ret -/*! - * @ingroup aux_vk - */ -void -vk_init_validation_callback(struct vk_bundle *vk); - -/*! - * @ingroup aux_vk - */ -void -vk_destroy_validation_callback(struct vk_bundle *vk); - /*! * @ingroup aux_vk */ diff --git a/src/xrt/compositor/CMakeLists.txt b/src/xrt/compositor/CMakeLists.txt index 6f01f72a6..9620a2944 100644 --- a/src/xrt/compositor/CMakeLists.txt +++ b/src/xrt/compositor/CMakeLists.txt @@ -38,11 +38,6 @@ set(MAIN_SOURCE_FILES main/comp_layer_renderer.c ) -if (XRT_VULKAN_ENABLE_VALIDATION) - add_definitions(-DXRT_ENABLE_VK_VALIDATION) -endif() - - ### # Client library # diff --git a/src/xrt/compositor/main/comp_compositor.c b/src/xrt/compositor/main/comp_compositor.c index 414b47ea5..5bcf18033 100644 --- a/src/xrt/compositor/main/comp_compositor.c +++ b/src/xrt/compositor/main/comp_compositor.c @@ -95,8 +95,6 @@ compositor_destroy(struct xrt_compositor *xc) vk->device = VK_NULL_HANDLE; } - vk_destroy_validation_callback(vk); - if (vk->instance != VK_NULL_HANDLE) { vk->vkDestroyInstance(vk->instance, NULL); vk->instance = VK_NULL_HANDLE; @@ -446,14 +444,7 @@ find_get_instance_proc_addr(struct comp_compositor *c) return vk_get_loader_functions(&c->vk, vkGetInstanceProcAddr); } -#ifdef XRT_ENABLE_VK_VALIDATION -#define COMPOSITOR_DEBUG_VULKAN_EXTENSIONS VK_EXT_DEBUG_REPORT_EXTENSION_NAME, -#else -#define COMPOSITOR_DEBUG_VULKAN_EXTENSIONS -#endif - #define COMPOSITOR_COMMON_VULKAN_EXTENSIONS \ - COMPOSITOR_DEBUG_VULKAN_EXTENSIONS \ VK_KHR_SURFACE_EXTENSION_NAME, \ VK_KHR_GET_PHYSICAL_DEVICE_PROPERTIES_2_EXTENSION_NAME, \ VK_KHR_EXTERNAL_MEMORY_CAPABILITIES_EXTENSION_NAME, \ @@ -550,17 +541,6 @@ create_instance(struct comp_compositor *c) .ppEnabledExtensionNames = instance_extensions, }; -#ifdef XRT_ENABLE_VK_VALIDATION - const char *instance_layers[] = { - "VK_LAYER_LUNARG_standard_validation", - }; - - if (c->settings.validate_vulkan) { - instance_info.enabledLayerCount = ARRAY_SIZE(instance_layers); - instance_info.ppEnabledLayerNames = instance_layers; - } -#endif - ret = c->vk.vkCreateInstance(&instance_info, NULL, &c->vk.instance); if (ret != VK_SUCCESS) { COMP_ERROR(c, "vkCreateInstance: %s\n", vk_result_string(ret)); @@ -575,11 +555,6 @@ create_instance(struct comp_compositor *c) return ret; } -#ifdef XRT_ENABLE_VK_VALIDATION - if (c->settings.validate_vulkan) - vk_init_validation_callback(&c->vk); -#endif - return ret; } diff --git a/src/xrt/compositor/main/comp_settings.c b/src/xrt/compositor/main/comp_settings.c index f5313dabc..17a5d3aa2 100644 --- a/src/xrt/compositor/main/comp_settings.c +++ b/src/xrt/compositor/main/comp_settings.c @@ -18,7 +18,6 @@ DEBUG_GET_ONCE_BOOL_OPTION(force_randr, "XRT_COMPOSITOR_FORCE_RANDR", false) DEBUG_GET_ONCE_BOOL_OPTION(force_nvidia, "XRT_COMPOSITOR_FORCE_NVIDIA", false) DEBUG_GET_ONCE_BOOL_OPTION(force_xcb, "XRT_COMPOSITOR_FORCE_XCB", false) DEBUG_GET_ONCE_BOOL_OPTION(force_wayland, "XRT_COMPOSITOR_FORCE_WAYLAND", false) -DEBUG_GET_ONCE_BOOL_OPTION(validate_vulkan, "XRT_COMPOSITOR_VULKAN_VALIDATION", false) DEBUG_GET_ONCE_BOOL_OPTION(wireframe, "XRT_COMPOSITOR_WIREFRAME", false) DEBUG_GET_ONCE_NUM_OPTION(force_gpu_index, "XRT_COMPOSITOR_FORCE_GPU_INDEX", -1) DEBUG_GET_ONCE_NUM_OPTION(desired_mode, "XRT_COMPOSITOR_DESIRED_MODE", -1) @@ -47,7 +46,6 @@ comp_settings_init(struct comp_settings *s, struct xrt_device *xdev) s->print_spew = debug_get_bool_option_print_spew(); s->print_debug = debug_get_bool_option_print_debug(); s->print_modes = debug_get_bool_option_print_modes(); - s->validate_vulkan = debug_get_bool_option_validate_vulkan(); s->gpu_index = debug_get_num_option_force_gpu_index(); s->debug.wireframe = debug_get_bool_option_wireframe(); s->desired_mode = debug_get_num_option_desired_mode(); diff --git a/src/xrt/compositor/main/comp_settings.h b/src/xrt/compositor/main/comp_settings.h index 65f6d4d7f..c396e82d4 100644 --- a/src/xrt/compositor/main/comp_settings.h +++ b/src/xrt/compositor/main/comp_settings.h @@ -94,9 +94,6 @@ struct comp_settings //! Nominal frame interval uint64_t nominal_frame_interval_ns; - //! Enable vulkan validation for compositor - bool validate_vulkan; - //! Run the compositor on this Vulkan physical device int gpu_index; diff --git a/src/xrt/compositor/meson.build b/src/xrt/compositor/meson.build index 797ad018f..84254d2ed 100644 --- a/src/xrt/compositor/meson.build +++ b/src/xrt/compositor/meson.build @@ -111,10 +111,6 @@ if build_wayland compositor_deps += [wayland, wl_protos] endif -if get_option('vulkan-validation') - compile_args += ['-DXRT_ENABLE_VK_VALIDATION'] -endif - lib_comp = static_library( 'comp', compositor_srcs,