Skip to content

Metal: Support vertex stride 0 and null buffers, and fix resource release race #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DarioSamo merged 5 commits into from
Jul 28, 2025
Merged

Metal: Support vertex stride 0 and null buffers, and fix resource release race #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a5a7760
Fix race on resource release + fixes for unleashed
tanmaysachan Apr 2, 2025
9e867e3
Add nullBuffer and stride=0 support
tanmaysachan Apr 2, 2025
a20b619
Use array instead of vector for vertex slot tracking.
squidbus Jul 28, 2025
62089d8
Remove extra comment
squidbus Jul 28, 2025
cefce65
Fix slotUsed initialization
squidbus Jul 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 37 additions & 13 deletions plume_metal.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ namespace plume {
static constexpr size_t PUSH_CONSTANT_MAX_INDEX = 15;
static constexpr size_t VERTEX_BUFFER_MAX_INDEX = 30;

static constexpr uint32_t COLOR_COUNT = DESCRIPTOR_SET_MAX_INDEX;
static constexpr uint32_t DEPTH_INDEX = COLOR_COUNT;
static constexpr uint32_t STENCIL_INDEX = DEPTH_INDEX + 1;
static constexpr uint32_t ATTACHMENT_COUNT = STENCIL_INDEX + 1;

// MARK: - Prototypes

MTL::PixelFormat mapPixelFormat(RenderFormat format);
Expand Down Expand Up @@ -1435,9 +1430,17 @@ namespace plume {
// Set index right after push constants, clamp at Metal's limit of 31
const uint32_t vertexBufferIndex = std::min(PUSH_CONSTANT_MAX_INDEX + 1 + inputSlot.index, VERTEX_BUFFER_MAX_INDEX);
MTL::VertexBufferLayoutDescriptor *layout = vertexDescriptor->layouts()->object(vertexBufferIndex);
layout->setStride(inputSlot.stride);
layout->setStepFunction(mapVertexStepFunction(inputSlot.classification));
layout->setStepRate((layout->stepFunction() == MTL::VertexStepFunctionPerInstance) ? inputSlot.stride : 1);
if (inputSlot.stride == 0) {
// Metal does not support stride 0, we must provide a
// substitute "null" buffer to match behaviour of other robust APIs.
layout->setStride(1);
layout->setStepFunction(MTL::VertexStepFunctionConstant);
layout->setStepRate(0);
} else {
layout->setStride(inputSlot.stride);
layout->setStepFunction(mapVertexStepFunction(inputSlot.classification));
layout->setStepRate((layout->stepFunction() == MTL::VertexStepFunctionPerInstance) ? inputSlot.stride : 1);
}
}

for (uint32_t i = 0; i < desc.inputElementsCount; i++) {
Expand Down Expand Up @@ -1605,6 +1608,10 @@ namespace plume {
}

MetalDescriptorSet::~MetalDescriptorSet() {
for (const auto resource: toReleaseOnDestruction) {
resource->release();
}

for (const auto &entry : resourceEntries) {
if (entry.resource != nullptr) {
entry.resource->release();
Expand Down Expand Up @@ -1700,8 +1707,7 @@ namespace plume {

if (dtype != MTL::DataTypeSampler) {
if (resourceEntries[descriptorIndex].resource != nullptr) {
resourceEntries[descriptorIndex].resource->release();
resourceEntries[descriptorIndex].resource = nullptr;
toReleaseOnDestruction.push_back(resourceEntries[descriptorIndex].resource);
}
}

Expand Down Expand Up @@ -2185,7 +2191,7 @@ namespace plume {
pushConstants[rangeIndex].binding = range.binding;
pushConstants[rangeIndex].set = range.set;
pushConstants[rangeIndex].offset = range.offset;
pushConstants[rangeIndex].size = range.size;
pushConstants[rangeIndex].size = alignUp(range.size);
pushConstants[rangeIndex].stageFlags = range.stageFlags;

dirtyComputeState.pushConstants = 1;
Expand Down Expand Up @@ -2236,7 +2242,7 @@ namespace plume {
pushConstants[rangeIndex].binding = range.binding;
pushConstants[rangeIndex].set = range.set;
pushConstants[rangeIndex].offset = range.offset;
pushConstants[rangeIndex].size = range.size;
pushConstants[rangeIndex].size = alignUp(range.size);
pushConstants[rangeIndex].stageFlags = range.stageFlags;

dirtyGraphicsState.pushConstants = 1;
Expand Down Expand Up @@ -2291,9 +2297,14 @@ namespace plume {
// Check for changes in bindings
for (uint32_t i = 0; i < viewCount; i++) {
const MetalBuffer* interfaceBuffer = static_cast<const MetalBuffer*>(views[i].buffer.ref);
const uint64_t newOffset = views[i].buffer.offset;
uint64_t newOffset = views[i].buffer.offset;
const uint32_t newIndex = startSlot + i;

if (interfaceBuffer == nullptr) {
interfaceBuffer = static_cast<const MetalBuffer*>(queue->device->nullBuffer.get());
newOffset = 0;
}

vertexBuffers[i] = interfaceBuffer->mtl;
vertexBufferOffsets[i] = newOffset;
vertexBufferIndices[i] = newIndex;
Expand Down Expand Up @@ -2931,10 +2942,21 @@ namespace plume {
}

if (dirtyGraphicsState.vertexBuffers) {
std::array<bool, 31> slotUsed{};

for (uint32_t i = 0; i < viewCount; i++) {
// Bind right after the push constants, up till the max vertex buffer index
const uint32_t bindIndex = std::min(PUSH_CONSTANT_MAX_INDEX + 1 + vertexBufferIndices[i], VERTEX_BUFFER_MAX_INDEX);
activeRenderEncoder->setVertexBuffer(vertexBuffers[i], vertexBufferOffsets[i], bindIndex);
slotUsed[bindIndex] = true;
}

// Bind all unbound slots to the null buffer
auto nullBuffer = static_cast<const MetalBuffer*>(queue->device->nullBuffer.get());
for (uint32_t i = PUSH_CONSTANT_MAX_INDEX + 1; i <= VERTEX_BUFFER_MAX_INDEX; i++) {
if (!slotUsed[i]) {
activeRenderEncoder->setVertexBuffer(nullBuffer->mtl, 0, i);
}
}

stateCache.lastVertexBuffers = vertexBuffers;
Expand Down Expand Up @@ -3231,6 +3253,8 @@ namespace plume {
capabilities.dynamicDepthBias = true;
capabilities.uma = mtl->hasUnifiedMemory();
capabilities.queryPools = false;

nullBuffer = createBuffer(RenderBufferDesc::DefaultBuffer(16, RenderBufferFlag::VERTEX));
}

MetalDevice::~MetalDevice() {
Expand Down
3 changes: 3 additions & 0 deletions plume_metal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ namespace plume {
std::vector<Descriptor> descriptors;
MetalArgumentBuffer argumentBuffer;
std::vector<ResourceEntry> resourceEntries;
std::vector<MTL::Resource *> toReleaseOnDestruction;

MetalDescriptorSet(MetalDevice *device, const RenderDescriptorSetDesc &desc);
MetalDescriptorSet(MetalDevice *device, uint32_t entryCount);
Expand Down Expand Up @@ -593,6 +594,8 @@ namespace plume {
// Blit functionality
MTL::BlitPassDescriptor *sharedBlitDescriptor = nullptr;

std::unique_ptr<RenderBuffer> nullBuffer;

explicit MetalDevice(MetalInterface *renderInterface, const std::string &preferredDeviceName);
~MetalDevice() override;
std::unique_ptr<RenderDescriptorSet> createDescriptorSet(const RenderDescriptorSetDesc &desc) override;
Expand Down
Loading