From 15c442e941fbabc6bf0055ef3583cf5bfabbaa11 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Mon, 13 Jan 2020 13:47:01 +0000 Subject: [PATCH] Revert "Metal: Add CommandRecordingContext" This reverts commit 2b3975f808fd6f5afc5a52e58a3dcd5e73984b17. Reason for revert: causes the failure in crbug.com/1041358 Original change's description: > Metal: Add CommandRecordingContext > > Introduces the idea of a CommandRecordingContext to the Metal backend, > similar to other backends. This is a class to track which Metal encoder > is open on the device-global pending MTLCommandBuffer. > It will be needed to open/close encoders for lazy clearing. > > Bug: dawn:145 > Change-Id: Ief6b71a079d73943677d2b61382d1c36b88a4f87 > Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/14780 > Reviewed-by: Corentin Wallez > Reviewed-by: Kai Ninomiya > Commit-Queue: Austin Eng TBR=cwallez@chromium.org,kainino@chromium.org,enga@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: dawn:145 Bug: chromium:1041358 Change-Id: I05c76cd96f723230d05cff65127dc8513d5e03c5 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15060 Commit-Queue: Corentin Wallez Reviewed-by: Corentin Wallez --- BUILD.gn | 2 - src/dawn_native/metal/CommandBufferMTL.h | 11 +- src/dawn_native/metal/CommandBufferMTL.mm | 167 ++++++++++-------- .../metal/CommandRecordingContext.h | 59 ------- .../metal/CommandRecordingContext.mm | 113 ------------ src/dawn_native/metal/DeviceMTL.h | 5 +- src/dawn_native/metal/DeviceMTL.mm | 65 ++++--- src/dawn_native/metal/QueueMTL.mm | 4 +- 8 files changed, 146 insertions(+), 280 deletions(-) delete mode 100644 src/dawn_native/metal/CommandRecordingContext.h delete mode 100644 src/dawn_native/metal/CommandRecordingContext.mm diff --git a/BUILD.gn b/BUILD.gn index c7df5827aa..c013899207 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -354,8 +354,6 @@ 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", diff --git a/src/dawn_native/metal/CommandBufferMTL.h b/src/dawn_native/metal/CommandBufferMTL.h index 67a1313eba..640d19666b 100644 --- a/src/dawn_native/metal/CommandBufferMTL.h +++ b/src/dawn_native/metal/CommandBufferMTL.h @@ -26,24 +26,25 @@ 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(CommandRecordingContext* commandContext); + void FillCommands(id commandBuffer); private: - void EncodeComputePass(CommandRecordingContext* commandContext); - void EncodeRenderPass(CommandRecordingContext* commandContext, + void EncodeComputePass(id commandBuffer); + void EncodeRenderPass(id commandBuffer, MTLRenderPassDescriptor* mtlRenderPass, + GlobalEncoders* globalEncoders, uint32_t width, uint32_t height); - void EncodeRenderPassInternal(CommandRecordingContext* commandContext, + void EncodeRenderPassInternal(id commandBuffer, 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 866a6fe631..77596dd09b 100644 --- a/src/dawn_native/metal/CommandBufferMTL.mm +++ b/src/dawn_native/metal/CommandBufferMTL.mm @@ -29,6 +29,23 @@ 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 @@ -116,7 +133,7 @@ namespace dawn_native { namespace metal { // Helper function for Toggle EmulateStoreAndMSAAResolve void ResolveInAnotherRenderPass( - CommandRecordingContext* commandContext, + id commandBuffer, const MTLRenderPassDescriptor* mtlRenderPass, const std::array, kMaxColorAttachments>& resolveTextures) { MTLRenderPassDescriptor* mtlRenderPassForResolve = @@ -138,8 +155,9 @@ namespace dawn_native { namespace metal { mtlRenderPass.colorAttachments[i].resolveSlice; } - commandContext->BeginRender(mtlRenderPassForResolve); - commandContext->EndRender(); + id encoder = + [commandBuffer renderCommandEncoderWithDescriptor:mtlRenderPassForResolve]; + [encoder endEncoding]; } // Helper functions for Toggle AlwaysResolveIntoZeroLevelAndLayer @@ -164,22 +182,24 @@ namespace dawn_native { namespace metal { return resolveTexture; } - void CopyIntoTrueResolveTarget(CommandRecordingContext* commandContext, + void CopyIntoTrueResolveTarget(id commandBuffer, id mtlTrueResolveTexture, uint32_t trueResolveLevel, uint32_t trueResolveSlice, id temporaryResolveTexture, uint32_t width, - 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)]; + 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)]; } // Metal uses a physical addressing mode which means buffers in the shading language are @@ -588,33 +608,34 @@ namespace dawn_native { namespace metal { FreeCommands(&mCommands); } - void CommandBuffer::FillCommands(CommandRecordingContext* commandContext) { + void CommandBuffer::FillCommands(id commandBuffer) { + GlobalEncoders encoders; + Command type; while (mCommands.NextCommandId(&type)) { switch (type) { case Command::BeginComputePass: { mCommands.NextCommand(); - - commandContext->EndBlit(); - EncodeComputePass(commandContext); + encoders.Finish(); + EncodeComputePass(commandBuffer); } break; case Command::BeginRenderPass: { BeginRenderPassCmd* cmd = mCommands.NextCommand(); - commandContext->EndBlit(); + encoders.Finish(); MTLRenderPassDescriptor* descriptor = CreateMTLRenderPassDescriptor(cmd); - EncodeRenderPass(commandContext, descriptor, cmd->width, cmd->height); + EncodeRenderPass(commandBuffer, descriptor, &encoders, cmd->width, cmd->height); } break; case Command::CopyBufferToBuffer: { CopyBufferToBufferCmd* copy = mCommands.NextCommand(); - [commandContext->EnsureBlit() - copyFromBuffer:ToBackend(copy->source)->GetMTLBuffer() - sourceOffset:copy->sourceOffset - toBuffer:ToBackend(copy->destination)->GetMTLBuffer() - destinationOffset:copy->destinationOffset - size:copy->size]; + encoders.EnsureBlit(commandBuffer); + [encoders.blit copyFromBuffer:ToBackend(copy->source)->GetMTLBuffer() + sourceOffset:copy->sourceOffset + toBuffer:ToBackend(copy->destination)->GetMTLBuffer() + destinationOffset:copy->destinationOffset + size:copy->size]; } break; case Command::CopyBufferToTexture: { @@ -630,17 +651,18 @@ 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]; - [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]; + [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]; } } break; @@ -657,17 +679,18 @@ 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]; - [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]; + [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]; } } break; @@ -677,38 +700,40 @@ namespace dawn_native { namespace metal { Texture* srcTexture = ToBackend(copy->source.texture.Get()); Texture* dstTexture = ToBackend(copy->destination.texture.Get()); - [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)]; + 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)]; } break; default: { UNREACHABLE(); } break; } } - commandContext->EndBlit(); + encoders.Finish(); } - void CommandBuffer::EncodeComputePass(CommandRecordingContext* commandContext) { + void CommandBuffer::EncodeComputePass(id commandBuffer) { ComputePipeline* lastPipeline = nullptr; StorageBufferLengthTracker storageBufferLengths = {}; BindGroupTracker bindGroups(&storageBufferLengths); - id encoder = commandContext->BeginCompute(); + // Will be autoreleased + id encoder = [commandBuffer computeCommandEncoder]; Command type; while (mCommands.NextCommandId(&type)) { switch (type) { case Command::EndComputePass: { mCommands.NextCommand(); - commandContext->EndCompute(); + [encoder endEncoding]; return; } break; @@ -788,11 +813,12 @@ namespace dawn_native { namespace metal { UNREACHABLE(); } - void CommandBuffer::EncodeRenderPass(CommandRecordingContext* commandContext, + void CommandBuffer::EncodeRenderPass(id commandBuffer, MTLRenderPassDescriptor* mtlRenderPass, + GlobalEncoders* globalEncoders, uint32_t width, uint32_t height) { - ASSERT(mtlRenderPass); + ASSERT(mtlRenderPass && globalEncoders); Device* device = ToBackend(GetDevice()); @@ -835,16 +861,17 @@ 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(commandContext, mtlRenderPass, width, height); + EncodeRenderPass(commandBuffer, mtlRenderPass, globalEncoders, width, height); for (uint32_t i = 0; i < kMaxColorAttachments; ++i) { if (trueResolveTextures[i] == nil) { continue; } ASSERT(temporaryResolveTextures[i] != nil); - CopyIntoTrueResolveTarget(commandContext, trueResolveTextures[i], + CopyIntoTrueResolveTarget(commandBuffer, trueResolveTextures[i], trueResolveLevels[i], trueResolveSlices[i], - temporaryResolveTextures[i], width, height); + temporaryResolveTextures[i], width, height, + globalEncoders); } return; } @@ -869,16 +896,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(commandContext, mtlRenderPass, width, height); - ResolveInAnotherRenderPass(commandContext, mtlRenderPass, resolveTextures); + EncodeRenderPass(commandBuffer, mtlRenderPass, globalEncoders, width, height); + ResolveInAnotherRenderPass(commandBuffer, mtlRenderPass, resolveTextures); return; } } - EncodeRenderPassInternal(commandContext, mtlRenderPass, width, height); + EncodeRenderPassInternal(commandBuffer, mtlRenderPass, width, height); } - void CommandBuffer::EncodeRenderPassInternal(CommandRecordingContext* commandContext, + void CommandBuffer::EncodeRenderPassInternal(id commandBuffer, MTLRenderPassDescriptor* mtlRenderPass, uint32_t width, uint32_t height) { @@ -889,7 +916,9 @@ namespace dawn_native { namespace metal { StorageBufferLengthTracker storageBufferLengths = {}; BindGroupTracker bindGroups(&storageBufferLengths); - id encoder = commandContext->BeginRender(mtlRenderPass); + // This will be autoreleased + id encoder = + [commandBuffer renderCommandEncoderWithDescriptor:mtlRenderPass]; auto EncodeRenderBundleCommand = [&](CommandIterator* iter, Command type) { switch (type) { @@ -1039,7 +1068,7 @@ namespace dawn_native { namespace metal { switch (type) { case Command::EndRenderPass: { mCommands.NextCommand(); - commandContext->EndRender(); + [encoder endEncoding]; return; } break; diff --git a/src/dawn_native/metal/CommandRecordingContext.h b/src/dawn_native/metal/CommandRecordingContext.h deleted file mode 100644 index 531681b4ba..0000000000 --- a/src/dawn_native/metal/CommandRecordingContext.h +++ /dev/null @@ -1,59 +0,0 @@ -// 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 deleted file mode 100644 index df4d6f8118..0000000000 --- a/src/dawn_native/metal/CommandRecordingContext.mm +++ /dev/null @@ -1,113 +0,0 @@ -// 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; - mBlit = [mCommands blitCommandEncoder]; - } - return mBlit; - } - - void CommandRecordingContext::EndBlit() { - ASSERT(mCommands != nil); - - if (mBlit != nil) { - [mBlit endEncoding]; - mBlit = nil; // This will be autoreleased. - mInEncoder = false; - } - } - - id CommandRecordingContext::BeginCompute() { - ASSERT(mCommands != nil); - ASSERT(mCompute == nil); - ASSERT(!mInEncoder); - - mInEncoder = true; - mCompute = [mCommands computeCommandEncoder]; - return mCompute; - } - - void CommandRecordingContext::EndCompute() { - ASSERT(mCommands != nil); - ASSERT(mCompute != nil); - - [mCompute endEncoding]; - mCompute = nil; // This will be autoreleased. - mInEncoder = false; - } - - id CommandRecordingContext::BeginRender( - MTLRenderPassDescriptor* descriptor) { - ASSERT(mCommands != nil); - ASSERT(mRender == nil); - ASSERT(!mInEncoder); - - mInEncoder = true; - mRender = [mCommands renderCommandEncoderWithDescriptor:descriptor]; - return mRender; - } - - void CommandRecordingContext::EndRender() { - ASSERT(mCommands != nil); - ASSERT(mRender != nil); - - [mRender endEncoding]; - mRender = nil; // This will be autoreleased. - mInEncoder = false; - } - -}} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/DeviceMTL.h b/src/dawn_native/metal/DeviceMTL.h index 667eb56728..2219ab6aa5 100644 --- a/src/dawn_native/metal/DeviceMTL.h +++ b/src/dawn_native/metal/DeviceMTL.h @@ -19,7 +19,6 @@ #include "common/Serial.h" #include "dawn_native/Device.h" -#include "dawn_native/metal/CommandRecordingContext.h" #include "dawn_native/metal/Forward.h" #import @@ -49,7 +48,7 @@ namespace dawn_native { namespace metal { id GetMTLDevice(); id GetMTLQueue(); - CommandRecordingContext* GetPendingCommandContext(); + id GetPendingCommandBuffer(); Serial GetPendingCommandSerial() const override; void SubmitPendingCommandBuffer(); @@ -99,7 +98,7 @@ namespace dawn_native { namespace metal { std::unique_ptr mMapTracker; Serial mLastSubmittedSerial = 0; - CommandRecordingContext mCommandContext; + id mPendingCommands = nil; // 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 54b1a19797..6cd73a9c1e 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 (mCommandContext.GetCommands() != nil) { + if (mPendingCommands != nil) { SubmitPendingCommandBuffer(); } else if (completedSerial == mLastSubmittedSerial) { // If there's no GPU work in flight we still need to artificially increment the serial @@ -164,43 +164,45 @@ namespace dawn_native { namespace metal { return mCommandQueue; } - CommandRecordingContext* Device::GetPendingCommandContext() { - if (mCommandContext.GetCommands() == nil) { - TRACE_EVENT0(GetPlatform(), General, "[MTLCommandQueue commandBuffer]"); - mCommandContext = CommandRecordingContext([mCommandQueue commandBuffer]); + id Device::GetPendingCommandBuffer() { + TRACE_EVENT0(GetPlatform(), General, "DeviceMTL::GetPendingCommandBuffer"); + if (mPendingCommands == nil) { + mPendingCommands = [mCommandQueue commandBuffer]; + [mPendingCommands retain]; } - return &mCommandContext; + return mPendingCommands; } void Device::SubmitPendingCommandBuffer() { - if (mCommandContext.GetCommands() == nil) { + if (mPendingCommands == 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 and retain the pending commands. We must keep them alive until scheduled. - id pendingCommands = [mCommandContext.AcquireCommands() retain]; - // 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 = pendingCommands; + mLastSubmittedCommands = mPendingCommands; } - [pendingCommands addScheduledHandler:^(id) { + // 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) { // 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) { - // Free mLastSubmittedCommands as soon as it is scheduled so that it doesn't hold - // references to its resources. [this->mLastSubmittedCommands release]; this->mLastSubmittedCommands = nil; } @@ -209,7 +211,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; - [pendingCommands addCompletedHandler:^(id) { + [mPendingCommands addCompletedHandler:^(id) { TRACE_EVENT_ASYNC_END0(GetPlatform(), GPUWork, "DeviceMTL::SubmitPendingCommandBuffer", pendingSerial); ASSERT(pendingSerial > mCompletedSerial.load()); @@ -218,7 +220,8 @@ namespace dawn_native { namespace metal { TRACE_EVENT_ASYNC_BEGIN0(GetPlatform(), GPUWork, "DeviceMTL::SubmitPendingCommandBuffer", pendingSerial); - [pendingCommands commit]; + [mPendingCommands commit]; + mPendingCommands = nil; } MapRequestTracker* Device::GetMapTracker() const { @@ -239,11 +242,15 @@ namespace dawn_native { namespace metal { uint64_t size) { id uploadBuffer = ToBackend(source)->GetBufferHandle(); id buffer = ToBackend(destination)->GetMTLBuffer(); - [GetPendingCommandContext()->EnsureBlit() copyFromBuffer:uploadBuffer - sourceOffset:sourceOffset - toBuffer:buffer - destinationOffset:destinationOffset - size:size]; + id commandBuffer = GetPendingCommandBuffer(); + id encoder = [commandBuffer blitCommandEncoder]; + [encoder copyFromBuffer:uploadBuffer + sourceOffset:sourceOffset + toBuffer:buffer + destinationOffset:destinationOffset + size:size]; + [encoder endEncoding]; + return {}; } @@ -266,7 +273,8 @@ namespace dawn_native { namespace metal { } MaybeError Device::WaitForIdleForDestruction() { - [mCommandContext.AcquireCommands() release]; + [mPendingCommands release]; + mPendingCommands = nil; // Wait for all commands to be finished so we can free resources while (GetCompletedCommandSerial() != mLastSubmittedSerial) { @@ -277,7 +285,10 @@ namespace dawn_native { namespace metal { } void Device::Destroy() { - [mCommandContext.AcquireCommands() release]; + if (mPendingCommands != nil) { + [mPendingCommands release]; + mPendingCommands = nil; + } mMapTracker = nullptr; mDynamicUploader = nullptr; diff --git a/src/dawn_native/metal/QueueMTL.mm b/src/dawn_native/metal/QueueMTL.mm index 7c5967ad81..dd360e970b 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(); - CommandRecordingContext* commandContext = device->GetPendingCommandContext(); + id commandBuffer = device->GetPendingCommandBuffer(); TRACE_EVENT_BEGIN0(GetDevice()->GetPlatform(), Recording, "CommandBufferMTL::FillCommands"); for (uint32_t i = 0; i < commandCount; ++i) { - ToBackend(commands[i])->FillCommands(commandContext); + ToBackend(commands[i])->FillCommands(commandBuffer); } TRACE_EVENT_END0(GetDevice()->GetPlatform(), Recording, "CommandBufferMTL::FillCommands");