diff --git a/BUILD.gn b/BUILD.gn index 866b2d472e..b6201ce845 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -358,6 +358,8 @@ source_set("libdawn_native_sources") { "src/dawn_native/metal/BufferMTL.mm", "src/dawn_native/metal/CommandBufferMTL.h", "src/dawn_native/metal/CommandBufferMTL.mm", + "src/dawn_native/metal/CommandRecordingContext.h", + "src/dawn_native/metal/CommandRecordingContext.mm", "src/dawn_native/metal/ComputePipelineMTL.h", "src/dawn_native/metal/ComputePipelineMTL.mm", "src/dawn_native/metal/DeviceMTL.h", @@ -967,7 +969,7 @@ source_set("dawn_end2end_tests_sources") { } source_set("dawn_white_box_tests_sources") { - configs += [ "${dawn_root}/src/common:dawn_internal" ] + configs += [ ":libdawn_native_internal" ] testonly = true deps = [ @@ -1002,6 +1004,10 @@ source_set("dawn_white_box_tests_sources") { sources += [ "src/tests/white_box/D3D12SmallTextureTests.cpp" ] } + if (dawn_enable_metal) { + sources += [ "src/tests/white_box/MetalAutoreleasePoolTests.mm" ] + } + if (dawn_enable_opengl) { deps += [ ":dawn_glfw" ] } diff --git a/src/dawn_native/metal/CommandBufferMTL.h b/src/dawn_native/metal/CommandBufferMTL.h index 640d19666b..67a1313eba 100644 --- a/src/dawn_native/metal/CommandBufferMTL.h +++ b/src/dawn_native/metal/CommandBufferMTL.h @@ -26,25 +26,24 @@ namespace dawn_native { namespace dawn_native { namespace metal { + class CommandRecordingContext; class Device; - struct GlobalEncoders; class CommandBuffer : public CommandBufferBase { public: CommandBuffer(CommandEncoder* encoder, const CommandBufferDescriptor* descriptor); ~CommandBuffer(); - void FillCommands(id commandBuffer); + void FillCommands(CommandRecordingContext* commandContext); private: - void EncodeComputePass(id commandBuffer); - void EncodeRenderPass(id commandBuffer, + void EncodeComputePass(CommandRecordingContext* commandContext); + void EncodeRenderPass(CommandRecordingContext* commandContext, MTLRenderPassDescriptor* mtlRenderPass, - GlobalEncoders* globalEncoders, uint32_t width, uint32_t height); - void EncodeRenderPassInternal(id commandBuffer, + void EncodeRenderPassInternal(CommandRecordingContext* commandContext, MTLRenderPassDescriptor* mtlRenderPass, uint32_t width, uint32_t height); diff --git a/src/dawn_native/metal/CommandBufferMTL.mm b/src/dawn_native/metal/CommandBufferMTL.mm index 77596dd09b..866a6fe631 100644 --- a/src/dawn_native/metal/CommandBufferMTL.mm +++ b/src/dawn_native/metal/CommandBufferMTL.mm @@ -29,23 +29,6 @@ namespace dawn_native { namespace metal { - struct GlobalEncoders { - id blit = nil; - - void Finish() { - if (blit != nil) { - [blit endEncoding]; - blit = nil; // This will be autoreleased. - } - } - - void EnsureBlit(id commandBuffer) { - if (blit == nil) { - blit = [commandBuffer blitCommandEncoder]; - } - } - }; - namespace { // Allows this file to use MTLStoreActionStoreAndMultismapleResolve because the logic is @@ -133,7 +116,7 @@ namespace dawn_native { namespace metal { // Helper function for Toggle EmulateStoreAndMSAAResolve void ResolveInAnotherRenderPass( - id commandBuffer, + CommandRecordingContext* commandContext, const MTLRenderPassDescriptor* mtlRenderPass, const std::array, kMaxColorAttachments>& resolveTextures) { MTLRenderPassDescriptor* mtlRenderPassForResolve = @@ -155,9 +138,8 @@ namespace dawn_native { namespace metal { mtlRenderPass.colorAttachments[i].resolveSlice; } - id encoder = - [commandBuffer renderCommandEncoderWithDescriptor:mtlRenderPassForResolve]; - [encoder endEncoding]; + commandContext->BeginRender(mtlRenderPassForResolve); + commandContext->EndRender(); } // Helper functions for Toggle AlwaysResolveIntoZeroLevelAndLayer @@ -182,24 +164,22 @@ namespace dawn_native { namespace metal { return resolveTexture; } - void CopyIntoTrueResolveTarget(id commandBuffer, + void CopyIntoTrueResolveTarget(CommandRecordingContext* commandContext, 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)]; + uint32_t height) { + [commandContext->EnsureBlit() 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)]; } // Metal uses a physical addressing mode which means buffers in the shading language are @@ -608,34 +588,33 @@ namespace dawn_native { namespace metal { FreeCommands(&mCommands); } - void CommandBuffer::FillCommands(id commandBuffer) { - GlobalEncoders encoders; - + void CommandBuffer::FillCommands(CommandRecordingContext* commandContext) { Command type; while (mCommands.NextCommandId(&type)) { switch (type) { case Command::BeginComputePass: { mCommands.NextCommand(); - encoders.Finish(); - EncodeComputePass(commandBuffer); + + commandContext->EndBlit(); + EncodeComputePass(commandContext); } break; case Command::BeginRenderPass: { BeginRenderPassCmd* cmd = mCommands.NextCommand(); - encoders.Finish(); + commandContext->EndBlit(); MTLRenderPassDescriptor* descriptor = CreateMTLRenderPassDescriptor(cmd); - EncodeRenderPass(commandBuffer, descriptor, &encoders, cmd->width, cmd->height); + EncodeRenderPass(commandContext, descriptor, cmd->width, cmd->height); } break; case Command::CopyBufferToBuffer: { CopyBufferToBufferCmd* copy = mCommands.NextCommand(); - encoders.EnsureBlit(commandBuffer); - [encoders.blit copyFromBuffer:ToBackend(copy->source)->GetMTLBuffer() - sourceOffset:copy->sourceOffset - toBuffer:ToBackend(copy->destination)->GetMTLBuffer() - destinationOffset:copy->destinationOffset - size:copy->size]; + [commandContext->EnsureBlit() + copyFromBuffer:ToBackend(copy->source)->GetMTLBuffer() + sourceOffset:copy->sourceOffset + toBuffer:ToBackend(copy->destination)->GetMTLBuffer() + destinationOffset:copy->destinationOffset + size:copy->size]; } break; case Command::CopyBufferToTexture: { @@ -651,18 +630,17 @@ namespace dawn_native { namespace metal { dst.origin, copySize, texture->GetFormat(), virtualSizeAtLevel, buffer->GetSize(), src.offset, src.rowPitch, src.imageHeight); - encoders.EnsureBlit(commandBuffer); for (uint32_t i = 0; i < splittedCopies.count; ++i) { const TextureBufferCopySplit::CopyInfo& copyInfo = splittedCopies.copies[i]; - [encoders.blit copyFromBuffer:buffer->GetMTLBuffer() - sourceOffset:copyInfo.bufferOffset - sourceBytesPerRow:copyInfo.bytesPerRow - sourceBytesPerImage:copyInfo.bytesPerImage - sourceSize:copyInfo.copyExtent - toTexture:texture->GetMTLTexture() - destinationSlice:dst.arrayLayer - destinationLevel:dst.mipLevel - destinationOrigin:copyInfo.textureOrigin]; + [commandContext->EnsureBlit() copyFromBuffer:buffer->GetMTLBuffer() + sourceOffset:copyInfo.bufferOffset + sourceBytesPerRow:copyInfo.bytesPerRow + sourceBytesPerImage:copyInfo.bytesPerImage + sourceSize:copyInfo.copyExtent + toTexture:texture->GetMTLTexture() + destinationSlice:dst.arrayLayer + destinationLevel:dst.mipLevel + destinationOrigin:copyInfo.textureOrigin]; } } break; @@ -679,18 +657,17 @@ namespace dawn_native { namespace metal { src.origin, copySize, texture->GetFormat(), virtualSizeAtLevel, buffer->GetSize(), dst.offset, dst.rowPitch, dst.imageHeight); - encoders.EnsureBlit(commandBuffer); for (uint32_t i = 0; i < splittedCopies.count; ++i) { const TextureBufferCopySplit::CopyInfo& copyInfo = splittedCopies.copies[i]; - [encoders.blit copyFromTexture:texture->GetMTLTexture() - sourceSlice:src.arrayLayer - sourceLevel:src.mipLevel - sourceOrigin:copyInfo.textureOrigin - sourceSize:copyInfo.copyExtent - toBuffer:buffer->GetMTLBuffer() - destinationOffset:copyInfo.bufferOffset - destinationBytesPerRow:copyInfo.bytesPerRow - destinationBytesPerImage:copyInfo.bytesPerImage]; + [commandContext->EnsureBlit() copyFromTexture:texture->GetMTLTexture() + sourceSlice:src.arrayLayer + sourceLevel:src.mipLevel + sourceOrigin:copyInfo.textureOrigin + sourceSize:copyInfo.copyExtent + toBuffer:buffer->GetMTLBuffer() + destinationOffset:copyInfo.bufferOffset + destinationBytesPerRow:copyInfo.bytesPerRow + destinationBytesPerImage:copyInfo.bytesPerImage]; } } break; @@ -700,40 +677,38 @@ namespace dawn_native { namespace metal { Texture* srcTexture = ToBackend(copy->source.texture.Get()); Texture* dstTexture = ToBackend(copy->destination.texture.Get()); - encoders.EnsureBlit(commandBuffer); - - [encoders.blit copyFromTexture:srcTexture->GetMTLTexture() - sourceSlice:copy->source.arrayLayer - sourceLevel:copy->source.mipLevel - sourceOrigin:MakeMTLOrigin(copy->source.origin) - sourceSize:MakeMTLSize(copy->copySize) - toTexture:dstTexture->GetMTLTexture() - destinationSlice:copy->destination.arrayLayer - destinationLevel:copy->destination.mipLevel - destinationOrigin:MakeMTLOrigin(copy->destination.origin)]; + [commandContext->EnsureBlit() + copyFromTexture:srcTexture->GetMTLTexture() + sourceSlice:copy->source.arrayLayer + sourceLevel:copy->source.mipLevel + sourceOrigin:MakeMTLOrigin(copy->source.origin) + sourceSize:MakeMTLSize(copy->copySize) + toTexture:dstTexture->GetMTLTexture() + destinationSlice:copy->destination.arrayLayer + destinationLevel:copy->destination.mipLevel + destinationOrigin:MakeMTLOrigin(copy->destination.origin)]; } break; default: { UNREACHABLE(); } break; } } - encoders.Finish(); + commandContext->EndBlit(); } - void CommandBuffer::EncodeComputePass(id commandBuffer) { + void CommandBuffer::EncodeComputePass(CommandRecordingContext* commandContext) { ComputePipeline* lastPipeline = nullptr; StorageBufferLengthTracker storageBufferLengths = {}; BindGroupTracker bindGroups(&storageBufferLengths); - // Will be autoreleased - id encoder = [commandBuffer computeCommandEncoder]; + id encoder = commandContext->BeginCompute(); Command type; while (mCommands.NextCommandId(&type)) { switch (type) { case Command::EndComputePass: { mCommands.NextCommand(); - [encoder endEncoding]; + commandContext->EndCompute(); return; } break; @@ -813,12 +788,11 @@ namespace dawn_native { namespace metal { UNREACHABLE(); } - void CommandBuffer::EncodeRenderPass(id commandBuffer, + void CommandBuffer::EncodeRenderPass(CommandRecordingContext* commandContext, MTLRenderPassDescriptor* mtlRenderPass, - GlobalEncoders* globalEncoders, uint32_t width, uint32_t height) { - ASSERT(mtlRenderPass && globalEncoders); + ASSERT(mtlRenderPass); Device* device = ToBackend(GetDevice()); @@ -861,17 +835,16 @@ namespace dawn_native { namespace metal { // 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); + EncodeRenderPass(commandContext, mtlRenderPass, width, height); for (uint32_t i = 0; i < kMaxColorAttachments; ++i) { if (trueResolveTextures[i] == nil) { continue; } ASSERT(temporaryResolveTextures[i] != nil); - CopyIntoTrueResolveTarget(commandBuffer, trueResolveTextures[i], + CopyIntoTrueResolveTarget(commandContext, trueResolveTextures[i], trueResolveLevels[i], trueResolveSlices[i], - temporaryResolveTextures[i], width, height, - globalEncoders); + temporaryResolveTextures[i], width, height); } return; } @@ -896,16 +869,16 @@ namespace dawn_native { namespace metal { // If we found a store + MSAA resolve we need to resolve in a different render pass. if (hasStoreAndMSAAResolve) { - EncodeRenderPass(commandBuffer, mtlRenderPass, globalEncoders, width, height); - ResolveInAnotherRenderPass(commandBuffer, mtlRenderPass, resolveTextures); + EncodeRenderPass(commandContext, mtlRenderPass, width, height); + ResolveInAnotherRenderPass(commandContext, mtlRenderPass, resolveTextures); return; } } - EncodeRenderPassInternal(commandBuffer, mtlRenderPass, width, height); + EncodeRenderPassInternal(commandContext, mtlRenderPass, width, height); } - void CommandBuffer::EncodeRenderPassInternal(id commandBuffer, + void CommandBuffer::EncodeRenderPassInternal(CommandRecordingContext* commandContext, MTLRenderPassDescriptor* mtlRenderPass, uint32_t width, uint32_t height) { @@ -916,9 +889,7 @@ namespace dawn_native { namespace metal { StorageBufferLengthTracker storageBufferLengths = {}; BindGroupTracker bindGroups(&storageBufferLengths); - // This will be autoreleased - id encoder = - [commandBuffer renderCommandEncoderWithDescriptor:mtlRenderPass]; + id encoder = commandContext->BeginRender(mtlRenderPass); auto EncodeRenderBundleCommand = [&](CommandIterator* iter, Command type) { switch (type) { @@ -1068,7 +1039,7 @@ namespace dawn_native { namespace metal { switch (type) { case Command::EndRenderPass: { mCommands.NextCommand(); - [encoder endEncoding]; + commandContext->EndRender(); return; } break; diff --git a/src/dawn_native/metal/CommandRecordingContext.h b/src/dawn_native/metal/CommandRecordingContext.h new file mode 100644 index 0000000000..531681b4ba --- /dev/null +++ b/src/dawn_native/metal/CommandRecordingContext.h @@ -0,0 +1,59 @@ +// Copyright 2020 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#ifndef DAWNNATIVE_METAL_COMMANDRECORDINGCONTEXT_H_ +#define DAWNNATIVE_METAL_COMMANDRECORDINGCONTEXT_H_ + +#import + +namespace dawn_native { namespace metal { + + // This class wraps a MTLCommandBuffer and tracks which Metal encoder is open. + // Only one encoder may be open at a time. + class CommandRecordingContext { + public: + CommandRecordingContext(); + CommandRecordingContext(id commands); + + CommandRecordingContext(const CommandRecordingContext& rhs) = delete; + CommandRecordingContext& operator=(const CommandRecordingContext& rhs) = delete; + + CommandRecordingContext(CommandRecordingContext&& rhs); + CommandRecordingContext& operator=(CommandRecordingContext&& rhs); + + ~CommandRecordingContext(); + + id GetCommands(); + + id AcquireCommands(); + + id EnsureBlit(); + void EndBlit(); + + id BeginCompute(); + void EndCompute(); + + id BeginRender(MTLRenderPassDescriptor* descriptor); + void EndRender(); + + private: + id mCommands = nil; + id mBlit = nil; + id mCompute = nil; + id mRender = nil; + bool mInEncoder = false; + }; + +}} // namespace dawn_native::metal + +#endif // DAWNNATIVE_METAL_COMMANDRECORDINGCONTEXT_H_ diff --git a/src/dawn_native/metal/CommandRecordingContext.mm b/src/dawn_native/metal/CommandRecordingContext.mm new file mode 100644 index 0000000000..3ede6268be --- /dev/null +++ b/src/dawn_native/metal/CommandRecordingContext.mm @@ -0,0 +1,119 @@ +// Copyright 2020 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "dawn_native/metal/CommandRecordingContext.h" + +#include "common/Assert.h" + +namespace dawn_native { namespace metal { + + CommandRecordingContext::CommandRecordingContext() = default; + + CommandRecordingContext::CommandRecordingContext(id commands) + : mCommands(commands) { + } + + CommandRecordingContext::CommandRecordingContext(CommandRecordingContext&& rhs) + : mCommands(rhs.AcquireCommands()) { + } + + CommandRecordingContext& CommandRecordingContext::operator=(CommandRecordingContext&& rhs) { + mCommands = rhs.AcquireCommands(); + return *this; + } + + CommandRecordingContext::~CommandRecordingContext() { + // Commands must be acquired. + ASSERT(mCommands == nil); + } + + id CommandRecordingContext::GetCommands() { + return mCommands; + } + + id CommandRecordingContext::AcquireCommands() { + ASSERT(!mInEncoder); + + id commands = mCommands; + mCommands = nil; + return commands; + } + + id CommandRecordingContext::EnsureBlit() { + ASSERT(mCommands != nil); + + if (mBlit == nil) { + ASSERT(!mInEncoder); + mInEncoder = true; + // The autorelease pool may drain before the encoder is ended. Retain so it stays alive. + mBlit = [[mCommands blitCommandEncoder] retain]; + } + return mBlit; + } + + void CommandRecordingContext::EndBlit() { + ASSERT(mCommands != nil); + + if (mBlit != nil) { + [mBlit endEncoding]; + [mBlit release]; + mBlit = nil; + mInEncoder = false; + } + } + + id CommandRecordingContext::BeginCompute() { + ASSERT(mCommands != nil); + ASSERT(mCompute == nil); + ASSERT(!mInEncoder); + + mInEncoder = true; + // The autorelease pool may drain before the encoder is ended. Retain so it stays alive. + mCompute = [[mCommands computeCommandEncoder] retain]; + return mCompute; + } + + void CommandRecordingContext::EndCompute() { + ASSERT(mCommands != nil); + ASSERT(mCompute != nil); + + [mCompute endEncoding]; + [mCompute release]; + mCompute = nil; + mInEncoder = false; + } + + id CommandRecordingContext::BeginRender( + MTLRenderPassDescriptor* descriptor) { + ASSERT(mCommands != nil); + ASSERT(mRender == nil); + ASSERT(!mInEncoder); + + mInEncoder = true; + // The autorelease pool may drain before the encoder is ended. Retain so it stays alive. + mRender = [[mCommands renderCommandEncoderWithDescriptor:descriptor] retain]; + return mRender; + } + + void CommandRecordingContext::EndRender() { + ASSERT(mCommands != nil); + ASSERT(mRender != nil); + + [mRender endEncoding]; + [mRender release]; + mRender = nil; + mInEncoder = false; + } + +}} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/DeviceMTL.h b/src/dawn_native/metal/DeviceMTL.h index 2219ab6aa5..667eb56728 100644 --- a/src/dawn_native/metal/DeviceMTL.h +++ b/src/dawn_native/metal/DeviceMTL.h @@ -19,6 +19,7 @@ #include "common/Serial.h" #include "dawn_native/Device.h" +#include "dawn_native/metal/CommandRecordingContext.h" #include "dawn_native/metal/Forward.h" #import @@ -48,7 +49,7 @@ namespace dawn_native { namespace metal { id GetMTLDevice(); id GetMTLQueue(); - id GetPendingCommandBuffer(); + CommandRecordingContext* GetPendingCommandContext(); Serial GetPendingCommandSerial() const override; void SubmitPendingCommandBuffer(); @@ -98,7 +99,7 @@ namespace dawn_native { namespace metal { std::unique_ptr mMapTracker; Serial mLastSubmittedSerial = 0; - id mPendingCommands = nil; + CommandRecordingContext mCommandContext; // The completed serial is updated in a Metal completion handler that can be fired on a // different thread, so it needs to be atomic. diff --git a/src/dawn_native/metal/DeviceMTL.mm b/src/dawn_native/metal/DeviceMTL.mm index 6cd73a9c1e..77faa4e377 100644 --- a/src/dawn_native/metal/DeviceMTL.mm +++ b/src/dawn_native/metal/DeviceMTL.mm @@ -144,7 +144,7 @@ namespace dawn_native { namespace metal { mDynamicUploader->Deallocate(completedSerial); mMapTracker->Tick(completedSerial); - if (mPendingCommands != nil) { + if (mCommandContext.GetCommands() != nil) { SubmitPendingCommandBuffer(); } else if (completedSerial == mLastSubmittedSerial) { // If there's no GPU work in flight we still need to artificially increment the serial @@ -164,46 +164,43 @@ namespace dawn_native { namespace metal { return mCommandQueue; } - id Device::GetPendingCommandBuffer() { - TRACE_EVENT0(GetPlatform(), General, "DeviceMTL::GetPendingCommandBuffer"); - if (mPendingCommands == nil) { - mPendingCommands = [mCommandQueue commandBuffer]; - [mPendingCommands retain]; + CommandRecordingContext* Device::GetPendingCommandContext() { + if (mCommandContext.GetCommands() == nil) { + TRACE_EVENT0(GetPlatform(), General, "[MTLCommandQueue commandBuffer]"); + // The MTLCommandBuffer will be autoreleased by default. + // The autorelease pool may drain before the command buffer is submitted. Retain so it + // stays alive. + mCommandContext = CommandRecordingContext([[mCommandQueue commandBuffer] retain]); } - return mPendingCommands; + return &mCommandContext; } void Device::SubmitPendingCommandBuffer() { - if (mPendingCommands == nil) { + if (mCommandContext.GetCommands() == nil) { return; } mLastSubmittedSerial++; + // Ensure the blit encoder is ended. It may have been opened to perform a lazy clear or + // buffer upload. + mCommandContext.EndBlit(); + + // Acquire the pending command buffer, which is retained. It must be released later. + id pendingCommands = mCommandContext.AcquireCommands(); + // Replace mLastSubmittedCommands with the mutex held so we avoid races between the // schedule handler and this code. { std::lock_guard lock(mLastSubmittedCommandsMutex); - [mLastSubmittedCommands release]; - mLastSubmittedCommands = mPendingCommands; + mLastSubmittedCommands = pendingCommands; } - // Ok, ObjC blocks are weird. My understanding is that local variables are captured by - // value so this-> works as expected. However it is unclear how members are captured, (are - // they captured using this-> or by value?). To be safe we copy members to local variables - // to ensure they are captured "by value". - - // Free mLastSubmittedCommands as soon as it is scheduled so that it doesn't hold - // references to its resources. Make a local copy of pendingCommands first so it is - // captured "by-value" by the block. - id pendingCommands = mPendingCommands; - - [mPendingCommands addScheduledHandler:^(id) { + [pendingCommands addScheduledHandler:^(id) { // This is DRF because we hold the mutex for mLastSubmittedCommands and pendingCommands // is a local value (and not the member itself). std::lock_guard lock(mLastSubmittedCommandsMutex); if (this->mLastSubmittedCommands == pendingCommands) { - [this->mLastSubmittedCommands release]; this->mLastSubmittedCommands = nil; } }]; @@ -211,7 +208,7 @@ namespace dawn_native { namespace metal { // Update the completed serial once the completed handler is fired. Make a local copy of // mLastSubmittedSerial so it is captured by value. Serial pendingSerial = mLastSubmittedSerial; - [mPendingCommands addCompletedHandler:^(id) { + [pendingCommands addCompletedHandler:^(id) { TRACE_EVENT_ASYNC_END0(GetPlatform(), GPUWork, "DeviceMTL::SubmitPendingCommandBuffer", pendingSerial); ASSERT(pendingSerial > mCompletedSerial.load()); @@ -220,8 +217,8 @@ namespace dawn_native { namespace metal { TRACE_EVENT_ASYNC_BEGIN0(GetPlatform(), GPUWork, "DeviceMTL::SubmitPendingCommandBuffer", pendingSerial); - [mPendingCommands commit]; - mPendingCommands = nil; + [pendingCommands commit]; + [pendingCommands release]; } MapRequestTracker* Device::GetMapTracker() const { @@ -242,15 +239,11 @@ namespace dawn_native { namespace metal { uint64_t size) { id uploadBuffer = ToBackend(source)->GetBufferHandle(); id buffer = ToBackend(destination)->GetMTLBuffer(); - id commandBuffer = GetPendingCommandBuffer(); - id encoder = [commandBuffer blitCommandEncoder]; - [encoder copyFromBuffer:uploadBuffer - sourceOffset:sourceOffset - toBuffer:buffer - destinationOffset:destinationOffset - size:size]; - [encoder endEncoding]; - + [GetPendingCommandContext()->EnsureBlit() copyFromBuffer:uploadBuffer + sourceOffset:sourceOffset + toBuffer:buffer + destinationOffset:destinationOffset + size:size]; return {}; } @@ -273,8 +266,7 @@ namespace dawn_native { namespace metal { } MaybeError Device::WaitForIdleForDestruction() { - [mPendingCommands release]; - mPendingCommands = nil; + [mCommandContext.AcquireCommands() release]; // Wait for all commands to be finished so we can free resources while (GetCompletedCommandSerial() != mLastSubmittedSerial) { @@ -285,10 +277,7 @@ namespace dawn_native { namespace metal { } void Device::Destroy() { - if (mPendingCommands != nil) { - [mPendingCommands release]; - mPendingCommands = nil; - } + [mCommandContext.AcquireCommands() release]; mMapTracker = nullptr; mDynamicUploader = nullptr; diff --git a/src/dawn_native/metal/QueueMTL.mm b/src/dawn_native/metal/QueueMTL.mm index dd360e970b..7c5967ad81 100644 --- a/src/dawn_native/metal/QueueMTL.mm +++ b/src/dawn_native/metal/QueueMTL.mm @@ -27,11 +27,11 @@ namespace dawn_native { namespace metal { MaybeError Queue::SubmitImpl(uint32_t commandCount, CommandBufferBase* const* commands) { Device* device = ToBackend(GetDevice()); device->Tick(); - id commandBuffer = device->GetPendingCommandBuffer(); + CommandRecordingContext* commandContext = device->GetPendingCommandContext(); TRACE_EVENT_BEGIN0(GetDevice()->GetPlatform(), Recording, "CommandBufferMTL::FillCommands"); for (uint32_t i = 0; i < commandCount; ++i) { - ToBackend(commands[i])->FillCommands(commandBuffer); + ToBackend(commands[i])->FillCommands(commandContext); } TRACE_EVENT_END0(GetDevice()->GetPlatform(), Recording, "CommandBufferMTL::FillCommands"); diff --git a/src/tests/white_box/MetalAutoreleasePoolTests.mm b/src/tests/white_box/MetalAutoreleasePoolTests.mm new file mode 100644 index 0000000000..a5c49d676f --- /dev/null +++ b/src/tests/white_box/MetalAutoreleasePoolTests.mm @@ -0,0 +1,61 @@ +// Copyright 2020 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "tests/DawnTest.h" + +#include "dawn_native/metal/DeviceMTL.h" + +using namespace dawn_native::metal; + +class MetalAutoreleasePoolTests : public DawnTest { + private: + void TestSetUp() override { + DAWN_SKIP_TEST_IF(UsesWire()); + + mMtlDevice = reinterpret_cast(device.Get()); + } + + protected: + Device* mMtlDevice = nullptr; +}; + +// Test that the MTLCommandBuffer owned by the pending command context can +// outlive an autoreleasepool block. +TEST_P(MetalAutoreleasePoolTests, CommandBufferOutlivesAutorelease) { + @autoreleasepool { + // Get the recording context which will allocate a MTLCommandBuffer. + // It will get autoreleased at the end of this block. + mMtlDevice->GetPendingCommandContext(); + } + + // Submitting the command buffer should succeed. + mMtlDevice->SubmitPendingCommandBuffer(); +} + +// Test that the MTLBlitCommandEncoder owned by the pending command context +// can outlive an autoreleasepool block. +TEST_P(MetalAutoreleasePoolTests, EncoderOutlivesAutorelease) { + @autoreleasepool { + // Get the recording context which will allocate a MTLCommandBuffer. + // Begin a blit encoder. + // Both will get autoreleased at the end of this block. + mMtlDevice->GetPendingCommandContext()->EnsureBlit(); + } + + // Submitting the command buffer should succeed. + mMtlDevice->GetPendingCommandContext()->EndBlit(); + mMtlDevice->SubmitPendingCommandBuffer(); +} + +DAWN_INSTANTIATE_TEST(MetalAutoreleasePoolTests, MetalBackend);