Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 16 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
10 changes: 8 additions & 2 deletions impeller/renderer/backend/metal/context_mtl.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ class ContextMTL final : public Context,
#endif // IMPELLER_DEBUG

// |Context|
void StoreTaskForGPU(const std::function<void()>& task) override;
void StoreTaskForGPU(const std::function<void()>& task,
const std::function<void()>& failure) override;

private:
class SyncSwitchObserver : public fml::SyncSwitch::Observer {
Expand All @@ -149,6 +150,11 @@ class ContextMTL final : public Context,
ContextMTL& parent_;
};

struct PendingTasks {
std::function<void()> task;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: fml::closure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, missed this one.

std::function<void()> failure;
};

id<MTLDevice> device_ = nullptr;
id<MTLCommandQueue> command_queue_ = nullptr;
std::shared_ptr<ShaderLibraryMTL> shader_library_;
Expand All @@ -157,7 +163,7 @@ class ContextMTL final : public Context,
std::shared_ptr<AllocatorMTL> resource_allocator_;
std::shared_ptr<const Capabilities> device_capabilities_;
std::shared_ptr<const fml::SyncSwitch> is_gpu_disabled_sync_switch_;
std::deque<std::function<void()>> tasks_awaiting_gpu_;
std::deque<PendingTasks> tasks_awaiting_gpu_;
std::unique_ptr<SyncSwitchObserver> sync_switch_observer_;
std::shared_ptr<CommandQueue> command_queue_ip_;
#ifdef IMPELLER_DEBUG
Expand Down
12 changes: 8 additions & 4 deletions impeller/renderer/backend/metal/context_mtl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -377,17 +377,21 @@ new ContextMTL(device, command_queue,
return buffer;
}

