From c0c7e2f85bce063edf595ac014c040f0c7d04280 Mon Sep 17 00:00:00 2001 From: Jiawei Shao Date: Wed, 22 May 2019 00:05:14 +0000 Subject: [PATCH] Add workaround on using non-zero ResolveLevel or ResolveSlice on Metal Current Metal drivers have a bug that doing MSAA resolve with non-zero ResolveLevel and ResolveSlice cannot work correctly. This patch adds a workaround for this bug that first resolving into a temporary one-level 2D texture, then copying the result into the true resolve target. Although the end2end test ResolveIntoOneMipmapLevelOf2DTexture and ResolveInto2DArrayTexture can pass on the try bot using AMD GPU, we find they fail on some other AMD GPUs (Macbook Pro 2018), so currently this workaround is enabled on all Metal backends. BUG=dawn:56 TEST=dawn_end2end_tests Change-Id: Ie85858e58ff486d49dc11c270d8b6d95e216fd42 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/6780 Reviewed-by: Kai Ninomiya Commit-Queue: Jiawei Shao --- src/dawn_native/Instance.cpp | 10 +- src/dawn_native/Toggles.h | 1 + src/dawn_native/metal/CommandBufferMTL.h | 1 - src/dawn_native/metal/CommandBufferMTL.mm | 98 +++++++++++++++++++ src/dawn_native/metal/DeviceMTL.mm | 3 + .../end2end/MultisampledRenderingTests.cpp | 7 +- 6 files changed, 113 insertions(+), 7 deletions(-) diff --git a/src/dawn_native/Instance.cpp b/src/dawn_native/Instance.cpp index b0886ba4c1..9458dbf382 100644 --- a/src/dawn_native/Instance.cpp +++ b/src/dawn_native/Instance.cpp @@ -73,7 +73,15 @@ namespace dawn_native { "Clears texture to full 1 bits as soon as they are created, but doesn't update " "the tracking state of the texture. This way we can test the logic of clearing " "textures that use recycled memory.", - "https://bugs.chromium.org/p/dawn/issues/detail?id=145"}}}}; + "https://bugs.chromium.org/p/dawn/issues/detail?id=145"}}, + {Toggle::AlwaysResolveIntoZeroLevelAndLayer, + {"always_resolve_into_zero_level_and_layer", + "When the resolve target is a texture view that is created on the non-zero level or " + "layer of a texture, we first resolve into a temporarily 2D texture with only one " + "mipmap level and one array layer, and copy the result of MSAA resolve into the " + "true resolve target. This workaround is enabled by default on the Metal drivers " + "that have bugs when setting non-zero resolveLevel or resolveSlice.", + "https://bugs.chromium.org/p/dawn/issues/detail?id=56"}}}}; } // anonymous namespace diff --git a/src/dawn_native/Toggles.h b/src/dawn_native/Toggles.h index c60f5f95e3..4ff15c1ff2 100644 --- a/src/dawn_native/Toggles.h +++ b/src/dawn_native/Toggles.h @@ -24,6 +24,7 @@ namespace dawn_native { enum class Toggle { EmulateStoreAndMSAAResolve, NonzeroClearResourcesOnCreationForTesting, + AlwaysResolveIntoZeroLevelAndLayer, EnumCount, InvalidEnum = EnumCount, diff --git a/src/dawn_native/metal/CommandBufferMTL.h b/src/dawn_native/metal/CommandBufferMTL.h index 4b08a9f46a..a127c515ac 100644 --- a/src/dawn_native/metal/CommandBufferMTL.h +++ b/src/dawn_native/metal/CommandBufferMTL.h @@ -21,7 +21,6 @@ #import namespace dawn_native { - struct BeginRenderPassCmd; class CommandEncoderBase; } diff --git a/src/dawn_native/metal/CommandBufferMTL.mm b/src/dawn_native/metal/CommandBufferMTL.mm index 505cd85b2a..d44740e1d3 100644 --- a/src/dawn_native/metal/CommandBufferMTL.mm +++ b/src/dawn_native/metal/CommandBufferMTL.mm @@ -119,6 +119,7 @@ namespace dawn_native { namespace metal { return descriptor; } + // Helper function for Toggle EmulateStoreAndMSAAResolve void ResolveInAnotherRenderPass( id commandBuffer, const MTLRenderPassDescriptor* mtlRenderPass, @@ -147,6 +148,48 @@ namespace dawn_native { namespace metal { [encoder endEncoding]; } + // Helper functions for Toggle AlwaysResolveIntoZeroLevelAndLayer + id CreateResolveTextureForWorkaround(Device* device, + MTLPixelFormat mtlFormat, + uint32_t width, + uint32_t height) { + MTLTextureDescriptor* mtlDesc = [MTLTextureDescriptor new]; + mtlDesc.textureType = MTLTextureType2D; + mtlDesc.usage = MTLTextureUsageRenderTarget; + mtlDesc.pixelFormat = mtlFormat; + mtlDesc.width = width; + mtlDesc.height = height; + mtlDesc.depth = 1; + mtlDesc.mipmapLevelCount = 1; + mtlDesc.arrayLength = 1; + mtlDesc.storageMode = MTLStorageModePrivate; + mtlDesc.sampleCount = 1; + id resolveTexture = + [device->GetMTLDevice() newTextureWithDescriptor:mtlDesc]; + [mtlDesc release]; + return resolveTexture; + } + + void CopyIntoTrueResolveTarget(id commandBuffer, + id mtlTrueResolveTexture, + uint32_t trueResolveLevel, + uint32_t trueResolveSlice, + id temporaryResolveTexture, + uint32_t width, + uint32_t height, + GlobalEncoders* encoders) { + encoders->EnsureBlit(commandBuffer); + [encoders->blit copyFromTexture:temporaryResolveTexture + sourceSlice:0 + sourceLevel:0 + sourceOrigin:MTLOriginMake(0, 0, 0) + sourceSize:MTLSizeMake(width, height, 1) + toTexture:mtlTrueResolveTexture + destinationSlice:trueResolveSlice + destinationLevel:trueResolveLevel + destinationOrigin:MTLOriginMake(0, 0, 0)]; + } + // Handles a call to SetBindGroup, directing the commands to the correct encoder. // There is a single function that takes both encoders to factor code. Other approaches like // templates wouldn't work because the name of methods are different between the two encoder @@ -627,6 +670,61 @@ namespace dawn_native { namespace metal { Device* device = ToBackend(GetDevice()); + // Handle Toggle AlwaysResolveIntoZeroLevelAndLayer. We must handle this before applying + // the store + MSAA resolve workaround, otherwise this toggle will never be handled because + // the resolve texture is removed when applying the store + MSAA resolve workaround. + if (device->IsToggleEnabled(Toggle::AlwaysResolveIntoZeroLevelAndLayer)) { + std::array, kMaxColorAttachments> trueResolveTextures = {}; + std::array trueResolveLevels = {}; + std::array trueResolveSlices = {}; + + // Use temporary resolve texture on the resolve targets with non-zero resolveLevel or + // resolveSlice. + bool useTemporaryResolveTexture = false; + std::array, kMaxColorAttachments> temporaryResolveTextures = {}; + for (uint32_t i = 0; i < kMaxColorAttachments; ++i) { + if (mtlRenderPass.colorAttachments[i].resolveTexture == nil) { + continue; + } + + if (mtlRenderPass.colorAttachments[i].resolveLevel == 0 && + mtlRenderPass.colorAttachments[i].resolveSlice == 0) { + continue; + } + + trueResolveTextures[i] = mtlRenderPass.colorAttachments[i].resolveTexture; + trueResolveLevels[i] = mtlRenderPass.colorAttachments[i].resolveLevel; + trueResolveSlices[i] = mtlRenderPass.colorAttachments[i].resolveSlice; + + const MTLPixelFormat mtlFormat = trueResolveTextures[i].pixelFormat; + temporaryResolveTextures[i] = + CreateResolveTextureForWorkaround(device, mtlFormat, width, height); + + mtlRenderPass.colorAttachments[i].resolveTexture = temporaryResolveTextures[i]; + mtlRenderPass.colorAttachments[i].resolveLevel = 0; + mtlRenderPass.colorAttachments[i].resolveSlice = 0; + useTemporaryResolveTexture = true; + } + + // If we need to use a temporary resolve texture we need to copy the result of MSAA + // resolve back to the true resolve targets. + if (useTemporaryResolveTexture) { + EncodeRenderPass(commandBuffer, mtlRenderPass, globalEncoders, width, height); + for (uint32_t i = 0; i < kMaxColorAttachments; ++i) { + if (trueResolveTextures[i] == nil) { + continue; + } + + ASSERT(temporaryResolveTextures[i] != nil); + CopyIntoTrueResolveTarget(commandBuffer, trueResolveTextures[i], + trueResolveLevels[i], trueResolveSlices[i], + temporaryResolveTextures[i], width, height, + globalEncoders); + } + return; + } + } + // Handle Store + MSAA resolve workaround (Toggle EmulateStoreAndMSAAResolve). if (device->IsToggleEnabled(Toggle::EmulateStoreAndMSAAResolve)) { bool hasStoreAndMSAAResolve = false; diff --git a/src/dawn_native/metal/DeviceMTL.mm b/src/dawn_native/metal/DeviceMTL.mm index e38d4231a6..d0b58eeb73 100644 --- a/src/dawn_native/metal/DeviceMTL.mm +++ b/src/dawn_native/metal/DeviceMTL.mm @@ -79,6 +79,9 @@ namespace dawn_native { namespace metal { bool emulateStoreAndMSAAResolve = ![mMtlDevice supportsFeatureSet:MTLFeatureSet_macOS_GPUFamily1_v2]; SetToggle(Toggle::EmulateStoreAndMSAAResolve, emulateStoreAndMSAAResolve); + + // TODO(jiawei.shao@intel.com): tighten this workaround when the driver bug is fixed. + SetToggle(Toggle::AlwaysResolveIntoZeroLevelAndLayer, true); } ResultOrError Device::CreateBindGroupImpl( diff --git a/src/tests/end2end/MultisampledRenderingTests.cpp b/src/tests/end2end/MultisampledRenderingTests.cpp index ff66ed0f21..0f1806d7bc 100644 --- a/src/tests/end2end/MultisampledRenderingTests.cpp +++ b/src/tests/end2end/MultisampledRenderingTests.cpp @@ -410,8 +410,6 @@ TEST_P(MultisampledRenderingTest, ResolveOneMultisampledTextureTwice) { // Test using a layer of a 2D texture as resolve target works correctly. TEST_P(MultisampledRenderingTest, ResolveIntoOneMipmapLevelOf2DTexture) { - // TODO(jiawei.shao@intel.com): investigate why this case fails on Intel and Nvidia. - DAWN_SKIP_TEST_IF(IsMetal() && (IsIntel() || IsNvidia())); constexpr uint32_t kBaseMipLevel = 2; dawn::TextureViewDescriptor textureViewDescriptor; @@ -450,8 +448,6 @@ TEST_P(MultisampledRenderingTest, ResolveIntoOneMipmapLevelOf2DTexture) { // Test using a level or a layer of a 2D array texture as resolve target works correctly. TEST_P(MultisampledRenderingTest, ResolveInto2DArrayTexture) { - // TODO(jiawei.shao@intel.com): investigate why this case fails on Intel and Nvidia. - DAWN_SKIP_TEST_IF(IsMetal() && (IsIntel() || IsNvidia())); dawn::TextureView multisampledColorView2 = CreateTextureForOutputAttachment(kColorFormat, kSampleCount).CreateDefaultView(); @@ -515,4 +511,5 @@ DAWN_INSTANTIATE_TEST(MultisampledRenderingTest, MetalBackend, OpenGLBackend, VulkanBackend, - ForceWorkaround(MetalBackend, "emulate_store_and_msaa_resolve")); + ForceWorkaround(MetalBackend, "emulate_store_and_msaa_resolve"), + ForceWorkaround(MetalBackend, "always_resolve_into_zero_level_and_layer"));