encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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>
This commit is contained in:
Corentin Wallez 2020-06-11 11:07:05 +00:00 committed by Commit Bot service account
parent b6eff5acf0
commit 13f4650155
5 changed files with 131 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
src/dawn_native/Buffer.cpp
View File

@ -163,14 +163,22 @@ namespace dawn_native {
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 @@ namespace dawn_native {
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 @@ namespace dawn_native {
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 @@ namespace dawn_native {
}
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 @@ namespace dawn_native {
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 @@ namespace dawn_native {
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 @@ namespace dawn_native {
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 {};

1
src/dawn_native/Buffer.h
View File

@ -35,6 +35,7 @@ namespace dawn_native {
enum class BufferState {
Unmapped,
Mapped,
MappedAtCreation,
Destroyed,
};

6
src/dawn_native/opengl/BufferGL.cpp
View File

@ -22,9 +22,13 @@ namespace dawn_native { namespace opengl {
Buffer::Buffer(Device* device, const BufferDescriptor* descriptor)
: BufferBase(device, descriptor) {
// TODO(cwallez@chromium.org): Have a global "zero" buffer instead of creating a new 4-byte
// buffer?
uint64_t size = std::max(GetSize(), uint64_t(4u));
device->gl.GenBuffers(1, &mBuffer);
device->gl.BindBuffer(GL_ARRAY_BUFFER, mBuffer);
device->gl.BufferData(GL_ARRAY_BUFFER, GetSize(), nullptr, GL_STATIC_DRAW);
device->gl.BufferData(GL_ARRAY_BUFFER, size, nullptr, GL_STATIC_DRAW);
}
Buffer::~Buffer() {

1
src/dawn_wire/server/ServerBuffer.cpp
View File

@ -202,6 +202,7 @@ namespace dawn_wire { namespace server {
// to Unmap and attempt to update mapped data of an error buffer.
return false;
}
// Deserialize the flush info and flush updated data from the handle into the target
// of the handle. The target is set via WriteHandle::SetTarget.
return buffer->writeHandle->DeserializeFlush(writeFlushInfo,

91
src/tests/end2end/BufferTests.cpp
View File

@ -108,6 +108,17 @@ TEST_P(BufferMapReadTests, LargeRead) {
UnmapBuffer(buffer);
}
// Test mapping a zero-sized buffer.
TEST_P(BufferMapReadTests, ZeroSized) {
wgpu::BufferDescriptor descriptor;
descriptor.size = 0;
descriptor.usage = wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopyDst;
wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
MapReadAsyncAndWait(buffer);
UnmapBuffer(buffer);
}
DAWN_INSTANTIATE_TEST(BufferMapReadTests, D3D12Backend(), MetalBackend(), OpenGLBackend(), VulkanBackend());
class BufferMapWriteTests : public DawnTest {
@ -202,6 +213,17 @@ TEST_P(BufferMapWriteTests, LargeWrite) {
EXPECT_BUFFER_U32_RANGE_EQ(myData.data(), buffer, 0, kDataSize);
}
// Test mapping a zero-sized buffer.
TEST_P(BufferMapWriteTests, ZeroSized) {
wgpu::BufferDescriptor descriptor;
descriptor.size = 0;
descriptor.usage = wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopySrc;
wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
MapWriteAsyncAndWait(buffer);
UnmapBuffer(buffer);
}
// Stress test mapping many buffers.
TEST_P(BufferMapWriteTests, ManyWrites) {
constexpr uint32_t kDataSize = 1000;
@ -370,6 +392,21 @@ TEST_P(CreateBufferMappedTests, NonMappableUsageLarge) {
EXPECT_BUFFER_U32_RANGE_EQ(myData.data(), result.buffer, 0, kDataSize);
}
// Test destroying a non-mappable buffer mapped at creation.
// This is a regression test for an issue where the D3D12 backend thought the buffer was actually
// mapped and tried to unlock the heap residency (when actually the buffer was using a staging
// buffer)
TEST_P(CreateBufferMappedTests, DestroyNonMappableWhileMappedForCreation) {
wgpu::CreateBufferMappedResult result = CreateBufferMapped(wgpu::BufferUsage::CopySrc, 4);
result.buffer.Destroy();
}
// Test destroying a mappable buffer mapped at creation.
TEST_P(CreateBufferMappedTests, DestroyMappableWhileMappedForCreation) {
wgpu::CreateBufferMappedResult result = CreateBufferMapped(wgpu::BufferUsage::MapRead, 4);
result.buffer.Destroy();
}
// Test that mapping a buffer is valid after CreateBufferMapped and Unmap
TEST_P(CreateBufferMappedTests, CreateThenMapSuccess) {
static uint32_t myData = 230502;
@ -437,6 +474,48 @@ TEST_P(CreateBufferMappedTests, LargeBufferFails) {
ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
}
// Test that creating a zero-sized buffer mapped is allowed.
TEST_P(CreateBufferMappedTests, ZeroSized) {
wgpu::BufferDescriptor descriptor;
descriptor.size = 0;
descriptor.usage = wgpu::BufferUsage::Vertex;
wgpu::CreateBufferMappedResult result = device.CreateBufferMapped(&descriptor);
ASSERT_EQ(0u, result.dataLength);
ASSERT_NE(nullptr, result.data);
// Check that unmapping the buffer works too.
UnmapBuffer(result.buffer);
}
// Test that creating a zero-sized mapppable buffer mapped. (it is a different code path)
TEST_P(CreateBufferMappedTests, ZeroSizedMappableBuffer) {
wgpu::BufferDescriptor descriptor;
descriptor.size = 0;
descriptor.usage = wgpu::BufferUsage::MapWrite;
wgpu::CreateBufferMappedResult result = device.CreateBufferMapped(&descriptor);
ASSERT_EQ(0u, result.dataLength);
ASSERT_NE(nullptr, result.data);
// Check that unmapping the buffer works too.
UnmapBuffer(result.buffer);
}
// Test that creating a zero-sized error buffer mapped. (it is a different code path)
TEST_P(CreateBufferMappedTests, ZeroSizedErrorBuffer) {
DAWN_SKIP_TEST_IF(IsDawnValidationSkipped());
wgpu::BufferDescriptor descriptor;
descriptor.size = 0;
descriptor.usage = wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::Storage;
wgpu::CreateBufferMappedResult result;
ASSERT_DEVICE_ERROR(result = device.CreateBufferMapped(&descriptor));
ASSERT_EQ(0u, result.dataLength);
ASSERT_NE(nullptr, result.data);
}
DAWN_INSTANTIATE_TEST(CreateBufferMappedTests,
D3D12Backend(),
D3D12Backend({}, {"use_d3d12_resource_heap_tier2"}),
@ -446,6 +525,7 @@ DAWN_INSTANTIATE_TEST(CreateBufferMappedTests,
class BufferTests : public DawnTest {};
// Test that creating a zero-buffer is allowed.
TEST_P(BufferTests, ZeroSizedBuffer) {
wgpu::BufferDescriptor desc;
desc.size = 0;
@ -453,6 +533,17 @@ TEST_P(BufferTests, ZeroSizedBuffer) {
device.CreateBuffer(&desc);
}
// Test that creating a very large buffers fails gracefully.
TEST_P(BufferTests, LargeBufferFails) {
// TODO(http://crbug.com/dawn/27): Missing support.
DAWN_SKIP_TEST_IF(IsOpenGL());
wgpu::BufferDescriptor descriptor;
descriptor.size = std::numeric_limits<uint64_t>::max();
descriptor.usage = wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopyDst;
ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
}
DAWN_INSTANTIATE_TEST(BufferTests,
D3D12Backend(),
MetalBackend(),