[Impeller] Fixes to YUV imports on Android, Incomplete read of pipeline cache data, missing enabled extensions. by jonahwilliams · Pull Request #164744 · flutter/flutter

jonahwilliams · 2025-03-06T22:00:40Z

Handle textures that require a YUV import but aren't an undefined format.
INCOMPLETE is actually a success case for the pipeline cache. CERTAIN drivers ALWAYS return incomplete, even when they wrote all the data. Probably an off by one or something like that...
Ensures Optional AndroidExtensions are enabled
Only creates a YUV conversion if necessary

gaaclarke

lgtm, what are our options for tests?

gaaclarke · 2025-03-07T19:26:56Z

engine/src/flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk.cc


+namespace {
+
+static bool RequiresYCBCRConversion(vk::Format format) {


nit: static isn't necessary, you are in an anonymous namespace

gaaclarke · 2025-03-07T19:27:31Z

engine/src/flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk.cc

+
+static bool RequiresYCBCRConversion(vk::Format format) {
+  switch (format) {
+    case vk::Format::eG8B8R83Plane420Unorm:


add [[fallthrough]]

https://google.github.io/styleguide/cppguide.html#Switch_Statements

AFAIK from reading this we only need [[fallthrough]] if there is a body, not when we group multiple cases together?

Sorry, you're right. Didn't point this out to me the other day?

jonahwilliams · 2025-03-07T19:46:15Z

Not great, but possible via mocked vulkan. I think for the enabled extensions, we can run the hcpp tests w/ vulkan validation enabled.

jonahwilliams · 2025-03-07T22:08:27Z

I'm seeing if I can get vulkan tests running on Android so we don't have to mock (everything) out.

engine/src/flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk_unittests.cc

gaaclarke

lgtm

jonahwilliams · 2025-03-10T18:40:13Z

Tests added!

gaaclarke · 2025-03-10T19:45:30Z

engine/src/flutter/ci/builders/linux_android_emulator.json

+                    "flutter/impeller/renderer/backend/vulkan:vulkan_android_unittests",
+                    "flutter/impeller/renderer/backend/vulkan:vulkan_android_apk_unittests",


This is just building the tests right? it isn't executing them.

correct, but the tests are run as part of the existing android test shard as I've added them here: https://github.com/flutter/flutter/pull/164744/files#diff-208f1d19de16cc0af95bc6c1c374cde2f450d73098b217e66b3cf98f1b097b3cR767

got it, awesome

jonahwilliams · 2025-03-10T20:17:07Z

And of course we can see the tests are running because they failed on the API 28 emulator due to a missing cap check :)

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

…ne cache data, missing enabled extensions. (flutter#164744) - Handle textures that require a YUV import but aren't an undefined format. - INCOMPLETE is actually a success case for the pipeline cache. CERTAIN drivers ALWAYS return incomplete, even when they wrote all the data. Probably an off by one or something like that... - Ensures Optional AndroidExtensions are enabled - Only creates a YUV conversion if necessary

[Impeller] assorted Vk Fixes.

d5cdb48

github-actions bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Mar 6, 2025

jonahwilliams added 7 commits March 6, 2025 15:17

more fixes.

016609e

fixes in case cache size is too small.:

c72c471

++

1704c63

fix write size.

a4655f0

revert pipeline cache changes.

8f5a50e

++

9e40bcc

++

6bb3e86

jonahwilliams marked this pull request as ready for review March 7, 2025 19:17

jonahwilliams requested a review from gaaclarke March 7, 2025 19:18

gaaclarke reviewed Mar 7, 2025

View reviewed changes

add test configuration to run vulkan tests on Android.

e614580

jonahwilliams added 2 commits March 7, 2025 14:20

gn fixes.

fda5d80

++

568a55e

jonahwilliams commented Mar 8, 2025

View reviewed changes

engine/src/flutter/impeller/renderer/backend/vulkan/android/ahb_texture_source_vk_unittests.cc Outdated Show resolved Hide resolved

remove runIf.

1c0b831

gaaclarke approved these changes Mar 10, 2025

View reviewed changes

add unit tests of import of AHBs.

b56808a

gaaclarke reviewed Mar 10, 2025

View reviewed changes

++

9daa37d

jonahwilliams changed the title ~~[Impeller] assorted Vk Fixes.~~ [Impeller] Fixes to YUV imports on Android, Incomplete read of pipeline cache data, missing enabled extensions. Mar 10, 2025

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

auto-submit bot added this pull request to the merge queue Mar 10, 2025

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

e0b349d

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

0371290

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

d716d0a

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

733ee72

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

aa6777a

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 17, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

f3dea59

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 20, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

a44cbeb

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 25, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

412f98e

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 25, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

b0055e2

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

1ee2904

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

9b7fa41

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

587e505

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

bbbf5cb

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

66e6506

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

bf62c5d

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

cc35d91

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 26, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

75a9984

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

33ec8c6

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

a27ca3a

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

bc89161

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

6d4f8e2

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

0d20086

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

0f5c34e

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

25d397c

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 27, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

d27c975

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 28, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

7ef8770

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

724b1ef

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

d4461eb

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025

95e1012 [Impeller] Fixes to YUV imports on Android, Incomplete read o…

63eadcf

…f pipeline cache data, missing enabled extensions. (flutter/flutter#164744)


		namespace {

		static bool RequiresYCBCRConversion(vk::Format format) {

		"flutter/impeller/renderer/backend/vulkan:vulkan_android_unittests",
		"flutter/impeller/renderer/backend/vulkan:vulkan_android_apk_unittests",

Conversation

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

Uh oh!

Uh oh!

gaaclarke 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!

jonahwilliams commented Mar 7, 2025

Uh oh!

jonahwilliams commented Mar 7, 2025

Uh oh!

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Mar 10, 2025

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 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jonahwilliams commented Mar 6, 2025 •

edited

Loading