From 5dee56f39c422e0b1e8d9f3f1f8f22ee42554603 Mon Sep 17 00:00:00 2001 From: Jiawei Shao Date: Sat, 29 Dec 2018 10:47:28 +0000 Subject: [PATCH] Fix crash when creating texture view on textures from Metal swap chain For the textures got from Metal swap chain, their "framebufferOnly" may be true, which means they can only be used as attachments in a render pass, and they are not allowed to be used in MTLRenderCommandEncoder, MTLBlitCommandEncoder or MTLComputeCommandEncoder. So currently Dawn examples all crash when METAL_DEVICE_WRAPPER_TYPE = 1 is set into environmental variables. This patch adds checks on the situations that we do not need to create a Metal texture view: 1. We create Metal texture only when the usage of the texture includes Sampled or Storage. 2. We won't create Metal texture view if the view uses the same format as the original texture, the whole mipmap levels and array slices. 3. We use the original MTLTexture and set the slice and level in MTLRenderPassDescriptor. Furthermore, with this patch, "setFramebufferOnly" is set to true only when the usage passed to configure is a subset of (OutputAttachment | Present). BUG=dawn:69 Change-Id: Ie2670f383c16eafa3b1c6f99126922e940721174 Reviewed-on: https://dawn-review.googlesource.com/c/3400 Reviewed-by: Corentin Wallez Reviewed-by: Kai Ninomiya Commit-Queue: Corentin Wallez --- src/dawn_native/metal/CommandBufferMTL.mm | 5 +- src/dawn_native/metal/TextureMTL.mm | 66 +++++++++++++++++------ src/utils/MetalBinding.mm | 10 +++- 3 files changed, 63 insertions(+), 18 deletions(-) diff --git a/src/dawn_native/metal/CommandBufferMTL.mm b/src/dawn_native/metal/CommandBufferMTL.mm index 1b2bb8ebba..cbd8262e9d 100644 --- a/src/dawn_native/metal/CommandBufferMTL.mm +++ b/src/dawn_native/metal/CommandBufferMTL.mm @@ -64,7 +64,10 @@ namespace dawn_native { namespace metal { } descriptor.colorAttachments[i].texture = - ToBackend(attachmentInfo.view)->GetMTLTexture(); + ToBackend(attachmentInfo.view->GetTexture())->GetMTLTexture(); + descriptor.colorAttachments[i].level = attachmentInfo.view->GetBaseMipLevel(); + descriptor.colorAttachments[i].slice = attachmentInfo.view->GetBaseArrayLayer(); + descriptor.colorAttachments[i].storeAction = MTLStoreActionStore; } diff --git a/src/dawn_native/metal/TextureMTL.mm b/src/dawn_native/metal/TextureMTL.mm index 818c8daf6d..b6ebdfa7f9 100644 --- a/src/dawn_native/metal/TextureMTL.mm +++ b/src/dawn_native/metal/TextureMTL.mm @@ -17,7 +17,6 @@ #include "dawn_native/metal/DeviceMTL.h" namespace dawn_native { namespace metal { - MTLPixelFormat MetalPixelFormat(dawn::TextureFormat format) { switch (format) { case dawn::TextureFormat::R8G8B8A8Unorm: @@ -40,6 +39,12 @@ namespace dawn_native { namespace metal { } namespace { + bool UsageNeedsTextureView(dawn::TextureUsageBit usage) { + constexpr dawn::TextureUsageBit kUsageNeedsTextureView = + dawn::TextureUsageBit::Storage | dawn::TextureUsageBit::Sampled; + return usage & kUsageNeedsTextureView; + } + MTLTextureUsage MetalTextureUsage(dawn::TextureUsageBit usage) { MTLTextureUsage result = MTLTextureUsageUnknown; // This is 0 @@ -55,9 +60,9 @@ namespace dawn_native { namespace metal { result |= MTLTextureUsageRenderTarget; } - // TODO(jiawei.shao@intel.com): investigate if we should skip setting this flag when the - // texture is only used as a render target. - result |= MTLTextureUsagePixelFormatView; + if (UsageNeedsTextureView(usage)) { + result |= MTLTextureUsagePixelFormatView; + } return result; } @@ -85,6 +90,31 @@ namespace dawn_native { namespace metal { return MTLTextureType2D; } } + + bool RequiresCreatingNewTextureView(const TextureBase* texture, + const TextureViewDescriptor* textureViewDescriptor) { + if (texture->GetFormat() != textureViewDescriptor->format) { + return true; + } + + if (texture->GetArrayLayers() != textureViewDescriptor->layerCount) { + return true; + } + + if (texture->GetNumMipLevels() != textureViewDescriptor->levelCount) { + return true; + } + + switch (textureViewDescriptor->dimension) { + case dawn::TextureViewDimension::Cube: + case dawn::TextureViewDimension::CubeArray: + return true; + default: + break; + } + + return false; + } } Texture::Texture(Device* device, const TextureDescriptor* descriptor) @@ -121,20 +151,25 @@ namespace dawn_native { namespace metal { return mMtlTexture; } - // TODO(jiawei.shao@intel.com): use the original texture directly when the descriptor covers the - // whole texture in the same format (for example, when CreateDefaultTextureView() is called). TextureView::TextureView(TextureBase* texture, const TextureViewDescriptor* descriptor) : TextureViewBase(texture, descriptor) { - MTLPixelFormat format = MetalPixelFormat(descriptor->format); - MTLTextureType textureViewType = MetalTextureViewType(descriptor->dimension); - auto mipLevelRange = NSMakeRange(descriptor->baseMipLevel, descriptor->levelCount); - auto arrayLayerRange = NSMakeRange(descriptor->baseArrayLayer, descriptor->layerCount); - id mtlTexture = ToBackend(texture)->GetMTLTexture(); - mMtlTextureView = [mtlTexture newTextureViewWithPixelFormat:format - textureType:textureViewType - levels:mipLevelRange - slices:arrayLayerRange]; + + if (!UsageNeedsTextureView(texture->GetUsage())) { + mMtlTextureView = nil; + } else if (!RequiresCreatingNewTextureView(texture, descriptor)) { + mMtlTextureView = [mtlTexture retain]; + } else { + MTLPixelFormat format = MetalPixelFormat(descriptor->format); + MTLTextureType textureViewType = MetalTextureViewType(descriptor->dimension); + auto mipLevelRange = NSMakeRange(descriptor->baseMipLevel, descriptor->levelCount); + auto arrayLayerRange = NSMakeRange(descriptor->baseArrayLayer, descriptor->layerCount); + + mMtlTextureView = [mtlTexture newTextureViewWithPixelFormat:format + textureType:textureViewType + levels:mipLevelRange + slices:arrayLayerRange]; + } } TextureView::~TextureView() { @@ -142,6 +177,7 @@ namespace dawn_native { namespace metal { } id TextureView::GetMTLTexture() { + ASSERT(mMtlTextureView != nil); return mMtlTextureView; } }} // namespace dawn_native::metal diff --git a/src/utils/MetalBinding.mm b/src/utils/MetalBinding.mm index 8eaba1712b..0b41c149ef 100644 --- a/src/utils/MetalBinding.mm +++ b/src/utils/MetalBinding.mm @@ -41,7 +41,7 @@ namespace utils { } dawnSwapChainError Configure(dawnTextureFormat format, - dawnTextureUsageBit, + dawnTextureUsageBit usage, uint32_t width, uint32_t height) { if (format != DAWN_TEXTURE_FORMAT_B8_G8_R8_A8_UNORM) { @@ -60,9 +60,15 @@ namespace utils { mLayer = [CAMetalLayer layer]; [mLayer setDevice:mMtlDevice]; [mLayer setPixelFormat:MTLPixelFormatBGRA8Unorm]; - [mLayer setFramebufferOnly:YES]; [mLayer setDrawableSize:size]; + constexpr uint32_t kFramebufferOnlyTextureUsages = + DAWN_TEXTURE_USAGE_BIT_OUTPUT_ATTACHMENT | DAWN_TEXTURE_USAGE_BIT_PRESENT; + bool hasOnlyFramebufferUsages = !(usage & (~kFramebufferOnlyTextureUsages)); + if (hasOnlyFramebufferUsages) { + [mLayer setFramebufferOnly:YES]; + } + [contentView setLayer:mLayer]; return DAWN_SWAP_CHAIN_NO_ERROR;