Emit primitiveRestartEnable disabled warning only for strip topology.

- Move check and warning to MVKRenderingCommandEncoderState.
- Pass primitiveRestartEnable to MVKRenderingCommandEncoderState.
- Warn only if primitiveRestartEnable disabled and strip topology is used.
This commit is contained in:
Bill Hollings 2023-10-18 09:58:11 -04:00
parent 37268cadba
commit 37e4fefe5f
6 changed files with 28 additions and 17 deletions

View File

@ -584,6 +584,8 @@ public:
protected:
MVKCommandTypePool<MVKCommand>* getTypePool(MVKCommandPool* cmdPool) override;
VkBool32 _primitiveRestartEnable;
};

View File

@ -524,19 +524,13 @@ void MVKCmdSetPrimitiveTopology::encode(MVKCommandEncoder* cmdEncoder) {
VkResult MVKCmdSetPrimitiveRestartEnable::setContent(MVKCommandBuffer* cmdBuff,
VkBool32 primitiveRestartEnable) {
// Validate
// In Metal, primitive restart cannot be disabled.
// Just issue warning here, as it is very likely the app is not actually expecting
// to use primitive restart at all, and is just setting this as a "just-in-case",
// and forcing an error here would be unexpected to the app (including CTS).
if ( !primitiveRestartEnable ) {
reportWarning(VK_ERROR_FEATURE_NOT_PRESENT, "Metal does not support disabling primitive restart.");
}
_primitiveRestartEnable = primitiveRestartEnable;
return VK_SUCCESS;
}
void MVKCmdSetPrimitiveRestartEnable::encode(MVKCommandEncoder* cmdEncoder) {}
void MVKCmdSetPrimitiveRestartEnable::encode(MVKCommandEncoder* cmdEncoder) {
cmdEncoder->_renderingState.setPrimitiveRestartEnable(_primitiveRestartEnable, true);
}
#pragma mark -

View File

@ -312,6 +312,8 @@ public:
void setViewports(const MVKArrayRef<VkViewport> viewports, uint32_t firstViewport, bool isDynamic);
void setScissors(const MVKArrayRef<VkRect2D> scissors, uint32_t firstScissor, bool isDynamic);
void setPrimitiveRestartEnable(VkBool32 primitiveRestartEnable, bool isDynamic);
void setRasterizerDiscardEnable(VkBool32 rasterizerDiscardEnable, bool isDynamic);
void beginMetalRenderPass() override;
@ -345,6 +347,7 @@ protected:
MVKRenderStateFlags _dirtyStates;
MVKRenderStateFlags _modifiedStates;
bool _mtlDepthBiasEnable[StateScope::Count] = {};
bool _mtlPrimitiveRestartEnable[StateScope::Count] = {};
bool _mtlRasterizerDiscardEnable[StateScope::Count] = {};
bool _cullBothFaces[StateScope::Count] = {};
};

View File

@ -436,6 +436,11 @@ void MVKRenderingCommandEncoderState::setScissors(const MVKArrayRef<VkRect2D> sc
setContent(Scissors);
}
void MVKRenderingCommandEncoderState::setPrimitiveRestartEnable(VkBool32 primitiveRestartEnable, bool isDynamic) {
bool mtlPrimitiveRestartEnable = static_cast<bool>(primitiveRestartEnable);
setContent(PrimitiveRestartEnable);
}
void MVKRenderingCommandEncoderState::setRasterizerDiscardEnable(VkBool32 rasterizerDiscardEnable, bool isDynamic) {
bool mtlRasterizerDiscardEnable = static_cast<bool>(rasterizerDiscardEnable);
setContent(RasterizerDiscardEnable);
@ -473,6 +478,17 @@ void MVKRenderingCommandEncoderState::encodeImpl(uint32_t stage) {
[rendEnc setStencilFrontReferenceValue: sr.frontFaceValue backReferenceValue: sr.backFaceValue];
}
// Validate
// In Metal, primitive restart cannot be disabled.
// Just issue warning here, as it is very likely the app is not actually expecting
// to use primitive restart at all, and is just setting this as a "just-in-case",
// and forcing an error here would be unexpected to the app (including CTS).
auto mtlPrimType = getPrimitiveType();
if (isDirty(PrimitiveRestartEnable) && !getContent(PrimitiveRestartEnable) &&
(mtlPrimType == MTLPrimitiveTypeTriangleStrip || mtlPrimType == MTLPrimitiveTypeLineStrip)) {
reportWarning(VK_ERROR_FEATURE_NOT_PRESENT, "Metal does not support disabling primitive restart.");
}
if (isDirty(Viewports)) {
auto& mtlViewports = getContent(Viewports);
if (_cmdEncoder->_pDeviceFeatures->multiViewport) {

View File

@ -427,6 +427,7 @@ protected:
uint32_t _tessCtlPatchOutputBufferIndex = 0;
uint32_t _tessCtlLevelBufferIndex = 0;
bool _primitiveRestartEnable = true;
bool _hasRasterInfo = false;
bool _needsVertexSwizzleBuffer = false;
bool _needsVertexBufferSizeBuffer = false;

View File

@ -295,6 +295,7 @@ void MVKGraphicsPipeline::encode(MVKCommandEncoder* cmdEncoder, uint32_t stage)
// Rasterization
cmdEncoder->_renderingState.setPrimitiveTopology(_vkPrimitiveTopology, false);
cmdEncoder->_renderingState.setPrimitiveRestartEnable(_primitiveRestartEnable, false);
cmdEncoder->_renderingState.setBlendConstants(_blendConstants, false);
cmdEncoder->_renderingState.setStencilReferenceValues(_depthStencilInfo);
cmdEncoder->_renderingState.setViewports(_viewports.contents(), 0, false);
@ -507,13 +508,7 @@ MVKGraphicsPipeline::MVKGraphicsPipeline(MVKDevice* device,
? pCreateInfo->pInputAssemblyState->topology
: VK_PRIMITIVE_TOPOLOGY_POINT_LIST);
// In Metal, primitive restart cannot be disabled.
// Just issue warning here, as it is very likely the app is not actually expecting
// to use primitive restart at all, and is just setting this as a "just-in-case",
// and forcing an error here would be unexpected to the app (including CTS).
if (pCreateInfo->pInputAssemblyState && !pCreateInfo->pInputAssemblyState->primitiveRestartEnable) {
reportWarning(VK_ERROR_FEATURE_NOT_PRESENT, "vkCreateGraphicsPipeline(): Metal does not support disabling primitive restart.");
}
_primitiveRestartEnable = pCreateInfo->pInputAssemblyState ? pCreateInfo->pInputAssemblyState->primitiveRestartEnable : true;
// Rasterization
_hasRasterInfo = mvkSetOrClear(&_rasterInfo, pCreateInfo->pRasterizationState);