From d76cf643c5da85fc29383a64d5cb59df8e4d206a Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Wed, 25 Oct 2023 12:14:02 -0400 Subject: [PATCH] Improve Metal vertex buffer bindings (#7293) --- .../include/private/backend/HandleAllocator.h | 4 +- filament/backend/src/metal/MetalDriver.mm | 25 +++--- filament/backend/src/metal/MetalHandles.h | 14 ++++ filament/backend/src/metal/MetalHandles.mm | 81 +++++++++++++------ 4 files changed, 83 insertions(+), 41 deletions(-) diff --git a/filament/backend/include/private/backend/HandleAllocator.h b/filament/backend/include/private/backend/HandleAllocator.h index 95481f5484e..610ad3729e2 100644 --- a/filament/backend/include/private/backend/HandleAllocator.h +++ b/filament/backend/include/private/backend/HandleAllocator.h @@ -33,7 +33,7 @@ #define HandleAllocatorGL HandleAllocator<16, 64, 208> #define HandleAllocatorVK HandleAllocator<16, 64, 880> -#define HandleAllocatorMTL HandleAllocator<16, 64, 576> +#define HandleAllocatorMTL HandleAllocator<16, 64, 584> namespace filament::backend { @@ -256,6 +256,7 @@ class HandleAllocator { HandleBase::HandleId allocateHandle() noexcept { if constexpr (SIZE <= P0) { return allocateHandleInPool(); } if constexpr (SIZE <= P1) { return allocateHandleInPool(); } + static_assert(SIZE <= P2); return allocateHandleInPool(); } @@ -266,6 +267,7 @@ class HandleAllocator { } else if constexpr (SIZE <= P1) { deallocateHandleFromPool(id); } else { + static_assert(SIZE <= P2); deallocateHandleFromPool(id); } } diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index c88abe34d16..1666edf8cf4 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -1686,26 +1686,23 @@ // Bind the user vertex buffers. - MetalBuffer* buffers[MAX_VERTEX_BUFFER_COUNT] = {}; + MetalBuffer* vertexBuffers[MAX_VERTEX_BUFFER_COUNT] = {}; size_t vertexBufferOffsets[MAX_VERTEX_BUFFER_COUNT] = {}; - size_t bufferIndex = 0; + size_t maxBufferIndex = 0; auto vb = primitive->vertexBuffer; - for (uint32_t attributeIndex = 0; attributeIndex < vb->attributes.size(); attributeIndex++) { - const auto& attribute = vb->attributes[attributeIndex]; - if (attribute.buffer == Attribute::BUFFER_UNUSED) { - continue; - } - - assert_invariant(vb->buffers[attribute.buffer]); - buffers[bufferIndex] = vb->buffers[attribute.buffer]; - vertexBufferOffsets[bufferIndex] = attribute.offset; - bufferIndex++; + for (auto m : primitive->bufferMapping) { + assert_invariant( + m.bufferArgumentIndex >= USER_VERTEX_BUFFER_BINDING_START && + m.bufferArgumentIndex < USER_VERTEX_BUFFER_BINDING_START + MAX_VERTEX_BUFFER_COUNT); + size_t vertexBufferIndex = m.bufferArgumentIndex - USER_VERTEX_BUFFER_BINDING_START; + vertexBuffers[vertexBufferIndex] = vb->buffers[m.sourceBufferIndex]; + maxBufferIndex = std::max(maxBufferIndex, vertexBufferIndex); } - const auto bufferCount = bufferIndex; + const auto bufferCount = maxBufferIndex + 1; MetalBuffer::bindBuffers(getPendingCommandBuffer(mContext), mContext->currentRenderPassEncoder, - USER_VERTEX_BUFFER_BINDING_START, MetalBuffer::Stage::VERTEX, buffers, + USER_VERTEX_BUFFER_BINDING_START, MetalBuffer::Stage::VERTEX, vertexBuffers, vertexBufferOffsets, bufferCount); // Bind the zero buffer, used for missing vertex attributes. diff --git a/filament/backend/src/metal/MetalHandles.h b/filament/backend/src/metal/MetalHandles.h index 97a7085c03c..fdc4a5e6a98 100644 --- a/filament/backend/src/metal/MetalHandles.h +++ b/filament/backend/src/metal/MetalHandles.h @@ -160,6 +160,7 @@ struct MetalIndexBuffer : public HwIndexBuffer { }; struct MetalRenderPrimitive : public HwRenderPrimitive { + MetalRenderPrimitive(); void setBuffers(MetalVertexBuffer* vertexBuffer, MetalIndexBuffer* indexBuffer); // The pointers to MetalVertexBuffer and MetalIndexBuffer are "weak". // The MetalVertexBuffer and MetalIndexBuffer must outlive the MetalRenderPrimitive. @@ -169,6 +170,19 @@ struct MetalRenderPrimitive : public HwRenderPrimitive { // This struct is used to create the pipeline description to describe vertex assembly. VertexDescription vertexDescription = {}; + + struct Entry { + uint8_t sourceBufferIndex = 0; + uint8_t stride = 0; + // maps to -> + uint8_t bufferArgumentIndex = 0; + + Entry(uint8_t sourceBufferIndex, uint8_t stride, uint8_t bufferArgumentIndex) + : sourceBufferIndex(sourceBufferIndex), + stride(stride), + bufferArgumentIndex(bufferArgumentIndex) {} + }; + utils::FixedCapacityVector bufferMapping; }; class MetalProgram : public HwProgram { diff --git a/filament/backend/src/metal/MetalHandles.mm b/filament/backend/src/metal/MetalHandles.mm index 9f4625123f9..b866053e167 100644 --- a/filament/backend/src/metal/MetalHandles.mm +++ b/filament/backend/src/metal/MetalHandles.mm @@ -299,6 +299,9 @@ void presentDrawable(bool presentFrame, void* user) { uint32_t indexCount) : HwIndexBuffer(elementSize, indexCount), buffer(context, BufferObjectBinding::VERTEX, usage, elementSize * indexCount, true) { } +MetalRenderPrimitive::MetalRenderPrimitive() + : bufferMapping(utils::FixedCapacityVector::with_capacity(MAX_VERTEX_BUFFER_COUNT)) {} + void MetalRenderPrimitive::setBuffers(MetalVertexBuffer* vertexBuffer, MetalIndexBuffer* indexBuffer) { this->vertexBuffer = vertexBuffer; @@ -306,44 +309,70 @@ void presentDrawable(bool presentFrame, void* user) { const size_t attributeCount = vertexBuffer->attributes.size(); + auto& mapping = bufferMapping; + mapping.clear(); vertexDescription = {}; - // Each attribute gets its own vertex buffer, starting at logical buffer 1. - uint32_t bufferIndex = 1; + // Set the layout for the zero buffer, which unused attributes are mapped to. + vertexDescription.layouts[ZERO_VERTEX_BUFFER_LOGICAL_INDEX] = { + .step = MTLVertexStepFunctionConstant, .stride = 16 + }; + + // Here we map each source buffer to a Metal buffer argument. + // Each attribute has a source buffer, offset, and stride. + // Two source buffers with the same index and stride can share the same Metal buffer argument + // index. + // + // The source buffer is the buffer index that the Filament client sets. + // * source buffer + // .attribute(VertexAttribute::POSITION, 0, VertexBuffer::AttributeType::FLOAT2, 0, 12) + // .attribute(VertexAttribute::UV, 0, VertexBuffer::AttributeType::HALF2, 8, 12) + // .attribute(VertexAttribute::COLOR, 1, VertexBuffer::AttributeType::UBYTE4, 0, 4) + + auto allocateOrGetBufferArgumentIndex = + [&mapping, currentBufferArgumentIndex = USER_VERTEX_BUFFER_BINDING_START, this]( + auto sourceBuffer, auto sourceBufferStride) mutable -> uint8_t { + auto match = [&](const auto& e) { + return e.sourceBufferIndex == sourceBuffer && e.stride == sourceBufferStride; + }; + if (auto it = std::find_if(mapping.begin(), mapping.end(), match); it != mapping.end()) { + return it->bufferArgumentIndex; + } else { + auto bufferArgumentIndex = currentBufferArgumentIndex++; + mapping.emplace_back(sourceBuffer, sourceBufferStride, bufferArgumentIndex); + vertexDescription.layouts[bufferArgumentIndex] = { + .step = MTLVertexStepFunctionPerVertex, .stride = sourceBufferStride + }; + return bufferArgumentIndex; + } + }; + for (uint32_t attributeIndex = 0; attributeIndex < attributeCount; attributeIndex++) { const auto& attribute = vertexBuffer->attributes[attributeIndex]; - if (attribute.buffer == Attribute::BUFFER_UNUSED) { - const uint8_t flags = attribute.flags; - const MTLVertexFormat format = (flags & Attribute::FLAG_INTEGER_TARGET) ? - MTLVertexFormatUInt4 : MTLVertexFormatFloat4; - // If the attribute is not enabled, bind it to the zero buffer. It's a Metal error for a - // shader to read from missing vertex attributes. + // If the attribute is unused, bind it to the zero buffer. It's a Metal error for a shader + // to read from missing vertex attributes. + if (attribute.buffer == Attribute::BUFFER_UNUSED) { + const MTLVertexFormat format = (attribute.flags & Attribute::FLAG_INTEGER_TARGET) + ? MTLVertexFormatUInt4 + : MTLVertexFormatFloat4; vertexDescription.attributes[attributeIndex] = { - .format = format, - .buffer = ZERO_VERTEX_BUFFER_LOGICAL_INDEX, - .offset = 0 - }; - vertexDescription.layouts[ZERO_VERTEX_BUFFER_LOGICAL_INDEX] = { - .step = MTLVertexStepFunctionConstant, - .stride = 16 + .format = format, .buffer = ZERO_VERTEX_BUFFER_LOGICAL_INDEX, .offset = 0 }; continue; } + // Map the source buffer and stride of this attribute to a Metal buffer argument. + auto bufferArgumentIndex = + allocateOrGetBufferArgumentIndex(attribute.buffer, attribute.stride); + vertexDescription.attributes[attributeIndex] = { - .format = getMetalFormat(attribute.type, - attribute.flags & Attribute::FLAG_NORMALIZED), - .buffer = bufferIndex, - .offset = 0 + .format = getMetalFormat( + attribute.type, attribute.flags & Attribute::FLAG_NORMALIZED), + .buffer = uint32_t(bufferArgumentIndex), + .offset = attribute.offset }; - vertexDescription.layouts[bufferIndex] = { - .step = MTLVertexStepFunctionPerVertex, - .stride = attribute.stride - }; - - bufferIndex++; - }; + } } MetalProgram::MetalProgram(MetalContext& context, Program&& program) noexcept