Add Buffer Destroyed state and validation tests

Adds Destroy() to the Buffer frontend but doesn't implement
it in the backend yet.

Bug: dawn:46, dawn:7
Change-Id: I2e6bdba1484fc5f49599801b4ffe3e9a6aaf60a5
Reviewed-on: https://dawn-review.googlesource.com/c/3720
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>

dawn.json

@@ -185,6 +185,9 @@
             },
             {
                 "name": "unmap"
+            },
+            {
+                "name": "destroy"
             }
         ]
     },

src/dawn_native/Buffer.cpp

@@ -77,14 +77,18 @@ namespace dawn_native {
     // Buffer
 
     BufferBase::BufferBase(DeviceBase* device, const BufferDescriptor* descriptor)
-        : ObjectBase(device), mSize(descriptor->size), mUsage(descriptor->usage) {
+        : ObjectBase(device),
+          mSize(descriptor->size),
+          mUsage(descriptor->usage),
+          mState(BufferState::Unmapped) {
     }
 
-    BufferBase::BufferBase(DeviceBase* device, ObjectBase::ErrorTag tag) : ObjectBase(device, tag) {
+    BufferBase::BufferBase(DeviceBase* device, ObjectBase::ErrorTag tag)
+        : ObjectBase(device, tag), mState(BufferState::Unmapped) {
     }
 
     BufferBase::~BufferBase() {
-        if (mIsMapped) {
+        if (mState == BufferState::Mapped) {
             ASSERT(!IsError());
             CallMapReadCallback(mMapSerial, DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr);
             CallMapWriteCallback(mMapSerial, DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr);
@@ -109,11 +113,15 @@ namespace dawn_native {
     MaybeError BufferBase::ValidateCanUseInSubmitNow() const {
         ASSERT(!IsError());
 
-        if (mIsMapped) {
+        switch (mState) {
+            case BufferState::Destroyed:
+                return DAWN_VALIDATION_ERROR("Destroyed buffer used in a submit");
+            case BufferState::Mapped:
                 return DAWN_VALIDATION_ERROR("Buffer used in a submit while mapped");
-        }
+            case BufferState::Unmapped:
                 return {};
         }
+    }
 
     void BufferBase::CallMapReadCallback(uint32_t serial,
                                          dawnBufferMapAsyncStatus status,
@@ -172,7 +180,7 @@ namespace dawn_native {
         mMapSerial++;
         mMapReadCallback = callback;
         mMapUserdata = userdata;
-        mIsMapped = true;
+        mState = BufferState::Mapped;
 
         MapReadAsyncImpl(mMapSerial, start, size);
     }
@@ -193,11 +201,23 @@ namespace dawn_native {
         mMapSerial++;
         mMapWriteCallback = callback;
         mMapUserdata = userdata;
-        mIsMapped = true;
+        mState = BufferState::Mapped;
 
         MapWriteAsyncImpl(mMapSerial, start, size);
     }
 
+    void BufferBase::Destroy() {
+        if (GetDevice()->ConsumedError(ValidateDestroy())) {
+            return;
+        }
+        ASSERT(!IsError());
+
+        if (mState == BufferState::Mapped) {
+            Unmap();
+        }
+        mState = BufferState::Destroyed;
+    }
+
     void BufferBase::Unmap() {
         if (GetDevice()->ConsumedError(ValidateUnmap())) {
             return;
@@ -209,7 +229,7 @@ namespace dawn_native {
         CallMapReadCallback(mMapSerial, DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr);
         CallMapWriteCallback(mMapSerial, DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr);
         UnmapImpl();
-        mIsMapped = false;
+        mState = BufferState::Unmapped;
         mMapReadCallback = nullptr;
         mMapWriteCallback = nullptr;
         mMapUserdata = 0;
@@ -218,6 +238,14 @@ namespace dawn_native {
     MaybeError BufferBase::ValidateSetSubData(uint32_t start, uint32_t count) const {
         DAWN_TRY(GetDevice()->ValidateObject(this));
 
+        if (mState == BufferState::Destroyed) {
+            return DAWN_VALIDATION_ERROR("Buffer is destroyed");
+        }
+
+        if (mState == BufferState::Mapped) {
+            return DAWN_VALIDATION_ERROR("Buffer is mapped");
+        }
+
         if (count > GetSize()) {
             return DAWN_VALIDATION_ERROR("Buffer subdata with too much data");
         }
@@ -248,7 +276,11 @@ namespace dawn_native {
             return DAWN_VALIDATION_ERROR("Buffer mapping out of range");
         }
 
-        if (mIsMapped) {
+        if (mState == BufferState::Destroyed) {
+            return DAWN_VALIDATION_ERROR("Buffer is destroyed");
+        }
+
+        if (mState == BufferState::Mapped) {
             return DAWN_VALIDATION_ERROR("Buffer already mapped");
         }
 
@@ -262,10 +294,20 @@ namespace dawn_native {
     MaybeError BufferBase::ValidateUnmap() const {
         DAWN_TRY(GetDevice()->ValidateObject(this));
 
-        if (!mIsMapped) {
+        if ((mUsage & (dawn::BufferUsageBit::MapRead | dawn::BufferUsageBit::MapWrite)) == 0) {
-            return DAWN_VALIDATION_ERROR("Buffer wasn't mapped");
+            return DAWN_VALIDATION_ERROR("Buffer does not have map usage");
+        }
+        switch (mState) {
+            case BufferState::Unmapped:
+            case BufferState::Mapped:
+                return {};
+            case BufferState::Destroyed:
+                return DAWN_VALIDATION_ERROR("Buffer is destroyed");
+        }
     }
 
+    MaybeError BufferBase::ValidateDestroy() const {
+        DAWN_TRY(GetDevice()->ValidateObject(this));
         return {};
     }
 

src/dawn_native/Buffer.h

@@ -35,6 +35,12 @@ namespace dawn_native {
         dawn::BufferUsageBit::Storage;
 
     class BufferBase : public ObjectBase {
+        enum class BufferState {
+            Unmapped,
+            Mapped,
+            Destroyed,
+        };
+
       public:
         BufferBase(DeviceBase* device, const BufferDescriptor* descriptor);
         ~BufferBase();
@@ -57,6 +63,7 @@ namespace dawn_native {
                            dawnBufferMapWriteCallback callback,
                            dawnCallbackUserdata userdata);
         void Unmap();
+        void Destroy();
 
       protected:
         BufferBase(DeviceBase* device, ObjectBase::ErrorTag tag);
@@ -77,6 +84,7 @@ namespace dawn_native {
                                uint32_t size,
                                dawn::BufferUsageBit requiredUsage) const;
         MaybeError ValidateUnmap() const;
+        MaybeError ValidateDestroy() const;
 
         uint32_t mSize = 0;
         dawn::BufferUsageBit mUsage = dawn::BufferUsageBit::None;
@@ -86,7 +94,7 @@ namespace dawn_native {
         dawnCallbackUserdata mMapUserdata = 0;
         uint32_t mMapSerial = 0;
 
-        bool mIsMapped = false;
+        BufferState mState;
     };
 
     // This builder class is kept around purely for testing but should not be used.

src/tests/unittests/validation/BufferValidationTests.cpp

@@ -495,3 +495,234 @@ TEST_F(BufferValidationTest, SetSubDataWrongUsage) {
     uint8_t foo = 0;
     ASSERT_DEVICE_ERROR(buf.SetSubData(0, sizeof(foo), &foo));
 }
+
+// Test that it is valid to destroy an unmapped buffer
+TEST_F(BufferValidationTest, DestroyUnmappedBuffer) {
+    {
+        dawn::Buffer buf = CreateMapReadBuffer(4);
+        buf.Destroy();
+    }
+    {
+        dawn::Buffer buf = CreateMapWriteBuffer(4);
+        buf.Destroy();
+    }
+}
+
+// Test that it is valid to destroy a mapped buffer
+TEST_F(BufferValidationTest, DestroyMappedBuffer) {
+    {
+        dawn::Buffer buf = CreateMapReadBuffer(4);
+        buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, 30303);
+        buf.Destroy();
+    }
+    {
+        dawn::Buffer buf = CreateMapWriteBuffer(4);
+        buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, 30233);
+        buf.Destroy();
+    }
+}
+
+// Test that destroying a buffer implicitly unmaps it
+TEST_F(BufferValidationTest, DestroyMappedBufferCausesImplicitUnmap) {
+    {
+        dawn::Buffer buf = CreateMapReadBuffer(4);
+        dawn::CallbackUserdata userdata = 40598;
+        buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, userdata);
+        // Buffer is destroyed. Callback should be called with UNKNOWN status
+        EXPECT_CALL(*mockBufferMapReadCallback,
+                    Call(DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, userdata))
+            .Times(1);
+        buf.Destroy();
+        queue.Submit(0, nullptr);
+    }
+    {
+        dawn::Buffer buf = CreateMapWriteBuffer(4);
+        dawn::CallbackUserdata userdata = 23980;
+        buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, userdata);
+        // Buffer is destroyed. Callback should be called with UNKNOWN status
+        EXPECT_CALL(*mockBufferMapWriteCallback,
+                    Call(DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr, userdata))
+            .Times(1);
+        buf.Destroy();
+        queue.Submit(0, nullptr);
+    }
+}
+
+// Test that it is valid to Destroy a destroyed buffer
+TEST_F(BufferValidationTest, DestroyDestroyedBuffer) {
+    dawn::Buffer buf = CreateSetSubDataBuffer(4);
+    buf.Destroy();
+    buf.Destroy();
+}
+
+// Test that it is invalid to Unmap a destroyed buffer
+TEST_F(BufferValidationTest, UnmapDestroyedBuffer) {
+    {
+        dawn::Buffer buf = CreateMapReadBuffer(4);
+        buf.Destroy();
+        ASSERT_DEVICE_ERROR(buf.Unmap());
+    }
+    {
+        dawn::Buffer buf = CreateMapWriteBuffer(4);
+        buf.Destroy();
+        ASSERT_DEVICE_ERROR(buf.Unmap());
+    }
+}
+
+// Test that it is invalid to map a destroyed buffer
+TEST_F(BufferValidationTest, MapDestroyedBuffer) {
+    {
+        dawn::Buffer buf = CreateMapReadBuffer(4);
+        buf.Destroy();
+        ASSERT_DEVICE_ERROR(buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, 11303));
+    }
+    {
+        dawn::Buffer buf = CreateMapWriteBuffer(4);
+        buf.Destroy();
+        ASSERT_DEVICE_ERROR(buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, 56303));
+    }
+}
+
+// Test that it is invalid to call SetSubData on a destroyed buffer
+TEST_F(BufferValidationTest, SetSubDataDestroyedBuffer) {
+    dawn::Buffer buf = CreateSetSubDataBuffer(4);
+    buf.Destroy();
+    uint8_t foo = 0;
+    ASSERT_DEVICE_ERROR(buf.SetSubData(0, sizeof(foo), &foo));
+}
+
+// Test that is is invalid to Map a mapped buffer
+TEST_F(BufferValidationTest, MapMappedbuffer) {
+    {
+        dawn::Buffer buf = CreateMapReadBuffer(4);
+        buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, 43309);
+        ASSERT_DEVICE_ERROR(buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, 34309));
+        queue.Submit(0, nullptr);
+    }
+    {
+        dawn::Buffer buf = CreateMapWriteBuffer(4);
+        buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, 20301);
+        ASSERT_DEVICE_ERROR(buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, 40303));
+        queue.Submit(0, nullptr);
+    }
+}
+
+// Test that it is invalid to call SetSubData on a mapped buffer
+TEST_F(BufferValidationTest, SetSubDataMappedBuffer) {
+    {
+        dawn::Buffer buf = CreateMapReadBuffer(4);
+        buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, 42899);
+        uint8_t foo = 0;
+        ASSERT_DEVICE_ERROR(buf.SetSubData(0, sizeof(foo), &foo));
+        queue.Submit(0, nullptr);
+    }
+    {
+        dawn::Buffer buf = CreateMapWriteBuffer(4);
+        buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, 40329);
+        uint8_t foo = 0;
+        ASSERT_DEVICE_ERROR(buf.SetSubData(0, sizeof(foo), &foo));
+        queue.Submit(0, nullptr);
+    }
+}
+
+// Test that it is valid to submit a buffer in a queue with a map usage if it is unmapped
+TEST_F(BufferValidationTest, SubmitBufferWithMapUsage) {
+    dawn::BufferDescriptor descriptorA;
+    descriptorA.size = 4;
+    descriptorA.usage = dawn::BufferUsageBit::TransferSrc | dawn::BufferUsageBit::MapWrite;
+
+    dawn::BufferDescriptor descriptorB;
+    descriptorB.size = 4;
+    descriptorB.usage = dawn::BufferUsageBit::TransferDst | dawn::BufferUsageBit::MapRead;
+
+    dawn::Buffer bufA = device.CreateBuffer(&descriptorA);
+    dawn::Buffer bufB = device.CreateBuffer(&descriptorB);
+
+    dawn::CommandBufferBuilder builder = device.CreateCommandBufferBuilder();
+    builder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4);
+    dawn::CommandBuffer commands = builder.GetResult();
+    queue.Submit(1, &commands);
+}
+
+// Test that it is invalid to submit a mapped buffer in a queue
+TEST_F(BufferValidationTest, SubmitMappedBuffer) {
+    dawn::BufferDescriptor descriptorA;
+    descriptorA.size = 4;
+    descriptorA.usage = dawn::BufferUsageBit::TransferSrc | dawn::BufferUsageBit::MapWrite;
+
+    dawn::BufferDescriptor descriptorB;
+    descriptorB.size = 4;
+    descriptorB.usage = dawn::BufferUsageBit::TransferDst | dawn::BufferUsageBit::MapRead;
+    {
+        dawn::Buffer bufA = device.CreateBuffer(&descriptorA);
+        dawn::Buffer bufB = device.CreateBuffer(&descriptorB);
+
+        bufA.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, 40329);
+
+        dawn::CommandBufferBuilder builder = device.CreateCommandBufferBuilder();
+        builder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4);
+        dawn::CommandBuffer commands = builder.GetResult();
+        ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
+        queue.Submit(0, nullptr);
+    }
+    {
+        dawn::Buffer bufA = device.CreateBuffer(&descriptorA);
+        dawn::Buffer bufB = device.CreateBuffer(&descriptorB);
+
+        bufB.MapReadAsync(0, 4, ToMockBufferMapReadCallback, 11329);
+
+        dawn::CommandBufferBuilder builder = device.CreateCommandBufferBuilder();
+        builder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4);
+        dawn::CommandBuffer commands = builder.GetResult();
+        ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
+        queue.Submit(0, nullptr);
+    }
+}
+
+// Test that it is invalid to submit a destroyed buffer in a queue
+TEST_F(BufferValidationTest, SubmitDestroyedBuffer) {
+    dawn::BufferDescriptor descriptorA;
+    descriptorA.size = 4;
+    descriptorA.usage = dawn::BufferUsageBit::TransferSrc;
+
+    dawn::BufferDescriptor descriptorB;
+    descriptorB.size = 4;
+    descriptorB.usage = dawn::BufferUsageBit::TransferDst;
+
+    dawn::Buffer bufA = device.CreateBuffer(&descriptorA);
+    dawn::Buffer bufB = device.CreateBuffer(&descriptorB);
+
+    bufA.Destroy();
+    dawn::CommandBufferBuilder builder = device.CreateCommandBufferBuilder();
+    builder.CopyBufferToBuffer(bufA, 0, bufB, 0, 4);
+    dawn::CommandBuffer commands = builder.GetResult();
+    ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
+}
+
+// Test that a map usage is required to call Unmap
+TEST_F(BufferValidationTest, UnmapWithoutMapUsage) {
+    dawn::Buffer buf = CreateSetSubDataBuffer(4);
+    ASSERT_DEVICE_ERROR(buf.Unmap());
+}
+
+// Test that it is valid to call Unmap on a buffer that is not mapped
+TEST_F(BufferValidationTest, UnmapUnmappedBuffer) {
+    {
+        dawn::Buffer buf = CreateMapReadBuffer(4);
+        // Buffer starts unmapped. Unmap should succeed.
+        buf.Unmap();
+        buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, 30603);
+        buf.Unmap();
+        // Unmapping twice should succeed
+        buf.Unmap();
+    }
+    {
+        dawn::Buffer buf = CreateMapWriteBuffer(4);
+        // Buffer starts unmapped. Unmap should succeed.
+        buf.Unmap();
+        buf.MapWriteAsync(0, 4, ToMockBufferMapWriteCallback, 23890);
+        // Unmapping twice should succeed
+        buf.Unmap();
+        buf.Unmap();
+    }
+}