From 0055d95fa7c03751a3b896582772f4ff7ee1c3a5 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Mon, 16 Nov 2020 23:07:56 +0000 Subject: [PATCH] Metal: Wrap NS classes and protocols in NSRef. This makes refcounting of these objects more automatic to try and prevent leaks or use-after-frees in the future. Also removes operator* from RefBase (and Ref) because it is never used and cannot work in a normal way for ObjectiveC protocols that cannot be dereferenced. Bug: dawn:89 Change-Id: I2e3fbfd638e2ba76d8c563f30bc489a384152552 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/32161 Reviewed-by: Austin Eng Reviewed-by: Stephen White Commit-Queue: Corentin Wallez --- src/common/BUILD.gn | 1 + src/common/CMakeLists.txt | 1 + src/common/NSRef.h | 123 ++++++++++++++++++ src/common/RefBase.h | 13 +- src/common/RefCounted.h | 5 +- src/dawn_native/metal/BackendMTL.mm | 24 ++-- src/dawn_native/metal/BufferMTL.h | 3 +- src/dawn_native/metal/BufferMTL.mm | 16 +-- src/dawn_native/metal/CommandBufferMTL.mm | 104 +++++++-------- .../metal/CommandRecordingContext.h | 14 +- .../metal/CommandRecordingContext.mm | 83 ++++++------ src/dawn_native/metal/ComputePipelineMTL.h | 5 +- src/dawn_native/metal/ComputePipelineMTL.mm | 15 +-- src/dawn_native/metal/DeviceMTL.h | 12 +- src/dawn_native/metal/DeviceMTL.mm | 62 ++++----- src/dawn_native/metal/QuerySetMTL.h | 8 +- src/dawn_native/metal/QuerySetMTL.mm | 32 ++--- src/dawn_native/metal/RenderPipelineMTL.h | 7 +- src/dawn_native/metal/RenderPipelineMTL.mm | 63 +++++---- src/dawn_native/metal/SamplerMTL.h | 5 +- src/dawn_native/metal/SamplerMTL.mm | 14 +- src/dawn_native/metal/ShaderModuleMTL.h | 10 +- src/dawn_native/metal/ShaderModuleMTL.mm | 22 ++-- src/dawn_native/metal/StagingBufferMTL.h | 5 +- src/dawn_native/metal/StagingBufferMTL.mm | 16 +-- src/dawn_native/metal/SwapChainMTL.h | 6 +- src/dawn_native/metal/SwapChainMTL.mm | 32 ++--- src/dawn_native/metal/TextureMTL.h | 14 +- src/dawn_native/metal/TextureMTL.mm | 91 ++++++------- src/tests/unittests/RefCountedTests.cpp | 2 - 30 files changed, 462 insertions(+), 346 deletions(-) create mode 100644 src/common/NSRef.h diff --git a/src/common/BUILD.gn b/src/common/BUILD.gn index 11bb461dae..1e318012d1 100644 --- a/src/common/BUILD.gn +++ b/src/common/BUILD.gn @@ -167,6 +167,7 @@ if (is_win || is_linux || is_chromeos || is_mac || is_fuchsia || is_android) { "Log.h", "Math.cpp", "Math.h", + "NSRef.h", "PlacementAllocated.h", "Platform.h", "RefBase.h", diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 63a5c24df2..8ea48e0bdf 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -29,6 +29,7 @@ target_sources(dawn_common PRIVATE "Log.h" "Math.cpp" "Math.h" + "NSRef.h" "PlacementAllocated.h" "Platform.h" "RefBase.h" diff --git a/src/common/NSRef.h b/src/common/NSRef.h new file mode 100644 index 0000000000..6423856e52 --- /dev/null +++ b/src/common/NSRef.h @@ -0,0 +1,123 @@ +// 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 COMMON_NSREF_H_ +#define COMMON_NSREF_H_ + +#include "common/RefBase.h" + +#import + +#if !defined(__OBJC__) +# error "NSRef can only be used in Objective C/C++ code." +#endif + +// This file contains smart pointers that automatically reference and release Objective C objects +// and prototocals in a manner very similar to Ref<>. Note that NSRef<> and NSPRef's constructor add +// a reference to the object by default, so the pattern to get a reference for a newly created +// NSObject is the following: +// +// NSRef foo = AcquireNSRef([NSFoo alloc]); +// +// NSRef overloads -> and * but these operators don't work extremely well with Objective C's +// features. For example automatic dereferencing when doing the following doesn't work: +// +// NSFoo* foo; +// foo.member = 1; +// someVar = foo.member; +// +// Instead use the message passing syntax: +// +// NSRef foo; +// [*foo setMember: 1]; +// someVar = [*foo member]; +// +// Also did you notive the extra '*' in the example above? That's because Objective C's message +// passing doesn't automatically call a C++ operator to dereference smart pointers (like -> does) so +// we have to dereference manually using '*'. In some cases the extra * or message passing syntax +// can get a bit annoying so instead a local "naked" pointer can be borrowed from the NSRef. This +// would change the syntax overload in the following: +// +// NSRef foo; +// [*foo setA:1]; +// [*foo setB:2]; +// [*foo setC:3]; +// +// Into (note access to members of ObjC classes referenced via pointer is done with . and not ->): +// +// NSRef fooRef; +// NSFoo* foo = fooRef.Get(); +// foo.a = 1; +// foo.b = 2; +// boo.c = 3; +// +// Which can be subjectively easier to read. + +template +struct NSRefTraits { + static constexpr T kNullValue = nullptr; + static void Reference(T value) { + [value retain]; + } + static void Release(T value) { + [value release]; + } +}; + +template +class NSRef : public RefBase> { + public: + using RefBase>::RefBase; + + const T* operator*() const { + return this->Get(); + } + + T* operator*() { + return this->Get(); + } +}; + +template +NSRef AcquireNSRef(T* pointee) { + NSRef ref; + ref.Acquire(pointee); + return ref; +} + +// This is a RefBase<> for an Objective C protocol (hence the P). Objective C protocols must always +// be referenced with id and not just ProtocolName* so they cannot use NSRef<> +// itself. That's what the P in NSPRef stands for: Protocol. +template +class NSPRef : public RefBase> { + public: + using RefBase>::RefBase; + + const T operator*() const { + return this->Get(); + } + + T operator*() { + return this->Get(); + } +}; + +template +NSPRef AcquireNSPRef(T pointee) { + NSPRef ref; + ref.Acquire(pointee); + return ref; +} + +#endif // COMMON_NSREF_H_ diff --git a/src/common/RefBase.h b/src/common/RefBase.h index 1f4705466d..508b424269 100644 --- a/src/common/RefBase.h +++ b/src/common/RefBase.h @@ -137,14 +137,6 @@ class RefBase { return mValue != kNullValue; } - // Operator * and -> to act like a pointer. - const typename Traits::PointedType& operator*() const { - return *mValue; - } - typename Traits::PointedType& operator*() { - return *mValue; - } - const T operator->() const { return mValue; } @@ -166,6 +158,11 @@ class RefBase { return value; } + void Acquire(T value) { + Release(); + mValue = value; + } + private: // Friend is needed so that instances of RefBase can call Reference and Release on // RefBase. diff --git a/src/common/RefCounted.h b/src/common/RefCounted.h index 5b36ca8e1a..9328a4c5c4 100644 --- a/src/common/RefCounted.h +++ b/src/common/RefCounted.h @@ -42,7 +42,6 @@ class RefCounted { template struct RefCountedTraits { - using PointedType = T; static constexpr T* kNullValue = nullptr; static void Reference(T* value) { value->Reference(); @@ -60,8 +59,8 @@ class Ref : public RefBase> { template Ref AcquireRef(T* pointee) { - Ref ref(pointee); - ref->Release(); + Ref ref; + ref.Acquire(pointee); return ref; } diff --git a/src/dawn_native/metal/BackendMTL.mm b/src/dawn_native/metal/BackendMTL.mm index f710ecb65f..f7b17423f9 100644 --- a/src/dawn_native/metal/BackendMTL.mm +++ b/src/dawn_native/metal/BackendMTL.mm @@ -15,6 +15,7 @@ #include "dawn_native/metal/BackendMTL.h" #include "common/GPUInfo.h" +#include "common/NSRef.h" #include "common/Platform.h" #include "dawn_native/Instance.h" #include "dawn_native/MetalBackend.h" @@ -176,8 +177,8 @@ namespace dawn_native { namespace metal { class Adapter : public AdapterBase { public: Adapter(InstanceBase* instance, id device) - : AdapterBase(instance, wgpu::BackendType::Metal), mDevice([device retain]) { - mPCIInfo.name = std::string([mDevice.name UTF8String]); + : AdapterBase(instance, wgpu::BackendType::Metal), mDevice(device) { + mPCIInfo.name = std::string([[*mDevice name] UTF8String]); PCIIDs ids; if (!instance->ConsumedError(GetDevicePCIInfo(device, &ids))) { @@ -206,10 +207,6 @@ namespace dawn_native { namespace metal { InitializeSupportedExtensions(); } - ~Adapter() override { - [mDevice release]; - } - private: ResultOrError CreateDeviceImpl(const DeviceDescriptor* descriptor) override { return Device::Create(this, mDevice, descriptor); @@ -217,14 +214,14 @@ namespace dawn_native { namespace metal { void InitializeSupportedExtensions() { #if defined(DAWN_PLATFORM_MACOS) - if ([mDevice supportsFeatureSet:MTLFeatureSet_macOS_GPUFamily1_v1]) { + if ([*mDevice supportsFeatureSet:MTLFeatureSet_macOS_GPUFamily1_v1]) { mSupportedExtensions.EnableExtension(Extension::TextureCompressionBC); } #endif if (@available(macOS 10.15, iOS 14.0, *)) { - if ([mDevice supportsFamily:MTLGPUFamilyMac2] || - [mDevice supportsFamily:MTLGPUFamilyApple5]) { + if ([*mDevice supportsFamily:MTLGPUFamilyMac2] || + [*mDevice supportsFamily:MTLGPUFamilyApple5]) { mSupportedExtensions.EnableExtension(Extension::PipelineStatisticsQuery); mSupportedExtensions.EnableExtension(Extension::TimestampQuery); } @@ -233,7 +230,7 @@ namespace dawn_native { namespace metal { mSupportedExtensions.EnableExtension(Extension::ShaderFloat16); } - id mDevice = nil; + NSPRef> mDevice; }; // Implementation of the Metal backend's BackendConnection @@ -251,13 +248,12 @@ namespace dawn_native { namespace metal { #if defined(DAWN_PLATFORM_MACOS) if (@available(macOS 10.11, *)) { supportedVersion = YES; - NSArray>* devices = MTLCopyAllDevices(); - for (id device in devices) { + NSRef>> devices = AcquireNSRef(MTLCopyAllDevices()); + + for (id device in devices.Get()) { adapters.push_back(std::make_unique(GetInstance(), device)); } - - [devices release]; } #endif diff --git a/src/dawn_native/metal/BufferMTL.h b/src/dawn_native/metal/BufferMTL.h index db06e1db83..409b89b5ab 100644 --- a/src/dawn_native/metal/BufferMTL.h +++ b/src/dawn_native/metal/BufferMTL.h @@ -15,6 +15,7 @@ #ifndef DAWNNATIVE_METAL_BUFFERMTL_H_ #define DAWNNATIVE_METAL_BUFFERMTL_H_ +#include "common/NSRef.h" #include "common/SerialQueue.h" #include "dawn_native/Buffer.h" @@ -53,7 +54,7 @@ namespace dawn_native { namespace metal { void InitializeToZero(CommandRecordingContext* commandContext); void ClearBuffer(CommandRecordingContext* commandContext, uint8_t clearValue); - id mMtlBuffer = nil; + NSPRef> mMtlBuffer; }; }} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/BufferMTL.mm b/src/dawn_native/metal/BufferMTL.mm index 14f6c42add..568430d446 100644 --- a/src/dawn_native/metal/BufferMTL.mm +++ b/src/dawn_native/metal/BufferMTL.mm @@ -84,9 +84,10 @@ namespace dawn_native { namespace metal { return DAWN_OUT_OF_MEMORY_ERROR("Buffer allocation is too large"); } - mMtlBuffer = [ToBackend(GetDevice())->GetMTLDevice() newBufferWithLength:currentSize - options:storageMode]; - if (mMtlBuffer == nil) { + mMtlBuffer.Acquire([ToBackend(GetDevice())->GetMTLDevice() + newBufferWithLength:currentSize + options:storageMode]); + if (mMtlBuffer == nullptr) { return DAWN_OUT_OF_MEMORY_ERROR("Buffer allocation failed"); } @@ -107,7 +108,7 @@ namespace dawn_native { namespace metal { } id Buffer::GetMTLBuffer() const { - return mMtlBuffer; + return mMtlBuffer.Get(); } bool Buffer::IsCPUWritableAtCreation() const { @@ -128,7 +129,7 @@ namespace dawn_native { namespace metal { } void* Buffer::GetMappedPointerImpl() { - return [mMtlBuffer contents]; + return [*mMtlBuffer contents]; } void Buffer::UnmapImpl() { @@ -136,8 +137,7 @@ namespace dawn_native { namespace metal { } void Buffer::DestroyImpl() { - [mMtlBuffer release]; - mMtlBuffer = nil; + mMtlBuffer = nullptr; } void Buffer::EnsureDataInitialized(CommandRecordingContext* commandContext) { @@ -196,7 +196,7 @@ namespace dawn_native { namespace metal { return; } - [commandContext->EnsureBlit() fillBuffer:mMtlBuffer + [commandContext->EnsureBlit() fillBuffer:mMtlBuffer.Get() range:NSMakeRange(0, GetSize()) value:clearValue]; } diff --git a/src/dawn_native/metal/CommandBufferMTL.mm b/src/dawn_native/metal/CommandBufferMTL.mm index 4e4156b3b6..69782250e2 100644 --- a/src/dawn_native/metal/CommandBufferMTL.mm +++ b/src/dawn_native/metal/CommandBufferMTL.mm @@ -53,9 +53,12 @@ namespace dawn_native { namespace metal { } } - // Creates an autoreleased MTLRenderPassDescriptor matching desc - MTLRenderPassDescriptor* CreateMTLRenderPassDescriptor(BeginRenderPassCmd* renderPass) { - MTLRenderPassDescriptor* descriptor = [MTLRenderPassDescriptor renderPassDescriptor]; + NSRef CreateMTLRenderPassDescriptor( + BeginRenderPassCmd* renderPass) { + // Note that this creates a descriptor that's autoreleased so we don't use AcquireNSRef + NSRef descriptorRef = + [MTLRenderPassDescriptor renderPassDescriptor]; + MTLRenderPassDescriptor* descriptor = descriptorRef.Get(); for (ColorAttachmentIndex attachment : IterateBitSet(renderPass->attachmentState->GetColorAttachmentsMask())) { @@ -167,7 +170,7 @@ namespace dawn_native { namespace metal { } } - return descriptor; + return descriptorRef; } // Helper function for Toggle EmulateStoreAndMSAAResolve @@ -175,10 +178,13 @@ namespace dawn_native { namespace metal { CommandRecordingContext* commandContext, const MTLRenderPassDescriptor* mtlRenderPass, const std::array, kMaxColorAttachments>& resolveTextures) { - MTLRenderPassDescriptor* mtlRenderPassForResolve = + // Note that this creates a descriptor that's autoreleased so we don't use AcquireNSRef + NSRef mtlRenderPassForResolveRef = [MTLRenderPassDescriptor renderPassDescriptor]; + MTLRenderPassDescriptor* mtlRenderPassForResolve = mtlRenderPassForResolveRef.Get(); + for (uint32_t i = 0; i < kMaxColorAttachments; ++i) { - if (resolveTextures[i] == nil) { + if (resolveTextures[i] == nullptr) { continue; } @@ -199,11 +205,13 @@ namespace dawn_native { namespace metal { } // Helper functions for Toggle AlwaysResolveIntoZeroLevelAndLayer - id CreateResolveTextureForWorkaround(Device* device, - MTLPixelFormat mtlFormat, - uint32_t width, - uint32_t height) { - MTLTextureDescriptor* mtlDesc = [MTLTextureDescriptor new]; + NSPRef> CreateResolveTextureForWorkaround(Device* device, + MTLPixelFormat mtlFormat, + uint32_t width, + uint32_t height) { + NSRef mtlDescRef = AcquireNSRef([MTLTextureDescriptor new]); + MTLTextureDescriptor* mtlDesc = mtlDescRef.Get(); + mtlDesc.textureType = MTLTextureType2D; mtlDesc.usage = MTLTextureUsageRenderTarget; mtlDesc.pixelFormat = mtlFormat; @@ -214,10 +222,8 @@ namespace dawn_native { namespace metal { mtlDesc.arrayLength = 1; mtlDesc.storageMode = MTLStorageModePrivate; mtlDesc.sampleCount = 1; - id resolveTexture = - [device->GetMTLDevice() newTextureWithDescriptor:mtlDesc]; - [mtlDesc release]; - return resolveTexture; + + return AcquireNSPRef([device->GetMTLDevice() newTextureWithDescriptor:mtlDesc]); } void CopyIntoTrueResolveTarget(CommandRecordingContext* commandContext, @@ -350,11 +356,11 @@ namespace dawn_native { namespace metal { group->GetLayout()->GetBindingInfo(bindingIndex); bool hasVertStage = - bindingInfo.visibility & wgpu::ShaderStage::Vertex && render != nil; + bindingInfo.visibility & wgpu::ShaderStage::Vertex && render != nullptr; bool hasFragStage = - bindingInfo.visibility & wgpu::ShaderStage::Fragment && render != nil; + bindingInfo.visibility & wgpu::ShaderStage::Fragment && render != nullptr; bool hasComputeStage = - bindingInfo.visibility & wgpu::ShaderStage::Compute && compute != nil; + bindingInfo.visibility & wgpu::ShaderStage::Compute && compute != nullptr; uint32_t vertIndex = 0; uint32_t fragIndex = 0; @@ -461,12 +467,12 @@ namespace dawn_native { namespace metal { template void ApplyBindGroup(id encoder, Args&&... args) { - ApplyBindGroupImpl(encoder, nil, std::forward(args)...); + ApplyBindGroupImpl(encoder, nullptr, std::forward(args)...); } template void ApplyBindGroup(id encoder, Args&&... args) { - ApplyBindGroupImpl(nil, encoder, std::forward(args)...); + ApplyBindGroupImpl(nullptr, encoder, std::forward(args)...); } StorageBufferLengthTracker* mLengthTracker; @@ -578,8 +584,9 @@ namespace dawn_native { namespace metal { commandContext->EndBlit(); LazyClearRenderPassAttachments(cmd); - MTLRenderPassDescriptor* descriptor = CreateMTLRenderPassDescriptor(cmd); - DAWN_TRY(EncodeRenderPass(commandContext, descriptor, cmd->width, cmd->height)); + NSRef descriptor = CreateMTLRenderPassDescriptor(cmd); + DAWN_TRY(EncodeRenderPass(commandContext, descriptor.Get(), cmd->width, + cmd->height)); nextPassNumber++; break; @@ -792,9 +799,9 @@ namespace dawn_native { namespace metal { char* label = mCommands.NextData(cmd->length + 1); if (@available(macos 10.13, *)) { - NSString* mtlLabel = [[NSString alloc] initWithUTF8String:label]; - [commandContext->GetCommands() pushDebugGroup:mtlLabel]; - [mtlLabel release]; + NSRef mtlLabel = + AcquireNSRef([[NSString alloc] initWithUTF8String:label]); + [commandContext->GetCommands() pushDebugGroup:mtlLabel.Get()]; } break; @@ -876,10 +883,9 @@ namespace dawn_native { namespace metal { case Command::InsertDebugMarker: { InsertDebugMarkerCmd* cmd = mCommands.NextCommand(); char* label = mCommands.NextData(cmd->length + 1); - NSString* mtlLabel = [[NSString alloc] initWithUTF8String:label]; - - [encoder insertDebugSignpost:mtlLabel]; - [mtlLabel release]; + NSRef mtlLabel = + AcquireNSRef([[NSString alloc] initWithUTF8String:label]); + [encoder insertDebugSignpost:mtlLabel.Get()]; break; } @@ -893,10 +899,9 @@ namespace dawn_native { namespace metal { case Command::PushDebugGroup: { PushDebugGroupCmd* cmd = mCommands.NextCommand(); char* label = mCommands.NextData(cmd->length + 1); - NSString* mtlLabel = [[NSString alloc] initWithUTF8String:label]; - - [encoder pushDebugGroup:mtlLabel]; - [mtlLabel release]; + NSRef mtlLabel = + AcquireNSRef([[NSString alloc] initWithUTF8String:label]); + [encoder pushDebugGroup:mtlLabel.Get()]; break; } @@ -944,9 +949,9 @@ namespace dawn_native { namespace metal { // Use temporary resolve texture on the resolve targets with non-zero resolveLevel or // resolveSlice. bool useTemporaryResolveTexture = false; - std::array, kMaxColorAttachments> temporaryResolveTextures = {}; + std::array>, kMaxColorAttachments> temporaryResolveTextures = {}; for (uint32_t i = 0; i < kMaxColorAttachments; ++i) { - if (mtlRenderPass.colorAttachments[i].resolveTexture == nil) { + if (mtlRenderPass.colorAttachments[i].resolveTexture == nullptr) { continue; } @@ -963,7 +968,8 @@ namespace dawn_native { namespace metal { temporaryResolveTextures[i] = CreateResolveTextureForWorkaround(device, mtlFormat, width, height); - mtlRenderPass.colorAttachments[i].resolveTexture = temporaryResolveTextures[i]; + mtlRenderPass.colorAttachments[i].resolveTexture = + temporaryResolveTextures[i].Get(); mtlRenderPass.colorAttachments[i].resolveLevel = 0; mtlRenderPass.colorAttachments[i].resolveSlice = 0; useTemporaryResolveTexture = true; @@ -974,16 +980,14 @@ namespace dawn_native { namespace metal { if (useTemporaryResolveTexture) { DAWN_TRY(EncodeRenderPass(commandContext, mtlRenderPass, width, height)); for (uint32_t i = 0; i < kMaxColorAttachments; ++i) { - if (trueResolveTextures[i] == nil) { + if (trueResolveTextures[i] == nullptr) { continue; } - ASSERT(temporaryResolveTextures[i] != nil); + ASSERT(temporaryResolveTextures[i] != nullptr); CopyIntoTrueResolveTarget(commandContext, trueResolveTextures[i], trueResolveLevels[i], trueResolveSlices[i], - temporaryResolveTextures[i], width, height); - [temporaryResolveTextures[i] release]; - temporaryResolveTextures[i] = nil; + temporaryResolveTextures[i].Get(), width, height); } return {}; } @@ -1002,7 +1006,7 @@ namespace dawn_native { namespace metal { resolveTextures[i] = mtlRenderPass.colorAttachments[i].resolveTexture; mtlRenderPass.colorAttachments[i].storeAction = MTLStoreActionStore; - mtlRenderPass.colorAttachments[i].resolveTexture = nil; + mtlRenderPass.colorAttachments[i].resolveTexture = nullptr; } } @@ -1024,7 +1028,7 @@ namespace dawn_native { namespace metal { uint32_t height) { bool enableVertexPulling = GetDevice()->IsToggleEnabled(Toggle::MetalEnableVertexPulling); RenderPipeline* lastPipeline = nullptr; - id indexBuffer = nil; + id indexBuffer = nullptr; uint32_t indexBufferBaseOffset = 0; wgpu::IndexFormat indexBufferFormat = wgpu::IndexFormat::Undefined; StorageBufferLengthTracker storageBufferLengths = {}; @@ -1148,10 +1152,9 @@ namespace dawn_native { namespace metal { case Command::InsertDebugMarker: { InsertDebugMarkerCmd* cmd = iter->NextCommand(); char* label = iter->NextData(cmd->length + 1); - NSString* mtlLabel = [[NSString alloc] initWithUTF8String:label]; - - [encoder insertDebugSignpost:mtlLabel]; - [mtlLabel release]; + NSRef mtlLabel = + AcquireNSRef([[NSString alloc] initWithUTF8String:label]); + [encoder insertDebugSignpost:mtlLabel.Get()]; break; } @@ -1165,10 +1168,9 @@ namespace dawn_native { namespace metal { case Command::PushDebugGroup: { PushDebugGroupCmd* cmd = iter->NextCommand(); char* label = iter->NextData(cmd->length + 1); - NSString* mtlLabel = [[NSString alloc] initWithUTF8String:label]; - - [encoder pushDebugGroup:mtlLabel]; - [mtlLabel release]; + NSRef mtlLabel = + AcquireNSRef([[NSString alloc] initWithUTF8String:label]); + [encoder pushDebugGroup:mtlLabel.Get()]; break; } diff --git a/src/dawn_native/metal/CommandRecordingContext.h b/src/dawn_native/metal/CommandRecordingContext.h index 531681b4ba..a0047a831a 100644 --- a/src/dawn_native/metal/CommandRecordingContext.h +++ b/src/dawn_native/metal/CommandRecordingContext.h @@ -14,6 +14,8 @@ #ifndef DAWNNATIVE_METAL_COMMANDRECORDINGCONTEXT_H_ #define DAWNNATIVE_METAL_COMMANDRECORDINGCONTEXT_H_ +#include "common/NSRef.h" + #import namespace dawn_native { namespace metal { @@ -23,7 +25,7 @@ namespace dawn_native { namespace metal { class CommandRecordingContext { public: CommandRecordingContext(); - CommandRecordingContext(id commands); + CommandRecordingContext(NSPRef> commands); CommandRecordingContext(const CommandRecordingContext& rhs) = delete; CommandRecordingContext& operator=(const CommandRecordingContext& rhs) = delete; @@ -35,7 +37,7 @@ namespace dawn_native { namespace metal { id GetCommands(); - id AcquireCommands(); + NSPRef> AcquireCommands(); id EnsureBlit(); void EndBlit(); @@ -47,10 +49,10 @@ namespace dawn_native { namespace metal { void EndRender(); private: - id mCommands = nil; - id mBlit = nil; - id mCompute = nil; - id mRender = nil; + NSPRef> mCommands; + NSPRef> mBlit; + NSPRef> mCompute; + NSPRef> mRender; bool mInEncoder = false; }; diff --git a/src/dawn_native/metal/CommandRecordingContext.mm b/src/dawn_native/metal/CommandRecordingContext.mm index 971691a4b2..6f2aefa566 100644 --- a/src/dawn_native/metal/CommandRecordingContext.mm +++ b/src/dawn_native/metal/CommandRecordingContext.mm @@ -20,8 +20,8 @@ namespace dawn_native { namespace metal { CommandRecordingContext::CommandRecordingContext() = default; - CommandRecordingContext::CommandRecordingContext(id commands) - : mCommands(commands) { + CommandRecordingContext::CommandRecordingContext(NSPRef> commands) + : mCommands(std::move(commands)) { } CommandRecordingContext::CommandRecordingContext(CommandRecordingContext&& rhs) @@ -35,90 +35,87 @@ namespace dawn_native { namespace metal { CommandRecordingContext::~CommandRecordingContext() { // Commands must be acquired. - ASSERT(mCommands == nil); + ASSERT(mCommands == nullptr); } id CommandRecordingContext::GetCommands() { - return mCommands; + return mCommands.Get(); } - id CommandRecordingContext::AcquireCommands() { - if (mCommands == nil) { - return nil; + NSPRef> CommandRecordingContext::AcquireCommands() { + // A blit encoder can be left open from WriteBuffer, make sure we close it. + if (mCommands != nullptr) { + EndBlit(); } - // A blit encoder can be left open from WriteBuffer, make sure we close it. - EndBlit(); - ASSERT(!mInEncoder); - id commands = mCommands; - mCommands = nil; - return commands; + return std::move(mCommands); } id CommandRecordingContext::EnsureBlit() { - ASSERT(mCommands != nil); + ASSERT(mCommands != nullptr); - if (mBlit == nil) { + if (mBlit == nullptr) { ASSERT(!mInEncoder); mInEncoder = true; - // The autorelease pool may drain before the encoder is ended. Retain so it stays alive. - mBlit = [[mCommands blitCommandEncoder] retain]; + + // The encoder is created autoreleased. Retain it to avoid the autoreleasepool from + // draining from under us. + mBlit = [*mCommands blitCommandEncoder]; } - return mBlit; + return mBlit.Get(); } void CommandRecordingContext::EndBlit() { - ASSERT(mCommands != nil); + ASSERT(mCommands != nullptr); - if (mBlit != nil) { - [mBlit endEncoding]; - [mBlit release]; - mBlit = nil; + if (mBlit != nullptr) { + [*mBlit endEncoding]; + mBlit = nullptr; mInEncoder = false; } } id CommandRecordingContext::BeginCompute() { - ASSERT(mCommands != nil); - ASSERT(mCompute == nil); + ASSERT(mCommands != nullptr); + ASSERT(mCompute == nullptr); 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; + // The encoder is created autoreleased. Retain it to avoid the autoreleasepool from draining + // from under us. + mCompute = [*mCommands computeCommandEncoder]; + return mCompute.Get(); } void CommandRecordingContext::EndCompute() { - ASSERT(mCommands != nil); - ASSERT(mCompute != nil); + ASSERT(mCommands != nullptr); + ASSERT(mCompute != nullptr); - [mCompute endEncoding]; - [mCompute release]; - mCompute = nil; + [*mCompute endEncoding]; + mCompute = nullptr; mInEncoder = false; } id CommandRecordingContext::BeginRender( MTLRenderPassDescriptor* descriptor) { - ASSERT(mCommands != nil); - ASSERT(mRender == nil); + ASSERT(mCommands != nullptr); + ASSERT(mRender == nullptr); 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; + // The encoder is created autoreleased. Retain it to avoid the autoreleasepool from draining + // from under us. + mRender = [*mCommands renderCommandEncoderWithDescriptor:descriptor]; + return mRender.Get(); } void CommandRecordingContext::EndRender() { - ASSERT(mCommands != nil); - ASSERT(mRender != nil); + ASSERT(mCommands != nullptr); + ASSERT(mRender != nullptr); - [mRender endEncoding]; - [mRender release]; - mRender = nil; + [*mRender endEncoding]; + mRender = nullptr; mInEncoder = false; } diff --git a/src/dawn_native/metal/ComputePipelineMTL.h b/src/dawn_native/metal/ComputePipelineMTL.h index 7c2d34b818..f9e874d659 100644 --- a/src/dawn_native/metal/ComputePipelineMTL.h +++ b/src/dawn_native/metal/ComputePipelineMTL.h @@ -17,6 +17,8 @@ #include "dawn_native/ComputePipeline.h" +#include "common/NSRef.h" + #import namespace dawn_native { namespace metal { @@ -33,11 +35,10 @@ namespace dawn_native { namespace metal { bool RequiresStorageBufferLength() const; private: - ~ComputePipeline() override; using ComputePipelineBase::ComputePipelineBase; MaybeError Initialize(const ComputePipelineDescriptor* descriptor); - id mMtlComputePipelineState = nil; + NSPRef> mMtlComputePipelineState; MTLSize mLocalWorkgroupSize; bool mRequiresStorageBufferLength; }; diff --git a/src/dawn_native/metal/ComputePipelineMTL.mm b/src/dawn_native/metal/ComputePipelineMTL.mm index 6c965152a4..7de57bdf20 100644 --- a/src/dawn_native/metal/ComputePipelineMTL.mm +++ b/src/dawn_native/metal/ComputePipelineMTL.mm @@ -37,10 +37,11 @@ namespace dawn_native { namespace metal { DAWN_TRY(computeModule->CreateFunction(computeEntryPoint, SingleShaderStage::Compute, ToBackend(GetLayout()), &computeData)); - NSError* error = nil; - mMtlComputePipelineState = - [mtlDevice newComputePipelineStateWithFunction:computeData.function error:&error]; - if (error != nil) { + NSError* error = nullptr; + mMtlComputePipelineState.Acquire([mtlDevice + newComputePipelineStateWithFunction:computeData.function.Get() + error:&error]); + if (error != nullptr) { NSLog(@" error => %@", error); return DAWN_INTERNAL_ERROR("Error creating pipeline state"); } @@ -53,12 +54,8 @@ namespace dawn_native { namespace metal { return {}; } - ComputePipeline::~ComputePipeline() { - [mMtlComputePipelineState release]; - } - void ComputePipeline::Encode(id encoder) { - [encoder setComputePipelineState:mMtlComputePipelineState]; + [encoder setComputePipelineState:mMtlComputePipelineState.Get()]; } MTLSize ComputePipeline::GetLocalWorkGroupSize() const { diff --git a/src/dawn_native/metal/DeviceMTL.h b/src/dawn_native/metal/DeviceMTL.h index 2a00e90534..c582758de1 100644 --- a/src/dawn_native/metal/DeviceMTL.h +++ b/src/dawn_native/metal/DeviceMTL.h @@ -35,7 +35,7 @@ namespace dawn_native { namespace metal { class Device : public DeviceBase { public: static ResultOrError Create(AdapterBase* adapter, - id mtlDevice, + NSPRef> mtlDevice, const DeviceDescriptor* descriptor); ~Device() override; @@ -72,7 +72,9 @@ namespace dawn_native { namespace metal { uint64_t GetOptimalBufferToTextureCopyOffsetAlignment() const override; private: - Device(AdapterBase* adapter, id mtlDevice, const DeviceDescriptor* descriptor); + Device(AdapterBase* adapter, + NSPRef> mtlDevice, + const DeviceDescriptor* descriptor); ResultOrError CreateBindGroupImpl( const BindGroupDescriptor* descriptor) override; @@ -108,8 +110,8 @@ namespace dawn_native { namespace metal { MaybeError WaitForIdleForDestruction() override; ExecutionSerial CheckAndUpdateCompletedSerials() override; - id mMtlDevice = nil; - id mCommandQueue = nil; + NSPRef> mMtlDevice; + NSPRef> mCommandQueue; CommandRecordingContext mCommandContext; @@ -120,7 +122,7 @@ namespace dawn_native { namespace metal { // mLastSubmittedCommands will be accessed in a Metal schedule handler that can be fired on // a different thread so we guard access to it with a mutex. std::mutex mLastSubmittedCommandsMutex; - id mLastSubmittedCommands = nil; + NSPRef> mLastSubmittedCommands; }; }} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/DeviceMTL.mm b/src/dawn_native/metal/DeviceMTL.mm index a9a1d1eaf5..9bd4fea36b 100644 --- a/src/dawn_native/metal/DeviceMTL.mm +++ b/src/dawn_native/metal/DeviceMTL.mm @@ -44,18 +44,17 @@ namespace dawn_native { namespace metal { // static ResultOrError Device::Create(AdapterBase* adapter, - id mtlDevice, + NSPRef> mtlDevice, const DeviceDescriptor* descriptor) { - Ref device = AcquireRef(new Device(adapter, mtlDevice, descriptor)); + Ref device = AcquireRef(new Device(adapter, std::move(mtlDevice), descriptor)); DAWN_TRY(device->Initialize()); return device.Detach(); } Device::Device(AdapterBase* adapter, - id mtlDevice, + NSPRef> mtlDevice, const DeviceDescriptor* descriptor) - : DeviceBase(adapter, descriptor), mMtlDevice([mtlDevice retain]), mCompletedSerial(0) { - [mMtlDevice retain]; + : DeviceBase(adapter, descriptor), mMtlDevice(std::move(mtlDevice)), mCompletedSerial(0) { } Device::~Device() { @@ -69,7 +68,7 @@ namespace dawn_native { namespace metal { ForceSetToggle(Toggle::MetalEnableVertexPulling, false); } - mCommandQueue = [mMtlDevice newCommandQueue]; + mCommandQueue.Acquire([*mMtlDevice newCommandQueue]); return DeviceBase::Initialize(new Queue(this)); } @@ -80,18 +79,18 @@ namespace dawn_native { namespace metal { #if defined(DAWN_PLATFORM_MACOS) if (@available(macOS 10.12, *)) { haveStoreAndMSAAResolve = - [mMtlDevice supportsFeatureSet:MTLFeatureSet_macOS_GPUFamily1_v2]; + [*mMtlDevice supportsFeatureSet:MTLFeatureSet_macOS_GPUFamily1_v2]; } #elif defined(DAWN_PLATFORM_IOS) haveStoreAndMSAAResolve = - [mMtlDevice supportsFeatureSet:MTLFeatureSet_iOS_GPUFamily3_v2]; + [*mMtlDevice supportsFeatureSet:MTLFeatureSet_iOS_GPUFamily3_v2]; #endif // On tvOS, we would need MTLFeatureSet_tvOS_GPUFamily2_v1. SetToggle(Toggle::EmulateStoreAndMSAAResolve, !haveStoreAndMSAAResolve); bool haveSamplerCompare = true; #if defined(DAWN_PLATFORM_IOS) - haveSamplerCompare = [mMtlDevice supportsFeatureSet:MTLFeatureSet_iOS_GPUFamily3_v1]; + haveSamplerCompare = [*mMtlDevice supportsFeatureSet:MTLFeatureSet_iOS_GPUFamily3_v1]; #endif // TODO(crbug.com/dawn/342): Investigate emulation -- possibly expensive. SetToggle(Toggle::MetalDisableSamplerCompare, !haveSamplerCompare); @@ -99,7 +98,7 @@ namespace dawn_native { namespace metal { bool haveBaseVertexBaseInstance = true; #if defined(DAWN_PLATFORM_IOS) haveBaseVertexBaseInstance = - [mMtlDevice supportsFeatureSet:MTLFeatureSet_iOS_GPUFamily3_v1]; + [*mMtlDevice supportsFeatureSet:MTLFeatureSet_iOS_GPUFamily3_v1]; #endif // TODO(crbug.com/dawn/343): Investigate emulation. SetToggle(Toggle::DisableBaseVertex, !haveBaseVertexBaseInstance); @@ -187,7 +186,7 @@ namespace dawn_native { namespace metal { } MaybeError Device::TickImpl() { - if (mCommandContext.GetCommands() != nil) { + if (mCommandContext.GetCommands() != nullptr) { SubmitPendingCommandBuffer(); } @@ -195,33 +194,33 @@ namespace dawn_native { namespace metal { } id Device::GetMTLDevice() { - return mMtlDevice; + return mMtlDevice.Get(); } id Device::GetMTLQueue() { - return mCommandQueue; + return mCommandQueue.Get(); } CommandRecordingContext* Device::GetPendingCommandContext() { - if (mCommandContext.GetCommands() == nil) { + 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([[mCommandQueue commandBuffer] retain]); + mCommandContext = CommandRecordingContext([*mCommandQueue commandBuffer]); } return &mCommandContext; } void Device::SubmitPendingCommandBuffer() { - if (mCommandContext.GetCommands() == nil) { + if (mCommandContext.GetCommands() == nullptr) { return; } IncrementLastSubmittedCommandSerial(); // Acquire the pending command buffer, which is retained. It must be released later. - id pendingCommands = mCommandContext.AcquireCommands(); + NSPRef> pendingCommands = mCommandContext.AcquireCommands(); // Replace mLastSubmittedCommands with the mutex held so we avoid races between the // schedule handler and this code. @@ -230,12 +229,15 @@ namespace dawn_native { namespace metal { mLastSubmittedCommands = pendingCommands; } - [pendingCommands addScheduledHandler:^(id) { + // Make a local copy of the pointer to the commands because it's not clear how ObjC blocks + // handle types with copy / move constructors being referenced in the block.. + id pendingCommandsPointer = pendingCommands.Get(); + [*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 = nil; + if (this->mLastSubmittedCommands.Get() == pendingCommandsPointer) { + this->mLastSubmittedCommands = nullptr; } }]; @@ -243,7 +245,7 @@ namespace dawn_native { namespace metal { // mLastSubmittedSerial so it is captured by value. ExecutionSerial pendingSerial = GetLastSubmittedCommandSerial(); // this ObjC block runs on a different thread - [pendingCommands addCompletedHandler:^(id) { + [*pendingCommands addCompletedHandler:^(id) { TRACE_EVENT_ASYNC_END0(GetPlatform(), GPUWork, "DeviceMTL::SubmitPendingCommandBuffer", uint64_t(pendingSerial)); ASSERT(uint64_t(pendingSerial) > mCompletedSerial.load()); @@ -252,8 +254,7 @@ namespace dawn_native { namespace metal { TRACE_EVENT_ASYNC_BEGIN0(GetPlatform(), GPUWork, "DeviceMTL::SubmitPendingCommandBuffer", uint64_t(pendingSerial)); - [pendingCommands commit]; - [pendingCommands release]; + [*pendingCommands commit]; } ResultOrError> Device::CreateStagingBuffer(size_t size) { @@ -356,11 +357,12 @@ namespace dawn_native { namespace metal { void Device::WaitForCommandsToBeScheduled() { SubmitPendingCommandBuffer(); - [mLastSubmittedCommands waitUntilScheduled]; + [*mLastSubmittedCommands waitUntilScheduled]; } MaybeError Device::WaitForIdleForDestruction() { - [mCommandContext.AcquireCommands() release]; + // Forget all pending commands. + mCommandContext.AcquireCommands(); CheckPassedSerials(); // Wait for all commands to be finished so we can free resources @@ -375,13 +377,11 @@ namespace dawn_native { namespace metal { void Device::ShutDownImpl() { ASSERT(GetState() == State::Disconnected); - [mCommandContext.AcquireCommands() release]; + // Forget all pending commands. + mCommandContext.AcquireCommands(); - [mCommandQueue release]; - mCommandQueue = nil; - - [mMtlDevice release]; - mMtlDevice = nil; + mCommandQueue = nullptr; + mMtlDevice = nullptr; } uint32_t Device::GetOptimalBytesPerRowAlignment() const { diff --git a/src/dawn_native/metal/QuerySetMTL.h b/src/dawn_native/metal/QuerySetMTL.h index d1ac5df8f1..b0f84a6241 100644 --- a/src/dawn_native/metal/QuerySetMTL.h +++ b/src/dawn_native/metal/QuerySetMTL.h @@ -17,6 +17,8 @@ #include "dawn_native/QuerySet.h" +#include "common/NSRef.h" + #import namespace dawn_native { namespace metal { @@ -40,9 +42,11 @@ namespace dawn_native { namespace metal { // Dawn API void DestroyImpl() override; - id mVisibilityBuffer = nil; + NSPRef> mVisibilityBuffer; + // Note that mCounterSampleBuffer cannot be an NSRef because the API_AVAILABLE macros don't + // propagate nicely through templates. id mCounterSampleBuffer API_AVAILABLE(macos(10.15), - ios(14.0)) = nil; + ios(14.0)) = nullptr; }; }} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/QuerySetMTL.mm b/src/dawn_native/metal/QuerySetMTL.mm index 4e395e328e..472fdf011b 100644 --- a/src/dawn_native/metal/QuerySetMTL.mm +++ b/src/dawn_native/metal/QuerySetMTL.mm @@ -26,7 +26,9 @@ namespace dawn_native { namespace metal { Device* device, MTLCommonCounterSet counterSet, uint32_t count) API_AVAILABLE(macos(10.15), ios(14.0)) { - MTLCounterSampleBufferDescriptor* descriptor = [MTLCounterSampleBufferDescriptor new]; + NSRef descriptorRef = + AcquireNSRef([MTLCounterSampleBufferDescriptor new]); + MTLCounterSampleBufferDescriptor* descriptor = descriptorRef.Get(); // To determine which counters are available from a device, we need to iterate through // the counterSets property of a MTLDevice. Then configure which counters will be @@ -38,18 +40,19 @@ namespace dawn_native { namespace metal { break; } } - ASSERT(descriptor.counterSet != nil); + ASSERT(descriptor.counterSet != nullptr); + descriptor.sampleCount = count; descriptor.storageMode = MTLStorageModePrivate; if (device->IsToggleEnabled(Toggle::MetalUseSharedModeForCounterSampleBuffer)) { descriptor.storageMode = MTLStorageModeShared; } - NSError* error = nil; + NSError* error = nullptr; id counterSampleBuffer = [device->GetMTLDevice() newCounterSampleBufferWithDescriptor:descriptor error:&error]; - if (error != nil) { + if (error != nullptr) { const char* errorString = [error.localizedDescription UTF8String]; return DAWN_INTERNAL_ERROR(std::string("Error creating query set: ") + errorString); } @@ -73,9 +76,9 @@ namespace dawn_native { namespace metal { case wgpu::QueryType::Occlusion: { // Create buffer for writing 64-bit results. NSUInteger bufferSize = static_cast(GetQueryCount() * sizeof(uint64_t)); - mVisibilityBuffer = - [device->GetMTLDevice() newBufferWithLength:bufferSize - options:MTLResourceStorageModePrivate]; + mVisibilityBuffer = AcquireNSPRef([device->GetMTLDevice() + newBufferWithLength:bufferSize + options:MTLResourceStorageModePrivate]); break; } case wgpu::QueryType::PipelineStatistics: @@ -105,7 +108,7 @@ namespace dawn_native { namespace metal { } id QuerySet::GetVisibilityBuffer() const { - return mVisibilityBuffer; + return mVisibilityBuffer.Get(); } id QuerySet::GetCounterSampleBuffer() const @@ -118,16 +121,13 @@ namespace dawn_native { namespace metal { } void QuerySet::DestroyImpl() { - if (mVisibilityBuffer != nil) { - [mVisibilityBuffer release]; - mVisibilityBuffer = nil; - } + mVisibilityBuffer = nullptr; + // mCounterSampleBuffer isn't an NSRef because API_AVAILABLE doesn't work will with + // templates. if (@available(macOS 10.15, iOS 14.0, *)) { - if (mCounterSampleBuffer != nil) { - [mCounterSampleBuffer release]; - mCounterSampleBuffer = nil; - } + [mCounterSampleBuffer release]; + mCounterSampleBuffer = nullptr; } } diff --git a/src/dawn_native/metal/RenderPipelineMTL.h b/src/dawn_native/metal/RenderPipelineMTL.h index 80f3bba863..9b3372bcf0 100644 --- a/src/dawn_native/metal/RenderPipelineMTL.h +++ b/src/dawn_native/metal/RenderPipelineMTL.h @@ -17,6 +17,8 @@ #include "dawn_native/RenderPipeline.h" +#include "common/NSRef.h" + #import namespace dawn_native { namespace metal { @@ -43,7 +45,6 @@ namespace dawn_native { namespace metal { wgpu::ShaderStage GetStagesRequiringStorageBufferLength() const; private: - ~RenderPipeline() override; using RenderPipelineBase::RenderPipelineBase; MaybeError Initialize(const RenderPipelineDescriptor* descriptor); @@ -52,8 +53,8 @@ namespace dawn_native { namespace metal { MTLPrimitiveType mMtlPrimitiveTopology; MTLWinding mMtlFrontFace; MTLCullMode mMtlCullMode; - id mMtlRenderPipelineState = nil; - id mMtlDepthStencilState = nil; + NSPRef> mMtlRenderPipelineState; + NSPRef> mMtlDepthStencilState; ityp::array mMtlVertexBufferIndices; wgpu::ShaderStage mStagesRequiringStorageBufferLength = wgpu::ShaderStage::None; diff --git a/src/dawn_native/metal/RenderPipelineMTL.mm b/src/dawn_native/metal/RenderPipelineMTL.mm index 3735e8954a..1f690b4cfe 100644 --- a/src/dawn_native/metal/RenderPipelineMTL.mm +++ b/src/dawn_native/metal/RenderPipelineMTL.mm @@ -236,17 +236,23 @@ namespace dawn_native { namespace metal { } } - MTLDepthStencilDescriptor* MakeDepthStencilDesc( + NSRef MakeDepthStencilDesc( const DepthStencilStateDescriptor* descriptor) { - MTLDepthStencilDescriptor* mtlDepthStencilDescriptor = [MTLDepthStencilDescriptor new]; + NSRef mtlDepthStencilDescRef = + AcquireNSRef([MTLDepthStencilDescriptor new]); + MTLDepthStencilDescriptor* mtlDepthStencilDescriptor = mtlDepthStencilDescRef.Get(); mtlDepthStencilDescriptor.depthCompareFunction = ToMetalCompareFunction(descriptor->depthCompare); mtlDepthStencilDescriptor.depthWriteEnabled = descriptor->depthWriteEnabled; if (StencilTestEnabled(descriptor)) { - MTLStencilDescriptor* backFaceStencil = [MTLStencilDescriptor new]; - MTLStencilDescriptor* frontFaceStencil = [MTLStencilDescriptor new]; + NSRef backFaceStencilRef = + AcquireNSRef([MTLStencilDescriptor new]); + MTLStencilDescriptor* backFaceStencil = backFaceStencilRef.Get(); + NSRef frontFaceStencilRef = + AcquireNSRef([MTLStencilDescriptor new]); + MTLStencilDescriptor* frontFaceStencil = frontFaceStencilRef.Get(); backFaceStencil.stencilCompareFunction = ToMetalCompareFunction(descriptor->stencilBack.compare); @@ -272,12 +278,9 @@ namespace dawn_native { namespace metal { mtlDepthStencilDescriptor.backFaceStencil = backFaceStencil; mtlDepthStencilDescriptor.frontFaceStencil = frontFaceStencil; - - [backFaceStencil release]; - [frontFaceStencil release]; } - return mtlDepthStencilDescriptor; + return mtlDepthStencilDescRef; } MTLWinding MTLFrontFace(wgpu::FrontFace face) { @@ -317,20 +320,19 @@ namespace dawn_native { namespace metal { mMtlCullMode = ToMTLCullMode(GetCullMode()); auto mtlDevice = ToBackend(GetDevice())->GetMTLDevice(); - MTLRenderPipelineDescriptor* descriptorMTL = [MTLRenderPipelineDescriptor new]; + NSRef descriptorMTLRef = + AcquireNSRef([MTLRenderPipelineDescriptor new]); + MTLRenderPipelineDescriptor* descriptorMTL = descriptorMTLRef.Get(); // TODO: MakeVertexDesc should be const in the future, so we don't need to call it here when // vertex pulling is enabled - MTLVertexDescriptor* vertexDesc = MakeVertexDesc(); - descriptorMTL.vertexDescriptor = vertexDesc; - [vertexDesc release]; + NSRef vertexDesc = MakeVertexDesc(); + // Calling MakeVertexDesc first is important since it sets indices for packed bindings if (GetDevice()->IsToggleEnabled(Toggle::MetalEnableVertexPulling)) { - // Calling MakeVertexDesc first is important since it sets indices for packed bindings - MTLVertexDescriptor* emptyVertexDesc = [MTLVertexDescriptor new]; - descriptorMTL.vertexDescriptor = emptyVertexDesc; - [emptyVertexDesc release]; + vertexDesc = AcquireNSRef([MTLVertexDescriptor new]); } + descriptorMTL.vertexDescriptor = vertexDesc.Get(); ShaderModule* vertexModule = ToBackend(descriptor->vertexStage.module); const char* vertexEntryPoint = descriptor->vertexStage.entryPoint; @@ -339,7 +341,7 @@ namespace dawn_native { namespace metal { ToBackend(GetLayout()), &vertexData, 0xFFFFFFFF, this)); - descriptorMTL.vertexFunction = vertexData.function; + descriptorMTL.vertexFunction = vertexData.function.Get(); if (vertexData.needsStorageBufferLength) { mStagesRequiringStorageBufferLength |= wgpu::ShaderStage::Vertex; } @@ -351,7 +353,7 @@ namespace dawn_native { namespace metal { ToBackend(GetLayout()), &fragmentData, descriptor->sampleMask)); - descriptorMTL.fragmentFunction = fragmentData.function; + descriptorMTL.fragmentFunction = fragmentData.function.Get(); if (fragmentData.needsStorageBufferLength) { mStagesRequiringStorageBufferLength |= wgpu::ShaderStage::Fragment; } @@ -384,11 +386,11 @@ namespace dawn_native { namespace metal { descriptorMTL.alphaToCoverageEnabled = descriptor->alphaToCoverageEnabled; { - NSError* error = nil; - mMtlRenderPipelineState = [mtlDevice newRenderPipelineStateWithDescriptor:descriptorMTL - error:&error]; - [descriptorMTL release]; - if (error != nil) { + NSError* error = nullptr; + mMtlRenderPipelineState = + AcquireNSPRef([mtlDevice newRenderPipelineStateWithDescriptor:descriptorMTL + error:&error]); + if (error != nullptr) { NSLog(@" error => %@", error); return DAWN_INTERNAL_ERROR("Error creating rendering pipeline state"); } @@ -397,19 +399,14 @@ namespace dawn_native { namespace metal { // Create depth stencil state and cache it, fetch the cached depth stencil state when we // call setDepthStencilState() for a given render pipeline in CommandEncoder, in order to // improve performance. - MTLDepthStencilDescriptor* depthStencilDesc = + NSRef depthStencilDesc = MakeDepthStencilDesc(GetDepthStencilStateDescriptor()); - mMtlDepthStencilState = [mtlDevice newDepthStencilStateWithDescriptor:depthStencilDesc]; - [depthStencilDesc release]; + mMtlDepthStencilState = + AcquireNSPRef([mtlDevice newDepthStencilStateWithDescriptor:depthStencilDesc.Get()]); return {}; } - RenderPipeline::~RenderPipeline() { - [mMtlRenderPipelineState release]; - [mMtlDepthStencilState release]; - } - MTLPrimitiveType RenderPipeline::GetMTLPrimitiveTopology() const { return mMtlPrimitiveTopology; } @@ -423,11 +420,11 @@ namespace dawn_native { namespace metal { } void RenderPipeline::Encode(id encoder) { - [encoder setRenderPipelineState:mMtlRenderPipelineState]; + [encoder setRenderPipelineState:mMtlRenderPipelineState.Get()]; } id RenderPipeline::GetMTLDepthStencilState() { - return mMtlDepthStencilState; + return mMtlDepthStencilState.Get(); } uint32_t RenderPipeline::GetMtlVertexBufferIndex(VertexBufferSlot slot) const { diff --git a/src/dawn_native/metal/SamplerMTL.h b/src/dawn_native/metal/SamplerMTL.h index 31c4ce5641..535a0c7e43 100644 --- a/src/dawn_native/metal/SamplerMTL.h +++ b/src/dawn_native/metal/SamplerMTL.h @@ -17,6 +17,8 @@ #include "dawn_native/Sampler.h" +#include "common/NSRef.h" + #import namespace dawn_native { namespace metal { @@ -31,9 +33,8 @@ namespace dawn_native { namespace metal { private: Sampler(Device* device, const SamplerDescriptor* descriptor); - ~Sampler() override; - id mMtlSamplerState = nil; + NSPRef> mMtlSamplerState; }; }} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/SamplerMTL.mm b/src/dawn_native/metal/SamplerMTL.mm index eabc73527a..0559605e0b 100644 --- a/src/dawn_native/metal/SamplerMTL.mm +++ b/src/dawn_native/metal/SamplerMTL.mm @@ -62,7 +62,8 @@ namespace dawn_native { namespace metal { Sampler::Sampler(Device* device, const SamplerDescriptor* descriptor) : SamplerBase(device, descriptor) { - MTLSamplerDescriptor* mtlDesc = [MTLSamplerDescriptor new]; + NSRef mtlDescRef = AcquireNSRef([MTLSamplerDescriptor new]); + MTLSamplerDescriptor* mtlDesc = mtlDescRef.Get(); mtlDesc.minFilter = FilterModeToMinMagFilter(descriptor->minFilter); mtlDesc.magFilter = FilterModeToMinMagFilter(descriptor->magFilter); @@ -83,17 +84,12 @@ namespace dawn_native { namespace metal { // Metal debug device errors. } - mMtlSamplerState = [device->GetMTLDevice() newSamplerStateWithDescriptor:mtlDesc]; - - [mtlDesc release]; - } - - Sampler::~Sampler() { - [mMtlSamplerState release]; + mMtlSamplerState = + AcquireNSPRef([device->GetMTLDevice() newSamplerStateWithDescriptor:mtlDesc]); } id Sampler::GetMTLSamplerState() { - return mMtlSamplerState; + return mMtlSamplerState.Get(); } }} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/ShaderModuleMTL.h b/src/dawn_native/metal/ShaderModuleMTL.h index 4e543c7c0f..6ecf57d000 100644 --- a/src/dawn_native/metal/ShaderModuleMTL.h +++ b/src/dawn_native/metal/ShaderModuleMTL.h @@ -17,10 +17,11 @@ #include "dawn_native/ShaderModule.h" -#import - +#include "common/NSRef.h" #include "dawn_native/Error.h" +#import + namespace spirv_cross { class CompilerMSL; } @@ -37,11 +38,8 @@ namespace dawn_native { namespace metal { const ShaderModuleDescriptor* descriptor); struct MetalFunctionData { - id function = nil; + NSPRef> function; bool needsStorageBufferLength; - ~MetalFunctionData() { - [function release]; - } }; MaybeError CreateFunction(const char* entryPointName, SingleShaderStage stage, diff --git a/src/dawn_native/metal/ShaderModuleMTL.mm b/src/dawn_native/metal/ShaderModuleMTL.mm index 0dce6b10ab..5af363bc55 100644 --- a/src/dawn_native/metal/ShaderModuleMTL.mm +++ b/src/dawn_native/metal/ShaderModuleMTL.mm @@ -142,7 +142,6 @@ namespace dawn_native { namespace metal { { // SPIRV-Cross also supports re-ordering attributes but it seems to do the correct thing // by default. - NSString* mslSource; std::string msl = compiler.compile(); // Some entry point names are forbidden in MSL so SPIRV-Cross modifies them. Query the @@ -159,14 +158,17 @@ namespace dawn_native { namespace metal { #pragma clang diagnostic ignored "-Wall" #endif )" + msl; - mslSource = [[NSString alloc] initWithUTF8String:msl.c_str()]; + + NSRef mslSource = + AcquireNSRef([[NSString alloc] initWithUTF8String:msl.c_str()]); auto mtlDevice = ToBackend(GetDevice())->GetMTLDevice(); - NSError* error = nil; - id library = [mtlDevice newLibraryWithSource:mslSource - options:nil - error:&error]; - if (error != nil) { + NSError* error = nullptr; + NSPRef> library = + AcquireNSPRef([mtlDevice newLibraryWithSource:mslSource.Get() + options:nullptr + error:&error]); + if (error != nullptr) { if (error.code != MTLLibraryErrorCompileWarning) { const char* errorString = [error.localizedDescription UTF8String]; return DAWN_VALIDATION_ERROR(std::string("Unable to create library object: ") + @@ -174,9 +176,9 @@ namespace dawn_native { namespace metal { } } - NSString* name = [[NSString alloc] initWithUTF8String:modifiedEntryPointName.c_str()]; - out->function = [library newFunctionWithName:name]; - [library release]; + NSRef name = + AcquireNSRef([[NSString alloc] initWithUTF8String:modifiedEntryPointName.c_str()]); + out->function = AcquireNSPRef([*library newFunctionWithName:name.Get()]); } out->needsStorageBufferLength = compiler.needs_buffer_size_buffer(); diff --git a/src/dawn_native/metal/StagingBufferMTL.h b/src/dawn_native/metal/StagingBufferMTL.h index 7e9815b491..b2d6551cd5 100644 --- a/src/dawn_native/metal/StagingBufferMTL.h +++ b/src/dawn_native/metal/StagingBufferMTL.h @@ -17,6 +17,8 @@ #include "dawn_native/StagingBuffer.h" +#include "common/NSRef.h" + #import namespace dawn_native { namespace metal { @@ -26,7 +28,6 @@ namespace dawn_native { namespace metal { class StagingBuffer : public StagingBufferBase { public: StagingBuffer(size_t size, Device* device); - ~StagingBuffer() override; id GetBufferHandle() const; @@ -34,7 +35,7 @@ namespace dawn_native { namespace metal { private: Device* mDevice; - id mBuffer = nil; + NSPRef> mBuffer; }; }} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/StagingBufferMTL.mm b/src/dawn_native/metal/StagingBufferMTL.mm index 390f00cfcf..af06b3548d 100644 --- a/src/dawn_native/metal/StagingBufferMTL.mm +++ b/src/dawn_native/metal/StagingBufferMTL.mm @@ -23,14 +23,15 @@ namespace dawn_native { namespace metal { MaybeError StagingBuffer::Initialize() { const size_t bufferSize = GetSize(); - mBuffer = [mDevice->GetMTLDevice() newBufferWithLength:bufferSize - options:MTLResourceStorageModeShared]; + mBuffer = AcquireNSPRef([mDevice->GetMTLDevice() + newBufferWithLength:bufferSize + options:MTLResourceStorageModeShared]); - if (mBuffer == nil) { + if (mBuffer == nullptr) { return DAWN_OUT_OF_MEMORY_ERROR("Unable to allocate buffer."); } - mMappedPointer = [mBuffer contents]; + mMappedPointer = [*mBuffer contents]; if (mMappedPointer == nullptr) { return DAWN_INTERNAL_ERROR("Unable to map staging buffer."); } @@ -38,13 +39,8 @@ namespace dawn_native { namespace metal { return {}; } - StagingBuffer::~StagingBuffer() { - [mBuffer release]; - mBuffer = nil; - } - id StagingBuffer::GetBufferHandle() const { - return mBuffer; + return mBuffer.Get(); } }} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/SwapChainMTL.h b/src/dawn_native/metal/SwapChainMTL.h index 6f15c4b3cd..11fad51a4e 100644 --- a/src/dawn_native/metal/SwapChainMTL.h +++ b/src/dawn_native/metal/SwapChainMTL.h @@ -17,6 +17,8 @@ #include "dawn_native/SwapChain.h" +#include "common/NSRef.h" + @class CAMetalLayer; @protocol CAMetalDrawable; @@ -47,9 +49,9 @@ namespace dawn_native { namespace metal { using NewSwapChainBase::NewSwapChainBase; MaybeError Initialize(NewSwapChainBase* previousSwapChain); - CAMetalLayer* mLayer = nullptr; + NSRef mLayer; - id mCurrentDrawable = nil; + NSPRef> mCurrentDrawable; Ref mTexture; MaybeError PresentImpl() override; diff --git a/src/dawn_native/metal/SwapChainMTL.mm b/src/dawn_native/metal/SwapChainMTL.mm index 512cd879c5..d6d57a968f 100644 --- a/src/dawn_native/metal/SwapChainMTL.mm +++ b/src/dawn_native/metal/SwapChainMTL.mm @@ -92,15 +92,15 @@ namespace dawn_native { namespace metal { CGSize size = {}; size.width = GetWidth(); size.height = GetHeight(); - [mLayer setDrawableSize:size]; + [*mLayer setDrawableSize:size]; - [mLayer setFramebufferOnly:(GetUsage() == wgpu::TextureUsage::RenderAttachment)]; - [mLayer setDevice:ToBackend(GetDevice())->GetMTLDevice()]; - [mLayer setPixelFormat:MetalPixelFormat(GetFormat())]; + [*mLayer setFramebufferOnly:(GetUsage() == wgpu::TextureUsage::RenderAttachment)]; + [*mLayer setDevice:ToBackend(GetDevice())->GetMTLDevice()]; + [*mLayer setPixelFormat:MetalPixelFormat(GetFormat())]; #if defined(DAWN_PLATFORM_MACOS) if (@available(macos 10.13, *)) { - [mLayer setDisplaySyncEnabled:(GetPresentMode() != wgpu::PresentMode::Immediate)]; + [*mLayer setDisplaySyncEnabled:(GetPresentMode() != wgpu::PresentMode::Immediate)]; } #endif // defined(DAWN_PLATFORM_MACOS) @@ -110,40 +110,36 @@ namespace dawn_native { namespace metal { } MaybeError SwapChain::PresentImpl() { - ASSERT(mCurrentDrawable != nil); - [mCurrentDrawable present]; + ASSERT(mCurrentDrawable != nullptr); + [*mCurrentDrawable present]; mTexture->Destroy(); mTexture = nullptr; - [mCurrentDrawable release]; - mCurrentDrawable = nil; + mCurrentDrawable = nullptr; return {}; } ResultOrError SwapChain::GetCurrentTextureViewImpl() { - ASSERT(mCurrentDrawable == nil); - mCurrentDrawable = [mLayer nextDrawable]; - [mCurrentDrawable retain]; + ASSERT(mCurrentDrawable == nullptr); + mCurrentDrawable = [*mLayer nextDrawable]; TextureDescriptor textureDesc = GetSwapChainBaseTextureDescriptor(this); - // mTexture will add a reference to mCurrentDrawable.texture to keep it alive. - mTexture = - AcquireRef(new Texture(ToBackend(GetDevice()), &textureDesc, mCurrentDrawable.texture)); + mTexture = AcquireRef( + new Texture(ToBackend(GetDevice()), &textureDesc, [*mCurrentDrawable texture])); return mTexture->CreateView(nullptr); } void SwapChain::DetachFromSurfaceImpl() { - ASSERT((mTexture.Get() == nullptr) == (mCurrentDrawable == nil)); + ASSERT((mTexture.Get() == nullptr) == (mCurrentDrawable == nullptr)); if (mTexture.Get() != nullptr) { mTexture->Destroy(); mTexture = nullptr; - [mCurrentDrawable release]; - mCurrentDrawable = nil; + mCurrentDrawable = nullptr; } } diff --git a/src/dawn_native/metal/TextureMTL.h b/src/dawn_native/metal/TextureMTL.h index 7ace3963a0..d578446c92 100644 --- a/src/dawn_native/metal/TextureMTL.h +++ b/src/dawn_native/metal/TextureMTL.h @@ -17,9 +17,11 @@ #include "dawn_native/Texture.h" +#include "common/NSRef.h" +#include "dawn_native/DawnNative.h" + #include #import -#include "dawn_native/DawnNative.h" namespace dawn_native { namespace metal { @@ -34,7 +36,9 @@ namespace dawn_native { namespace metal { class Texture final : public TextureBase { public: Texture(Device* device, const TextureDescriptor* descriptor); - Texture(Device* device, const TextureDescriptor* descriptor, id mtlTexture); + Texture(Device* device, + const TextureDescriptor* descriptor, + NSPRef> mtlTexture); Texture(Device* device, const ExternalImageDescriptor* descriptor, IOSurfaceRef ioSurface, @@ -51,7 +55,7 @@ namespace dawn_native { namespace metal { MaybeError ClearTexture(const SubresourceRange& range, TextureBase::ClearValue clearValue); - id mMtlTexture = nil; + NSPRef> mMtlTexture; }; class TextureView final : public TextureViewBase { @@ -61,9 +65,7 @@ namespace dawn_native { namespace metal { id GetMTLTexture(); private: - ~TextureView() override; - - id mMtlTextureView = nil; + NSPRef> mMtlTextureView; }; }} // namespace dawn_native::metal diff --git a/src/dawn_native/metal/TextureMTL.mm b/src/dawn_native/metal/TextureMTL.mm index e2423d90bb..a124bc4c1a 100644 --- a/src/dawn_native/metal/TextureMTL.mm +++ b/src/dawn_native/metal/TextureMTL.mm @@ -298,9 +298,10 @@ namespace dawn_native { namespace metal { return {}; } - MTLTextureDescriptor* CreateMetalTextureDescriptor(DeviceBase* device, - const TextureDescriptor* descriptor) { - MTLTextureDescriptor* mtlDesc = [MTLTextureDescriptor new]; + NSRef CreateMetalTextureDescriptor(DeviceBase* device, + const TextureDescriptor* descriptor) { + NSRef mtlDescRef = AcquireNSRef([MTLTextureDescriptor new]); + MTLTextureDescriptor* mtlDesc = mtlDescRef.Get(); mtlDesc.width = descriptor->size.width; mtlDesc.height = descriptor->size.height; @@ -338,14 +339,14 @@ namespace dawn_native { namespace metal { UNREACHABLE(); } - return mtlDesc; + return mtlDescRef; } Texture::Texture(Device* device, const TextureDescriptor* descriptor) : TextureBase(device, descriptor, TextureState::OwnedInternal) { - MTLTextureDescriptor* mtlDesc = CreateMetalTextureDescriptor(device, descriptor); - mMtlTexture = [device->GetMTLDevice() newTextureWithDescriptor:mtlDesc]; - [mtlDesc release]; + NSRef mtlDesc = CreateMetalTextureDescriptor(device, descriptor); + mMtlTexture = + AcquireNSPRef([device->GetMTLDevice() newTextureWithDescriptor:mtlDesc.Get()]); if (device->IsToggleEnabled(Toggle::NonzeroClearResourcesOnCreationForTesting)) { device->ConsumedError( @@ -353,9 +354,11 @@ namespace dawn_native { namespace metal { } } - Texture::Texture(Device* device, const TextureDescriptor* descriptor, id mtlTexture) - : TextureBase(device, descriptor, TextureState::OwnedInternal), mMtlTexture(mtlTexture) { - [mMtlTexture retain]; + Texture::Texture(Device* device, + const TextureDescriptor* descriptor, + NSPRef> mtlTexture) + : TextureBase(device, descriptor, TextureState::OwnedInternal), + mMtlTexture(std::move(mtlTexture)) { } Texture::Texture(Device* device, @@ -365,13 +368,13 @@ namespace dawn_native { namespace metal { : TextureBase(device, reinterpret_cast(descriptor->cTextureDescriptor), TextureState::OwnedInternal) { - MTLTextureDescriptor* mtlDesc = CreateMetalTextureDescriptor( + NSRef mtlDesc = CreateMetalTextureDescriptor( device, reinterpret_cast(descriptor->cTextureDescriptor)); - mtlDesc.storageMode = kIOSurfaceStorageMode; - mMtlTexture = [device->GetMTLDevice() newTextureWithDescriptor:mtlDesc - iosurface:ioSurface - plane:plane]; - [mtlDesc release]; + [*mtlDesc setStorageMode:kIOSurfaceStorageMode]; + + mMtlTexture = AcquireNSPRef([device->GetMTLDevice() newTextureWithDescriptor:mtlDesc.Get() + iosurface:ioSurface + plane:plane]); SetIsSubresourceContentInitialized(descriptor->isInitialized, GetAllSubresources()); } @@ -381,14 +384,11 @@ namespace dawn_native { namespace metal { } void Texture::DestroyImpl() { - if (GetTextureState() == TextureState::OwnedInternal) { - [mMtlTexture release]; - mMtlTexture = nil; - } + mMtlTexture = nullptr; } id Texture::GetMTLTexture() { - return mMtlTexture; + return mMtlTexture.Get(); } MaybeError Texture::ClearTexture(const SubresourceRange& range, @@ -419,8 +419,11 @@ namespace dawn_native { namespace metal { continue; } - MTLRenderPassDescriptor* descriptor = + // Note that this creates a descriptor that's autoreleased so we don't use + // AcquireNSRef + NSRef descriptorRef = [MTLRenderPassDescriptor renderPassDescriptor]; + MTLRenderPassDescriptor* descriptor = descriptorRef.Get(); // At least one aspect needs clearing. Iterate the aspects individually to // determine which to clear. @@ -462,7 +465,7 @@ namespace dawn_native { namespace metal { // Create multiple render passes with each subresource as a color attachment to // clear them all. Only do this for array layers to ensure all attachments have // the same size. - MTLRenderPassDescriptor* descriptor = nil; + NSRef descriptor; uint32_t attachment = 0; for (uint32_t arrayLayer = range.baseArrayLayer; @@ -474,30 +477,33 @@ namespace dawn_native { namespace metal { continue; } - if (descriptor == nil) { + if (descriptor == nullptr) { + // Note that this creates a descriptor that's autoreleased so we don't + // use AcquireNSRef descriptor = [MTLRenderPassDescriptor renderPassDescriptor]; } - descriptor.colorAttachments[attachment].texture = GetMTLTexture(); - descriptor.colorAttachments[attachment].loadAction = MTLLoadActionClear; - descriptor.colorAttachments[attachment].storeAction = MTLStoreActionStore; - descriptor.colorAttachments[attachment].clearColor = + [*descriptor colorAttachments][attachment].texture = GetMTLTexture(); + [*descriptor colorAttachments][attachment].loadAction = MTLLoadActionClear; + [*descriptor colorAttachments][attachment].storeAction = + MTLStoreActionStore; + [*descriptor colorAttachments][attachment].clearColor = MTLClearColorMake(dClearColor, dClearColor, dClearColor, dClearColor); - descriptor.colorAttachments[attachment].level = level; - descriptor.colorAttachments[attachment].slice = arrayLayer; + [*descriptor colorAttachments][attachment].level = level; + [*descriptor colorAttachments][attachment].slice = arrayLayer; attachment++; if (attachment == kMaxColorAttachments) { attachment = 0; - commandContext->BeginRender(descriptor); + commandContext->BeginRender(descriptor.Get()); commandContext->EndRender(); - descriptor = nil; + descriptor = nullptr; } } - if (descriptor != nil) { - commandContext->BeginRender(descriptor); + if (descriptor != nullptr) { + commandContext->BeginRender(descriptor.Get()); commandContext->EndRender(); } } @@ -591,9 +597,9 @@ namespace dawn_native { namespace metal { id mtlTexture = ToBackend(texture)->GetMTLTexture(); if (!UsageNeedsTextureView(texture->GetUsage())) { - mMtlTextureView = nil; + mMtlTextureView = nullptr; } else if (!RequiresCreatingNewTextureView(texture, descriptor)) { - mMtlTextureView = [mtlTexture retain]; + mMtlTextureView = mtlTexture; } else { MTLPixelFormat format = MetalPixelFormat(descriptor->format); if (descriptor->aspect == wgpu::TextureAspect::StencilOnly) { @@ -616,19 +622,16 @@ namespace dawn_native { namespace metal { auto arrayLayerRange = NSMakeRange(descriptor->baseArrayLayer, descriptor->arrayLayerCount); - mMtlTextureView = [mtlTexture newTextureViewWithPixelFormat:format + mMtlTextureView = + AcquireNSPRef([mtlTexture newTextureViewWithPixelFormat:format textureType:textureViewType levels:mipLevelRange - slices:arrayLayerRange]; + slices:arrayLayerRange]); } } - TextureView::~TextureView() { - [mMtlTextureView release]; - } - id TextureView::GetMTLTexture() { - ASSERT(mMtlTextureView != nil); - return mMtlTextureView; + ASSERT(mMtlTextureView != nullptr); + return mMtlTextureView.Get(); } }} // namespace dawn_native::metal diff --git a/src/tests/unittests/RefCountedTests.cpp b/src/tests/unittests/RefCountedTests.cpp index 3096d68485..5da311860f 100644 --- a/src/tests/unittests/RefCountedTests.cpp +++ b/src/tests/unittests/RefCountedTests.cpp @@ -118,7 +118,6 @@ TEST(Ref, Gets) { test->Release(); EXPECT_EQ(test.Get(), original); - EXPECT_EQ(&*test, original); EXPECT_EQ(test->GetThis(), original); } @@ -127,7 +126,6 @@ TEST(Ref, DefaultsToNull) { Ref test; EXPECT_EQ(test.Get(), nullptr); - EXPECT_EQ(&*test, nullptr); EXPECT_EQ(test->GetThis(), nullptr); }