Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] simpler labels for render target textures and cmd buffers. #55936

Merged
merged 1 commit into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions impeller/core/texture.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 2 additions & 6 deletions impeller/entity/inline_pass_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
{
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions impeller/renderer/backend/gles/reactor_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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> worker) {
Lock lock(workers_mutex_);
auto id = WorkerID{};
Expand Down
7 changes: 7 additions & 0 deletions impeller/renderer/backend/gles/reactor_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<void(const ReactorGLES& reactor)>;

//----------------------------------------------------------------------------
Expand Down
15 changes: 14 additions & 1 deletion impeller/renderer/backend/gles/texture_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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|
Expand Down
3 changes: 3 additions & 0 deletions impeller/renderer/backend/gles/texture_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions impeller/renderer/backend/metal/texture_mtl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
13 changes: 13 additions & 0 deletions impeller/renderer/backend/metal/texture_mtl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "impeller/renderer/backend/metal/texture_mtl.h"
#include <memory>

#include "impeller/base/strings.h"
#include "impeller/base/validation.h"
#include "impeller/core/formats.h"
#include "impeller/core/texture_descriptor.h"
Expand Down Expand Up @@ -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|
Expand Down
13 changes: 13 additions & 0 deletions impeller/renderer/backend/vulkan/context_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -110,6 +111,18 @@ class ContextVK final : public Context,
return SetDebugName(GetDevice(), handle, label);
}

template <typename T>
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 <typename T>
static bool SetDebugName(const vk::Device& device,
T handle,
Expand Down
15 changes: 15 additions & 0 deletions impeller/renderer/backend/vulkan/texture_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,28 @@ TextureVK::TextureVK(std::weak_ptr<Context> 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.
return;
}
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,
Expand Down
3 changes: 3 additions & 0 deletions impeller/renderer/backend/vulkan/texture_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ class TextureVK final : public Texture, public BackendCast<TextureVK, 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,
Expand Down
17 changes: 4 additions & 13 deletions impeller/renderer/render_target.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -351,10 +349,7 @@ RenderTarget RenderTargetAllocator::CreateOffscreenMSAA(
return {};
}
}
#ifdef IMPELLER_DEBUG
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this IFDEFS because the texture setlabel implemenations have the ifdef and we're not string formatting here anymore.

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<Texture> color0_resolve_tex;
Expand All @@ -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.

Expand Down Expand Up @@ -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));
}
Expand Down
4 changes: 4 additions & 0 deletions impeller/renderer/testing/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions lib/ui/painting/image_decoder_no_gl_unittests.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down