-
Notifications
You must be signed in to change notification settings - Fork 436
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 partial VK_EXT_image_2d_view_of_3d support #2332
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -188,15 +188,11 @@ | |
// Can't create MTLHeaps of zero size. | ||
if (_allocationSize == 0) { return true; } | ||
|
||
#if MVK_MACOS | ||
// MTLHeaps on macOS must use private storage for now. | ||
if (_mtlStorageMode != MTLStorageModePrivate) { return true; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of being removed, does this need to be modified to include a test for non-Apple-Silicon devices? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See comment below. |
||
#endif | ||
#if MVK_IOS | ||
// MTLHeaps on iOS must use private or shared storage for now. | ||
if ( !(_mtlStorageMode == MTLStorageModePrivate || | ||
_mtlStorageMode == MTLStorageModeShared) ) { return true; } | ||
#endif | ||
if (getPhysicalDevice()->getMTLDeviceCapabilities().isAppleGPU) { | ||
// MTLHeaps on Apple silicon must use private or shared storage for now. | ||
if ( !(_mtlStorageMode == MTLStorageModePrivate || | ||
_mtlStorageMode == MTLStorageModeShared) ) { return true; } | ||
} | ||
|
||
MTLHeapDescriptor* heapDesc = [MTLHeapDescriptor new]; | ||
heapDesc.type = MTLHeapTypePlacement; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,10 @@ | |
MVKImageMemoryBinding* memoryBinding = getMemoryBinding(); | ||
MVKDeviceMemory* dvcMem = memoryBinding->_deviceMemory; | ||
|
||
if (_image->_is2DViewOn3DImageCompatible && !dvcMem->ensureMTLHeap()) { | ||
MVKAssert(0, "Creating a 2D view of a 3D texture currently requires a placement heap, which is not available."); | ||
} | ||
|
||
if (_image->_ioSurface) { | ||
_mtlTexture = [_image->getMTLDevice() | ||
newTextureWithDescriptor: mtlTexDesc | ||
|
@@ -60,6 +64,11 @@ | |
bytesPerRow: _subresources[0].layout.rowPitch]; | ||
} else if (dvcMem && dvcMem->getMTLHeap() && !_image->getIsDepthStencil()) { | ||
// Metal support for depth/stencil from heaps is flaky | ||
_heapAllocation.heap = dvcMem->getMTLHeap(); | ||
_heapAllocation.offset = memoryBinding->getDeviceMemoryOffset() + _subresources[0].layout.offset; | ||
const auto texSizeAlign = [dvcMem->getMTLDevice() heapTextureSizeAndAlignWithDescriptor:mtlTexDesc]; | ||
_heapAllocation.size = texSizeAlign.size; | ||
_heapAllocation.align = texSizeAlign.align; | ||
_mtlTexture = [dvcMem->getMTLHeap() | ||
newTextureWithDescriptor: mtlTexDesc | ||
offset: memoryBinding->getDeviceMemoryOffset() + _subresources[0].layout.offset]; | ||
|
@@ -823,6 +832,10 @@ static MTLRegion getMTLRegion(const ImgRgn& imgRgn) { | |
imgData.usage = getCombinedUsage(); | ||
} | ||
|
||
MTLTextureDescriptor* MVKImage::getTextureDescriptor(uint32_t planeIndex) { | ||
return _planes[planeIndex]->newMTLTextureDescriptor(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Since this function is only used once, it might be better to remove this |
||
|
||
// Returns whether an MVKImageView can have the specified format. | ||
// If the list of pre-declared view formats is not empty, | ||
// and the format is not on that list, the view format is not valid. | ||
|
@@ -1069,6 +1082,11 @@ static MTLRegion getMTLRegion(const ImgRgn& imgRgn) { | |
return _memoryBindings[0]->_deviceMemory ? _memoryBindings[0]->_deviceMemory->getMTLCPUCacheMode() : MTLCPUCacheModeDefaultCache; | ||
} | ||
|
||
HeapAllocation* MVKImage::getHeapAllocation(uint32_t planeIndex) { | ||
auto& heapAllocation = _planes[planeIndex]->_heapAllocation; | ||
return (heapAllocation.isValid()) ? &heapAllocation : nullptr; | ||
} | ||
|
||
MTLTextureUsage MVKImage::getMTLTextureUsage(MTLPixelFormat mtlPixFmt) { | ||
|
||
// In the special case of a dedicated aliasable image, we must presume the texture can be used for anything. | ||
|
@@ -1254,6 +1272,8 @@ static MTLRegion getMTLRegion(const ImgRgn& imgRgn) { | |
if (pExportInfo && pExportInfo->exportObjectType == VK_EXPORT_METAL_OBJECT_TYPE_METAL_IOSURFACE_BIT_EXT && !_ioSurface) { | ||
setConfigurationResult(useIOSurface(nil)); | ||
} | ||
|
||
_is2DViewOn3DImageCompatible = mvkIsAnyFlagEnabled(pCreateInfo->flags, VK_IMAGE_CREATE_2D_VIEW_COMPATIBLE_BIT_EXT); | ||
} | ||
|
||
VkSampleCountFlagBits MVKImage::validateSamples(const VkImageCreateInfo* pCreateInfo, bool isAttachment) { | ||
|
@@ -1801,12 +1821,34 @@ static void signalAndUntrack(const MVKSwapchainSignaler& signaler) { | |
MTLTextureType mtlTextureType = _imageView->_mtlTextureType; | ||
NSRange sliceRange = NSMakeRange(_imageView->_subresourceRange.baseArrayLayer, _imageView->_subresourceRange.layerCount); | ||
// Fake support for 2D views of 3D textures. | ||
if (_imageView->_image->getImageType() == VK_IMAGE_TYPE_3D && | ||
auto* image = _imageView->_image; | ||
id<MTLTexture> mtlTex = image->getMTLTexture(_planeIndex); | ||
if (image->getImageType() == VK_IMAGE_TYPE_3D && | ||
(mtlTextureType == MTLTextureType2D || mtlTextureType == MTLTextureType2DArray)) { | ||
mtlTextureType = MTLTextureType3D; | ||
sliceRange = NSMakeRange(0, 1); | ||
if (!image->_is2DViewOn3DImageCompatible) { | ||
mtlTextureType = MTLTextureType3D; | ||
sliceRange = NSMakeRange(0, 1); | ||
} else { | ||
if (!_mtlTexture) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test is unnecessary, since |
||
const auto heapAllocation = image->getHeapAllocation(_planeIndex); | ||
MVKAssert(heapAllocation, "Attempting to create a 2D view of a 3D texture without a placement heap"); | ||
// TODO (ncesario-lunarg) untested where _imageView->subresourceRange.layerCount > 1, but VK_EXT_image_2d_view_of_3d | ||
// allows for 2D_ARRAY views of 3D textures. | ||
const auto relativeSliceOffset = _imageView->_subresourceRange.baseArrayLayer * (heapAllocation->size / image->_extent.depth); | ||
MTLTextureDescriptor* mtlTexDesc = image->getTextureDescriptor(_planeIndex); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
mtlTexDesc.depth = _imageView->_subresourceRange.layerCount; | ||
mtlTexDesc.textureType = mtlTextureType; | ||
|
||
// Create a temporary texture that is backed by the 3D texture's memory | ||
_mtlTexture = [heapAllocation->heap | ||
newTextureWithDescriptor: mtlTexDesc | ||
offset: heapAllocation->offset + relativeSliceOffset]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two issues here.
I suggest:
It's important to release |
||
} | ||
mtlTex = _mtlTexture; | ||
sliceRange = NSMakeRange(0, _imageView->_subresourceRange.layerCount); | ||
} | ||
} | ||
id<MTLTexture> mtlTex = _imageView->_image->getMTLTexture(_planeIndex); | ||
if (_useNativeSwizzle) { | ||
return [mtlTex newTextureViewWithPixelFormat: _mtlPixFmt | ||
textureType: mtlTextureType | ||
|
@@ -2235,18 +2277,6 @@ static void signalAndUntrack(const MVKSwapchainSignaler& signaler) { | |
} | ||
} | ||
|
||
VkImageType imgType = _image->getImageType(); | ||
VkImageViewType viewType = pCreateInfo->viewType; | ||
|
||
// VK_KHR_maintenance1 supports taking 2D image views of 3D slices for sampling. | ||
// No dice in Metal. But we are able to fake out a 3D render attachment by making the Metal view | ||
// itself a 3D texture (when we create it), and setting the rendering depthPlane appropriately. | ||
if ((viewType == VK_IMAGE_VIEW_TYPE_2D || viewType == VK_IMAGE_VIEW_TYPE_2D_ARRAY) && (imgType == VK_IMAGE_TYPE_3D)) { | ||
if (!mvkIsOnlyAnyFlagEnabled(_usage, VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT)) { | ||
setConfigurationResult(reportError(VK_ERROR_FEATURE_NOT_PRESENT, "vkCreateImageView(): 2D views on 3D images can only be used as color attachments.")); | ||
} | ||
} | ||
|
||
// If a 2D array view on a 2D image with layerCount 1, and the only usages are | ||
// attachment usages, then force the use of a 2D non-arrayed view. This is important for | ||
// input attachments, or they won't match the types declared in the fragment shader. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,6 +144,11 @@ MVK_EXTENSION(EXT_swapchain_maintenance1, EXT_SWAPCHAIN_MAINTENANCE_ | |
MVK_EXTENSION(EXT_texel_buffer_alignment, EXT_TEXEL_BUFFER_ALIGNMENT, DEVICE, 10.13, 11.0, 1.0) | ||
MVK_EXTENSION(EXT_texture_compression_astc_hdr, EXT_TEXTURE_COMPRESSION_ASTC_HDR, DEVICE, 11.0, 13.0, 1.0) | ||
MVK_EXTENSION(EXT_vertex_attribute_divisor, EXT_VERTEX_ATTRIBUTE_DIVISOR, DEVICE, 10.11, 8.0, 1.0) | ||
|
||
#ifdef MVK_CONFIG_ENABLE_2DVIEWOF3D | ||
MVK_EXTENSION(EXT_image_2d_view_of_3d, EXT_IMAGE_2D_VIEW_OF_3D, DEVICE, 10.15, 13.0, 1.0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think we can modify mvkIsSupportedOnPlatform check and not advertise support on this when the config flag is not enabled? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I'll wrap that in an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrapping in To properly support allowing this extension to be enabled via settings, use something like:
in However, see my question above about whether we need to have this configurable at all. |
||
#endif | ||
|
||
MVK_EXTENSION(AMD_draw_indirect_count, AMD_DRAW_INDIRECT_COUNT, DEVICE, MVK_NA, MVK_NA, MVK_NA) | ||
MVK_EXTENSION(AMD_gpu_shader_half_float, AMD_GPU_SHADER_HALF_FLOAT, DEVICE, 10.11, 8.0, 1.0) | ||
MVK_EXTENSION(AMD_negative_viewport_height, AMD_NEGATIVE_VIEWPORT_HEIGHT, DEVICE, 10.11, 8.0, 1.0) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,10 +26,9 @@ | |
// Return the expected size of MVKConfiguration, based on contents of MVKConfigMembers.def. | ||
static constexpr uint32_t getExpectedMVKConfigurationSize() { | ||
#define MVK_CONFIG_MEMBER(member, mbrType, name) cfgSize += sizeof(mbrType); | ||
uint32_t cfgSize = 0; | ||
size_t cfgSize = 0; | ||
#include "MVKConfigMembers.def" | ||
cfgSize += kMVKConfigurationInternalPaddingByteCount; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @billhollings This seemed to be calculating the incorrect size, but perhaps that meant I added the config member incorrectly? I changed this function to align the expected size to the alignment of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. iiuc, this is to make the padding requirement of the structure clearer and it expects modification to Otherwise, I don't see any other functionality of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have a look at the description of
And the purpose of that The reason The solution to the issue you have dealt with here is to adjust the value of |
||
return cfgSize; | ||
return (uint32_t)((cfgSize + (alignof(MVKConfiguration) - 1)) & ~(alignof(MVKConfiguration) - 1)); | ||
} | ||
|
||
// Return the expected number of string members in MVKConfiguration, based on contents of MVKConfigMembers.def. | ||
|
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.
Is there a reason to have the app have to opt-in to this extension?
If not, I've run CTS on everything, and it all seems to be working fine, and doesn't seem to be interfering with other behaviour. I think I would be satisfied to remove
MVK_CONFIG_ENABLE_2DVIEWOF3D
andMVKConfiguration::enable2DViewOf3D
, and then where you test forenable2DViewOf3D
internally, useuseMTLHeap
instead.