Make SetSubData take size in uint8_t not uint32_t.

Also change data pointer to uint8_t*.
Add a test for a non-aligned SetSubData().
Implemented on all backends.
This commit is contained in:
Stephen White 2018-02-04 11:07:02 -05:00 committed by Corentin Wallez
parent ee66f25c4f
commit e5ae3274a3
27 changed files with 93 additions and 68 deletions

View File

@ -88,8 +88,8 @@ void initBuffers() {
.GetResult();
particleBuffers[i].SetSubData(0,
sizeof(Particle) * kNumParticles / sizeof(uint32_t),
reinterpret_cast<uint32_t*>(initialParticles.data()));
sizeof(Particle) * kNumParticles,
reinterpret_cast<uint8_t*>(initialParticles.data()));
}
}

View File

@ -45,7 +45,7 @@ void init() {
.SetInitialUsage(nxt::BufferUsageBit::TransferDst)
.SetSize(sizeof(s))
.GetResult();
buffer.SetSubData(0, sizeof(s) / sizeof(uint32_t), reinterpret_cast<uint32_t*>(&s));
buffer.SetSubData(0, sizeof(s), reinterpret_cast<uint8_t*>(&s));
nxt::BufferView view = buffer.CreateBufferViewBuilder()
.SetExtent(0, sizeof(s))

View File

@ -274,7 +274,7 @@ void frame() {
);
cameraBuffer.TransitionUsage(nxt::BufferUsageBit::TransferDst);
cameraBuffer.SetSubData(0, sizeof(CameraData) / sizeof(uint32_t), reinterpret_cast<uint32_t*>(&cameraData));
cameraBuffer.SetSubData(0, sizeof(CameraData), reinterpret_cast<uint8_t*>(&cameraData));
nxt::Texture backbuffer;
nxt::Framebuffer framebuffer;

View File

@ -96,7 +96,7 @@ void frame() {
if (s.b >= 1.0f) {s.b = 0.0f;}
buffer.TransitionUsage(nxt::BufferUsageBit::TransferDst);
buffer.SetSubData(0, sizeof(s) / sizeof(uint32_t), reinterpret_cast<uint32_t*>(&s));
buffer.SetSubData(0, sizeof(s), reinterpret_cast<uint8_t*>(&s));
nxt::Texture backbuffer;
nxt::Framebuffer framebuffer;

View File

@ -508,8 +508,8 @@ namespace {
// but the draw is queue level command that is pipelined. This causes bad rendering for models
// that use the same part multiple time.
material.uniformBuffer.SetSubData(0,
sizeof(u_transform_block) / sizeof(uint32_t),
reinterpret_cast<const uint32_t*>(&transforms));
sizeof(u_transform_block),
reinterpret_cast<const uint8_t*>(&transforms));
cmd.SetRenderPipeline(material.pipeline);
cmd.TransitionBufferUsage(material.uniformBuffer, nxt::BufferUsageBit::Uniform);
cmd.SetBindGroup(0, material.bindGroup0);

View File

@ -197,7 +197,7 @@
"args": [
{"name": "start", "type": "uint32_t"},
{"name": "count", "type": "uint32_t"},
{"name": "data", "type": "uint32_t", "annotation": "const*", "length": "count"}
{"name": "data", "type": "uint8_t", "annotation": "const*", "length": "count"}
]
},
{
@ -1252,6 +1252,9 @@
"void": {
"category": "native"
},
"uint8_t": {
"category": "native"
},
"uint32_t": {
"category": "native"
},

View File

@ -84,8 +84,8 @@ namespace backend {
}
}
void BufferBase::SetSubData(uint32_t start, uint32_t count, const uint32_t* data) {
if ((start + count) * sizeof(uint32_t) > GetSize()) {
void BufferBase::SetSubData(uint32_t start, uint32_t count, const uint8_t* data) {
if (start + count > GetSize()) {
mDevice->HandleError("Buffer subdata out of range");
return;
}

View File

@ -41,7 +41,7 @@ namespace backend {
// NXT API
BufferViewBuilder* CreateBufferViewBuilder();
void SetSubData(uint32_t start, uint32_t count, const uint32_t* data);
void SetSubData(uint32_t start, uint32_t count, const uint8_t* data);
void MapReadAsync(uint32_t start,
uint32_t size,
nxtBufferMapReadCallback callback,
@ -61,7 +61,7 @@ namespace backend {
void CallMapWriteCallback(uint32_t serial, nxtBufferMapAsyncStatus status, void* pointer);
private:
virtual void SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) = 0;
virtual void SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) = 0;
virtual void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t size) = 0;
virtual void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t size) = 0;
virtual void UnmapImpl() = 0;

View File

@ -152,9 +152,8 @@ namespace backend { namespace d3d12 {
}
}
void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) {
mDevice->GetResourceUploader()->BufferSubData(mResource, start * sizeof(uint32_t),
count * sizeof(uint32_t), data);
void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
mDevice->GetResourceUploader()->BufferSubData(mResource, start, count, data);
}
void Buffer::MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) {

View File

@ -42,7 +42,7 @@ namespace backend { namespace d3d12 {
ComPtr<ID3D12Resource> mResource;
// NXT API
void SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) override;
void SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override;
void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override;
void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override;
void UnmapImpl() override;

View File

@ -34,7 +34,7 @@ namespace backend { namespace metal {
void OnMapCommandSerialFinished(uint32_t mapSerial, uint32_t offset, bool isWrite);
private:
void SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) override;
void SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override;
void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override;
void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override;
void UnmapImpl() override;

View File

@ -49,10 +49,9 @@ namespace backend { namespace metal {
}
}
void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) {
void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
auto* uploader = ToBackend(GetDevice())->GetResourceUploader();
uploader->BufferSubData(mMtlBuffer, start * sizeof(uint32_t), count * sizeof(uint32_t),
data);
uploader->BufferSubData(mMtlBuffer, start, count, data);
}
void Buffer::MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t) {

View File

@ -140,7 +140,7 @@ namespace backend { namespace null {
}
}
void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) {
void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
ASSERT(start + count <= GetSize());
ASSERT(mBackingData);
memcpy(mBackingData.get() + start, data, count);

View File

@ -135,7 +135,7 @@ namespace backend { namespace null {
void MapReadOperationCompleted(uint32_t serial, void* ptr, bool isWrite);
private:
void SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) override;
void SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override;
void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override;
void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override;
void UnmapImpl() override;

View File

@ -30,9 +30,9 @@ namespace backend { namespace opengl {
return mBuffer;
}
void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) {
void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
glBindBuffer(GL_ARRAY_BUFFER, mBuffer);
glBufferSubData(GL_ARRAY_BUFFER, start * sizeof(uint32_t), count * sizeof(uint32_t), data);
glBufferSubData(GL_ARRAY_BUFFER, start, count, data);
}
void Buffer::MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) {

View File

@ -30,7 +30,7 @@ namespace backend { namespace opengl {
GLuint GetHandle() const;
private:
void SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) override;
void SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override;
void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override;
void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override;
void UnmapImpl() override;

View File

@ -184,9 +184,9 @@ namespace backend { namespace vulkan {
nullptr);
}
void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) {
void Buffer::SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) {
BufferUploader* uploader = ToBackend(GetDevice())->GetBufferUploader();
uploader->BufferSubData(mHandle, start * sizeof(uint32_t), count * sizeof(uint32_t), data);
uploader->BufferSubData(mHandle, start, count, data);
}
void Buffer::MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t /*count*/) {

View File

@ -40,7 +40,7 @@ namespace backend { namespace vulkan {
nxt::BufferUsageBit targetUsage) const;
private:
void SetSubDataImpl(uint32_t start, uint32_t count, const uint32_t* data) override;
void SetSubDataImpl(uint32_t start, uint32_t count, const uint8_t* data) override;
void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override;
void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) override;
void UnmapImpl() override;

View File

@ -430,6 +430,7 @@ namespace detail {
return testing::AssertionSuccess();
}
template class ExpectEq<uint8_t>;
template class ExpectEq<uint32_t>;
template class ExpectEq<RGBA8>;
}

View File

@ -26,6 +26,10 @@
#define EXPECT_BUFFER_U32_RANGE_EQ(expected, buffer, offset, count) \
AddBufferExpectation(__FILE__, __LINE__, buffer, offset, sizeof(uint32_t) * count, new detail::ExpectEq<uint32_t>(expected, count))
#define EXPECT_BUFFER_U8_EQ(expected, buffer, offset) \
AddBufferExpectation(__FILE__, __LINE__, buffer, offset, sizeof(uint8_t), \
new detail::ExpectEq<uint8_t>(expected))
// Test a pixel of the mip level 0 of a 2D texture.
#define EXPECT_PIXEL_RGBA8_EQ(expected, texture, x, y) \
AddTextureExpectation(__FILE__, __LINE__, texture, x, y, 1, 1, 0, sizeof(RGBA8), new detail::ExpectEq<RGBA8>(expected))
@ -166,6 +170,7 @@ namespace detail {
private:
std::vector<T> mExpected;
};
extern template class ExpectEq<uint8_t>;
extern template class ExpectEq<uint32_t>;
extern template class ExpectEq<RGBA8>;
}

View File

@ -28,10 +28,10 @@ TEST_P(BasicTests, BufferSetSubData) {
.SetInitialUsage(nxt::BufferUsageBit::TransferDst)
.GetResult();
uint32_t value = 3094587;
buffer.SetSubData(0, 1, &value);
uint8_t value = 187;
buffer.SetSubData(0, sizeof(value), &value);
EXPECT_BUFFER_U32_EQ(value, buffer, 0);
EXPECT_BUFFER_U8_EQ(value, buffer, 0);
}
NXT_INSTANTIATE_TEST(BasicTests, D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend)

View File

@ -41,20 +41,20 @@ class BufferMapReadTests : public NXTTest {
const void* mappedData = nullptr;
};
// Test that the simplest map read (one u32 at offset 0) works.
// Test that the simplest map read (one u8 at offset 0) works.
TEST_P(BufferMapReadTests, SmallReadAtZero) {
nxt::Buffer buffer = device.CreateBufferBuilder()
.SetSize(4)
.SetSize(1)
.SetAllowedUsage(nxt::BufferUsageBit::MapRead | nxt::BufferUsageBit::TransferDst)
.SetInitialUsage(nxt::BufferUsageBit::TransferDst)
.GetResult();
uint32_t myData = 2934875;
buffer.SetSubData(0, 1, &myData);
uint8_t myData = 187;
buffer.SetSubData(0, sizeof(myData), &myData);
buffer.TransitionUsage(nxt::BufferUsageBit::MapRead);
const void* mappedData = MapReadAsyncAndWait(buffer, 0, 4);
ASSERT_EQ(myData, *reinterpret_cast<const uint32_t*>(mappedData));
const void* mappedData = MapReadAsyncAndWait(buffer, 0, 1);
ASSERT_EQ(myData, *reinterpret_cast<const uint8_t*>(mappedData));
buffer.Unmap();
}
@ -67,12 +67,30 @@ TEST_P(BufferMapReadTests, SmallReadAtOffset) {
.SetInitialUsage(nxt::BufferUsageBit::TransferDst)
.GetResult();
uint32_t myData = 2934875;
buffer.SetSubData(2048 / sizeof(uint32_t), 1, &myData);
uint8_t myData = 234;
buffer.SetSubData(2048, sizeof(myData), &myData);
buffer.TransitionUsage(nxt::BufferUsageBit::MapRead);
const void* mappedData = MapReadAsyncAndWait(buffer, 2048, 4);
ASSERT_EQ(myData, *reinterpret_cast<const uint32_t*>(mappedData));
ASSERT_EQ(myData, *reinterpret_cast<const uint8_t*>(mappedData));
buffer.Unmap();
}
// Test mapping a buffer at an offset that's not uint32-aligned.
TEST_P(BufferMapReadTests, SmallReadAtUnalignedOffset) {
nxt::Buffer buffer = device.CreateBufferBuilder()
.SetSize(4000)
.SetAllowedUsage(nxt::BufferUsageBit::MapRead | nxt::BufferUsageBit::TransferDst)
.SetInitialUsage(nxt::BufferUsageBit::TransferDst)
.GetResult();
uint8_t myData = 213;
buffer.SetSubData(3, 1, &myData);
buffer.TransitionUsage(nxt::BufferUsageBit::MapRead);
const void* mappedData = MapReadAsyncAndWait(buffer, 3, 1);
ASSERT_EQ(myData, *reinterpret_cast<const uint8_t*>(mappedData));
buffer.Unmap();
}
@ -91,7 +109,7 @@ TEST_P(BufferMapReadTests, LargeRead) {
.SetInitialUsage(nxt::BufferUsageBit::TransferDst)
.GetResult();
buffer.SetSubData(0, static_cast<uint32_t>(kDataSize), myData.data());
buffer.SetSubData(0, kDataSize * sizeof(uint32_t), reinterpret_cast<uint8_t*>(myData.data()));
buffer.TransitionUsage(nxt::BufferUsageBit::MapRead);
const void* mappedData = MapReadAsyncAndWait(buffer, 0, static_cast<uint32_t>(kDataSize * sizeof(uint32_t)));
@ -185,18 +203,18 @@ NXT_INSTANTIATE_TEST(BufferMapWriteTests, D3D12Backend, MetalBackend, OpenGLBack
class BufferSetSubDataTests : public NXTTest {
};
// Test the simplest set sub data: setting one u32 at offset 0.
// Test the simplest set sub data: setting one u8 at offset 0.
TEST_P(BufferSetSubDataTests, SmallDataAtZero) {
nxt::Buffer buffer = device.CreateBufferBuilder()
.SetSize(4)
.SetSize(1)
.SetAllowedUsage(nxt::BufferUsageBit::TransferSrc | nxt::BufferUsageBit::TransferDst)
.SetInitialUsage(nxt::BufferUsageBit::TransferDst)
.GetResult();
uint32_t value = 298371;
buffer.SetSubData(0, 1, &value);
uint8_t value = 171;
buffer.SetSubData(0, sizeof(value), &value);
EXPECT_BUFFER_U32_EQ(value, buffer, 0);
EXPECT_BUFFER_U8_EQ(value, buffer, 0);
}
// Test that SetSubData offset works.
@ -208,10 +226,10 @@ TEST_P(BufferSetSubDataTests, SmallDataAtOffset) {
.GetResult();
constexpr uint32_t kOffset = 2000;
uint32_t value = 298371;
buffer.SetSubData(kOffset / 4, 1, &value);
uint8_t value = 231;
buffer.SetSubData(kOffset, sizeof(value), &value);
EXPECT_BUFFER_U32_EQ(value, buffer, kOffset);
EXPECT_BUFFER_U8_EQ(value, buffer, kOffset);
}
// Stress test for many calls to SetSubData
@ -233,7 +251,7 @@ TEST_P(BufferSetSubDataTests, ManySetSubData) {
std::vector<uint32_t> expectedData;
for (uint32_t i = 0; i < kElements; ++i) {
buffer.SetSubData(i, 1, &i);
buffer.SetSubData(i * sizeof(uint32_t), sizeof(i), reinterpret_cast<uint8_t*>(&i));
expectedData.push_back(i);
}
@ -255,7 +273,7 @@ TEST_P(BufferSetSubDataTests, LargeSetSubData) {
expectedData.push_back(i);
}
buffer.SetSubData(0, kElements, expectedData.data());
buffer.SetSubData(0, kElements * sizeof(uint32_t), reinterpret_cast<uint8_t*>(expectedData.data()));
EXPECT_BUFFER_U32_RANGE_EQ(expectedData.data(), buffer, 0, kElements);
}

View File

@ -107,7 +107,7 @@ class CopyTests_T2B : public CopyTests {
.SetInitialUsage(nxt::BufferUsageBit::TransferDst)
.GetResult();
std::vector<RGBA8> emptyData(bufferSpec.size / kBytesPerTexel);
buffer.SetSubData(0, static_cast<uint32_t>(emptyData.size()), reinterpret_cast<const uint32_t*>(emptyData.data()));
buffer.SetSubData(0, static_cast<uint32_t>(emptyData.size() * sizeof(RGBA8)), reinterpret_cast<const uint8_t*>(emptyData.data()));
// Copy the region [(`x`, `y`), (`x + copyWidth, `y + copyWidth`)] from the `level` mip into the buffer at the specified `offset` and `rowPitch`
commands[1] = device.CreateCommandBufferBuilder()
@ -158,7 +158,7 @@ protected:
.GetResult();
std::vector<RGBA8> bufferData(bufferSpec.size / kBytesPerTexel);
FillBufferData(bufferData.data(), bufferData.size());
buffer.SetSubData(0, static_cast<uint32_t>(bufferData.size()), reinterpret_cast<const uint32_t*>(bufferData.data()));
buffer.SetSubData(0, static_cast<uint32_t>(bufferData.size() * sizeof(RGBA8)), reinterpret_cast<const uint8_t*>(bufferData.data()));
// Create a texture that is `width` x `height` with (`level` + 1) mip levels.
nxt::Texture texture = device.CreateTextureBuilder()

View File

@ -36,7 +36,7 @@ class PushConstantTest: public NXTTest {
.SetInitialUsage(nxt::BufferUsageBit::TransferDst)
.GetResult();
uint32_t one = 1;
buf1.SetSubData(0, 1, &one);
buf1.SetSubData(0, sizeof(one), reinterpret_cast<uint8_t*>(&one));
nxt::Buffer buf2 = device.CreateBufferBuilder()
.SetSize(4)

View File

@ -487,28 +487,28 @@ TEST_F(BufferValidationTest, DestroyInsideMapWriteCallback) {
// Test the success case for Buffer::SetSubData
TEST_F(BufferValidationTest, SetSubDataSuccess) {
nxt::Buffer buf = CreateSetSubDataBuffer(4);
nxt::Buffer buf = CreateSetSubDataBuffer(1);
uint32_t foo = 0;
buf.SetSubData(0, 1, &foo);
uint8_t foo = 0;
buf.SetSubData(0, sizeof(foo), &foo);
}
// Test error case for SetSubData out of bounds
TEST_F(BufferValidationTest, SetSubDataOutOfBounds) {
nxt::Buffer buf = CreateSetSubDataBuffer(4);
nxt::Buffer buf = CreateSetSubDataBuffer(1);
uint32_t foo = 0;
uint8_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)
.SetSize(1)
.SetAllowedUsage(nxt::BufferUsageBit::TransferDst | nxt::BufferUsageBit::Vertex)
.SetInitialUsage(nxt::BufferUsageBit::Vertex)
.GetResult();
uint32_t foo = 0;
ASSERT_DEVICE_ERROR(buf.SetSubData(0, 1, &foo));
uint8_t foo = 0;
ASSERT_DEVICE_ERROR(buf.SetSubData(0, sizeof(foo), &foo));
}

View File

@ -35,16 +35,16 @@ TEST_F(UsageValidationTest, UsageAfterCommandBuffer) {
// Should we make an end2end test that tests this as well?
nxt::Buffer buf = device.CreateBufferBuilder()
.SetSize(4)
.SetSize(1)
.SetAllowedUsage(nxt::BufferUsageBit::TransferDst | nxt::BufferUsageBit::Vertex)
.SetInitialUsage(nxt::BufferUsageBit::TransferDst)
.GetResult();
uint32_t foo = 0;
buf.SetSubData(0, 1, &foo);
uint8_t foo = 0;
buf.SetSubData(0, sizeof(foo), &foo);
buf.TransitionUsage(nxt::BufferUsageBit::Vertex);
ASSERT_DEVICE_ERROR(buf.SetSubData(0, 1, &foo));
ASSERT_DEVICE_ERROR(buf.SetSubData(0, sizeof(foo), &foo));
nxt::CommandBuffer cmdbuf = device.CreateCommandBufferBuilder()
.TransitionBufferUsage(buf, nxt::BufferUsageBit::TransferDst)
@ -52,5 +52,5 @@ TEST_F(UsageValidationTest, UsageAfterCommandBuffer) {
queue.Submit(1, &cmdbuf);
// buf should be in TransferDst usage
buf.SetSubData(0, 1, &foo);
buf.SetSubData(0, sizeof(foo), &foo);
}

View File

@ -106,7 +106,7 @@ namespace utils {
.SetInitialUsage(nxt::BufferUsageBit::TransferDst)
.SetSize(size)
.GetResult();
buffer.SetSubData(0, size / sizeof(uint32_t), reinterpret_cast<const uint32_t*>(data));
buffer.SetSubData(0, size, reinterpret_cast<const uint8_t*>(data));
buffer.FreezeUsage(usage);
return buffer;
}