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] make labeling APIs exclusively use std::string_view. #55918

Merged
merged 3 commits into from
Oct 17, 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
4 changes: 2 additions & 2 deletions impeller/core/device_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ class DeviceBuffer {
Range source_range,
size_t offset = 0u);

virtual bool SetLabel(const std::string& label) = 0;
virtual bool SetLabel(std::string_view label) = 0;

virtual bool SetLabel(const std::string& label, Range range) = 0;
virtual bool SetLabel(std::string_view label, Range range) = 0;

/// @brief Create a buffer view of this entire buffer.
static BufferView AsBufferView(std::shared_ptr<DeviceBuffer> buffer);
Expand Down
4 changes: 0 additions & 4 deletions impeller/core/host_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,6 @@ HostBuffer::HostBuffer(const std::shared_ptr<Allocator>& allocator)

HostBuffer::~HostBuffer() = default;

void HostBuffer::SetLabel(std::string label) {
label_ = std::move(label);
}

BufferView HostBuffer::Emplace(const void* buffer,
size_t length,
size_t align) {
Expand Down
6 changes: 1 addition & 5 deletions impeller/core/host_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@ class HostBuffer {
static std::shared_ptr<HostBuffer> Create(
const std::shared_ptr<Allocator>& allocator);

// |Buffer|
virtual ~HostBuffer();
Copy link
Member Author

Choose a reason for hiding this comment

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

This label was never set


void SetLabel(std::string label);
~HostBuffer();

//----------------------------------------------------------------------------
/// @brief Emplace uniform data onto the host buffer. Ensure that backend
Expand Down Expand Up @@ -169,7 +166,6 @@ class HostBuffer {
size_t current_buffer_ = 0u;
size_t offset_ = 0u;
size_t frame_index_ = 0u;
std::string label_;
};

} // namespace impeller
Expand Down
4 changes: 2 additions & 2 deletions impeller/display_list/aiks_dl_basic_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ TEST_P(AiksTest, CanRenderColorFilterWithInvertColorsDrawPaint) {
namespace {
bool GenerateMipmap(const std::shared_ptr<Context>& context,
std::shared_ptr<Texture> texture,
std::string label) {
std::string_view label) {
auto buffer = context->CreateCommandBuffer();
if (!buffer) {
return false;
Expand All @@ -103,7 +103,7 @@ bool GenerateMipmap(const std::shared_ptr<Context>& context,
if (!pass) {
return false;
}
pass->GenerateMipmap(std::move(texture), std::move(label));
pass->GenerateMipmap(std::move(texture), label);

pass->EncodeCommands(context->GetResourceAllocator());
return context->GetCommandQueue()->Submit({buffer}).ok();
Expand Down
6 changes: 2 additions & 4 deletions impeller/entity/contents/atlas_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,7 @@ bool AtlasContents::Render(const ContentContext& renderer,
using FS = PorterDuffBlendPipeline::FragmentShader;

#ifdef IMPELLER_DEBUG
pass.SetCommandLabel(
SPrintF("DrawAtlas Blend (%s)", BlendModeToString(blend_mode)));
jtmcdole marked this conversation as resolved.
Show resolved Hide resolved
pass.SetCommandLabel("DrawAtlas Blend");
#endif // IMPELLER_DEBUG
pass.SetVertexBuffer(geometry_->CreateBlendVertexBuffer(host_buffer));
pass.SetPipeline(
Expand Down Expand Up @@ -138,8 +137,7 @@ bool AtlasContents::Render(const ContentContext& renderer,
using FS = VerticesUberShader::FragmentShader;

#ifdef IMPELLER_DEBUG
pass.SetCommandLabel(
SPrintF("DrawAtlas Advanced Blend (%s)", BlendModeToString(blend_mode)));
pass.SetCommandLabel("DrawAtlas Advanced Blend");
#endif // IMPELLER_DEBUG
pass.SetVertexBuffer(geometry_->CreateBlendVertexBuffer(host_buffer));

Expand Down
6 changes: 3 additions & 3 deletions impeller/entity/contents/content_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -493,12 +493,12 @@ fml::StatusOr<RenderTarget> ContentContext::MakeSubpass(
if (context->GetCapabilities()->SupportsOffscreenMSAA() && msaa_enabled) {
subpass_target = GetRenderTargetCache()->CreateOffscreenMSAA(
*context, texture_size,
/*mip_count=*/mip_count, SPrintF("%s Offscreen", label.data()),
/*mip_count=*/mip_count, label,
RenderTarget::kDefaultColorAttachmentConfigMSAA, depth_stencil_config);
} else {
subpass_target = GetRenderTargetCache()->CreateOffscreen(
*context, texture_size,
/*mip_count=*/mip_count, SPrintF("%s Offscreen", label.data()),
/*mip_count=*/mip_count, label,
RenderTarget::kDefaultColorAttachmentConfig, depth_stencil_config);
}
return MakeSubpass(label, subpass_target, command_buffer, subpass_callback);
Expand All @@ -520,7 +520,7 @@ fml::StatusOr<RenderTarget> ContentContext::MakeSubpass(
if (!sub_renderpass) {
return fml::Status(fml::StatusCode::kUnknown, "");
}
sub_renderpass->SetLabel(SPrintF("%s RenderPass", label.data()));
sub_renderpass->SetLabel(label);

if (!subpass_callback(*this, *sub_renderpass)) {
return fml::Status(fml::StatusCode::kUnknown, "");
Expand Down
2 changes: 1 addition & 1 deletion impeller/entity/contents/content_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,7 @@ class ContentContext {
PipelineDescriptor& desc) {
opts.ApplyToPipelineDescriptor(desc);
desc.SetLabel(
SPrintF("%s V#%zu", desc.GetLabel().c_str(), variants_count));
SPrintF("%s V#%zu", desc.GetLabel().data(), variants_count));
});
std::unique_ptr<RenderPipelineHandleT> variant =
std::make_unique<RenderPipelineHandleT>(std::move(variant_future));
Expand Down
2 changes: 1 addition & 1 deletion impeller/entity/contents/test/recording_render_pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ fml::Status RecordingRenderPass::Draw() {
}

// |RenderPass|
void RecordingRenderPass::OnSetLabel(std::string label) {
void RecordingRenderPass::OnSetLabel(std::string_view label) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion impeller/entity/contents/test/recording_render_pass.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class RecordingRenderPass : public RenderPass {
const std::unique_ptr<const Sampler>& sampler) override;

// |RenderPass|
void OnSetLabel(std::string label) override;
void OnSetLabel(std::string_view label) override;

// |RenderPass|
bool OnEncodeCommands(const Context& context) const override;
Expand Down
4 changes: 2 additions & 2 deletions impeller/entity/contents/texture_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ std::shared_ptr<TextureContents> TextureContents::MakeRect(Rect destination) {
return contents;
}

void TextureContents::SetLabel(std::string label) {
label_ = std::move(label);
void TextureContents::SetLabel(std::string_view label) {
label_ = label;
}

void TextureContents::SetDestinationRect(Rect rect) {
Expand Down
2 changes: 1 addition & 1 deletion impeller/entity/contents/texture_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class TextureContents final : public Contents {
/// when image filters are applied.
static std::shared_ptr<TextureContents> MakeRect(Rect destination);

void SetLabel(std::string label);
void SetLabel(std::string_view label);

void SetDestinationRect(Rect rect);

Expand Down
4 changes: 2 additions & 2 deletions impeller/entity/render_target_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ RenderTarget RenderTargetCache::CreateOffscreen(
const Context& context,
ISize size,
int mip_count,
const std::string& label,
std::string_view label,
RenderTarget::AttachmentConfig color_attachment_config,
std::optional<RenderTarget::AttachmentConfig> stencil_attachment_config,
const std::shared_ptr<Texture>& existing_color_texture,
Expand Down Expand Up @@ -79,7 +79,7 @@ RenderTarget RenderTargetCache::CreateOffscreenMSAA(
const Context& context,
ISize size,
int mip_count,
const std::string& label,
std::string_view label,
RenderTarget::AttachmentConfigMSAA color_attachment_config,
std::optional<RenderTarget::AttachmentConfig> stencil_attachment_config,
const std::shared_ptr<Texture>& existing_color_msaa_texture,
Expand Down
5 changes: 3 additions & 2 deletions impeller/entity/render_target_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef FLUTTER_IMPELLER_ENTITY_RENDER_TARGET_CACHE_H_
#define FLUTTER_IMPELLER_ENTITY_RENDER_TARGET_CACHE_H_

#include <string_view>
#include "impeller/renderer/render_target.h"

namespace impeller {
Expand All @@ -29,7 +30,7 @@ class RenderTargetCache : public RenderTargetAllocator {
const Context& context,
ISize size,
int mip_count,
const std::string& label = "Offscreen",
std::string_view label = "Offscreen",
RenderTarget::AttachmentConfig color_attachment_config =
RenderTarget::kDefaultColorAttachmentConfig,
std::optional<RenderTarget::AttachmentConfig> stencil_attachment_config =
Expand All @@ -42,7 +43,7 @@ class RenderTargetCache : public RenderTargetAllocator {
const Context& context,
ISize size,
int mip_count,
const std::string& label = "Offscreen MSAA",
std::string_view label = "Offscreen MSAA",
RenderTarget::AttachmentConfigMSAA color_attachment_config =
RenderTarget::kDefaultColorAttachmentConfigMSAA,
std::optional<RenderTarget::AttachmentConfig> stencil_attachment_config =
Expand Down
12 changes: 6 additions & 6 deletions impeller/renderer/backend/gles/blit_pass_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ bool BlitPassGLES::IsValid() const {
}

// |BlitPass|
void BlitPassGLES::OnSetLabel(std::string label) {
label_ = std::move(label);
void BlitPassGLES::OnSetLabel(std::string_view label) {
label_ = std::string(label);
}

[[nodiscard]] bool EncodeCommandsInReactor(
Expand Down Expand Up @@ -96,7 +96,7 @@ bool BlitPassGLES::OnCopyTextureToTextureCommand(
std::shared_ptr<Texture> destination,
IRect source_region,
IPoint destination_origin,
std::string label) {
std::string_view label) {
auto command = std::make_unique<BlitCopyTextureToTextureCommandGLES>();
command->label = label;
command->source = std::move(source);
Expand All @@ -114,7 +114,7 @@ bool BlitPassGLES::OnCopyTextureToBufferCommand(
std::shared_ptr<DeviceBuffer> destination,
IRect source_region,
size_t destination_offset,
std::string label) {
std::string_view label) {
auto command = std::make_unique<BlitCopyTextureToBufferCommandGLES>();
command->label = label;
command->source = std::move(source);
Expand All @@ -131,7 +131,7 @@ bool BlitPassGLES::OnCopyBufferToTextureCommand(
BufferView source,
std::shared_ptr<Texture> destination,
IRect destination_region,
std::string label,
std::string_view label,
uint32_t mip_level,
uint32_t slice,
bool convert_to_read) {
Expand All @@ -150,7 +150,7 @@ bool BlitPassGLES::OnCopyBufferToTextureCommand(

// |BlitPass|
bool BlitPassGLES::OnGenerateMipmapCommand(std::shared_ptr<Texture> texture,
std::string label) {
std::string_view label) {
auto command = std::make_unique<BlitGenerateMipmapCommandGLES>();
command->label = label;
command->texture = std::move(texture);
Expand Down
10 changes: 5 additions & 5 deletions impeller/renderer/backend/gles/blit_pass_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class BlitPassGLES final : public BlitPass,
bool IsValid() const override;

// |BlitPass|
void OnSetLabel(std::string label) override;
void OnSetLabel(std::string_view label) override;

// |BlitPass|
bool EncodeCommands(
Expand All @@ -50,27 +50,27 @@ class BlitPassGLES final : public BlitPass,
std::shared_ptr<Texture> destination,
IRect source_region,
IPoint destination_origin,
std::string label) override;
std::string_view label) override;

// |BlitPass|
bool OnCopyTextureToBufferCommand(std::shared_ptr<Texture> source,
std::shared_ptr<DeviceBuffer> destination,
IRect source_region,
size_t destination_offset,
std::string label) override;
std::string_view label) override;

// |BlitPass|
bool OnCopyBufferToTextureCommand(BufferView source,
std::shared_ptr<Texture> destination,
IRect destination_region,
std::string label,
std::string_view label,
uint32_t mip_level,
uint32_t slice,
bool convert_to_read) override;

// |BlitPass|
bool OnGenerateMipmapCommand(std::shared_ptr<Texture> texture,
std::string label) override;
std::string_view label) override;

BlitPassGLES(const BlitPassGLES&) = delete;

Expand Down
2 changes: 1 addition & 1 deletion impeller/renderer/backend/gles/command_buffer_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ CommandBufferGLES::CommandBufferGLES(std::weak_ptr<const Context> context,
CommandBufferGLES::~CommandBufferGLES() = default;

// |CommandBuffer|
void CommandBufferGLES::SetLabel(const std::string& label) const {
void CommandBufferGLES::SetLabel(std::string_view label) const {
// Cannot support.
}

Expand Down
2 changes: 1 addition & 1 deletion impeller/renderer/backend/gles/command_buffer_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class CommandBufferGLES final : public CommandBuffer {
ReactorGLES::Ref reactor);

// |CommandBuffer|
void SetLabel(const std::string& label) const override;
void SetLabel(std::string_view label) const override;

// |CommandBuffer|
bool IsValid() const override;
Expand Down
6 changes: 4 additions & 2 deletions impeller/renderer/backend/gles/device_buffer_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,15 @@ bool DeviceBufferGLES::BindAndUploadDataIfNecessary(BindingType type) const {
}

// |DeviceBuffer|
bool DeviceBufferGLES::SetLabel(const std::string& label) {
bool DeviceBufferGLES::SetLabel(std::string_view label) {
#ifdef IMPELLER_DEBUG
reactor_->SetDebugLabel(handle_, label);
#endif // IMPELLER_DEBUG
return true;
}

// |DeviceBuffer|
bool DeviceBufferGLES::SetLabel(const std::string& label, Range range) {
bool DeviceBufferGLES::SetLabel(std::string_view label, Range range) {
// Cannot support debug label on the range. Set the label for the entire
// range.
return SetLabel(label);
Expand Down
4 changes: 2 additions & 2 deletions impeller/renderer/backend/gles/device_buffer_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ class DeviceBufferGLES final
size_t offset) override;

// |DeviceBuffer|
bool SetLabel(const std::string& label) override;
bool SetLabel(std::string_view label) override;

// |DeviceBuffer|
bool SetLabel(const std::string& label, Range range) override;
bool SetLabel(std::string_view label, Range range) override;

DeviceBufferGLES(const DeviceBufferGLES&) = delete;

Expand Down
9 changes: 4 additions & 5 deletions impeller/renderer/backend/gles/pipeline_library_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ static std::string GetShaderSource(const ProcTableGLES& gl, GLuint shader) {

static void LogShaderCompilationFailure(const ProcTableGLES& gl,
GLuint shader,
const std::string& name,
std::string_view name,
const fml::Mapping& source_mapping,
ShaderStage stage) {
std::stringstream stream;
Expand Down Expand Up @@ -99,10 +99,9 @@ static bool LinkProgram(
}

gl.SetDebugLabel(DebugResourceType::kShader, vert_shader,
SPrintF("%s Vertex Shader", descriptor.GetLabel().c_str()));
gl.SetDebugLabel(
DebugResourceType::kShader, frag_shader,
SPrintF("%s Fragment Shader", descriptor.GetLabel().c_str()));
SPrintF("%s Vertex Shader", descriptor.GetLabel().data()));
gl.SetDebugLabel(DebugResourceType::kShader, frag_shader,
SPrintF("%s Fragment Shader", descriptor.GetLabel().data()));

fml::ScopedCleanupClosure delete_vert_shader(
[&gl, vert_shader]() { gl.DeleteShader(vert_shader); });
Expand Down
5 changes: 3 additions & 2 deletions impeller/renderer/backend/gles/reactor_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,8 @@ void ReactorGLES::SetupDebugGroups() {
}
}

void ReactorGLES::SetDebugLabel(const HandleGLES& handle, std::string label) {
void ReactorGLES::SetDebugLabel(const HandleGLES& handle,
std::string_view label) {
if (!can_set_debug_labels_) {
return;
}
Expand All @@ -284,7 +285,7 @@ void ReactorGLES::SetDebugLabel(const HandleGLES& handle, std::string label) {
}
WriterLock handles_lock(handles_mutex_);
if (auto found = handles_.find(handle); found != handles_.end()) {
found->second.pending_debug_label = std::move(label);
found->second.pending_debug_label = label;
}
}

Expand Down
2 changes: 1 addition & 1 deletion impeller/renderer/backend/gles/reactor_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ class ReactorGLES {
/// @param[in] handle The handle
/// @param[in] label The label
///
void SetDebugLabel(const HandleGLES& handle, std::string label);
void SetDebugLabel(const HandleGLES& handle, std::string_view label);

using Operation = std::function<void(const ReactorGLES& reactor)>;

Expand Down
4 changes: 2 additions & 2 deletions impeller/renderer/backend/gles/render_pass_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ bool RenderPassGLES::IsValid() const {
}

// |RenderPass|
void RenderPassGLES::OnSetLabel(std::string label) {
label_ = std::move(label);
void RenderPassGLES::OnSetLabel(std::string_view label) {
label_ = label;
}

void ConfigureBlending(const ProcTableGLES& gl,
Expand Down
Loading