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] libImpeller: Add support for Metal and Vulkan rendering. #56906

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

chinmaygarde
Copy link
Member

@chinmaygarde chinmaygarde commented Dec 2, 2024

  • Adds context creation and WSI routines for Metal and Vulkan.
  • Enables all tests for the Metal, Vulkan, and OpenGLES backends.
  • Separate standalone examples for Metal, Vulkan, and OpenGLES have been created. These will be packaged with the SDK.
    • Disallows the use of OpenGL ES on macOS.
  • All new public methods are documented.
  • The SDK version number has been bumped.
  • Some incorrect nullability annotations were patched.
  • Tests harness is overhauled to reuse the same underlying context as the playgrounds.
  • The C++ public header has been updated.

Fixes flutter/flutter#159512

@chinmaygarde chinmaygarde added the Work in progress (WIP) Not ready (yet) for review! label Dec 2, 2024
@chinmaygarde

This comment was marked as outdated.

@chinmaygarde

This comment was marked as outdated.

* Adds context creation and WSI routines for Metal and Vulkan.
* Enables all tests for the Metal, Vulkan, and OpenGLES backends.
* Separate standalone examples for Metal, Vulkan, and OpenGLES have been created. These will be packaged with the SDK.
  * Disallows the use of OpenGL ES on macOS.
* All new public methods are documented.
* The SDK version number has been bumped.
* Some incorrect nullability annotations were patched.
* Tests harness is overhauled to reuse the same underlying context as the playgrounds.

Fixes flutter/flutter#159512
@chinmaygarde
Copy link
Member Author

Ok, all done. All backends should be enabled, tested, documented, and have examples. I am sure more complicated WSI could be supported (like non vk_surface_khr swapchains) but we can add support for that as it becomes necessary.

impeller::ContextVK::Settings impeller_settings;
impeller_settings.shader_libraries_data = CreateShaderLibraryMappings();
impeller_settings.cache_directory = fml::paths::GetCachesDirectory();
impeller_settings.enable_validation = true;
Copy link
Member

Choose a reason for hiding this comment

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

should this be reading from the settings?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice spot. This gave me so much trouble since reading from the settings was what I was doing initially. But we set setting to this to true here. This causes the system global proc. table to be initialized twice. And, depending on whether the second instance enabled or disables validations, proc. entries are zeroed out. So teardown of say the debug printers in the second context to be torn down won't find the proc. table entry for the deleter. And cause a crash.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, but you mentioned settings. While I was using switches (that the settings are inferred from). Perhaps I could steal the underlying contexts settings directly. Momentido.

Copy link
Member

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Impeller] libImpeller: Wire up Metal and Vulkan backends.
2 participants