From f9743033b67ed7dc1e7978656176820dae75ccf1 Mon Sep 17 00:00:00 2001
From: spencer-lunarg <spencer@lunarg.com>
Date: Tue, 1 Oct 2024 10:51:06 -0400
Subject: [PATCH] layers: Applied feeback

---
 docs/debug_printf.md                                   | 2 +-
 layers/gpu/instrumentation/gpu_shader_instrumentor.cpp | 7 +++----
 layers/gpu/resources/gpuav_subclasses.cpp              | 9 ++++++---
 layers/layer_options.cpp                               | 3 ++-
 layers/layer_options.h                                 | 3 +--
 5 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/docs/debug_printf.md b/docs/debug_printf.md
index d6f88e35cd9..2a9b9001ee6 100644
--- a/docs/debug_printf.md
+++ b/docs/debug_printf.md
@@ -31,7 +31,7 @@ For those who "just need to quick use it" there is also a `set VK_LAYER_PRINTF_O
 
 > All settings are found in Vulkan Configurator (`VkConfig`)
 >
-> Even if you use `VK_LAYER_PRINTF_ONLY_PRESET` you need to set this settings yourself as desired
+> Even if you use `VK_LAYER_PRINTF_ONLY_PRESET` you need to set those settings yourself as desired
 
 There are currently 3 environment variables that are used for settings in Debug Printf
 
diff --git a/layers/gpu/instrumentation/gpu_shader_instrumentor.cpp b/layers/gpu/instrumentation/gpu_shader_instrumentor.cpp
index 91b0ba5e96f..4f68281f39b 100644
--- a/layers/gpu/instrumentation/gpu_shader_instrumentor.cpp
+++ b/layers/gpu/instrumentation/gpu_shader_instrumentor.cpp
@@ -1371,21 +1371,20 @@ bool GpuShaderInstrumentor::InstrumentShader(const vvl::span<const uint32_t> &in
 
     gpu::spirv::Module module(input_spirv, debug_report, module_settings);
 
-    GpuAVSettings::ShaderInstrumentation &shader_instrumentation = gpuav_settings.shader_instrumentation;
     bool modified = false;
 
     // If descriptor indexing is enabled, enable length checks and updated descriptor checks
-    if (shader_instrumentation.bindless_descriptor) {
+    if (gpuav_settings.shader_instrumentation.bindless_descriptor) {
         modified |= module.RunPassBindlessDescriptor();
         modified |= module.RunPassNonBindlessOOBBuffer();
         modified |= module.RunPassNonBindlessOOBTexelBuffer();
     }
 
-    if (shader_instrumentation.buffer_device_address) {
+    if (gpuav_settings.shader_instrumentation.buffer_device_address) {
         modified |= module.RunPassBufferDeviceAddress();
     }
 
-    if (shader_instrumentation.ray_query) {
+    if (gpuav_settings.shader_instrumentation.ray_query) {
         modified |= module.RunPassRayQuery();
     }
 
diff --git a/layers/gpu/resources/gpuav_subclasses.cpp b/layers/gpu/resources/gpuav_subclasses.cpp
index e02b4ba4ac4..12023794f48 100644
--- a/layers/gpu/resources/gpuav_subclasses.cpp
+++ b/layers/gpu/resources/gpuav_subclasses.cpp
@@ -485,10 +485,13 @@ void CommandBuffer::PostProcess(VkQueue queue, const Location &loc) {
         char *data;
 
         VkResult result = vmaMapMemory(gpuav->vma_allocator_, buffer_info.output_mem_block.allocation, (void **)&data);
-        if (result == VK_SUCCESS) {
-            debug_printf::AnalyzeAndGenerateMessage(*gpuav, VkHandle(), queue, buffer_info, (uint32_t *)data, loc);
-            vmaUnmapMemory(gpuav->vma_allocator_, buffer_info.output_mem_block.allocation);
+        if (result != VK_SUCCESS) {
+            gpuav->InternalError(queue, loc, "Unable to map device memory allocated for DebugPrintf buffer.", true);
+            return;
         }
+
+        debug_printf::AnalyzeAndGenerateMessage(*gpuav, VkHandle(), queue, buffer_info, (uint32_t *)data, loc);
+        vmaUnmapMemory(gpuav->vma_allocator_, buffer_info.output_mem_block.allocation);
     }
 
     // CommandBuffer::Destroy can happen on an other thread,
diff --git a/layers/layer_options.cpp b/layers/layer_options.cpp
index d79ab178451..99f17558470 100644
--- a/layers/layer_options.cpp
+++ b/layers/layer_options.cpp
@@ -1016,7 +1016,8 @@ void ProcessConfigAndEnvSettings(ConfigAndEnvSettings *settings_data) {
                              VK_LAYER_VALIDATE_BEST_PRACTICES_NVIDIA);
         SetValidationSetting(layer_setting_set, settings_data->enables, sync_validation, VK_LAYER_VALIDATE_SYNC);
 
-        // Before GPU-AV and DebugPrintf were megered, we used this enum to set GPU-AV and DebugPrintf in vkconfig.
+        // These were deprecated after the 1.3.296 SDK release
+        // Before GPU-AV and DebugPrintf were merged, we used this enum to set GPU-AV and DebugPrintf in vkconfig.
         // This code should in theory be dead since removing it from vkconfig, but keep just incase for a bit
         if (vkuHasLayerSetting(layer_setting_set, DEPRECATED_VK_LAYER_VALIDATE_GPU_BASED)) {
             std::string setting_value;
diff --git a/layers/layer_options.h b/layers/layer_options.h
index 9ecc9174962..94920b85cef 100644
--- a/layers/layer_options.h
+++ b/layers/layer_options.h
@@ -69,8 +69,7 @@ enum EnableFlags {
     vendor_specific_amd,
     vendor_specific_img,
     vendor_specific_nvidia,
-    // DebugPrintf was merged with GPU-AV, so we now just use this funnel into the new settings
-    debug_printf_validation,
+    debug_printf_validation,  // Will set GpuAVSettings::debug_printf_enabled underneath
     sync_validation,
     // Insert new enables above this line
     kMaxEnableFlags,