The MTLBuffers that serve as argument buffers in the GPUBindGroup backend shouldn't be created until the GPUBindGroup is created. Each GPUBindGroup requires its own unique MTLBuffer(s), but a GPUBindGroupLayout can be used by different GPUBindGroups or GPUPipelineLayouts.
<rdar://problem/48781297>
Created attachment 364287 [details] Patch
Created attachment 364298 [details] Patch
Comment on attachment 364298 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364298&action=review > Source/WebCore/platform/graphics/gpu/GPUBindGroup.h:60 > + RetainPtr<MTLBuffer> m_vertexArgsBuffer; > + RetainPtr<MTLBuffer> m_fragmentArgsBuffer; Compute :( > Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm:66 > +static RetainPtr<MTLArgumentEncoder> tryCreateMtlArgumentEncoder(const GPUDevice& device, ArgumentArray array) Do you need to say it's Mtl? It's in a Metal file already. > Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupMetal.mm:202 > + case GPUBindGroupLayoutBinding::BindingType::StorageBuffer: { > + auto bufferResource = tryGetResourceAsBufferBinding(resourceBinding.resource, functionName); > + if (!bufferResource) > + return nullptr; > + if (layoutBinding.visibility & GPUShaderStageBit::Flags::Vertex) { > + if (!trySetBufferOnEncoder(vertexEncoder, *bufferResource, resourceBinding.binding, functionName)) > + return nullptr; > + } > + if (layoutBinding.visibility & GPUShaderStageBit::Flags::Fragment) { > + if (!trySetBufferOnEncoder(fragmentEncoder, *bufferResource, resourceBinding.binding, functionName)) > + return nullptr; > + } > + boundBuffers.append(bufferResource->buffer.copyRef()); > + break; > + } Can we collapse the cases somehow? There seems to be a lot of duplicated code. It seems wrong that we mark the buffer as bound even if visibility == NONE. > Source/WebCore/platform/graphics/gpu/cocoa/GPUProgrammablePassEncoderMetal.mm:60 > + if (bindGroup.vertexArgsBuffer()) > + setVertexBuffer(bindGroup.vertexArgsBuffer(), 0, index); > + if (bindGroup.fragmentArgsBuffer()) > + setFragmentBuffer(bindGroup.fragmentArgsBuffer(), 0, index); Yes! This is awesome.
Created attachment 364336 [details] Patch for landing
Comment on attachment 364336 [details] Patch for landing Clearing flags on attachment: 364336 Committed r242766: <https://trac.webkit.org/changeset/242766>
All reviewed patches have been landed. Closing bug.