Skip to content

Commit 467e5ea

Browse files
author
Jonah Williams
authored
[Impeller] Don't decompress into device buffer for Vulkan/GLES. (flutter#43493)
My observations on the Pixel 6 device are that performing device allocations from multiple threads can dramatically slow down the raster task workload. As a stopgap solution, we can adjust image upload to only touch the device allocator on the IO thread which reduces the parallel access. This doesn't have any impact on the S10, but locally on the Pixel 6 it is a night and day difference. I am testing using jonahwilliams/forked_gallery and navigating to the Reply demo. This demo has a large number of images, several of which are quite large. Work towards flutter#129392 ### Before Page transition is ~4 frames. ![image](https://github.com/flutter/engine/assets/8975114/b6d1c225-060b-4a20-9737-ad668423799a) ### After Page transition is ~20 frames. ![image](https://github.com/flutter/engine/assets/8975114/5ff1f857-8327-4d04-b40a-3da4a5fc91a4)
1 parent 7d054ab commit 467e5ea

4 files changed

Lines changed: 59 additions & 11 deletions

File tree

lib/ui/painting/image_decoder_impeller.cc

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,42 @@
3232

3333
namespace flutter {
3434

35+
class MallocDeviceBuffer : public impeller::DeviceBuffer {
36+
public:
37+
explicit MallocDeviceBuffer(impeller::DeviceBufferDescriptor desc)
38+
: impeller::DeviceBuffer(desc) {
39+
data_ = static_cast<uint8_t*>(malloc(desc.size));
40+
}
41+
42+
~MallocDeviceBuffer() override { free(data_); }
43+
44+
bool SetLabel(const std::string& label) override { return true; }
45+
46+
bool SetLabel(const std::string& label, impeller::Range range) override {
47+
return true;
48+
}
49+
50+
uint8_t* OnGetContents() const override { return data_; }
51+
52+
bool OnCopyHostBuffer(const uint8_t* source,
53+
impeller::Range source_range,
54+
size_t offset) override {
55+
memcpy(data_ + offset, source + source_range.offset, source_range.length);
56+
return true;
57+
}
58+
59+
private:
60+
uint8_t* data_;
61+
62+
FML_DISALLOW_COPY_AND_ASSIGN(MallocDeviceBuffer);
63+
};
64+
65+
#ifdef FML_OS_ANDROID
66+
static constexpr bool kShouldUseMallocDeviceBuffer = true;
67+
#else
68+
static constexpr bool kShouldUseMallocDeviceBuffer = false;
69+
#endif // FML_OS_ANDROID
70+
3571
namespace {
3672
/**
3773
* Loads the gamut as a set of three points (triangle).
@@ -336,17 +372,20 @@ ImageDecoderImpeller::UploadTextureToPrivate(
336372
})
337373
.SetIfTrue([&result, context, bitmap, gpu_disabled_switch] {
338374
// create_mips is false because we already know the GPU is disabled.
339-
result = UploadTextureToShared(context, bitmap, gpu_disabled_switch,
340-
/*create_mips=*/false);
375+
result =
376+
UploadTextureToStorage(context, bitmap, gpu_disabled_switch,
377+
impeller::StorageMode::kHostVisible,
378+
/*create_mips=*/false);
341379
}));
342380
return result;
343381
}
344382

