From d973602edb4be0bcce3bdfce8d82487e1227a9bc Mon Sep 17 00:00:00 2001 From: Chip Davis Date: Mon, 17 Dec 2018 13:38:39 -0600 Subject: [PATCH] Use an inline constant buffer for the auxiliary buffer. Since a pipeline might be shared between multiple draws, but those draws could use different textures, it isn't tenable to have a single aux buffer shared between those draws. Instead, we'll set up constant buffers using the `setXxxBytes()` method on the command encoder. That way, separate draws using the same pipeline won't interfere with each other. This change also uses separate buffers for the vertex and fragment shaders, since they may have different sets of textures. --- .../Commands/MVKCommandEncoderState.h | 19 ++++--- .../Commands/MVKCommandEncoderState.mm | 54 +++++++++---------- MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h | 5 -- MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm | 15 ++---- 4 files changed, 37 insertions(+), 56 deletions(-) diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h b/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h index a87a7e0c..5ae6871d 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h +++ b/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.h @@ -391,14 +391,10 @@ protected: } } - struct AuxBuffer { - uint32_t swizzleConst[1]; - }; - - // Updates the swizzle for an image in the given buffer. - void updateSwizzle(id buffer, uint32_t index, uint32_t swizzle) { - auto* aux = (AuxBuffer*)buffer.contents; - aux->swizzleConst[index] = swizzle; + // Updates the swizzle for an image in the given vector. + void updateSwizzle(MVKVector &constants, uint32_t index, uint32_t swizzle) { + if (index >= constants.size()) { constants.resize(index + 1); } + constants[index] = swizzle; } }; @@ -438,7 +434,7 @@ public: } /** Sets the current auxiliary buffer state. */ - void bindAuxBuffer(id buffer, const MVKShaderAuxBufferBinding& binding, bool needVertexAuxBuffer, bool needFragmentAuxBuffer); + void bindAuxBuffer(const MVKShaderAuxBufferBinding& binding, bool needVertexAuxBuffer, bool needFragmentAuxBuffer); #pragma mark Construction @@ -457,6 +453,8 @@ protected: MVKVector _fragmentTextureBindings; MVKVector _vertexSamplerStateBindings; MVKVector _fragmentSamplerStateBindings; + MVKVector _vertexSwizzleConstants; + MVKVector _fragmentSwizzleConstants; MVKMTLBufferBinding _vertexAuxBufferBinding; MVKMTLBufferBinding _fragmentAuxBufferBinding; @@ -487,7 +485,7 @@ public: void bindSamplerState(const MVKMTLSamplerStateBinding& binding); /** Sets the current auxiliary buffer state. */ - void bindAuxBuffer(id buffer, const MVKShaderAuxBufferBinding& binding); + void bindAuxBuffer(const MVKShaderAuxBufferBinding& binding); #pragma mark Construction @@ -502,6 +500,7 @@ protected: MVKVector _bufferBindings; MVKVector _textureBindings; MVKVector _samplerStateBindings; + MVKVector _swizzleConstants; MVKMTLBufferBinding _auxBufferBinding; bool _areBufferBindingsDirty = false; diff --git a/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm b/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm index f50843fb..90cc5d41 100644 --- a/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm +++ b/MoltenVK/MoltenVK/Commands/MVKCommandEncoderState.mm @@ -425,11 +425,9 @@ void MVKGraphicsResourcesCommandEncoderState::bindFragmentSamplerState(const MVK bind(binding, _fragmentSamplerStateBindings, _areFragmentSamplerStateBindingsDirty); } -void MVKGraphicsResourcesCommandEncoderState::bindAuxBuffer(id buffer, const MVKShaderAuxBufferBinding& binding, bool needVertexAuxBuffer, bool needFragmentAuxBuffer) { - _vertexAuxBufferBinding.mtlBuffer = needVertexAuxBuffer ? buffer : nil; +void MVKGraphicsResourcesCommandEncoderState::bindAuxBuffer(const MVKShaderAuxBufferBinding& binding, bool needVertexAuxBuffer, bool needFragmentAuxBuffer) { _vertexAuxBufferBinding.index = binding.vertex; _vertexAuxBufferBinding.isDirty = needVertexAuxBuffer; - _fragmentAuxBufferBinding.mtlBuffer = needFragmentAuxBuffer ? buffer : nil; _fragmentAuxBufferBinding.index = binding.fragment; _fragmentAuxBufferBinding.isDirty = needFragmentAuxBuffer; } @@ -443,23 +441,21 @@ void MVKGraphicsResourcesCommandEncoderState::markDirty() { MVKResourcesCommandEncoderState::markDirty(_fragmentTextureBindings, _areFragmentTextureBindingsDirty); MVKResourcesCommandEncoderState::markDirty(_vertexSamplerStateBindings, _areVertexSamplerStateBindingsDirty); MVKResourcesCommandEncoderState::markDirty(_fragmentSamplerStateBindings, _areFragmentSamplerStateBindingsDirty); - _vertexAuxBufferBinding.isDirty = true; - _fragmentAuxBufferBinding.isDirty = true; } void MVKGraphicsResourcesCommandEncoderState::encodeImpl() { - if (_vertexAuxBufferBinding.mtlBuffer) { + if (_vertexAuxBufferBinding.isDirty) { for (auto& b : _vertexTextureBindings) { if (b.isDirty) - updateSwizzle(_vertexAuxBufferBinding.mtlBuffer, b.index, b.swizzle); + updateSwizzle(_vertexSwizzleConstants, b.index, b.swizzle); } } - if (_fragmentAuxBufferBinding.mtlBuffer) { + if (_fragmentAuxBufferBinding.isDirty) { for (auto& b : _fragmentTextureBindings) { if (b.isDirty) - updateSwizzle(_fragmentAuxBufferBinding.mtlBuffer, b.index, b.swizzle); + updateSwizzle(_fragmentSwizzleConstants, b.index, b.swizzle); } } @@ -478,17 +474,17 @@ void MVKGraphicsResourcesCommandEncoderState::encodeImpl() { }); if (_vertexAuxBufferBinding.isDirty) { - _vertexAuxBufferBinding.isDirty = false; - [_cmdEncoder->_mtlRenderEncoder setVertexBuffer: _vertexAuxBufferBinding.mtlBuffer - offset: 0 - atIndex: _vertexAuxBufferBinding.index]; + _cmdEncoder->setVertexBytes(_cmdEncoder->_mtlRenderEncoder, + _vertexSwizzleConstants.data(), + _vertexSwizzleConstants.size() * sizeof(uint32_t), + _vertexAuxBufferBinding.index); } if (_fragmentAuxBufferBinding.isDirty) { - _fragmentAuxBufferBinding.isDirty = false; - [_cmdEncoder->_mtlRenderEncoder setFragmentBuffer: _fragmentAuxBufferBinding.mtlBuffer - offset: 0 - atIndex: _fragmentAuxBufferBinding.index]; + _cmdEncoder->setFragmentBytes(_cmdEncoder->_mtlRenderEncoder, + _fragmentSwizzleConstants.data(), + _fragmentSwizzleConstants.size() * sizeof(uint32_t), + _fragmentAuxBufferBinding.index); } encodeBinding(_vertexTextureBindings, _areVertexTextureBindingsDirty, @@ -523,8 +519,8 @@ void MVKGraphicsResourcesCommandEncoderState::resetImpl() { _fragmentTextureBindings.clear(); _vertexSamplerStateBindings.clear(); _fragmentSamplerStateBindings.clear(); - _vertexAuxBufferBinding.mtlBuffer = nil; - _fragmentAuxBufferBinding.mtlBuffer = nil; + _vertexSwizzleConstants.clear(); + _fragmentSwizzleConstants.clear(); _areVertexBufferBindingsDirty = false; _areFragmentBufferBindingsDirty = false; @@ -552,10 +548,9 @@ void MVKComputeResourcesCommandEncoderState::bindSamplerState(const MVKMTLSample bind(binding, _samplerStateBindings, _areSamplerStateBindingsDirty); } -void MVKComputeResourcesCommandEncoderState::bindAuxBuffer(id buffer, const MVKShaderAuxBufferBinding& binding) { - _auxBufferBinding.mtlBuffer = buffer; +void MVKComputeResourcesCommandEncoderState::bindAuxBuffer(const MVKShaderAuxBufferBinding& binding) { _auxBufferBinding.index = binding.compute; - _auxBufferBinding.isDirty = buffer != nil; + _auxBufferBinding.isDirty = true; } // Mark everything as dirty @@ -564,15 +559,14 @@ void MVKComputeResourcesCommandEncoderState::markDirty() { MVKResourcesCommandEncoderState::markDirty(_bufferBindings, _areBufferBindingsDirty); MVKResourcesCommandEncoderState::markDirty(_textureBindings, _areTextureBindingsDirty); MVKResourcesCommandEncoderState::markDirty(_samplerStateBindings, _areSamplerStateBindingsDirty); - _auxBufferBinding.isDirty = true; } void MVKComputeResourcesCommandEncoderState::encodeImpl() { - if (_auxBufferBinding.mtlBuffer) { + if (_auxBufferBinding.isDirty) { for (auto& b : _textureBindings) { if (b.isDirty) - updateSwizzle(_auxBufferBinding.mtlBuffer, b.index, b.swizzle); + updateSwizzle(_swizzleConstants, b.index, b.swizzle); } } @@ -584,10 +578,10 @@ void MVKComputeResourcesCommandEncoderState::encodeImpl() { }); if (_auxBufferBinding.isDirty) { - _auxBufferBinding.isDirty = false; - [_cmdEncoder->getMTLComputeEncoder(kMVKCommandUseDispatch) setBuffer: _auxBufferBinding.mtlBuffer - offset: 0 - atIndex: _auxBufferBinding.index]; + _cmdEncoder->setComputeBytes(_cmdEncoder->getMTLComputeEncoder(kMVKCommandUseDispatch), + _swizzleConstants.data(), + _swizzleConstants.size() * sizeof(uint32_t), + _auxBufferBinding.index); } encodeBinding(_textureBindings, _areTextureBindingsDirty, @@ -607,7 +601,7 @@ void MVKComputeResourcesCommandEncoderState::resetImpl() { _bufferBindings.clear(); _textureBindings.clear(); _samplerStateBindings.clear(); - _auxBufferBinding.mtlBuffer = nil; + _swizzleConstants.clear(); _areBufferBindingsDirty = false; _areTextureBindingsDirty = false; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h b/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h index 47618250..bcde1489 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h @@ -108,14 +108,9 @@ public: MVKPipeline(MVKDevice* device, MVKPipelineCache* pipelineCache, MVKPipeline* parent) : MVKBaseDeviceObject(device), _pipelineCache(pipelineCache) {} - ~MVKPipeline() override { - [_auxBuffer release]; - }; - protected: MVKPipelineCache* _pipelineCache; MVKShaderAuxBufferBinding _auxBufferIndex; - id _auxBuffer = nil; }; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm b/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm index 09453168..ab890826 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm @@ -181,7 +181,7 @@ void MVKGraphicsPipeline::encode(MVKCommandEncoder* cmdEncoder) { [mtlCmdEnc setDepthClipMode: _mtlDepthClipMode]; } - cmdEncoder->_graphicsResourcesState.bindAuxBuffer(_auxBuffer, _auxBufferIndex, _needsVertexAuxBuffer, _needsFragmentAuxBuffer); + cmdEncoder->_graphicsResourcesState.bindAuxBuffer(_auxBufferIndex, _needsVertexAuxBuffer, _needsFragmentAuxBuffer); } bool MVKGraphicsPipeline::supportsDynamicState(VkDynamicState state) { @@ -309,7 +309,6 @@ MTLRenderPipelineDescriptor* MVKGraphicsPipeline::getMTLRenderPipelineDescriptor auto* mvkLayout = (MVKPipelineLayout*)pCreateInfo->layout; _auxBufferIndex = mvkLayout->getAuxBufferIndex(); - uint32_t auxBufferSize = sizeof(uint32_t) * mvkLayout->getTextureCount(); // Retrieve the render subpass for which this pipeline is being constructed MVKRenderPass* mvkRendPass = (MVKRenderPass*)pCreateInfo->renderPass; @@ -366,10 +365,6 @@ MTLRenderPipelineDescriptor* MVKGraphicsPipeline::getMTLRenderPipelineDescriptor } } - if (_needsVertexAuxBuffer || _needsFragmentAuxBuffer) { - _auxBuffer = [_device->getMTLDevice() newBufferWithLength: auxBufferSize options: MTLResourceStorageModeShared]; // retained - } - // Vertex attributes uint32_t vaCnt = pCreateInfo->pVertexInputState->vertexAttributeDescriptionCount; for (uint32_t i = 0; i < vaCnt; i++) { @@ -574,7 +569,9 @@ MVKGraphicsPipeline::~MVKGraphicsPipeline() { void MVKComputePipeline::encode(MVKCommandEncoder* cmdEncoder) { [cmdEncoder->getMTLComputeEncoder(kMVKCommandUseDispatch) setComputePipelineState: _mtlPipelineState]; cmdEncoder->_mtlThreadgroupSize = _mtlThreadgroupSize; - cmdEncoder->_computeResourcesState.bindAuxBuffer(_auxBuffer, _auxBufferIndex); + if (_needsAuxBuffer) { + cmdEncoder->_computeResourcesState.bindAuxBuffer(_auxBufferIndex); + } } MVKComputePipeline::MVKComputePipeline(MVKDevice* device, @@ -613,15 +610,11 @@ MVKMTLFunction MVKComputePipeline::getMTLFunction(const VkComputePipelineCreateI MVKPipelineLayout* layout = (MVKPipelineLayout*)pCreateInfo->layout; layout->populateShaderConverterContext(shaderContext); _auxBufferIndex = layout->getAuxBufferIndex(); - uint32_t auxBufferSize = sizeof(uint32_t) * layout->getTextureCount(); shaderContext.options.auxBufferIndex = _auxBufferIndex.compute; MVKShaderModule* mvkShdrMod = (MVKShaderModule*)pSS->module; auto func = mvkShdrMod->getMTLFunction(&shaderContext, pSS->pSpecializationInfo, _pipelineCache); _needsAuxBuffer = shaderContext.options.needsAuxBuffer; - if (_needsAuxBuffer) { - _auxBuffer = [_device->getMTLDevice() newBufferWithLength: auxBufferSize options: MTLResourceStorageModeShared]; // retained - } return func; }