Reland "Metal: Add CommandRecordingContext"

This is a reland of 2b3975f808

The previous CL failed to retain autoreleased ObjC objects which
should live longer than the autoreleasepool block. This reland fixes
the issue and adds tests for it.

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 <cwallez@chromium.org>
> Reviewed-by: Kai Ninomiya <kainino@chromium.org>
> Commit-Queue: Austin Eng <enga@chromium.org>

Bug: dawn:145
Change-Id: I67494b35225ce8f6443a3fa9787d054522e5d422
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15042
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

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" ]
   }

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<MTLCommandBuffer> commandBuffer);
+        void FillCommands(CommandRecordingContext* commandContext);
 
       private:
-        void EncodeComputePass(id<MTLCommandBuffer> commandBuffer);
+        void EncodeComputePass(CommandRecordingContext* commandContext);
-        void EncodeRenderPass(id<MTLCommandBuffer> commandBuffer,
+        void EncodeRenderPass(CommandRecordingContext* commandContext,
                               MTLRenderPassDescriptor* mtlRenderPass,
-                              GlobalEncoders* globalEncoders,
                               uint32_t width,
                               uint32_t height);
 
-        void EncodeRenderPassInternal(id<MTLCommandBuffer> commandBuffer,
+        void EncodeRenderPassInternal(CommandRecordingContext* commandContext,
                                       MTLRenderPassDescriptor* mtlRenderPass,
                                       uint32_t width,
                                       uint32_t height);

src/dawn_native/metal/CommandBufferMTL.mm

@@ -29,23 +29,6 @@
 
 namespace dawn_native { namespace metal {
 
-    struct GlobalEncoders {
-        id<MTLBlitCommandEncoder> blit = nil;
-
-        void Finish() {
-            if (blit != nil) {
-                [blit endEncoding];
-                blit = nil;  // This will be autoreleased.
-            }
-        }
-
-        void EnsureBlit(id<MTLCommandBuffer> 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<MTLCommandBuffer> commandBuffer,
+            CommandRecordingContext* commandContext,
             const MTLRenderPassDescriptor* mtlRenderPass,
             const std::array<id<MTLTexture>, kMaxColorAttachments>& resolveTextures) {
             MTLRenderPassDescriptor* mtlRenderPassForResolve =
@@ -155,9 +138,8 @@ namespace dawn_native { namespace metal {
                     mtlRenderPass.colorAttachments[i].resolveSlice;
             }
 
-            id<MTLRenderCommandEncoder> encoder =
+            commandContext->BeginRender(mtlRenderPassForResolve);
-                [commandBuffer renderCommandEncoderWithDescriptor:mtlRenderPassForResolve];
+            commandContext->EndRender();
-            [encoder endEncoding];
         }
 
         // Helper functions for Toggle AlwaysResolveIntoZeroLevelAndLayer
@@ -182,24 +164,22 @@ namespace dawn_native { namespace metal {
             return resolveTexture;
         }
 
-        void CopyIntoTrueResolveTarget(id<MTLCommandBuffer> commandBuffer,
+        void CopyIntoTrueResolveTarget(CommandRecordingContext* commandContext,
                                        id<MTLTexture> mtlTrueResolveTexture,
                                        uint32_t trueResolveLevel,
                                        uint32_t trueResolveSlice,
                                        id<MTLTexture> temporaryResolveTexture,
                                        uint32_t width,
-                                       uint32_t height,
+                                       uint32_t height) {
-                                       GlobalEncoders* encoders) {
+            [commandContext->EnsureBlit() copyFromTexture:temporaryResolveTexture
-            encoders->EnsureBlit(commandBuffer);
+                                              sourceSlice:0
-            [encoders->blit copyFromTexture:temporaryResolveTexture
+                                              sourceLevel:0
-                                sourceSlice:0
+                                             sourceOrigin:MTLOriginMake(0, 0, 0)
-                                sourceLevel:0
+                                               sourceSize:MTLSizeMake(width, height, 1)
-                               sourceOrigin:MTLOriginMake(0, 0, 0)
+                                                toTexture:mtlTrueResolveTexture
-                                 sourceSize:MTLSizeMake(width, height, 1)
+                                         destinationSlice:trueResolveSlice
-                                  toTexture:mtlTrueResolveTexture
+                                         destinationLevel:trueResolveLevel
-                           destinationSlice:trueResolveSlice
+                                        destinationOrigin:MTLOriginMake(0, 0, 0)];
-                           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<MTLCommandBuffer> commandBuffer) {
+    void CommandBuffer::FillCommands(CommandRecordingContext* commandContext) {
-        GlobalEncoders encoders;
-
         Command type;
         while (mCommands.NextCommandId(&type)) {
             switch (type) {
                 case Command::BeginComputePass: {
                     mCommands.NextCommand<BeginComputePassCmd>();
-                    encoders.Finish();
+
-                    EncodeComputePass(commandBuffer);
+                    commandContext->EndBlit();
+                    EncodeComputePass(commandContext);
                 } break;
 
                 case Command::BeginRenderPass: {
                     BeginRenderPassCmd* cmd = mCommands.NextCommand<BeginRenderPassCmd>();
-                    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<CopyBufferToBufferCmd>();
 
-                    encoders.EnsureBlit(commandBuffer);
+                    [commandContext->EnsureBlit()
-                    [encoders.blit copyFromBuffer:ToBackend(copy->source)->GetMTLBuffer()
+                           copyFromBuffer:ToBackend(copy->source)->GetMTLBuffer()
-                                     sourceOffset:copy->sourceOffset
+                             sourceOffset:copy->sourceOffset
-                                         toBuffer:ToBackend(copy->destination)->GetMTLBuffer()
+                                 toBuffer:ToBackend(copy->destination)->GetMTLBuffer()
-                                destinationOffset:copy->destinationOffset
+                        destinationOffset:copy->destinationOffset
-                                             size:copy->size];
+                                     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()
+                        [commandContext->EnsureBlit() copyFromBuffer:buffer->GetMTLBuffer()
-                                         sourceOffset:copyInfo.bufferOffset
+                                                        sourceOffset:copyInfo.bufferOffset
-                                    sourceBytesPerRow:copyInfo.bytesPerRow
+                                                   sourceBytesPerRow:copyInfo.bytesPerRow
-                                  sourceBytesPerImage:copyInfo.bytesPerImage
+                                                 sourceBytesPerImage:copyInfo.bytesPerImage
-                                           sourceSize:copyInfo.copyExtent
+                                                          sourceSize:copyInfo.copyExtent
-                                            toTexture:texture->GetMTLTexture()
+                                                           toTexture:texture->GetMTLTexture()
-                                     destinationSlice:dst.arrayLayer
+                                                    destinationSlice:dst.arrayLayer
-                                     destinationLevel:dst.mipLevel
+                                                    destinationLevel:dst.mipLevel
-                                    destinationOrigin:copyInfo.textureOrigin];
+                                                   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()
+                        [commandContext->EnsureBlit() copyFromTexture:texture->GetMTLTexture()
-                                           sourceSlice:src.arrayLayer
+                                                          sourceSlice:src.arrayLayer
-                                           sourceLevel:src.mipLevel
+                                                          sourceLevel:src.mipLevel
-                                          sourceOrigin:copyInfo.textureOrigin
+                                                         sourceOrigin:copyInfo.textureOrigin
-                                            sourceSize:copyInfo.copyExtent
+                                                           sourceSize:copyInfo.copyExtent
-                                              toBuffer:buffer->GetMTLBuffer()
+                                                             toBuffer:buffer->GetMTLBuffer()
-                                     destinationOffset:copyInfo.bufferOffset
+                                                    destinationOffset:copyInfo.bufferOffset
-                                destinationBytesPerRow:copyInfo.bytesPerRow
+                                               destinationBytesPerRow:copyInfo.bytesPerRow
-                              destinationBytesPerImage:copyInfo.bytesPerImage];
+                                             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);
+                    [commandContext->EnsureBlit()
-
+                          copyFromTexture:srcTexture->GetMTLTexture()
-                    [encoders.blit copyFromTexture:srcTexture->GetMTLTexture()
+                              sourceSlice:copy->source.arrayLayer
-                                       sourceSlice:copy->source.arrayLayer
+                              sourceLevel:copy->source.mipLevel
-                                       sourceLevel:copy->source.mipLevel
+                             sourceOrigin:MakeMTLOrigin(copy->source.origin)
-                                      sourceOrigin:MakeMTLOrigin(copy->source.origin)
+                               sourceSize:MakeMTLSize(copy->copySize)
-                                        sourceSize:MakeMTLSize(copy->copySize)
+                                toTexture:dstTexture->GetMTLTexture()
-                                         toTexture:dstTexture->GetMTLTexture()
+                         destinationSlice:copy->destination.arrayLayer
-                                  destinationSlice:copy->destination.arrayLayer
+                         destinationLevel:copy->destination.mipLevel
-                                  destinationLevel:copy->destination.mipLevel
+                        destinationOrigin:MakeMTLOrigin(copy->destination.origin)];
-                                 destinationOrigin:MakeMTLOrigin(copy->destination.origin)];
                 } break;
 
                 default: { UNREACHABLE(); } break;
             }
         }
 
-        encoders.Finish();
+        commandContext->EndBlit();
     }
 
-    void CommandBuffer::EncodeComputePass(id<MTLCommandBuffer> commandBuffer) {
+    void CommandBuffer::EncodeComputePass(CommandRecordingContext* commandContext) {
         ComputePipeline* lastPipeline = nullptr;
         StorageBufferLengthTracker storageBufferLengths = {};
         BindGroupTracker bindGroups(&storageBufferLengths);
 
-        // Will be autoreleased
+        id<MTLComputeCommandEncoder> encoder = commandContext->BeginCompute();
-        id<MTLComputeCommandEncoder> encoder = [commandBuffer computeCommandEncoder];
 
         Command type;
         while (mCommands.NextCommandId(&type)) {
             switch (type) {
                 case Command::EndComputePass: {
                     mCommands.NextCommand<EndComputePassCmd>();
-                    [encoder endEncoding];
+                    commandContext->EndCompute();
                     return;
                 } break;
 
@@ -813,12 +788,11 @@ namespace dawn_native { namespace metal {
         UNREACHABLE();
     }
 
-    void CommandBuffer::EncodeRenderPass(id<MTLCommandBuffer> 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,
+                                              temporaryResolveTextures[i], width, height);
-                                              globalEncoders);
                 }
                 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);
+                EncodeRenderPass(commandContext, mtlRenderPass, width, height);
-                ResolveInAnotherRenderPass(commandBuffer, mtlRenderPass, resolveTextures);
+                ResolveInAnotherRenderPass(commandContext, mtlRenderPass, resolveTextures);
                 return;
             }
         }
 
-        EncodeRenderPassInternal(commandBuffer, mtlRenderPass, width, height);
+        EncodeRenderPassInternal(commandContext, mtlRenderPass, width, height);
     }
 
-    void CommandBuffer::EncodeRenderPassInternal(id<MTLCommandBuffer> 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<MTLRenderCommandEncoder> encoder = commandContext->BeginRender(mtlRenderPass);
-        id<MTLRenderCommandEncoder> encoder =
-            [commandBuffer renderCommandEncoderWithDescriptor: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<EndRenderPassCmd>();
-                    [encoder endEncoding];
+                    commandContext->EndRender();
                     return;
                 } break;
 

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 <Metal/Metal.h>
+
+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<MTLCommandBuffer> commands);
+
+        CommandRecordingContext(const CommandRecordingContext& rhs) = delete;
+        CommandRecordingContext& operator=(const CommandRecordingContext& rhs) = delete;
+
+        CommandRecordingContext(CommandRecordingContext&& rhs);
+        CommandRecordingContext& operator=(CommandRecordingContext&& rhs);
+
+        ~CommandRecordingContext();
+
+        id<MTLCommandBuffer> GetCommands();
+
+        id<MTLCommandBuffer> AcquireCommands();
+
+        id<MTLBlitCommandEncoder> EnsureBlit();
+        void EndBlit();
+
+        id<MTLComputeCommandEncoder> BeginCompute();
+        void EndCompute();
+
+        id<MTLRenderCommandEncoder> BeginRender(MTLRenderPassDescriptor* descriptor);
+        void EndRender();
+
+      private:
+        id<MTLCommandBuffer> mCommands = nil;
+        id<MTLBlitCommandEncoder> mBlit = nil;
+        id<MTLComputeCommandEncoder> mCompute = nil;
+        id<MTLRenderCommandEncoder> mRender = nil;
+        bool mInEncoder = false;
+    };
+
+}}  // namespace dawn_native::metal
+
+#endif  // DAWNNATIVE_METAL_COMMANDRECORDINGCONTEXT_H_

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<MTLCommandBuffer> 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<MTLCommandBuffer> CommandRecordingContext::GetCommands() {
+        return mCommands;
+    }
+
+    id<MTLCommandBuffer> CommandRecordingContext::AcquireCommands() {
+        ASSERT(!mInEncoder);
+
+        id<MTLCommandBuffer> commands = mCommands;
+        mCommands = nil;
+        return commands;
+    }
+
+    id<MTLBlitCommandEncoder> 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<MTLComputeCommandEncoder> 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<MTLRenderCommandEncoder> 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

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 <IOSurface/IOSurfaceRef.h>
@@ -48,7 +49,7 @@ namespace dawn_native { namespace metal {
         id<MTLDevice> GetMTLDevice();
         id<MTLCommandQueue> GetMTLQueue();
 
-        id<MTLCommandBuffer> GetPendingCommandBuffer();
+        CommandRecordingContext* GetPendingCommandContext();
         Serial GetPendingCommandSerial() const override;
         void SubmitPendingCommandBuffer();
 
@@ -98,7 +99,7 @@ namespace dawn_native { namespace metal {
         std::unique_ptr<MapRequestTracker> mMapTracker;
 
         Serial mLastSubmittedSerial = 0;
-        id<MTLCommandBuffer> 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.

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<MTLCommandBuffer> Device::GetPendingCommandBuffer() {
+    CommandRecordingContext* Device::GetPendingCommandContext() {
-        TRACE_EVENT0(GetPlatform(), General, "DeviceMTL::GetPendingCommandBuffer");
+        if (mCommandContext.GetCommands() == nil) {
-        if (mPendingCommands == nil) {
+            TRACE_EVENT0(GetPlatform(), General, "[MTLCommandQueue commandBuffer]");
-            mPendingCommands = [mCommandQueue commandBuffer];
+            // The MTLCommandBuffer will be autoreleased by default.
-            [mPendingCommands retain];
+            // 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<MTLCommandBuffer> pendingCommands = mCommandContext.AcquireCommands();
+
         // Replace mLastSubmittedCommands with the mutex held so we avoid races between the
         // schedule handler and this code.
         {
             std::lock_guard<std::mutex> lock(mLastSubmittedCommandsMutex);
-            [mLastSubmittedCommands release];
+            mLastSubmittedCommands = pendingCommands;
-            mLastSubmittedCommands = mPendingCommands;
         }
 
-        // Ok, ObjC blocks are weird. My understanding is that local variables are captured by
+        [pendingCommands addScheduledHandler:^(id<MTLCommandBuffer>) {
-        // 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<MTLCommandBuffer> pendingCommands = mPendingCommands;
-
-        [mPendingCommands addScheduledHandler:^(id<MTLCommandBuffer>) {
             // This is DRF because we hold the mutex for mLastSubmittedCommands and pendingCommands
             // is a local value (and not the member itself).
             std::lock_guard<std::mutex> 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<MTLCommandBuffer>) {
+        [pendingCommands addCompletedHandler:^(id<MTLCommandBuffer>) {
             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];
+        [pendingCommands commit];
-        mPendingCommands = nil;
+        [pendingCommands release];
     }
 
     MapRequestTracker* Device::GetMapTracker() const {
@@ -242,15 +239,11 @@ namespace dawn_native { namespace metal {
                                                uint64_t size) {
         id<MTLBuffer> uploadBuffer = ToBackend(source)->GetBufferHandle();
         id<MTLBuffer> buffer = ToBackend(destination)->GetMTLBuffer();
-        id<MTLCommandBuffer> commandBuffer = GetPendingCommandBuffer();
+        [GetPendingCommandContext()->EnsureBlit() copyFromBuffer:uploadBuffer
-        id<MTLBlitCommandEncoder> encoder = [commandBuffer blitCommandEncoder];
+                                                    sourceOffset:sourceOffset
-        [encoder copyFromBuffer:uploadBuffer
+                                                        toBuffer:buffer
-                   sourceOffset:sourceOffset
+                                               destinationOffset:destinationOffset
-                       toBuffer:buffer
+                                                            size:size];
-              destinationOffset:destinationOffset
-                           size:size];
-        [encoder endEncoding];
-
         return {};
     }
 
@@ -273,8 +266,7 @@ namespace dawn_native { namespace metal {
     }
 
     MaybeError Device::WaitForIdleForDestruction() {
-        [mPendingCommands release];
+        [mCommandContext.AcquireCommands() release];
-        mPendingCommands = nil;
 
         // 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) {
+        [mCommandContext.AcquireCommands() release];
-            [mPendingCommands release];
-            mPendingCommands = nil;
-        }
 
         mMapTracker = nullptr;
         mDynamicUploader = nullptr;

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<MTLCommandBuffer> 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");
 

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*>(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);