diff --git a/filament/backend/src/metal/MetalBuffer.h b/filament/backend/src/metal/MetalBuffer.h index b48ccdd9d8..1c32515044 100644 --- a/filament/backend/src/metal/MetalBuffer.h +++ b/filament/backend/src/metal/MetalBuffer.h @@ -41,7 +41,7 @@ public: * Update the buffer with data inside src. Potentially allocates a new buffer allocation to hold * the bytes which will be released when the current frame is finished. */ - void copyIntoBuffer(void* src, size_t size); + void copyIntoBuffer(void* src, size_t size, size_t byteOffset = 0); /** * Denotes that this buffer is used for a draw call ensuring that its allocation remains valid diff --git a/filament/backend/src/metal/MetalBuffer.mm b/filament/backend/src/metal/MetalBuffer.mm index 8363e19070..57bf6bdf83 100644 --- a/filament/backend/src/metal/MetalBuffer.mm +++ b/filament/backend/src/metal/MetalBuffer.mm @@ -44,16 +44,17 @@ MetalBuffer::~MetalBuffer() { } } -void MetalBuffer::copyIntoBuffer(void* src, size_t size) { +void MetalBuffer::copyIntoBuffer(void* src, size_t size, size_t byteOffset) { if (size <= 0) { return; } - ASSERT_PRECONDITION(size <= mBufferSize, "Attempting to copy %d bytes into a buffer of size %d", - size, mBufferSize); + ASSERT_PRECONDITION(size + byteOffset <= mBufferSize, + "Attempting to copy %d bytes into a buffer of size %d at offset %d", + size, mBufferSize, byteOffset); // Either copy into the Metal buffer or into our cpu buffer. if (mCpuBuffer) { - memcpy(mCpuBuffer, src, size); + memcpy(static_cast(mCpuBuffer) + byteOffset, src, size); return; } @@ -64,7 +65,7 @@ void MetalBuffer::copyIntoBuffer(void* src, size_t size) { } mBufferPoolEntry = mContext.bufferPool->acquireBuffer(mBufferSize); - memcpy(static_cast(mBufferPoolEntry->buffer.contents), src, size); + memcpy(static_cast(mBufferPoolEntry->buffer.contents) + byteOffset, src, size); } id MetalBuffer::getGpuBufferForDraw(id cmdBuffer) noexcept { diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index 720724cab1..72829ed163 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -679,9 +679,8 @@ uint8_t MetalDriver::getMaxDrawBuffers() { void MetalDriver::updateIndexBuffer(Handle ibh, BufferDescriptor&& data, uint32_t byteOffset) { - assert_invariant(byteOffset == 0); // TODO: handle byteOffset for index buffers auto* ib = handle_cast(ibh); - ib->buffer.copyIntoBuffer(data.buffer, data.size); + ib->buffer.copyIntoBuffer(data.buffer, data.size, byteOffset); scheduleDestroy(std::move(data)); } diff --git a/filament/backend/src/metal/MetalHandles.mm b/filament/backend/src/metal/MetalHandles.mm index cb030f501b..e684c58174 100644 --- a/filament/backend/src/metal/MetalHandles.mm +++ b/filament/backend/src/metal/MetalHandles.mm @@ -271,8 +271,7 @@ MetalBufferObject::MetalBufferObject(MetalContext& context, uint32_t byteCount) : HwBufferObject(byteCount), buffer(context, byteCount) {} void MetalBufferObject::updateBuffer(void* data, size_t size, uint32_t byteOffset) { - assert_invariant(byteOffset + size <= byteCount); - buffer.copyIntoBuffer(data, size); + buffer.copyIntoBuffer(data, size, byteOffset); } MetalVertexBuffer::MetalVertexBuffer(MetalContext& context, uint8_t bufferCount, diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index fd6f396173..0e204a353a 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -1643,6 +1643,7 @@ void OpenGLDriver::updateBufferObject( if (bo->gl.binding == GL_UNIFORM_BUFFER) { // TODO: use updateBuffer() for all types of buffer? Make sure GL supports that. + // TODO: take byteOffset into account. updateBuffer(bo, bd, (uint32_t)gl.gets.uniform_buffer_offset_alignment); } else { if (bo->gl.binding == GL_ARRAY_BUFFER) { diff --git a/filament/backend/test/TrianglePrimitive.cpp b/filament/backend/test/TrianglePrimitive.cpp index 0b6c3656ba..d83d42be65 100644 --- a/filament/backend/test/TrianglePrimitive.cpp +++ b/filament/backend/test/TrianglePrimitive.cpp @@ -96,6 +96,18 @@ void TrianglePrimitive::updateIndices(const short indices[3]) noexcept { mDriverApi.updateIndexBuffer(mIndexBuffer, std::move(bufferDesc), 0); } +void TrianglePrimitive::updateIndices(const short* indices, int count, int offset) noexcept { + void* buffer = malloc(sizeof(short) * count); + short* indexBuffer = (short*) buffer; + std::copy(indices, indices + count, indexBuffer); + + BufferDescriptor bufferDesc(indexBuffer, sizeof(short) * count, + [] (void* buffer, size_t size, void* user) { + free(buffer); + }); + mDriverApi.updateIndexBuffer(mIndexBuffer, std::move(bufferDesc), offset * sizeof(short)); +} + TrianglePrimitive::~TrianglePrimitive() { mDriverApi.destroyBufferObject(mBufferObject); mDriverApi.destroyVertexBuffer(mVertexBuffer); diff --git a/filament/backend/test/TrianglePrimitive.h b/filament/backend/test/TrianglePrimitive.h index 3bff8799a3..a808975581 100644 --- a/filament/backend/test/TrianglePrimitive.h +++ b/filament/backend/test/TrianglePrimitive.h @@ -41,7 +41,8 @@ public: PrimitiveHandle getRenderPrimitive() const noexcept; void updateVertices(const filament::math::float2 vertices[3]) noexcept; - void updateIndices(const short indices[3]) noexcept; + void updateIndices(const short* indices) noexcept; + void updateIndices(const short* indices, int count, int offset) noexcept; private: diff --git a/filament/backend/test/test_BufferUpdates.cpp b/filament/backend/test/test_BufferUpdates.cpp index 28c5abfd9f..491ce716d7 100644 --- a/filament/backend/test/test_BufferUpdates.cpp +++ b/filament/backend/test/test_BufferUpdates.cpp @@ -40,8 +40,16 @@ std::string fragment (R"(#version 450 core layout(location = 0) out vec4 fragColor; +uniform Params { + highp vec4 padding[4]; // offset of 64 bytes + + highp float red; + highp float green; + highp float blue; +} params; + void main() { - fragColor = vec4(1.0); + fragColor = vec4(params.red, params.green, params.blue, 1.0f); } )"); @@ -107,8 +115,11 @@ TEST_F(BackendTest, VertexBufferUpdate) { if (updateIndices) { if (triangleIndex % 2 == 0) { + // Upload each index separately, to test offsets. const short i[3] {0, 1, 2}; - triangle.updateIndices(i); + triangle.updateIndices(i + 0, 1, 0); + triangle.updateIndices(i + 1, 1, 1); + triangle.updateIndices(i + 2, 1, 2); } else { // This effectively hides this triangle. const short i[3] {0, 0, 0}; @@ -135,4 +146,75 @@ TEST_F(BackendTest, VertexBufferUpdate) { executeCommands(); } +struct MaterialParams { + float red; + float green; + float blue; +}; + +static void uploadUniforms(DriverApi& dapi, Handle ubh, MaterialParams params) { + MaterialParams* tmp = new MaterialParams(params); + auto cb = [](void* buffer, size_t size, void* user) { + MaterialParams* sp = (MaterialParams*) buffer; + delete sp; + }; + BufferDescriptor bd(tmp, sizeof(MaterialParams), cb); + dapi.updateBufferObject(ubh, std::move(bd), 0); +} + +TEST_F(BackendTest, BufferObjectUpdateWithOffset) { + // Create a platform-specific SwapChain and make it current. + auto swapChain = createSwapChain(); + getDriverApi().makeCurrent(swapChain, swapChain); + + // Create a program. + ShaderGenerator shaderGen(vertex, fragment, sBackend, sIsMobilePlatform); + Program p = shaderGen.getProgram(); + p.setUniformBlock(1, utils::CString("params")); + auto program = getDriverApi().createProgram(std::move(p)); + + // Create a uniform buffer. + auto ubuffer = getDriverApi().createBufferObject(sizeof(MaterialParams) + 64, + BufferObjectBinding::UNIFORM, BufferUsage::STATIC); + getDriverApi().bindUniformBuffer(0, ubuffer); + + // Upload uniforms. + { + MaterialParams params { + .red = 1.0f, + .green = 0.0f, + .blue = 0.5f + }; + MaterialParams* tmp = new MaterialParams(params); + auto cb = [](void* buffer, size_t size, void* user) { + MaterialParams* sp = (MaterialParams*) buffer; + delete sp; + }; + BufferDescriptor bd(tmp, sizeof(MaterialParams), cb); + getDriverApi().updateBufferObject(ubuffer, std::move(bd), 64); + } + + auto defaultRenderTarget = getDriverApi().createDefaultRenderTarget(0); + + renderTriangle(defaultRenderTarget, swapChain, program); + + static const uint32_t expectedHash = 2000773999; + readPixelsAndAssertHash("BufferObjectUpdateWithOffset", 512, 512, defaultRenderTarget, expectedHash); + + getDriverApi().flush(); + getDriverApi().commit(swapChain); + getDriverApi().endFrame(0); + + getDriverApi().destroyProgram(program); + getDriverApi().destroySwapChain(swapChain); + getDriverApi().destroyRenderTarget(defaultRenderTarget); + + // This ensures all driver commands have finished before exiting the test. + getDriverApi().finish(); + + executeCommands(); + + getDriver().purge(); +} + } // namespace test