Skip to content

Commit

Permalink
Merge pull request #2248 from aitor-lunarg/apply-spirv-workarounds
Browse files Browse the repository at this point in the history
Apply SPIRV-Cross workarounds for Metal bugs
  • Loading branch information
billhollings committed Jun 28, 2024
2 parents e6b7806 + 190e993 commit c6373b8
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 2 deletions.
1 change: 1 addition & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ class MVKGraphicsPipeline : public MVKPipeline {
bool _isRasterizingColor = false;
bool _sampleLocationsEnable = false;
bool _isTessellationPipeline = false;
bool _inputAttachmentIsDSAttachment = false;
};


Expand Down
14 changes: 14 additions & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,16 @@
_outputControlPointCount = reflectData.numControlPoints;
mvkSetOrClear(&_tessInfo, _isTessellationPipeline ? pCreateInfo->pTessellationState : nullptr);

// Handles depth attachment being used as input attachment. However, it does not solve the issue when
// the pipeline is created without render pass (dynamic rendering) since we won't be able to know
// which resources will be used when rendering. Needs to be done before we do shaders
// Potential solution would be to generate 2 pipelines, one with the workaround for the Metal issue
// and one without it, and decide at bind time once we know the resources which one to use.
if (pCreateInfo->renderPass) {
MVKRenderSubpass* subpass = ((MVKRenderPass*)pCreateInfo->renderPass)->getSubpass(pCreateInfo->subpass);
_inputAttachmentIsDSAttachment = subpass->isInputAttachmentDepthStencilAttachment();
}

// Render pipeline state. Do this as early as possible, to fail fast if pipeline requires a fail on cache-miss.
initMTLRenderPipelineState(pCreateInfo, reflectData, pPipelineFB, pVertexSS, pVertexFB, pTessCtlSS, pTessCtlFB, pTessEvalSS, pTessEvalFB, pFragmentSS, pFragmentFB);
if ( !_hasValidMTLPipelineStates ) { return; }
Expand Down Expand Up @@ -1332,6 +1342,8 @@ static MTLVertexFormat mvkAdjustFormatVectorToSize(MTLVertexFormat format, uint3
shaderConfig.options.mslOptions.capture_output_to_buffer = false;
shaderConfig.options.mslOptions.fixed_subgroup_size = mvkIsAnyFlagEnabled(pFragmentSS->flags, VK_PIPELINE_SHADER_STAGE_CREATE_ALLOW_VARYING_SUBGROUP_SIZE_BIT_EXT) ? 0 : mtlFeats.maxSubgroupSize;
shaderConfig.options.mslOptions.check_discarded_frag_stores = true;
shaderConfig.options.mslOptions.force_fragment_with_side_effects_execution = true;
shaderConfig.options.mslOptions.input_attachment_is_ds_attachment = _inputAttachmentIsDSAttachment;
if (mtlFeats.needsSampleDrefLodArrayWorkaround) {
shaderConfig.options.mslOptions.sample_dref_lod_array_as_grad = true;
}
Expand Down Expand Up @@ -2580,6 +2592,8 @@ void serialize(Archive & archive, CompilerMSL::Options& opt) {
opt.force_sample_rate_shading,
opt.manual_helper_invocation_updates,
opt.check_discarded_frag_stores,
opt.force_fragment_with_side_effects_execution,
opt.input_attachment_is_ds_attachment,
opt.sample_dref_lod_array_as_grad,
opt.replace_recursive_inputs,
opt.agx_manual_cube_grad_fixup);
Expand Down
8 changes: 6 additions & 2 deletions MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ class MVKRenderSubpass : public MVKBaseObject {
bool isColorAttachmentAlsoInputAttachment(uint32_t colorAttIdx);

/** Returns whether or not the depth attachment is being used. */
bool isDepthAttachmentUsed() { return _depthAttachment.attachment != VK_ATTACHMENT_UNUSED; }
bool isDepthAttachmentUsed() const { return _depthAttachment.attachment != VK_ATTACHMENT_UNUSED; }

/** Returns whether or not the stencil attachment is being used. */
bool isStencilAttachmentUsed() { return _stencilAttachment.attachment != VK_ATTACHMENT_UNUSED; }
bool isStencilAttachmentUsed() const { return _stencilAttachment.attachment != VK_ATTACHMENT_UNUSED; }

/** Return the depth attachment format. */
VkFormat getDepthFormat();
Expand All @@ -90,6 +90,9 @@ class MVKRenderSubpass : public MVKBaseObject {
/** Returns whether or not this is a multiview subpass. */
bool isMultiview() const { return _pipelineRenderingCreateInfo.viewMask != 0; }

/** Returns whether or not the depth stencil is being used as input attachment */
bool isInputAttachmentDepthStencilAttachment() const { return _isInputAttachmentDepthStencilAttachment; }

/** Returns the multiview view mask. */
uint32_t getViewMask() const { return _pipelineRenderingCreateInfo.viewMask; }

Expand Down Expand Up @@ -177,6 +180,7 @@ class MVKRenderSubpass : public MVKBaseObject {
VkResolveModeFlagBits _stencilResolveMode = VK_RESOLVE_MODE_NONE;
VkSampleCountFlagBits _defaultSampleCount = VK_SAMPLE_COUNT_1_BIT;
uint32_t _subpassIndex;
bool _isInputAttachmentDepthStencilAttachment = false;
};


Expand Down
15 changes: 15 additions & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.mm
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,21 @@
_pipelineRenderingCreateInfo.pColorAttachmentFormats = _colorAttachmentFormats.data();
_pipelineRenderingCreateInfo.depthAttachmentFormat = getDepthFormat();
_pipelineRenderingCreateInfo.stencilAttachmentFormat = getStencilFormat();

// Needed to understand if we need to force the depth/stencil write to post fragment execution
// since Metal may try to do the write pre fragment exeuction which is against Vulkan
bool depthAttachmentUsed = isDepthAttachmentUsed();
bool stencilAttachmentUsed = isStencilAttachmentUsed();
for (uint32_t i = 0u; i < _inputAttachments.size(); ++i) {
bool isDepthInput = depthAttachmentUsed && (_inputAttachments[i].attachment == _depthAttachment.attachment) &&
(_inputAttachments[i].aspectMask & _depthAttachment.aspectMask);
bool isStencilInput = stencilAttachmentUsed && (_inputAttachments[i].attachment == _stencilAttachment.attachment) &&
(_inputAttachments[i].aspectMask & _stencilAttachment.aspectMask);
if (isDepthInput || isStencilInput) {
_isInputAttachmentDepthStencilAttachment = true;
break;
}
}
}

static const VkAttachmentReference2 _unusedAttachment = {VK_STRUCTURE_TYPE_ATTACHMENT_REFERENCE_2, nullptr, VK_ATTACHMENT_UNUSED, VK_IMAGE_LAYOUT_UNDEFINED, 0};
Expand Down

0 comments on commit c6373b8

Please sign in to comment.