Fix devices in the settings file removing layers

The settings file supports multiple different kind of settings, layers,
devices, and drivers, mixing and matching each kind. This didn't work
when there was only a single boolean to determine if the settings are
active. Adding a separate boolean for layers and devices in the settings
allows for only running settings file logic when the relevant parts of
the settings file are present, so that a settings file with only devices
doesn't interfere with the layer enumeration and vice versa.
Additionally, using booleans instead of just checking if the list of
layer/device settings are non-empty allows the user to specify
"no layers" and "no devices" on the system.

Author: charles-lunarg

loader/loader.c

@@ -4710,7 +4710,7 @@ VkResult loader_enable_instance_layers(struct loader_instance *inst, const VkIns
         goto out;
     }
 
-    if (inst->settings.settings_active) {
+    if (inst->settings.settings_active && inst->settings.layer_configurations_active) {
         res = enable_correct_layers_from_settings(inst, layer_filters, pCreateInfo->enabledLayerCount,
                                                   pCreateInfo->ppEnabledLayerNames, &inst->instance_layer_list,
                                                   &inst->app_activated_layer_list, &inst->expanded_activated_layer_list);
@@ -5571,7 +5571,8 @@ VkResult loader_validate_layers(const struct loader_instance *inst, const uint32
                        "loader_validate_layers: Layer %d does not exist in the list of available layers", i);
             return VK_ERROR_LAYER_NOT_PRESENT;
         }
-        if (inst->settings.settings_active && prop->settings_control_value != LOADER_SETTINGS_LAYER_CONTROL_ON &&
+        if (inst->settings.settings_active && inst->settings.layer_configurations_active &&
+            prop->settings_control_value != LOADER_SETTINGS_LAYER_CONTROL_ON &&
             prop->settings_control_value != LOADER_SETTINGS_LAYER_CONTROL_DEFAULT) {
             loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0,
                        "loader_validate_layers: Layer %d was explicitly prevented from being enabled by the loader settings file",
@@ -5610,7 +5611,7 @@ VkResult loader_validate_instance_extensions(struct loader_instance *inst, const
         goto out;
     }
 
-    if (inst->settings.settings_active) {
+    if (inst->settings.settings_active && inst->settings.layer_configurations_active) {
         res = enable_correct_layers_from_settings(inst, layer_filters, pCreateInfo->enabledLayerCount,
                                                   pCreateInfo->ppEnabledLayerNames, instance_layers, &active_layers,
                                                   &expanded_layers);
@@ -5986,8 +5987,9 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateInstance(const VkInstanceCreateI
         // If the settings file has device_configurations, we need to raise the ApiVersion drivers use to 1.1 if the driver
         // supports 1.1 or higher. This allows 1.0 apps to use the device_configurations without the app having to set its own
         // ApiVersion to 1.1 on its own.
-        if (ptr_instance->settings.settings_active && ptr_instance->settings.device_configuration_count > 0 &&
-            icd_version >= VK_API_VERSION_1_1 && requested_version < VK_API_VERSION_1_1) {
+        if (ptr_instance->settings.settings_active && ptr_instance->settings.device_configurations_active &&
+            ptr_instance->settings.device_configuration_count > 0 && icd_version >= VK_API_VERSION_1_1 &&
+            requested_version < VK_API_VERSION_1_1) {
             if (NULL != pCreateInfo->pApplicationInfo) {
                 memcpy(&icd_app_info, pCreateInfo->pApplicationInfo, sizeof(VkApplicationInfo));
             }
@@ -7114,7 +7116,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumeratePhysicalDevices(VkInstance in
         goto out;
     }
 
-    if (inst->settings.settings_active && inst->settings.device_configuration_count > 0) {
+    if (inst->settings.settings_active && inst->settings.device_configurations_active) {
         // Use settings file device_configurations if present
         if (NULL == pPhysicalDevices) {
             // take the minimum of the settings configurations count and number of terminators

loader/settings.c

@@ -190,6 +190,7 @@ VkResult parse_layer_configurations(const struct loader_instance* inst, cJSON* s
 
     uint32_t layer_configurations_count = loader_cJSON_GetArraySize(layer_configurations);
     if (layer_configurations_count == 0) {
+        loader_settings->layer_configurations_active = true;
         return VK_SUCCESS;
     }
 
@@ -214,6 +215,7 @@ VkResult parse_layer_configurations(const struct loader_instance* inst, cJSON* s
             goto out;
         }
     }
+    loader_settings->layer_configurations_active = true;
 out:
     if (res != VK_SUCCESS) {
         if (loader_settings->layer_configurations) {
@@ -380,6 +382,7 @@ VkResult parse_device_configurations(const struct loader_instance* inst, cJSON*
 
     uint32_t device_configuration_count = loader_cJSON_GetArraySize(device_configurations);
     if (device_configuration_count == 0) {
+        loader_settings->device_configurations_active = true;
         return VK_SUCCESS;
     }
 
@@ -407,6 +410,7 @@ VkResult parse_device_configurations(const struct loader_instance* inst, cJSON*
         }
         i++;
     }
+    loader_settings->device_configurations_active = true;
 out:
     if (res != VK_SUCCESS) {
         if (loader_settings->device_configurations) {
@@ -582,8 +586,10 @@ bool check_if_settings_are_equal(loader_settings* a, loader_settings* b) {
     are_equal &= a->settings_active == b->settings_active;
     are_equal &= a->has_unordered_layer_location == b->has_unordered_layer_location;
     are_equal &= a->debug_level == b->debug_level;
+    are_equal &= a->layer_configurations_active == b->layer_configurations_active;
     are_equal &= a->layer_configuration_count == b->layer_configuration_count;
     are_equal &= a->additional_driver_count == b->additional_driver_count;
+    are_equal &= a->device_configurations_active == b->device_configurations_active;
     are_equal &= a->device_configuration_count == b->device_configuration_count;
     if (!are_equal) return false;
     for (uint32_t i = 0; i < a->layer_configuration_count && i < b->layer_configuration_count; i++) {
@@ -614,18 +620,19 @@ void log_settings(const struct loader_instance* inst, loader_settings* settings)
     if (strlen(cmd_line_msg)) {
         loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Loader Settings Filters for Logging to Standard Error: %s", cmd_line_msg);
     }
-
-    loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Layer Configurations count = %d", settings->layer_configuration_count);
-    for (uint32_t i = 0; i < settings->layer_configuration_count; i++) {
-        loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "---- Layer Configuration [%d] ----", i);
-        if (settings->layer_configurations[i].control != LOADER_SETTINGS_LAYER_UNORDERED_LAYER_LOCATION) {
-            loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Name: %s", settings->layer_configurations[i].name);
-            loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Path: %s", settings->layer_configurations[i].path);
-            loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Layer Type: %s",
-                       settings->layer_configurations[i].treat_as_implicit_manifest ? "Implicit" : "Explicit");
+    if (settings->layer_configurations_active) {
+        loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Layer Configurations count = %d", settings->layer_configuration_count);
+        for (uint32_t i = 0; i < settings->layer_configuration_count; i++) {
+            loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "---- Layer Configuration [%d] ----", i);
+            if (settings->layer_configurations[i].control != LOADER_SETTINGS_LAYER_UNORDERED_LAYER_LOCATION) {
+                loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Name: %s", settings->layer_configurations[i].name);
+                loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Path: %s", settings->layer_configurations[i].path);
+                loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Layer Type: %s",
+                           settings->layer_configurations[i].treat_as_implicit_manifest ? "Implicit" : "Explicit");
+            }
+            loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Control: %s",
+                       loader_settings_layer_control_to_string(settings->layer_configurations[i].control));
         }
-        loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Control: %s",
-                   loader_settings_layer_control_to_string(settings->layer_configurations[i].control));
     }
     if (settings->additional_driver_count > 0) {
         loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "----");
@@ -638,7 +645,7 @@ void log_settings(const struct loader_instance* inst, loader_settings* settings)
             loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Path: %s", settings->additional_drivers[i].path);
         }
     }
-    if (settings->device_configuration_count > 0) {
+    if (settings->device_configurations_active) {
         loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "----");
         loader_log(inst, VULKAN_LOADER_DEBUG_BIT, 0, "Device Configurations count = %d", settings->device_configuration_count);
         for (uint32_t i = 0; i < settings->device_configuration_count; i++) {
@@ -844,8 +851,8 @@ VkResult get_loader_settings(const struct loader_instance* inst, loader_settings
 
     // Only consider the settings active if there is at least one "setting" active.
     // Those are either logging, layers, additional_drivers, or device_configurations.
-    if (loader_settings->debug_level != 0 || loader_settings->layer_configuration_count != 0 ||
-        loader_settings->additional_driver_count != 0 || loader_settings->device_configuration_count != 0) {
+    if (loader_settings->debug_level != 0 || loader_settings->layer_configurations_active ||
+        loader_settings->additional_driver_count != 0 || loader_settings->device_configurations_active) {
         loader_settings->settings_file_path = settings_file_path;
         settings_file_path = NULL;
         loader_settings->settings_active = true;
@@ -925,7 +932,7 @@ TEST_FUNCTION_EXPORT VkResult get_settings_layers(const struct loader_instance*
 
     const loader_settings* settings = get_current_settings_and_lock(inst);
 
-    if (NULL == settings || !settings->settings_active) {
+    if (NULL == settings || !settings->settings_active || !settings->layer_configurations_active) {
         goto out;
     }
 
@@ -1275,7 +1282,7 @@ bool loader_settings_should_use_driver_environment_variables(const struct loader
     if (NULL == settings || !settings->settings_active) {
         goto out;
     }
-    if (settings->device_configuration_count > 0) {
+    if (settings->device_configurations_active) {
         should_use = false;
     }
 out:

loader/settings.h

@@ -79,13 +79,15 @@ typedef struct loader_settings {
     bool has_unordered_layer_location;
     enum vulkan_loader_debug_flags debug_level;
 
+    bool layer_configurations_active;
     uint32_t layer_configuration_count;
     loader_settings_layer_configuration* layer_configurations;
 
     bool additional_drivers_use_exclusively;
     uint32_t additional_driver_count;
     loader_settings_driver_configuration* additional_drivers;
 
+    bool device_configurations_active;
     uint32_t device_configuration_count;
     loader_settings_device_configuration* device_configurations;
 

tests/loader_settings_tests.cpp

@@ -491,6 +491,7 @@ TEST(SettingsFile, ApplicationEnablesIgnored) {
     }
 }
 
+// If the layer list is empty, we want no layers found or loaded
 TEST(SettingsFile, LayerListIsEmpty) {
     FrameworkEnvironment env{};
     env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).add_physical_device({});
@@ -511,15 +512,13 @@ TEST(SettingsFile, LayerListIsEmpty) {
     writer.EndObject();
     env.write_settings_file(writer.output, true);
 
-    auto layer_props = env.GetLayerProperties(1);
-    ASSERT_TRUE(string_eq(layer_props.at(0).layerName, implicit_layer_name));
+    ASSERT_NO_FATAL_FAILURE(env.GetLayerProperties(0));
 
     InstWrapper inst{env.vulkan_functions};
     FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
     inst.CheckCreate();
-    ASSERT_TRUE(env.debug_log.find(get_settings_not_in_use_log_message(env, true)));
-    auto actice_layer_props = inst.GetActiveLayers(inst.GetPhysDev(), 1);
-    ASSERT_TRUE(string_eq(actice_layer_props.at(0).layerName, implicit_layer_name));
+    ASSERT_TRUE(env.debug_log.find(get_settings_location_log_message(env, true)));
+    ASSERT_NO_FATAL_FAILURE(inst.GetActiveLayers(inst.GetPhysDev(), 0));
 }
 
 // If a settings file exists but contains no valid settings - don't consider it
@@ -3663,3 +3662,22 @@ TEST(SettingsFile, DeviceConfigurationReordersExclusiveAdditionalDrivers) {
 
     ASSERT_TRUE(0 == memcmp(vulkan_11_props.deviceUUID, uuids[2].data(), VK_UUID_SIZE * sizeof(uint8_t)));
 }
+TEST(SettingsFile, DeviceConfigurationPreservesLayerEnumeration) {
+    FrameworkEnvironment env{};
+    VulkanUUID uuid{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15};
+
+    const char* layer_name = "VK_LAYER_layer";
+    env.add_explicit_layer(
+        ManifestLayer{}.add_layer(
+            ManifestLayer::LayerDescription{}.set_name(layer_name).set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2)),
+        "regular_test_layer.json");
+
+    env.loader_settings.set_file_format_version({1, 0, 0}).add_app_specific_setting(
+        AppSpecificSettings{}
+            .add_driver_configuration(LoaderSettingsDriverConfiguration{}.set_path("Shouldn't matter"))
+            .add_device_configuration(LoaderSettingsDeviceConfiguration{}.set_deviceUUID(uuid)));
+    env.update_loader_settings(env.loader_settings);
+
+    auto layer_props = env.GetLayerProperties(1);
+    EXPECT_TRUE(string_eq(layer_props.at(0).layerName, layer_name));
+}