[Impeller] For Android hardware buffers on Vulkan, use an alpha value of 1 if the buffer format always has opaque alpha#182974
Conversation
… of 1 if the buffer format always has opaque alpha The AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM format specifies that the alpha value should be ignored and should always be treated as opaque. For this format, configure the VkComponentMapping to set alpha to VK_COMPONENT_SWIZZLE_ONE. Fixes flutter#182825
There was a problem hiding this comment.
Code Review
This pull request correctly implements a fix for handling opaque alpha with AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM on Android by setting the alpha component swizzle to VK_COMPONENT_SWIZZLE_ONE. The changes include refactoring several free functions into static members of the AHBTextureSourceVK class, which improves code organization. A new unit test has been added to verify the correctness of the fix. My review includes feedback on a resource leak in the new unit test and a violation of the repository's style guide regarding documentation for new public methods.
| EXPECT_EQ(AHardwareBuffer_isSupported(&desc), 1); | ||
|
|
||
| AHardwareBuffer* buffer = nullptr; | ||
| ASSERT_EQ(AHardwareBuffer_allocate(&desc, &buffer), 0); |
There was a problem hiding this comment.
The AHardwareBuffer allocated here is not released, which will cause a resource leak in the test. Please ensure it's released before the function exits, for example by calling AHardwareBuffer_release(buffer) before context_vk->Shutdown(). Using an RAII wrapper would be even better to handle cleanup in case of test assertion failures.
| static vk::UniqueImage CreateVKImageWrapperForAndroidHarwareBuffer( | ||
| const vk::Device& device, | ||
| const AHBProperties& ahb_props, | ||
| const AHardwareBuffer_Desc& ahb_desc); | ||
|
|
||
| static ImageViewInfo CreateImageViewInfo( | ||
| const vk::Image& image, | ||
| const std::shared_ptr<YUVConversionVK>& yuv_conversion_wrapper, | ||
| const AHBProperties& ahb_props, | ||
| const AHardwareBuffer_Desc& ahb_desc); |
There was a problem hiding this comment.
These new public static methods should have documentation as per the repository style guide. Please add /// doc comments explaining what these functions do, their parameters, and what they return.
References
- All public members should have documentation. (link)
gaaclarke
left a comment
There was a problem hiding this comment.
Looks good for the most part, just some minor notes. Thanks jason.
| } | ||
|
|
||
| return std::move(image.value); | ||
| bool HardwareBufferFormatHasOpaqueAlpha(AHardwareBuffer_Format format) { |
There was a problem hiding this comment.
| bool HardwareBufferFormatHasOpaqueAlpha(AHardwareBuffer_Format format) { | |
| bool IsOpaque(AHardwareBuffer_Format format) { |
| return format == AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM; | ||
| } | ||
|
|
||
| using AHBProperties = AHBTextureSourceVK::AHBProperties; |
There was a problem hiding this comment.
nit: put at the beginning of the scope
| return {}; | ||
| } | ||
|
|
||
| return std::move(image.value); |
There was a problem hiding this comment.
I don't think you want to move in the return statement, that subverts copy elision.
There was a problem hiding this comment.
std::move is necessary because vk::UniqueImage does not have a copy constructor
| static vk::UniqueImage CreateVKImageWrapperForAndroidHarwareBuffer( | ||
| const vk::Device& device, | ||
| const AHBProperties& ahb_props, | ||
| const AHardwareBuffer_Desc& ahb_desc); | ||
|
|
||
| static ImageViewInfo CreateImageViewInfo( | ||
| const vk::Image& image, | ||
| const std::shared_ptr<YUVConversionVK>& yuv_conversion_wrapper, | ||
| const AHBProperties& ahb_props, | ||
| const AHardwareBuffer_Desc& ahb_desc); |
| EXPECT_EQ(AHardwareBuffer_isSupported(&desc), 1); | ||
|
|
||
| AHardwareBuffer* buffer = nullptr; | ||
| ASSERT_EQ(AHardwareBuffer_allocate(&desc, &buffer), 0); |
| auto image_info = AHBTextureSourceVK::CreateImageViewInfo( | ||
| image.get(), nullptr, ahb_props, desc); | ||
|
|
||
| ASSERT_EQ(image_info.get().components.a, vk::ComponentSwizzle::eOne); |
There was a problem hiding this comment.
| ASSERT_EQ(image_info.get().components.a, vk::ComponentSwizzle::eOne); | |
| EXPECT_EQ(image_info.get().components.a, vk::ComponentSwizzle::eOne); |
…lpha value of 1 if the buffer format always has opaque alpha (flutter/flutter#182974)
…lpha value of 1 if the buffer format always has opaque alpha (flutter/flutter#182974)
…lpha value of 1 if the buffer format always has opaque alpha (flutter/flutter#182974)
…lpha value of 1 if the buffer format always has opaque alpha (flutter/flutter#182974)
…lpha value of 1 if the buffer format always has opaque alpha (flutter/flutter#182974)
…lpha value of 1 if the buffer format always has opaque alpha (flutter/flutter#182974)
…lpha value of 1 if the buffer format always has opaque alpha (flutter/flutter#182974)
…lpha value of 1 if the buffer format always has opaque alpha (flutter/flutter#182974)
…lpha value of 1 if the buffer format always has opaque alpha (flutter/flutter#182974)
…lpha value of 1 if the buffer format always has opaque alpha (flutter/flutter#182974)
Roll Flutter from 1141b2bdce66 to 46fb7210422d (38 revisions) flutter/flutter@1141b2b...46fb721 2026-03-02 [email protected] [rules] Fix a few issues in the full-length rules file (flutter/flutter#182725) 2026-03-02 [email protected] Roll Dart SDK from 8063f5f5ac60 to e86dbe9aa742 (1 revision) (flutter/flutter#183120) 2026-03-02 [email protected] [web] Roll Chrome to 145 (flutter/flutter#182860) 2026-03-02 [email protected] Roll Skia from 61c0e71760f5 to e180358b7a7a (1 revision) (flutter/flutter#183118) 2026-03-02 [email protected] Roll Packages from a27d7c5 to faa4e22 (4 revisions) (flutter/flutter#183117) 2026-03-02 [email protected] Add information to issue triage page (flutter/flutter#182145) 2026-03-02 [email protected] Roll Skia from cc8ce92481f2 to 61c0e71760f5 (2 revisions) (flutter/flutter#183103) 2026-03-02 [email protected] Roll Skia from 4cf3cd27b620 to cc8ce92481f2 (1 revision) (flutter/flutter#183100) 2026-03-02 [email protected] Roll Fuchsia Linux SDK from zN2ZV9QD0LD8acUFF... to 0dCDM2oORHwDf_pyb... (flutter/flutter#183101) 2026-03-01 [email protected] Update fl_texture_gl.h (flutter/flutter#182999) 2026-03-01 [email protected] Roll Skia from be1362b5ca4e to 4cf3cd27b620 (1 revision) (flutter/flutter#183096) 2026-03-01 [email protected] Roll Skia from b9210eb7005f to be1362b5ca4e (1 revision) (flutter/flutter#183092) 2026-03-01 [email protected] Timeout when waiting for the correct sized frame from Flutter. (flutter/flutter#182971) 2026-03-01 [email protected] Roll Skia from 28172a4e03af to b9210eb7005f (1 revision) (flutter/flutter#183088) 2026-03-01 [email protected] Roll Fuchsia Linux SDK from D7IYacJUCpvc_1iU_... to zN2ZV9QD0LD8acUFF... (flutter/flutter#183076) 2026-02-28 [email protected] Roll Dart SDK from cdf45eaf995e to 8063f5f5ac60 (1 revision) (flutter/flutter#183064) 2026-02-28 [email protected] Roll Dart SDK from 54451fcdbcf9 to cdf45eaf995e (1 revision) (flutter/flutter#183057) 2026-02-28 [email protected] Roll Skia from c8bcc27f5319 to 28172a4e03af (3 revisions) (flutter/flutter#183056) 2026-02-28 [email protected] Roll Dart SDK from 148d91b8a603 to 54451fcdbcf9 (2 revisions) (flutter/flutter#183051) 2026-02-28 [email protected] [A11y] in calendar date picker, remove SemanticsService.sendAnnouncement usage for android. (flutter/flutter#182918) 2026-02-28 [email protected] Add desktop review teams (flutter/flutter#182972) 2026-02-28 [email protected] [framework] Fix Text.semanticsIdentifier being absorbed by ancestor nodes (flutter/flutter#181795) 2026-02-28 [email protected] Roll Skia from b150186d3e23 to c8bcc27f5319 (5 revisions) (flutter/flutter#183032) 2026-02-28 [email protected] [Impeller] For Android hardware buffers on Vulkan, use an alpha value of 1 if the buffer format always has opaque alpha (flutter/flutter#182974) 2026-02-27 [email protected] Adds float32 output to `Image.toByteData()` in float32 Image (flutter/flutter#182847) 2026-02-27 [email protected] Roll Dart SDK from 1cdb7dfd913e to 148d91b8a603 (1 revision) (flutter/flutter#183025) 2026-02-27 [email protected] Roll Fuchsia Linux SDK from G1GwOdVt5bM7GjMSY... to D7IYacJUCpvc_1iU_... (flutter/flutter#183021) 2026-02-27 [email protected] When impellerc fails with a long shader compilation error, truncate it and output to a file (flutter/flutter#182786) 2026-02-27 [email protected] Add missing mutation-safe delegate iteration and use idomatic syntax (flutter/flutter#183018) 2026-02-27 [email protected] Exclude arm64 if any dependencies do and print warning when using Xcode 26 (flutter/flutter#182913) 2026-02-27 [email protected] Ignore unawaited_futures lint in dev/automated_tests (flutter/flutter#182922) 2026-02-27 [email protected] licenses_cpp: pre-land changes for perfetto update (flutter/flutter#182965) 2026-02-27 [email protected] Re-add extended attribute removed by SwiftPM (flutter/flutter#183011) 2026-02-27 [email protected] Fixes future warning for `await`ing `Future` returns in `async` bodies inside `try` blocks (flutter/flutter#182301) 2026-02-27 [email protected] Roll Skia from ed220c490eea to b150186d3e23 (2 revisions) (flutter/flutter#183014) 2026-02-27 [email protected] Fix issue where web embedder is synthesizing key up events too eagerly (flutter/flutter#180692) 2026-02-27 [email protected] chore: Don't unconditionally check tools/gn formatting (flutter/flutter#182973) 2026-02-27 [email protected] Roll Packages from e1d0169 to a27d7c5 (8 revisions) (flutter/flutter#183009) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose ...
… of 1 if the buffer format always has opaque alpha (flutter#182974) The AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM format specifies that the alpha value should be ignored and should always be treated as opaque. For this format, configure the VkComponentMapping to set alpha to VK_COMPONENT_SWIZZLE_ONE. Fixes flutter#182825
The AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM format specifies that the alpha value should be ignored and should always be treated as opaque. For this format, configure the VkComponentMapping to set alpha to VK_COMPONENT_SWIZZLE_ONE.
Fixes #182825