commit 13f465015550d727279fc5dfd87a4e53b62b6a6b
author Corentin Wallez <cwallez@chromium.org> Thu Jun 11 11:07:05 2020 +0000
committer Commit Bot service account <commit-bot@chromium.org> Thu Jun 11 11:07:05 2020 +0000
tree 81e54e23588dd9b576d98b3d7c5b73083bb3cf8c
parent b6eff5acf06faf796e0be77558df2ca5411ff2e4

Add tests and fix (create) mapping zero-sized buffers

When creating a zero-sized buffer mapped, StagingBuffer creation
is skipped. This required adding a new MappedAtCreation state
since mStagingBuffer couldn't be used as a tag value for that.

Made the OpenGL backend always create non-zero-sized buffers.

Finally added tests for MapRead/WriteAsync and CreateBufferMapped
of zero-sized buffers.

Bug: dawn:446
Change-Id: I04f6fe98fd646f1867c21065cd1cd33a1595e19f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/21481
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/Buffer.cpp

diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp
index 46d00d1..4df366a 100644
--- a/src/dawn_native/Buffer.cpp
+++ b/src/dawn_native/Buffer.cpp
@@ -163,14 +163,22 @@
         ASSERT(!IsError());
         ASSERT(mappedPointer != nullptr);
 
-        mState = BufferState::Mapped;
-
+        // Mappable buffers don't use a staging buffer and are just as if mapped through MapAsync.
         if (IsMapWritable()) {
             DAWN_TRY(MapAtCreationImpl(mappedPointer));
+            mState = BufferState::Mapped;
             ASSERT(*mappedPointer != nullptr);
             return {};
         }
 
+        mState = BufferState::MappedAtCreation;
+
+        // 0-sized buffers are not supposed to be written to, Return back any non-null pointer.
+        if (mSize == 0) {
+            *mappedPointer = reinterpret_cast<uint8_t*>(intptr_t(0xCAFED00D));
+            return {};
+        }
+
         // If any of these fail, the buffer will be deleted and replaced with an
         // error buffer.
         // TODO(enga): Suballocate and reuse memory from a larger staging buffer so we don't create
@@ -190,6 +198,7 @@
             case BufferState::Destroyed:
                 return DAWN_VALIDATION_ERROR("Destroyed buffer used in a submit");
             case BufferState::Mapped:
+            case BufferState::MappedAtCreation:
                 return DAWN_VALIDATION_ERROR("Buffer used in a submit while mapped");
             case BufferState::Unmapped:
                 return {};
@@ -309,11 +318,15 @@
         ASSERT(!IsError());
 
         if (mState == BufferState::Mapped) {
-            if (mStagingBuffer == nullptr) {
-                Unmap();
+            Unmap();
+        } else if (mState == BufferState::MappedAtCreation) {
+            if (mStagingBuffer != nullptr) {
+                mStagingBuffer.reset();
+            } else {
+                ASSERT(mSize == 0);
             }
-            mStagingBuffer.reset();
         }
+
         DestroyInternal();
     }
 
@@ -342,9 +355,7 @@
         }
         ASSERT(!IsError());
 
-        if (mStagingBuffer != nullptr) {
-            GetDevice()->ConsumedError(CopyFromStagingBuffer());
-        } else {
+        if (mState == BufferState::Mapped) {
             // A map request can only be called once, so this will fire only if the request wasn't
             // completed before the Unmap.
             // Callbacks are not fired if there is no callback registered, so this is correct for
@@ -352,11 +363,20 @@
             CallMapReadCallback(mMapSerial, WGPUBufferMapAsyncStatus_Unknown, nullptr, 0u);
             CallMapWriteCallback(mMapSerial, WGPUBufferMapAsyncStatus_Unknown, nullptr, 0u);
             UnmapImpl();
+
+            mMapReadCallback = nullptr;
+            mMapWriteCallback = nullptr;
+            mMapUserdata = 0;
+
+        } else if (mState == BufferState::MappedAtCreation) {
+            if (mStagingBuffer != nullptr) {
+                GetDevice()->ConsumedError(CopyFromStagingBuffer());
+            } else {
+                ASSERT(mSize == 0);
+            }
         }
+
         mState = BufferState::Unmapped;
-        mMapReadCallback = nullptr;
-        mMapWriteCallback = nullptr;
-        mMapUserdata = 0;
     }
 
     MaybeError BufferBase::ValidateMap(wgpu::BufferUsage requiredUsage,
@@ -369,6 +389,7 @@
 
         switch (mState) {
             case BufferState::Mapped:
+            case BufferState::MappedAtCreation:
                 return DAWN_VALIDATION_ERROR("Buffer already mapped");
             case BufferState::Destroyed:
                 return DAWN_VALIDATION_ERROR("Buffer is destroyed");
@@ -390,6 +411,7 @@
 
         switch (mState) {
             case BufferState::Mapped:
+            case BufferState::MappedAtCreation:
                 // A buffer may be in the Mapped state if it was created with CreateBufferMapped
                 // even if it did not have a mappable usage.
                 return {};