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 5 commits into
base: main
Choose a base branch
from

Conversation

aitor-lunarg
Copy link
Collaborator

No description provided.

_externalMemoryHandleTypes = handleTypes;
auto& xmProps = getPhysicalDevice()->getExternalBufferProperties(VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_KHR);
auto& xmProps = getPhysicalDevice()->getExternalBufferProperties(VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_EXT);
_requiresDedicatedMemoryAllocation = _requiresDedicatedMemoryAllocation || mvkIsAnyFlagEnabled(xmProps.externalMemoryFeatures, VK_EXTERNAL_MEMORY_FEATURE_DEDICATED_ONLY_BIT);
} else {
setConfigurationResult(reportError(VK_ERROR_FEATURE_NOT_PRESENT, "vkCreateBuffer(): Only external memory handle type VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_KHR is supported."));

Choose a reason for hiding this comment

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

nit: VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_KHR usage remaining (ie. KHR not EXT) in the error message, similar issue also on MVKImage::initExternalMemory

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, thank you for catching that!


if ( !mvkIsOnlyAnyFlagEnabled(handleTypes, VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_KHR | VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_KHR) ) {
setConfigurationResult(reportError(VK_ERROR_INITIALIZATION_FAILED, "vkAllocateMemory(): Only external memory handle types VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_KHR or VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_KHR are supported."));
if ( !mvkIsOnlyAnyFlagEnabled(_externalMemoryHandleType, VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_EXT | VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT) ) {

Choose a reason for hiding this comment

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

VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLHEAP_BIT_EXT case is missing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, thank you for the catch!

requiresDedicated = requiresDedicated || mvkIsAnyFlagEnabled(xmProps.externalMemoryFeatures, VK_EXTERNAL_MEMORY_FEATURE_DEDICATED_ONLY_BIT);

// Make sure allocation happens at creation time since we may need to export the memory before usage
ensureMTLBuffer();

Choose a reason for hiding this comment

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

ensureMTLBuffer() called, instead of ensureMTLHeap().

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, thank you for the catch!

if (mvkIsAnyFlagEnabled(handleTypes, VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_KHR)) {
auto& xmProps = getPhysicalDevice()->getExternalBufferProperties(VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLBUFFER_BIT_KHR);
if (mvkIsAnyFlagEnabled(_externalMemoryHandleType, VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLHEAP_BIT_EXT)) {
auto& xmProps = getPhysicalDevice()->getExternalBufferProperties(VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLHEAP_BIT_EXT);

Choose a reason for hiding this comment

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

Call to getExternalBufferProperties(VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLHEAP_BIT_EXT) here is not returning _mtlTextureHeapExternalMemoryProperties and causes requiresDedicated to become false. Maybe it should check VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLTEXTURE_BIT_EXT along with the heap flag to see if getExternalImageProperties should be called instead?

// We cannot export images that have no Metal counterparts. This is because we are emulating them via multiple MTLTextures
// and we would require to export multiple MTLTextures. We let them export them as a heap whenever possible.
if (_pixelFormats.getChromaSubsamplingPlaneCount(format) > 1u)
return _mtlTextureHeapExternalMemoryProperties;

Choose a reason for hiding this comment

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

Do you think we should add a case for VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLHEAP_BIT_EXT here, like:

case VK_EXTERNAL_MEMORY_HANDLE_TYPE_MTLHEAP_BIT_EXT:
	return _mtlTextureHeapExternalMemoryProperties;

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.

2 participants