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.

Sign up for GitHub

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

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

WIP: Add partial VK_EXT_image_2d_view_of_3d support #2332

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
57d6688
Add VK_EXT_image_2d_view_of_3d support
ncesario-lunarg Aug 30, 2024
c0ce6d2
Add configuration variable for VK_EXT_image_2d_view_of_3d
ncesario-lunarg Oct 31, 2024
2a62041
Disable sampler2DViewOf3D
ncesario-lunarg Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion MoltenVK/MoltenVK/GPUObjects/MVKDevice.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@
portabilityFeatures->imageViewFormatReinterpretation = true;
portabilityFeatures->imageViewFormatSwizzle = (_metalFeatures.nativeTextureSwizzle ||
getMVKConfig().fullImageViewSwizzle);
portabilityFeatures->imageView2DOn3DImage = false;
portabilityFeatures->imageView2DOn3DImage = true;
portabilityFeatures->multisampleArrayImage = _metalFeatures.multisampleArrayTextures;
portabilityFeatures->mutableComparisonSamplers = _metalFeatures.depthSampleCompare;
portabilityFeatures->pointPolygons = false;
Expand Down Expand Up @@ -581,6 +581,12 @@
auto* shaderIntFuncsFeatures = (VkPhysicalDeviceShaderIntegerFunctions2FeaturesINTEL*)next;
shaderIntFuncsFeatures->shaderIntegerFunctions2 = true;
break;
}
case VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_IMAGE_2D_VIEW_OF_3D_FEATURES_EXT: {
auto* extFeatures = (VkPhysicalDeviceImage2DViewOf3DFeaturesEXT*)next;
extFeatures->image2DViewOf3D = true;
extFeatures->sampler2DViewOf3D = true;
break;
}
default:
break;
Expand Down
1 change: 1 addition & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKDeviceFeatureStructs.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ MVK_DEVICE_FEATURE_EXTN(ShaderAtomicFloat, SHADER_ATOMIC_FLOAT,
MVK_DEVICE_FEATURE_EXTN(SwapchainMaintenance1, SWAPCHAIN_MAINTENANCE_1, EXT, 1)
MVK_DEVICE_FEATURE_EXTN(TexelBufferAlignment, TEXEL_BUFFER_ALIGNMENT, EXT, 1)
MVK_DEVICE_FEATURE_EXTN(VertexAttributeDivisor, VERTEX_ATTRIBUTE_DIVISOR, EXT, 2)
MVK_DEVICE_FEATURE_EXTN(Image2DViewOf3D, IMAGE_2D_VIEW_OF_3D, EXT, 2)
MVK_DEVICE_FEATURE_EXTN(ShaderIntegerFunctions2, SHADER_INTEGER_FUNCTIONS_2, INTEL, 1)

#pragma pop_macro("INTEL")
Expand Down
10 changes: 10 additions & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,16 @@ typedef struct MVKMappedMemoryRange {
VkDeviceSize size = 0;
} MVKMappedMemoryRange;

struct HeapAllocation {
id<MTLHeap> heap = nil; // Reference to the heap containing this allocation
size_t offset = 0; // Offset into the heap
size_t size = 0; // Total size of this allocation
size_t align = 0; // Allocation alignment requirement

bool isValid() const {
return (heap != nil) && (size != 0);
}
};

/** Represents a Vulkan device-space memory allocation. */
class MVKDeviceMemory : public MVKVulkanAPIDeviceObject {
Expand Down
14 changes: 5 additions & 9 deletions MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Author

Choose a reason for hiding this comment

The 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;
Expand Down
6 changes: 6 additions & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKImage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class MVKImagePlane : public MVKBaseObject {
id<MTLTexture> _mtlTexture;
std::unordered_map<NSUInteger, id<MTLTexture>> _mtlTextureViews;
MVKSmallVector<MVKImageSubresource, 1> _subresources;

HeapAllocation _heapAllocation;
};


Expand Down Expand Up @@ -233,6 +235,7 @@ class MVKImage : public MVKVulkanAPIDeviceObject {
/** Populates the specified transfer image descriptor data structure. */
void getTransferDescriptorData(MVKImageDescriptorData& imgData);

MTLTextureDescriptor* getTextureDescriptor(uint32_t planeIndex);

#pragma mark Resource memory

Expand Down Expand Up @@ -334,6 +337,8 @@ class MVKImage : public MVKVulkanAPIDeviceObject {
/** Returns the Metal CPU cache mode used by this image. */
MTLCPUCacheMode getMTLCPUCacheMode();

HeapAllocation* getHeapAllocation(uint32_t planeIndex);


#pragma mark Construction

Expand Down Expand Up @@ -395,6 +400,7 @@ class MVKImage : public MVKVulkanAPIDeviceObject {
bool _hasMutableFormat;
bool _shouldSupportAtomics;
bool _isLinearForAtomics;
bool _is2DViewOn3DImageCompatible = false;
};


Expand Down
62 changes: 46 additions & 16 deletions MoltenVK/MoltenVK/GPUObjects/MVKImage.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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];
Expand Down Expand Up @@ -823,6 +832,10 @@ static MTLRegion getMTLRegion(const ImgRgn& imgRgn) {
imgData.usage = getCombinedUsage();
}

MTLTextureDescriptor* MVKImage::getTextureDescriptor(uint32_t planeIndex) {
return _planes[planeIndex]->newMTLTextureDescriptor();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

getTextureDescriptor() should be renamed to newMTLTextureDescriptor, to clarify that the returned object is retained, and needs to be released by the caller.

Since this function is only used once, it might be better to remove this getTextureDescriptor(), and replace its use with just _planes[planeIndex]->newMTLTextureDescriptor().


// 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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is unnecessary, since newMTLTexture() is only called if _mtlTexture is nil.

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);
Copy link
Contributor

Choose a reason for hiding this comment

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

mtlTexDesc will leak if it isn't released after it is used. See other uses of newMTLTextureDescriptor() for guidance on that.


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];
Copy link
Contributor

Choose a reason for hiding this comment

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

Two issues here.

  1. This unexpectedly plugs a value into _mtlTexture, which will be overridden by the value returned by this function.

  2. The new MTLTexture created here is retained, and will leak if it is not released at some point.

I suggest:

  • Declare a id<MTLTexture> aliasTex = nil; at the top of this function.
  • Assign aliasTex here from the call to newTextureWithDescriptor(), and set mtlTex = aliasTex;.
  • Replace the two return [mtlTex newTextureView... below with texView = [mtlTex newTextureView....
  • End the function with:
[aliasTex release];
return texView;

It's important to release aliasTex after texView has been created, and aliasTex will be retained within texView, and will be released and deallocated when texView is released and deallocated.

}
mtlTex = _mtlTexture;
sliceRange = NSMakeRange(0, _imageView->_subresourceRange.layerCount);
}
}
id<MTLTexture> mtlTex = _imageView->_image->getMTLTexture(_planeIndex);
if (_useNativeSwizzle) {
return [mtlTex newTextureViewWithPixelFormat: _mtlPixFmt
textureType: mtlTextureType
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions MoltenVK/MoltenVK/Layers/MVKExtensions.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ 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)
MVK_EXTENSION(EXT_image_2d_view_of_3d, EXT_IMAGE_2D_VIEW_OF_3D, DEVICE, 10.15, 13.0, 1.0)
Copy link

Choose a reason for hiding this comment

The 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?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I'll wrap that in an #ifdef.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wrapping in #ifdef here only works for environment variables, and not VK_EXT_layer_settings.

To properly support allowing this extension to be enabled via settings, use something like:

pWritableExtns->vk_EXT_image_2d_view_of_3d.enabled = getMVKConfig().enable2DViewOf3D;

in MVKPhysicalDevice::initExtensions().

However, see my question above about whether we need to have this configurable at all.

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)
Expand Down