345383
std::pair<sk_sp<DlImage>, std::string>
346-
ImageDecoderImpeller::UploadTextureToShared(
384+
ImageDecoderImpeller::UploadTextureToStorage(
347385
const std::shared_ptr<impeller::Context>& context,
348386
std::shared_ptr<SkBitmap> bitmap,
349387
const std::shared_ptr<fml::SyncSwitch>& gpu_disabled_switch,
388+
impeller::StorageMode storage_mode,
350389
bool create_mips) {
351390
TRACE_EVENT0("impeller", __FUNCTION__);
352391
if (!context) {
@@ -366,7 +405,7 @@ ImageDecoderImpeller::UploadTextureToShared(
366405
}
367406

368407
impeller::TextureDescriptor texture_descriptor;
369-
texture_descriptor.storage_mode = impeller::StorageMode::kHostVisible;
408+
texture_descriptor.storage_mode = storage_mode;
370409
texture_descriptor.format = pixel_format.value();
371410
texture_descriptor.size = {image_info.width(), image_info.height()};
372411
texture_descriptor.mip_count =
@@ -483,14 +522,16 @@ void ImageDecoderImpeller::Decode(fml::RefPtr<ImageDescriptor> descriptor,
483522
gpu_disabled_switch]() {
484523
sk_sp<DlImage> image;
485524
std::string decode_error;
486-
if (context->GetCapabilities()->SupportsBufferToTextureBlits()) {
525+
if (!kShouldUseMallocDeviceBuffer &&
526+
context->GetCapabilities()->SupportsBufferToTextureBlits()) {
487527
std::tie(image, decode_error) = UploadTextureToPrivate(
488528
context, bitmap_result.device_buffer, bitmap_result.image_info,
489529
bitmap_result.sk_bitmap, gpu_disabled_switch);
490530
result(image, decode_error);
491531
} else {
492-
std::tie(image, decode_error) = UploadTextureToShared(
532+
std::tie(image, decode_error) = UploadTextureToStorage(
493533
context, bitmap_result.sk_bitmap, gpu_disabled_switch,
534+
impeller::StorageMode::kDevicePrivate,
494535
/*create_mips=*/true);
495536
result(image, decode_error);
496537
}
@@ -525,7 +566,10 @@ bool ImpellerAllocator::allocPixelRef(SkBitmap* bitmap) {
525566
descriptor.size = ((bitmap->height() - 1) * bitmap->rowBytes()) +
526567
(bitmap->width() * bitmap->bytesPerPixel());
527568

528-
auto device_buffer = allocator_->CreateBuffer(descriptor);
569+
std::shared_ptr<impeller::DeviceBuffer> device_buffer =
570+
kShouldUseMallocDeviceBuffer
571+
? std::make_shared<MallocDeviceBuffer>(descriptor)
572+
: allocator_->CreateBuffer(descriptor);
529573

530574
struct ImpellerPixelRef final : public SkPixelRef {
531575
ImpellerPixelRef(int w, int h, void* s, size_t r)

lib/ui/painting/image_decoder_impeller.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include "flutter/fml/macros.h"
1111
#include "flutter/lib/ui/painting/image_decoder.h"
12+
#include "impeller/core/formats.h"
1213
#include "impeller/geometry/size.h"
1314
#include "third_party/skia/include/core/SkBitmap.h"
1415

@@ -90,10 +91,11 @@ class ImageDecoderImpeller final : public ImageDecoder {
9091
/// @param gpu_disabled_switch Whether the GPU is available for mipmap
9192
/// creation.
9293
/// @return A DlImage.
93-
static std::pair<sk_sp<DlImage>, std::string> UploadTextureToShared(
94+
static std::pair<sk_sp<DlImage>, std::string> UploadTextureToStorage(
9495
const std::shared_ptr<impeller::Context>& context,
9596
std::shared_ptr<SkBitmap> bitmap,
9697
const std::shared_ptr<fml::SyncSwitch>& gpu_disabled_switch,
98+
impeller::StorageMode storage_mode,
9799
bool create_mips = true);
98100

99101
private:

lib/ui/painting/image_decoder_unittests.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -455,8 +455,9 @@ TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) {
455455
ASSERT_EQ(no_gpu_access_context->command_buffer_count_, 0ul);
456456
ASSERT_EQ(result.second, "");
457457

458-
result = ImageDecoderImpeller::UploadTextureToShared(
459-
no_gpu_access_context, bitmap, gpu_disabled_switch, true);
458+
result = ImageDecoderImpeller::UploadTextureToStorage(
459+
no_gpu_access_context, bitmap, gpu_disabled_switch,
460+
impeller::StorageMode::kHostVisible, true);
460461
ASSERT_EQ(no_gpu_access_context->command_buffer_count_, 0ul);
461462
ASSERT_EQ(result.second, "");
462463
}

lib/ui/painting/multi_frame_codec.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,10 @@ MultiFrameCodec::State::GetNextFrameImage(
147147
if (is_impeller_enabled_) {
148148
// This is safe regardless of whether the GPU is available or not because
149149
// without mipmap creation there is no command buffer encoding done.
150-
return ImageDecoderImpeller::UploadTextureToShared(
150+
return ImageDecoderImpeller::UploadTextureToStorage(
151151
impeller_context, std::make_shared<SkBitmap>(bitmap),
152152
std::make_shared<fml::SyncSwitch>(),
153+
impeller::StorageMode::kHostVisible,
153154
/*create_mips=*/false);
154155
}
155156
#endif // IMPELLER_SUPPORTS_RENDERING

0 commit comments

Comments
 (0)