diff --git a/src/common/NonCopyable.h b/src/common/NonCopyable.h index e711f7133a..61f15cabcf 100644 --- a/src/common/NonCopyable.h +++ b/src/common/NonCopyable.h @@ -15,10 +15,7 @@ #ifndef COMMON_NONCOPYABLE_H_ #define COMMON_NONCOPYABLE_H_ -// NonCopyable: -// the base class for the classes that are not copyable. -// - +// A base class to make a class non-copyable. class NonCopyable { protected: constexpr NonCopyable() = default; @@ -29,4 +26,15 @@ class NonCopyable { void operator=(const NonCopyable&) = delete; }; +// A base class to make a class non-movable. +class NonMovable : NonCopyable { + protected: + constexpr NonMovable() = default; + ~NonMovable() = default; + + private: + NonMovable(NonMovable&&) = delete; + void operator=(NonMovable&&) = delete; +}; + #endif diff --git a/src/dawn_native/metal/CommandRecordingContext.h b/src/dawn_native/metal/CommandRecordingContext.h index a0047a831a..5189a53e74 100644 --- a/src/dawn_native/metal/CommandRecordingContext.h +++ b/src/dawn_native/metal/CommandRecordingContext.h @@ -15,6 +15,8 @@ #define DAWNNATIVE_METAL_COMMANDRECORDINGCONTEXT_H_ #include "common/NSRef.h" +#include "common/NonCopyable.h" +#include "dawn_native/Error.h" #import @@ -22,21 +24,16 @@ 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 { + class CommandRecordingContext : NonMovable { public: CommandRecordingContext(); - CommandRecordingContext(NSPRef> commands); - - CommandRecordingContext(const CommandRecordingContext& rhs) = delete; - CommandRecordingContext& operator=(const CommandRecordingContext& rhs) = delete; - - CommandRecordingContext(CommandRecordingContext&& rhs); - CommandRecordingContext& operator=(CommandRecordingContext&& rhs); - ~CommandRecordingContext(); id GetCommands(); + void MarkUsed(); + bool WasUsed() const; + MaybeError PrepareNextCommandBuffer(id queue); NSPRef> AcquireCommands(); id EnsureBlit(); @@ -54,6 +51,7 @@ namespace dawn_native { namespace metal { NSPRef> mCompute; NSPRef> mRender; bool mInEncoder = false; + bool mUsed = false; }; }} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/CommandRecordingContext.mm b/src/dawn_native/metal/CommandRecordingContext.mm index decb650676..f07c48c7ae 100644 --- a/src/dawn_native/metal/CommandRecordingContext.mm +++ b/src/dawn_native/metal/CommandRecordingContext.mm @@ -20,19 +20,6 @@ namespace dawn_native { namespace metal { CommandRecordingContext::CommandRecordingContext() = default; - CommandRecordingContext::CommandRecordingContext(NSPRef> commands) - : mCommands(std::move(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 == nullptr); @@ -42,6 +29,28 @@ namespace dawn_native { namespace metal { return mCommands.Get(); } + void CommandRecordingContext::MarkUsed() { + mUsed = true; + } + bool CommandRecordingContext::WasUsed() const { + return mUsed; + } + + MaybeError CommandRecordingContext::PrepareNextCommandBuffer(id queue) { + ASSERT(mCommands == nil); + ASSERT(!mUsed); + + // The MTLCommandBuffer will be autoreleased by default. + // The autorelease pool may drain before the command buffer is submitted. Retain so it stays + // alive. + mCommands = AcquireNSPRef([[queue commandBuffer] retain]); + if (mCommands == nil) { + return DAWN_INTERNAL_ERROR("Failed to allocate an MTLCommandBuffer"); + } + + return {}; + } + NSPRef> CommandRecordingContext::AcquireCommands() { // A blit encoder can be left open from WriteBuffer, make sure we close it. if (mCommands != nullptr) { @@ -49,6 +58,7 @@ namespace dawn_native { namespace metal { } ASSERT(!mInEncoder); + mUsed = false; return std::move(mCommands); } diff --git a/src/dawn_native/metal/DeviceMTL.h b/src/dawn_native/metal/DeviceMTL.h index 442741809b..d1881e51e6 100644 --- a/src/dawn_native/metal/DeviceMTL.h +++ b/src/dawn_native/metal/DeviceMTL.h @@ -51,7 +51,7 @@ namespace dawn_native { namespace metal { id GetMTLQueue(); CommandRecordingContext* GetPendingCommandContext(); - void SubmitPendingCommandBuffer(); + MaybeError SubmitPendingCommandBuffer(); Ref CreateTextureWrappingIOSurface(const ExternalImageDescriptor* descriptor, IOSurfaceRef ioSurface, diff --git a/src/dawn_native/metal/DeviceMTL.mm b/src/dawn_native/metal/DeviceMTL.mm index 0359698a25..359dfda4d1 100644 --- a/src/dawn_native/metal/DeviceMTL.mm +++ b/src/dawn_native/metal/DeviceMTL.mm @@ -128,6 +128,11 @@ namespace dawn_native { namespace metal { InitTogglesFromDriver(); mCommandQueue.Acquire([*mMtlDevice newCommandQueue]); + if (mCommandQueue == nil) { + return DAWN_INTERNAL_ERROR("Failed to allocate MTLCommandQueue."); + } + + DAWN_TRY(mCommandContext.PrepareNextCommandBuffer(*mCommandQueue)); if (GetAdapter()->GetSupportedExtensions().IsEnabled(Extension::TimestampQuery)) { // Make a best guess of timestamp period based on device vendor info, and converge it to @@ -281,9 +286,7 @@ namespace dawn_native { namespace metal { } MaybeError Device::TickImpl() { - if (mCommandContext.GetCommands() != nullptr) { - SubmitPendingCommandBuffer(); - } + DAWN_TRY(SubmitPendingCommandBuffer()); // Just run timestamp period calculation when timestamp extension is enabled. if (IsExtensionEnabled(Extension::TimestampQuery)) { @@ -305,20 +308,13 @@ namespace dawn_native { namespace metal { } CommandRecordingContext* Device::GetPendingCommandContext() { - if (mCommandContext.GetCommands() == nullptr) { - 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(AcquireNSPRef([[*mCommandQueue commandBuffer] retain])); - } + mCommandContext.MarkUsed(); return &mCommandContext; } - void Device::SubmitPendingCommandBuffer() { - if (mCommandContext.GetCommands() == nullptr) { - return; + MaybeError Device::SubmitPendingCommandBuffer() { + if (!mCommandContext.WasUsed()) { + return {}; } IncrementLastSubmittedCommandSerial(); @@ -359,6 +355,8 @@ namespace dawn_native { namespace metal { TRACE_EVENT_ASYNC_BEGIN0(GetPlatform(), GPUWork, "DeviceMTL::SubmitPendingCommandBuffer", uint64_t(pendingSerial)); [*pendingCommands commit]; + + return mCommandContext.PrepareNextCommandBuffer(*mCommandQueue); } ResultOrError> Device::CreateStagingBuffer(size_t size) { @@ -432,7 +430,9 @@ namespace dawn_native { namespace metal { } void Device::WaitForCommandsToBeScheduled() { - SubmitPendingCommandBuffer(); + if (ConsumedError(SubmitPendingCommandBuffer())) { + return; + } // Only lock the object while we take a reference to it, otherwise we could block further // progress if the driver calls the scheduled handler (which also acquires the lock) before diff --git a/src/dawn_native/metal/QueueMTL.mm b/src/dawn_native/metal/QueueMTL.mm index bfa33ac2c1..ad1fad6f0e 100644 --- a/src/dawn_native/metal/QueueMTL.mm +++ b/src/dawn_native/metal/QueueMTL.mm @@ -42,8 +42,7 @@ namespace dawn_native { namespace metal { } TRACE_EVENT_END0(GetDevice()->GetPlatform(), Recording, "CommandBufferMTL::FillCommands"); - device->SubmitPendingCommandBuffer(); - return {}; + return device->SubmitPendingCommandBuffer(); } }} // namespace dawn_native::metal diff --git a/src/tests/white_box/MetalAutoreleasePoolTests.mm b/src/tests/white_box/MetalAutoreleasePoolTests.mm index 47632395ca..1202307e85 100644 --- a/src/tests/white_box/MetalAutoreleasePoolTests.mm +++ b/src/tests/white_box/MetalAutoreleasePoolTests.mm @@ -41,7 +41,7 @@ TEST_P(MetalAutoreleasePoolTests, CommandBufferOutlivesAutorelease) { } // Submitting the command buffer should succeed. - mMtlDevice->SubmitPendingCommandBuffer(); + ASSERT_TRUE(mMtlDevice->SubmitPendingCommandBuffer().IsSuccess()); } // Test that the MTLBlitCommandEncoder owned by the pending command context @@ -56,7 +56,7 @@ TEST_P(MetalAutoreleasePoolTests, EncoderOutlivesAutorelease) { // Submitting the command buffer should succeed. mMtlDevice->GetPendingCommandContext()->EndBlit(); - mMtlDevice->SubmitPendingCommandBuffer(); + ASSERT_TRUE(mMtlDevice->SubmitPendingCommandBuffer().IsSuccess()); } DAWN_INSTANTIATE_TEST(MetalAutoreleasePoolTests, MetalBackend());