[go: nahoru, domu]

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] Replace FML_OS_PHYSICAL_IOS compile check with runtime capabilties check based on metal GPU family. #40124

Merged
merged 7 commits into from
Mar 8, 2023

Conversation

jonahwilliams
Copy link
Member
@jonahwilliams jonahwilliams commented Mar 7, 2023

Removes all usage of FML_OS_PHYSICAL_IOS. Instead, we create a new capabilities check and set this in the metal backend based on the metal GPU family. This allows programmable blending to work on M1 and M2 devices in additon to iOS devices.

The entity_pass logic that relied on FML_OS_PHYSICAL_IOS checks has been refactored to track whether an advanced blend or backdrop filter was used separately. This allows us to determine whether or not to end the pass when we have access to the device capabilities. I could have turned the compile time checks into runtime checks, but then I'd need to plumb device capabilities into the Canvas.

Fixes flutter/flutter#121886
Also part of flutter/flutter#120223 by enabling this on some macOS machines

@jonahwilliams jonahwilliams self-assigned this Mar 7, 2023
@jonahwilliams jonahwilliams changed the title [Impeller] reduce quantity of FML_OS_PHYSICAL_IOS checks [Impeller] Reduce quantity of FML_OS_PHYSICAL_IOS checks. Mar 7, 2023
@jonahwilliams
Copy link
Member Author

@jonahwilliams
Copy link
Member Author

This is called programmable blending, which looks like it is only supported on "Apple" GPU families

@jonahwilliams
Copy link
Member Author

OK, I've got runtime checks working correctly, and confirmed we can use programmable blending on M1 and M2 devices. Yay!

@jonahwilliams jonahwilliams changed the title [Impeller] Reduce quantity of FML_OS_PHYSICAL_IOS checks. [Impeller] Replace FML_OS_PHYSICAL_IOS compile check with runtime capabilties check based on metal GPU family Mar 7, 2023
@jonahwilliams jonahwilliams changed the title [Impeller] Replace FML_OS_PHYSICAL_IOS compile check with runtime capabilties check based on metal GPU family [Impeller] Replace FML_OS_PHYSICAL_IOS compile check with runtime capabilties check based on metal GPU family. Mar 7, 2023
@jonahwilliams jonahwilliams marked this pull request as ready for review March 8, 2023 00:12
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Copy link
Member
@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM with a minor nit on naming.

@@ -23,6 +23,8 @@ class IDeviceCapabilities {

bool SupportsTextureToTextureBlits() const;

bool SupportsFramebufferBlending() const;
Copy link
Member

Choose a reason for hiding this comment

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

SupportsFramebufferFetch maybe? That we use it for blending perhaps is not related to the feature being supported.

Copy link
Member

Choose a reason for hiding this comment

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

I just saw you comments on what Apple calls it. SGTM.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could call it programmable blending or framebuffer fetch. Your call, it doesn't really matter to me. Though it might be different than programmable blending on opengl/vulkan.

Copy link
Member

Choose a reason for hiding this comment

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

My vote if for framebuffer fetch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@jonahwilliams
Copy link
Member Author

I think the iOS simulator might be lying about its support for this API 😆

@chinmaygarde
Copy link
Member

So, its simulating the device in the response for support but not actually supporting it :/

@jonahwilliams
Copy link
Member Author

Or I wonder if we're picking up the real macOS metal device but the simulator itself has different shims or validation layers? 🤷‍♂️

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 8, 2023
@auto-submit auto-submit bot merged commit e763087 into flutter:main Mar 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 8, 2023
@jonahwilliams jonahwilliams deleted the special_blend branch March 9, 2023 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller needs tests
Projects
No open projects
Archived in project
2 participants