fix: ore buffer per-frame update race (#12976) cc34cea963 * add ore GM exposing the per-frame buffer update race * fix Metal ore buffer update race with per-write versioning * add ore_buffer_update_between_draws goldens for metal d3d11 and gl * fix Vulkan ore buffer update race with per-write versioning * fix D3D12 ore buffer update race with per-write versioning * fix WebGPU ore buffer update race with per-write versioning * add ore_buffer_update_between_draws goldens for vulkan d3d12 and webgpu * address review notes and tighten comments for the ore buffer race fix * fix wgpu partial-update shadow seed and harden ore buffer race edge cases * add multi-frame ore buffer race regression unit test * scope ore buffer race test to racy backends, fixing linux headless GL abort * free manager-less GPUResources on release instead of leaking them * harden ore buffer orphaning against gpu allocation failure on metal, vulkan and wgpu * make d3d12 buffer orphaning retry on alloc failure, matching the other backends * wgpu: don't cache a failed bind group creation and correct the shadow comment * wgpu/vulkan: mark UBOs bound only after a successful resolve, and restore the wgpu bind group label * fail makeBuffer cleanly on gpu allocation failure (metal, vulkan, wgpu) * harden ore bind group OOM handling and orphan-failure diagnostics across vulkan/metal/wgpu Co-authored-by: Luigi Rosso <luigi-rosso@users.noreply.github.com>
diff --git a/.rive_head b/.rive_head index f8c5e45..e38af11 100644 --- a/.rive_head +++ b/.rive_head
@@ -1 +1 @@ -9f3eef86340a3ef53568116c197fd343a9e1d4e3 +cc34cea96329320072dcc92cc4d302db44369dbe
diff --git a/renderer/include/rive/renderer/ore/ore_context_d3d12.hpp b/renderer/include/rive/renderer/ore/ore_context_d3d12.hpp index 85d6675..b4103dd 100644 --- a/renderer/include/rive/renderer/ore/ore_context_d3d12.hpp +++ b/renderer/include/rive/renderer/ore/ore_context_d3d12.hpp
@@ -44,6 +44,12 @@ void endFrame() override; void waitForGPU() override; + // Frame numbers from the host, used by BufferD3D12 to version backings: + // a backing bound in `currentFrameNumber` is reclaimable once + // `safeFrameNumber` reaches it. + uint64_t currentFrameNumber() const { return m_currentFrameNumber; } + uint64_t safeFrameNumber() const { return m_safeFrameNumber; } + rcp<TextureView> wrapCanvasTexture(gpu::RenderCanvas* canvas) override; rcp<TextureView> wrapRiveTexture(gpu::Texture* gpuTex, uint32_t width, @@ -58,6 +64,7 @@ friend class RenderPassD3D12; friend class BindGroupD3D12; friend class TextureD3D12; + friend class BufferD3D12; ContextD3D12(rcp<rive::gpu::GPUResourceManager> manager) : Context(std::move(manager)) @@ -65,6 +72,12 @@ // D3D12 implementation helpers — defined in ore_context_d3d12.cpp. rcp<Buffer> d3d12MakeBuffer(const BufferDesc& desc); + // Create + persistently map one UPLOAD-heap backing of exactly + // `alignedSize` bytes. Shared by buffer creation and orphan-on-bound. + bool d3d12AllocBufferBacking( + UINT64 alignedSize, + Microsoft::WRL::ComPtr<ID3D12Resource>& outResource, + void** outMapped); rcp<Texture> d3d12MakeTexture(const TextureDesc& desc); rcp<TextureView> d3d12MakeTextureView(const TextureViewDesc& desc); rcp<Sampler> d3d12MakeSampler(const SamplerDesc& desc); @@ -109,6 +122,9 @@ Microsoft::WRL::ComPtr<ID3D12DescriptorHeap> m_d3dGpuSamplerHeap; UINT m_d3dGpuSrvAllocated = 0; UINT m_d3dGpuSamplerAllocated = 0; + // Host frame numbers captured at beginFrame for backing reclamation. + uint64_t m_currentFrameNumber = 0; + uint64_t m_safeFrameNumber = 0; // Helpers called by RenderPass to allocate and resolve descriptor handles. UINT d3d12AllocGpuSrvSlots(UINT count); UINT d3d12AllocGpuSamplerSlots(UINT count);
diff --git a/renderer/include/rive/renderer/ore/ore_context_metal.hpp b/renderer/include/rive/renderer/ore/ore_context_metal.hpp index a5ebd31..e23a199 100644 --- a/renderer/include/rive/renderer/ore/ore_context_metal.hpp +++ b/renderer/include/rive/renderer/ore/ore_context_metal.hpp
@@ -8,6 +8,9 @@ #import <Metal/Metal.h> +#include <atomic> +#include <memory> + namespace rive::ore { @@ -49,6 +52,16 @@ ShaderTarget shaderTarget() const override { return ShaderTarget::msl; } + // Buffer versioning serials. currentSerial is the command buffer being + // recorded. completedSerial is the highest the GPU has finished, so a + // backing at or below it can be recycled. See ore_buffer_metal.mm. + uint64_t currentSerial() const { return m_currentSerial; } + uint64_t completedSerial() const + { + return m_completedSerial->load(std::memory_order_relaxed); + } + id<MTLDevice> device() const { return m_mtlDevice; } + ContextMetal(const ContextMetal&) = delete; ContextMetal& operator=(const ContextMetal&) = delete; @@ -79,6 +92,14 @@ id<MTLCommandBuffer> m_mtlCommandBuffer = nil; std::vector<rcp<BindGroup>> m_deferredBindGroups; + + // Serial of the command buffer being recorded, bumped each frame. + uint64_t m_currentSerial = 0; + // Highest serial the GPU has finished. Written on the completion handler + // thread, read on the recording thread, hence atomic. shared_ptr so the + // handler stays valid even if the context dies first. + std::shared_ptr<std::atomic<uint64_t>> m_completedSerial = + std::make_shared<std::atomic<uint64_t>>(0); }; } // namespace rive::ore
diff --git a/renderer/include/rive/renderer/ore/ore_context_vulkan.hpp b/renderer/include/rive/renderer/ore/ore_context_vulkan.hpp index 907b6ed..b0a7ce3 100644 --- a/renderer/include/rive/renderer/ore/ore_context_vulkan.hpp +++ b/renderer/include/rive/renderer/ore/ore_context_vulkan.hpp
@@ -158,6 +158,17 @@ // `vkDeferDestroy` if needed. VkDescriptorSetLayout m_vkEmptyDSL = VK_NULL_HANDLE; VkDescriptorSetLayout vkGetOrCreateEmptyDSL(); + + // Allocate a descriptor set from the active generation, rolling a fresh one + // on capacity. The returned pool ref keeps the set alive. Used by + // makeBindGroup and by BindGroupVulkan when a UBO orphans onto a new + // backing and needs a freshly written set. + struct DescriptorSetAllocation + { + VkDescriptorSet set = VK_NULL_HANDLE; + rcp<DescriptorPoolGeneration> pool; + }; + DescriptorSetAllocation vkAllocateDescriptorSet(VkDescriptorSetLayout dsl); // Render pass cache — typically <10 entries, keyed on format+load/store // ops. std::vector<std::pair<VKRenderPassKey, VkRenderPass>> m_vkRenderPassCache;
diff --git a/renderer/include/rive/renderer/ore/ore_context_wgpu.hpp b/renderer/include/rive/renderer/ore/ore_context_wgpu.hpp index 28ffa7e..048f26c 100644 --- a/renderer/include/rive/renderer/ore/ore_context_wgpu.hpp +++ b/renderer/include/rive/renderer/ore/ore_context_wgpu.hpp
@@ -68,6 +68,12 @@ // Use this to select between GLES and Vulkan GLSL shader source. bool isGLES() const { return m_wgpuBackend == WGPUBackend::OpenGLES; } + // Monotonic frame serial for buffer versioning. A backing bound this frame + // is not reused until a later frame, when its WriteBuffer is ordered after + // this frame's submit. The host does not thread frame numbers to the wgpu + // backend, so the context owns the serial. + uint64_t currentFrameSerial() const { return m_frameSerial; } + ContextWGPU(const ContextWGPU&) = delete; ContextWGPU& operator=(const ContextWGPU&) = delete; @@ -87,6 +93,7 @@ wgpu::Device m_wgpuDevice; wgpu::Queue m_wgpuQueue; wgpu::CommandEncoder m_wgpuCommandEncoder; + uint64_t m_frameSerial = 0; }; } // namespace rive::ore
diff --git a/renderer/src/gpu_resource.cpp b/renderer/src/gpu_resource.cpp index 0cfd0ec..ad62594 100644 --- a/renderer/src/gpu_resource.cpp +++ b/renderer/src/gpu_resource.cpp
@@ -11,9 +11,13 @@ void GPUResource::onRefCntReachedZero() const { // GPUResourceManager will hold off on deleting "this" until its safe frame - // number has been reached. + // number has been reached. With no manager (the Metal Ore backend keeps + // native objects alive via ARC and command-buffer retention rather than a + // manager) there is no deferred reclaim, so delete now instead of leaking. if (m_manager) m_manager->onRenderingResourceReleased(const_cast<GPUResource*>(this)); + else + delete const_cast<GPUResource*>(this); } GPUResourceManager::~GPUResourceManager()
diff --git a/renderer/src/ore/d3d12/ore_bind_group_d3d12.hpp b/renderer/src/ore/d3d12/ore_bind_group_d3d12.hpp index 0305f7a..927d239 100644 --- a/renderer/src/ore/d3d12/ore_bind_group_d3d12.hpp +++ b/renderer/src/ore/d3d12/ore_bind_group_d3d12.hpp
@@ -7,6 +7,7 @@ namespace rive::ore { class ContextD3D12; +class BufferD3D12; class BindGroupD3D12 : public LITE_RTTI_OVERRIDE(BindGroup, BindGroupD3D12) { @@ -19,17 +20,21 @@ private: friend class ContextD3D12; friend class RenderPassD3D12; - // UBO (CBV) slots — GPU virtual addresses and sizes indexed by native slot. - D3D12_GPU_VIRTUAL_ADDRESS m_d3dUBOAddresses[8] = {}; + // UBO slots. Live buffer back-refs, offsets and sizes indexed by native + // slot. The GPU-VA is resolved from the buffer at apply time so a post-bind + // update that orphaned the backing is seen. Buffers kept alive by + // m_retainedBuffers. + BufferD3D12* m_d3dUBOBuffers[8] = {}; + uint32_t m_d3dUBOOffsets[8] = {}; uint32_t m_d3dUBOSizes[8] = {}; uint8_t m_d3dUBOSlotMask = 0; uint8_t m_d3dUBODynamicOffsetMask = 0; - // SRV (texture) slots — CPU descriptor handles and texture back-refs. + // SRV texture slots. CPU descriptor handles and texture back-refs. D3D12_CPU_DESCRIPTOR_HANDLE m_d3dSrvHandles[8] = {}; Texture* m_d3dSrvTextures[8] = - {}; // Weak refs; kept alive by m_retainedViews. + {}; // Weak refs kept alive by m_retainedViews uint8_t m_d3dSrvSlotMask = 0; - // Sampler slots — CPU descriptor handles. + // Sampler slots. CPU descriptor handles. D3D12_CPU_DESCRIPTOR_HANDLE m_d3dSamplerHandles[8] = {}; uint8_t m_d3dSamplerSlotMask = 0; };
diff --git a/renderer/src/ore/d3d12/ore_buffer_d3d12.cpp b/renderer/src/ore/d3d12/ore_buffer_d3d12.cpp index ccf33a8..a0ce961 100644 --- a/renderer/src/ore/d3d12/ore_buffer_d3d12.cpp +++ b/renderer/src/ore/d3d12/ore_buffer_d3d12.cpp
@@ -13,18 +13,81 @@ namespace rive::ore { -// --- Public method definitions (D3D12-only builds) --- -// When both D3D11 and D3D12 are compiled, the combined -// ore_context_d3d11_d3d12.cpp file provides these methods with dispatch. +// Guarded by ORE_BACKEND_D3D12. The Windows build compiles both the d3d11 and +// d3d12 dirs with both backend macros defined; BufferD3D11 and BufferD3D12 are +// distinct classes under their own guards and the context factory selects the +// concrete type at runtime, so there is no shared dispatch file. #if defined(ORE_BACKEND_D3D12) +void BufferD3D12::markBound() +{ + m_currentSerial = m_ctx->currentFrameNumber(); + m_boundSinceUpdate = true; +} + +bool BufferD3D12::acquireFreshBacking(uint32_t writeOffset, uint32_t writeSize) +{ + // Retire the current backing since an in flight bind may still read it. + void* oldMapped = m_d3dMappedPtr; + m_retired.push_back( + {std::move(m_d3dBuffer), m_d3dMappedPtr, m_currentSerial}); + + // Reuse a retired backing the GPU has finished with, else allocate one. + // Require the bind to predate the current frame, not just safeFrameNumber, + // so a host that ever reports safe == current cannot hand back an in flight + // backing. + const uint64_t safe = m_ctx->safeFrameNumber(); + const uint64_t current = m_ctx->currentFrameNumber(); + for (auto it = m_retired.begin(); it != m_retired.end(); ++it) + { + if (it->serial <= safe && it->serial < current) + { + m_d3dBuffer = std::move(it->resource); + m_d3dMappedPtr = it->mapped; + m_retired.erase(it); + break; + } + } + if (m_d3dBuffer == nullptr && + !m_ctx->d3d12AllocBufferBacking(m_allocatedSize, + m_d3dBuffer, + &m_d3dMappedPtr)) + { + // Allocation failed. Keep the backing we just retired so we do not + // write through a null map. This reraces this one update but the data + // is already there, and it avoids a crash. + m_ctx->setLastError("ore: D3D12 buffer backing allocation failed; " + "reusing in flight backing for this update"); + Backing& old = m_retired.back(); + m_d3dBuffer = std::move(old.resource); + m_d3dMappedPtr = old.mapped; + m_retired.pop_back(); + return false; // Kept the current backing; retry orphaning next update. + } + + m_currentSerial = 0; + // Carry contents forward so a partial update keeps untouched bytes. Skip + // when this update covers the whole buffer, and skip the self copy when the + // reused backing is the one we just retired. + if (!(writeOffset == 0 && writeSize == m_size) && + m_d3dMappedPtr != oldMapped) + memcpy(m_d3dMappedPtr, oldMapped, m_size); + return true; +} + void BufferD3D12::update(const void* data, uint32_t size, uint32_t offset) { assert(offset + size <= m_size); assert(m_d3dBuffer != nullptr); assert(m_d3dMappedPtr != nullptr); - // UPLOAD heap buffer is persistently mapped — write directly. + if (m_boundSinceUpdate) + { + // On allocation failure keep the current backing and retry next update. + if (acquireFreshBacking(offset, size)) + m_boundSinceUpdate = false; + } + // UPLOAD heap buffer is persistently mapped so write directly. memcpy(static_cast<uint8_t*>(m_d3dMappedPtr) + offset, data, size); }
diff --git a/renderer/src/ore/d3d12/ore_buffer_d3d12.hpp b/renderer/src/ore/d3d12/ore_buffer_d3d12.hpp index 567944f..8db4b8c 100644 --- a/renderer/src/ore/d3d12/ore_buffer_d3d12.hpp +++ b/renderer/src/ore/d3d12/ore_buffer_d3d12.hpp
@@ -2,11 +2,15 @@ #include "rive/renderer/ore/ore_buffer.hpp" #include <d3d12.h> #include <wrl/client.h> +#include <vector> namespace rive::ore { class ContextD3D12; +// A write after a bind orphans onto a fresh backing instead of racing the GPU +// still reading the bound one. Bindings resolve the live backing at apply time. +// Immutable or never rebound buffers stay at a single backing. class BufferD3D12 : public LITE_RTTI_OVERRIDE(Buffer, BufferD3D12) { public: @@ -18,13 +22,42 @@ ~BufferD3D12() override = default; // ComPtr released automatically void update(const void* data, uint32_t size, uint32_t offset) override; + // GPU-VA of the live backing. Call at apply time, not creation time. + D3D12_GPU_VIRTUAL_ADDRESS currentGpuVA() const + { + return m_d3dBuffer->GetGPUVirtualAddress(); + } + + // Stamp the live backing with this frame so a later update() orphans + // instead of overwriting memory the GPU may still read. + void markBound(); + private: friend class ContextD3D12; friend class RenderPassD3D12; friend class TextureD3D12; // for staging buffer access during texture // uploads - // UPLOAD heap — persistently mapped; GPU reads directly. + + // Move to a fresh backing, copying current contents so a partial update + // keeps untouched bytes. The pending write's range lets a full-buffer + // update skip the copy. Returns false if a fresh backing could not be + // allocated, in which case the current backing is kept. + bool acquireFreshBacking(uint32_t writeOffset, uint32_t writeSize); + + // Live backing in an UPLOAD heap, persistently mapped so the GPU reads it. Microsoft::WRL::ComPtr<ID3D12Resource> m_d3dBuffer; void* m_d3dMappedPtr = nullptr; + + struct Backing + { + Microsoft::WRL::ComPtr<ID3D12Resource> resource; + void* mapped = nullptr; + uint64_t serial = 0; // frame it was last bound in + }; + std::vector<Backing> m_retired; // rotated-out backings awaiting GPU retire + uint64_t m_currentSerial = 0; // frame the live backing was last bound in + bool m_boundSinceUpdate = false; + UINT64 m_allocatedSize = 0; // aligned size for fresh backings + ContextD3D12* m_ctx = nullptr; }; } // namespace rive::ore
diff --git a/renderer/src/ore/d3d12/ore_context_d3d12.cpp b/renderer/src/ore/d3d12/ore_context_d3d12.cpp index 909509e..a340a2e 100644 --- a/renderer/src/ore/d3d12/ore_context_d3d12.cpp +++ b/renderer/src/ore/d3d12/ore_context_d3d12.cpp
@@ -569,6 +569,8 @@ void ContextD3D12::beginFrame(const FrameDescriptor& desc) { #if defined(ORE_BACKEND_D3D12) + m_currentFrameNumber = desc.currentFrameNumber; + m_safeFrameNumber = desc.safeFrameNumber; m_d3dCmdList = static_cast<ID3D12GraphicsCommandList*>(desc.externalCommandBuffer); @@ -676,17 +678,13 @@ // d3d12MakeBuffer // ============================================================================ -rcp<Buffer> ContextD3D12::d3d12MakeBuffer(const BufferDesc& desc) +bool ContextD3D12::d3d12AllocBufferBacking( + UINT64 alignedSize, + Microsoft::WRL::ComPtr<ID3D12Resource>& outResource, + void** outMapped) { #if defined(ORE_BACKEND_D3D12) - auto buffer = - rcp<BufferD3D12>(new BufferD3D12(m_manager, desc.size, desc.usage)); - // All ORE buffers live in UPLOAD heap — simple, safe for test workloads. - UINT64 alignedSize = desc.size; - if (desc.usage == BufferUsage::uniform) - alignedSize = (alignedSize + 255) & ~255ULL; // 256-byte CB alignment. - D3D12_HEAP_PROPERTIES heapProps = {}; heapProps.Type = D3D12_HEAP_TYPE_UPLOAD; @@ -706,16 +704,42 @@ &resDesc, D3D12_RESOURCE_STATE_GENERIC_READ, nullptr, - IID_PPV_ARGS(buffer->m_d3dBuffer.GetAddressOf())); + IID_PPV_ARGS(outResource.GetAddressOf())); if (FAILED(hr)) - return nullptr; + return false; // Persistently map. UPLOAD heap memory is write-combined; keep mapped. D3D12_RANGE readRange = {0, 0}; - hr = buffer->m_d3dBuffer->Map(0, &readRange, &buffer->m_d3dMappedPtr); - if (FAILED(hr)) + hr = outResource->Map(0, &readRange, outMapped); + return SUCCEEDED(hr); +#else + (void)alignedSize; + (void)outResource; + (void)outMapped; + return false; +#endif +} + +rcp<Buffer> ContextD3D12::d3d12MakeBuffer(const BufferDesc& desc) +{ +#if defined(ORE_BACKEND_D3D12) + auto buffer = + rcp<BufferD3D12>(new BufferD3D12(m_manager, desc.size, desc.usage)); + + UINT64 alignedSize = desc.size; + if (desc.usage == BufferUsage::uniform) + alignedSize = (alignedSize + 255) & ~255ULL; // 256-byte CB alignment. + + if (!d3d12AllocBufferBacking(alignedSize, + buffer->m_d3dBuffer, + &buffer->m_d3dMappedPtr)) return nullptr; + // Seed versioning state so a write after a bind orphans onto a fresh + // backing instead of racing the GPU still reading the bound one. + buffer->m_ctx = this; + buffer->m_allocatedSize = alignedSize; + if (desc.data) memcpy(buffer->m_d3dMappedPtr, desc.data, desc.size); @@ -1435,8 +1459,10 @@ assert(slot < 8); auto buffer = lite_rtti_cast<BufferD3D12*>(entry.buffer); assert(buffer != nullptr); - bg->m_d3dUBOAddresses[slot] = - buffer->m_d3dBuffer->GetGPUVirtualAddress() + entry.offset; + // Store the buffer, not its GPU-VA: the backing can change on a + // later update(), so the address must be resolved at apply time. + bg->m_d3dUBOBuffers[slot] = buffer; + bg->m_d3dUBOOffsets[slot] = entry.offset; bg->m_d3dUBOSizes[slot] = entry.size > 0 ? entry.size : static_cast<uint32_t>(buffer->size()); bg->m_d3dUBOSlotMask |= static_cast<uint8_t>(1u << slot);
diff --git a/renderer/src/ore/d3d12/ore_d3d12_bind_group_apply.hpp b/renderer/src/ore/d3d12/ore_d3d12_bind_group_apply.hpp index 9f72938..a23dccc 100644 --- a/renderer/src/ore/d3d12/ore_d3d12_bind_group_apply.hpp +++ b/renderer/src/ore/d3d12/ore_d3d12_bind_group_apply.hpp
@@ -19,6 +19,7 @@ #include "rive/renderer/ore/ore_context_d3d12.hpp" #include "ore_render_pass_d3d12.hpp" #include "ore_bind_group_d3d12.hpp" +#include "ore_buffer_d3d12.hpp" #include "ore_pipeline_d3d12.hpp" #include <d3d12.h> @@ -73,8 +74,12 @@ m_d3dContext->d3d12GpuSrvCpuHandle(heapStart + offset++); if (bg->m_d3dUBOSlotMask & (1u << globalSlot)) { + // Resolve the live backing now and mark it bound so a later + // update() orphans instead of racing this draw. + BufferD3D12* ubo = bg->m_d3dUBOBuffers[globalSlot]; + ubo->markBound(); D3D12_GPU_VIRTUAL_ADDRESS gpuVA = - bg->m_d3dUBOAddresses[globalSlot]; + ubo->currentGpuVA() + bg->m_d3dUBOOffsets[globalSlot]; if ((bg->m_d3dUBODynamicOffsetMask & (1u << globalSlot)) && dynIdx < dynamicOffsetCount) {
diff --git a/renderer/src/ore/d3d12/ore_render_pass_d3d12.cpp b/renderer/src/ore/d3d12/ore_render_pass_d3d12.cpp index 829b6c3..a4caf3e 100644 --- a/renderer/src/ore/d3d12/ore_render_pass_d3d12.cpp +++ b/renderer/src/ore/d3d12/ore_render_pass_d3d12.cpp
@@ -176,8 +176,9 @@ ? m_d3dCurrentPipeline->m_d3dVertexStrides[slot] : 0; + buffer->markBound(); D3D12_VERTEX_BUFFER_VIEW vbv = {}; - vbv.BufferLocation = buffer->m_d3dBuffer->GetGPUVirtualAddress() + offset; + vbv.BufferLocation = buffer->currentGpuVA() + offset; vbv.SizeInBytes = buffer->m_size - offset; vbv.StrideInBytes = stride; m_d3dCmdList->IASetVertexBuffers(slot, 1, &vbv); @@ -205,8 +206,9 @@ : DXGI_FORMAT_R16_UINT; m_d3dIndexOffset = offset; + buffer->markBound(); D3D12_INDEX_BUFFER_VIEW ibv = {}; - ibv.BufferLocation = buffer->m_d3dBuffer->GetGPUVirtualAddress() + offset; + ibv.BufferLocation = buffer->currentGpuVA() + offset; ibv.SizeInBytes = buffer->m_size - offset; ibv.Format = m_d3dIndexFormat; m_d3dCmdList->IASetIndexBuffer(&ibv);
diff --git a/renderer/src/ore/metal/ore_bind_group_metal.hpp b/renderer/src/ore/metal/ore_bind_group_metal.hpp index d779cc0..bf534fd 100644 --- a/renderer/src/ore/metal/ore_bind_group_metal.hpp +++ b/renderer/src/ore/metal/ore_bind_group_metal.hpp
@@ -8,13 +8,16 @@ { class ContextMetal; class RenderPassMetal; +class BufferMetal; class BindGroupMetal : public LITE_RTTI_OVERRIDE(BindGroup, BindGroupMetal) { public: struct MTLBufferBinding { - id<MTLBuffer> buffer = nil; + // Source buffer, not a raw handle, so the live backing resolves at + // encode time. Kept alive by m_retainedBuffers. + BufferMetal* src = nullptr; uint32_t offset = 0; uint32_t binding = 0; // WGSL @binding, for sort bool hasDynamicOffset = false;
diff --git a/renderer/src/ore/metal/ore_buffer_metal.hpp b/renderer/src/ore/metal/ore_buffer_metal.hpp index c169825..32075d7 100644 --- a/renderer/src/ore/metal/ore_buffer_metal.hpp +++ b/renderer/src/ore/metal/ore_buffer_metal.hpp
@@ -1,24 +1,55 @@ #pragma once #include "rive/renderer/ore/ore_buffer.hpp" #import <Metal/Metal.h> +#include <string> +#include <vector> namespace rive::ore { class ContextMetal; class RenderPassMetal; +// Pool of native backings so an update() after a bind orphans onto a fresh +// backing instead of racing the GPU still reading the bound one. Bindings +// resolve the live backing at encode time. Immutable buffers stay at one. class BufferMetal : public LITE_RTTI_OVERRIDE(Buffer, BufferMetal) { public: BufferMetal(uint32_t size, BufferUsage usage) : lite_rtti_override(size, usage) {} - ~BufferMetal() override = default; // ARC releases m_mtlBuffer + ~BufferMetal() override = default; // ARC releases the backings + void update(const void* data, uint32_t size, uint32_t offset) override; + // Backing to bind right now. + id<MTLBuffer> current() const { return m_pool[m_currentIndex].mtl; } + + // Tag the current backing with this frame's serial so a later update() + // orphans instead of overwriting in-flight memory. + void markBound(); + private: friend class ContextMetal; friend class RenderPassMetal; - id<MTLBuffer> m_mtlBuffer = nil; + + struct Backing + { + id<MTLBuffer> mtl = nil; + uint64_t serial = 0; // serial it was last bound in + }; + + // Move to a fresh backing, copying current contents so a partial update + // keeps untouched bytes. The pending write's range lets a full-buffer + // update skip the copy. + // Returns false if a fresh backing could not be allocated, in which case + // the current backing is kept. + bool acquireFreshBacking(uint32_t writeOffset, uint32_t writeSize); + + std::vector<Backing> m_pool; + size_t m_currentIndex = 0; + bool m_boundSinceUpdate = false; + ContextMetal* m_ctx = nullptr; + std::string m_label; }; } // namespace rive::ore
diff --git a/renderer/src/ore/metal/ore_buffer_metal.mm b/renderer/src/ore/metal/ore_buffer_metal.mm index c161c00..c111702 100644 --- a/renderer/src/ore/metal/ore_buffer_metal.mm +++ b/renderer/src/ore/metal/ore_buffer_metal.mm
@@ -3,6 +3,7 @@ */ #include "ore_buffer_metal.hpp" +#include "rive/renderer/ore/ore_context_metal.hpp" #include "rive/rive_types.hpp" #import <Metal/Metal.h> @@ -10,10 +11,66 @@ namespace rive::ore { +void BufferMetal::markBound() +{ + m_pool[m_currentIndex].serial = m_ctx->currentSerial(); + m_boundSinceUpdate = true; +} + +bool BufferMetal::acquireFreshBacking(uint32_t writeOffset, uint32_t writeSize) +{ + id<MTLBuffer> old = m_pool[m_currentIndex].mtl; + const uint64_t completed = m_ctx->completedSerial(); + + // Reuse a backing the GPU has finished with, else allocate one. + size_t fresh = m_pool.size(); + for (size_t i = 0; i < m_pool.size(); ++i) + { + if (i != m_currentIndex && m_pool[i].serial <= completed) + { + fresh = i; + break; + } + } + if (fresh == m_pool.size()) + { + id<MTLBuffer> mtl = + [m_ctx->device() newBufferWithLength:m_size + options:MTLResourceStorageModeShared]; + if (mtl == nil) + { + // Keep the current backing. Report it like the D3D12 path so the + // degraded (racy) fallback is diagnosable. + m_ctx->setLastError("ore: Metal buffer backing allocation failed; " + "reusing in flight backing for this update"); + return false; + } + if (!m_label.empty()) + mtl.label = @(m_label.c_str()); + m_pool.push_back({mtl, 0}); + } + + m_currentIndex = fresh; + // Carry contents forward so a partial update keeps untouched bytes. Skip + // when this update covers the whole buffer. + if (!(writeOffset == 0 && writeSize == m_size)) + memcpy([m_pool[m_currentIndex].mtl contents], [old contents], m_size); + return true; +} + void BufferMetal::update(const void* data, uint32_t size, uint32_t offset) { assert(offset + size <= m_size); - memcpy(static_cast<uint8_t*>([m_mtlBuffer contents]) + offset, data, size); + if (m_boundSinceUpdate) + { + // On allocation failure keep the current backing and retry next update. + if (acquireFreshBacking(offset, size)) + m_boundSinceUpdate = false; + } + memcpy(static_cast<uint8_t*>([m_pool[m_currentIndex].mtl contents]) + + offset, + data, + size); } } // namespace rive::ore
diff --git a/renderer/src/ore/metal/ore_context_metal.mm b/renderer/src/ore/metal/ore_context_metal.mm index b872099..5d2bdc6 100644 --- a/renderer/src/ore/metal/ore_context_metal.mm +++ b/renderer/src/ore/metal/ore_context_metal.mm
@@ -415,23 +415,29 @@ inline rcp<Buffer> ContextMetal::mtlMakeBuffer(const BufferDesc& desc) { auto buffer = rcp<BufferMetal>(new BufferMetal(desc.size, desc.usage)); + buffer->m_ctx = this; + id<MTLBuffer> mtlBuffer; if (desc.data) { - buffer->m_mtlBuffer = + mtlBuffer = [m_mtlDevice newBufferWithBytes:desc.data length:desc.size options:MTLResourceStorageModeShared]; } else { - buffer->m_mtlBuffer = + mtlBuffer = [m_mtlDevice newBufferWithLength:desc.size options:MTLResourceStorageModeShared]; } + if (mtlBuffer == nil) + return nullptr; // Allocation failed. if (desc.label) { - buffer->m_mtlBuffer.label = @(desc.label); + mtlBuffer.label = @(desc.label); + buffer->m_label = desc.label; } + buffer->m_pool.push_back({mtlBuffer, 0}); return buffer; } @@ -910,7 +916,7 @@ auto* buf = lite_rtti_cast<BufferMetal*>(u.buffer); assert(buf); BindGroupMetal::MTLBufferBinding binding; - binding.buffer = buf->m_mtlBuffer; + binding.src = buf; binding.offset = u.offset; binding.binding = u.slot; if (!lookupStages(u.slot, @@ -1144,6 +1150,8 @@ void ContextMetal::beginFrame(const FrameDescriptor&) { m_mtlCommandBuffer = [m_mtlQueue commandBuffer]; + // Serial of the command buffer about to be recorded. + ++m_currentSerial; } void ContextMetal::waitForGPU() @@ -1167,15 +1175,21 @@ // and Apple-Silicon shared-memory races. Refcount drops happen // on a Metal-internal thread; safe because each resource's // destructor is `delete this` against ARC-managed handles. - if (!m_deferredBindGroups.empty()) - { - __block std::vector<rcp<BindGroup>> deferred = - std::move(m_deferredBindGroups); - m_deferredBindGroups.clear(); - [m_mtlCommandBuffer addCompletedHandler:^(id<MTLCommandBuffer>) { - deferred.clear(); - }]; - } + __block std::vector<rcp<BindGroup>> deferred = + std::move(m_deferredBindGroups); + m_deferredBindGroups.clear(); + // Publish GPU completion so backings up to this serial can be reused. + uint64_t finishedSerial = m_currentSerial; + std::shared_ptr<std::atomic<uint64_t>> completed = m_completedSerial; + [m_mtlCommandBuffer addCompletedHandler:^(id<MTLCommandBuffer>) { + deferred.clear(); + uint64_t prev = completed->load(std::memory_order_relaxed); + while (finishedSerial > prev && + !completed->compare_exchange_weak( + prev, finishedSerial, std::memory_order_relaxed)) + { + } + }]; [m_mtlCommandBuffer commit]; m_mtlCommandBuffer = nil; }
diff --git a/renderer/src/ore/metal/ore_render_pass_metal.mm b/renderer/src/ore/metal/ore_render_pass_metal.mm index 5022a9d..eeec330 100644 --- a/renderer/src/ore/metal/ore_render_pass_metal.mm +++ b/renderer/src/ore/metal/ore_render_pass_metal.mm
@@ -158,7 +158,8 @@ { validate(); auto* b = static_cast<BufferMetal*>(buffer); - [m_mtlEncoder setVertexBuffer:b->m_mtlBuffer + b->markBound(); + [m_mtlEncoder setVertexBuffer:b->current() offset:offset atIndex:slot + kMetalVertexBufferBase]; } @@ -169,7 +170,8 @@ { validate(); auto* b = static_cast<BufferMetal*>(buffer); - m_mtlIndexBuffer = b->m_mtlBuffer; + b->markBound(); + m_mtlIndexBuffer = b->current(); m_mtlIndexType = oreIndexFormatToMTL(format); m_mtlIndexBufferOffset = offset; } @@ -190,12 +192,15 @@ uint32_t offset = b.offset; if (b.hasDynamicOffset && dynIdx < dynamicOffsetCount) offset += dynamicOffsets[dynIdx++]; + // Resolve the live backing and mark it bound. + b.src->markBound(); + id<MTLBuffer> mtlBuffer = b.src->current(); if (b.vsSlot != BindingMap::kAbsent) - [m_mtlEncoder setVertexBuffer:b.buffer + [m_mtlEncoder setVertexBuffer:mtlBuffer offset:offset atIndex:b.vsSlot]; if (b.fsSlot != BindingMap::kAbsent) - [m_mtlEncoder setFragmentBuffer:b.buffer + [m_mtlEncoder setFragmentBuffer:mtlBuffer offset:offset atIndex:b.fsSlot]; }
diff --git a/renderer/src/ore/vulkan/ore_bind_group_vulkan.cpp b/renderer/src/ore/vulkan/ore_bind_group_vulkan.cpp index f09285e..af06ed4 100644 --- a/renderer/src/ore/vulkan/ore_bind_group_vulkan.cpp +++ b/renderer/src/ore/vulkan/ore_bind_group_vulkan.cpp
@@ -3,11 +3,104 @@ */ #include "ore_bind_group_vulkan.hpp" +#include "ore_buffer_vulkan.hpp" + +#include <cassert> namespace rive::ore { -// Pool destruction handles the set, no per-set free. +// Pool destruction handles every cached set, no per-set free. BindGroupVulkan::~BindGroupVulkan() = default; +void BindGroupVulkan::markUBOsBound() +{ + for (const auto& w : m_uboWrites) + w.buffer->markBound(); +} + +VkDescriptorSet BindGroupVulkan::resolveDescriptorSet() +{ + auto* ctx = static_cast<ContextVulkan*>(m_context); + + // Reuse the cached set if every UBO is on the same backing as last time. + for (const auto& cached : m_setCache) + { + bool match = true; + for (size_t i = 0; i < m_uboWrites.size(); ++i) + { + if (cached.key[i] != m_uboWrites[i].buffer->current()) + { + match = false; + break; + } + } + if (match) + return cached.set; + } + + auto alloc = ctx->vkAllocateDescriptorSet(m_vkDSL); + // Only fails on device OOM (the generation auto rolls on capacity). Return + // null so setBindGroup reports it via setLastError instead of aborting. + if (alloc.set == VK_NULL_HANDLE) + return VK_NULL_HANDLE; + + constexpr uint32_t kMaxWrites = 32; + assert(m_uboWrites.size() + m_imageWrites.size() <= kMaxWrites); + VkWriteDescriptorSet writes[kMaxWrites] = {}; + VkDescriptorBufferInfo bufferInfos[kMaxWrites] = {}; + VkDescriptorImageInfo imageInfos[kMaxWrites] = {}; + uint32_t writeIdx = 0; + + CachedSet cached; + cached.key.reserve(m_uboWrites.size()); + for (const auto& u : m_uboWrites) + { + VkBuffer vkBuffer = u.buffer->current(); + cached.key.push_back(vkBuffer); + + VkDescriptorBufferInfo& bi = bufferInfos[writeIdx]; + bi.buffer = vkBuffer; + bi.offset = u.offset; + bi.range = u.range; + + VkWriteDescriptorSet& w = writes[writeIdx++]; + w.sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET; + w.dstSet = alloc.set; + w.dstBinding = u.dstBinding; + w.dstArrayElement = 0; + w.descriptorCount = 1; + w.descriptorType = u.type; + w.pBufferInfo = &bi; + } + for (const auto& im : m_imageWrites) + { + VkDescriptorImageInfo& ii = imageInfos[writeIdx]; + ii.imageView = im.imageView; + ii.imageLayout = im.imageLayout; + ii.sampler = im.sampler; + + VkWriteDescriptorSet& w = writes[writeIdx++]; + w.sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET; + w.dstSet = alloc.set; + w.dstBinding = im.dstBinding; + w.dstArrayElement = 0; + w.descriptorCount = 1; + w.descriptorType = im.type; + w.pImageInfo = ⅈ + } + + if (writeIdx > 0) + ctx->m_vk->UpdateDescriptorSets(ctx->m_vk->device, + writeIdx, + writes, + 0, + nullptr); + + cached.set = alloc.set; + cached.pool = std::move(alloc.pool); + m_setCache.push_back(std::move(cached)); + return m_setCache.back().set; +} + } // namespace rive::ore
diff --git a/renderer/src/ore/vulkan/ore_bind_group_vulkan.hpp b/renderer/src/ore/vulkan/ore_bind_group_vulkan.hpp index b2aa246..41b8c2a 100644 --- a/renderer/src/ore/vulkan/ore_bind_group_vulkan.hpp +++ b/renderer/src/ore/vulkan/ore_bind_group_vulkan.hpp
@@ -1,9 +1,11 @@ #pragma once #include "rive/renderer/ore/ore_bind_group.hpp" #include "rive/renderer/ore/ore_context_vulkan.hpp" +#include <vector> namespace rive::ore { +class BufferVulkan; class BindGroupVulkan : public LITE_RTTI_OVERRIDE(BindGroup, BindGroupVulkan) { @@ -13,11 +15,51 @@ {} ~BindGroupVulkan() override; + // Descriptor set for the current backings of this group's UBOs. A UBO that + // orphaned onto a fresh backing gets a freshly written set. Descriptor sets + // are immutable once written, so the binding must re-resolve, not the set. + VkDescriptorSet resolveDescriptorSet(); + + // Stamp every UBO's current backing as bound this frame. + void markUBOsBound(); + private: friend class ContextVulkan; friend class RenderPassVulkan; - VkDescriptorSet m_vkDescriptorSet = VK_NULL_HANDLE; - // Keeps the pool alive while this set is outstanding. - rcp<DescriptorPoolGeneration> m_pool; + + // Replayable write recipe captured at makeBindGroup so a set can be + // rewritten against a UBO's new backing. Buffers stay alive via + // m_retainedBuffers, views and samplers via m_retainedViews and + // m_retainedSamplers. + struct UBOWrite + { + BufferVulkan* buffer; + uint32_t dstBinding; + uint32_t offset; + uint32_t range; + VkDescriptorType type; + }; + struct ImageWrite + { + uint32_t dstBinding; + VkDescriptorType type; + VkImageView imageView; + VkImageLayout imageLayout; + VkSampler sampler; + }; + std::vector<UBOWrite> m_uboWrites; + std::vector<ImageWrite> m_imageWrites; + + VkDescriptorSetLayout m_vkDSL = VK_NULL_HANDLE; + + // One set per distinct combination of UBO backings, bounded by + // frames-in-flight. Keyed on the current VkBuffer of each UBO write. + struct CachedSet + { + std::vector<VkBuffer> key; + VkDescriptorSet set = VK_NULL_HANDLE; + rcp<DescriptorPoolGeneration> pool; + }; + std::vector<CachedSet> m_setCache; }; } // namespace rive::ore
diff --git a/renderer/src/ore/vulkan/ore_buffer_vulkan.cpp b/renderer/src/ore/vulkan/ore_buffer_vulkan.cpp index 5c872a4..822b25f 100644 --- a/renderer/src/ore/vulkan/ore_buffer_vulkan.cpp +++ b/renderer/src/ore/vulkan/ore_buffer_vulkan.cpp
@@ -14,13 +14,104 @@ BufferVulkan::~BufferVulkan() { - if (m_vkBuffer != VK_NULL_HANDLE) + if (!m_pool.empty()) + { + // Pooled buffer. m_vkBuffer aliases the current pool entry, so free the + // pool rather than m_vkBuffer separately to avoid a double free. + for (auto& b : m_pool) + vmaDestroyBuffer(m_vk->allocator(), b.vkBuffer, b.vmaAllocation); + } + else if (m_vkBuffer != VK_NULL_HANDLE) + { + // Staging buffer. A single allocation set directly by TextureVulkan. vmaDestroyBuffer(m_vk->allocator(), m_vkBuffer, m_vmaAllocation); + } +} + +BufferVulkan::Backing BufferVulkan::allocateBacking() +{ + VkBufferCreateInfo bufCI{}; + bufCI.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO; + bufCI.size = m_size; + bufCI.usage = m_vkUsage; + + // Host visible and persistently mapped so CPU writes go direct, no staging. + VmaAllocationCreateInfo allocCI{}; + allocCI.usage = VMA_MEMORY_USAGE_CPU_TO_GPU; + allocCI.flags = VMA_ALLOCATION_CREATE_MAPPED_BIT; + + VmaAllocationInfo allocInfo{}; + Backing b{}; + if (vmaCreateBuffer(m_vk->allocator(), + &bufCI, + &allocCI, + &b.vkBuffer, + &b.vmaAllocation, + &allocInfo) != VK_SUCCESS) + return {}; // Null backing; caller keeps the current one. + b.mappedPtr = allocInfo.pMappedData; + return b; +} + +void BufferVulkan::markBound() +{ + if (!m_pool.empty()) + m_pool[m_currentIndex].frameStamp = m_manager->currentFrameNumber(); + m_boundSinceUpdate = true; +} + +bool BufferVulkan::acquireFreshBacking(uint32_t writeOffset, uint32_t writeSize) +{ + const uint64_t safe = m_manager->safeFrameNumber(); + const uint64_t current = m_manager->currentFrameNumber(); + void* oldMapped = m_pool[m_currentIndex].mappedPtr; + + // Reuse a backing the GPU has retired, else allocate one. The current + // backing is never a candidate since it is the one we are orphaning from. + // Require the bind to predate the current frame, not just safeFrameNumber, + // so a host that ever reports safe == current cannot hand back an in flight + // backing. + size_t fresh = m_pool.size(); + for (size_t i = 0; i < m_pool.size(); ++i) + { + if (i != m_currentIndex && m_pool[i].frameStamp <= safe && + m_pool[i].frameStamp < current) + { + fresh = i; + break; + } + } + if (fresh == m_pool.size()) + { + Backing nb = allocateBacking(); + if (nb.vkBuffer == VK_NULL_HANDLE) + return false; // Allocation failed; keep the current backing. + m_pool.push_back(nb); + } + + m_currentIndex = fresh; + Backing& b = m_pool[m_currentIndex]; + // Carry contents forward so a partial update keeps untouched bytes. Skip + // when this update covers the whole buffer. + if (!(writeOffset == 0 && writeSize == m_size)) + memcpy(b.mappedPtr, oldMapped, m_size); + + // Mirror for direct readers. + m_vkBuffer = b.vkBuffer; + m_vmaAllocation = b.vmaAllocation; + m_vkMappedPtr = b.mappedPtr; + return true; } void BufferVulkan::update(const void* data, uint32_t size, uint32_t offset) { assert(offset + size <= m_size); + if (m_boundSinceUpdate && !m_pool.empty()) + { + // On allocation failure keep the current backing and retry next update. + if (acquireFreshBacking(offset, size)) + m_boundSinceUpdate = false; + } assert(m_vkMappedPtr != nullptr); memcpy(static_cast<uint8_t*>(m_vkMappedPtr) + offset, data, size); }
diff --git a/renderer/src/ore/vulkan/ore_buffer_vulkan.hpp b/renderer/src/ore/vulkan/ore_buffer_vulkan.hpp index 94114e1..7b42252 100644 --- a/renderer/src/ore/vulkan/ore_buffer_vulkan.hpp +++ b/renderer/src/ore/vulkan/ore_buffer_vulkan.hpp
@@ -1,6 +1,7 @@ #pragma once #include "rive/renderer/ore/ore_buffer.hpp" #include "rive/renderer/vulkan/vulkan_context.hpp" +#include <vector> #include <vulkan/vulkan.h> VK_DEFINE_HANDLE(VmaAllocation); @@ -8,6 +9,9 @@ { class ContextVulkan; +// An update after a bind orphans onto a fresh backing instead of racing the GPU +// still reading the bound one. Never rebound buffers stay at one backing. +// Reclamation uses the manager frame numbers. class BufferVulkan : public LITE_RTTI_OVERRIDE(Buffer, BufferVulkan) { public: @@ -19,15 +23,52 @@ ~BufferVulkan() override; void update(const void* data, uint32_t size, uint32_t offset) override; + // Backing to bind right now. + VkBuffer current() const { return m_vkBuffer; } + + // Tag the current backing with this frame's number so a later update() + // orphans instead of overwriting in-flight memory. + void markBound(); + private: friend class ContextVulkan; friend class RenderPassVulkan; friend class TextureVulkan; // for staging buffer access during texture // uploads + + // The current backing is mirrored into m_vkBuffer, m_vmaAllocation and + // m_vkMappedPtr so existing direct readers like texture staging and the + // descriptor copy region always see it. + struct Backing + { + VkBuffer vkBuffer = VK_NULL_HANDLE; + VmaAllocation vmaAllocation = VK_NULL_HANDLE; + void* mappedPtr = nullptr; + uint64_t frameStamp = 0; // frame number it was last bound in + }; + + // Move to a fresh backing, reusing one the GPU has retired or allocating + // one. Copies current contents forward so a partial update keeps untouched + // bytes; the pending write's range lets a full-buffer update skip the copy. + // Returns false if a fresh backing could not be allocated, in which case + // the current backing is kept. + bool acquireFreshBacking(uint32_t writeOffset, uint32_t writeSize); + Backing allocateBacking(); + + // Current backing, mirrored for direct readers. Staging buffers created by + // TextureVulkan set these directly and leave m_pool empty. VkBuffer m_vkBuffer = VK_NULL_HANDLE; VmaAllocation m_vmaAllocation = VK_NULL_HANDLE; void* m_vkMappedPtr = nullptr; VkDevice m_vkDevice = VK_NULL_HANDLE; // Weak ref. rcp<rive::gpu::VulkanContext> m_vk; + + // Backing pool for orphan on bound. Empty for staging buffers, which are + // never bound. Bounded across frames by frames in flight, one extra per + // write after bind. + std::vector<Backing> m_pool; + size_t m_currentIndex = 0; + bool m_boundSinceUpdate = false; + VkBufferUsageFlags m_vkUsage = 0; }; } // namespace rive::ore
diff --git a/renderer/src/ore/vulkan/ore_context_vulkan.cpp b/renderer/src/ore/vulkan/ore_context_vulkan.cpp index 82d4ab8..8aee71c 100644 --- a/renderer/src/ore/vulkan/ore_context_vulkan.cpp +++ b/renderer/src/ore/vulkan/ore_context_vulkan.cpp
@@ -84,6 +84,21 @@ return set; } +ContextVulkan::DescriptorSetAllocation ContextVulkan::vkAllocateDescriptorSet( + VkDescriptorSetLayout dsl) +{ + if (m_currentDescriptorPool == nullptr) + m_currentDescriptorPool = make_rcp<DescriptorPoolGeneration>(m_vk); + VkDescriptorSet set = m_currentDescriptorPool->tryAllocate(dsl); + if (set == VK_NULL_HANDLE) + { + // Capacity or driver refusal. Roll a fresh generation and retry. + m_currentDescriptorPool = make_rcp<DescriptorPoolGeneration>(m_vk); + set = m_currentDescriptorPool->tryAllocate(dsl); + } + return {set, m_currentDescriptorPool}; +} + // ============================================================================ // Enum → VkFormat helper (shared with ore_texture_vulkan.cpp via header, // but declared again here as a file-local helper for the render pass builder). @@ -856,23 +871,28 @@ break; } + // Remembered so the buffer can allocate matching backings when it orphans + // on an update after bind. See BufferVulkan::acquireFreshBacking. + buffer->m_vkUsage = usage; + VkBufferCreateInfo bufCI{}; bufCI.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO; bufCI.size = desc.size; bufCI.usage = usage; - // Host-visible, persistently mapped — direct CPU writes, no staging. + // Host visible and persistently mapped so CPU writes go direct, no staging. VmaAllocationCreateInfo allocCI{}; allocCI.usage = VMA_MEMORY_USAGE_CPU_TO_GPU; allocCI.flags = VMA_ALLOCATION_CREATE_MAPPED_BIT; VmaAllocationInfo allocInfo{}; - vmaCreateBuffer(m_vk->allocator(), - &bufCI, - &allocCI, - &buffer->m_vkBuffer, - &buffer->m_vmaAllocation, - &allocInfo); + if (vmaCreateBuffer(m_vk->allocator(), + &bufCI, + &allocCI, + &buffer->m_vkBuffer, + &buffer->m_vmaAllocation, + &allocInfo) != VK_SUCCESS) + return nullptr; // Allocation failed. buffer->m_vkMappedPtr = allocInfo.pMappedData; if (desc.data != nullptr) @@ -880,6 +900,13 @@ memcpy(buffer->m_vkMappedPtr, desc.data, desc.size); } + // Seed the backing pool with this first allocation, mirrored above. The + // pool stays at one backing unless the buffer is updated after being bound. + buffer->m_pool.push_back({buffer->m_vkBuffer, + buffer->m_vmaAllocation, + buffer->m_vkMappedPtr, + 0}); + return buffer; } @@ -1207,35 +1234,13 @@ ++dynamicCount; } bg->m_dynamicOffsetCount = dynamicCount; + bg->m_vkDSL = layout->m_vkDSL; - VkDescriptorSetLayout dsl = layout->m_vkDSL; - if (m_currentDescriptorPool == nullptr) - { - m_currentDescriptorPool = make_rcp<DescriptorPoolGeneration>(m_vk); - } - bg->m_vkDescriptorSet = m_currentDescriptorPool->tryAllocate(dsl); - if (bg->m_vkDescriptorSet == VK_NULL_HANDLE) - { - // Capacity or driver refusal. Roll a fresh generation and retry. - m_currentDescriptorPool = make_rcp<DescriptorPoolGeneration>(m_vk); - bg->m_vkDescriptorSet = m_currentDescriptorPool->tryAllocate(dsl); - } - if (bg->m_vkDescriptorSet == VK_NULL_HANDLE) - { - return nullptr; - } - bg->m_pool = m_currentDescriptorPool; - - // Build VkWriteDescriptorSet entries for each binding. Vulkan binding - // numbers equal WGSL `@binding` directly (Vulkan is per-set namespaced). - // Validate against the layout's entries. - constexpr uint32_t kMaxWrites = 32; - VkWriteDescriptorSet writes[kMaxWrites] = {}; - VkDescriptorBufferInfo bufferInfos[kMaxWrites] = {}; - VkDescriptorImageInfo imageInfos[kMaxWrites] = {}; - uint32_t writeIdx = 0; - uint32_t bufInfoIdx = 0; - uint32_t imgInfoIdx = 0; + // Validate and capture a replayable write recipe. Descriptor sets are + // resolved lazily at bind time in BindGroupVulkan::resolveDescriptorSet, so + // a UBO that orphans onto a fresh backing gets a set written against it. + // Vulkan binding numbers equal WGSL `@binding` directly since they are + // per set namespaced. // Resolves the layout's native descriptor binding for a WGSL @binding. // The SPIR-V emitter compacts sparse @binding's into per-group @@ -1283,24 +1288,17 @@ uint32_t dstBinding = 0; if (!resolveLayout(ubo.slot, BindingKind::uniformBuffer, &dstBinding)) continue; - assert(writeIdx < kMaxWrites && bufInfoIdx < kMaxWrites); - VkDescriptorBufferInfo& bi = bufferInfos[bufInfoIdx++]; auto buffer = lite_rtti_cast<BufferVulkan*>(ubo.buffer); - bi.buffer = buffer->m_vkBuffer; - bi.offset = ubo.offset; - bi.range = (ubo.size > 0) ? ubo.size : buffer->size(); - - VkWriteDescriptorSet& w = writes[writeIdx++]; - w.sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET; - w.dstSet = bg->m_vkDescriptorSet; + BindGroupVulkan::UBOWrite w; + w.buffer = buffer; w.dstBinding = dstBinding; - w.dstArrayElement = 0; - w.descriptorCount = 1; - w.descriptorType = layout->hasDynamicOffset(ubo.slot) - ? VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC - : VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; - w.pBufferInfo = &bi; + w.offset = ubo.offset; + w.range = (ubo.size > 0) ? ubo.size : buffer->size(); + w.type = layout->hasDynamicOffset(ubo.slot) + ? VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC + : VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + bg->m_uboWrites.push_back(w); bg->m_retainedBuffers.push_back(ref_rcp(ubo.buffer)); } @@ -1313,7 +1311,6 @@ uint32_t dstBinding = 0; if (!resolveLayout(tex.slot, BindingKind::sampledTexture, &dstBinding)) continue; - assert(writeIdx < kMaxWrites && imgInfoIdx < kMaxWrites); // Queue any-layout to SHADER_READ_ONLY_OPTIMAL transition; // flushed before the next draw. Mirrors Dawn's @@ -1329,21 +1326,15 @@ aspect, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL); - VkDescriptorImageInfo& ii = imageInfos[imgInfoIdx++]; auto view = lite_rtti_cast<TextureViewVulkan*>(tex.view); assert(view != nullptr); - ii.imageView = view->m_vkImageView; - ii.imageLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; - ii.sampler = VK_NULL_HANDLE; // Sampler bound separately. - - VkWriteDescriptorSet& w = writes[writeIdx++]; - w.sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET; - w.dstSet = bg->m_vkDescriptorSet; + BindGroupVulkan::ImageWrite w; w.dstBinding = dstBinding; - w.dstArrayElement = 0; - w.descriptorCount = 1; - w.descriptorType = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; - w.pImageInfo = ⅈ + w.type = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; + w.imageView = view->m_vkImageView; + w.imageLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; + w.sampler = VK_NULL_HANDLE; // Sampler bound separately. + bg->m_imageWrites.push_back(w); bg->m_retainedViews.push_back(ref_rcp(tex.view)); } @@ -1355,30 +1346,23 @@ uint32_t dstBinding = 0; if (!resolveLayout(samp.slot, BindingKind::sampler, &dstBinding)) continue; - assert(writeIdx < kMaxWrites && imgInfoIdx < kMaxWrites); - VkDescriptorImageInfo& ii = imageInfos[imgInfoIdx++]; auto sampler = lite_rtti_cast<SamplerVulkan*>(samp.sampler); - ii.sampler = sampler->m_vkSampler; - ii.imageView = VK_NULL_HANDLE; - ii.imageLayout = VK_IMAGE_LAYOUT_UNDEFINED; - - VkWriteDescriptorSet& w = writes[writeIdx++]; - w.sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET; - w.dstSet = bg->m_vkDescriptorSet; + BindGroupVulkan::ImageWrite w; w.dstBinding = dstBinding; - w.dstArrayElement = 0; - w.descriptorCount = 1; - w.descriptorType = VK_DESCRIPTOR_TYPE_SAMPLER; - w.pImageInfo = ⅈ + w.type = VK_DESCRIPTOR_TYPE_SAMPLER; + w.imageView = VK_NULL_HANDLE; + w.imageLayout = VK_IMAGE_LAYOUT_UNDEFINED; + w.sampler = sampler->m_vkSampler; + bg->m_imageWrites.push_back(w); bg->m_retainedSamplers.push_back(ref_rcp(samp.sampler)); } - if (writeIdx > 0) - { - m_vk->UpdateDescriptorSets(m_vk->device, writeIdx, writes, 0, nullptr); - } + // Resolve once now to preserve fail-fast on device OOM (matching the WGPU + // path); later orphans allocate additional sets as their UBOs need them. + if (bg->resolveDescriptorSet() == VK_NULL_HANDLE) + return nullptr; return bg; }
diff --git a/renderer/src/ore/vulkan/ore_render_pass_vulkan.cpp b/renderer/src/ore/vulkan/ore_render_pass_vulkan.cpp index 6079a2e..a618f08 100644 --- a/renderer/src/ore/vulkan/ore_render_pass_vulkan.cpp +++ b/renderer/src/ore/vulkan/ore_render_pass_vulkan.cpp
@@ -54,10 +54,12 @@ VkDeviceSize vkOffset = offset; auto buffer = lite_rtti_cast<BufferVulkan*>(inBuffer); assert(buffer != nullptr); + buffer->markBound(); + VkBuffer vkBuffer = buffer->current(); m_vkContext->m_vk->CmdBindVertexBuffers(m_vkCmdBuf, slot, 1, - &buffer->m_vkBuffer, + &vkBuffer, &vkOffset); } @@ -67,12 +69,13 @@ { auto buffer = lite_rtti_cast<BufferVulkan*>(inBuffer); assert(buffer != nullptr); - m_vkIndexBuffer = buffer->m_vkBuffer; + buffer->markBound(); + m_vkIndexBuffer = buffer->current(); m_vkIndexType = (format == IndexFormat::uint32) ? VK_INDEX_TYPE_UINT32 : VK_INDEX_TYPE_UINT16; m_vkIndexOffset = offset; m_vkContext->m_vk->CmdBindIndexBuffer(m_vkCmdBuf, - buffer->m_vkBuffer, + m_vkIndexBuffer, offset, m_vkIndexType); } @@ -88,13 +91,28 @@ auto bg = lite_rtti_cast<BindGroupVulkan*>(inBg); assert(bg != nullptr); + // Resolve the live backing of each UBO. Only stamp the UBOs bound after a + // successful resolve, so a failed (OOM) resolve doesn't force later + // update()s to orphan needlessly. + VkDescriptorSet set = bg->resolveDescriptorSet(); + if (set == VK_NULL_HANDLE) + { + // Descriptor set allocation failed (device OOM). Report it instead of + // binding null; the Lua layer reads lastError after the call. Pre-fix + // this surfaced at makeBindGroup time. + m_vkContext->setLastError( + "ore: Vulkan descriptor set allocation failed for group %u", + groupIndex); + return; + } + bg->markUBOsBound(); m_vkContext->m_vk->CmdBindDescriptorSets( m_vkCmdBuf, VK_PIPELINE_BIND_POINT_GRAPHICS, m_currentPipeline->m_vkPipelineLayout, groupIndex, 1, - &bg->m_vkDescriptorSet, + &set, dynamicOffsetCount, dynamicOffsets);
diff --git a/renderer/src/ore/wgpu/ore_bind_group_wgpu.cpp b/renderer/src/ore/wgpu/ore_bind_group_wgpu.cpp index 3907694..6565eb7 100644 --- a/renderer/src/ore/wgpu/ore_bind_group_wgpu.cpp +++ b/renderer/src/ore/wgpu/ore_bind_group_wgpu.cpp
@@ -3,3 +3,79 @@ */ #include "ore_bind_group_wgpu.hpp" +#include "ore_buffer_wgpu.hpp" +#include "rive/renderer/ore/ore_context_wgpu.hpp" + +namespace rive::ore +{ + +void BindGroupWGPU::markUBOsBound() +{ + for (const auto& u : m_uboEntries) + u.buffer->markBound(); +} + +const wgpu::BindGroup& BindGroupWGPU::resolveBindGroup() +{ + // Reuse a cached bind group if every UBO is on the same backing as when it + // was built. + for (const auto& cached : m_cache) + { + bool match = true; + for (size_t i = 0; i < m_uboEntries.size(); ++i) + { + if (cached.key[i] != m_uboEntries[i].buffer->current().Get()) + { + match = false; + break; + } + } + if (match) + return cached.bindGroup; + } + + std::vector<wgpu::BindGroupEntry> entries; + entries.reserve(m_uboEntries.size() + m_texEntries.size() + + m_sampEntries.size()); + CachedGroup cached; + cached.key.reserve(m_uboEntries.size()); + for (const auto& u : m_uboEntries) + { + cached.key.push_back(u.buffer->current().Get()); + wgpu::BindGroupEntry e{}; + e.binding = u.binding; + e.buffer = u.buffer->current(); + e.offset = u.offset; + e.size = u.size; + entries.push_back(e); + } + for (const auto& t : m_texEntries) + { + wgpu::BindGroupEntry e{}; + e.binding = t.binding; + e.textureView = t.view; + entries.push_back(e); + } + for (const auto& s : m_sampEntries) + { + wgpu::BindGroupEntry e{}; + e.binding = s.binding; + e.sampler = s.sampler; + entries.push_back(e); + } + + auto* ctx = static_cast<ContextWGPU*>(m_context); + wgpu::BindGroupDescriptor bgDesc{}; + bgDesc.layout = m_wgpuBGL; + bgDesc.label = m_label.empty() ? nullptr : m_label.c_str(); + bgDesc.entryCount = static_cast<uint32_t>(entries.size()); + bgDesc.entries = entries.empty() ? nullptr : entries.data(); + cached.bindGroup = ctx->m_wgpuDevice.CreateBindGroup(&bgDesc); + if (cached.bindGroup == nullptr) + return m_nullBindGroup; // Don't cache a failed creation; retry later. + + m_cache.push_back(std::move(cached)); + return m_cache.back().bindGroup; +} + +} // namespace rive::ore
diff --git a/renderer/src/ore/wgpu/ore_bind_group_wgpu.hpp b/renderer/src/ore/wgpu/ore_bind_group_wgpu.hpp index f0a540e..ccae9c9 100644 --- a/renderer/src/ore/wgpu/ore_bind_group_wgpu.hpp +++ b/renderer/src/ore/wgpu/ore_bind_group_wgpu.hpp
@@ -1,10 +1,13 @@ #pragma once #include "rive/renderer/ore/ore_bind_group.hpp" #include <webgpu/webgpu_cpp.h> +#include <string> +#include <vector> namespace rive::ore { class ContextWGPU; +class BufferWGPU; class BindGroupWGPU : public LITE_RTTI_OVERRIDE(BindGroup, BindGroupWGPU) { @@ -13,9 +16,55 @@ ~BindGroupWGPU() override = default; // wgpu::BindGroup RAII destructor releases + // Bind group for the current backings of this group's UBOs. A UBO that + // orphaned onto a fresh backing gets a freshly built bind group, since + // WebGPU bind groups are immutable once created. + const wgpu::BindGroup& resolveBindGroup(); + + // Stamp every UBO's current backing as bound this frame. + void markUBOsBound(); + private: friend class ContextWGPU; friend class RenderPassWGPU; - wgpu::BindGroup m_wgpuBindGroup; + + // Replayable recipe captured at makeBindGroup so a bind group can be + // rebuilt against a UBO's new backing. Buffers stay alive via + // m_retainedBuffers; views/samplers via m_retainedViews/m_retainedSamplers. + struct UBOEntry + { + BufferWGPU* buffer; + uint32_t binding; + uint64_t offset; + uint64_t size; + }; + struct TexEntry + { + uint32_t binding; + wgpu::TextureView view; + }; + struct SampEntry + { + uint32_t binding; + wgpu::Sampler sampler; + }; + std::vector<UBOEntry> m_uboEntries; + std::vector<TexEntry> m_texEntries; + std::vector<SampEntry> m_sampEntries; + wgpu::BindGroupLayout m_wgpuBGL; + std::string m_label; // propagated to rebuilt bind groups for GPU captures + + // One bind group per distinct combination of UBO backings, bounded by + // frames-in-flight. Keyed on the current WGPUBuffer of each UBO. + struct CachedGroup + { + std::vector<WGPUBuffer> key; + wgpu::BindGroup bindGroup; + }; + std::vector<CachedGroup> m_cache; + + // Returned (null) when CreateBindGroup fails so the failure is not cached + // and a later call can retry. + wgpu::BindGroup m_nullBindGroup; }; } // namespace rive::ore
diff --git a/renderer/src/ore/wgpu/ore_buffer_wgpu.cpp b/renderer/src/ore/wgpu/ore_buffer_wgpu.cpp index 5f32966..a7858d5 100644 --- a/renderer/src/ore/wgpu/ore_buffer_wgpu.cpp +++ b/renderer/src/ore/wgpu/ore_buffer_wgpu.cpp
@@ -3,16 +3,83 @@ */ #include "ore_buffer_wgpu.hpp" +#include "rive/renderer/ore/ore_context_wgpu.hpp" #include "rive/rive_types.hpp" +#include <cstring> + namespace rive::ore { +void BufferWGPU::markBound() +{ + m_pool[m_currentIndex].frameStamp = m_ctx->currentFrameSerial(); + m_boundSinceUpdate = true; +} + +bool BufferWGPU::acquireFreshBacking() +{ + const uint64_t serial = m_ctx->currentFrameSerial(); + + // Reuse a backing from a prior frame, else allocate one. The current + // backing is never a candidate, and neither is one bound this frame. This + // relies on exactly one queue submit per frame serial: the rewrite of a + // prior-frame backing is ordered after that frame's submit, so its reads + // finish first. If Ore ever shares the host encoder across frames with no + // submit between, stamp on submit instead of on beginFrame. + size_t fresh = m_pool.size(); + for (size_t i = 0; i < m_pool.size(); ++i) + { + if (i != m_currentIndex && m_pool[i].frameStamp != serial) + { + fresh = i; + break; + } + } + if (fresh == m_pool.size()) + { + wgpu::BufferDescriptor wDesc{}; + wDesc.size = m_size; + wDesc.usage = m_wgpuUsage; + wDesc.mappedAtCreation = false; + wgpu::Buffer buffer = m_wgpuDevice.CreateBuffer(&wDesc); + if (buffer == nullptr) + { + // Keep the current backing. Report it like the D3D12 path so the + // degraded (racy) fallback is diagnosable. + m_ctx->setLastError("ore: WGPU buffer backing allocation failed; " + "reusing in flight backing for this update"); + return false; + } + m_pool.push_back({std::move(buffer), 0}); + } + m_currentIndex = fresh; + return true; +} + void BufferWGPU::update(const void* data, uint32_t size, uint32_t offset) { assert(offset + size <= m_size); - assert(m_wgpuBuffer != nullptr); - m_wgpuQueue.WriteBuffer(m_wgpuBuffer, offset, data, size); + bool orphaned = false; + if (m_boundSinceUpdate) + { + // On allocation failure keep the current backing and retry next update. + if (acquireFreshBacking()) + { + m_boundSinceUpdate = false; + orphaned = true; + } + } + // Allocate the shadow on first write so a never updated buffer pays + // nothing. It tracks writes from here so an orphan can rewrite the whole + // backing. + if (m_shadow.empty()) + m_shadow.resize(m_size, 0); + memcpy(m_shadow.data() + offset, data, size); + if (orphaned) + m_wgpuQueue.WriteBuffer(current(), 0, m_shadow.data(), m_size); + else + m_wgpuQueue.WriteBuffer(current(), offset, data, size); } } // namespace rive::ore
diff --git a/renderer/src/ore/wgpu/ore_buffer_wgpu.hpp b/renderer/src/ore/wgpu/ore_buffer_wgpu.hpp index 2cba374..88fcb2b 100644 --- a/renderer/src/ore/wgpu/ore_buffer_wgpu.hpp +++ b/renderer/src/ore/wgpu/ore_buffer_wgpu.hpp
@@ -1,11 +1,16 @@ #pragma once #include "rive/renderer/ore/ore_buffer.hpp" #include <webgpu/webgpu_cpp.h> +#include <vector> namespace rive::ore { class ContextWGPU; +// An update after a bind orphans onto a fresh backing so the GPU keeps reading +// the value it was bound with. WriteBuffer copies are queued ahead of the frame +// submit, so without this both draws in a frame would read the last write. +// Never rebound buffers stay at one backing. class BufferWGPU : public LITE_RTTI_OVERRIDE(Buffer, BufferWGPU) { public: @@ -13,12 +18,47 @@ lite_rtti_override(size, usage) {} ~BufferWGPU() override = default; // wgpu::Buffer RAII destructor releases + void update(const void* data, uint32_t size, uint32_t offset) override; + // Backing to bind right now. + const wgpu::Buffer& current() const + { + return m_pool[m_currentIndex].buffer; + } + + // Tag the current backing with this frame's serial so a later update() + // orphans instead of overwriting in-flight memory. + void markBound(); + private: friend class ContextWGPU; friend class RenderPassWGPU; - wgpu::Buffer m_wgpuBuffer; - wgpu::Queue m_wgpuQueue; // Weak ref (addref'd copy). + + struct Backing + { + wgpu::Buffer buffer; + uint64_t frameStamp = 0; // serial it was last bound in + }; + + // Move to a fresh backing. Cross frame reuse is safe because the new + // WriteBuffer is ordered after that frame submit, so only a backing bound + // this frame is skipped. Returns false if a fresh backing could not be + // allocated, in which case the current backing is kept. + bool acquireFreshBacking(); + + std::vector<Backing> m_pool; + size_t m_currentIndex = 0; + bool m_boundSinceUpdate = false; + + // Lazily allocated CPU mirror of the contents. An orphan rewrites the whole + // fresh backing from it, since WGPU buffers are not CPU readable and we + // cannot copy the old backing forward the way Metal and Vulkan do. + std::vector<uint8_t> m_shadow; + + wgpu::BufferUsage m_wgpuUsage{}; + wgpu::Device m_wgpuDevice; + wgpu::Queue m_wgpuQueue; + ContextWGPU* m_ctx = nullptr; }; } // namespace rive::ore
diff --git a/renderer/src/ore/wgpu/ore_context_wgpu.cpp b/renderer/src/ore/wgpu/ore_context_wgpu.cpp index b5634ce..e0ccfd2 100644 --- a/renderer/src/ore/wgpu/ore_context_wgpu.cpp +++ b/renderer/src/ore/wgpu/ore_context_wgpu.cpp
@@ -484,6 +484,7 @@ assert(desc.externalCommandBuffer != nullptr); m_wgpuCommandEncoder = std::move( *static_cast<wgpu::CommandEncoder*>(desc.externalCommandBuffer)); + ++m_frameSerial; } void ContextWGPU::waitForGPU() {} // WGPU submit is synchronous for Dawn. @@ -505,6 +506,8 @@ { auto buffer = rcp<BufferWGPU>(new BufferWGPU(desc.size, desc.usage)); buffer->m_wgpuQueue = m_wgpuQueue; // addref'd copy for WriteBuffer + buffer->m_wgpuDevice = m_wgpuDevice; + buffer->m_ctx = this; wgpu::BufferUsage usage = wgpu::BufferUsage::CopyDst; switch (desc.usage) @@ -522,6 +525,7 @@ usage |= wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopySrc; break; } + buffer->m_wgpuUsage = usage; wgpu::BufferDescriptor wDesc{}; wDesc.label = desc.label; @@ -529,11 +533,21 @@ wDesc.usage = usage; wDesc.mappedAtCreation = false; - buffer->m_wgpuBuffer = m_wgpuDevice.CreateBuffer(&wDesc); + wgpu::Buffer wbuf = m_wgpuDevice.CreateBuffer(&wDesc); + if (wbuf == nullptr) + return nullptr; // Allocation failed. + buffer->m_pool.push_back({std::move(wbuf), 0}); + // A buffer created with initial data seeds its CPU shadow now so a later + // partial update that orphans rewrites the fresh backing with real + // contents, not zeros. A buffer with no initial data allocates the shadow + // lazily on first update, so a never-updated buffer carries no extra host + // memory. if (desc.data != nullptr) { - m_wgpuQueue.WriteBuffer(buffer->m_wgpuBuffer, 0, desc.data, desc.size); + m_wgpuQueue.WriteBuffer(buffer->current(), 0, desc.data, desc.size); + const auto* bytes = static_cast<const uint8_t*>(desc.data); + buffer->m_shadow.assign(bytes, bytes + desc.size); } return buffer; @@ -992,24 +1006,23 @@ return true; }; - uint32_t totalCapacity = - desc.uboCount + desc.textureCount + desc.samplerCount; - std::vector<wgpu::BindGroupEntry> entries(totalCapacity); - uint32_t idx = 0; + // Capture a replayable recipe instead of building the bind group inline, + // so it can be rebuilt against a UBO's fresh backing after an orphan. + bg->m_wgpuBGL = layout->m_wgpuBGL; + bg->m_label = desc.label ? desc.label : ""; for (uint32_t i = 0; i < desc.uboCount; ++i) { const auto& ubo = desc.ubos[i]; if (!checkLayout(ubo.slot, BindingKind::uniformBuffer)) continue; - wgpu::BindGroupEntry& e = entries[idx++]; - e = {}; - e.binding = ubo.slot; auto buffer = lite_rtti_cast<BufferWGPU*>(ubo.buffer); assert(buffer != nullptr); - e.buffer = buffer->m_wgpuBuffer; - e.offset = ubo.offset; - e.size = (ubo.size > 0) ? ubo.size : buffer->size(); + bg->m_uboEntries.push_back( + {buffer, + ubo.slot, + ubo.offset, + (ubo.size > 0) ? ubo.size : buffer->size()}); bg->m_retainedBuffers.push_back(ref_rcp(buffer)); } @@ -1018,12 +1031,9 @@ const auto& tex = desc.textures[i]; if (!checkLayout(tex.slot, BindingKind::sampledTexture)) continue; - wgpu::BindGroupEntry& e = entries[idx++]; - e = {}; - e.binding = tex.slot; auto view = lite_rtti_cast<TextureViewWGPU*>(tex.view); assert(view != nullptr); - e.textureView = view->m_wgpuTextureView; + bg->m_texEntries.push_back({tex.slot, view->m_wgpuTextureView}); bg->m_retainedViews.push_back(ref_rcp(view)); } @@ -1032,23 +1042,14 @@ const auto& samp = desc.samplers[i]; if (!checkLayout(samp.slot, BindingKind::sampler)) continue; - wgpu::BindGroupEntry& e = entries[idx++]; - e = {}; - e.binding = samp.slot; auto sampler = lite_rtti_cast<SamplerWGPU*>(samp.sampler); assert(sampler != nullptr); - e.sampler = sampler->m_wgpuSampler; + bg->m_sampEntries.push_back({samp.slot, sampler->m_wgpuSampler}); bg->m_retainedSamplers.push_back(ref_rcp(sampler)); } - wgpu::BindGroupDescriptor bgDesc{}; - bgDesc.label = desc.label; - bgDesc.layout = layout->m_wgpuBGL; - bgDesc.entryCount = idx; - bgDesc.entries = (idx > 0) ? entries.data() : nullptr; - - bg->m_wgpuBindGroup = m_wgpuDevice.CreateBindGroup(&bgDesc); - if (bg->m_wgpuBindGroup == nullptr) + // Build the initial bind group against the current backings. + if (bg->resolveBindGroup() == nullptr) return nullptr; return bg;
diff --git a/renderer/src/ore/wgpu/ore_render_pass_wgpu.cpp b/renderer/src/ore/wgpu/ore_render_pass_wgpu.cpp index fb8b20b..1681cff 100644 --- a/renderer/src/ore/wgpu/ore_render_pass_wgpu.cpp +++ b/renderer/src/ore/wgpu/ore_render_pass_wgpu.cpp
@@ -80,8 +80,9 @@ validate(); auto buffer = lite_rtti_cast<BufferWGPU*>(inBuffer); assert(buffer); + buffer->markBound(); m_wgpuPassEncoder.SetVertexBuffer(slot, - buffer->m_wgpuBuffer, + buffer->current(), offset, buffer->size() - offset); } @@ -96,10 +97,11 @@ wgpu::IndexFormat wFmt = (format == IndexFormat::uint32) ? wgpu::IndexFormat::Uint32 : wgpu::IndexFormat::Uint16; - m_wgpuIndexBuffer = buffer->m_wgpuBuffer; + buffer->markBound(); + m_wgpuIndexBuffer = buffer->current(); m_wgpuIndexFormat = wFmt; m_wgpuIndexOffset = offset; - m_wgpuPassEncoder.SetIndexBuffer(buffer->m_wgpuBuffer, + m_wgpuPassEncoder.SetIndexBuffer(m_wgpuIndexBuffer, wFmt, offset, buffer->size() - offset); @@ -113,8 +115,22 @@ validate(); auto bg = lite_rtti_cast<BindGroupWGPU*>(inBg); assert(bg != nullptr); + // Resolve the live backing of each UBO. Only stamp the UBOs bound after a + // successful resolve, so a failed (OOM) resolve doesn't force later + // update()s to orphan needlessly. + const wgpu::BindGroup& wgpuBg = bg->resolveBindGroup(); + if (wgpuBg == nullptr) + { + // Bind group creation failed (device OOM). Report instead of binding + // null; the Lua layer reads lastError after the call. + m_wgpuContext->setLastError( + "ore: WGPU bind group creation failed for group %u", + groupIndex); + return; + } + bg->markUBOsBound(); m_wgpuPassEncoder.SetBindGroup(groupIndex, - bg->m_wgpuBindGroup, + wgpuBg, dynamicOffsetCount, dynamicOffsets);
diff --git a/tests/gm/gmmain.cpp b/tests/gm/gmmain.cpp index 36dc0ce..ae21375 100644 --- a/tests/gm/gmmain.cpp +++ b/tests/gm/gmmain.cpp
@@ -207,6 +207,7 @@ MAKE_GM(ore_binding_shadow_sampler_d32) MAKE_GM(ore_binding_vs_texture) MAKE_GM(ore_binding_dynamic_ubo) + MAKE_GM(ore_buffer_update_between_draws) MAKE_GM(ore_msaa_resolve) MAKE_GM(ore_blend_stencil) MAKE_GM(ore_array_upload)
diff --git a/tests/gm/ore_buffer_update_between_draws.cpp b/tests/gm/ore_buffer_update_between_draws.cpp new file mode 100644 index 0000000..d64ec09 --- /dev/null +++ b/tests/gm/ore_buffer_update_between_draws.cpp
@@ -0,0 +1,228 @@ +/* + * Copyright 2026 Rive + * + * GM test / regression witness for the GPUBuffer per-frame update race + * (`/Users/luigi/Projects/plan/Ore/Ore — GPUBuffer Per-Frame Update Race + * & Fix Plan.md`). + * + * Deterministic, intra-frame proxy for the cross-frame timing race: the + * runtime memcpy's both UBO values into the buffer before the GPU runs, + * so a backend that updates one shared GPU-visible allocation in place + * (Metal/Vulkan/D3D12) shows the LAST value for BOTH draws. + * + * The GM (64x32): + * - write UBO color = red + * - pass 1: clear black, scissor the left half, draw a fullscreen + * triangle whose fs returns the UBO color -> left half red + * - write UBO color = green (in-place overwrite — the bug trigger) + * - pass 2: load, scissor the right half, re-bind the SAME bind group, + * draw -> right half green + * + * Expected (golden): left red | right green. + * On the unfixed runtime: left green | right green (both draws read the + * final memcpy). + * + * Cross-backend prediction: fails (green|green) on Metal/Vulkan/D3D12, + * passes (red|green) on D3D11 (MAP_WRITE_DISCARD renames) and GL + * (glBufferSubData is ordered with the command stream). + * + * Reuses the `kBindingWitness` shader: u_low carries the color we vary, + * u_high is left zero, so the fs output (u_low + u_high) equals the color + * written to u_low. This avoids adding a new RSTB shader. + */ + +#include "gm.hpp" +#include "gmutils.hpp" +#include "ore_gm_helper.hpp" +#if defined(ORE_BACKEND_METAL) || defined(ORE_BACKEND_GL) || \ + defined(ORE_BACKEND_VK) || defined(ORE_BACKEND_WGPU) || \ + defined(ORE_BACKEND_D3D11) || defined(ORE_BACKEND_D3D12) +#include "rive/renderer/render_canvas.hpp" +#include "rive/renderer/ore/ore_buffer.hpp" +#include "rive/renderer/ore/ore_bind_group.hpp" +#include "rive/renderer/ore/ore_shader_module.hpp" +#include "rive/renderer/ore/ore_pipeline.hpp" +#include "rive/renderer/ore/ore_render_pass.hpp" +#endif + +using namespace rivegm; +using namespace rive; +using namespace rive::gpu; +#if defined(ORE_BACKEND_METAL) || defined(ORE_BACKEND_GL) || \ + defined(ORE_BACKEND_VK) || defined(ORE_BACKEND_WGPU) || \ + defined(ORE_BACKEND_D3D11) || defined(ORE_BACKEND_D3D12) +using namespace rive::ore; +#endif + +#if defined(ORE_BACKEND_METAL) || defined(ORE_BACKEND_GL) || \ + defined(ORE_BACKEND_VK) || defined(ORE_BACKEND_WGPU) || \ + defined(ORE_BACKEND_D3D11) || defined(ORE_BACKEND_D3D12) +#define ORE_BUFFER_UPDATE_BETWEEN_DRAWS_ACTIVE +#endif + +class OreBufferUpdateBetweenDrawsGM : public GM +{ +public: + OreBufferUpdateBetweenDrawsGM() : GM(64, 32) {} + + ColorInt clearColor() const override { return 0xff000000; } + + void onDraw(rive::Renderer* originalRenderer) override + { + auto renderContext = TestingWindow::Get()->renderContext(); + if (!renderContext || !m_ore.ensureContext(renderContext)) + return; + +#ifdef ORE_BUFFER_UPDATE_BETWEEN_DRAWS_ACTIVE + auto& ctx = *renderContext->getOreContext(); + + struct Uniforms + { + float r, g, b, a; + }; + static const Uniforms kRed = {1.0f, 0.0f, 0.0f, 0.0f}; + static const Uniforms kGreen = {0.0f, 1.0f, 0.0f, 0.0f}; + static const Uniforms kZero = {0.0f, 0.0f, 0.0f, 0.0f}; + + // The color UBO we overwrite between the two draws. + BufferDesc colorDesc{}; + colorDesc.usage = BufferUsage::uniform; + colorDesc.size = sizeof(Uniforms); + colorDesc.data = &kRed; + colorDesc.label = "update_between_draws_color"; + auto colorBuf = ctx.makeBuffer(colorDesc); + + // Constant zero UBO so the shader output equals the color UBO. + BufferDesc zeroDesc{}; + zeroDesc.usage = BufferUsage::uniform; + zeroDesc.size = sizeof(Uniforms); + zeroDesc.data = &kZero; + zeroDesc.label = "update_between_draws_zero"; + auto zeroBuf = ctx.makeBuffer(zeroDesc); + + auto shader = ore_gm::loadShader(ctx, ore_gm::kBindingWitness); + if (!shader.vsModule) + return; + + auto layout0 = + ore_gm::makeLayoutFromShader(ctx, shader.vsModule.get(), 0); + BindGroupLayout* layouts[] = {layout0.get()}; + + PipelineDesc pipeDesc{}; + pipeDesc.vertexModule = shader.vsModule.get(); + pipeDesc.fragmentModule = shader.psModule.get(); + pipeDesc.vertexEntryPoint = shader.vsEntryPoint; + pipeDesc.fragmentEntryPoint = shader.fsEntryPoint; + pipeDesc.vertexBufferCount = 0; + pipeDesc.topology = PrimitiveTopology::triangleList; + pipeDesc.colorTargets[0].format = TextureFormat::rgba8unorm; + pipeDesc.colorCount = 1; + pipeDesc.depthStencil.depthCompare = CompareFunction::always; + pipeDesc.depthStencil.depthWriteEnabled = false; + pipeDesc.bindGroupLayouts = layouts; + pipeDesc.bindGroupLayoutCount = 1; + pipeDesc.label = "ore_buffer_update_between_draws_pipeline"; + auto pipeline = ctx.makePipeline(pipeDesc); + if (!pipeline) + { + fprintf(stderr, + "[ore_buffer_update_between_draws] pipeline creation " + "failed: %s\n", + ctx.lastError().c_str()); + return; + } + + BindGroupDesc::UBOEntry uboEntries[2]{}; + uboEntries[0].slot = 0; // u_low, the color we vary + uboEntries[0].buffer = colorBuf.get(); + uboEntries[0].offset = 0; + uboEntries[0].size = sizeof(Uniforms); + uboEntries[1].slot = 7; // u_high, constant zero + uboEntries[1].buffer = zeroBuf.get(); + uboEntries[1].offset = 0; + uboEntries[1].size = sizeof(Uniforms); + + BindGroupDesc bgDesc{}; + bgDesc.layout = layout0.get(); + bgDesc.ubos = uboEntries; + bgDesc.uboCount = 2; + bgDesc.label = "update_between_draws_bg"; + auto bg = ctx.makeBindGroup(bgDesc); + if (!bg) + return; + + auto canvas = renderContext->makeRenderCanvas(64, 32); + if (!canvas) + return; + + auto canvasTarget = ctx.wrapCanvasTexture(canvas.get()); + if (!canvasTarget) + return; + + m_ore.beginFrame(renderContext); + + // ── Pass 1: red into the left half. ── + colorBuf->update(&kRed, sizeof(kRed), 0); + { + ColorAttachment ca{}; + ca.view = canvasTarget.get(); + ca.loadOp = LoadOp::clear; + ca.storeOp = StoreOp::store; + ca.clearColor = {0, 0, 0, 1}; + + RenderPassDesc rpDesc{}; + rpDesc.colorAttachments[0] = ca; + rpDesc.colorCount = 1; + rpDesc.label = "ore_buffer_update_between_draws_pass1"; + + auto pass = ctx.beginRenderPass(rpDesc); + pass->setPipeline(pipeline.get()); + pass->setBindGroup(0, bg.get()); + pass->setViewport(0, 0, 64, 32); + pass->setScissorRect(0, 0, 32, 32); + pass->draw(3); + pass->finish(); + } + + // In-place overwrite, the bug trigger. + colorBuf->update(&kGreen, sizeof(kGreen), 0); + + // ── Pass 2: green into the right half, re-binding the SAME group. ── + { + ColorAttachment ca{}; + ca.view = canvasTarget.get(); + ca.loadOp = LoadOp::load; + ca.storeOp = StoreOp::store; + + RenderPassDesc rpDesc{}; + rpDesc.colorAttachments[0] = ca; + rpDesc.colorCount = 1; + rpDesc.label = "ore_buffer_update_between_draws_pass2"; + + auto pass = ctx.beginRenderPass(rpDesc); + pass->setPipeline(pipeline.get()); + pass->setBindGroup(0, bg.get()); + pass->setViewport(0, 0, 64, 32); + pass->setScissorRect(32, 0, 32, 32); + pass->draw(3); + pass->finish(); + } + + m_ore.endFrame(renderContext); + ore_gm::invalidateGLStateAfterOre(renderContext); + + originalRenderer->save(); + originalRenderer->drawImage(canvas->renderImage(), + {.filter = ImageFilter::nearest}, + BlendMode::srcOver, + 1); + originalRenderer->restore(); +#endif // ORE_BUFFER_UPDATE_BETWEEN_DRAWS_ACTIVE + } + +private: + ore_gm::OreGMContext m_ore; +}; + +GMREGISTER(ore_buffer_update_between_draws, + return new OreBufferUpdateBetweenDrawsGM)
diff --git a/tests/gm/ore_buffer_update_between_draws.mm b/tests/gm/ore_buffer_update_between_draws.mm new file mode 100644 index 0000000..2c8bda2 --- /dev/null +++ b/tests/gm/ore_buffer_update_between_draws.mm
@@ -0,0 +1,2 @@ +// Obj-C++ wrapper — ore headers pull in <Metal/Metal.h> on Apple. +#include "ore_buffer_update_between_draws.cpp"
diff --git a/tests/unit_tests/premake5.lua b/tests/unit_tests/premake5.lua index 79b5bf4..8c24978 100644 --- a/tests/unit_tests/premake5.lua +++ b/tests/unit_tests/premake5.lua
@@ -120,6 +120,10 @@ 'CoreGraphics.framework', 'CoreText.framework', }) + -- The ore helper pulls in <Metal/Metal.h>, so compile this test as + -- Obj-C++ on Apple instead of the C++ glob. + files({ 'renderer/ore_buffer_race_test.mm' }) + removefiles({ 'renderer/ore_buffer_race_test.cpp' }) end filter({ 'toolset:not msc' })
diff --git a/tests/unit_tests/renderer/ore_buffer_race_test.cpp b/tests/unit_tests/renderer/ore_buffer_race_test.cpp new file mode 100644 index 0000000..783f32e --- /dev/null +++ b/tests/unit_tests/renderer/ore_buffer_race_test.cpp
@@ -0,0 +1,295 @@ +/* + * Copyright 2026 Rive + * + * Multi-frame regression test for the Ore GPUBuffer per-frame update race fix + * (PR #12971). Unlike the ore_buffer_update_between_draws GM, which is a single + * host frame compared against a golden, this drives several real host frames + * with a persistent buffer and asserts read-back pixels in code, so it can + * exercise paths the GM structurally cannot: + * + * - WebGPU partial-update shadow seeding, and the Metal/Vulkan/D3D12 + * copy-forward: bind, then a PARTIAL update that orphans, must keep the + * untouched bytes. The witness color makes a regression visible — a draw + * that lost the preserved channels renders red instead of yellow. + * - Cross-frame backing reclaim: a backing bound in frame N is reused in a + * later frame; reusing one the GPU still reads would corrupt the pixel. + * - The D3D12 reuse-the-just-retired-backing self-copy guard (bind in one + * frame, update in the next), which aborts an ASan build if it regresses. + * + * Runs on the racy backends the fix changed (Metal, D3D12, Vulkan, WebGPU) that + * are both compiled in and creatable headless: Metal in the macOS job, D3D12 in + * the Windows job, Vulkan/WebGPU when their build flags are set. GL and D3D11 + * are unchanged by the fix and are intentionally not exercised here. + */ + +#include "common/testing_window.hpp" +#include "gm/ore_gm_helper.hpp" +#include <catch.hpp> + +#if defined(ORE_BACKEND_METAL) || defined(ORE_BACKEND_VK) || \ + defined(ORE_BACKEND_WGPU) || defined(ORE_BACKEND_D3D12) +#include "rive/renderer/render_context.hpp" +#include "rive/renderer/render_canvas.hpp" +#include "rive/renderer/ore/ore_buffer.hpp" +#include "rive/renderer/ore/ore_bind_group.hpp" +#include "rive/renderer/ore/ore_shader_module.hpp" +#include "rive/renderer/ore/ore_pipeline.hpp" +#include "rive/renderer/ore/ore_render_pass.hpp" +#define ORE_BUFFER_RACE_TEST_ACTIVE +#endif + +#ifdef ORE_BUFFER_RACE_TEST_ACTIVE +using namespace rive; +using namespace rive::gpu; +using namespace rive::ore; + +namespace +{ +struct Uniforms +{ + float r, g, b, a; +}; +constexpr Uniforms kGreen = {0.0f, 1.0f, 0.0f, 1.0f}; +constexpr Uniforms kZero = {0.0f, 0.0f, 0.0f, 0.0f}; +constexpr int kW = 64; +constexpr int kH = 32; + +// The racy backends the fix changed. The test runs on each that is compiled in +// and creatable headless, so one job covers every such backend it built. GL and +// D3D11 update in place correctly and are deliberately excluded; GL also has no +// headless path on the Linux unit-test runner. +const TestingWindow::Backend kRacyBackends[] = { +#if defined(ORE_BACKEND_METAL) + TestingWindow::Backend::metal, +#endif +#if defined(ORE_BACKEND_D3D12) + TestingWindow::Backend::d3d12, +#endif +#if defined(ORE_BACKEND_VK) + TestingWindow::Backend::vk, +#endif +#if defined(ORE_BACKEND_WGPU) + TestingWindow::Backend::dawn, +#endif +}; + +// A channel is "high" near 255 and "low" near 0, with slack for rgba8 rounding +// and the srcOver composite of the canvas onto the window target. +bool high(uint8_t c) { return c >= 0xf0; } +bool low(uint8_t c) { return c <= 0x10; } + +void runScenario(TestingWindow* window) +{ + window->resize(kW, kH); + auto* renderContext = window->renderContext(); + + ore_gm::OreGMContext oreGM; + REQUIRE(oreGM.ensureContext(renderContext)); + auto& ctx = *renderContext->getOreContext(); + + // Persistent across every frame — that is what makes the cross-frame paths + // reachable. Seeded green; a later partial update only rewrites red. + BufferDesc colorDesc{}; + colorDesc.usage = BufferUsage::uniform; + colorDesc.size = sizeof(Uniforms); + colorDesc.data = &kGreen; + colorDesc.label = "ore_buffer_race_color"; + auto colorBuf = ctx.makeBuffer(colorDesc); + REQUIRE(colorBuf != nullptr); + + BufferDesc zeroDesc{}; + zeroDesc.usage = BufferUsage::uniform; + zeroDesc.size = sizeof(Uniforms); + zeroDesc.data = &kZero; + zeroDesc.label = "ore_buffer_race_zero"; + auto zeroBuf = ctx.makeBuffer(zeroDesc); + REQUIRE(zeroBuf != nullptr); + + auto shader = ore_gm::loadShader(ctx, ore_gm::kBindingWitness); + REQUIRE(shader.vsModule); + + auto layout0 = ore_gm::makeLayoutFromShader(ctx, shader.vsModule.get(), 0); + BindGroupLayout* layouts[] = {layout0.get()}; + + PipelineDesc pipeDesc{}; + pipeDesc.vertexModule = shader.vsModule.get(); + pipeDesc.fragmentModule = shader.psModule.get(); + pipeDesc.vertexEntryPoint = shader.vsEntryPoint; + pipeDesc.fragmentEntryPoint = shader.fsEntryPoint; + pipeDesc.vertexBufferCount = 0; + pipeDesc.topology = PrimitiveTopology::triangleList; + pipeDesc.colorTargets[0].format = TextureFormat::rgba8unorm; + pipeDesc.colorCount = 1; + pipeDesc.depthStencil.depthCompare = CompareFunction::always; + pipeDesc.depthStencil.depthWriteEnabled = false; + pipeDesc.bindGroupLayouts = layouts; + pipeDesc.bindGroupLayoutCount = 1; + pipeDesc.label = "ore_buffer_race_pipeline"; + auto pipeline = ctx.makePipeline(pipeDesc); + REQUIRE(pipeline != nullptr); + + BindGroupDesc::UBOEntry uboEntries[2]{}; + uboEntries[0].slot = 0; // u_low, the color we vary + uboEntries[0].buffer = colorBuf.get(); + uboEntries[0].offset = 0; + uboEntries[0].size = sizeof(Uniforms); + uboEntries[1].slot = 7; // u_high, constant zero + uboEntries[1].buffer = zeroBuf.get(); + uboEntries[1].offset = 0; + uboEntries[1].size = sizeof(Uniforms); + + BindGroupDesc bgDesc{}; + bgDesc.layout = layout0.get(); + bgDesc.ubos = uboEntries; + bgDesc.uboCount = 2; + bgDesc.label = "ore_buffer_race_bg"; + auto bg = ctx.makeBindGroup(bgDesc); + REQUIRE(bg != nullptr); + + auto canvas = renderContext->makeRenderCanvas(kW, kH); + REQUIRE(canvas != nullptr); + auto canvasTarget = ctx.wrapCanvasTexture(canvas.get()); + REQUIRE(canvasTarget != nullptr); + + // Runs one host frame. Pass 1 draws the left half from the bound color, + // then optionally a partial update rewrites only red and orphans, then pass + // 2 draws the right half from the live (post-orphan) backing. + auto runFrame = [&](bool partialUpdate) -> std::vector<uint8_t> { + auto renderer = window->beginFrame({ + .name = "ore_buffer_race", + .clearColor = 0xff000000, + .doClear = true, + }); + oreGM.beginFrame(renderContext); + + // Reset to green up front. Not bound yet this frame on the first frame; + // on later frames this is itself an update-after-bind, exercising the + // full-overwrite orphan (skip-copy) and cross-frame reclaim. + colorBuf->update(&kGreen, sizeof(kGreen), 0); + + { + ColorAttachment ca{}; + ca.view = canvasTarget.get(); + ca.loadOp = LoadOp::clear; + ca.storeOp = StoreOp::store; + ca.clearColor = {0, 0, 0, 1}; + RenderPassDesc rpDesc{}; + rpDesc.colorAttachments[0] = ca; + rpDesc.colorCount = 1; + rpDesc.label = "ore_buffer_race_pass1"; + auto pass = ctx.beginRenderPass(rpDesc); + pass->setPipeline(pipeline.get()); + pass->setBindGroup(0, bg.get()); + pass->setViewport(0, 0, kW, kH); + pass->setScissorRect(0, 0, kW / 2, kH); + pass->draw(3); + pass->finish(); + } + + if (partialUpdate) + { + // Rewrite only red. The orphan must keep green/blue/alpha; a + // backend that zeroes the untouched bytes renders red on the right. + const float red = 1.0f; + colorBuf->update(&red, sizeof(red), 0); + } + + { + ColorAttachment ca{}; + ca.view = canvasTarget.get(); + ca.loadOp = LoadOp::load; + ca.storeOp = StoreOp::store; + RenderPassDesc rpDesc{}; + rpDesc.colorAttachments[0] = ca; + rpDesc.colorCount = 1; + rpDesc.label = "ore_buffer_race_pass2"; + auto pass = ctx.beginRenderPass(rpDesc); + pass->setPipeline(pipeline.get()); + pass->setBindGroup(0, bg.get()); + pass->setViewport(0, 0, kW, kH); + pass->setScissorRect(kW / 2, 0, kW / 2, kH); + pass->draw(3); + pass->finish(); + } + + oreGM.endFrame(renderContext); + ore_gm::invalidateGLStateAfterOre(renderContext); + + renderer->drawImage(canvas->renderImage(), + {.filter = ImageFilter::nearest}, + BlendMode::srcOver, + 1); + std::vector<uint8_t> pixels; + window->endFrame(&pixels); + return pixels; + }; + + auto pixelAt = [](const std::vector<uint8_t>& px, int x, int y) { + return &px[(static_cast<size_t>(y) * kW + x) * 4]; + }; + + // Phase 1: several frames of bind → partial-update-orphan → bind. The left + // half stays green (bound before the update); the right half is yellow only + // if the orphan preserved green. Persisting across frames also reuses prior + // frames' backings. + for (int frame = 0; frame < 4; ++frame) + { + auto px = runFrame(/*partialUpdate=*/true); + REQUIRE(px.size() == static_cast<size_t>(kW) * kH * 4); + const uint8_t* left = pixelAt(px, kW / 4, kH / 2); + const uint8_t* right = pixelAt(px, 3 * kW / 4, kH / 2); + + INFO("frame " << frame << " left=" << +left[0] << "," << +left[1] << "," + << +left[2] << " right=" << +right[0] << "," << +right[1] + << "," << +right[2]); + // Left: bound-before-update snapshot is green. + CHECK(low(left[0])); + CHECK(high(left[1])); + CHECK(low(left[2])); + // Right: post-orphan backing is yellow — green preserved through the + // partial red rewrite. A preservation bug drops green here. + CHECK(high(right[0])); + CHECK(high(right[1])); + CHECK(low(right[2])); + } + + // Phase 2: bind in one frame with no trailing update, then update at the + // start of the next. The orphan reuses the just-retired backing, which is + // the D3D12 self-copy guard's path (a regression aborts under ASan). No + // partial update, so both halves stay green; assert it still renders. + runFrame(/*partialUpdate=*/false); + { + auto px = runFrame(/*partialUpdate=*/false); + const uint8_t* left = pixelAt(px, kW / 4, kH / 2); + const uint8_t* right = pixelAt(px, 3 * kW / 4, kH / 2); + CHECK(low(left[0])); + CHECK(high(left[1])); + CHECK(low(right[0])); + CHECK(high(right[1])); + } +} +} // namespace + +TEST_CASE("ore buffer per-frame update race", "[ore]") +{ + int ran = 0; + for (auto backend : kRacyBackends) + { + auto* window = TestingWindow::Init(backend, + {}, + TestingWindow::Visibility::headless); + if (window == nullptr || window->renderContext() == nullptr || + !ore_gm::isOreBackendActive()) + { + TestingWindow::Destroy(); + continue; + } + INFO("backend " << TestingWindow::BackendName(backend)); + runScenario(window); + ++ran; + TestingWindow::Destroy(); + } + if (ran == 0) + WARN("no racy Ore backend available headless; skipping"); +} +#endif // ORE_BUFFER_RACE_TEST_ACTIVE
diff --git a/tests/unit_tests/renderer/ore_buffer_race_test.mm b/tests/unit_tests/renderer/ore_buffer_race_test.mm new file mode 100644 index 0000000..f448fb3 --- /dev/null +++ b/tests/unit_tests/renderer/ore_buffer_race_test.mm
@@ -0,0 +1,2 @@ +// Obj-C++ wrapper — the ore helper pulls in <Metal/Metal.h> on Apple. +#include "ore_buffer_race_test.cpp"