Add copy constructors to the C++ Dawn interface
This removes the need for Clone() so it is removed and also adds tests
for the new constructors.
BUG=dawn:11
Change-Id: Ia45c765c2d30e40b0e036427793a62327b2008fc
Reviewed-on: https://dawn-review.googlesource.com/c/1901
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/examples/ComputeBoids.cpp b/examples/ComputeBoids.cpp
index 77b2ad8..869f95f 100644
--- a/examples/ComputeBoids.cpp
+++ b/examples/ComputeBoids.cpp
@@ -232,9 +232,9 @@
dawn::PipelineLayout pl = utils::MakeBasicPipelineLayout(device, &bgl);
dawn::ComputePipelineDescriptor csDesc;
- csDesc.module = module.Clone();
+ csDesc.module = module;
csDesc.entryPoint = "main";
- csDesc.layout = pl.Clone();
+ csDesc.layout = pl;
updatePipeline = device.CreateComputePipeline(&csDesc);
dawn::BufferView updateParamsView = updateParams.CreateBufferViewBuilder()
diff --git a/generator/templates/apicpp.h b/generator/templates/apicpp.h
index 68873ac..af142b0 100644
--- a/generator/templates/apicpp.h
+++ b/generator/templates/apicpp.h
@@ -70,19 +70,29 @@
if (mHandle) Derived::DawnRelease(mHandle);
}
- ObjectBase(ObjectBase const& other) = delete;
- Derived& operator=(ObjectBase const& other) = delete;
+ ObjectBase(ObjectBase const& other)
+ : ObjectBase(other.Get()) {
+ }
+ Derived& operator=(ObjectBase const& other) {
+ if (&other != this) {
+ if (mHandle) Derived::DawnRelease(mHandle);
+ mHandle = other.mHandle;
+ if (mHandle) Derived::DawnReference(mHandle);
+ }
+
+ return static_cast<Derived&>(*this);
+ }
ObjectBase(ObjectBase&& other) {
mHandle = other.mHandle;
other.mHandle = 0;
}
Derived& operator=(ObjectBase&& other) {
- if (&other == this) return static_cast<Derived&>(*this);
-
- if (mHandle) Derived::DawnRelease(mHandle);
- mHandle = other.mHandle;
- other.mHandle = 0;
+ if (&other != this) {
+ if (mHandle) Derived::DawnRelease(mHandle);
+ mHandle = other.mHandle;
+ other.mHandle = 0;
+ }
return static_cast<Derived&>(*this);
}
@@ -98,9 +108,6 @@
mHandle = 0;
return result;
}
- Derived Clone() const {
- return Derived(mHandle);
- }
static Derived Acquire(CType handle) {
Derived result;
result.mHandle = handle;
diff --git a/src/tests/DawnTest.cpp b/src/tests/DawnTest.cpp
index ad88763..f58dd2b 100644
--- a/src/tests/DawnTest.cpp
+++ b/src/tests/DawnTest.cpp
@@ -258,15 +258,13 @@
uint32_t offset,
uint32_t size,
detail::Expectation* expectation) {
- dawn::Buffer source = buffer.Clone();
-
auto readback = ReserveReadback(size);
// We need to enqueue the copy immediately because by the time we resolve the expectation,
// the buffer might have been modified.
dawn::CommandBuffer commands =
device.CreateCommandBufferBuilder()
- .CopyBufferToBuffer(source, offset, readback.buffer, readback.offset, size)
+ .CopyBufferToBuffer(buffer, offset, readback.buffer, readback.offset, size)
.GetResult();
queue.Submit(1, &commands);
@@ -296,7 +294,6 @@
uint32_t level,
uint32_t pixelSize,
detail::Expectation* expectation) {
- dawn::Texture source = texture.Clone();
uint32_t rowPitch = Align(width * pixelSize, kTextureRowPitchAlignment);
uint32_t size = rowPitch * (height - 1) + width * pixelSize;
@@ -306,7 +303,7 @@
// the texture might have been modified.
dawn::CommandBuffer commands =
device.CreateCommandBufferBuilder()
- .CopyTextureToBuffer(source, x, y, 0, width, height, 1, level, 0, readback.buffer,
+ .CopyTextureToBuffer(texture, x, y, 0, width, height, 1, level, 0, readback.buffer,
readback.offset, rowPitch)
.GetResult();
@@ -359,7 +356,7 @@
slot.buffer = device.CreateBuffer(&descriptor);
ReadbackReservation reservation;
- reservation.buffer = slot.buffer.Clone();
+ reservation.buffer = slot.buffer;
reservation.slot = mReadbackSlots.size();
reservation.offset = 0;
diff --git a/src/tests/end2end/BindGroupTests.cpp b/src/tests/end2end/BindGroupTests.cpp
index 9510e06..8585151 100644
--- a/src/tests/end2end/BindGroupTests.cpp
+++ b/src/tests/end2end/BindGroupTests.cpp
@@ -52,9 +52,9 @@
dawn::ShaderModule module =
utils::CreateShaderModule(device, dawn::ShaderStage::Compute, shader);
dawn::ComputePipelineDescriptor cpDesc;
- cpDesc.module = module.Clone();
+ cpDesc.module = module;
cpDesc.entryPoint = "main";
- cpDesc.layout = pl.Clone();
+ cpDesc.layout = pl;
dawn::ComputePipeline cp = device.CreateComputePipeline(&cpDesc);
dawn::BufferDescriptor bufferDesc;
diff --git a/src/tests/end2end/ComputeCopyStorageBufferTests.cpp b/src/tests/end2end/ComputeCopyStorageBufferTests.cpp
index 9ee3021..d047ec1 100644
--- a/src/tests/end2end/ComputeCopyStorageBufferTests.cpp
+++ b/src/tests/end2end/ComputeCopyStorageBufferTests.cpp
@@ -39,9 +39,9 @@
auto pl = utils::MakeBasicPipelineLayout(device, &bgl);
dawn::ComputePipelineDescriptor csDesc;
- csDesc.module = module.Clone();
+ csDesc.module = module;
csDesc.entryPoint = "main";
- csDesc.layout = pl.Clone();
+ csDesc.layout = pl;
dawn::ComputePipeline pipeline = device.CreateComputePipeline(&csDesc);
// Set up src storage buffer
diff --git a/src/tests/end2end/PushConstantTests.cpp b/src/tests/end2end/PushConstantTests.cpp
index 070abec..80fa857 100644
--- a/src/tests/end2end/PushConstantTests.cpp
+++ b/src/tests/end2end/PushConstantTests.cpp
@@ -146,9 +146,9 @@
);
dawn::ComputePipelineDescriptor descriptor;
- descriptor.module = module.Clone();
+ descriptor.module = module;
descriptor.entryPoint = "main";
- descriptor.layout = pl.Clone();
+ descriptor.layout = pl;
return device.CreateComputePipeline(&descriptor);
}
diff --git a/src/tests/unittests/ObjectBaseTests.cpp b/src/tests/unittests/ObjectBaseTests.cpp
index 1e6058a..2c9dccb 100644
--- a/src/tests/unittests/ObjectBaseTests.cpp
+++ b/src/tests/unittests/ObjectBaseTests.cpp
@@ -22,7 +22,7 @@
using ObjectBase::operator=;
static void DawnReference(int* handle) {
- ASSERT_LT(0, *handle);
+ ASSERT_LE(0, *handle);
*handle += 1;
}
static void DawnRelease(int* handle) {
@@ -52,16 +52,14 @@
ASSERT_EQ(0, refcount);
}
-// Test that cloning takes a new ref. Also test .Get().
-TEST(ObjectBase, Clone) {
+// Test .Get().
+TEST(ObjectBase, Get) {
int refcount = 1;
{
Object obj1(&refcount);
- Object obj2 = obj1.Clone();
- ASSERT_EQ(3, refcount);
+ ASSERT_EQ(2, refcount);
ASSERT_EQ(&refcount, obj1.Get());
- ASSERT_EQ(&refcount, obj2.Get());
}
ASSERT_EQ(1, refcount);
}
@@ -91,6 +89,51 @@
}
}
+// Test the copy constructor of C++ objects
+TEST(ObjectBase, CopyConstructor) {
+ int refcount = 1;
+
+ Object source(&refcount);
+ Object destination(source);
+
+ ASSERT_EQ(source.Get(), &refcount);
+ ASSERT_EQ(destination.Get(), &refcount);
+ ASSERT_EQ(3, refcount);
+
+ destination = Object();
+ ASSERT_EQ(refcount, 2);
+}
+
+// Test the copy assignment of C++ objects
+TEST(ObjectBase, CopyAssignment) {
+ int refcount = 1;
+ Object source(&refcount);
+
+ Object destination;
+ destination = source;
+
+ ASSERT_EQ(source.Get(), &refcount);
+ ASSERT_EQ(destination.Get(), &refcount);
+ ASSERT_EQ(3, refcount);
+
+ destination = Object();
+ ASSERT_EQ(refcount, 2);
+}
+
+// Test the copy assignment of C++ objects onto themselves
+TEST(ObjectBase, CopyAssignmentSelf) {
+ int refcount = 1;
+
+ Object obj(&refcount);
+
+ // Fool the compiler to avoid a -Wself-assign-overload
+ Object* objPtr = &obj;
+ obj = *objPtr;
+
+ ASSERT_EQ(obj.Get(), &refcount);
+ ASSERT_EQ(refcount, 2);
+}
+
// Test the move constructor of C++ objects
TEST(ObjectBase, MoveConstructor) {
int refcount = 1;
@@ -121,3 +164,16 @@
ASSERT_EQ(refcount, 1);
}
+// Test the move assignment of C++ objects onto themselves
+TEST(ObjectBase, MoveAssignmentSelf) {
+ int refcount = 1;
+
+ Object obj(&refcount);
+
+ // Fool the compiler to avoid a -Wself-move
+ Object* objPtr = &obj;
+ obj = std::move(*objPtr);
+
+ ASSERT_EQ(obj.Get(), &refcount);
+ ASSERT_EQ(refcount, 2);
+}
