Skip to content

Commit

Permalink
Apply SPIRV-Cross workarounds for Metal bugs
Browse files Browse the repository at this point in the history
2 workarounds introduced:
 - Fragments with side effects will now execute following Vulkan
 - Depth/Stencil write postponed to post fragment execution when
   input attachment and depth/stencil attachment reference the
   same resource
  • Loading branch information
aitor-lunarg committed Jun 5, 2024
1 parent 100747d commit 8242fd9
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 _input_attachment_is_ds_attachment = 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);
_input_attachment_is_ds_attachment = 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 = _input_attachment_is_ds_attachment;
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
7 changes: 5 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;

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

Expand Down
16 changes: 16 additions & 0 deletions MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.mm
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,22 @@
return VK_SAMPLE_COUNT_1_BIT;
}

bool MVKRenderSubpass::isInputAttachmentDepthStencilAttachment() const {
bool is_depth_attachment_used = isDepthAttachmentUsed();
bool is_stencil_attachment_used = isStencilAttachmentUsed();
if (!is_depth_attachment_used && !is_stencil_attachment_used) return false;

for (uint32_t i = 0u; i < _inputAttachments.size(); ++i) {
bool is_depth_input = is_depth_attachment_used && (_inputAttachments[i].attachment == _depthAttachment.attachment) &&
(_inputAttachments[i].aspectMask & _depthAttachment.aspectMask);
bool is_stencil_input = is_stencil_attachment_used && (_inputAttachments[i].attachment == _stencilAttachment.attachment) &&
(_inputAttachments[i].aspectMask & _stencilAttachment.aspectMask);
if (is_depth_input || is_stencil_input)
return true;
}
return false;
}

// Get the portion of the view mask that will be rendered in the specified Metal render pass.
uint32_t MVKRenderSubpass::getViewMaskGroupForMetalPass(uint32_t passIdx) {
if (!_pipelineRenderingCreateInfo.viewMask) { return 0; }
Expand Down

0 comments on commit 8242fd9

Please sign in to comment.