Scarfski/filament
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Metal: correctly handle byte offsets for updateIndexBuffer and updateBufferObject (#4349)

Browse Source
This commit is contained in:
Ben Doherty
2021-07-20 21:03:00 -07:00
committed by GitHub
parent 67b53d0115
commit 0d8cdab4e4
8 changed files with 108 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
filament/backend/src/metal/MetalBuffer.h
+1 -1
View File
@@ -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
filament/backend/src/metal/MetalBuffer.mm
+6 -5
View File
@@ -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<uint8_t*>(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<uint8_t*>(mBufferPoolEntry->buffer.contents), src, size);
memcpy(static_cast<uint8_t*>(mBufferPoolEntry->buffer.contents) + byteOffset, src, size);
}
id<MTLBuffer> MetalBuffer::getGpuBufferForDraw(id<MTLCommandBuffer> cmdBuffer) noexcept {
filament/backend/src/metal/MetalDriver.mm
+1 -2
View File
@@ -679,9 +679,8 @@ uint8_t MetalDriver::getMaxDrawBuffers() {
void MetalDriver::updateIndexBuffer(Handle<HwIndexBuffer> ibh, BufferDescriptor&& data,
uint32_t byteOffset) {
assert_invariant(byteOffset == 0); // TODO: handle byteOffset for index buffers
auto* ib = handle_cast<MetalIndexBuffer>(ibh);
ib->buffer.copyIntoBuffer(data.buffer, data.size);
ib->buffer.copyIntoBuffer(data.buffer, data.size, byteOffset);
scheduleDestroy(std::move(data));
}
filament/backend/src/metal/MetalHandles.mm
+1 -2
View File
@@ -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,
filament/backend/src/opengl/OpenGLDriver.cpp
+1
View File
@@ -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) {
filament/backend/test/TrianglePrimitive.cpp
+12
View File
@@ -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);
filament/backend/test/TrianglePrimitive.h
+2 -1
View File
@@ -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:
filament/backend/test/test_BufferUpdates.cpp
+84 -2
View File
@@ -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<HwBufferObject> 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