[Impeller] Wire up Android Hardware Buffer backed swapchains.

[chinmaygarde]

• This avoids the use of VK_KHR_swapchain and VK_KHR_surface for swapchains on Android.
• Instead, a pool of Android Hardware Buffers allocated by Impeller is used as swapchain images.
• The hardware buffers are presented as contents on surface controls via surface transactions.
• The total number of buffers either being used on the client side or in the compositor is capped.
• The swapchain caches a certain (configurable) number of buffer on the client side but these can expire.
• All swapchain images are lazily allocated. This is different from the KHR swapchains where all swapchain images are eagerly created when the swapchain size is updated.
• The number of swapchain images used should generally be less than the KHR swapchain except in cases where there either compositor backpressure (where the client is able to wait on the semaphore till the internal count is reached), and when the backpressure subsides (where there are more recyclables generated but they haven't yet expired). To ameliorate the memory usage in case of the latter, the recyclables count will be less than the total number of allowable images in flight.
• The format of the swapchain buffers is always RGBA8888. This may allow for earlier creation of the pipeline state objects since we no longer have to wait for surface acquisition to find the root surface format.
• In terms of code organization, the old swapchains have been renamed to KHRSwapchainVK while the replacements are called AHBSwapchainVK. Corresponding utilities and dependencies have been renamed.
• In order to make working with Android handles easier, a new toolkit has been introduced in //impeller/toolkit/android. This adds type-safe, reference-counted wrappers around the Android handles with lazy proc table setup.
• These new swapchains may only be used on Android API 29 and above. This is the baseline for Impeller Vulkan on Android. So the KHR swapchains should never be used on Android. Instead, non-Android platforms will use this.
• There is some recycling of device transient textures in the KHR swapchains that is being reproduced by caching the entire render target in the AHB swapchains.

[chinmaygarde]

Still WIP but good for an initial review. The current swapchains create a new impeller::Surface around each image per acquire. I am attempting to reuse the surface as well as the render target so the device transient textures are also saved (I think @jonahwilliams was trying to do something similar). But I believe I am running into the same issues as https://github.com/flutter/engine/pull/51208. The recycling is not done yet because of this issue.

[jonahwilliams]

On the current swapchain implementation, the MSAA texture is cached on the swapchain implementation and not the surface. I have https://github.com/flutter/engine/pull/51206 which adds the depth stencil to that list too.

[chinmaygarde]

You are referring to the HasMSAATexture sidecar texture right? I must admit I don't remember why I setup the surface to be created each time a swapchain image is acquired. Wouldn't keeping it around and its rendertarget solve all caching issues?

[jonahwilliams]

Yeah something like that would work. I think we can actually make all surfaces share the same MSAA/Depth+Stencil attachment, which should reduce memory usage for the devices without lazily allocated memory support.

[chinmaygarde]

I think we can actually make all surfaces share the same MSAA/Depth+Stencil attachment, which should reduce memory usage for the devices without lazily allocated memory support.

Oh, yeah. That's a good idea.

I am also wondering if I am trying to boil the ocean here. I can just do the same thing the current swapchain is doing but that was causing code repetition. Can clean it up in a later attempt though.

[jonahwilliams]

I'm definitely partial to smaller changes, ideally separating out renaming/moving with adding the new code.

[chinmaygarde]

I'm going to leave all the caching stuff in the existing swapchains alone.

[jonahwilliams]

FWIW this crashes on a Pixel 4 immediately:

A Dart VM Service on Pixel 4 XL is available at: http://127.0.0.1:64174/DN2SNbWi1nw=/
The Flutter DevTools debugger and profiler on Pixel 4 XL is available at: http://127.0.0.1:9102?uri=http://127.0.0.1:64174/DN2SNbWi1nw=/
I/AdrenoGLES-0(28633): QUALCOMM build                   : 85da404, I46ff5fc46f
I/AdrenoGLES-0(28633): Build Date                       : 11/30/20
I/AdrenoGLES-0(28633): OpenGL ES Shader Compiler Version: EV031.31.04.01
I/AdrenoGLES-0(28633): Local Branch                     : promo490_3_Google
I/AdrenoGLES-0(28633): Remote Branch                    : 
I/AdrenoGLES-0(28633): Remote Branch                    : 
I/AdrenoGLES-0(28633): Reconstruct Branch               : 
I/AdrenoGLES-0(28633): Build Config                     : S P 10.0.4 AArch64
I/AdrenoGLES-0(28633): Driver Path                      : /vendor/lib64/egl/libGLESv2_adreno.so
I/AdrenoGLES-0(28633): PFP: 0x016ee190, ME: 0x00000000
W/AdrenoUtils(28633): <ReadGpuID_from_sysfs:197>: Failed to open /sys/class/kgsl/kgsl-3d0/gpu_model
W/AdrenoUtils(28633): <ReadGpuID:221>: Failed to read chip ID from gpu_model. Fallback to use the GSL path
I/Choreographer(28633): Skipped 38 frames!  The application may be doing too much work on its main thread.
I/flutter (28633): [IMPORTANT:flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_vk.cc(155)] Created surface.
I/flutter (28633): [IMPORTANT:flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_vk.cc(155)] Created surface.
I/flutter (28633): [IMPORTANT:flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_vk.cc(214)] There are 1 recyclables.
I/flutter (28633): [IMPORTANT:flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_vk.cc(150)] Recycled surface.
E/flutter (28633): [ERROR:flutter/impeller/base/validation.cc(57)] Break on 'ImpellerValidationBreak' to inspect point of failure: 
E/flutter (28633): --- Vulkan Debug Report  ----------------------------------------
E/flutter (28633): |                Severity: Error
E/flutter (28633): |                    Type: { Validation }
E/flutter (28633): |                 ID Name: VUID-VkRenderPassBeginInfo-framebuffer-parameter
E/flutter (28633): |               ID Number: -533747612
E/flutter (28633): |       Queue Breadcrumbs: [NONE]
E/flutter (28633): |  CMD Buffer Breadcrumbs: [NONE]
E/flutter (28633): |         Related Objects: RenderPass [452998790644124] [EntityPass Render Pass: Depth=0 Count=0], Framebuffer [454098302271901] [UNNAMED], ImageView [451899279016347] [UNNAMED]
E/flutter (28633): |                 Trigger: Validation Error: [ VUID-VkRenderPassBeginInfo-framebuffer-parameter ] Object 0: handle = 0x19c000000019c, name = EntityPass Render Pass: Depth=0 Count=0, type = VK_OBJECT_TYPE_RENDER_PASS; Object 1: handle = 0x19d000000019d, type = VK_OBJECT_TYPE_FRAMEBUFFER; Object 2: handle = 0x19b000000019b, type = VK_OBJECT_TYPE_IMAGE_VIEW; | MessageID = 0xe02fa864 | vkCmdBeginRenderPass(): pCreateInfo->pAttachments[2] VkImageView 0x19b000000019b[] is invalid. The Vulkan spec states: framebuffer must be a valid VkFramebuffer handle (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkRenderPassBeginInfo-framebuffer-parameter)

[chinmaygarde]

That was the render target caching. I’ve backed out the recycling. Try now. It will be slow though because no recycling.

On Tue, Mar 5, 2024 at 4:21 PM Jonah Williams @.***> wrote:

FWIW this crashes on a Pixel 4 immediately:

A Dart VM Service on Pixel 4 XL is available at: http://127.0.0.1:64174/DN2SNbWi1nw=/ The Flutter DevTools debugger and profiler on Pixel 4 XL is available at: http://127.0.0.1:9102?uri=http://127.0.0.1:64174/DN2SNbWi1nw=/ I/AdrenoGLES-0(28633) http://127.0.0.1:9102?uri=http://127.0.0.1:64174/DN2SNbWi1nw=/I/AdrenoGLES-0(28633): QUALCOMM build : 85da404, I46ff5fc46f I/AdrenoGLES-0(28633): Build Date : 11/30/20 I/AdrenoGLES-0(28633): OpenGL ES Shader Compiler Version: EV031.31.04.01 I/AdrenoGLES-0(28633): Local Branch : promo490_3_Google I/AdrenoGLES-0(28633): Remote Branch : I/AdrenoGLES-0(28633): Remote Branch : I/AdrenoGLES-0(28633): Reconstruct Branch : I/AdrenoGLES-0(28633): Build Config : S P 10.0.4 AArch64 I/AdrenoGLES-0(28633): Driver Path : /vendor/lib64/egl/libGLESv2_adreno.so I/AdrenoGLES-0(28633): PFP: 0x016ee190, ME: 0x00000000 W/AdrenoUtils(28633): <ReadGpuID_from_sysfs:197>: Failed to open /sys/class/kgsl/kgsl-3d0/gpu_model W/AdrenoUtils(28633): ReadGpuID:221: Failed to read chip ID from gpu_model. Fallback to use the GSL path I/Choreographer(28633): Skipped 38 frames! The application may be doing too much work on its main thread. I/flutter (28633): [IMPORTANT:flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_vk.cc(155)] Created surface. I/flutter (28633): [IMPORTANT:flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_vk.cc(155)] Created surface. I/flutter (28633): [IMPORTANT:flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_vk.cc(214)] There are 1 recyclables. I/flutter (28633): [IMPORTANT:flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_vk.cc(150)] Recycled surface. E/flutter (28633): [ERROR:flutter/impeller/base/validation.cc(57)] Break on 'ImpellerValidationBreak' to inspect point of failure: E/flutter (28633): --- Vulkan Debug Report ---------------------------------------- E/flutter (28633): | Severity: Error E/flutter (28633): | Type: { Validation } E/flutter (28633): | ID Name: VUID-VkRenderPassBeginInfo-framebuffer-parameter E/flutter (28633): | ID Number: -533747612 E/flutter (28633): | Queue Breadcrumbs: [NONE] E/flutter (28633): | CMD Buffer Breadcrumbs: [NONE] E/flutter (28633): | Related Objects: RenderPass [452998790644124] [EntityPass Render Pass: Depth=0 Count=0], Framebuffer [454098302271901] [UNNAMED], ImageView [451899279016347] [UNNAMED] E/flutter (28633): | Trigger: Validation Error: [ VUID-VkRenderPassBeginInfo-framebuffer-parameter ] Object 0: handle = 0x19c000000019c, name = EntityPass Render Pass: Depth=0 Count=0, type = VK_OBJECT_TYPE_RENDER_PASS; Object 1: handle = 0x19d000000019d, type = VK_OBJECT_TYPE_FRAMEBUFFER; Object 2: handle = 0x19b000000019b, type = VK_OBJECT_TYPE_IMAGE_VIEW; | MessageID = 0xe02fa864 | vkCmdBeginRenderPass(): pCreateInfo->pAttachments[2] VkImageView 0x19b000000019b[] is invalid. The Vulkan spec states: framebuffer must be a valid VkFramebuffer handle (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkRenderPassBeginInfo-framebuffer-parameter)

— Reply to this email directly, view it on GitHub https://github.com/flutter/engine/pull/51213#issuecomment-1979856909, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAKYNNNHWGULD2YWOK3UZ3YWZORHAVCNFSM6AAAAABEH6FJZSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZZHA2TMOJQHE . You are receiving this because you authored the thread.Message ID: @.***>

[jonahwilliams]

Now it occassionally freezes on the "backpressure" step or flashes black:

https://github.com/flutter/engine/assets/8975114/5a583144-74f8-44c1-9d8f-672b9bf34aa4

[jonahwilliams]

Also if I rotate I get:

E/flutter (29648): [ERROR:flutter/impeller/base/validation.cc(57)] Break on 'ImpellerValidationBreak' to inspect point of failure: New surface size is not allocatable.

[chinmaygarde]

Yeah, both should be fixed. I still haven't figure out if I can depend on Android to always fire the OnComplete callback too. If not, there is a chance of deadlock and our semaphores don't allow for timeouts. I'm just going to patch that in for safety.

https://github.com/flutter/engine/assets/44085/03e17eb4-57aa-41a2-b6a2-5e569afc9c25

[chinmaygarde]

if I can depend on Android to always fire the OnComplete

This isn't documented. But following along the Android code seems to indicate that it does.

[chinmaygarde]

This will not land as-is. I am splitting it up into smaller patches.

[jonahwilliams]

Not crashing anymore! That said, we need to have at least recyclying of the underlying AHBs implemented, otherwise re-allocation of these starts to dominate the per frame costs:

[chinmaygarde]

Yeah. Recycling is table stakes. I'm landing bits and pieces of this patch at the moment and I still expect to have a flag for comparison.

[jonahwilliams]

I would also start thinking about how we test this - what it means to test it. We have some capability to unit test android code on CI too.

[chinmaygarde]

I added a unit-test for just the toolkit in this patch. I am able to create hardware buffers in a unit-test just fine. We can test the minutia in that harness or another just like it. Presenting them in a window would be an integration test I suppose and would be covered by everything else.

[chinmaygarde]

I just realized we can also move the binder transaction (SurfaceTransaction::Apply) to a background thread.

[flutter-dashboard[bot]]

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

[chinmaygarde]

Closing this as most of it has landed and the new patch isn't going to build on this.

[chinmaygarde]

xref https://github.com/flutter/engine/pull/52087