[Impeller] leave glyph atlas in transfer dst to improve vulkan throughput. (#52908)
On the vulkan backend everytime we blit a glyph we go shader read -> transfer dst -> shader read. This is pretty inefficient if we're appending many glyphs.
Poke a hole in the blitpass API so we can leave the glyph atlas in transfer_dst to reduce the number of layout transitions.
diff --git a/impeller/renderer/backend/gles/blit_pass_gles.cc b/impeller/renderer/backend/gles/blit_pass_gles.cc
index 363400d..d83ef32 100644
--- a/impeller/renderer/backend/gles/blit_pass_gles.cc
+++ b/impeller/renderer/backend/gles/blit_pass_gles.cc
@@ -132,7 +132,8 @@
std::shared_ptr<Texture> destination,
IRect destination_region,
std::string label,
- uint32_t slice) {
+ uint32_t slice,
+ bool convert_to_read) {
auto command = std::make_unique<BlitCopyBufferToTextureCommandGLES>();
command->label = label;
command->source = std::move(source);
diff --git a/impeller/renderer/backend/gles/blit_pass_gles.h b/impeller/renderer/backend/gles/blit_pass_gles.h
index 1b5385f..e34bb16 100644
--- a/impeller/renderer/backend/gles/blit_pass_gles.h
+++ b/impeller/renderer/backend/gles/blit_pass_gles.h
@@ -59,7 +59,8 @@
std::shared_ptr<Texture> destination,
IRect destination_region,
std::string label,
- uint32_t slice) override;
+ uint32_t slice,
+ bool convert_to_read) override;
// |BlitPass|
bool OnGenerateMipmapCommand(std::shared_ptr<Texture> texture,
diff --git a/impeller/renderer/backend/metal/blit_pass_mtl.h b/impeller/renderer/backend/metal/blit_pass_mtl.h
index 3363eb9..1de0f45 100644
--- a/impeller/renderer/backend/metal/blit_pass_mtl.h
+++ b/impeller/renderer/backend/metal/blit_pass_mtl.h
@@ -58,7 +58,8 @@
std::shared_ptr<Texture> destination,
IRect destination_region,
std::string label,
- uint32_t slice) override;
+ uint32_t slice,
+ bool convert_to_read) override;
// |BlitPass|
bool OnGenerateMipmapCommand(std::shared_ptr<Texture> texture,
diff --git a/impeller/renderer/backend/metal/blit_pass_mtl.mm b/impeller/renderer/backend/metal/blit_pass_mtl.mm
index 9caa449..329fe51 100644
--- a/impeller/renderer/backend/metal/blit_pass_mtl.mm
+++ b/impeller/renderer/backend/metal/blit_pass_mtl.mm
@@ -166,7 +166,8 @@
std::shared_ptr<Texture> destination,
IRect destination_region,
std::string label,
- uint32_t slice) {
+ uint32_t slice,
+ bool convert_to_read) {
auto source_mtl = DeviceBufferMTL::Cast(*source.buffer).GetMTLBuffer();
if (!source_mtl) {
return false;
diff --git a/impeller/renderer/backend/vulkan/blit_pass_vk.cc b/impeller/renderer/backend/vulkan/blit_pass_vk.cc
index 289561a..b8c5e98 100644
--- a/impeller/renderer/backend/vulkan/blit_pass_vk.cc
+++ b/impeller/renderer/backend/vulkan/blit_pass_vk.cc
@@ -221,13 +221,37 @@
return true;
}
+bool BlitPassVK::ConvertTextureToShaderRead(
+ const std::shared_ptr<Texture>& texture) {
+ auto& encoder = *command_buffer_->GetEncoder();
+ const auto& cmd_buffer = encoder.GetCommandBuffer();
+
+ BarrierVK barrier;
+ barrier.cmd_buffer = cmd_buffer;
+ barrier.src_access = vk::AccessFlagBits::eTransferWrite;
+ barrier.src_stage = vk::PipelineStageFlagBits::eTransfer;
+ barrier.dst_access = vk::AccessFlagBits::eShaderRead;
+ barrier.dst_stage = vk::PipelineStageFlagBits::eFragmentShader;
+
+ barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal;
+
+ const auto& texture_vk = TextureVK::Cast(*texture);
+
+ if (!encoder.Track(texture)) {
+ return false;
+ }
+
+ return texture_vk.SetLayout(barrier);
+}
+
// |BlitPass|
bool BlitPassVK::OnCopyBufferToTextureCommand(
BufferView source,
std::shared_ptr<Texture> destination,
IRect destination_region,
std::string label,
- uint32_t slice) {
+ uint32_t slice,
+ bool convert_to_read) {
auto& encoder = *command_buffer_->GetEncoder();
const auto& cmd_buffer = encoder.GetCommandBuffer();
@@ -262,6 +286,9 @@
image_copy.imageExtent.height = destination_region.GetHeight();
image_copy.imageExtent.depth = 1u;
+ // Note: this barrier should do nothing if we're already in the transfer dst
+ // optimal state. This is important for performance of repeated blit pass
+ // encoding.
if (!dst.SetLayout(dst_barrier)) {
VALIDATION_LOG << "Could not encode layout transition.";
return false;
@@ -274,7 +301,7 @@
);
// Transition to shader-read.
- {
+ if (convert_to_read) {
BarrierVK barrier;
barrier.cmd_buffer = cmd_buffer;
barrier.src_access = vk::AccessFlagBits::eTransferWrite;
diff --git a/impeller/renderer/backend/vulkan/blit_pass_vk.h b/impeller/renderer/backend/vulkan/blit_pass_vk.h
index b3967ed..7d9c4c7 100644
--- a/impeller/renderer/backend/vulkan/blit_pass_vk.h
+++ b/impeller/renderer/backend/vulkan/blit_pass_vk.h
@@ -38,6 +38,10 @@
const std::shared_ptr<Allocator>& transients_allocator) const override;
// |BlitPass|
+ bool ConvertTextureToShaderRead(
+ const std::shared_ptr<Texture>& texture) override;
+
+ // |BlitPass|
bool OnCopyTextureToTextureCommand(std::shared_ptr<Texture> source,
std::shared_ptr<Texture> destination,
IRect source_region,
@@ -56,7 +60,8 @@
std::shared_ptr<Texture> destination,
IRect destination_region,
std::string label,
- uint32_t slice) override;
+ uint32_t slice,
+ bool convert_to_read) override;
// |BlitPass|
bool OnGenerateMipmapCommand(std::shared_ptr<Texture> texture,
std::string label) override;
diff --git a/impeller/renderer/blit_pass.cc b/impeller/renderer/blit_pass.cc
index e215b05..0126c65 100644
--- a/impeller/renderer/blit_pass.cc
+++ b/impeller/renderer/blit_pass.cc
@@ -124,7 +124,8 @@
std::shared_ptr<Texture> destination,
std::optional<IRect> destination_region,
std::string label,
- uint32_t slice) {
+ uint32_t slice,
+ bool convert_to_read) {
if (!destination) {
VALIDATION_LOG << "Attempted to add a texture blit with no destination.";
return false;
@@ -156,7 +157,12 @@
return OnCopyBufferToTextureCommand(std::move(source), std::move(destination),
destination_region_value,
- std::move(label), slice);
+ std::move(label), slice, convert_to_read);
+}
+
+bool BlitPass::ConvertTextureToShaderRead(
+ const std::shared_ptr<Texture>& texture) {
+ return true;
}
bool BlitPass::GenerateMipmap(std::shared_ptr<Texture> texture,
diff --git a/impeller/renderer/blit_pass.h b/impeller/renderer/blit_pass.h
index 5b2f56b..6f5f332 100644
--- a/impeller/renderer/blit_pass.h
+++ b/impeller/renderer/blit_pass.h
@@ -32,11 +32,18 @@
void SetLabel(std::string label);
//----------------------------------------------------------------------------
+ /// @brief If the texture is not already in a shader read internal
+ /// state, then convert it to that state.
+ ///
+ /// This API is only used by Vulkan.
+ virtual bool ConvertTextureToShaderRead(
+ const std::shared_ptr<Texture>& texture);
+
+ //----------------------------------------------------------------------------
/// @brief Record a command to copy the contents of one texture to
/// another texture. The blit area is limited by the intersection
/// of the texture coverage with respect the source region and
/// destination origin.
- /// No work is encoded into the command buffer at this time.
///
/// @param[in] source The texture to read for copying.
/// @param[in] destination The texture to overwrite using the source
@@ -60,7 +67,6 @@
//----------------------------------------------------------------------------
/// @brief Record a command to copy the contents of the buffer to
/// the texture.
- /// No work is encoded into the command buffer at this time.
///
/// @param[in] source The texture to read for copying.
/// @param[in] destination The buffer to overwrite using the source
@@ -84,7 +90,6 @@
//----------------------------------------------------------------------------
/// @brief Record a command to copy the contents of the buffer to
/// the texture.
- /// No work is encoded into the command buffer at this time.
///
/// @param[in] source The buffer view to read for copying.
/// @param[in] destination The texture to overwrite using the source
@@ -96,6 +101,8 @@
/// command.
/// @param[in] slice For cubemap textures, the slice to write
/// data to.
+ /// @param[in] convert_to_read Whether to convert the texture to a shader
+ /// read state. Defaults to true.
///
/// @return If the command was valid for subsequent commitment.
///
@@ -111,11 +118,11 @@
std::shared_ptr<Texture> destination,
std::optional<IRect> destination_region = std::nullopt,
std::string label = "",
- uint32_t slice = 0);
+ uint32_t slice = 0,
+ bool convert_to_read = true);
//----------------------------------------------------------------------------
/// @brief Record a command to generate all mip levels for a texture.
- /// No work is encoded into the command buffer at this time.
///
/// @param[in] texture The texture to generate mipmaps for.
/// @param[in] label The optional debug label to give the command.
@@ -159,7 +166,8 @@
std::shared_ptr<Texture> destination,
IRect destination_region,
std::string label,
- uint32_t slice) = 0;
+ uint32_t slice,
+ bool convert_to_read) = 0;
virtual bool OnGenerateMipmapCommand(std::shared_ptr<Texture> texture,
std::string label) = 0;
diff --git a/impeller/renderer/testing/mocks.h b/impeller/renderer/testing/mocks.h
index c333ac9..d4e2744 100644
--- a/impeller/renderer/testing/mocks.h
+++ b/impeller/renderer/testing/mocks.h
@@ -84,7 +84,8 @@
std::shared_ptr<Texture> destination,
IRect destination_rect,
std::string label,
- uint32_t slice),
+ uint32_t slice,
+ bool convert_to_read),
(override));
MOCK_METHOD(bool,
OnGenerateMipmapCommand,
diff --git a/impeller/typographer/backends/skia/typographer_context_skia.cc b/impeller/typographer/backends/skia/typographer_context_skia.cc
index 14485f4..52616ed 100644
--- a/impeller/typographer/backends/skia/typographer_context_skia.cc
+++ b/impeller/typographer/backends/skia/typographer_context_skia.cc
@@ -298,13 +298,21 @@
DrawGlyph(canvas, pair.scaled_font, pair.glyph, has_color);
- if (!blit_pass->AddCopy(allocator.TakeBufferView(), texture,
+ // convert_to_read is set to false so that the texture remains in a transfer
+ // dst layout until we finish writing to it below. This only has an impact
+ // on Vulkan where we are responsible for managing image layouts.
+ if (!blit_pass->AddCopy(allocator.TakeBufferView(), //
+ texture, //
IRect::MakeXYWH(pos->GetLeft(), pos->GetTop(),
- size.width, size.height))) {
+ size.width, size.height), //
+ /*label=*/"", //
+ /*slice=*/0, //
+ /*convert_to_read=*/false //
+ )) {
return false;
}
}
- return true;
+ return blit_pass->ConvertTextureToShaderRead(texture);
}
std::shared_ptr<GlyphAtlas> TypographerContextSkia::CreateGlyphAtlas(
