Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] Remove support of toggling Vulkan playgrounds in addition to enabling that backend. #50561

Closed
chinmaygarde wants to merge 1 commit into from
Closed

[Impeller] Remove support of toggling Vulkan playgrounds in addition to enabling that backend. #50561

chinmaygarde wants to merge 1 commit into from

Conversation

@chinmaygarde
Copy link
Member

@chinmaygarde chinmaygarde commented Feb 12, 2024

Now, you get playgrounds no all backends if you enable that backend.

Second attempt at #49949

@chinmaygarde
[Impeller] Remove support of toggling Vulkan playgrounds in addition …
b8f95d6 
…to enabling that backend.

Now, you get playgrounds no all backends if you enable that backend.

Second attempt at flutter#49949
@chinmaygarde
Copy link
Member Author

chinmaygarde commented Feb 12, 2024

Lets see if CI is happy this time.

gaaclarke
gaaclarke reviewed Feb 12, 2024
View reviewed changes
impeller/tools/impeller.gni
Comment on lines -32 to -33
impeller_enable_vulkan_playgrounds =
(is_linux || is_win || is_android) && target_os != "fuchsia"
Copy link
Member

@gaaclarke gaaclarke Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might have to do impeller_enable_vulkan_playgrounds = impeller_enable_vulkan for now. You might be turning the playgrounds on without compiling in vulkan?

Copy link
Member Author

@chinmaygarde chinmaygarde Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the ifdefs for impeller_enable_vulkan_playgrounds too. So IMPELLER_ENABLE_VULKAN is the only flag gating Vulkan playgrounds.

@chinmaygarde
Copy link
Member Author

chinmaygarde commented Feb 12, 2024

Ok, there are still some validation errors on CI we missed.

@chinmaygarde chinmaygarde mentioned this pull request Feb 12, 2024
Merged
@chinmaygarde
Copy link
Member Author

chinmaygarde commented Feb 12, 2024 

[ RUN      ] Play/EntityTest.SpecializationConstantsAreAppliedToVariants/Vulkan
[INFO:flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc(51)] Vulkan validations are enabled.
[FATAL:flutter/impeller/base/validation.cc(47)] Break on 'ImpellerValidationBreak' to inspect point of failure: 
--- Vulkan Debug Report  ----------------------------------------
|                Severity: Error
|                    Type: { Validation }
|                 ID Name: VUID-VkAttachmentDescription-format-06698
|               ID Number: -1840906592
|       Queue Breadcrumbs: [NONE]
|  CMD Buffer Breadcrumbs: [NONE]
|         Related Objects: Device [140298079502352] [ImpellerDevice]
|                 Trigger: Validation Error: [ VUID-VkAttachmentDescription-format-06698 ] Object 0: handle = 0x7f99b1310010, name = ImpellerDevice, type = VK_OBJECT_TYPE_DEVICE; | MessageID = 0x9245fea0 | vkCreateRenderPass: pCreateInfo->pAttachments[0].format is VK_FORMAT_UNDEFINED. The Vulkan spec states: format must not be VK_FORMAT_UNDEFINED (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkAttachmentDescription-format-06698)
-----------------------------------------------------------------

@jonahwilliams
Copy link
Contributor

jonahwilliams commented Feb 12, 2024

If I run that test locally on macOS I get:

jonahwilliams-macbookpro3:flutter jonahwilliams$ ../out/host_debug/impeller_unittests --gtest_filter=*SpecializationConstantsAreAppliedToVariants*Vulkan --enable_playground
[INFO:flutter/testing/test_timeout_listener.cc(76)] Test timeout of 300 seconds per test case will be enforced.
Note: Google Test filter = *SpecializationConstantsAreAppliedToVariants*Vulkan
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from Play/EntityTest
[ RUN      ] Play/EntityTest.SpecializationConstantsAreAppliedToVariants/Vulkan
[INFO:flutter/impeller/renderer/backend/vulkan/capabilities_vk.cc(51)] Vulkan validations are enabled.
[mvk-error] VK_ERROR_FORMAT_NOT_SUPPORTED: vkCreateRenderPass(): Attachment format VK_FORMAT_UNDEFINED is not supported on this device.
[mvk-error] VK_ERROR_FORMAT_NOT_SUPPORTED: vkCreateRenderPass(): Attachment format VK_FORMAT_UNDEFINED on this device does not support the VkFormat attachment capabilities required by the subpass at index 0.
[FATAL:flutter/impeller/base/validation.cc(47)] Break on 'ImpellerValidationBreak' to inspect point of failure: Failed to create render pass: ErrorFormatNotSupported
Abort trap: 6

I think this is a problem with the compatibility render pass on the advanced blend shaders?

@jonahwilliams
Copy link
Contributor

jonahwilliams commented Feb 12, 2024

I would recommend filling a bug and adding a skip to test if possible, @gaaclarke did something similar for the golden validations and I believe we managed to fix all of those.

@gaaclarke
Copy link
Member

gaaclarke commented Feb 12, 2024

Ahh yea, EntityTests. We fixed all the validations (or skipped them) when running golden tests, but that doesn't include EntityTests.

@jonahwilliams
Copy link
Contributor

jonahwilliams commented Feb 12, 2024

This feels like a moltenvk issue...

@chinmaygarde chinmaygarde added the Work in progress (WIP) Not ready (yet) for review! label Feb 15, 2024
@chinmaygarde
Copy link
Member Author

chinmaygarde commented Feb 15, 2024

I believe we have already filed issues for these validation errors. Don't have the cycles to work on fixing them this week. Adding WIP tag.

@chinmaygarde
Copy link
Member Author

chinmaygarde commented Feb 29, 2024

We still have validation errors on this issue. I'm stashing this work for now. Will followup the week from next.

@chinmaygarde chinmaygarde deleted the no_vk_playgrounds_flag branch August 22, 2024 19:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@gaaclarke gaaclarke gaaclarke left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

e: impeller Work in progress (WIP) Not ready (yet) for review!

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chinmaygarde @jonahwilliams @gaaclarke