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

WIP: Add VK_EXT_external_memory_metal #2314

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

aitor-lunarg
Copy link
Collaborator

No description provided.

@aitor-lunarg aitor-lunarg force-pushed the VK_EXT_external_memory_metal branch from ba02ae0 to 0dbc5be Compare August 26, 2024 23:01
@aitor-lunarg aitor-lunarg force-pushed the VK_EXT_external_memory_metal branch from cd7b8b0 to e25fc84 Compare September 26, 2024 07:20
@aitor-lunarg aitor-lunarg force-pushed the VK_EXT_external_memory_metal branch from 20f34fd to 1ee5c58 Compare October 21, 2024 03:11
@aitor-lunarg aitor-lunarg force-pushed the VK_EXT_external_memory_metal branch from 1ee5c58 to 016a57a Compare October 21, 2024 03:13
@@ -1 +1 @@
fc6c06ac529e4b4b6e34c17cc650a8f62dee2eb0
5a895c363dd689fdd58a4d336cceda77eca3a62f
Copy link

Choose a reason for hiding this comment

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

@aitor-lunarg , the extension is working fine for us so far with the additional handling of the case I've mentioned in the other comment. Would be great to see this merged in soon, but otherwise how can we get this external dependencies merged into Vulkan-Headers to avoid issues with the future updates on MoltenVK?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The main issue is that until the spec PR is not approved and merged, Vulkan-Headers cannot be updated with the xml data that contains the extension. Once the spec PR is merged then I will be able to start merging all other repositories that rely on Vulkan-Headers (which are VVL, CTS and MoltenVK). It's pretty much a domino effect, but as mentioned until the spec PR is approved and merged, the only thing I can do is update the Vulkan-Headers repository this change is pointing to so it is based on top of current MoltenVK's hash with the patch applied on top. If there's anything else I could do to facilitate pulling this change do let me know. Otherwise, I'll just update this PR once MoltenVK pulls new headers so there are not conflicts.

@billhollings
Copy link
Contributor

@aitor-lunarg What's the status of this PR?

  1. Do you expect to push an MR to the Vulkan Headers repo (either GitHub or GitLab) soon?
  2. How comfortable are we with this PR now? Based on feedback here, looks like it's pretty close.

@aitor-lunarg
Copy link
Collaborator Author

1. Do you expect to push an MR to the Vulkan Headers repo (either GitHub or GitLab) soon?

I am waiting for the spec PR to be approved and merged. This will trigger a domino effect and the required changes for Vulkan Headers repo will happen some time after spec merge. Once that happens, I will modify this PR to point to the right Vulkan Headers hash.

2. How comfortable are we with this PR now? Based on feedback here, looks like it's pretty close.

I believe it works for our use case, so right now I'm just waiting for the spec PR to be approved

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

Successfully merging this pull request may close these issues.

3 participants