Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit cfe5add

Browse files
author
Jonah Williams
authored
[Impeller] make labeling APIs exclusively use std::string_view. (#55918)
Many of our labeling APIs accepted a std::string. When provided with a string literal (const char*), this would force the allocation of a new std::string object and a copy of the original char*. std::string_view is a view of either a std::string, or a const char* (or other), which allows an API that doesn not need to store the string to accept both without a copy. Of course, we can't store a std::string_view, as that doesn't provide memory ownership - so many of the GLES implementations will still copy the string_view into a string locally. Also updates several of the debug labelling functions to no-op in release mode and removes some expensive interpolations that aren't super necessary.
1 parent ed40c59 commit cfe5add

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+195
-178
lines changed

impeller/core/device_buffer.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ class DeviceBuffer {
2323
Range source_range,
2424
size_t offset = 0u);
2525

26-
virtual bool SetLabel(const std::string& label) = 0;
26+
virtual bool SetLabel(std::string_view label) = 0;
2727

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

3030
/// @brief Create a buffer view of this entire buffer.
3131
static BufferView AsBufferView(std::shared_ptr<DeviceBuffer> buffer);

impeller/core/host_buffer.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,6 @@ HostBuffer::HostBuffer(const std::shared_ptr<Allocator>& allocator)
3737

3838
HostBuffer::~HostBuffer() = default;
3939

40-
void HostBuffer::SetLabel(std::string label) {
41-
label_ = std::move(label);
42-
}
43-
4440
BufferView HostBuffer::Emplace(const void* buffer,
4541
size_t length,
4642
size_t align) {

impeller/core/host_buffer.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,7 @@ class HostBuffer {
3030
static std::shared_ptr<HostBuffer> Create(
3131
const std::shared_ptr<Allocator>& allocator);
3232

33-
// |Buffer|
34-
virtual ~HostBuffer();
35-
36-
void SetLabel(std::string label);
33+
~HostBuffer();
3734

3835
//----------------------------------------------------------------------------
3936
/// @brief Emplace uniform data onto the host buffer. Ensure that backend
@@ -169,7 +166,6 @@ class HostBuffer {
169166
size_t current_buffer_ = 0u;
170167
size_t offset_ = 0u;
171168
size_t frame_index_ = 0u;
172-
std::string label_;
173169
};
174170

175171
} // namespace impeller

impeller/display_list/aiks_dl_basic_unittests.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ TEST_P(AiksTest, CanRenderColorFilterWithInvertColorsDrawPaint) {
9494
namespace {
9595
bool GenerateMipmap(const std::shared_ptr<Context>& context,
9696
std::shared_ptr<Texture> texture,
97-
std::string label) {
97+
std::string_view label) {
9898
auto buffer = context->CreateCommandBuffer();
9999
if (!buffer) {
100100
return false;
@@ -103,7 +103,7 @@ bool GenerateMipmap(const std::shared_ptr<Context>& context,
103103
if (!pass) {
104104
return false;
105105
}
106-
pass->GenerateMipmap(std::move(texture), std::move(label));
106+
pass->GenerateMipmap(std::move(texture), label);
107107

108108
pass->EncodeCommands(context->GetResourceAllocator());
109109
return context->GetCommandQueue()->Submit({buffer}).ok();

impeller/entity/contents/atlas_contents.cc

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,7 @@ bool AtlasContents::Render(const ContentContext& renderer,
9494
using FS = PorterDuffBlendPipeline::FragmentShader;
9595

9696
#ifdef IMPELLER_DEBUG
97-
pass.SetCommandLabel(
98-
SPrintF("DrawAtlas Blend (%s)", BlendModeToString(blend_mode)));
97+
pass.SetCommandLabel("DrawAtlas Blend");
9998
#endif // IMPELLER_DEBUG
10099
pass.SetVertexBuffer(geometry_->CreateBlendVertexBuffer(host_buffer));
101100
pass.SetPipeline(
@@ -138,8 +137,7 @@ bool AtlasContents::Render(const ContentContext& renderer,
138137
using FS = VerticesUberShader::FragmentShader;
139138

140139
#ifdef IMPELLER_DEBUG
141-
pass.SetCommandLabel(
142-
SPrintF("DrawAtlas Advanced Blend (%s)", BlendModeToString(blend_mode)));
140+
pass.SetCommandLabel("DrawAtlas Advanced Blend");
143141
#endif // IMPELLER_DEBUG
144142
pass.SetVertexBuffer(geometry_->CreateBlendVertexBuffer(host_buffer));
145143

impeller/entity/contents/content_context.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -493,12 +493,12 @@ fml::StatusOr<RenderTarget> ContentContext::MakeSubpass(
493493
if (context->GetCapabilities()->SupportsOffscreenMSAA() && msaa_enabled) {
494494
subpass_target = GetRenderTargetCache()->CreateOffscreenMSAA(
495495
*context, texture_size,
496-
/*mip_count=*/mip_count, SPrintF("%s Offscreen", label.data()),
496+
/*mip_count=*/mip_count, label,
497497
RenderTarget::kDefaultColorAttachmentConfigMSAA, depth_stencil_config);
498498
} else {
499499
subpass_target = GetRenderTargetCache()->CreateOffscreen(
500500
*context, texture_size,
501-
/*mip_count=*/mip_count, SPrintF("%s Offscreen", label.data()),
501+
/*mip_count=*/mip_count, label,
502502
RenderTarget::kDefaultColorAttachmentConfig, depth_stencil_config);
503503
}
504504
return MakeSubpass(label, subpass_target, command_buffer, subpass_callback);
@@ -520,7 +520,7 @@ fml::StatusOr<RenderTarget> ContentContext::MakeSubpass(
520520
if (!sub_renderpass) {
521521
return fml::Status(fml::StatusCode::kUnknown, "");
522522
}
523-
sub_renderpass->SetLabel(SPrintF("%s RenderPass", label.data()));
523+
sub_renderpass->SetLabel(label);
524524

525525
if (!subpass_callback(*this, *sub_renderpass)) {
526526
return fml::Status(fml::StatusCode::kUnknown, "");

impeller/entity/contents/content_context.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -992,7 +992,7 @@ class ContentContext {
992992
PipelineDescriptor& desc) {
993993
opts.ApplyToPipelineDescriptor(desc);
994994
desc.SetLabel(
995-
SPrintF("%s V#%zu", desc.GetLabel().c_str(), variants_count));
995+
SPrintF("%s V#%zu", desc.GetLabel().data(), variants_count));
996996
});
997997
std::unique_ptr<RenderPipelineHandleT> variant =
998998
std::make_unique<RenderPipelineHandleT>(std::move(variant_future));

impeller/entity/contents/test/recording_render_pass.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ fml::Status RecordingRenderPass::Draw() {
9292
}
9393

9494
// |RenderPass|
95-
void RecordingRenderPass::OnSetLabel(std::string label) {
95+
void RecordingRenderPass::OnSetLabel(std::string_view label) {
9696
return;
9797
}
9898

impeller/entity/contents/test/recording_render_pass.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class RecordingRenderPass : public RenderPass {
6969
const std::unique_ptr<const Sampler>& sampler) override;
7070

7171
// |RenderPass|
72-
void OnSetLabel(std::string label) override;
72+
void OnSetLabel(std::string_view label) override;
7373

7474
// |RenderPass|
7575
bool OnEncodeCommands(const Context& context) const override;

impeller/entity/contents/texture_contents.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ std::shared_ptr<TextureContents> TextureContents::MakeRect(Rect destination) {
3131
return contents;
3232
}
3333

34-
void TextureContents::SetLabel(std::string label) {
35-
label_ = std::move(label);
34+
void TextureContents::SetLabel(std::string_view label) {
35+
label_ = label;
3636
}
3737

3838
void TextureContents::SetDestinationRect(Rect rect) {

0 commit comments

Comments
 (0)