From 62629046c595e3a5662c6bad87ca1dc936426a30 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 17 Oct 2024 14:24:03 -0700 Subject: [PATCH] [Impeller] simpler labels for render target textures. --- impeller/core/texture.h | 9 +++++++++ impeller/entity/inline_pass_context.cc | 8 ++------ impeller/renderer/backend/gles/reactor_gles.cc | 4 ++++ impeller/renderer/backend/gles/reactor_gles.h | 7 +++++++ impeller/renderer/backend/gles/texture_gles.cc | 15 ++++++++++++++- impeller/renderer/backend/gles/texture_gles.h | 3 +++ impeller/renderer/backend/metal/texture_mtl.h | 3 +++ impeller/renderer/backend/metal/texture_mtl.mm | 13 +++++++++++++ impeller/renderer/backend/vulkan/context_vk.h | 13 +++++++++++++ impeller/renderer/backend/vulkan/texture_vk.cc | 15 +++++++++++++++ impeller/renderer/backend/vulkan/texture_vk.h | 3 +++ impeller/renderer/render_target.cc | 17 ++++------------- impeller/renderer/testing/mocks.h | 4 ++++ lib/ui/painting/image_decoder_no_gl_unittests.h | 1 + 14 files changed, 95 insertions(+), 20 deletions(-) diff --git a/impeller/core/texture.h b/impeller/core/texture.h index 3be29ae71ca14..d0d24c6dacdbe 100644 --- a/impeller/core/texture.h +++ b/impeller/core/texture.h @@ -18,8 +18,17 @@ class Texture { public: virtual ~Texture(); + /// @brief Label this resource for inspection in GPU debugging tools. + /// + /// This functionality may be disabled in release builds. virtual void SetLabel(std::string_view label) = 0; + /// @brief Label this resource for inspection in GPU debugging tools, with + /// label and trailing will be concatenated together. + /// + /// This functionality may be disabled in release builds. + virtual void SetLabel(std::string_view label, std::string_view trailing) = 0; + // Deprecated: use BlitPass::AddCopy instead. [[nodiscard]] bool SetContents(const uint8_t* contents, size_t length, diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index 1a1e2a92d88e5..8b5f7fda54e18 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -112,9 +112,7 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass( return {}; } - command_buffer_->SetLabel( - "EntityPass Command Buffer: Depth=" + std::to_string(pass_depth) + - " Count=" + std::to_string(pass_count_)); + command_buffer_->SetLabel("EntityPass Command Buffer"); RenderPassResult result; { @@ -183,9 +181,7 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass( // buffer while encoding can add a surprising amount of overhead. We make a // conservative npot estimate to avoid this case. pass_->ReserveCommands(Allocation::NextPowerOfTwoSize(entity_count_)); - pass_->SetLabel( - "EntityPass Render Pass: Depth=" + std::to_string(pass_depth) + - " Count=" + std::to_string(pass_count_)); + pass_->SetLabel("EntityPass Render Pass"); result.pass = pass_; result.just_created = true; diff --git a/impeller/renderer/backend/gles/reactor_gles.cc b/impeller/renderer/backend/gles/reactor_gles.cc index d8b7cb694d3c5..544c1f1a4fdd0 100644 --- a/impeller/renderer/backend/gles/reactor_gles.cc +++ b/impeller/renderer/backend/gles/reactor_gles.cc @@ -28,6 +28,10 @@ bool ReactorGLES::IsValid() const { return is_valid_; } +bool ReactorGLES::CanSetDebugLabels() const { + return can_set_debug_labels_; +} + ReactorGLES::WorkerID ReactorGLES::AddWorker(std::weak_ptr worker) { Lock lock(workers_mutex_); auto id = WorkerID{}; diff --git a/impeller/renderer/backend/gles/reactor_gles.h b/impeller/renderer/backend/gles/reactor_gles.h index a2e3ce66fa063..fa727c0869466 100644 --- a/impeller/renderer/backend/gles/reactor_gles.h +++ b/impeller/renderer/backend/gles/reactor_gles.h @@ -192,6 +192,13 @@ class ReactorGLES { /// void SetDebugLabel(const HandleGLES& handle, std::string_view label); + //---------------------------------------------------------------------------- + /// @brief Whether the device is capable of writing debug labels. + /// + /// This function is useful for short circuiting expensive debug + /// labeling. + bool CanSetDebugLabels() const; + using Operation = std::function; //---------------------------------------------------------------------------- diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc index c0a8678998de2..f2167de515c08 100644 --- a/impeller/renderer/backend/gles/texture_gles.cc +++ b/impeller/renderer/backend/gles/texture_gles.cc @@ -11,6 +11,7 @@ #include "flutter/fml/mapping.h" #include "flutter/fml/trace_event.h" #include "impeller/base/allocation.h" +#include "impeller/base/strings.h" #include "impeller/base/validation.h" #include "impeller/core/formats.h" #include "impeller/core/texture_descriptor.h" @@ -207,7 +208,19 @@ bool TextureGLES::IsValid() const { // |Texture| void TextureGLES::SetLabel(std::string_view label) { - reactor_->SetDebugLabel(handle_, std::string{label.data(), label.size()}); +#ifdef IMPELLER_DEBUG + reactor_->SetDebugLabel(handle_, label); +#endif // IMPELLER_DEBUG +} + +// |Texture| +void TextureGLES::SetLabel(std::string_view label, std::string_view trailing) { +#ifdef IMPELLER_DEBUG + if (reactor_->CanSetDebugLabels()) { + reactor_->SetDebugLabel(handle_, + SPrintF("%s %s", label.data(), trailing.data())); + } +#endif // IMPELLER_DEBUG } // |Texture| diff --git a/impeller/renderer/backend/gles/texture_gles.h b/impeller/renderer/backend/gles/texture_gles.h index c799eb44e6868..05ad1d952c11b 100644 --- a/impeller/renderer/backend/gles/texture_gles.h +++ b/impeller/renderer/backend/gles/texture_gles.h @@ -94,6 +94,9 @@ class TextureGLES final : public Texture, // |Texture| void SetLabel(std::string_view label) override; + // |Texture| + void SetLabel(std::string_view label, std::string_view trailing) override; + // |Texture| bool OnSetContents(const uint8_t* contents, size_t length, diff --git a/impeller/renderer/backend/metal/texture_mtl.h b/impeller/renderer/backend/metal/texture_mtl.h index 6fedd97d0d837..3651c0bdc6057 100644 --- a/impeller/renderer/backend/metal/texture_mtl.h +++ b/impeller/renderer/backend/metal/texture_mtl.h @@ -66,6 +66,9 @@ class TextureMTL final : public Texture, // |Texture| void SetLabel(std::string_view label) override; + // |Texture| + void SetLabel(std::string_view label, std::string_view trailing) override; + // |Texture| bool OnSetContents(const uint8_t* contents, size_t length, diff --git a/impeller/renderer/backend/metal/texture_mtl.mm b/impeller/renderer/backend/metal/texture_mtl.mm index 2f21c324b5eeb..4aebc3190c78b 100644 --- a/impeller/renderer/backend/metal/texture_mtl.mm +++ b/impeller/renderer/backend/metal/texture_mtl.mm @@ -5,6 +5,7 @@ #include "impeller/renderer/backend/metal/texture_mtl.h" #include +#include "impeller/base/strings.h" #include "impeller/base/validation.h" #include "impeller/core/formats.h" #include "impeller/core/texture_descriptor.h" @@ -73,10 +74,22 @@ new TextureMTL( } void TextureMTL::SetLabel(std::string_view label) { +#ifdef IMPELLER_DEBUG if (is_drawable_) { return; } [aquire_proc_() setLabel:@(label.data())]; +#endif // IMPELLER_DEBUG +} + +void TextureMTL::SetLabel(std::string_view label, std::string_view trailing) { +#ifdef IMPELLER_DEBUG + if (is_drawable_) { + return; + } + std::string combined = SPrintF("%s %s", label.data(), trailing.data()); + [aquire_proc_() setLabel:@(combined.data())]; +#endif // IMPELLER_DEBUG } // |Texture| diff --git a/impeller/renderer/backend/vulkan/context_vk.h b/impeller/renderer/backend/vulkan/context_vk.h index fe7f4da002f91..4cb1233f5a488 100644 --- a/impeller/renderer/backend/vulkan/context_vk.h +++ b/impeller/renderer/backend/vulkan/context_vk.h @@ -12,6 +12,7 @@ #include "flutter/fml/unique_fd.h" #include "fml/thread.h" #include "impeller/base/backend_cast.h" +#include "impeller/base/strings.h" #include "impeller/core/formats.h" #include "impeller/renderer/backend/vulkan/command_pool_vk.h" #include "impeller/renderer/backend/vulkan/device_holder_vk.h" @@ -110,6 +111,18 @@ class ContextVK final : public Context, return SetDebugName(GetDevice(), handle, label); } + template + bool SetDebugName(T handle, + std::string_view label, + std::string_view trailing) const { + if (!HasValidationLayers()) { + // No-op if validation layers are not enabled. + return true; + } + std::string combined = SPrintF("%s %s", label.data(), trailing.data()); + return SetDebugName(GetDevice(), handle, combined); + } + template static bool SetDebugName(const vk::Device& device, T handle, diff --git a/impeller/renderer/backend/vulkan/texture_vk.cc b/impeller/renderer/backend/vulkan/texture_vk.cc index 7285e4c63faa6..28668b6d3f667 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.cc +++ b/impeller/renderer/backend/vulkan/texture_vk.cc @@ -19,6 +19,7 @@ TextureVK::TextureVK(std::weak_ptr context, TextureVK::~TextureVK() = default; void TextureVK::SetLabel(std::string_view label) { +#ifdef IMPELLER_DEBUG auto context = context_.lock(); if (!context) { // The context may have died. @@ -26,6 +27,20 @@ void TextureVK::SetLabel(std::string_view label) { } ContextVK::Cast(*context).SetDebugName(GetImage(), label); ContextVK::Cast(*context).SetDebugName(GetImageView(), label); +#endif // IMPELLER_DEBUG +} + +void TextureVK::SetLabel(std::string_view label, std::string_view trailing) { +#ifdef IMPELLER_DEBUG + auto context = context_.lock(); + if (!context) { + // The context may have died. + return; + } + + ContextVK::Cast(*context).SetDebugName(GetImage(), label, trailing); + ContextVK::Cast(*context).SetDebugName(GetImageView(), label, trailing); +#endif // IMPELLER_DEBUG } bool TextureVK::OnSetContents(const uint8_t* contents, diff --git a/impeller/renderer/backend/vulkan/texture_vk.h b/impeller/renderer/backend/vulkan/texture_vk.h index fe243dc3e6f31..b608a26cc88af 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.h +++ b/impeller/renderer/backend/vulkan/texture_vk.h @@ -81,6 +81,9 @@ class TextureVK final : public Texture, public BackendCast { // |Texture| void SetLabel(std::string_view label) override; + // |Texture| + void SetLabel(std::string_view label, std::string_view trailing) override; + // |Texture| bool OnSetContents(const uint8_t* contents, size_t length, diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index 389ecd24ebf89..dd70f9f202a9d 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -289,9 +289,7 @@ RenderTarget RenderTargetAllocator::CreateOffscreen( return {}; } } -#ifdef IMPELLER_DEBUG - color0_tex->SetLabel(SPrintF("%s Color Texture", label.data())); -#endif // IMPELLER_DEBUG + color0_tex->SetLabel(label, "Color Texture"); ColorAttachment color0; color0.clear_color = color_attachment_config.clear_color; @@ -351,10 +349,7 @@ RenderTarget RenderTargetAllocator::CreateOffscreenMSAA( return {}; } } -#ifdef IMPELLER_DEBUG - color0_msaa_tex->SetLabel( - SPrintF("%s Color Texture (Multisample)", label.data())); -#endif // IMPELLER_DEBUG + color0_msaa_tex->SetLabel(label, "Color Texture (Multisample)"); // Create color resolve texture. std::shared_ptr color0_resolve_tex; @@ -376,9 +371,7 @@ RenderTarget RenderTargetAllocator::CreateOffscreenMSAA( return {}; } } -#ifdef IMPELLER_DEBUG - color0_resolve_tex->SetLabel(SPrintF("%s Color Texture", label.data())); -#endif // IMPELLER_DEBUG + color0_resolve_tex->SetLabel(label, "Color Texture"); // Color attachment. @@ -456,10 +449,8 @@ void RenderTarget::SetupDepthStencilAttachments( stencil0.store_action = stencil_attachment_config.store_action; stencil0.clear_stencil = 0u; stencil0.texture = std::move(depth_stencil_texture); + stencil0.texture->SetLabel(label, "Depth+Stencil Texture"); -#ifdef IMPELLER_DEBUG - stencil0.texture->SetLabel(SPrintF("%s Depth+Stencil Texture", label.data())); -#endif // IMPELLER_DEBUG SetDepthAttachment(std::move(depth0)); SetStencilAttachment(std::move(stencil0)); } diff --git a/impeller/renderer/testing/mocks.h b/impeller/renderer/testing/mocks.h index 1e02636bcd7bf..fd4172134b4db 100644 --- a/impeller/renderer/testing/mocks.h +++ b/impeller/renderer/testing/mocks.h @@ -189,6 +189,10 @@ class MockTexture : public Texture { public: explicit MockTexture(const TextureDescriptor& desc) : Texture(desc) {} MOCK_METHOD(void, SetLabel, (std::string_view label), (override)); + MOCK_METHOD(void, + SetLabel, + (std::string_view label, std::string_view trailing), + (override)); MOCK_METHOD(bool, IsValid, (), (const, override)); MOCK_METHOD(ISize, GetSize, (), (const, override)); MOCK_METHOD(bool, diff --git a/lib/ui/painting/image_decoder_no_gl_unittests.h b/lib/ui/painting/image_decoder_no_gl_unittests.h index 4f4cdf2d2732c..6338873260702 100644 --- a/lib/ui/painting/image_decoder_no_gl_unittests.h +++ b/lib/ui/painting/image_decoder_no_gl_unittests.h @@ -22,6 +22,7 @@ class TestImpellerTexture : public Texture { explicit TestImpellerTexture(TextureDescriptor desc) : Texture(desc) {} void SetLabel(std::string_view label) override {} + void SetLabel(std::string_view label, std::string_view trailing) override {} bool IsValid() const override { return true; } ISize GetSize() const { return GetTextureDescriptor().size; }