Merge pull request #1408 from billhollings/handle-bad-api-pointers
Properly ignore non-null pipeline creation pointers that should be ignored.
diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h b/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h
index f262b82..61c9fdf 100644
--- a/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h
+++ b/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.h
@@ -398,6 +398,9 @@
bool _needsFragmentBufferSizeBuffer = false;
bool _needsFragmentDynamicOffsetBuffer = false;
bool _needsFragmentViewRangeBuffer = false;
+ bool _isRasterizing = false;
+ bool _isRasterizingColor = false;
+ bool _isRasterizingDepthStencil = false;
};
diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm b/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm
index b6bde21..b47ad78 100644
--- a/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm
+++ b/MoltenVK/MoltenVK/GPUObjects/MVKPipeline.mm
@@ -365,6 +365,13 @@
const VkGraphicsPipelineCreateInfo* pCreateInfo) :
MVKPipeline(device, pipelineCache, (MVKPipelineLayout*)pCreateInfo->layout, parent) {
+ // Determine rasterization early, as various other structs are validated and interpreted in this context.
+ MVKRenderPass* mvkRendPass = (MVKRenderPass*)pCreateInfo->renderPass;
+ MVKRenderSubpass* mvkRenderSubpass = mvkRendPass->getSubpass(pCreateInfo->subpass);
+ _isRasterizing = !isRasterizationDisabled(pCreateInfo);
+ _isRasterizingColor = _isRasterizing && mvkRenderSubpass->hasColorAttachments();
+ _isRasterizingDepthStencil = _isRasterizing && mvkRenderSubpass->hasDepthStencilAttachment();
+
// Get the tessellation shaders, if present. Do this now, because we need to extract
// reflection data from them that informs everything else.
for (uint32_t i = 0; i < pCreateInfo->stageCount; i++) {
@@ -405,8 +412,8 @@
}
}
- // Blending
- if (pCreateInfo->pColorBlendState) {
+ // Blending - must ignore allowed bad pColorBlendState pointer if rasterization disabled or no color attachments
+ if (_isRasterizingColor && pCreateInfo->pColorBlendState) {
memcpy(&_blendConstants, &pCreateInfo->pColorBlendState->blendConstants, sizeof(_blendConstants));
}
@@ -421,9 +428,9 @@
}
}
- // Tessellation
+ // Tessellation - must ignore allowed bad pTessellationState pointer if not tess pipeline
_outputControlPointCount = reflectData.numControlPoints;
- mvkSetOrClear(&_tessInfo, pCreateInfo->pTessellationState);
+ mvkSetOrClear(&_tessInfo, (_pTessCtlSS && _pTessEvalSS) ? pCreateInfo->pTessellationState : nullptr);
// Rasterization
_mtlCullMode = MTLCullModeNone;
@@ -448,10 +455,11 @@
initMTLRenderPipelineState(pCreateInfo, reflectData);
// Depth stencil content - clearing will disable depth and stencil testing
- mvkSetOrClear(&_depthStencilInfo, pCreateInfo->pDepthStencilState);
+ // Must ignore allowed bad pDepthStencilState pointer if rasterization disabled or no depth attachment
+ mvkSetOrClear(&_depthStencilInfo, _isRasterizingDepthStencil ? pCreateInfo->pDepthStencilState : nullptr);
- // Viewports and scissors
- auto pVPState = pCreateInfo->pViewportState;
+ // Viewports and scissors - must ignore allowed bad pViewportState pointer if rasterization is disabled
+ auto pVPState = _isRasterizing ? pCreateInfo->pViewportState : nullptr;
if (pVPState) {
uint32_t vpCnt = pVPState->viewportCount;
_viewports.reserve(vpCnt);
@@ -920,7 +928,7 @@
shaderConfig.options.mslOptions.dynamic_offsets_buffer_index = _dynamicOffsetBufferIndex.stages[kMVKShaderStageVertex];
shaderConfig.options.mslOptions.view_mask_buffer_index = _viewRangeBufferIndex.stages[kMVKShaderStageVertex];
shaderConfig.options.mslOptions.capture_output_to_buffer = false;
- shaderConfig.options.mslOptions.disable_rasterization = isRasterizationDisabled(pCreateInfo);
+ shaderConfig.options.mslOptions.disable_rasterization = !_isRasterizing;
addVertexInputToShaderConversionConfig(shaderConfig, pCreateInfo);
MVKMTLFunction func = ((MVKShaderModule*)_pVertexSS->module)->getMTLFunction(&shaderConfig, _pVertexSS->pSpecializationInfo, _pipelineCache);
@@ -1108,7 +1116,7 @@
shaderConfig.options.mslOptions.buffer_size_buffer_index = _bufferSizeBufferIndex.stages[kMVKShaderStageTessEval];
shaderConfig.options.mslOptions.dynamic_offsets_buffer_index = _dynamicOffsetBufferIndex.stages[kMVKShaderStageTessEval];
shaderConfig.options.mslOptions.capture_output_to_buffer = false;
- shaderConfig.options.mslOptions.disable_rasterization = isRasterizationDisabled(pCreateInfo);
+ shaderConfig.options.mslOptions.disable_rasterization = !_isRasterizing;
addPrevStageOutputToShaderConversionConfig(shaderConfig, tcOutputs);
MVKMTLFunction func = ((MVKShaderModule*)_pTessEvalSS->module)->getMTLFunction(&shaderConfig, _pTessEvalSS->pSpecializationInfo, _pipelineCache);
@@ -1157,7 +1165,7 @@
shaderConfig.options.entryPointName = _pFragmentSS->pName;
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 : _device->_pMetalFeatures->maxSubgroupSize;
- if (pCreateInfo->pMultisampleState) {
+ if (_isRasterizing && pCreateInfo->pMultisampleState) { // Must ignore allowed bad pMultisampleState pointer if rasterization disabled
if (pCreateInfo->pMultisampleState->pSampleMask && pCreateInfo->pMultisampleState->pSampleMask[0] != 0xffffffff) {
shaderConfig.options.mslOptions.additional_fixed_sample_mask = pCreateInfo->pMultisampleState->pSampleMask[0];
}
@@ -1444,9 +1452,9 @@
: mvkMTLPrimitiveTopologyClassFromVkPrimitiveTopology(pCreateInfo->pInputAssemblyState->topology);
}
- // Color attachments
+ // Color attachments - must ignore bad pColorBlendState pointer if rasterization is disabled or subpass has no color attachments
uint32_t caCnt = 0;
- if (pCreateInfo->pColorBlendState) {
+ if (_isRasterizingColor && pCreateInfo->pColorBlendState) {
for (uint32_t caIdx = 0; caIdx < pCreateInfo->pColorBlendState->attachmentCount; caIdx++) {
const VkPipelineColorBlendAttachmentState* pCA = &pCreateInfo->pColorBlendState->pAttachments[caIdx];
@@ -1493,8 +1501,8 @@
colorDesc.writeMask = MTLColorWriteMaskNone;
}
- // Multisampling
- if (pCreateInfo->pMultisampleState) {
+ // Multisampling - must ignore allowed bad pMultisampleState pointer if rasterization disabled
+ if (_isRasterizing && pCreateInfo->pMultisampleState) {
plDesc.sampleCount = mvkSampleCountFromVkSampleCountFlagBits(pCreateInfo->pMultisampleState->rasterizationSamples);
mvkRenderSubpass->setDefaultSampleCount(pCreateInfo->pMultisampleState->rasterizationSamples);
plDesc.alphaToCoverageEnabled = pCreateInfo->pMultisampleState->alphaToCoverageEnable;
@@ -1507,8 +1515,9 @@
const VkGraphicsPipelineCreateInfo* pCreateInfo,
const SPIRVTessReflectionData& reflectData) {
+ // Tessellation - must ignore allowed bad pTessellationState pointer if not tess pipeline
VkPipelineTessellationDomainOriginStateCreateInfo* pTessDomainOriginState = nullptr;
- if (pCreateInfo->pTessellationState) {
+ if (isTessellationPipeline() && pCreateInfo->pTessellationState) {
for (const auto* next = (VkBaseInStructure*)pCreateInfo->pTessellationState->pNext; next; next = next->pNext) {
switch (next->sType) {
case VK_STRUCTURE_TYPE_PIPELINE_TESSELLATION_DOMAIN_ORIGIN_STATE_CREATE_INFO:
@@ -1550,9 +1559,10 @@
// fragment shader outputs a color value without a corresponding color attachment.
// However, if alpha-to-coverage is enabled, we must enable the fragment shader first color output,
// even without a color attachment present or in use, so that coverage can be calculated.
- bool hasA2C = pCreateInfo->pMultisampleState && pCreateInfo->pMultisampleState->alphaToCoverageEnable;
+ // Must ignore allowed bad pMultisampleState pointer if rasterization disabled
+ bool hasA2C = _isRasterizing && pCreateInfo->pMultisampleState && pCreateInfo->pMultisampleState->alphaToCoverageEnable;
shaderConfig.options.mslOptions.enable_frag_output_mask = hasA2C ? 1 : 0;
- if (pCreateInfo->pColorBlendState) {
+ if (_isRasterizingColor && pCreateInfo->pColorBlendState) {
for (uint32_t caIdx = 0; caIdx < pCreateInfo->pColorBlendState->attachmentCount; caIdx++) {
if (mvkRenderSubpass->isColorAttachmentUsed(caIdx)) {
mvkEnableFlags(shaderConfig.options.mslOptions.enable_frag_output_mask, 1 << caIdx);
diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.h b/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.h
index 18d2d24..76f83a6 100644
--- a/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.h
+++ b/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.h
@@ -47,13 +47,19 @@
MVKVulkanAPIObject* getVulkanAPIObject() override;
/** Returns the parent render pass of this subpass. */
- inline MVKRenderPass* getRenderPass() { return _renderPass; }
+ MVKRenderPass* getRenderPass() { return _renderPass; }
/** Returns the index of this subpass in its parent render pass. */
- inline uint32_t getSubpassIndex() { return _subpassIndex; }
+ uint32_t getSubpassIndex() { return _subpassIndex; }
+
+ /** Returns whether this subpass has any color attachments. */
+ bool hasColorAttachments();
+
+ /** Returns whether this subpass has a depth/stencil attachment. */
+ bool hasDepthStencilAttachment() { return _depthStencilAttachment.attachment != VK_ATTACHMENT_UNUSED; }
/** Returns the number of color attachments, which may be zero for depth-only rendering. */
- inline uint32_t getColorAttachmentCount() { return uint32_t(_colorAttachments.size()); }
+ uint32_t getColorAttachmentCount() { return uint32_t(_colorAttachments.size()); }
/** Returns the format of the color attachment at the specified index. */
VkFormat getColorAttachmentFormat(uint32_t colorAttIdx);
@@ -74,7 +80,7 @@
bool isMultiview() const { return _viewMask != 0; }
/** Returns the total number of views to be rendered. */
- inline uint32_t getViewCount() const { return __builtin_popcount(_viewMask); }
+ uint32_t getViewCount() const { return __builtin_popcount(_viewMask); }
/** Returns the number of Metal render passes needed to render all views. */
uint32_t getMultiviewMetalPassCount() const;
diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.mm b/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.mm
index 332efd1..bced648 100644
--- a/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.mm
+++ b/MoltenVK/MoltenVK/GPUObjects/MVKRenderPass.mm
@@ -35,6 +35,13 @@
MVKVulkanAPIObject* MVKRenderSubpass::getVulkanAPIObject() { return _renderPass->getVulkanAPIObject(); };
+bool MVKRenderSubpass::hasColorAttachments() {
+ for (auto& ca : _colorAttachments) {
+ if (ca.attachment != VK_ATTACHMENT_UNUSED) { return true; }
+ }
+ return false;
+}
+
VkFormat MVKRenderSubpass::getColorAttachmentFormat(uint32_t colorAttIdx) {
if (colorAttIdx < _colorAttachments.size()) {
uint32_t rpAttIdx = _colorAttachments[colorAttIdx].attachment;