-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
Conversation
This is called programmable blending, which looks like it is only supported on "Apple" GPU families |
OK, I've got runtime checks working correctly, and confirmed we can use programmable blending on M1 and M2 devices. Yay! |
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. |
There was a problem hiding this 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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
I think the iOS simulator might be lying about its support for this API 😆 |
So, its simulating the device in the response for support but not actually supporting it :/ |
Or I wonder if we're picking up the real macOS metal device but the simulator itself has different shims or validation layers? 🤷♂️ |
…untime capabilties check based on metal GPU family. (flutter/engine#40124)
…untime capabilties check based on metal GPU family. (flutter/engine#40124)
…untime capabilties check based on metal GPU family. (flutter/engine#40124)
…untime capabilties check based on metal GPU family. (flutter/engine#40124)
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