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

Commit

Permalink
[Impeller] leave glyph atlas in transfer dst to improve vulkan throug…
Browse files Browse the repository at this point in the history
…hput. (#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.
  • Loading branch information
jonahwilliams authored May 20, 2024
1 parent 1daa271 commit 8eba63f
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 19 deletions.
3 changes: 2 additions & 1 deletion impeller/renderer/backend/gles/blit_pass_gles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ bool BlitPassGLES::OnCopyBufferToTextureCommand(
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);
Expand Down
3 changes: 2 additions & 1 deletion impeller/renderer/backend/gles/blit_pass_gles.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ class BlitPassGLES final : public BlitPass,
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,
Expand Down
3 changes: 2 additions & 1 deletion impeller/renderer/backend/metal/blit_pass_mtl.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ class BlitPassMTL final : public BlitPass {
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,
Expand Down
3 changes: 2 additions & 1 deletion impeller/renderer/backend/metal/blit_pass_mtl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
31 changes: 29 additions & 2 deletions impeller/renderer/backend/vulkan/blit_pass_vk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -221,13 +221,37 @@ bool BlitPassVK::OnCopyTextureToBufferCommand(
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();

Expand Down Expand Up @@ -262,6 +286,9 @@ bool BlitPassVK::OnCopyBufferToTextureCommand(
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;
Expand All @@ -274,7 +301,7 @@ bool BlitPassVK::OnCopyBufferToTextureCommand(
);

// Transition to shader-read.
{
if (convert_to_read) {
BarrierVK barrier;
barrier.cmd_buffer = cmd_buffer;
barrier.src_access = vk::AccessFlagBits::eTransferWrite;
Expand Down
7 changes: 6 additions & 1 deletion impeller/renderer/backend/vulkan/blit_pass_vk.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ class BlitPassVK final : public BlitPass {
bool EncodeCommands(
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,
Expand All @@ -56,7 +60,8 @@ class BlitPassVK final : public BlitPass {
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;
Expand Down
10 changes: 8 additions & 2 deletions impeller/renderer/blit_pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ bool BlitPass::AddCopy(BufferView source,
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;
Expand Down Expand Up @@ -156,7 +157,12 @@ bool BlitPass::AddCopy(BufferView source,

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,
Expand Down
20 changes: 14 additions & 6 deletions impeller/renderer/blit_pass.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,19 @@ class BlitPass {

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
Expand All @@ -60,7 +67,6 @@ class BlitPass {
//----------------------------------------------------------------------------
/// @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
Expand All @@ -84,7 +90,6 @@ class BlitPass {
//----------------------------------------------------------------------------
/// @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
Expand All @@ -96,6 +101,8 @@ class BlitPass {
/// 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.
///
Expand All @@ -111,11 +118,11 @@ class BlitPass {
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.
Expand Down Expand Up @@ -159,7 +166,8 @@ class BlitPass {
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;
Expand Down
3 changes: 2 additions & 1 deletion impeller/renderer/testing/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ class MockBlitPass : public BlitPass {
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,
Expand Down
14 changes: 11 additions & 3 deletions impeller/typographer/backends/skia/typographer_context_skia.cc
Original file line number Diff line number Diff line change
Expand Up @@ -298,13 +298,21 @@ static bool UpdateAtlasBitmap(const GlyphAtlas& atlas,

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(
Expand Down

0 comments on commit 8eba63f

Please sign in to comment.