void ContextMTL::StoreTaskForGPU(const std::function<void()>& task) {
tasks_awaiting_gpu_.emplace_back(task);
void ContextMTL::StoreTaskForGPU(const std::function<void()>& task,
const std::function<void()>& failure) {
tasks_awaiting_gpu_.push_back(PendingTasks{task, failure});
while (tasks_awaiting_gpu_.size() > kMaxTasksAwaitingGPU) {
tasks_awaiting_gpu_.front()();
PendingTasks front = std::move(tasks_awaiting_gpu_.front());
if (front.failure) {
front.failure();
}
tasks_awaiting_gpu_.pop_front();
}
}

void ContextMTL::FlushTasksAwaitingGPU() {
for (const auto& task : tasks_awaiting_gpu_) {
task();
task.task();
Copy link
Member

Choose a reason for hiding this comment

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

Perform null checks here or wrap a null ptr in a do-nothing fml::closure in StoreTaskForCPU perhaps?

}
tasks_awaiting_gpu_.clear();
}
Expand Down
6 changes: 5 additions & 1 deletion impeller/renderer/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,14 @@ class Context {
/// being available or that the task has been canceled. The task should
/// operate with the `SyncSwitch` to make sure the GPU is accessible.
///
/// If the queue of pending tasks is cleared without GPU access, then the
/// failure callback will be invoked and the primary task function will not
///
/// Threadsafe.
///
/// `task` will be executed on the platform thread.
virtual void StoreTaskForGPU(const std::function<void()>& task) {
virtual void StoreTaskForGPU(const std::function<void()>& task,
const std::function<void()>& failure) {
FML_CHECK(false && "not supported in this context");
}

Expand Down
221 changes: 105 additions & 116 deletions lib/ui/painting/image_decoder_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "flutter/impeller/renderer/context.h"
#include "impeller/base/strings.h"
#include "impeller/core/device_buffer.h"
#include "impeller/core/formats.h"
#include "impeller/display_list/skia_conversions.h"
#include "impeller/geometry/size.h"
#include "third_party/skia/include/core/SkAlphaType.h"
Expand All @@ -26,7 +27,6 @@
#include "third_party/skia/include/core/SkPixelRef.h"
#include "third_party/skia/include/core/SkPixmap.h"
#include "third_party/skia/include/core/SkPoint.h"
#include "third_party/skia/include/core/SkSamplingOptions.h"
#include "third_party/skia/include/core/SkSize.h"

namespace flutter {
Expand Down Expand Up @@ -224,61 +224,35 @@ DecompressResult ImageDecoderImpeller::DecompressTexture(
bitmap = premul_bitmap;
}

if (bitmap->dimensions() == target_size) {
std::shared_ptr<impeller::DeviceBuffer> buffer =
bitmap_allocator->GetDeviceBuffer();
if (!buffer) {
return DecompressResult{.decode_error = "Unable to get device buffer"};
}
buffer->Flush();

return DecompressResult{.device_buffer = std::move(buffer),
.sk_bitmap = bitmap,
.image_info = bitmap->info()};
}

//----------------------------------------------------------------------------
/// 2. If the decoded image isn't the requested target size, resize it.
///

TRACE_EVENT0("impeller", "DecodeScale");
const auto scaled_image_info = image_info.makeDimensions(target_size);

auto scaled_bitmap = std::make_shared<SkBitmap>();
auto scaled_allocator = std::make_shared<ImpellerAllocator>(allocator);
scaled_bitmap->setInfo(scaled_image_info);
if (!scaled_bitmap->tryAllocPixels(scaled_allocator.get())) {
std::string decode_error(
"Could not allocate scaled bitmap for image decompression.");
FML_DLOG(ERROR) << decode_error;
return DecompressResult{.decode_error = decode_error};
}
if (!bitmap->pixmap().scalePixels(
scaled_bitmap->pixmap(),
SkSamplingOptions(SkFilterMode::kLinear, SkMipmapMode::kNone))) {
FML_LOG(ERROR) << "Could not scale decoded bitmap data.";
}
scaled_bitmap->setImmutable();

std::shared_ptr<impeller::DeviceBuffer> buffer =
scaled_allocator->GetDeviceBuffer();
buffer->Flush();

bitmap_allocator->GetDeviceBuffer();
if (!buffer) {
return DecompressResult{.decode_error = "Unable to get device buffer"};
}
buffer->Flush();

std::optional<SkImageInfo> resize_info =
bitmap->dimensions() == target_size
? std::nullopt
: std::optional<SkImageInfo>(image_info.makeDimensions(target_size));
return DecompressResult{.device_buffer = std::move(buffer),
.sk_bitmap = scaled_bitmap,
.image_info = scaled_bitmap->info()};
.sk_bitmap = bitmap,
.image_info = bitmap->info(),
.resize_info = resize_info};
}

/// Only call this method if the GPU is available.
static std::pair<sk_sp<DlImage>, std::string> UnsafeUploadTextureToPrivate(
// static
std::pair<sk_sp<DlImage>, std::string>
ImageDecoderImpeller::UnsafeUploadTextureToPrivate(
const std::shared_ptr<impeller::Context>& context,
const std::shared_ptr<impeller::DeviceBuffer>& buffer,
const SkImageInfo& image_info) {
const SkImageInfo& image_info,
const std::optional<SkImageInfo>& resize_info,
bool create_mips) {
const auto pixel_format =
impeller::skia_conversions::ToPixelFormat(image_info.colorType());
// mips should still be created if the source image is being resized.
const bool should_create_mips = create_mips || resize_info.has_value();
if (!pixel_format) {
std::string decode_error(impeller::SPrintF(
"Unsupported pixel format (SkColorType=%d)", image_info.colorType()));
Expand All @@ -290,7 +264,8 @@ static std::pair<sk_sp<DlImage>, std::string> UnsafeUploadTextureToPrivate(
texture_descriptor.storage_mode = impeller::StorageMode::kDevicePrivate;
texture_descriptor.format = pixel_format.value();
texture_descriptor.size = {image_info.width(), image_info.height()};
texture_descriptor.mip_count = texture_descriptor.size.MipCount();
texture_descriptor.mip_count =
should_create_mips ? texture_descriptor.size.MipCount() : 1;
texture_descriptor.compression_type = impeller::CompressionType::kLossy;

auto dest_texture =
Expand Down Expand Up @@ -323,59 +298,104 @@ static std::pair<sk_sp<DlImage>, std::string> UnsafeUploadTextureToPrivate(
blit_pass->SetLabel("Mipmap Blit Pass");
blit_pass->AddCopy(impeller::DeviceBuffer::AsBufferView(buffer),
dest_texture);
if (texture_descriptor.size.MipCount() > 1) {
if (texture_descriptor.mip_count > 1) {
Copy link
Member

Choose a reason for hiding this comment

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

Oh, oops.

blit_pass->GenerateMipmap(dest_texture);
Copy link
Member

Choose a reason for hiding this comment

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

This is unnecessary and wasteful. The mip chain needs to be generated for the resize texture after the base level from the dest texture has been used as the blit source. Generating a mip chain for the dest texture does nothing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might be misunderstanding how this works. I think what I actually need to do is blit starting from the closest larger size mip level, otherwise I'm going to drop rows of data, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I think I need to do is change blit_pass_vk.cc to check if the dst size is smaller than src and if src has mipmaps. If so, for the https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkImageCopy.html specify the src mip level better.

Also if we have mips, we should copy the mips too.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I may be misunderstanding this now. Can you not use the blit pass to resize? From the docs: "The textures can be different sizes as long as the larger texture has a mipmap level that’s the same size as the smaller texture’s level 0 mipmap.".

In that case, the resize must happen via a draw call.

Copy link
Member

Choose a reason for hiding this comment

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

And then you can use a any sampling mode you want.

So, draw from the source to the dest and then generate the mips just once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These coordinates are used to sample from the source image, as described in Image Operations chapter, with the filter mode equal to that of filter, a mipmap mode of VK_SAMPLER_MIPMAP_MODE_NEAREST and an address mode of VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_EDGE. Implementations must clamp at the edge of the source image, and may additionally clamp to the edge of the source region.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just need to specify a filter in blit_pass_vk. But since I only copy mip level 0 I still need to regenerate the mip chain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh but we use vkcopyImage hmmm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can detect that the sizes don't match and do the full blit. Unknown if https://developer.apple.com/documentation/metal/mtlblitcommandencoder/1400754-copyfromtexture supports resizing but I could use a MPS instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Spoiler: it does not, switched to use MPS

}

std::shared_ptr<impeller::Texture> result_texture = dest_texture;
if (resize_info.has_value()) {
impeller::TextureDescriptor resize_desc;
resize_desc.storage_mode = impeller::StorageMode::kDevicePrivate;
resize_desc.format = pixel_format.value();
resize_desc.size = {resize_info->width(), resize_info->height()};
resize_desc.mip_count = create_mips ? resize_desc.size.MipCount() : 1;
resize_desc.compression_type = impeller::CompressionType::kLossy;

auto resize_texture =
context->GetResourceAllocator()->CreateTexture(resize_desc);
if (!resize_texture) {
std::string decode_error("Could not create resized Impeller texture.");
FML_DLOG(ERROR) << decode_error;
return std::make_pair(nullptr, decode_error);
}

blit_pass->AddCopy(/*source=*/dest_texture, /*destination=*/resize_texture);
if (resize_desc.mip_count > 1) {
blit_pass->GenerateMipmap(resize_texture);
}

result_texture = std::move(resize_texture);
}
blit_pass->EncodeCommands(context->GetResourceAllocator());

if (!context->GetCommandQueue()->Submit({command_buffer}).ok()) {
std::string decode_error("Failed to submit blit pass command buffer.");
std::string decode_error("Failed to submit image deocding command buffer.");
Copy link
Member

Choose a reason for hiding this comment

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

Nit: typo in "decoding"

FML_DLOG(ERROR) << decode_error;
return std::make_pair(nullptr, decode_error);
}

return std::make_pair(
impeller::DlImageImpeller::Make(std::move(dest_texture)), std::string());
impeller::DlImageImpeller::Make(std::move(result_texture)),
std::string());
}

std::pair<sk_sp<DlImage>, std::string>
ImageDecoderImpeller::UploadTextureToPrivate(
void ImageDecoderImpeller::UploadTextureToPrivate(
ImageResult result,
const std::shared_ptr<impeller::Context>& context,
const std::shared_ptr<impeller::DeviceBuffer>& buffer,
const SkImageInfo& image_info,
const std::shared_ptr<SkBitmap>& bitmap,
const std::shared_ptr<fml::SyncSwitch>& gpu_disabled_switch) {
const std::optional<SkImageInfo>& resize_info,
const std::shared_ptr<fml::SyncSwitch>& gpu_disabled_switch,
bool create_mips) {
TRACE_EVENT0("impeller", __FUNCTION__);
if (!context) {
return std::make_pair(nullptr, "No Impeller context is available");
result(nullptr, "No Impeller context is available");
return;
}
if (!buffer) {
return std::make_pair(nullptr, "No Impeller device buffer is available");
result(nullptr, "No Impeller device buffer is available");
return;
}

std::pair<sk_sp<DlImage>, std::string> result;
gpu_disabled_switch->Execute(
fml::SyncSwitch::Handlers()
.SetIfFalse([&result, context, buffer, image_info] {
result = UnsafeUploadTextureToPrivate(context, buffer, image_info);
})
.SetIfTrue([&result, context, bitmap, gpu_disabled_switch] {
// create_mips is false because we already know the GPU is disabled.
result =
UploadTextureToStorage(context, bitmap, gpu_disabled_switch,
impeller::StorageMode::kHostVisible,
/*create_mips=*/false);
.SetIfFalse(
[&result, context, buffer, image_info, resize_info, create_mips] {
sk_sp<DlImage> image;
std::string decode_error;
std::tie(image, decode_error) = std::tie(image, decode_error) =
UnsafeUploadTextureToPrivate(context, buffer, image_info,
resize_info, create_mips);
result(image, decode_error);
})
.SetIfTrue([&result, context, buffer, image_info, resize_info,
create_mips] {
// The `result` function must be copied in the capture list for each
// closure or the stack allocated callback will be cleared by the
// time to closure is executed later.
context->StoreTaskForGPU(
[result, context, buffer, image_info, resize_info,
create_mips]() {
sk_sp<DlImage> image;
std::string decode_error;
std::tie(image, decode_error) = std::tie(image,
decode_error) =
UnsafeUploadTextureToPrivate(context, buffer, image_info,
resize_info, create_mips);
result(image, decode_error);
},
[result]() {
result(nullptr,
"Image upload failed due to loss of GPU access.");
});
}));
return result;
}

std::pair<sk_sp<DlImage>, std::string>
ImageDecoderImpeller::UploadTextureToStorage(
const std::shared_ptr<impeller::Context>& context,
std::shared_ptr<SkBitmap> bitmap,
const std::shared_ptr<fml::SyncSwitch>& gpu_disabled_switch,
impeller::StorageMode storage_mode,
bool create_mips) {
std::shared_ptr<SkBitmap> bitmap) {
TRACE_EVENT0("impeller", __FUNCTION__);
if (!context) {
return std::make_pair(nullptr, "No Impeller context is available");
Expand All @@ -394,11 +414,10 @@ ImageDecoderImpeller::UploadTextureToStorage(
}

impeller::TextureDescriptor texture_descriptor;
texture_descriptor.storage_mode = storage_mode;
texture_descriptor.storage_mode = impeller::StorageMode::kHostVisible;
texture_descriptor.format = pixel_format.value();
texture_descriptor.size = {image_info.width(), image_info.height()};
texture_descriptor.mip_count =
create_mips ? texture_descriptor.size.MipCount() : 1;
texture_descriptor.mip_count = 1;

auto texture =
context->GetResourceAllocator()->CreateTexture(texture_descriptor);
Expand All @@ -421,43 +440,6 @@ ImageDecoderImpeller::UploadTextureToStorage(
}

texture->SetLabel(impeller::SPrintF("ui.Image(%p)", texture.get()).c_str());

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this function is only used for gifs now we don't need to worry about mip support.

if (texture_descriptor.mip_count > 1u && create_mips) {
std::optional<std::string> decode_error;

// The only platform that needs mipmapping unconditionally is GL.
// GL based platforms never disable GPU access.
// This is only really needed for iOS.
gpu_disabled_switch->Execute(fml::SyncSwitch::Handlers().SetIfFalse(
[context, &texture, &decode_error] {
auto command_buffer = context->CreateCommandBuffer();
if (!command_buffer) {
decode_error =
"Could not create command buffer for mipmap generation.";
return;
}
command_buffer->SetLabel("Mipmap Command Buffer");

auto blit_pass = command_buffer->CreateBlitPass();
if (!blit_pass) {
decode_error = "Could not create blit pass for mipmap generation.";
return;
}
blit_pass->SetLabel("Mipmap Blit Pass");
blit_pass->GenerateMipmap(texture);
blit_pass->EncodeCommands(context->GetResourceAllocator());
if (!context->GetCommandQueue()->Submit({command_buffer}).ok()) {
decode_error = "Failed to submit blit pass command buffer.";
return;
}
command_buffer->WaitUntilScheduled();
}));
if (decode_error.has_value()) {
FML_DLOG(ERROR) << decode_error.value();
return std::make_pair(nullptr, decode_error.value());
}
}

return std::make_pair(impeller::DlImageImpeller::Make(std::move(texture)),
std::string());
}
Expand Down Expand Up @@ -509,14 +491,21 @@ void ImageDecoderImpeller::Decode(fml::RefPtr<ImageDescriptor> descriptor,

auto upload_texture_and_invoke_result = [result, context, bitmap_result,
gpu_disabled_switch]() {
sk_sp<DlImage> image;
std::string decode_error;
std::tie(image, decode_error) = UploadTextureToPrivate(
context, bitmap_result.device_buffer, bitmap_result.image_info,
bitmap_result.sk_bitmap, gpu_disabled_switch);
result(image, decode_error);
UploadTextureToPrivate(result, context, //
bitmap_result.device_buffer, //
bitmap_result.image_info, //
bitmap_result.sk_bitmap, //
bitmap_result.resize_info, //
gpu_disabled_switch //
);
};
io_runner->PostTask(upload_texture_and_invoke_result);
// The I/O image uploads are not threadsafe on GLES.
if (context->GetBackendType() ==
impeller::Context::BackendType::kOpenGLES) {
io_runner->PostTask(upload_texture_and_invoke_result);
} else {
upload_texture_and_invoke_result();
}
});
}

Expand Down
Loading