[Impeller] Allocate images into host buffer, blit to device private texture. #40410

jonahwilliams · 2023-03-17T23:05:37Z

~~I'm not sure if we can land this until we figure out image upload performance issues~~

Patch landed to force serialization of texture upload in #40464

Updated patch so that multiframe codecs always go to host visible textures and don't generate mipmaps. This should help throughput performance since UMA don't have much cost for creating these textures

This would allow us to apply lossy compression to image textures, which dramatically reduces (50%?) memory usage on certain formats. From experimenting with the metal memory profiler, this does appear to use less memory that using host visible textures - possibly due to lossless compression. I've also confirmed that the primary memory category we measure does not include private metal resources, so there is an accounting aspect to it. That said, all categories appear lower after this change.

I experimented with this by swiping through wonderous for a few seconds and then letting the app rest a bit:

Using the VM tracker which includes all alloctions, without this patch we stabilize at ~320 MB of dirty memory and 2 MB of swap, while with this patch we stabilize at 237 MB of dirty memory and 42 MB of swap.

The metal resource events category also is reduced with this patch, previously peaking at 325 MB and dropping down to 287. Finally, mostly due to accounting the all heap and anon VM category drops from 280 to 90 MB.

Leaves the existing path more or less for gles/vulkan as we need to determine if private textures make sense there and implement support for buffer to texture blit.

flutter/flutter#122914
flutter/flutter#122918
flutter/flutter#122406

…exture

jonahwilliams · 2023-03-17T23:08:29Z

impeller/renderer/backend/gles/blit_pass_gles.h

+                                    size_t source_offset,
+                                    IPoint destination_origin,
+                                    std::string label) override {
+    return false;


Totally fine. But IMPELLER_UNIMPLEMENTED in //impeller/base/config.h

jonahwilliams · 2023-03-17T23:08:41Z

impeller/renderer/backend/vulkan/blit_pass_vk.h

+                                    size_t source_offset,
+                                    IPoint destination_origin,
+                                    std::string label) override {
+    return false;


No problem. I'll handle it. But again, FML_UNIMPLEMENTED so its easier to grep for more than anything else.

jonahwilliams · 2023-03-17T23:09:05Z

impeller/renderer/backend/metal/allocator_mtl.mm


  mtl_texture_desc.storageMode = ToMTLStorageMode(
      desc.storage_mode, supports_memoryless_targets_, supports_uma_);
+  if (@available(macOS 12.5, ios 15.0, *)) {


TODO: make configurable, figure out if it has reasonable behavior for image textures

jonahwilliams · 2023-03-17T23:09:27Z

lib/ui/painting/image_decoder_unittests.cc

-  return (negative ? -1.f : 1.f) * pow_value * (1.0f + fFraction);
-}
-}  // namespace
+// namespace {


TODO: all of these tests need to be moved to a different TU that has an impeller context

Mocked out the allocator. Done

…te_texture

chinmaygarde

Initial comments but all nits. We can rework this later as needed. Let's not worry about the other backends either.

chinmaygarde · 2023-03-18T01:11:09Z

impeller/renderer/backend/gles/blit_pass_gles.h

+                                    size_t source_offset,
+                                    IPoint destination_origin,
+                                    std::string label) override {
+    return false;


Totally fine. But IMPELLER_UNIMPLEMENTED in //impeller/base/config.h

chinmaygarde · 2023-03-18T01:12:08Z

impeller/renderer/backend/metal/allocator_mtl.mm

      desc.storage_mode, supports_memoryless_targets_, supports_uma_);
