From 68358b5c2319784b58c892be32b9dbb9e35ec801 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Mon, 12 Jun 2017 13:11:54 -0400 Subject: [PATCH] Add Buffer::MapReadAsync validation tests. This also expands the Buffer validation tests to cover more creation code paths and SetSubData. It also introduces a mechanism for ValidationTests to check for device errors. --- .../validation/BufferValidationTests.cpp | 283 ++++++++++++++++-- .../DepthStencilStateValidationTests.cpp | 2 + .../unittests/validation/ValidationTest.cpp | 27 ++ .../unittests/validation/ValidationTest.h | 12 + 4 files changed, 306 insertions(+), 18 deletions(-) diff --git a/src/tests/unittests/validation/BufferValidationTests.cpp b/src/tests/unittests/validation/BufferValidationTests.cpp index d98a174e75..723f5a9f70 100644 --- a/src/tests/unittests/validation/BufferValidationTests.cpp +++ b/src/tests/unittests/validation/BufferValidationTests.cpp @@ -14,24 +14,271 @@ #include "ValidationTest.h" -class BufferValidationTest : public ValidationTest { +#include + +using namespace testing; + +class MockBufferMapReadCallback { + public: + MOCK_METHOD3(Call, void(nxtBufferMapReadStatus status, const uint32_t* ptr, nxtCallbackUserdata userdata)); }; -TEST_F(BufferValidationTest, Creation) { - // Success - nxt::Buffer buf0 = AssertWillBeSuccess(device.CreateBufferBuilder()) - .SetSize(4) - .SetAllowedUsage(nxt::BufferUsageBit::Uniform) - .GetResult(); - - // Test failure when specifying properties multiple times - nxt::Buffer buf1 = AssertWillBeError(device.CreateBufferBuilder()) - .SetSize(4) - .SetSize(3) - .GetResult(); - - // Test failure when properties are missing - nxt::Buffer buf2 = AssertWillBeError(device.CreateBufferBuilder()) - .SetSize(4) - .GetResult(); +static MockBufferMapReadCallback* mockBufferMapReadCallback = nullptr; +static void ToMockBufferMapReadCallback(nxtBufferMapReadStatus status, const void* ptr, nxtCallbackUserdata userdata) { + // Assume the data is uint32_t to make writing matchers easier + mockBufferMapReadCallback->Call(status, reinterpret_cast(ptr), userdata); +} + +class BufferValidationTest : public ValidationTest { + protected: + nxt::Buffer CreateMapReadBuffer(uint32_t size) { + return device.CreateBufferBuilder() + .SetSize(size) + .SetAllowedUsage(nxt::BufferUsageBit::MapRead) + .SetInitialUsage(nxt::BufferUsageBit::MapRead) + .GetResult(); + } + nxt::Buffer CreateSetSubDataBuffer(uint32_t size) { + return device.CreateBufferBuilder() + .SetSize(size) + .SetAllowedUsage(nxt::BufferUsageBit::MapWrite) + .SetInitialUsage(nxt::BufferUsageBit::MapWrite) + .GetResult(); + } + + nxt::Queue queue; + + private: + void SetUp() override { + ValidationTest::SetUp(); + + mockBufferMapReadCallback = new MockBufferMapReadCallback; + queue = device.CreateQueueBuilder().GetResult(); + } + + void TearDown() override { + delete mockBufferMapReadCallback; + + ValidationTest::TearDown(); + } +}; + +// Test case where creation should succeed +TEST_F(BufferValidationTest, CreationSuccess) { + // Success + { + nxt::Buffer buf = AssertWillBeSuccess(device.CreateBufferBuilder()) + .SetSize(4) + .SetAllowedUsage(nxt::BufferUsageBit::Uniform) + .SetInitialUsage(nxt::BufferUsageBit::Uniform) + .GetResult(); + } + + // Success, when no initial usage is set + { + nxt::Buffer buf = AssertWillBeSuccess(device.CreateBufferBuilder()) + .SetSize(4) + .SetAllowedUsage(nxt::BufferUsageBit::Uniform) + .GetResult(); + } +} + +// Test failure when specifying properties multiple times +TEST_F(BufferValidationTest, CreationDuplicates) { + // When size is specified multiple times + { + nxt::Buffer buf = AssertWillBeError(device.CreateBufferBuilder()) + .SetSize(4) + .SetSize(3) + .SetAllowedUsage(nxt::BufferUsageBit::Uniform) + .SetInitialUsage(nxt::BufferUsageBit::Uniform) + .GetResult(); + } + + // When allowed usage is specified multiple times + { + nxt::Buffer buf = AssertWillBeError(device.CreateBufferBuilder()) + .SetSize(4) + .SetAllowedUsage(nxt::BufferUsageBit::Vertex) + .SetAllowedUsage(nxt::BufferUsageBit::Uniform) + .SetInitialUsage(nxt::BufferUsageBit::Uniform) + .GetResult(); + } + + // When initial usage is specified multiple times + { + nxt::Buffer buf = AssertWillBeError(device.CreateBufferBuilder()) + .SetSize(4) + .SetAllowedUsage(nxt::BufferUsageBit::Uniform | nxt::BufferUsageBit::Vertex) + .SetInitialUsage(nxt::BufferUsageBit::Uniform) + .SetInitialUsage(nxt::BufferUsageBit::Vertex) + .GetResult(); + } +} + +// Test failure when the initial usage isn't a subset of the allowed usage +TEST_F(BufferValidationTest, CreationInitialNotSubsetOfAllowed) { + nxt::Buffer buf = AssertWillBeError(device.CreateBufferBuilder()) + .SetSize(4) + .SetAllowedUsage(nxt::BufferUsageBit::Uniform) + .SetInitialUsage(nxt::BufferUsageBit::Vertex) + .GetResult(); +} + +// Test failure when required properties are missing +TEST_F(BufferValidationTest, CreationMissingProperties) { + // When allowed usage is missing + { + nxt::Buffer buf = AssertWillBeError(device.CreateBufferBuilder()) + .SetSize(4) + .GetResult(); + } + + // When size is missing + { + nxt::Buffer buf = AssertWillBeError(device.CreateBufferBuilder()) + .SetAllowedUsage(nxt::BufferUsageBit::Vertex) + .GetResult(); + } +} + +// Test the success cause for mapping buffer for reading +TEST_F(BufferValidationTest, MapReadSuccess) { + nxt::Buffer buf = CreateMapReadBuffer(4); + + nxt::CallbackUserdata userdata = 40598; + buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, userdata); + + EXPECT_CALL(*mockBufferMapReadCallback, Call(NXT_BUFFER_MAP_READ_STATUS_SUCCESS, Ne(nullptr), userdata)) + .Times(1); + queue.Submit(0, nullptr); + + buf.Unmap(); +} + +// Test map reading out of range causes an error +TEST_F(BufferValidationTest, MapReadOutOfRange) { + nxt::Buffer buf = CreateMapReadBuffer(4); + + nxt::CallbackUserdata userdata = 40599; + EXPECT_CALL(*mockBufferMapReadCallback, Call(NXT_BUFFER_MAP_READ_STATUS_ERROR, nullptr, userdata)) + .Times(1); + + ASSERT_DEVICE_ERROR(buf.MapReadAsync(0, 5, ToMockBufferMapReadCallback, userdata)); +} + +// Test map reading a buffer with wrong current usage +TEST_F(BufferValidationTest, MapReadWrongUsage) { + nxt::Buffer buf = device.CreateBufferBuilder() + .SetSize(4) + .SetAllowedUsage(nxt::BufferUsageBit::MapRead | nxt::BufferUsageBit::TransferDst) + .SetInitialUsage(nxt::BufferUsageBit::TransferDst) + .GetResult(); + + nxt::CallbackUserdata userdata = 40600; + EXPECT_CALL(*mockBufferMapReadCallback, Call(NXT_BUFFER_MAP_READ_STATUS_ERROR, nullptr, userdata)) + .Times(1); + + ASSERT_DEVICE_ERROR(buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, userdata)); +} + +// Test map reading a buffer that is already mapped +TEST_F(BufferValidationTest, MapReadAlreadyMapped) { + nxt::Buffer buf = CreateMapReadBuffer(4); + + nxt::CallbackUserdata userdata1 = 40601; + buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, userdata1); + EXPECT_CALL(*mockBufferMapReadCallback, Call(NXT_BUFFER_MAP_READ_STATUS_SUCCESS, Ne(nullptr), userdata1)) + .Times(1); + + nxt::CallbackUserdata userdata2 = 40602; + EXPECT_CALL(*mockBufferMapReadCallback, Call(NXT_BUFFER_MAP_READ_STATUS_ERROR, nullptr, userdata2)) + .Times(1); + ASSERT_DEVICE_ERROR(buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, userdata2)); + + queue.Submit(0, nullptr); +} + +// Test unmapping before having the result gives UNKNOWN +TEST_F(BufferValidationTest, MapReadUnmapBeforeResult) { + nxt::Buffer buf = CreateMapReadBuffer(4); + + nxt::CallbackUserdata userdata = 40603; + buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, userdata); + + EXPECT_CALL(*mockBufferMapReadCallback, Call(NXT_BUFFER_MAP_READ_STATUS_UNKNOWN, nullptr, userdata)) + .Times(1); + buf.Unmap(); + + // Submitting the queue makes the null backend process map request, but the callback shouldn't + // be called again + queue.Submit(0, nullptr); +} + +// Test destroying the buffer before having the result gives UNKNOWN +// TODO(cwallez@chromium.org) currently this doesn't work because the buffer doesn't know +// when its external ref count reaches 0. +TEST_F(BufferValidationTest, DISABLED_MapReadDestroyBeforeResult) { + { + nxt::Buffer buf = CreateMapReadBuffer(4); + + nxt::CallbackUserdata userdata = 40604; + buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, userdata); + + EXPECT_CALL(*mockBufferMapReadCallback, Call(NXT_BUFFER_MAP_READ_STATUS_UNKNOWN, nullptr, userdata)) + .Times(1); + } + + // Submitting the queue makes the null backend process map request, but the callback shouldn't + // be called again + queue.Submit(0, nullptr); +} + +// When a request is cancelled with Unmap it might still be in flight, test doing a new request +// works as expected and we don't get the cancelled request's data. +TEST_F(BufferValidationTest, MapReadUnmapBeforeResultThenMapAgain) { + nxt::Buffer buf = CreateMapReadBuffer(4); + + nxt::CallbackUserdata userdata = 40605; + buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, userdata); + + EXPECT_CALL(*mockBufferMapReadCallback, Call(NXT_BUFFER_MAP_READ_STATUS_UNKNOWN, nullptr, userdata)) + .Times(1); + buf.Unmap(); + + userdata ++; + + buf.MapReadAsync(0, 4, ToMockBufferMapReadCallback, userdata); + + EXPECT_CALL(*mockBufferMapReadCallback, Call(NXT_BUFFER_MAP_READ_STATUS_SUCCESS, Ne(nullptr), userdata)) + .Times(1); + queue.Submit(0, nullptr); +} + +// Test the success case for Buffer::SetSubData +TEST_F(BufferValidationTest, SetSubDataSuccess) { + nxt::Buffer buf = CreateSetSubDataBuffer(4); + + uint32_t foo = 0; + buf.SetSubData(0, 1, &foo); +} + +// Test error case for SetSubData out of bounds +TEST_F(BufferValidationTest, SetSubDataOutOfBounds) { + nxt::Buffer buf = CreateSetSubDataBuffer(4); + + uint32_t foo = 0; + ASSERT_DEVICE_ERROR(buf.SetSubData(0, 2, &foo)); +} + +// Test error case for SetSubData with the wrong usage +TEST_F(BufferValidationTest, SetSubDataWrongUsage) { + nxt::Buffer buf = device.CreateBufferBuilder() + .SetSize(4) + .SetAllowedUsage(nxt::BufferUsageBit::MapWrite | nxt::BufferUsageBit::Vertex) + .SetInitialUsage(nxt::BufferUsageBit::Vertex) + .GetResult(); + + uint32_t foo = 0; + ASSERT_DEVICE_ERROR(buf.SetSubData(0, 1, &foo)); } diff --git a/src/tests/unittests/validation/DepthStencilStateValidationTests.cpp b/src/tests/unittests/validation/DepthStencilStateValidationTests.cpp index 9cff8ae2c9..b7f6aff31d 100644 --- a/src/tests/unittests/validation/DepthStencilStateValidationTests.cpp +++ b/src/tests/unittests/validation/DepthStencilStateValidationTests.cpp @@ -17,6 +17,7 @@ class DepthStencilStateValidationTest : public ValidationTest { }; +// Test cases where creation should succeed TEST_F(DepthStencilStateValidationTest, CreationSuccess) { // Success for setting all properties { @@ -46,6 +47,7 @@ TEST_F(DepthStencilStateValidationTest, CreationSuccess) { } } +// Test creation failure when specifying properties multiple times TEST_F(DepthStencilStateValidationTest, CreationDuplicates) { // Test failure when specifying depth write enabled multiple times { diff --git a/src/tests/unittests/validation/ValidationTest.cpp b/src/tests/unittests/validation/ValidationTest.cpp index 62ee5020c6..2bbd0a0365 100644 --- a/src/tests/unittests/validation/ValidationTest.cpp +++ b/src/tests/unittests/validation/ValidationTest.cpp @@ -29,6 +29,8 @@ ValidationTest::ValidationTest() { nxtSetProcs(&procs); device = nxt::Device::Acquire(cDevice); + + device.SetErrorCallback(ValidationTest::OnDeviceError, static_cast(reinterpret_cast(this))); } ValidationTest::~ValidationTest() { @@ -39,6 +41,8 @@ ValidationTest::~ValidationTest() { } void ValidationTest::TearDown() { + ASSERT_FALSE(expectError); + for (auto& expectation : expectations) { std::string name = expectation.debugName; if (name.empty()) { @@ -56,6 +60,29 @@ void ValidationTest::TearDown() { } } +void ValidationTest::StartExpectDeviceError() { + expectError = true; + error = false; +} +bool ValidationTest::EndExpectDeviceError() { + expectError = false; + return error; +} + +void ValidationTest::OnDeviceError(const char* message, nxtCallbackUserdata userdata) { + // Skip this one specific error that is raised when a builder is used after it got an error + // this is important because we don't want to wrap all creation tests in ASSERT_DEVICE_ERROR. + // Yes the error message is misleading. + if (std::string(message) == "Builder cannot be used after GetResult") { + return; + } + + auto self = reinterpret_cast(static_cast(userdata)); + ASSERT_TRUE(self->expectError) << "Got unexpected device error: " << message; + ASSERT_FALSE(self->error) << "Got two errors in expect block"; + self->error = true; +} + void ValidationTest::OnBuilderErrorStatus(nxtBuilderErrorStatus status, const char* message, nxt::CallbackUserdata userdata1, nxt::CallbackUserdata userdata2) { auto* self = reinterpret_cast(static_cast(userdata1)); size_t index = userdata2; diff --git a/src/tests/unittests/validation/ValidationTest.h b/src/tests/unittests/validation/ValidationTest.h index 82d5befc94..67d680356c 100644 --- a/src/tests/unittests/validation/ValidationTest.h +++ b/src/tests/unittests/validation/ValidationTest.h @@ -19,6 +19,11 @@ #include "nxt/nxtcpp.h" #include "nxt/nxtcpp_traits.h" +#define ASSERT_DEVICE_ERROR(statement) \ + StartExpectDeviceError(); \ + statement; \ + ASSERT_TRUE(EndExpectDeviceError()); + class ValidationTest : public testing::Test { public: ValidationTest(); @@ -38,12 +43,19 @@ class ValidationTest : public testing::Test { template Builder AssertWillBeError(Builder builder, std::string debugName = ""); + void StartExpectDeviceError(); + bool EndExpectDeviceError(); + void TearDown() override; protected: nxt::Device device; private: + static void OnDeviceError(const char* message, nxtCallbackUserdata userdata); + bool expectError = false; + bool error = false; + struct BuilderStatusExpectations { bool expectSuccess; std::string debugName;