Use Ref<TextureBase> instead of TextureBase* in more places
To avoid accidental memory leaks on account of using raw pointers,
use Ref<TextureBase> as method return type except at Dawn interface
boundaries.
Change-Id: I6459062ee28984de2cb1d5a2059bc70cf82b2faf
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/19580
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Rafael Cintron <rafael.cintron@microsoft.com>
diff --git a/src/common/Result.h b/src/common/Result.h
index 8509568..4cea7f2 100644
--- a/src/common/Result.h
+++ b/src/common/Result.h
@@ -177,11 +177,14 @@
static_assert(alignof_if_defined_else_default<E, 4> >= 4,
"Result<Ref<T>, E> reserves two bits for tagging pointers");
- Result(Ref<T>&& success);
+ template <typename U>
+ Result(Ref<U>&& success);
Result(std::unique_ptr<E> error);
- Result(Result<Ref<T>, E>&& other);
- Result<Ref<T>, E>& operator=(Result<Ref<T>, E>&& other);
+ template <typename U>
+ Result(Result<Ref<U>, E>&& other);
+ template <typename U>
+ Result<Ref<U>, E>& operator=(Result<Ref<U>, E>&& other);
~Result();
@@ -192,6 +195,9 @@
std::unique_ptr<E> AcquireError();
private:
+ template <typename T2, typename E2>
+ friend class Result;
+
intptr_t mPayload = detail::kEmptyPayload;
};
@@ -399,8 +405,10 @@
// Implementation of Result<Ref<T>, E>
template <typename T, typename E>
-Result<Ref<T>, E>::Result(Ref<T>&& success)
+template <typename U>
+Result<Ref<T>, E>::Result(Ref<U>&& success)
: mPayload(detail::MakePayload(success.Detach(), detail::Success)) {
+ static_assert(std::is_convertible<U*, T*>::value, "");
}
template <typename T, typename E>
@@ -409,12 +417,16 @@
}
template <typename T, typename E>
-Result<Ref<T>, E>::Result(Result<Ref<T>, E>&& other) : mPayload(other.mPayload) {
+template <typename U>
+Result<Ref<T>, E>::Result(Result<Ref<U>, E>&& other) : mPayload(other.mPayload) {
+ static_assert(std::is_convertible<U*, T*>::value, "");
other.mPayload = detail::kEmptyPayload;
}
template <typename T, typename E>
-Result<Ref<T>, E>& Result<Ref<T>, E>::operator=(Result<Ref<T>, E>&& other) {
+template <typename U>
+Result<Ref<U>, E>& Result<Ref<T>, E>::operator=(Result<Ref<U>, E>&& other) {
+ static_assert(std::is_convertible<U*, T*>::value, "");
ASSERT(mPayload == detail::kEmptyPayload);
mPayload = other.mPayload;
other.mPayload = detail::kEmptyPayload;
diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 41c7664..2fbf2c8 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -628,13 +628,13 @@
return result;
}
TextureBase* DeviceBase::CreateTexture(const TextureDescriptor* descriptor) {
- TextureBase* result = nullptr;
+ Ref<TextureBase> result;
- if (ConsumedError(CreateTextureInternal(&result, descriptor))) {
+ if (ConsumedError(CreateTextureInternal(descriptor), &result)) {
return TextureBase::MakeError(this);
}
- return result;
+ return result.Detach();
}
TextureViewBase* DeviceBase::CreateTextureView(TextureBase* texture,
const TextureViewDescriptor* descriptor) {
@@ -918,14 +918,13 @@
return {};
}
- MaybeError DeviceBase::CreateTextureInternal(TextureBase** result,
- const TextureDescriptor* descriptor) {
+ ResultOrError<Ref<TextureBase>> DeviceBase::CreateTextureInternal(
+ const TextureDescriptor* descriptor) {
DAWN_TRY(ValidateIsAlive());
if (IsValidationEnabled()) {
DAWN_TRY(ValidateTextureDescriptor(this, descriptor));
}
- DAWN_TRY_ASSIGN(*result, CreateTextureImpl(descriptor));
- return {};
+ return CreateTextureImpl(descriptor);
}
MaybeError DeviceBase::CreateTextureViewInternal(TextureViewBase** result,
diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h
index 8bce4ab..5187427 100644
--- a/src/dawn_native/Device.h
+++ b/src/dawn_native/Device.h
@@ -243,7 +243,7 @@
Surface* surface,
NewSwapChainBase* previousSwapChain,
const SwapChainDescriptor* descriptor) = 0;
- virtual ResultOrError<TextureBase*> CreateTextureImpl(
+ virtual ResultOrError<Ref<TextureBase>> CreateTextureImpl(
const TextureDescriptor* descriptor) = 0;
virtual ResultOrError<TextureViewBase*> CreateTextureViewImpl(
TextureBase* texture,
@@ -269,7 +269,7 @@
MaybeError CreateSwapChainInternal(SwapChainBase** result,
Surface* surface,
const SwapChainDescriptor* descriptor);
- MaybeError CreateTextureInternal(TextureBase** result, const TextureDescriptor* descriptor);
+ ResultOrError<Ref<TextureBase>> CreateTextureInternal(const TextureDescriptor* descriptor);
MaybeError CreateTextureViewInternal(TextureViewBase** result,
TextureBase* texture,
const TextureViewDescriptor* descriptor);
diff --git a/src/dawn_native/d3d12/D3D12Backend.cpp b/src/dawn_native/d3d12/D3D12Backend.cpp
index 3543c63..f78975e 100644
--- a/src/dawn_native/d3d12/D3D12Backend.cpp
+++ b/src/dawn_native/d3d12/D3D12Backend.cpp
@@ -63,10 +63,10 @@
WGPUTexture WrapSharedHandle(WGPUDevice device,
const ExternalImageDescriptorDXGISharedHandle* descriptor) {
Device* backendDevice = reinterpret_cast<Device*>(device);
- TextureBase* texture = backendDevice->WrapSharedHandle(descriptor, descriptor->sharedHandle,
- descriptor->acquireMutexKey,
- descriptor->isSwapChainTexture);
- return reinterpret_cast<WGPUTexture>(texture);
+ Ref<TextureBase> texture = backendDevice->WrapSharedHandle(
+ descriptor, descriptor->sharedHandle, descriptor->acquireMutexKey,
+ descriptor->isSwapChainTexture);
+ return reinterpret_cast<WGPUTexture>(texture.Detach());
}
}} // namespace dawn_native::d3d12
diff --git a/src/dawn_native/d3d12/DeviceD3D12.cpp b/src/dawn_native/d3d12/DeviceD3D12.cpp
index 41895b3..8f09bf9 100644
--- a/src/dawn_native/d3d12/DeviceD3D12.cpp
+++ b/src/dawn_native/d3d12/DeviceD3D12.cpp
@@ -292,7 +292,7 @@
const SwapChainDescriptor* descriptor) {
return DAWN_VALIDATION_ERROR("New swapchains not implemented.");
}
- ResultOrError<TextureBase*> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
+ ResultOrError<Ref<TextureBase>> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
return Texture::Create(this, descriptor);
}
ResultOrError<TextureViewBase*> Device::CreateTextureViewImpl(
@@ -339,11 +339,11 @@
initialUsage);
}
- TextureBase* Device::WrapSharedHandle(const ExternalImageDescriptor* descriptor,
- HANDLE sharedHandle,
- uint64_t acquireMutexKey,
- bool isSwapChainTexture) {
- TextureBase* dawnTexture;
+ Ref<TextureBase> Device::WrapSharedHandle(const ExternalImageDescriptor* descriptor,
+ HANDLE sharedHandle,
+ uint64_t acquireMutexKey,
+ bool isSwapChainTexture) {
+ Ref<TextureBase> dawnTexture;
if (ConsumedError(Texture::Create(this, descriptor, sharedHandle, acquireMutexKey,
isSwapChainTexture),
&dawnTexture))
diff --git a/src/dawn_native/d3d12/DeviceD3D12.h b/src/dawn_native/d3d12/DeviceD3D12.h
index f7298b5..615f1fb 100644
--- a/src/dawn_native/d3d12/DeviceD3D12.h
+++ b/src/dawn_native/d3d12/DeviceD3D12.h
@@ -113,10 +113,10 @@
StagingDescriptorAllocator* GetDepthStencilViewAllocator() const;
- TextureBase* WrapSharedHandle(const ExternalImageDescriptor* descriptor,
- HANDLE sharedHandle,
- uint64_t acquireMutexKey,
- bool isSwapChainTexture);
+ Ref<TextureBase> WrapSharedHandle(const ExternalImageDescriptor* descriptor,
+ HANDLE sharedHandle,
+ uint64_t acquireMutexKey,
+ bool isSwapChainTexture);
ResultOrError<ComPtr<IDXGIKeyedMutex>> CreateKeyedMutexForTexture(
ID3D12Resource* d3d12Resource);
void ReleaseKeyedMutexForTexture(ComPtr<IDXGIKeyedMutex> dxgiKeyedMutex);
@@ -146,7 +146,8 @@
Surface* surface,
NewSwapChainBase* previousSwapChain,
const SwapChainDescriptor* descriptor) override;
- ResultOrError<TextureBase*> CreateTextureImpl(const TextureDescriptor* descriptor) override;
+ ResultOrError<Ref<TextureBase>> CreateTextureImpl(
+ const TextureDescriptor* descriptor) override;
ResultOrError<TextureViewBase*> CreateTextureViewImpl(
TextureBase* texture,
const TextureViewDescriptor* descriptor) override;
diff --git a/src/dawn_native/d3d12/TextureD3D12.cpp b/src/dawn_native/d3d12/TextureD3D12.cpp
index bbcd4e1..a6043e7 100644
--- a/src/dawn_native/d3d12/TextureD3D12.cpp
+++ b/src/dawn_native/d3d12/TextureD3D12.cpp
@@ -273,19 +273,19 @@
return {};
}
- ResultOrError<TextureBase*> Texture::Create(Device* device,
- const TextureDescriptor* descriptor) {
+ ResultOrError<Ref<TextureBase>> Texture::Create(Device* device,
+ const TextureDescriptor* descriptor) {
Ref<Texture> dawnTexture =
AcquireRef(new Texture(device, descriptor, TextureState::OwnedInternal));
DAWN_TRY(dawnTexture->InitializeAsInternalTexture());
- return dawnTexture.Detach();
+ return std::move(dawnTexture);
}
- ResultOrError<TextureBase*> Texture::Create(Device* device,
- const ExternalImageDescriptor* descriptor,
- HANDLE sharedHandle,
- uint64_t acquireMutexKey,
- bool isSwapChainTexture) {
+ ResultOrError<Ref<TextureBase>> Texture::Create(Device* device,
+ const ExternalImageDescriptor* descriptor,
+ HANDLE sharedHandle,
+ uint64_t acquireMutexKey,
+ bool isSwapChainTexture) {
const TextureDescriptor* textureDescriptor =
reinterpret_cast<const TextureDescriptor*>(descriptor->cTextureDescriptor);
@@ -297,7 +297,7 @@
dawnTexture->SetIsSubresourceContentInitialized(descriptor->isCleared, 0,
textureDescriptor->mipLevelCount, 0,
textureDescriptor->arrayLayerCount);
- return dawnTexture.Detach();
+ return std::move(dawnTexture);
}
MaybeError Texture::InitializeAsExternalTexture(const TextureDescriptor* descriptor,
diff --git a/src/dawn_native/d3d12/TextureD3D12.h b/src/dawn_native/d3d12/TextureD3D12.h
index 2f4380d..88a24f4 100644
--- a/src/dawn_native/d3d12/TextureD3D12.h
+++ b/src/dawn_native/d3d12/TextureD3D12.h
@@ -34,13 +34,13 @@
class Texture final : public TextureBase {
public:
- static ResultOrError<TextureBase*> Create(Device* device,
- const TextureDescriptor* descriptor);
- static ResultOrError<TextureBase*> Create(Device* device,
- const ExternalImageDescriptor* descriptor,
- HANDLE sharedHandle,
- uint64_t acquireMutexKey,
- bool isSwapChainTexture);
+ static ResultOrError<Ref<TextureBase>> Create(Device* device,
+ const TextureDescriptor* descriptor);
+ static ResultOrError<Ref<TextureBase>> Create(Device* device,
+ const ExternalImageDescriptor* descriptor,
+ HANDLE sharedHandle,
+ uint64_t acquireMutexKey,
+ bool isSwapChainTexture);
Texture(Device* device,
const TextureDescriptor* descriptor,
ComPtr<ID3D12Resource> d3d12Texture);
diff --git a/src/dawn_native/metal/DeviceMTL.h b/src/dawn_native/metal/DeviceMTL.h
index 4a5195e..bc75591 100644
--- a/src/dawn_native/metal/DeviceMTL.h
+++ b/src/dawn_native/metal/DeviceMTL.h
@@ -94,7 +94,8 @@
Surface* surface,
NewSwapChainBase* previousSwapChain,
const SwapChainDescriptor* descriptor) override;
- ResultOrError<TextureBase*> CreateTextureImpl(const TextureDescriptor* descriptor) override;
+ ResultOrError<Ref<TextureBase>> CreateTextureImpl(
+ const TextureDescriptor* descriptor) override;
ResultOrError<TextureViewBase*> CreateTextureViewImpl(
TextureBase* texture,
const TextureViewDescriptor* descriptor) override;
diff --git a/src/dawn_native/metal/DeviceMTL.mm b/src/dawn_native/metal/DeviceMTL.mm
index 341c131..30521b8 100644
--- a/src/dawn_native/metal/DeviceMTL.mm
+++ b/src/dawn_native/metal/DeviceMTL.mm
@@ -145,8 +145,8 @@
const SwapChainDescriptor* descriptor) {
return new SwapChain(this, surface, previousSwapChain, descriptor);
}
- ResultOrError<TextureBase*> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
- return new Texture(this, descriptor);
+ ResultOrError<Ref<TextureBase>> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
+ return AcquireRef(new Texture(this, descriptor));
}
ResultOrError<TextureViewBase*> Device::CreateTextureViewImpl(
TextureBase* texture,
diff --git a/src/dawn_native/null/DeviceNull.cpp b/src/dawn_native/null/DeviceNull.cpp
index 701e720..1f75cf9 100644
--- a/src/dawn_native/null/DeviceNull.cpp
+++ b/src/dawn_native/null/DeviceNull.cpp
@@ -159,8 +159,8 @@
const SwapChainDescriptor* descriptor) {
return new SwapChain(this, surface, previousSwapChain, descriptor);
}
- ResultOrError<TextureBase*> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
- return new Texture(this, descriptor, TextureBase::TextureState::OwnedInternal);
+ ResultOrError<Ref<TextureBase>> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
+ return AcquireRef(new Texture(this, descriptor, TextureBase::TextureState::OwnedInternal));
}
ResultOrError<TextureViewBase*> Device::CreateTextureViewImpl(
TextureBase* texture,
diff --git a/src/dawn_native/null/DeviceNull.h b/src/dawn_native/null/DeviceNull.h
index 83f09ae..fc67316 100644
--- a/src/dawn_native/null/DeviceNull.h
+++ b/src/dawn_native/null/DeviceNull.h
@@ -132,7 +132,8 @@
Surface* surface,
NewSwapChainBase* previousSwapChain,
const SwapChainDescriptor* descriptor) override;
- ResultOrError<TextureBase*> CreateTextureImpl(const TextureDescriptor* descriptor) override;
+ ResultOrError<Ref<TextureBase>> CreateTextureImpl(
+ const TextureDescriptor* descriptor) override;
ResultOrError<TextureViewBase*> CreateTextureViewImpl(
TextureBase* texture,
const TextureViewDescriptor* descriptor) override;
diff --git a/src/dawn_native/opengl/DeviceGL.cpp b/src/dawn_native/opengl/DeviceGL.cpp
index 38b7123..47d2eec 100644
--- a/src/dawn_native/opengl/DeviceGL.cpp
+++ b/src/dawn_native/opengl/DeviceGL.cpp
@@ -136,8 +136,8 @@
const SwapChainDescriptor* descriptor) {
return DAWN_VALIDATION_ERROR("New swapchains not implemented.");
}
- ResultOrError<TextureBase*> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
- return new Texture(this, descriptor);
+ ResultOrError<Ref<TextureBase>> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
+ return AcquireRef(new Texture(this, descriptor));
}
ResultOrError<TextureViewBase*> Device::CreateTextureViewImpl(
TextureBase* texture,
diff --git a/src/dawn_native/opengl/DeviceGL.h b/src/dawn_native/opengl/DeviceGL.h
index f892a84..a768c89 100644
--- a/src/dawn_native/opengl/DeviceGL.h
+++ b/src/dawn_native/opengl/DeviceGL.h
@@ -89,7 +89,8 @@
Surface* surface,
NewSwapChainBase* previousSwapChain,
const SwapChainDescriptor* descriptor) override;
- ResultOrError<TextureBase*> CreateTextureImpl(const TextureDescriptor* descriptor) override;
+ ResultOrError<Ref<TextureBase>> CreateTextureImpl(
+ const TextureDescriptor* descriptor) override;
ResultOrError<TextureViewBase*> CreateTextureViewImpl(
TextureBase* texture,
const TextureViewDescriptor* descriptor) override;
diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp
index 7ce2e6c..f3b176b 100644
--- a/src/dawn_native/vulkan/DeviceVk.cpp
+++ b/src/dawn_native/vulkan/DeviceVk.cpp
@@ -148,7 +148,7 @@
const SwapChainDescriptor* descriptor) {
return DAWN_VALIDATION_ERROR("New swapchains not implemented.");
}
- ResultOrError<TextureBase*> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
+ ResultOrError<Ref<TextureBase>> Device::CreateTextureImpl(const TextureDescriptor* descriptor) {
return Texture::Create(this, descriptor);
}
ResultOrError<TextureViewBase*> Device::CreateTextureViewImpl(
diff --git a/src/dawn_native/vulkan/DeviceVk.h b/src/dawn_native/vulkan/DeviceVk.h
index 255971f..63758c1 100644
--- a/src/dawn_native/vulkan/DeviceVk.h
+++ b/src/dawn_native/vulkan/DeviceVk.h
@@ -123,7 +123,8 @@
Surface* surface,
NewSwapChainBase* previousSwapChain,
const SwapChainDescriptor* descriptor) override;
- ResultOrError<TextureBase*> CreateTextureImpl(const TextureDescriptor* descriptor) override;
+ ResultOrError<Ref<TextureBase>> CreateTextureImpl(
+ const TextureDescriptor* descriptor) override;
ResultOrError<TextureViewBase*> CreateTextureViewImpl(
TextureBase* texture,
const TextureViewDescriptor* descriptor) override;
diff --git a/src/dawn_native/vulkan/TextureVk.cpp b/src/dawn_native/vulkan/TextureVk.cpp
index a98a2b7..0c85fa9 100644
--- a/src/dawn_native/vulkan/TextureVk.cpp
+++ b/src/dawn_native/vulkan/TextureVk.cpp
@@ -406,11 +406,12 @@
}
// static
- ResultOrError<Texture*> Texture::Create(Device* device, const TextureDescriptor* descriptor) {
+ ResultOrError<Ref<TextureBase>> Texture::Create(Device* device,
+ const TextureDescriptor* descriptor) {
Ref<Texture> texture =
AcquireRef(new Texture(device, descriptor, TextureState::OwnedInternal));
DAWN_TRY(texture->InitializeAsInternalTexture());
- return texture.Detach();
+ return std::move(texture);
}
// static
diff --git a/src/dawn_native/vulkan/TextureVk.h b/src/dawn_native/vulkan/TextureVk.h
index 600e9a6..f9b44cd 100644
--- a/src/dawn_native/vulkan/TextureVk.h
+++ b/src/dawn_native/vulkan/TextureVk.h
@@ -40,7 +40,8 @@
class Texture final : public TextureBase {
public:
// Used to create a regular texture from a descriptor.
- static ResultOrError<Texture*> Create(Device* device, const TextureDescriptor* descriptor);
+ static ResultOrError<Ref<TextureBase>> Create(Device* device,
+ const TextureDescriptor* descriptor);
// Creates a texture and initializes it with a VkImage that references an external memory
// object. Before the texture can be used, the VkDeviceMemory associated with the external
diff --git a/src/tests/unittests/ResultTests.cpp b/src/tests/unittests/ResultTests.cpp
index 8e2f05f..7b21d2e 100644
--- a/src/tests/unittests/ResultTests.cpp
+++ b/src/tests/unittests/ResultTests.cpp
@@ -296,6 +296,42 @@
TestSuccess(&result, &success);
}
+class OtherClass {
+ public:
+ int a = 0;
+};
+class Base : public RefCounted {};
+class Child : public OtherClass, public Base {};
+
+// Test constructing a Result<Ref<TChild>, E>
+TEST(ResultRefT, ConversionFromChildConstructor) {
+ Child child;
+ Ref<Child> refChild(&child);
+
+ Result<Ref<Base>, int> result(std::move(refChild));
+ TestSuccess<Base>(&result, &child);
+}
+
+// Test copy constructing Result<Ref<TChild>, E>
+TEST(ResultRefT, ConversionFromChildCopyConstructor) {
+ Child child;
+ Ref<Child> refChild(&child);
+
+ Result<Ref<Child>, int> resultChild(std::move(refChild));
+ Result<Ref<Base>, int> result(std::move(resultChild));
+ TestSuccess<Base>(&result, &child);
+}
+
+// Test assignment operator for Result<Ref<TChild>, E>
+TEST(ResultRefT, ConversionFromChildAssignmentOperator) {
+ Child child;
+ Ref<Child> refChild(&child);
+
+ Result<Ref<Child>, int> resultChild(std::move(refChild));
+ Result<Ref<Base>, int> result = std::move(resultChild);
+ TestSuccess<Base>(&result, &child);
+}
+
// Result<T, E>
// Test constructing an error Result<T, E>