+  if (@available(macOS 12.5, ios 15.0, *)) {
+    if (mtl_texture_desc.storageMode == MTLStorageModePrivate) {
+      mtl_texture_desc.compressionType = MTLTextureCompressionTypeLossy;


I'd like to attempt this in another patch and have this flag be present on impeller::TextureDescriptor.

Makes sense, removed for now.

chinmaygarde · 2023-03-18T01:13:50Z

impeller/renderer/backend/gles/blit_pass_gles.h

                                    std::string label) override;

+  // |BlitPass|
+  bool OnCopyBufferToTextureCommand(std::shared_ptr<DeviceBuffer> source,


I think we should make the source a buffer view. That way we can specify ranges in the source. Also, you'll be able to get rid of source_offset which pretty much does the same thing.

chinmaygarde · 2023-03-18T01:14:41Z

impeller/renderer/backend/vulkan/blit_pass_vk.h

+                                    size_t source_offset,
+                                    IPoint destination_origin,
+                                    std::string label) override {
+    return false;


No problem. I'll handle it. But again, FML_UNIMPLEMENTED so its easier to grep for more than anything else.

chinmaygarde · 2023-03-18T01:15:27Z

impeller/renderer/blit_command.h

 };

+struct BlitCopyBufferToTextureCommand : public BlitCommand {
+  std::shared_ptr<DeviceBuffer> source;


Consider a buffer view here.

jonahwilliams · 2023-03-19T17:13:59Z

To make the BufferView change work I had to update the mtl texture to allow casting from a DeviceBuffer to casting from a Buffer.

jonahwilliams · 2023-03-20T19:02:46Z

Landing this as is will likely regress worst frame times on gallery, as texture blitting competes with the frame workload

…te_texture

jonahwilliams · 2023-03-21T18:29:59Z

I've updated this patch so that we retain two different strategies, one to create hostVisible and one to create device private. For host visible, I've exposed an option to disable mip creation. For mutiframe codecs, I use the host visible texture with no mips for the fastest throughput

…rivate texture. (flutter/engine#40410)

[impeller] allocate images into host buffer, blit to device private t…

1d6fc76

…exture

jonahwilliams commented Mar 17, 2023

View reviewed changes

jonahwilliams force-pushed the blit_to_private_texture branch from 9dde59a to 4fc53d3 Compare March 17, 2023 23:12

Merge branch 'master' of github.com:flutter/engine into blit_to_priva…

a8b8aff

…te_texture

jonahwilliams force-pushed the blit_to_private_texture branch from 4fc53d3 to a8b8aff Compare March 17, 2023 23:14

fix resized images

3127b22

chinmaygarde changed the title ~~[impeller] allocate images into host buffer, blit to device private texture~~ [Impeller] allocate images into host buffer, blit to device private texture. Mar 18, 2023

chinmaygarde reviewed Mar 18, 2023

View reviewed changes

chinmaygarde changed the title ~~[Impeller] allocate images into host buffer, blit to device private texture.~~ [Impeller] Allocate images into host buffer, blit to device private texture. Mar 18, 2023

chinmaygarde assigned jonahwilliams Mar 18, 2023

chinmaygarde added the e: impeller label Mar 18, 2023

jonahwilliams added 4 commits March 19, 2023 08:47

Fix tests, update to use buffer view

e82b763

Remove lossy texture compression

252f053

++

76b151f

++

5fbc677

jonahwilliams force-pushed the blit_to_private_texture branch from 6d816e0 to 5fbc677 Compare March 19, 2023 17:12

jonahwilliams marked this pull request as ready for review March 19, 2023 17:14

jonahwilliams added 4 commits March 19, 2023 10:22

update gn

5658ada

++

c73a0d7

fix bitmap

b5d86ef

++

c4513c4

jonahwilliams requested a review from chinmaygarde March 20, 2023 16:53

jonahwilliams marked this pull request as draft March 20, 2023 19:25

jonahwilliams marked this pull request as ready for review March 20, 2023 22:53

Merge branch 'master' of github.com:flutter/engine into blit_to_priva…

e6c9fb0

…te_texture

move it?

c1fb18a

jonahwilliams force-pushed the blit_to_private_texture branch from d9fc482 to c1fb18a Compare March 21, 2023 00:47

jonahwilliams added 2 commits March 21, 2023 10:43

split impl

bead252

Merge branch 'master' of github.com:flutter/engine into blit_to_priva…

70c77fd

…te_texture

chinmaygarde approved these changes Mar 21, 2023

View reviewed changes

jonahwilliams added 2 commits March 21, 2023 12:16

clang tidy

a77137d

Merge branch 'main' into blit_to_private_texture

02f905b

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 21, 2023

auto-submit bot merged commit 0451913 into flutter:main Mar 21, 2023

jonahwilliams deleted the blit_to_private_texture branch March 21, 2023 21:24

engine-flutter-autoroll mentioned this pull request Mar 21, 2023

Roll Flutter Engine from 619d721c3686 to 66491ad5b931 (2 revisions) flutter/flutter#123166

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 21, 2023

0451913 [Impeller] Allocate images into host buffer, blit to device p…

9aef3ab

…rivate texture. (flutter/engine#40410)

engine-flutter-autoroll mentioned this pull request Mar 21, 2023

Roll Flutter Engine from 619d721c3686 to 45da241c4162 (5 revisions) flutter/flutter#123169

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 21, 2023

0451913 [Impeller] Allocate images into host buffer, blit to device p…

73649e1

…rivate texture. (flutter/engine#40410)

engine-flutter-autoroll mentioned this pull request Mar 21, 2023

Roll Flutter Engine from 619d721c3686 to 004d841b7efd (7 revisions) flutter/flutter#123173

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 21, 2023

0451913 [Impeller] Allocate images into host buffer, blit to device p…

4ef0e9a

…rivate texture. (flutter/engine#40410)

engine-flutter-autoroll mentioned this pull request Mar 22, 2023

Roll Flutter Engine from 619d721c3686 to 253076df3d9f (8 revisions) flutter/flutter#123175

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 22, 2023

0451913 [Impeller] Allocate images into host buffer, blit to device p…

56c09c4

…rivate texture. (flutter/engine#40410)

engine-flutter-autoroll mentioned this pull request Mar 22, 2023

Roll Flutter Engine from 619d721c3686 to 659721823be3 (9 revisions) flutter/flutter#123177

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 22, 2023

0451913 [Impeller] Allocate images into host buffer, blit to device p…

d262909

…rivate texture. (flutter/engine#40410)

engine-flutter-autoroll mentioned this pull request Mar 22, 2023

Roll Flutter Engine from 619d721c3686 to 62f284479485 (10 revisions) flutter/flutter#123178

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 22, 2023

0451913 [Impeller] Allocate images into host buffer, blit to device p…

68d40b4

…rivate texture. (flutter/engine#40410)

bdero mentioned this pull request May 26, 2023

[Impeller] Switch back to shared buffer UI image uploading on iOS #42333

Closed

[Impeller] Allocate images into host buffer, blit to device private texture. #40410

[Impeller] Allocate images into host buffer, blit to device private texture. #40410

Uh oh!

Conversation

jonahwilliams commented Mar 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Mar 19, 2023

Uh oh!

jonahwilliams commented Mar 20, 2023

Uh oh!

jonahwilliams commented Mar 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jonahwilliams commented Mar 17, 2023 •

edited

Loading