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(