From 3231bf945e3827a8494c0e8a2c4bb9d652e036c5 Mon Sep 17 00:00:00 2001 From: Charles Giessen Date: Fri, 16 Jan 2026 14:18:15 -0600 Subject: [PATCH] Remove duplicates from vkEnumerateInstanceLayerProperties If multiple layers share the same name, the loader should remove them as the application cannot specify which of the duplicate layers should be loaded when vkCreateInstance is called. This mirrors the behavior of vkCreateInstance which removes duplicate layers before loading. By de-duplicating during vkEnumerateInstanceLayerProperties and vkEnumerateInstanceExtensionProperties, it prevents situations where the duplicate layers differed in available instance extensions and the extension would be reported as unavailable during vkCreateInstance because the layer chosen to load didn't have the extension. --- loader/loader.c | 27 +++++++++++ tests/loader_layer_tests.cpp | 76 +++++++++++++++++++++++-------- tests/loader_regression_tests.cpp | 2 +- 3 files changed, 86 insertions(+), 19 deletions(-) diff --git a/loader/loader.c b/loader/loader.c index e751fd6d5..f8b01b6df 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -4272,6 +4272,27 @@ VkResult get_override_layer_override_paths(struct loader_instance *inst, struct return VK_SUCCESS; } +void loader_remove_duplicate_layers(struct loader_instance *inst, struct loader_layer_list *layers) { + // Remove duplicate layers (that share the same layerName) + for (uint32_t i = 0; i < layers->count; ++i) { + bool has_duplicate = false; + uint32_t duplicate_index = 0; + for (uint32_t j = i + 1; j < layers->count; ++j) { + if (strcmp(layers->list[i].info.layerName, layers->list[j].info.layerName) == 0) { + has_duplicate = true; + duplicate_index = j; + break; + } + } + if (has_duplicate) { + loader_log(inst, VULKAN_LOADER_WARN_BIT, 0, "Removing layer %s (%s) because it is a duplicate of %s (%s)", + layers->list[duplicate_index].info.layerName, layers->list[duplicate_index].manifest_file_name, + layers->list[i].info.layerName, layers->list[i].manifest_file_name); + loader_remove_layer_in_list(inst, layers, duplicate_index); + } + } +} + VkResult loader_scan_for_layers(struct loader_instance *inst, struct loader_layer_list *instance_layers, const struct loader_envvar_all_filters *filters) { VkResult res = VK_SUCCESS; @@ -4342,6 +4363,9 @@ VkResult loader_scan_for_layers(struct loader_instance *inst, struct loader_laye } } + // Make sure no layers have the same layerName + loader_remove_duplicate_layers(inst, ®ular_instance_layers); + res = combine_settings_layers_with_regular_layers(inst, &settings_layers, ®ular_instance_layers, instance_layers); out: @@ -4442,6 +4466,9 @@ VkResult loader_scan_for_implicit_layers(struct loader_instance *inst, struct lo } } + // Make sure no layers have the same layerName + loader_remove_duplicate_layers(inst, ®ular_instance_layers); + res = combine_settings_layers_with_regular_layers(inst, &settings_layers, ®ular_instance_layers, instance_layers); out: diff --git a/tests/loader_layer_tests.cpp b/tests/loader_layer_tests.cpp index 52f1f2c1c..a88280c87 100644 --- a/tests/loader_layer_tests.cpp +++ b/tests/loader_layer_tests.cpp @@ -979,11 +979,9 @@ TEST(ImplicitLayers, DuplicateLayers) { layer2.set_description("actually_layer_2"); layer2.set_make_spurious_log_in_create_instance("actually_layer_2"); - auto layer_props = env.GetLayerProperties(2); + auto layer_props = env.GetLayerProperties(1); ASSERT_TRUE(string_eq(same_layer_name_1, layer_props[0].layerName)); - ASSERT_TRUE(string_eq(same_layer_name_1, layer_props[1].layerName)); ASSERT_TRUE(string_eq(layer1.description.c_str(), layer_props[0].description)); - ASSERT_TRUE(string_eq(layer2.description.c_str(), layer_props[1].description)); InstWrapper inst{env.vulkan_functions}; FillDebugUtilsCreateDetails(inst.create_info, env.debug_log); @@ -996,6 +994,58 @@ TEST(ImplicitLayers, DuplicateLayers) { ASSERT_FALSE(env.debug_log.find("actually_layer_2")); } +TEST(ImplicitLayer, DuplicateLayersWithDifferentExtensions) { + // Put the duplicate layer without the extension first + { + FrameworkEnvironment env{}; + env.add_icd(TEST_ICD_PATH_VERSION_2).setup_WSI(); + + env.add_implicit_layer({}, ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name("VK_LAYER_tesssst") + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2) + .set_disable_environment("delete me"))); + env.add_implicit_layer({}, ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name("VK_LAYER_tesssst") + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2) + .add_instance_extension({"VK_EXT_layer_settings"}) + .set_disable_environment("delete me"))); + env.layers.back().get_test_binary().instance_extensions.push_back("VK_EXT_layer_settings"); + + auto layer_props = env.GetLayerProperties(1); + + auto ext_props = env.GetInstanceExtensions(6); + + InstWrapper inst{env.vulkan_functions}; + inst.create_info.add_extension("VK_EXT_layer_settings"); + inst.CheckCreate(VK_ERROR_EXTENSION_NOT_PRESENT); + } + // Do the same but put the layer with the extension first + { + FrameworkEnvironment env{}; + env.add_icd(TEST_ICD_PATH_VERSION_2).setup_WSI(); + + env.add_implicit_layer({}, ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name("VK_LAYER_tesssst") + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2) + .add_instance_extension({"VK_EXT_layer_settings"}) + .set_disable_environment("delete me"))); + env.layers.back().get_test_binary().instance_extensions.push_back("VK_EXT_layer_settings"); + + env.add_implicit_layer({}, ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name("VK_LAYER_tesssst") + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2) + .set_disable_environment("delete me"))); + + auto layer_props = env.GetLayerProperties(1); + + auto ext_props = env.GetInstanceExtensions(7); + + InstWrapper inst{env.vulkan_functions}; + inst.create_info.add_extension("VK_EXT_layer_settings"); + inst.CheckCreate(); + } +} + TEST(ImplicitLayers, VkImplicitLayerPathEnvVar) { FrameworkEnvironment env; env.add_icd(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA).add_physical_device({}); @@ -1093,11 +1143,9 @@ TEST(ImplicitLayers, DuplicateLayersInVkImplicitLayerPath) { layer2.set_description("actually_layer_2"); env.env_var_vk_implicit_layer_paths.add_to_list(env.get_folder(ManifestLocation::override_layer).location().string()); - auto layer_props = env.GetLayerProperties(2); + auto layer_props = env.GetLayerProperties(1); ASSERT_TRUE(string_eq(layer_name, layer_props[0].layerName)); ASSERT_TRUE(string_eq(layer1.description.c_str(), layer_props[0].description)); - ASSERT_TRUE(string_eq(layer_name, layer_props[1].layerName)); - ASSERT_TRUE(string_eq(layer2.description.c_str(), layer_props[1].description)); EnvVarWrapper inst_layers_env_var{"VK_INSTANCE_LAYERS"}; inst_layers_env_var.add_to_list(layer_name); @@ -1139,11 +1187,9 @@ TEST(ImplicitLayers, DuplicateLayersInVK_ADD_IMPLICIT_LAYER_PATH) { layer2.set_description("actually_layer_2"); layer2.set_make_spurious_log_in_create_instance("actually_layer_2"); - auto layer_props = env.GetLayerProperties(2); + auto layer_props = env.GetLayerProperties(1); ASSERT_TRUE(string_eq(same_layer_name_1, layer_props[0].layerName)); - ASSERT_TRUE(string_eq(same_layer_name_1, layer_props[1].layerName)); ASSERT_TRUE(string_eq(layer1.description.c_str(), layer_props[0].description)); - ASSERT_TRUE(string_eq(layer2.description.c_str(), layer_props[1].description)); InstWrapper inst{env.vulkan_functions}; FillDebugUtilsCreateDetails(inst.create_info, env.debug_log); @@ -2470,11 +2516,9 @@ TEST(ExplicitLayers, DuplicateLayersInVK_LAYER_PATH) { layer2.set_description("actually_layer_2"); layer2.set_make_spurious_log_in_create_instance("actually_layer_2"); - auto layer_props = env.GetLayerProperties(2); + auto layer_props = env.GetLayerProperties(1); ASSERT_TRUE(string_eq(same_layer_name_1, layer_props[0].layerName)); - ASSERT_TRUE(string_eq(same_layer_name_1, layer_props[1].layerName)); ASSERT_TRUE(string_eq(layer1.description.c_str(), layer_props[0].description)); - ASSERT_TRUE(string_eq(layer2.description.c_str(), layer_props[1].description)); { InstWrapper inst{env.vulkan_functions}; inst.create_info.add_layer(same_layer_name_1); @@ -2541,11 +2585,9 @@ TEST(ExplicitLayers, DuplicateLayersInVK_ADD_LAYER_PATH) { layer2.set_description("actually_layer_2"); layer2.set_make_spurious_log_in_create_instance("actually_layer_2"); - auto layer_props = env.GetLayerProperties(2); + auto layer_props = env.GetLayerProperties(1); ASSERT_TRUE(string_eq(same_layer_name_1, layer_props[0].layerName)); - ASSERT_TRUE(string_eq(same_layer_name_1, layer_props[1].layerName)); ASSERT_TRUE(string_eq(layer1.description.c_str(), layer_props[0].description)); - ASSERT_TRUE(string_eq(layer2.description.c_str(), layer_props[1].description)); { InstWrapper inst{env.vulkan_functions}; inst.create_info.add_layer(same_layer_name_1); @@ -2671,11 +2713,9 @@ TEST(ExplicitLayers, DuplicateLayersInVkLayerPath) { layer2.set_description("actually_layer_2"); env.env_var_vk_layer_paths.add_to_list(env.get_folder(ManifestLocation::override_layer).location().string()); - auto layer_props = env.GetLayerProperties(2); + auto layer_props = env.GetLayerProperties(1); ASSERT_TRUE(string_eq(layer_name, layer_props[0].layerName)); ASSERT_TRUE(string_eq(layer1.description.c_str(), layer_props[0].description)); - ASSERT_TRUE(string_eq(layer_name, layer_props[1].layerName)); - ASSERT_TRUE(string_eq(layer2.description.c_str(), layer_props[1].description)); EnvVarWrapper inst_layers_env_var{"VK_INSTANCE_LAYERS"}; inst_layers_env_var.add_to_list(layer_name); diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp index 6650cfb2e..a76960f34 100644 --- a/tests/loader_regression_tests.cpp +++ b/tests/loader_regression_tests.cpp @@ -174,7 +174,7 @@ TEST(EnumerateInstanceLayerProperties, UsageChecks) { env.add_icd(TEST_ICD_PATH_VERSION_2); const char* layer_name_1 = "TestLayer1"; - const char* layer_name_2 = "TestLayer1"; + const char* layer_name_2 = "TestLayer2"; env.add_explicit_layer( {}, ManifestLayer{}.add_layer(