diff --git a/layersvt/debug_marker/debug_marker.cpp b/layersvt/debug_marker/debug_marker.cpp index 2857455d9c..45fa52af62 100644 --- a/layersvt/debug_marker/debug_marker.cpp +++ b/layersvt/debug_marker/debug_marker.cpp @@ -24,6 +24,8 @@ namespace { // The packet shares the track event sequence, whose defaults declare an incremental clock, so the // absolute timestamp must carry an explicit clock id to not be read as a delta. +// An empty object_name, emitted by RemoveDebugObjectName, means the object is unnamed from this +// packet's timestamp on. void EmitDebugObjectNamePacket(uint64_t device, int32_t type, uint64_t handle, const std::string& name) { perfetto::TrackEvent::Trace([device, type, handle, &name](perfetto::TrackEvent::TraceContext ctx) { auto packet = ctx.NewTracePacket(); @@ -94,13 +96,25 @@ void DebugMarker::PreDestroyDevice(VkDevice device, const VkAllocationCallbacks* } void DebugMarker::SetDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const char* name) { + if (name == nullptr || name[0] == '\0') { + RemoveDebugObjectName(device, type, handle); + return; + } + std::lock_guard lock(mutex_); - std::string name_str = name ? name : "NULL"; + std::string name_str = name; debug_object_names_[std::make_tuple(device, type, handle)] = DebugObjectName(device, type, handle, name_str); EmitDebugObjectNamePacket(device, type, handle, name_str); } +void DebugMarker::RemoveDebugObjectName(uint64_t device, int32_t type, uint64_t handle) { + std::lock_guard lock(mutex_); + if (debug_object_names_.erase(std::make_tuple(device, type, handle)) == 0) return; + // An empty name tells trace consumers the object is unnamed from here on. + EmitDebugObjectNamePacket(device, type, handle, std::string()); +} + void DebugMarker::EmitAllDebugMarkers() { std::lock_guard lock(mutex_); for (const auto& entry : debug_object_names_) { @@ -109,11 +123,11 @@ void DebugMarker::EmitAllDebugMarkers() { } } -bool DebugMarker::HasDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const std::string& name) { +std::optional DebugMarker::GetDebugObjectName(uint64_t device, int32_t type, uint64_t handle) const { std::lock_guard lock(mutex_); auto it = debug_object_names_.find(std::make_tuple(device, type, handle)); - if (it == debug_object_names_.end()) return false; - return it->second.name == name; + if (it == debug_object_names_.end()) return std::nullopt; + return it->second.name; } VkResult DebugMarker::CreateEmulatedMessenger(VkInstance instance, const VkDebugUtilsMessengerCreateInfoEXT* pCreateInfo, diff --git a/layersvt/debug_marker/debug_marker.h b/layersvt/debug_marker/debug_marker.h index fc811bed88..10e26dc07c 100644 --- a/layersvt/debug_marker/debug_marker.h +++ b/layersvt/debug_marker/debug_marker.h @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -63,24 +64,26 @@ class DebugMarker : public layersvt::LayerBase { } /** - * @brief Sets or updates the name associated with a Vulkan object. - * @param device The handle of the Vulkan device that owns the object. - * @param type The type of the Vulkan object (represented as int32_t). - * @param handle The handle of the Vulkan object. - * @param name The name to associate with the object. + * Sets or updates the name associated with a Vulkan object. A null or empty name + * removes the object's name. */ void SetDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const char* name); /** - * @brief Emits all stored debug markers to the tracing system. + * Removes the name associated with a Vulkan object (no-op if unnamed). + */ + void RemoveDebugObjectName(uint64_t device, int32_t type, uint64_t handle); + + /** + * Emits all stored debug markers to the tracing system. */ void EmitAllDebugMarkers(); /** - * @brief Checks if a debug name is stored for a given object. - * @note This function is for testing only. + * Returns the name stored for a Vulkan object, or std::nullopt if the object has no name. + * For testing only. */ - bool HasDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const std::string& name); + std::optional GetDebugObjectName(uint64_t device, int32_t type, uint64_t handle) const; /** * @brief Emulated VK_EXT_debug_utils messengers, used for instances whose downstream lacks the @@ -127,7 +130,7 @@ class DebugMarker : public layersvt::LayerBase { : vk_device(dev), object_type(type), handle(h), name(n) {} }; - std::mutex mutex_; + mutable std::mutex mutex_; /** * @brief Maps (vk_device, object_type, object_handle) to its debug name information. * Non-dispatchable handles are not guaranteed to be unique across object types or devices. diff --git a/layersvt/test/test_debugmarker.cpp b/layersvt/test/test_debugmarker.cpp index 62e11549cd..3605bf4c1b 100644 --- a/layersvt/test/test_debugmarker.cpp +++ b/layersvt/test/test_debugmarker.cpp @@ -60,16 +60,15 @@ TEST_F(DebugMarkerTests, CombinedTest) { // 1. Set instance name DebugMarker::Get().SetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance), "MyInstance"); // 2. Override new name DebugMarker::Get().SetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed")); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance), "MyInstanceRenamed"); // 3. Reset layer_test::ResetLayer(); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance), std::nullopt); } TEST_F(DebugMarkerTests, ManifestTest) { @@ -160,18 +159,39 @@ TEST_F(DebugMarkerTests, PreDestroyDeviceCleanupTest) { DebugMarker::Get().SetDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1"); DebugMarker::Get().SetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1")); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111), "Buffer1"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222), "Buffer2"); // Destroy dev1 - should remove Buffer1 but keep Buffer2 layersvt::LayerBaseTestPeer::DestroyDevice(dev1, nullptr); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1")); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111), std::nullopt); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222), "Buffer2"); // Destroy dev2 - should remove Buffer2 layersvt::LayerBaseTestPeer::DestroyDevice(dev2, nullptr); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222), std::nullopt); +} + +TEST_F(DebugMarkerTests, NullOrEmptyNameRemovesNameTest) { + TEST_DESCRIPTION("Verify a null or empty object name removes the stored name instead of storing it"); + + const uint64_t device = 0x1000; + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x1, "Buffer"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x1), "Buffer"); + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x1, nullptr); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x1), std::nullopt); + + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x2, "Image"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x2), "Image"); + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x2, ""); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x2), std::nullopt); + + // Removing the name of an object that was never named leaves other names untouched. + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x3, "Other"); + DebugMarker::Get().RemoveDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x4); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x3), "Other"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x4), std::nullopt); } TEST_F(DebugMarkerTests, SameHandleOnDifferentDevicesTest) { @@ -182,8 +202,8 @@ TEST_F(DebugMarkerTests, SameHandleOnDifferentDevicesTest) { DebugMarker::Get().SetDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1, "Device1Buffer"); DebugMarker::Get().SetDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1, "Device1Buffer")); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1), "Device1Buffer"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1), "Device2Buffer"); } TEST_F(DebugMarkerTests, TemplateMethodDispatchTest) { @@ -265,7 +285,7 @@ TEST_F(DebugMarkerTests, MissingDownstreamExtensionTest) { name_info.objectHandle = 0x4444; name_info.pObjectName = "Buffer"; EXPECT_EQ(set_object_name(mock_device, &name_info), VK_SUCCESS); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)mock_device, VK_OBJECT_TYPE_BUFFER, 0x4444, "Buffer")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)mock_device, VK_OBJECT_TYPE_BUFFER, 0x4444), "Buffer"); } namespace {