Validation of stencil ops complaining about read-only layout while writemask is 0

[ShabbyX]

Since 0e4dc22c2, ANGLE is hitting a validation error (VUID VUID-vkCmdDraw-None-06887) in this condition:

• Pipeline's stencil ops are not KEEP
• Stencil test is enabled
• Stencil write mask is 0 (dynamic state)

The complaint is:

The layout (VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL) of the stencil aspect of the depth/stencil attachment in the render pass is read only but not all stencil ops are VK_STENCIL_OP_KEEP. front = { .failOp = VK_STENCIL_OP_REPLACE, .passOp = VK_STENCIL_OP_REPLACE , .depthFailOp = VK_STENCIL_OP_REPLACE } back = { .failOp = VK_STENCIL_OP_REPLACE, .passOp = VK_STENCIL_OP_REPLACE, .depthFailOp = VK_STENCIL_OP_REPLACE } The Vulkan spec states: If the current render pass instance uses a depth/stencil attachment with a read-only layout for the stencil aspect and stencil test is enabled, all stencil ops must be VK_STENCIL_OP_KEEP

However, ANGLE's behavior is logically correct; all write is masked off, so it shouldn't matter what the ops are. @spencer-lunarg should the VU be fixed?

[spencer-lunarg]

related issue: https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/4137 - this VUIDs were added in response to your internal issue https://gitlab.khronos.org/vulkan/vulkan/-/issues/3110 (for future tracking references)

I think the issue, in your case at least, is that VK_DYNAMIC_STATE_STENCIL_WRITE_MASK is not being taken into account... agree that this seems like the VUID needs a spec update, but can get a pending Validation Layer changes going orthogonally too

[spencer-lunarg]

The 1.3.237 spec fixed the language, just need to get a test done and can get a fix pushed up soon

[ShabbyX]

So I noticed this lying around and tried to remove the VU suppressions in https://chromium-review.googlesource.com/c/angle/angle/+/4103721, and I still see failures coming in.

Basically, ANGLE doesn't care about e.g. stencil ops when stencil write mask is 0 (because whatever the ops are, the result is the same). Did we reach a conclusion at some point whether that's correct or not? Does spec require all dynamic state to be set correctly regardless of whether they are effective?

[spencer-lunarg]

The check is guarded with

 const bool write_mask_enabled = (front.writeMask != 0) && (back.writeMask != 0);

 if (!all_keep_op && write_mask_enabled) {

and these are gathered from VK_DYNAMIC_STATE_STENCIL_WRITE_MASK so curious what is still wrong, you have a test I can run to try and reproduce

[ShabbyX]

Yes sure, try ./angle_end2end_tests --gtest_filter=FramebufferTest_ES31.DepthStencilResolveThenChangeAttachmentThenResolveAgain/ES3_1_Vulkan (after unsuppressing the VU failure, you can take https://chromium-review.googlesource.com/c/angle/angle/+/4103721).

The first failure is:

[ VUID-vkCmdDraw-None-06887 ] Object 0: handle = Graphics Pipeline 312, type = VK_OBJECT_TYPE_PIPELINE; Object 1: handle = Render Pass 324, type = VK_OBJECT_TYPE_RENDER_PASS; Object 2: handle = Command Buffer 168, type = VK_OBJECT_TYPE_COMMAND_BUFFER; | MessageID = 0x1044e121 | vkCmdDraw(): The layout (VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL) of the stencil aspect of the depth/stencil attachment in the render pass is read only but not all stencil ops are VK_STENCIL_OP_KEEP. front = { .failOp = VK_STENCIL_OP_REPLACE, .passOp = VK_STENCIL_OP_REPLACE , .depthFailOp = VK_STENCIL_OP_REPLACE } back = { .failOp = VK_STENCIL_OP_REPLACE, .passOp = VK_STENCIL_OP_REPLACE, .depthFailOp = VK_STENCIL_OP_REPLACE } The Vulkan spec states: If the current render pass instance uses a depth/stencil attachment with a read-only layout for the stencil aspect, both front and back writeMask are not zero, and stencil test is enabled, all stencil ops must be VK_STENCIL_OP_KEEP (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-vkCmdDraw-None-06887)

And it's on a draw call that has this dynamic state:

Rasterizer discard is not enabled ;)

[ShabbyX]

Ok I guess VVL is checking for write mask 0, but not whether stencil testing is enabled in the first place?

[spencer-lunarg]

reopening issue, currently in "get SDK out the door mode" and will have L1 mental cache space for Dynamic State soon